-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix(perps): enforce 5 significant figure limit for TP/SL price inputs #24169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
… significant figures
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
|
Do not merge, this approach needs to be confirmed |
|
| ) | ||
| return; | ||
|
|
||
| if ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent price rounding between button and text handlers
The PR adds roundToSignificantFigures to the percentage button handlers (lines 725-727 and 779-781) but not to handleTakeProfitPercentageChange and handleStopLossPercentageChange. When a user types in the percentage input field, calculatePriceForRoE can return prices with up to 8 decimal places for small prices, and these are set directly via setTakeProfitPrice(price.toString()) without rounding. This allows users to bypass the 5 significant figures limit by typing percentages instead of prices, creating inconsistent behavior between button clicks and text input.
Additional Locations (1)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may want to validate that we round correctly when pressing the percent calculation buttons directly as well as when we directly update the trigger price input field
| isValidStopLossPercentage, | ||
| sanitizePercentageInput, | ||
| countSignificantFigures, | ||
| hasExceededSignificantFigures, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New utility function lacks required unit tests (Bugbot Rules)
The roundToSignificantFigures function is introduced and used in production code (button handlers at lines 726 and 780) but has zero test coverage. The test file imports countSignificantFigures and hasExceededSignificantFigures but not roundToSignificantFigures. Per the BUGBOT_RULES requiring mandatory test coverage: "EVERY component MUST test: Happy path, Edge cases, Error conditions, Different code paths." This function has 6 distinct code paths (empty/whitespace, NaN/zero, integers, negative allowed decimals, within limit, needs rounding) - none are tested.
Additional Locations (1)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes sense to include unit tests for roundToSignificantFigures as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the PR includes new utility functions to prevent a user from entering more than 5 significant digits when inputing TP/SL. But, we're still supporting asset pairs with more than 5 significant digits like MON PENGU PUMP etc.
So I run into a situation where I can't set a TP/SL to the same precision as the listed asset pair.
Are we sure that we only want to support 5 significant digits? According to the docs, it appears that we can actually support more than 5 significant digits in certain cases. https://hyperliquid.gitbook.io/hyperliquid-docs/for-developers/api/tick-and-lot-size#perp-price-examples
Adding this comment here for viz, but I'm happy to chat about this more in Slack
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsAll changes are contained within the Perps (Perpetuals trading) feature directory (
These changes are focused on improving the Take Profit/Stop Loss (TP/SL) form validation to comply with HyperLiquid API requirements (max 5 significant figures). The changes are well-scoped to the Perps feature with no impact on other wallet functionality. The risk is medium because while the changes are isolated, they affect trading functionality where incorrect validation could impact user orders. |
|
Actually, after reviewing this again, I think that the input validation is working as it should. It is actually valid to not be able to add a TP/SL to the same level of precision as the actual asset pair (still seems weird to me) I'm ready to approve this, but do think that the bugbot comments are legitimate |
| const [integerPart, decimalPart = ''] = normalized.split('.'); | ||
|
|
||
| // Count integer significant digits (without leading zeros) | ||
| const trimmedInteger = integerPart.replace(/^-?0+/, '') || ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent regex between counting and rounding functions
The PR description mentions fixing a regex bug from /^-?0+/ to /^-?0*/ in countSignificantFigures, but roundToSignificantFigures uses the old buggy pattern /^-?0+/ at line 123. The 0+ quantifier requires at least one zero to match, so for a negative number like -12.345, the regex fails to match and the minus sign remains, causing integerSigFigs to incorrectly include the minus sign character. This creates inconsistent significant figure counting between the two related functions. The correct regex /^-?0*/ (used in countSignificantFigures) matches zero or more zeros and properly strips the minus sign.



This PR addresses HyperLiquid API rejections that occur when TP/SL prices exceed 5 significant figures.
Problem:
Solution:
MAX_SIGNIFICANT_FIGURESconstant for centralized configurationPRICE_RANGES_UNIVERSAL)Technical changes:
countSignificantFigures,hasExceededSignificantFigures,roundToSignificantFigurescountSignificantFiguresthat incorrectly handled negative numbers (/^-?0+/→/^-?0*/)Changelog
CHANGELOG entry: Fixed TP/SL price input validation to respect HyperLiquid's 5 significant figure limit
Related issues
Fixes: https://consensyssoftware.atlassian.net/browse/TAT-2063
Manual testing steps
Screenshots/Recordings
Before
User was able to enter more decimals than supported
After
Simulator.Screen.Recording.-.iPhone.17.Pro.-.2026-01-05.at.12.16.56.mov
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Ensures TP/SL prices comply with HyperLiquid’s 5 significant-figure limit across input, calculations, and display.
MAX_SIGNIFICANT_FIGUREStoDECIMAL_PRECISION_CONFIGand utilities:countSignificantFigures,hasExceededSignificantFigures,roundToSignificantFiguresusePerpsTPSLForm(handleTakeProfitPriceChange/handleStopLossPriceChange), and rounds RoE-derived prices before formattingPerpsPositionCardto usePRICE_RANGES_UNIVERSALfor consistencytpslValidation.test.tsfor significant-figure counting/limits and edge casesWritten by Cursor Bugbot for commit b415c15. This will update automatically on new commits. Configure here.