Skip to content

Commit 84da92c

Browse files
abretonc7sracitoreschristopherferreira9javiergarciaveragambinish
authored
fix(perps): TAT-1966 close all positions not including discount (MetaMask#22112)
## **Description** **Problem:** Fee discount was not being displayed in the "Close all positions" view, despite working correctly in the single position close view. This caused users to not see their rewards discount (e.g., -65%) when closing all positions. **Solution:** Added account-level fee discount fetching to `usePerpsCloseAllCalculations` hook, matching the implementation in the single position close flow. The fix includes: 1. **Fee Discount Fetching**: Added `RewardsController.getPerpsDiscountForAccount()` call to fetch user's discount 2. **Discount Application**: Applied discount to MetaMask fees using formula: `adjusted_rate = original_rate * (1 - discount_bips/10000)` 3. **Batch API Migration**: Migrated to batch points estimation API for better performance (N+1 API calls � 2 total calls) 4. **Error Handling**: Preserved `undefined` state for unavailable fees instead of defaulting to 0 **Technical Details:** - Account-level discount applies uniformly to all positions - Per-position fee calculations for accuracy (coin-specific parameters) - Freeze mechanism prevents repeated calculations on WebSocket position updates - Removed `feeDiscountBips` from effect dependencies to prevent recalculation on every update ## **Changelog** CHANGELOG entry: Fixed fee discount not displaying in close all positions view ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TAT-1966 ## **Manual testing steps** ```gherkin Feature: Fee discount display in close all positions Scenario: user views fee discount when closing all positions Given user has an account with fee discount (e.g., 65%) And user has multiple open positions When user navigates to "Close all positions" view Then fee discount badge should display (e.g., "-65%") And fees breakdown should show discounted MetaMask fee rate And estimated points should be calculated correctly ``` ## **Screenshots/Recordings** ### **Before** Fee discount was hardcoded to 0%, not showing user's actual rewards discount. ### **After** Fee discount now displays correctly (e.g., -65%) in the close all positions view, matching the single position close behavior. https://github.com/user-attachments/assets/76fa2bbc-bc1f-4b15-8655-82df18df8f3f ## **Implementation Details** ### Files Changed - `app/components/UI/Perps/hooks/usePerpsCloseAllCalculations.ts` ### Key Changes 1. **Added Fee Discount State** (lines 108-109): ```typescript const [feeDiscountBips, setFeeDiscountBips] = useState<number>(0); ``` 2. **Fetch Account-Level Discount** (lines 137-166): ```typescript useEffect(() => { async function fetchFeeDiscount() { const discountBips = await Engine.context.RewardsController.getPerpsDiscountForAccount(caipAccountId); setFeeDiscountBips(discountBips); } fetchFeeDiscount(); }, [selectedAddress, currentChainId]); ``` 3. **Apply Discount to Fees** (lines 226-252): ```typescript const discountMultiplier = feeDiscountBips > 0 ? 1 - feeDiscountBips / 10000 : 1; const adjustedMetamaskFeeRate = baseFees.metamaskFeeRate * discountMultiplier; const adjustedMetamaskFeeAmount = baseFees.metamaskFeeAmount !== undefined ? baseFees.metamaskFeeAmount * discountMultiplier : undefined; ``` 4. **Batch Points Estimation** (lines 274-295): ```typescript const batchEstimateBody: EstimatePointsDto = { activityType: 'PERPS', account: caipAccountId, activityContext: { perpsContext: perpsContextArray, // Array of all positions }, }; batchPoints = await Engine.context.RewardsController.estimatePoints(batchEstimateBody); ``` 5. **Calculate Discount Percentage** (lines 387-395): ```typescript const avgOriginalMetamaskFeeRate = feeDiscountBips > 0 && avgMetamaskFeeRate > 0 ? avgMetamaskFeeRate / (1 - feeDiscountBips / 10000) : avgMetamaskFeeRate; const avgFeeDiscountPercentage = feeDiscountBips > 0 ? feeDiscountBips / 100 : 0; ``` 6. **Optimized Dependencies** (line 333): - Removed `feeDiscountBips` from effect dependencies to prevent recalculation on every WebSocket position update - Uses ref-based freeze mechanism (`hasValidResultsRef`) to prevent repeated calculations ### Performance Improvements - **Batch API**: Reduced API calls from N+1 (one per position + one for discount) to 2 total calls - **Freeze Mechanism**: Prevents slow points computation from retriggering on WebSocket position updates ## **Known Limitations** There is a separate UI issue with tooltip z-index in the close position views where the "Close position" button appears on top of tooltip modals. This will be addressed in a follow-up PR. ## **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] > Adds account-level fee discount to close-all flow, switches rewards to batch estimation, and makes fee rates/price-driven calculations resilient with UI fallbacks. > > - **Perps Close All (hook `usePerpsCloseAllCalculations`)**: > - Fetches and applies account-level discount (`RewardsController.getPerpsDiscountForAccount`) to MetaMask fees; computes original vs discounted rates and avg discount. > - Migrates rewards to batch points estimation; aggregates totals/averages and guards errors; introduces `isFetchingInBackground`. > - Freezes results to avoid WS-induced recomputes; resets on positions/account/discount changes; robust cleanup/race guards. > - Treats fee rates/amounts as optional (`undefined`) and propagates through aggregates; `receiveAmount` falls back when fees unavailable. > - **Perps Close Single (`PerpsClosePositionView`)**: > - Stabilizes calculations using price refs; uses limit price when valid; updates P&L/position value derivations; expands tests for limit validation and price parsing. > - **UI Summary (`PerpsCloseSummary`)**: > - Accepts optional fees/rates; shows `"--"` when unavailable; adds tooltip interactions; updated tests. > - **Types & Fees Hook**: > - Marks `FeeCalculationResult` fee fields as optional; `usePerpsOrderFees` handles undefined rates (no misleading $0), updates caching checks, and tests edge cases. > - **Styling**: > - Ensures close-all footer stays behind overlays (`zIndex`/`elevation`). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit bf388c8. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Ramon AC <36987446+racitores@users.noreply.github.com> Co-authored-by: Christopher Ferreira <104831203+christopherferreira9@users.noreply.github.com> Co-authored-by: javiergarciavera <76975121+javiergarciavera@users.noreply.github.com> Co-authored-by: Nick Gambino <35090461+gambinish@users.noreply.github.com> Co-authored-by: George Weiler <georgejweiler@gmail.com> Co-authored-by: Luis Taniça <matallui@gmail.com> Co-authored-by: AxelGes <34173844+AxelGes@users.noreply.github.com> Co-authored-by: Pedro Pablo Aste Kompen <wachunei@gmail.com> Co-authored-by: Kylan Hurt <6249205+smilingkylan@users.noreply.github.com> Co-authored-by: SteP-n-s <stylianos.panagakos@consensys.net> Co-authored-by: Brian August Nguyen <brianacnguyen@gmail.com> Co-authored-by: tommasini <46944231+tommasini@users.noreply.github.com> Co-authored-by: Kevin Bluer <kevin@bluer.com> Co-authored-by: Vince Howard <vincenguyenhoward@gmail.com> Co-authored-by: Curtis David <Curtis.David7@gmail.com> Co-authored-by: Caainã Jeronimo <caainaje@gmail.com>
1 parent bde8943 commit 84da92c

11 files changed

Lines changed: 855 additions & 208 deletions

File tree

app/components/UI/Perps/Views/PerpsCloseAllPositionsView/PerpsCloseAllPositionsView.styles.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ export const createStyles = (_theme: Theme) =>
2929
},
3030
footerContainer: {
3131
paddingHorizontal: 16,
32+
zIndex: 0, // Ensure footer stays behind modal overlays
33+
elevation: 0, // Android equivalent
3234
},
3335
labelWithTooltip: {
3436
flexDirection: 'row',

app/components/UI/Perps/Views/PerpsCloseAllPositionsView/PerpsCloseAllPositionsView.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ describe('PerpsCloseAllPositionsView', () => {
151151
avgProtocolFeeRate: 0.00045,
152152
avgOriginalMetamaskFeeRate: 0.015,
153153
isLoading: false,
154+
isFetchingInBackground: false,
154155
hasError: false,
155156
shouldShowRewards: true,
156157
};

app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.test.tsx

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1377,6 +1377,99 @@ describe('PerpsClosePositionView', () => {
13771377
expect(getByTestId('effective-pnl').props.children).toBe(20);
13781378
});
13791379
});
1380+
1381+
it('prevents confirm when limit price is missing for limit orders', () => {
1382+
const mockHandleClosePosition = jest.fn();
1383+
1384+
const TestComponent = () => {
1385+
const [orderType] = React.useState<'market' | 'limit'>('limit');
1386+
const [limitPrice] = React.useState<string>('');
1387+
1388+
const handleConfirm = () => {
1389+
if (orderType === 'limit' && !limitPrice) {
1390+
return;
1391+
}
1392+
mockHandleClosePosition();
1393+
};
1394+
1395+
return (
1396+
<View>
1397+
<TouchableOpacity testID="confirm-button" onPress={handleConfirm}>
1398+
<Text>Confirm</Text>
1399+
</TouchableOpacity>
1400+
<Text testID="order-type">{orderType}</Text>
1401+
<Text testID="limit-price">{limitPrice}</Text>
1402+
</View>
1403+
);
1404+
};
1405+
1406+
const { getByTestId } = render(<TestComponent />);
1407+
1408+
fireEvent.press(getByTestId('confirm-button'));
1409+
1410+
expect(mockHandleClosePosition).not.toHaveBeenCalled();
1411+
});
1412+
1413+
it('validates invalid limit price values before using as effective price', () => {
1414+
const TestComponent = () => {
1415+
const [limitPrice, setLimitPrice] = React.useState<string>('invalid');
1416+
const orderType = 'limit';
1417+
const currentPrice = 50000;
1418+
1419+
const effectivePrice = () => {
1420+
if (orderType === 'limit' && limitPrice) {
1421+
const parsed = parseFloat(limitPrice);
1422+
if (!isNaN(parsed) && parsed > 0) {
1423+
return parsed;
1424+
}
1425+
}
1426+
return currentPrice;
1427+
};
1428+
1429+
return (
1430+
<View>
1431+
<TouchableOpacity
1432+
testID="set-invalid"
1433+
onPress={() => setLimitPrice('invalid')}
1434+
>
1435+
<Text>Set Invalid</Text>
1436+
</TouchableOpacity>
1437+
<TouchableOpacity
1438+
testID="set-negative"
1439+
onPress={() => setLimitPrice('-100')}
1440+
>
1441+
<Text>Set Negative</Text>
1442+
</TouchableOpacity>
1443+
<TouchableOpacity
1444+
testID="set-zero"
1445+
onPress={() => setLimitPrice('0')}
1446+
>
1447+
<Text>Set Zero</Text>
1448+
</TouchableOpacity>
1449+
<TouchableOpacity
1450+
testID="set-valid"
1451+
onPress={() => setLimitPrice('51000')}
1452+
>
1453+
<Text>Set Valid</Text>
1454+
</TouchableOpacity>
1455+
<Text testID="effective-price">{effectivePrice()}</Text>
1456+
</View>
1457+
);
1458+
};
1459+
1460+
const { getByTestId } = render(<TestComponent />);
1461+
1462+
expect(getByTestId('effective-price').props.children).toBe(50000);
1463+
1464+
fireEvent.press(getByTestId('set-negative'));
1465+
expect(getByTestId('effective-price').props.children).toBe(50000);
1466+
1467+
fireEvent.press(getByTestId('set-zero'));
1468+
expect(getByTestId('effective-price').props.children).toBe(50000);
1469+
1470+
fireEvent.press(getByTestId('set-valid'));
1471+
expect(getByTestId('effective-price').props.children).toBe(51000);
1472+
});
13801473
});
13811474

13821475
describe('Input Handling', () => {

app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.tsx

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,11 @@ const PerpsClosePositionView: React.FC = () => {
115115
? parseFloat(priceData[position.coin].price)
116116
: parseFloat(position.entryPrice);
117117

118+
// Use ref to access latest price without triggering fee recalculations
119+
// This prevents continuous recalculations on every WebSocket price update
120+
const currentPriceRef = useRef(currentPrice);
121+
currentPriceRef.current = currentPrice;
122+
118123
// Get top of book data for maker/taker fee determination
119124
const currentTopOfBook = usePerpsTopOfBook({
120125
symbol: position.coin,
@@ -157,22 +162,31 @@ const PerpsClosePositionView: React.FC = () => {
157162

158163
// Calculate position value and effective margin
159164
// For limit orders, use limit price for display calculations
160-
const positionValue = useMemo(
161-
() => absSize * effectivePrice,
162-
[absSize, effectivePrice], // Round to 2 decimal places
163-
);
165+
// Use ref for market price to prevent recalculation on every WebSocket update
166+
const positionValue = useMemo(() => {
167+
const priceToUse =
168+
orderType === 'limit' && limitPrice
169+
? parseFloat(limitPrice)
170+
: currentPriceRef.current;
171+
return absSize * priceToUse;
172+
}, [absSize, orderType, limitPrice]); // Exclude currentPrice from deps to prevent recalculation
164173

165174
// Calculate P&L based on effective price (limit price for limit orders)
175+
// Use ref for market price to prevent recalculation on every WebSocket update
166176
const entryPrice = parseFloat(position.entryPrice);
167177
const effectivePnL = useMemo(() => {
168178
// Calculate P&L based on the effective price (limit price for limit orders)
169179
// For long positions: (effectivePrice - entryPrice) * absSize
170180
// For short positions: (entryPrice - effectivePrice) * absSize
181+
const priceToUse =
182+
orderType === 'limit' && limitPrice
183+
? parseFloat(limitPrice)
184+
: currentPriceRef.current;
171185
const priceDiff = isLong
172-
? effectivePrice - entryPrice
173-
: entryPrice - effectivePrice;
186+
? priceToUse - entryPrice
187+
: entryPrice - priceToUse;
174188
return priceDiff * absSize;
175-
}, [effectivePrice, entryPrice, absSize, isLong]);
189+
}, [entryPrice, absSize, isLong, orderType, limitPrice]); // Exclude effectivePrice from deps
176190

177191
// Use the actual initial margin from the position
178192
const initialMargin = parseFloat(position.marginUsed);

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

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import React from 'react';
2-
import { render } from '@testing-library/react-native';
2+
import { render, fireEvent } from '@testing-library/react-native';
33
import PerpsCloseSummary from './PerpsCloseSummary';
44
import { strings } from '../../../../../../locales/i18n';
55

@@ -205,4 +205,60 @@ describe('PerpsCloseSummary', () => {
205205
// Assert - rewards section still renders with error state
206206
expect(getByText('perps.estimated_points')).toBeTruthy();
207207
});
208+
209+
it('handles tooltip press to open fees tooltip', () => {
210+
const props = {
211+
...defaultProps,
212+
enableTooltips: true,
213+
testIDs: { feesTooltip: 'fees-tooltip' },
214+
};
215+
216+
const { getByTestId } = render(<PerpsCloseSummary {...props} />);
217+
218+
fireEvent.press(getByTestId('fees-tooltip'));
219+
220+
expect(getByTestId('fees-tooltip')).toBeTruthy();
221+
});
222+
223+
it('handles tooltip press to open receive amount tooltip', () => {
224+
const props = {
225+
...defaultProps,
226+
enableTooltips: true,
227+
testIDs: { receiveTooltip: 'receive-tooltip' },
228+
};
229+
230+
const { getByTestId } = render(<PerpsCloseSummary {...props} />);
231+
232+
fireEvent.press(getByTestId('receive-tooltip'));
233+
234+
expect(getByTestId('receive-tooltip')).toBeTruthy();
235+
});
236+
237+
it('handles tooltip press to open points tooltip when rewards enabled', () => {
238+
const props = {
239+
...defaultProps,
240+
shouldShowRewards: true,
241+
enableTooltips: true,
242+
estimatedPoints: 100,
243+
testIDs: { pointsTooltip: 'points-tooltip' },
244+
};
245+
246+
const { getByTestId } = render(<PerpsCloseSummary {...props} />);
247+
248+
fireEvent.press(getByTestId('points-tooltip'));
249+
250+
expect(getByTestId('points-tooltip')).toBeTruthy();
251+
});
252+
253+
it('does not trigger tooltip callback when tooltips are disabled', () => {
254+
const props = {
255+
...defaultProps,
256+
enableTooltips: false,
257+
testIDs: { feesTooltip: 'fees-tooltip' },
258+
};
259+
260+
const { queryByTestId } = render(<PerpsCloseSummary {...props} />);
261+
262+
expect(queryByTestId('fees-tooltip')).toBeNull();
263+
});
208264
});

app/components/UI/Perps/components/PerpsCloseSummary/PerpsCloseSummary.tsx

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,15 @@ export interface PerpsCloseSummaryProps {
3434
/** Total unrealized P&L (for "includes P&L" breakdown) */
3535
totalPnl: number;
3636

37-
/** Total fees for closing */
38-
totalFees: number;
39-
/** Fee discount percentage (0-100) */
37+
/** Total fees for closing (undefined when unavailable) */
38+
totalFees?: number;
39+
/** Fee discount percentage (0-100, undefined when unavailable) */
4040
feeDiscountPercentage?: number;
41-
/** MetaMask fee rate (as decimal, e.g. 0.01 for 1%) */
42-
metamaskFeeRate: number;
43-
/** Protocol fee rate (as decimal, e.g. 0.00045 for 0.045%) */
44-
protocolFeeRate: number;
45-
/** Original MetaMask fee rate before discounts */
41+
/** MetaMask fee rate (as decimal, e.g. 0.01 for 1%) - undefined means unavailable/error state */
42+
metamaskFeeRate?: number;
43+
/** Protocol fee rate (as decimal, e.g. 0.00045 for 0.045%) - undefined means unavailable/error state */
44+
protocolFeeRate?: number;
45+
/** Original MetaMask fee rate before discounts - undefined means unavailable/error state */
4646
originalMetamaskFeeRate?: number;
4747

4848
/** Amount user will receive after closing */
@@ -213,14 +213,16 @@ const PerpsCloseSummary: React.FC<PerpsCloseSummaryProps> = ({
213213
color={theme.colors.icon.alternative}
214214
/>
215215
</View>
216-
) : (
216+
) : totalFees !== undefined ? (
217217
<PerpsFeesDisplay
218218
feeDiscountPercentage={feeDiscountPercentage}
219219
formatFeeText={`-${formatPerpsFiat(totalFees, {
220220
ranges: PRICE_RANGES_MINIMAL_VIEW,
221221
})}`}
222222
variant={TextVariant.BodyMD}
223223
/>
224+
) : (
225+
<Text variant={TextVariant.BodyMD}>--</Text>
224226
)}
225227
</View>
226228
</View>

app/components/UI/Perps/controllers/types/index.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -622,15 +622,15 @@ export interface FeeCalculationParams {
622622

623623
export interface FeeCalculationResult {
624624
// Total fees (protocol + MetaMask)
625-
feeRate: number; // Total fee rate as decimal (e.g., 0.00145 for 0.145%)
625+
feeRate?: number; // Total fee rate as decimal (e.g., 0.00145 for 0.145%), undefined when unavailable
626626
feeAmount?: number; // Total fee amount in USD (when amount is provided)
627627

628628
// Protocol-specific base fees
629-
protocolFeeRate: number; // Protocol fee rate (e.g., 0.00045 for HyperLiquid taker)
629+
protocolFeeRate?: number; // Protocol fee rate (e.g., 0.00045 for HyperLiquid taker), undefined when unavailable
630630
protocolFeeAmount?: number; // Protocol fee amount in USD
631631

632632
// MetaMask builder/revenue fee
633-
metamaskFeeRate: number; // MetaMask fee rate (e.g., 0.001 for 0.1%)
633+
metamaskFeeRate?: number; // MetaMask fee rate (e.g., 0.001 for 0.1%), undefined when unavailable
634634
metamaskFeeAmount?: number; // MetaMask fee amount in USD
635635

636636
// Optional detailed breakdown for transparency

0 commit comments

Comments
 (0)