fix(perps): Cannot use +/- to define TP/SL thresholds#41919
Merged
Conversation
Contributor
|
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. |
Contributor
Builds ready [52a4c14] [reused from 4ede4f5]
⚡ Performance Benchmarks (Total: 🟢 6 pass · 🟡 9 warn · 🔴 0 fail)
Bundle size diffs
|
- Preserve sign in formatRoePercent so SL below entry shows negative % - Unify percentToPrice/priceToPercent to use signed RoE convention (positive = profit direction, negative = loss direction) for both TP and SL - Support '+' prefix in isSignedDecimalInput for intermediate typing states - Update tests to reflect signed RoE convention
Contributor
✨ Files requiring CODEOWNER review ✨👨🔧 @MetaMask/perps (8 files, +183 -77)
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a4601a1. Configure here.
Contributor
Author
Worker reportFix Report — TAT-2947Branch: fix/tat-2947-fix-tpsl-sign-input Root CauseThree interrelated bugs:
FixAdopted unified signed RoE convention: positive = profitable direction, negative = loss direction. Both TP and SL use the same formula.
Files Changed
Validation
EvidenceSee Workflow Diagram (Mermaid) |
Contributor
Builds ready [a006655]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 8 warn · 🔴 0 fail)
Bundle size diffs
|
Contributor
Author
Worker reportNo report available. |
abretonc7s
changed the title
michalconsensys
approved these changes
Apr 20, 2026
|
Contributor
Builds ready [6388d60]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 8 warn · 🔴 0 fail)
Bundle size diffs
|
gambinish
approved these changes
Apr 22, 2026
metamaskbot
added
the
release-13.29.0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes a bug where users could not type
+or-to set TP/SL thresholds as signed RoE percentages. Three interrelated bugs:formatRoePercentstripped the sign,percentToPriceForEdit/percentToPriceused anisTPflag that inverted SL math instead of a unified signed convention, and the input regex rejected+as a leading character.After the fix: positive % = profitable direction, negative % = loss direction. Both TP and SL use the same formula. Typing
+15or-10is accepted and converts to the correct price.Changelog
CHANGELOG entry: Fixed a bug where typing
+or-in TP/SL percent fields was rejected or produced prices on the wrong side of the entry price.Related issues
Fixes: TAT-2947
Manual testing steps
+15— verify it is accepted and a price above entry appears.-5— verify it is accepted and a price below entry appears.Screenshots/Recordings
Before: + prefix rejected, SL sign inverted. After: +15 accepted in TP, -5 creates SL below entry.
Pre-merge author checklist
Pre-merge reviewer checklist
Validation Recipe
recipe.json
{ "title": "TAT-2947: TP/SL signed percent input — +/- sign acceptance and correct price derivation", "description": "Verifies that the TP/SL modal accepts + prefix and that - prefix sets prices on the correct side of entry. Requires an open ETH long position.", "initial_conditions": {}, "validate": { "workflow": { "pre_conditions": ["wallet.unlocked", "perps.feature_enabled"], "entry": "setup-dismiss-any-modal", "teardown": [ { "id": "teardown-close-modal", "action": "eval_sync", "expression": "(()=>{const closeBtn=document.querySelector('[data-testid=\"perps-update-tpsl-modal\"] [aria-label=\"Close\"]');if(closeBtn)closeBtn.click();return JSON.stringify({ok:true});})()", "assert": { "operator": "eq", "field": "ok", "value": true } } ], "nodes": { "setup-dismiss-any-modal": { "action": "eval_sync", "...": "..." }, "setup-navigate-market": { "action": "call", "ref": "perps/navigate-to-market-detail", "...": "..." }, "ac1-type-plus-in-tp": { "action": "eval_sync", "...": "types +15 in TP percent input" }, "ac1-assert-tp-value-accepted": { "action": "eval_sync", "assert": { "operator": "eq", "field": "accepted", "value": true } }, "ac2-type-minus-in-sl": { "action": "eval_sync", "...": "types -5 in SL percent input" }, "ac2-assert-sl-price-below-entry": { "action": "eval_sync", "assert": { "operator": "eq", "field": "belowEntry", "value": true } } } } } }Recipe Workflow
workflow.mmd
Note
Medium Risk
Changes TP/SL percent-to-price and price-to-percent math and input validation; incorrect sign handling could place TP/SL on the wrong side of entry for some directions/leverage combinations.
Overview
Fixes TP/SL RoE% inputs to use a signed convention across both order entry
AutoCloseSectionand theUpdateTPSLModalContent: positive values mean profit direction and negative values mean loss direction, for both long and short.Updates percent/price conversion helpers to drop the old
isTP-based sign inversion, preserves negative signs informatRoePercent(including avoiding-0), and expands percent input validation to accept leading+(including intermediate+/+.states). Tests are updated/added to cover signed SL values,+prefixes, and preset SL negation behavior.Reviewed by Cursor Bugbot for commit 6388d60. Bugbot is set up for automated code reviews on this repo. Configure here.