Skip to content

Commit b2ac9a2

Browse files
authored
fix(perps): incorrect fee estimate when flipping a position (MetaMask#28013)
## **Description** The flip position confirmation sheet showed a fee estimate ~2x lower than the actual fee charged. A flip order is 2x position size (close current + open opposite), but the fee estimate was calculated on 1x notional. A secondary bug in `TradingService` applied the combined fee rate to 2x notional for the balance check, incorrectly blocking users with sufficient balance. ## **Changelog** CHANGELOG entry: Fixed flip position fee estimate being ~2x lower than actual fee charged ## **Related issues** Fixes: [TAT-2418](https://consensyssoftware.atlassian.net/browse/TAT-2418) ## **Manual testing steps** ```gherkin Feature: Flip position fee estimate accuracy Scenario: Fee estimate matches actual fee charged Given a wallet with an open BTC long position of ~$11 When user taps Modify then selects Flip Position Then the fee shown on the confirmation sheet is approximately 0.09% of the full position value (2x notional) not 0.045% (1x notional) Scenario: User with sufficient balance is not blocked Given a wallet with an open BTC position And available balance is $30 (above 1x fee ~$22.50, below 2x fee ~$45) When user attempts to flip the position Then the flip proceeds successfully ``` ## **Screenshots/Recordings** _Evidence available in task artifacts — will be added by reviewer if needed._ ### **Before** <!-- [screenshots/recordings] --> <img width="1206" height="2622" alt="before-flip-fee-estimate" src="https://github.com/user-attachments/assets/80b989b3-8ae2-4c87-ae03-610371a592e7" /> ### **After** <!-- [screenshots/recordings] --> <img width="1206" height="2622" alt="after-flip-fee-estimate" src="https://github.com/user-attachments/assets/5b9701b8-ab9d-4f82-88d5-08be1bf2c4b7" /> ## **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. ## **Validation Recipe** <details> <summary>recipe.json — automated validation (14 steps)</summary> ```json { "pr": "28013", "title": "Flip position fee estimate is ~2x lower than actual fee charged", "jira": "TAT-2418", "acceptance_criteria": [ "Fee estimate shown on flip confirmation sheet matches actual fee charged (within normal rounding tolerance)", "Users with sufficient margin for the actual fee are not blocked from submitting a flip order", "No new TypeScript errors" ], "validate": { "static": ["yarn lint:tsc"], "runtime": { "pre_conditions": ["wallet.unlocked", "perps.feature_enabled"], "steps": [ { "id": "open_pos", "description": "Open a BTC long position to flip", "action": "flow_ref", "ref": "trade-open-market", "params": { "symbol": "BTC", "side": "long", "usdAmount": "11" } }, { "id": "wait_position", "description": "Wait for BTC position to appear after opening", "action": "wait_for", "expression": "Engine.context.PerpsController.getPositions().then(function(positions){ var btc = positions.filter(function(p){ return p.symbol === 'BTC'; }); return JSON.stringify({positions: btc}); })", "timeout_ms": 20000, "assert": { "operator": "length_gt", "field": "positions", "value": 0 } }, { "id": "nav_market_details", "description": "Navigate to BTC market details to access flip confirmation sheet", "action": "navigate", "target": "PerpsMarketDetails", "params": { "symbol": "BTC" } }, { "id": "wait_market_details", "description": "Wait for market details to load", "action": "wait_for", "route": "PerpsMarketDetails" }, { "id": "wait_modify_button", "description": "Wait for the Modify button to appear (position must be loaded)", "action": "wait_for", "test_id": "perps-market-details-modify-button" }, { "id": "press_modify", "description": "Open the modify action sheet", "action": "press", "test_id": "perps-market-details-modify-button" }, { "id": "wait_flip_option", "description": "Wait for the flip position option in the modify action sheet", "action": "wait_for", "test_id": "undefined-flip_position" }, { "id": "press_flip_option", "description": "Select flip position to open flip confirmation sheet", "action": "press", "test_id": "undefined-flip_position" }, { "id": "wait_flip_sheet", "description": "Wait for flip confirmation bottom sheet to appear", "action": "wait_for", "test_id": "perps-flip-position-confirm-sheet" }, { "id": "check_fee_calculation", "description": "Assert the fee is calculated on 2x notional.", "action": "eval_async", "expression": "Engine.context.PerpsController.getPositions().then(function(positions) { var pos = null; for (var i = 0; i < positions.length; i++) { if (positions[i].symbol === 'BTC') { pos = positions[i]; break; } } if (!pos) return JSON.stringify({error: 'no BTC position'}); var size = Math.abs(parseFloat(pos.size)); var price = parseFloat(pos.entryPrice); var usdAmount1x = size * price; var usdAmount2x = size * 2 * price; return JSON.stringify({positionSize: size, entryPrice: price, fee_base_1x: usdAmount1x, fee_base_2x_correct: usdAmount2x}); })", "assert": { "operator": "not_null" } }, { "id": "evidence_fee_sheet", "description": "Capture fee estimate shown on confirmation sheet", "action": "screenshot", "filename": "evidence-flip-fee-estimate.png" }, { "id": "check_no_errors", "description": "Verify no errors in Metro logs during flip flow", "action": "log_watch", "window_seconds": 5, "must_not_appear": ["TypeError", "undefined is not an object"] }, { "id": "cancel_flip", "description": "Cancel the flip sheet without submitting", "action": "press", "test_id": "perps-flip-position-cancel-button" }, { "id": "cleanup_position", "description": "Close BTC position to leave clean state", "action": "flow_ref", "ref": "trade-close-position", "params": { "symbol": "BTC" } } ] } } } ``` </details> <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes fee/notional math used in the flip-position UI and the `TradingService.flipPosition` balance gate; incorrect values could affect whether users can submit flips and what fees they expect. > > **Overview** > Fixes flip-position fee math to align with how fees are actually charged. > > The flip confirmation sheet now estimates fees using **2x position notional** (close + open) and adds stable `testID`s for the sheet and its action buttons, with a unit test asserting the `usePerpsOrderFees` amount. > > `TradingService.flipPosition` updates its **available-balance fee check** to apply the combined fee rate to **1x notional** (since the rate already includes both legs), and adds a regression test ensuring a flip is allowed when balance covers that corrected estimate. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit dcf9361. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent e2bb36a commit b2ac9a2

5 files changed

Lines changed: 65 additions & 7 deletions

File tree

app/components/UI/Perps/Perps.testIds.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -759,3 +759,13 @@ export const PerpsTransactionsViewSelectorsIDs = {
759759
TAB_FUNDING: 'perps-transactions-tab-funding',
760760
TAB_DEPOSITS: 'perps-transactions-tab-deposits',
761761
} as const;
762+
763+
// ========================================
764+
// PERPS FLIP POSITION CONFIRM SHEET SELECTORS
765+
// ========================================
766+
767+
export const PerpsFlipPositionConfirmSheetSelectorsIDs = {
768+
SHEET: 'perps-flip-position-confirm-sheet',
769+
CANCEL_BUTTON: 'perps-flip-position-cancel-button',
770+
FLIP_BUTTON: 'perps-flip-position-flip-button',
771+
} as const;

app/components/UI/Perps/components/PerpsFlipPositionConfirmSheet/PerpsFlipPositionConfirmSheet.test.tsx

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
} from '@testing-library/react-native';
88
import PerpsFlipPositionConfirmSheet from './PerpsFlipPositionConfirmSheet';
99
import { type Position } from '@metamask/perps-controller';
10+
import { usePerpsOrderFees } from '../../hooks';
1011

1112
const mockHandleFlipPosition = jest.fn();
1213
let mockIsFlipping = false;
@@ -52,12 +53,12 @@ jest.mock('../../../../../../locales/i18n', () => ({
5253
}));
5354

5455
jest.mock('../../hooks', () => ({
55-
usePerpsOrderFees: () => ({
56+
usePerpsOrderFees: jest.fn(() => ({
5657
totalFee: 0.5,
5758
makerFee: 0.2,
5859
takerFee: 0.3,
5960
isLoadingMetamaskFee: false,
60-
}),
61+
})),
6162
usePerpsRewards: () => ({
6263
shouldShowRewardsRow: false,
6364
estimatedPoints: undefined,
@@ -270,6 +271,12 @@ describe('PerpsFlipPositionConfirmSheet', () => {
270271
beforeEach(() => {
271272
jest.clearAllMocks();
272273
mockIsFlipping = false;
274+
(usePerpsOrderFees as jest.Mock).mockReturnValue({
275+
totalFee: 0.5,
276+
makerFee: 0.2,
277+
takerFee: 0.3,
278+
isLoadingMetamaskFee: false,
279+
});
273280
});
274281

275282
it('renders the flip position title', () => {
@@ -366,4 +373,13 @@ describe('PerpsFlipPositionConfirmSheet', () => {
366373
// Math.abs(-2.5) = 2.5
367374
expect(screen.getByText('2.5 ETH')).toBeOnTheScreen();
368375
});
376+
377+
it('passes 2x position notional to usePerpsOrderFees for accurate fee estimate', () => {
378+
render(<PerpsFlipPositionConfirmSheet position={mockLongPosition} />);
379+
380+
// ETH position: size=2.5, markPrice=2502 → 2x notional = 2.5 * 2 * 2502 = 12510
381+
expect(usePerpsOrderFees).toHaveBeenCalledWith(
382+
expect.objectContaining({ amount: '12510' }),
383+
);
384+
});
369385
});

app/components/UI/Perps/components/PerpsFlipPositionConfirmSheet/PerpsFlipPositionConfirmSheet.tsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import React, { useCallback, useMemo, useRef } from 'react';
22
import { View, ActivityIndicator } from 'react-native';
33
import { strings } from '../../../../../../locales/i18n';
4+
import { PerpsFlipPositionConfirmSheetSelectorsIDs } from '../../Perps.testIds';
45
import BottomSheet, {
56
BottomSheetRef,
67
} from '../../../../../component-library/components/BottomSheets/BottomSheet';
@@ -67,9 +68,11 @@ const PerpsFlipPositionConfirmSheet: React.FC<
6768
const price = parseFloat(currentPrice?.price || '0');
6869
const markPrice = parseFloat(currentPrice?.markPrice || '0');
6970

70-
// Calculate USD amount for fee estimation
71+
// Calculate USD amount for fee estimation.
72+
// A flip places one order of 2x position size (1x to close current, 1x to open opposite).
73+
// Fee is charged on the full 2x notional, so multiply by 2 for an accurate estimate.
7174
const usdAmount = useMemo(
72-
() => (positionSize * (markPrice || price)).toString(),
75+
() => (positionSize * 2 * (markPrice || price)).toString(),
7376
[positionSize, markPrice, price],
7477
);
7578

@@ -140,6 +143,7 @@ const PerpsFlipPositionConfirmSheet: React.FC<
140143
variant: ButtonVariants.Secondary,
141144
size: ButtonSize.Lg,
142145
disabled: isFlipping,
146+
testID: PerpsFlipPositionConfirmSheetSelectorsIDs.CANCEL_BUTTON,
143147
},
144148
{
145149
label: isFlipping
@@ -150,6 +154,7 @@ const PerpsFlipPositionConfirmSheet: React.FC<
150154
size: ButtonSize.Lg,
151155
disabled: isFlipping || !hasValidAmount,
152156
danger: true,
157+
testID: PerpsFlipPositionConfirmSheetSelectorsIDs.FLIP_BUTTON,
153158
},
154159
],
155160
[handleCloseInternal, handleReverse, isFlipping, hasValidAmount],
@@ -160,6 +165,7 @@ const PerpsFlipPositionConfirmSheet: React.FC<
160165
ref={sheetRef}
161166
shouldNavigateBack={!externalSheetRef}
162167
onClose={externalSheetRef ? onClose : undefined}
168+
testID={PerpsFlipPositionConfirmSheetSelectorsIDs.SHEET}
163169
>
164170
<BottomSheetHeader onClose={handleCloseInternal}>
165171
<Text variant={TextVariant.HeadingMD}>

app/controllers/perps/services/TradingService.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2070,6 +2070,31 @@ describe('TradingService', () => {
20702070
).rejects.toThrow(/Insufficient balance for flip fees/);
20712071
});
20722072

2073+
it('allows flip when balance covers 1x notional fee estimate', async () => {
2074+
// position: size=0.5, entryPrice=50000
2075+
// estimatedFees = positionSize * entryPrice * ESTIMATED_FEE_RATE
2076+
// = 0.5 * 50000 * 0.0009 = $22.50 (1x notional, correct)
2077+
// pre-fix would compute 2x: 1.0 * 50000 * 0.0009 = $45 → would block this user
2078+
mockProvider.getAccountState = jest.fn().mockResolvedValue({
2079+
...mockAccountState,
2080+
availableBalance: '30', // $30 > $22.50, sufficient with 1x
2081+
});
2082+
mockProvider.placeOrder.mockResolvedValue({
2083+
success: true,
2084+
orderId: 'flip-balance-fixed',
2085+
filledSize: '1.0',
2086+
averagePrice: '50000',
2087+
});
2088+
2089+
const result = await tradingService.flipPosition({
2090+
provider: mockProvider,
2091+
position: mockPosition,
2092+
context: mockContext,
2093+
});
2094+
2095+
expect(result.success).toBe(true);
2096+
});
2097+
20732098
it('throws error when account state cannot be retrieved', async () => {
20742099
mockProvider.getAccountState = jest.fn().mockResolvedValue(null);
20752100

app/controllers/perps/services/TradingService.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1951,11 +1951,12 @@ export class TradingService {
19511951

19521952
const availableBalance = parseFloat(accountState.availableBalance);
19531953

1954-
// Estimate fees (close + open, approximately 0.09% of notional)
1955-
// Flip requires 2x position size (1x to close, 1x to open opposite)
1954+
// Estimate fees: ESTIMATED_FEE_RATE (0.09%) already accounts for both legs
1955+
// (close at 0.045% + open at 0.045% = 0.09% of position notional).
1956+
// Apply to 1x notional (positionSize * entryPrice), not 2x (flipSize * entryPrice).
19561957
const entryPrice = parseFloat(position.entryPrice);
19571958
const flipSize = positionSize * 2;
1958-
const notionalValue = flipSize * entryPrice;
1959+
const notionalValue = positionSize * entryPrice;
19591960
const estimatedFees = notionalValue * ESTIMATED_FEE_RATE;
19601961

19611962
if (estimatedFees > availableBalance) {

0 commit comments

Comments
 (0)