Skip to content

Conversation

@runway-github
Copy link
Contributor

@runway-github runway-github bot commented Oct 24, 2025

Description

Problem

Users were blocked from closing positions when fees exceeded recoverable
value (receiveAmount <= 0), creating a financial trap where they
couldn't exit losing positions. This particularly affected accounts that
lost all available margin in the position after price movement.

Solution

Changed negative receive amount validation from a blocking error to a
non-blocking warning, allowing users to always close positions while
still informing them of the cost.

Changes

usePerpsClosePositionValidation.ts: Moved receiveAmount < 0 check from
errors to warnings array
PerpsClosePositionView.tsx: Removed receiveAmount <= 0 from button
disable logic
en.json: Added negative_receive_warning locale string
usePerpsClosePositionValidation.test.ts: Updated test to expect warning
instead of error

Impact

✅ Users can now always exit positions (critical for risk management)
✅ Clear warning can be shown when closing will cost money (we are not
currently showing warnings)

Changelog

CHANGELOG entry: null

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/TAT-1887
Fixes: #21624

Manual testing steps

*** You may need to mock receiveAmount to simulate less than 0 on main
and then compare with this branch

Feature: close position disabled when receiveAmount <0

  Scenario: user creates a btc position at the min amount
    Given user loses all of the margin in their position to the point of have nothing left to get back
    When user attempts to close their position at total loss <0 
    Then user should be able to close that position

Screenshots/Recordings

Before

Message from Matthieu Saint Olive in
#tmp-beta-bip44-mobile

After

n/a position can close

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

Converts negative receive amount from blocking error to non-blocking warning so users can close positions; updates validation, button disable logic, tests, and locale strings.

  • Perps Close Position UI:
    • Remove receiveAmount <= 0 from confirm button isDisabled logic in PerpsClosePositionView.tsx.
  • Validation Hook (usePerpsClosePositionValidation):
    • Change negative receiveAmount check from error to warning (negative_receive_warning with formatted amount).
    • Update validation docs and numbering; keep other validations unchanged.
  • Tests:
    • Adjust test to expect a warning (and still valid) for negative receiveAmount in usePerpsClosePositionValidation.test.ts.
  • Localization:
    • Add perps.close_position.negative_receive_warning string in en.json.

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

cfb4d79

…n receiveAmount <0 (#21622)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

### Problem

Users were blocked from closing positions when fees exceeded recoverable
value (receiveAmount <= 0), creating a financial trap where they
couldn't exit losing positions. This particularly affected accounts that
lost all available margin in the position after price movement.

### Solution

Changed negative receive amount validation from a blocking error to a
non-blocking warning, allowing users to always close positions while
still informing them of the cost.

### Changes
usePerpsClosePositionValidation.ts: Moved receiveAmount < 0 check from
errors to warnings array
PerpsClosePositionView.tsx: Removed receiveAmount <= 0 from button
disable logic
en.json: Added negative_receive_warning locale string
usePerpsClosePositionValidation.test.ts: Updated test to expect warning
instead of error

### Impact
✅ Users can now always exit positions (critical for risk management)
✅ Clear warning can be shown when closing will cost money (we are not
currently showing warnings)

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

## **Changelog**

<!--
If this PR is not End-User-Facing and should not show up in the
CHANGELOG, you can choose to either:
1. Write `CHANGELOG entry: null`
2. Label with `no-changelog`

If this PR is End-User-Facing, please write a short User-Facing
description in the past tense like:
`CHANGELOG entry: Added a new tab for users to see their NFTs`
`CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker`

(This helps the Release Engineer do their job more quickly and
accurately)
-->

CHANGELOG entry: null

## **Related issues**

Fixes: https://consensyssoftware.atlassian.net/browse/TAT-1887
Fixes: #21624

## **Manual testing steps**

*** You may need to mock receiveAmount to simulate less than 0 on main
and then compare with this branch

```gherkin
Feature: close position disabled when receiveAmount <0

  Scenario: user creates a btc position at the min amount
    Given user loses all of the margin in their position to the point of have nothing left to get back
    When user attempts to close their position at total loss <0 
    Then user should be able to close that position
```

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

[Message from Matthieu Saint Olive in
#tmp-beta-bip44-mobile](https://consensys.slack.com/archives/C09H8PG9FBJ/p1760716074716439)

<!-- [screenshots/recordings] -->

### **After**

n/a position can close

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> Negative receive amount is now a non-blocking warning (not an error);
confirm button no longer disabled by receiveAmount <= 0; tests and
locales updated.
> 
> - **Perps Close Position Validation**:
> - Downgrades `receiveAmount < 0` from error to warning with formatted
amount in `usePerpsClosePositionValidation`.
> - Updates validation docs/comments and preserves other error/warning
rules.
> - **UI**:
> - Removes `receiveAmount <= 0` from confirm button `isDisabled` logic
in `PerpsClosePositionView`.
> - **Tests**:
> - Adjusts test to expect valid state with warning for negative receive
amount.
> - **i18n**:
>   - Adds `perps.close_position.negative_receive_warning` string.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
7a15b64. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
@runway-github runway-github bot requested a review from a team as a code owner October 24, 2025 19:13
@github-actions
Copy link
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-bots Bot team (for MetaMask Bot, Runway Bot, etc.) label Oct 24, 2025
cursor[bot]

This comment was marked as outdated.

@sonarqubecloud
Copy link

Copy link
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

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

LGTM

@Cal-L Cal-L merged commit 2b39326 into release/7.58.0 Oct 27, 2025
78 of 79 checks passed
@Cal-L Cal-L deleted the runway-cherry-pick-7.58.0-1761333193 branch October 27, 2025 20:28
@github-actions github-actions bot locked and limited conversation to collaborators Oct 27, 2025
@metamaskbot metamaskbot added the release-7.58.0 Issue or pull request that will be included in release 7.58.0 label Oct 27, 2025
@metamaskbot
Copy link
Collaborator

No release label on PR. Adding release label release-7.58.0 on PR, as PR was cherry-picked in branch 7.58.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-7.58.0 Issue or pull request that will be included in release 7.58.0 size-S team-bots Bot team (for MetaMask Bot, Runway Bot, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants