Skip to content

fix(perps): validate TP/SL prices, clear stale config, and block invalid orders cp-7.71.0#27791

Merged
michalconsensys merged 10 commits into
mainfrom
fix/perps-tpsl-validation
Mar 24, 2026
Merged

fix(perps): validate TP/SL prices, clear stale config, and block invalid orders cp-7.71.0#27791
michalconsensys merged 10 commits into
mainfrom
fix/perps-tpsl-validation

Conversation

@michalconsensys
Copy link
Copy Markdown
Contributor

@michalconsensys michalconsensys commented Mar 23, 2026

Description

Fixes three related bugs in the Perps order form involving Take Profit (TP) and Stop Loss (SL) prices that were restored from a previous session's pending trade configuration:

  1. Stale TP/SL persisted after order submission: The pendingTradeConfiguration was not cleared after a successful order, causing previously set TP/SL values to reappear on the next order form visit — even auto-submitting a stop loss the user never intended.

  2. TP/SL displayed as "off" despite being set: When the RoE calculation clamped to zero (e.g., the TP/SL price was on the wrong side of the current market price), the "Auto close" summary row showed "off" instead of the actual price. The TP/SL edit form, however, showed the correct value — a confusing inconsistency.

  3. No validation or blocking for invalid TP/SL direction: A restored TP/SL price that ended up on the wrong side of the market (e.g., take profit below entry for a long) was silently accepted and could be submitted, leading to immediate execution or unexpected behavior.

Changes

  • Call clearPendingTradeConfiguration after successful order execution to prevent stale TP/SL restoration.
  • Display the formatted price in the "Auto close" row when RoE rounds to zero, instead of showing "off".
  • Validate TP/SL prices against current market price and trade direction using existing isValidTakeProfitPrice / isValidStopLossPrice utilities.
  • Show inline error warnings when TP or SL is on the wrong side of the current price.
  • Disable the "Place order" button while TP/SL is invalid.

Changelog

CHANGELOG entry: Fixed a bug where stale Take Profit and Stop Loss prices could persist across orders and display incorrectly in the Perps order form

Related issues

Fixes: #27793

Manual testing steps

Feature: Perps order TP/SL validation

  Scenario: stale TP/SL is cleared after placing an order
    Given the user has a Perps position open with TP and SL set
    And the user navigates to the order form

    When the user places the order successfully
    And the user returns to the order form for the same asset
    Then the TP and SL fields should be empty (not restored from previous order)

  Scenario: TP/SL on the wrong side shows warning and blocks submission
    Given the user is on the Perps order form for a Long position
    And the user sets a Take Profit price below the current market price

    When the order form validates the TP price
    Then a warning is displayed: "Take profit must be above current price. Update or clear it to place the order."
    And the "Place order" button is disabled

  Scenario: TP/SL on the wrong side for Short position
    Given the user is on the Perps order form for a Short position
    And the user sets a Stop Loss price below the current market price

    When the order form validates the SL price
    Then a warning is displayed: "Stop loss must be above current price. Update or clear it to place the order."
    And the "Place order" button is disabled

  Scenario: TP/SL with zero RoE displays price instead of "off"
    Given the user is on the Perps order form with a TP or SL set
    And the TP/SL price results in an RoE that rounds to 0%

    When the "Auto close" summary row renders
    Then it displays the formatted price value instead of "off"

Screenshots/Recordings

N/A — validation logic and text changes only; no layout or visual design changes.

Before

N/A

After

Simulator Screenshot - iPhone 17 Pro Max - 2026-03-23 at 11 46 16

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Medium Risk
Changes the Perps order submission/validation path to block orders when TP/SL is on the wrong side and clears stored pending trade config after submission, which could affect order placement behavior. Risk is mitigated by added unit coverage for market vs limit reference price and button-disabled states.

Overview
Prevents Perps orders from being submitted with invalid TP/SL trigger prices by validating TP/SL against the appropriate reference price (current for market orders, entry/limit for limit orders), showing inline warnings, and disabling the Place order button when TP/SL is wrong-side.

Fixes TP/SL display inconsistencies by showing the formatted TP/SL price in the summary row when computed RoE clamps to 0% instead of rendering off, and clears PerpsController.clearPendingTradeConfiguration(asset) after successful submission to avoid restoring stale TP/SL on subsequent visits.

Adds new i18n strings for the wrong-side TP/SL warnings and expands PerpsOrderView tests to cover wrong-side validation, limit-order reference pricing, and the monochrome A/B button variant disablement.

Written by Cursor Bugbot for commit 7c10dc8. This will update automatically on new commits. Configure here.

…of market

When a restored TP/SL price ends up on the wrong side of the current
market price (e.g. take profit below entry for a long), show a warning
and disable the Place Order button. Also display the formatted price
instead of "off" when the RoE calculation clamps to zero.
@michalconsensys michalconsensys self-assigned this Mar 23, 2026
@github-actions
Copy link
Copy Markdown
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.

@metamaskbot metamaskbot added the team-perps Perps team label Mar 23, 2026
@michalconsensys michalconsensys changed the title fix(perps): validate TP/SL prices, clear stale config, and block invalid orders fix(perps): validate TP/SL prices, clear stale config, and block invalid orders cp-7.71.0 Mar 23, 2026
@michalconsensys michalconsensys marked this pull request as ready for review March 23, 2026 10:57
@michalconsensys michalconsensys requested a review from a team as a code owner March 23, 2026 10:57
Cover the new TP/SL validation logic, warning UI, and button disabled
state for both long/short positions and monochrome button A/B variant.
@github-actions github-actions Bot added size-M risk-medium Moderate testing recommended · Possible bug introduction risk and removed size-S labels Mar 23, 2026
// stale TP/SL values from being restored on the next order form visit
Engine.context.PerpsController?.clearPendingTradeConfiguration(
orderForm.asset,
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pending config cleared even when order execution fails

Medium Severity

clearPendingTradeConfiguration runs unconditionally after executeOrder, but executeOrder (from usePerpsOrderExecution) never throws — it catches all errors internally and resolves regardless of success or failure. This means the pending trade config (including TP/SL) is cleared even when the order fails on the exchange, contradicting the comment's stated intent of clearing only "after successful submission." If a user's order fails, their TP/SL settings are lost when they return to the order form.

Fix in Cursor Fix in Web

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intended

…s tests

Define RelatedAsset locally since @metamask/ai-controllers no longer
exports it, and simplify the usePerpsABTest mock to avoid spreading
unknown[] into a rest parameter.
@github-actions github-actions Bot added risk-medium Moderate testing recommended · Possible bug introduction risk and removed risk-medium Moderate testing recommended · Possible bug introduction risk labels Mar 23, 2026
Comment thread app/components/UI/Perps/Views/PerpsOrderView/PerpsOrderView.tsx
Copy link
Copy Markdown
Contributor

@geositta geositta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a suggestion regarding the bugbot finding. Please add a test that fails on current code and passes after the fix, proving limit order TP/SL is validated against entry price when entry and market prices differ.

Comment thread app/components/UI/Perps/Views/PerpsOrderView/PerpsOrderView.tsx Outdated
…t price

TP/SL wrong-side validation was comparing against the current market
price for all order types. For limit orders the reference should be the
limit (entry) price, matching the pattern already used for liquidation
price and TP/SL summary display in the same file.
@github-actions github-actions Bot added size-L risk-medium Moderate testing recommended · Possible bug introduction risk and removed size-M risk-medium Moderate testing recommended · Possible bug introduction risk labels Mar 24, 2026
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Comment thread app/components/UI/Perps/Views/PerpsOrderView/PerpsOrderView.tsx
The warning strings hardcoded "current price" but for limit orders the
validation runs against the entry (limit) price. Pass a dynamic priceType
param ("current" or "entry") matching the existing pattern in usePerpsTPSLForm.
Replace raw English strings ("current", "entry") with proper i18n calls
via strings('perps.tpsl.current') and strings('perps.tpsl.entry') in both
PerpsOrderView and usePerpsTPSLForm, ensuring the values are localizable.
@github-actions github-actions Bot removed the risk-medium Moderate testing recommended · Possible bug introduction risk label Mar 24, 2026
@github-actions github-actions Bot added risk-medium Moderate testing recommended · Possible bug introduction risk and removed risk-medium Moderate testing recommended · Possible bug introduction risk labels Mar 24, 2026
@github-actions github-actions Bot added risk-medium Moderate testing recommended · Possible bug introduction risk and removed risk-medium Moderate testing recommended · Possible bug introduction risk labels Mar 24, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokePerps, SmokeWalletPlatform, SmokeConfirmations
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: medium
  • AI Confidence: 90%
click to see 🤖 AI reasoning details

E2E Test Selection:
The changes are scoped to the Perps order view (PerpsOrderView.tsx) with the following modifications:

  1. TP/SL wrong-side price validation: New logic using isValidTakeProfitPrice and isValidStopLossPrice to validate that take profit and stop loss prices are on the correct side of the current/entry price. For limit orders, validation uses the limit price as reference instead of market price.

  2. Button disable logic: The Place Order button is now disabled when hasInvalidTPSL is true, preventing order submission with invalid TP/SL prices.

  3. Warning messages: New UI warnings displayed when TP/SL prices are on the wrong side, using new localization strings added to en.json.

  4. Post-order cleanup: After successful order submission, Engine.context.PerpsController?.clearPendingTradeConfiguration is called to prevent stale TP/SL values from being restored.

  5. TP/SL display fix: When RoE is 0, shows formatted price instead of "off".

Tag selection rationale:

  • SmokePerps: Directly tests Perps functionality including the Add Funds flow and order placement, which is the core area changed.
  • SmokeWalletPlatform: Required by SmokePerps tag description since Perps is a section inside the Trending tab.
  • SmokeConfirmations: Required by SmokePerps tag description since Add Funds deposits are on-chain transactions.

The changes are contained within the Perps feature area with no impact on shared navigation, modals, or other wallet components. The localization changes are Perps-specific strings only.

Performance Test Selection:
The changes are UI validation logic additions (warning display, button disable state) within the Perps order view. There are no changes to rendering performance, data loading, state management at scale, or any components that would affect app startup, account/network list rendering, or other performance-sensitive areas. No performance tests are warranted.

View GitHub Actions results

@github-actions
Copy link
Copy Markdown
Contributor

E2E Fixture Validation — Schema is up to date
18 value mismatches detected (expected — fixture represents an existing user).
View details

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@geositta geositta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding that limit order validation - looks good from my end.

@michalconsensys michalconsensys added this pull request to the merge queue Mar 24, 2026
Merged via the queue into main with commit ef3a973 Mar 24, 2026
104 checks passed
@michalconsensys michalconsensys deleted the fix/perps-tpsl-validation branch March 24, 2026 14:55
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 24, 2026
@metamaskbot metamaskbot added the release-7.72.0 Issue or pull request that will be included in release 7.72.0 label Mar 24, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-7.72.0 Issue or pull request that will be included in release 7.72.0 risk-medium Moderate testing recommended · Possible bug introduction risk size-L team-perps Perps team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Position auto close config not displayed in order screen

3 participants