Skip to content

Commit dbad6ed

Browse files
runway-github[bot]michalconsensysnickewansmithjoaoloureirop
authored
chore(runway): cherry-pick fix(perps): cp-7.59.0 exclude P&L from total margin calculation in close all positions (#22451)
- fix(perps): exclude P&L from total margin calculation in close all positions (#22391) <!-- 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** This PR fixes a calculation error in the "Close All Positions" feature for Perps. Previously, the total margin calculation incorrectly included unrealized P&L (profit and loss), which led to incorrect amount calculations when closing all positions. **What is the reason for the change?** The total margin should represent only the actual margin used in positions, not the combined value of margin + P&L. Including P&L in the margin calculation resulted in incorrect receive amounts being displayed to users. ## **Changelog** CHANGELOG entry: Fixed margin calculation in Close All Positions to exclude P&L ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TAT-2030 ## **Manual testing steps** ```gherkin Feature: Close All Positions margin calculation Scenario: user closes all positions with positive P&L Given user has multiple open positions with positive unrealized P&L And user navigates to Close All Positions view When user views the close summary Then the total margin displayed should only include marginUsed (excluding P&L) And the receive amount should equal total margin minus fees And P&L should be displayed separately ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <img width="1170" height="2532" alt="Simulator Screenshot - iPhone 16e - 2025-11-10 at 12 16 24" src="https://github.com/user-attachments/assets/0f566135-2acc-46fa-89ec-06db62139f31" /> ### **After** <img width="1170" height="2532" alt="Simulator Screenshot - iPhone 16e - 2025-11-10 at 12 11 04" src="https://github.com/user-attachments/assets/0f1762ac-0da3-45b7-80a8-d0d18b0c450b" /> ## **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] > Exclude P&L from total margin in close-all calculations, compute receive amount as margin minus fees, and update tests accordingly. > > - **Perps close-all calculations (`usePerpsCloseAllCalculations`)**: > - `totalMargin` now excludes `unrealizedPnl`; `totalPnl` calculated separately. > - `receiveAmount` = `totalMargin` − `totalFees` (no P&L mixing). > - JSDoc updated to reflect margin definition; use `Number.parseFloat` for parsing. > - **Tests (`usePerpsCloseAllCalculations.test.ts`)**: > - Update expectations to exclude P&L from `totalMargin` and verify `totalPnl` separately. > - Add/adjust cases for positive/negative/zero P&L, multiple positions, and fee scenarios (e.g., fees equal margin → receive amount 0). > - Verify fee aggregation, discounts, averages, and points remain consistent with new margin logic. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 1504819. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Nicholas Smith <nick.smith@consensys.net> [ecf1712](ecf1712) Co-authored-by: Michal Szorad <michal.szorad@consensys.net> Co-authored-by: Nicholas Smith <nick.smith@consensys.net> Co-authored-by: João Loureiro <175489935+joaoloureirop@users.noreply.github.com>
1 parent 791afc4 commit dbad6ed

2 files changed

Lines changed: 101 additions & 14 deletions

File tree

app/components/UI/Perps/hooks/usePerpsCloseAllCalculations.test.ts

Lines changed: 96 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ describe('usePerpsCloseAllCalculations', () => {
145145
});
146146

147147
describe('Total Margin Calculation', () => {
148-
it('calculates total margin including P&L for single position', () => {
148+
it('calculates total margin excluding P&L for single position', () => {
149149
// Arrange
150150
const positions = [
151151
createMockPosition({
@@ -161,7 +161,7 @@ describe('usePerpsCloseAllCalculations', () => {
161161
);
162162

163163
// Assert
164-
expect(result.current.totalMargin).toBe(1100); // 1000 + 100
164+
expect(result.current.totalMargin).toBe(1000); // Only marginUsed, PnL excluded
165165
});
166166

167167
it('calculates total margin for multiple positions', () => {
@@ -189,10 +189,10 @@ describe('usePerpsCloseAllCalculations', () => {
189189
);
190190

191191
// Assert
192-
expect(result.current.totalMargin).toBe(1550); // (1000+100) + (500-50)
192+
expect(result.current.totalMargin).toBe(1500); // 1000 + 500, PnL excluded
193193
});
194194

195-
it('handles negative P&L correctly', () => {
195+
it('excludes negative P&L from margin calculation', () => {
196196
// Arrange
197197
const positions = [
198198
createMockPosition({
@@ -208,7 +208,47 @@ describe('usePerpsCloseAllCalculations', () => {
208208
);
209209

210210
// Assert
211-
expect(result.current.totalMargin).toBe(800); // 1000 - 200
211+
expect(result.current.totalMargin).toBe(1000); // Only marginUsed, negative PnL excluded
212+
});
213+
214+
it('excludes positive P&L from margin calculation', () => {
215+
// Arrange
216+
const positions = [
217+
createMockPosition({
218+
marginUsed: '500',
219+
unrealizedPnl: '250',
220+
}),
221+
];
222+
const priceData = { BTC: { price: '52000' } };
223+
224+
// Act
225+
const { result } = renderHook(() =>
226+
usePerpsCloseAllCalculations({ positions, priceData }),
227+
);
228+
229+
// Assert
230+
expect(result.current.totalMargin).toBe(500); // Only marginUsed, positive PnL excluded
231+
expect(result.current.totalPnl).toBe(250); // PnL is tracked separately
232+
});
233+
234+
it('calculates total margin with zero P&L', () => {
235+
// Arrange
236+
const positions = [
237+
createMockPosition({
238+
marginUsed: '800',
239+
unrealizedPnl: '0',
240+
}),
241+
];
242+
const priceData = { BTC: { price: '50000' } };
243+
244+
// Act
245+
const { result } = renderHook(() =>
246+
usePerpsCloseAllCalculations({ positions, priceData }),
247+
);
248+
249+
// Assert
250+
expect(result.current.totalMargin).toBe(800); // Only marginUsed
251+
expect(result.current.totalPnl).toBe(0);
212252
});
213253
});
214254

@@ -778,10 +818,12 @@ describe('usePerpsCloseAllCalculations', () => {
778818
expect(result.current.isLoading).toBe(false);
779819
});
780820
// Total fee recalculated: 25 + 25 = 50
781-
expect(result.current.receiveAmount).toBe(1050); // (1000 + 100) - 50
821+
// Receive amount: 1000 (margin only, PnL excluded) - 50 (fees) = 950
822+
expect(result.current.receiveAmount).toBe(950);
823+
expect(result.current.totalPnl).toBe(100); // PnL tracked separately
782824
});
783825

784-
it('handles negative receive amount when fees exceed margin', async () => {
826+
it('returns zero receive amount when fees equal margin', async () => {
785827
// Arrange
786828
const positions = [
787829
createMockPosition({
@@ -807,7 +849,53 @@ describe('usePerpsCloseAllCalculations', () => {
807849
await waitFor(() => {
808850
expect(result.current.isLoading).toBe(false);
809851
});
810-
expect(result.current.receiveAmount).toBe(-50); // (100 - 50) - 100
852+
// Receive amount: 100 (margin only) - 100 (fees) = 0
853+
expect(result.current.receiveAmount).toBe(0);
854+
expect(result.current.totalPnl).toBe(-50); // PnL tracked separately
855+
});
856+
857+
it('calculates receive amount excluding P&L for multiple positions', async () => {
858+
// Arrange
859+
const positions = [
860+
createMockPosition({
861+
coin: 'BTC',
862+
marginUsed: '2000',
863+
unrealizedPnl: '300',
864+
}),
865+
createMockPosition({
866+
coin: 'ETH',
867+
marginUsed: '1500',
868+
unrealizedPnl: '-100',
869+
}),
870+
];
871+
const priceData = {
872+
BTC: { price: '51000' },
873+
ETH: { price: '3000' },
874+
};
875+
mockCalculateFees.mockResolvedValue(
876+
createMockFeeResult({
877+
feeAmount: 150,
878+
metamaskFeeAmount: 100,
879+
protocolFeeAmount: 50,
880+
}),
881+
);
882+
883+
// Act
884+
const { result } = renderHook(() =>
885+
usePerpsCloseAllCalculations({ positions, priceData }),
886+
);
887+
888+
// Assert
889+
await waitFor(() => {
890+
expect(result.current.isLoading).toBe(false);
891+
});
892+
// Total margin: 2000 + 1500 = 3500 (PnL excluded)
893+
// Total fees: 150 + 150 = 300 (two positions)
894+
// Receive amount: 3500 - 300 = 3200
895+
expect(result.current.totalMargin).toBe(3500);
896+
expect(result.current.totalPnl).toBe(200); // 300 - 100
897+
expect(result.current.totalFees).toBe(300);
898+
expect(result.current.receiveAmount).toBe(3200);
811899
});
812900
});
813901

app/components/UI/Perps/hooks/usePerpsCloseAllCalculations.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { formatAccountToCaipAccountId } from '../utils/rewardsUtils';
1414
* Aggregated calculations result for closing all positions
1515
*/
1616
export interface CloseAllCalculationsResult {
17-
/** Total margin across all positions (includes P&L) */
17+
/** Total margin across all positions (excludes P&L) */
1818
totalMargin: number;
1919
/** Total unrealized P&L across all positions */
2020
totalPnl: number;
@@ -120,13 +120,12 @@ export function usePerpsCloseAllCalculations({
120120
const hasValidResultsRef = useRef(false);
121121
const hasValidDiscountRef = useRef(false);
122122

123-
// Calculate total margin (including P&L)
123+
// Calculate total margin
124124
const totalMargin = useMemo(
125125
() =>
126126
positions.reduce((sum, pos) => {
127-
const margin = parseFloat(pos.marginUsed) || 0;
128-
const pnl = parseFloat(pos.unrealizedPnl) || 0;
129-
return sum + margin + pnl;
127+
const margin = Number.parseFloat(pos.marginUsed) || 0;
128+
return sum + margin;
130129
}, 0),
131130
[positions],
132131
);
@@ -135,7 +134,7 @@ export function usePerpsCloseAllCalculations({
135134
const totalPnl = useMemo(
136135
() =>
137136
positions.reduce(
138-
(sum, pos) => sum + (parseFloat(pos.unrealizedPnl) || 0),
137+
(sum, pos) => sum + (Number.parseFloat(pos.unrealizedPnl) || 0),
139138
0,
140139
),
141140
[positions],

0 commit comments

Comments
 (0)