Skip to content

Commit e734f45

Browse files
runway-github[bot]nickewansmithabretonc7s
authored
chore(runway): cherry-pick fix: cp-7.59.0 hotfix-7.58.2 update close position calculation with funding fees and live data (#22333)
- fix: cp-7.59.0 hotfix-7.58.2 update close position calculation with funding fees and live data (#22229) <!-- 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** Fixes bug where close position view showed incorrect receive amounts, sometimes displaying negative values when position value had dropped. Root causes: 1. Used stale position data from route params instead of live WebSocket updates 2. HyperLiquid's marginUsed already includes PnL, but code was double-counting 3. Recalculated PnL from prices, which missed accumulated funding fees 4. Timing mismatch between price updates and position updates Changes: - Add usePerpsLivePositions to subscribe to real-time position data - Use livePosition.marginUsed which already includes accumulated PnL and funding fees - Use livePosition.positionValue for fee calculations to keep margin and fees synchronized - Remove effectivePnL recalculation that missed funding fees - Round margin and fees separately before subtraction for transparent calculation - Update tests to reflect that marginUsed includes PnL from HyperLiquid Result: - Position card and close position view now show matching margin values - Funding fees correctly included in all calculations - No more incorrect negative receive amounts - Calculation transparency: displayed margin - displayed fees = displayed receive amount <!-- 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-1956 ## **Manual testing steps** ```gherkin Feature: Close Position Calculation with Funding Fees Scenario: Close position with accumulated funding fees shows correct receive amount Given I have an open position with the following details: | Coin | ETH | | Margin Used | $100 | | Unrealized PnL| -$5 | | Entry Price | $2000 | | Current Price | $2000 | When I navigate to the close position view And I view the close position summary Then the margin displayed should be "$100" And the receive amount should match "margin - fees" And the receive amount should be positive And the receive amount should not be negative or zero Scenario: Receive amount calculation matches visual breakdown Given I have an open position When I view the close position summary Then the displayed margin should be rounded to 2 decimals And the displayed fees should be rounded to 2 decimals And the receive amount should equal "rounded margin minus rounded fees" And the calculation should be transparent to the user ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** https://github.com/user-attachments/assets/29289663-7b77-45f7-8ff6-c48917e1e21a <!-- [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] > Close Position view now uses live position data and computes receive amount as rounded marginUsed minus fees, syncing PnL/margin (incl. funding) and updating limit/fee logic and tests. > > - **Perps Close Position (UI/logic)**: > - Subscribe to `usePerpsLivePositions` and derive `livePosition` for real-time `marginUsed` and `unrealizedPnl` (includes funding fees). > - Rework calculations: > - `receiveAmount` = rounded `(closePercentage * marginUsed)` − rounded fees. > - `effectivePnL`: use live `pnl` for market orders; recompute only for limit price. > - `positionValue`/`closingValue` use current or limit price with correct deps. > - Back-calc `unrealizedPnlPercent` from `marginUsed - pnl`. > - Pass `livePosition` into `handleClosePosition`; include updated `receivedAmount`, `realizedPnl`, and price string. > - Keep USD input in sync without jumps; auto-open limit price sheet on limit; minor deps/rounding adjustments. > - **Tests**: > - Mock `usePerpsLivePositions`; update assertions to reflect `marginUsed` includes PnL and new receive formula. > - Add price update synchronization test and adjust PnL/receive expectations for market/limit scenarios. > - Update event/confirm handler expectations and various calculation paths (fees, partial closes, short/long, limit price variations). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit e008a6a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: abretonc7s <107169956+abretonc7s@users.noreply.github.com> [9186674](9186674) Co-authored-by: Nicholas Smith <nick.smith@consensys.net> Co-authored-by: abretonc7s <107169956+abretonc7s@users.noreply.github.com>
1 parent 16f0321 commit e734f45

2 files changed

Lines changed: 171 additions & 58 deletions

File tree

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

Lines changed: 119 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ jest.mock('../../hooks', () => ({
5151
}));
5252

5353
jest.mock('../../hooks/stream', () => ({
54+
usePerpsLivePositions: jest.fn(),
5455
usePerpsLivePrices: jest.fn(),
5556
usePerpsTopOfBook: jest.fn(),
5657
}));
@@ -119,6 +120,9 @@ describe('PerpsClosePositionView', () => {
119120
const useRouteMock = jest.mocked(
120121
jest.requireMock('@react-navigation/native').useRoute,
121122
);
123+
const usePerpsLivePositionsMock = jest.mocked(
124+
jest.requireMock('../../hooks/stream').usePerpsLivePositions,
125+
);
122126
const usePerpsLivePricesMock = jest.mocked(
123127
jest.requireMock('../../hooks/stream').usePerpsLivePrices,
124128
);
@@ -166,6 +170,10 @@ describe('PerpsClosePositionView', () => {
166170
});
167171

168172
// Setup hook mocks with default values
173+
usePerpsLivePositionsMock.mockReturnValue({
174+
positions: [defaultPerpsPositionMock],
175+
isInitialLoading: false,
176+
});
169177
usePerpsLivePricesMock.mockReturnValue(defaultPerpsLivePricesMock);
170178
usePerpsTopOfBookMock.mockReturnValue(defaultPerpsTopOfBookMock);
171179
usePerpsOrderFeesMock.mockReturnValue(defaultPerpsOrderFeesMock);
@@ -446,14 +454,14 @@ describe('PerpsClosePositionView', () => {
446454
expect(usePerpsOrderFeesMock).toHaveBeenCalled();
447455
});
448456

449-
// Test that receiveAmount = (initial margin + effective P&L) - fees
457+
// Test that receiveAmount uses marginUsed directly (which already includes PnL)
450458
it('calculates receive amount including P&L at effective price', () => {
451459
// Arrange
452460
const mockPosition = {
453461
...defaultPerpsPositionMock,
454462
entryPrice: '100', // Entry at $100
455-
marginUsed: '1000', // $1000 initial margin
456-
unrealizedPnl: '200', // Current unrealized P&L (not used in new calc)
463+
marginUsed: '1200', // $1200 (includes $1000 initial + $200 unrealized PnL)
464+
unrealizedPnl: '200', // Current unrealized P&L from HyperLiquid
457465
size: '1', // 1 token long position
458466
};
459467
const mockFees = {
@@ -462,7 +470,7 @@ describe('PerpsClosePositionView', () => {
462470
protocolFeeRate: 0.5,
463471
};
464472

465-
// Set current price to $150 for clear P&L calculation
473+
// Set current price to $150 for reference
466474
usePerpsLivePricesMock.mockReturnValue({
467475
ETH: { price: '150' }, // Current price $150
468476
});
@@ -471,6 +479,10 @@ describe('PerpsClosePositionView', () => {
471479
params: { position: mockPosition },
472480
});
473481
usePerpsOrderFeesMock.mockReturnValue(mockFees);
482+
usePerpsLivePositionsMock.mockReturnValue({
483+
positions: [mockPosition],
484+
isInitialLoading: false,
485+
});
474486

475487
// Act
476488
const { getByText } = renderWithProvider(
@@ -481,25 +493,24 @@ describe('PerpsClosePositionView', () => {
481493
true,
482494
);
483495

484-
// Assert - receiveAmount = initialMargin + P&L - fees (P&L now included in calculation)
485-
// P&L = (150 - 100) * 1 = 50
486-
// receiveAmount = 1000 + 50 - 50 = 1000
496+
// Assert - receiveAmount = marginUsed - fees
497+
// HyperLiquid's marginUsed already includes PnL
498+
// receiveAmount = 1200 - 50 = 1150
487499
const receiveText = getByText(
488500
strings('perps.close_position.you_receive'),
489501
);
490502
expect(receiveText).toBeDefined();
491-
// Look for 1000 in the display (margin + P&L - fees)
492503
// PRICE_RANGES_MINIMAL_VIEW: Fixed 2 decimals, trailing zeros removed
493-
expect(getByText('$1,000')).toBeDefined();
504+
expect(getByText('$1,150')).toBeDefined();
494505
});
495506

496507
it('calculates receive amount correctly for partial close percentages', () => {
497508
// Arrange
498509
const mockPosition = {
499510
...defaultPerpsPositionMock,
500511
entryPrice: '100', // Entry at $100
501-
marginUsed: '2000', // $2000 initial margin
502-
unrealizedPnl: '-300', // Current unrealized (not used in new calc)
512+
marginUsed: '1700', // $1700 (includes $2000 initial - $300 unrealized loss)
513+
unrealizedPnl: '-300', // Current unrealized loss from HyperLiquid
503514
size: '2', // 2 tokens long
504515
};
505516
const mockFees = {
@@ -517,6 +528,10 @@ describe('PerpsClosePositionView', () => {
517528
params: { position: mockPosition },
518529
});
519530
usePerpsOrderFeesMock.mockReturnValue(mockFees);
531+
usePerpsLivePositionsMock.mockReturnValue({
532+
positions: [mockPosition],
533+
isInitialLoading: false,
534+
});
520535

521536
// Act
522537
const { getByText } = renderWithProvider(
@@ -527,15 +542,15 @@ describe('PerpsClosePositionView', () => {
527542
true,
528543
);
529544

530-
// For 100% close (default) with new calculation:
531-
// P&L = (75 - 100) * 2 = -50 (loss)
532-
// receiveAmount = 2000 + (-50) - 25 = 1925
545+
// For 100% close (default):
546+
// HyperLiquid's marginUsed already includes PnL
547+
// receiveAmount = 1700 - 25 = 1675
533548
const receiveText = getByText(
534549
strings('perps.close_position.you_receive'),
535550
);
536551
expect(receiveText).toBeDefined();
537-
// Look for 1925 in the display (margin + P&L - fees)
538-
expect(getByText(/1,925/)).toBeDefined();
552+
// Look for 1675 in the display
553+
expect(getByText(/1,675/)).toBeDefined();
539554
});
540555
});
541556

@@ -632,8 +647,9 @@ describe('PerpsClosePositionView', () => {
632647
const positionWithLoss = {
633648
...defaultPerpsPositionMock,
634649
entryPrice: '150', // Entry at $150
650+
marginUsed: '1350', // $1500 initial - $150 unrealized loss (includes funding)
635651
size: '1', // 1 token long
636-
unrealizedPnl: '-100', // Current unrealized (not used for display)
652+
unrealizedPnl: '-150', // Current unrealized loss including funding fees
637653
};
638654

639655
// Set current price lower than entry for loss
@@ -644,6 +660,10 @@ describe('PerpsClosePositionView', () => {
644660
useRouteMock.mockReturnValue({
645661
params: { position: positionWithLoss },
646662
});
663+
usePerpsLivePositionsMock.mockReturnValue({
664+
positions: [positionWithLoss],
665+
isInitialLoading: false,
666+
});
647667

648668
// Act
649669
const { getByText } = renderWithProvider(
@@ -654,9 +674,9 @@ describe('PerpsClosePositionView', () => {
654674
true,
655675
);
656676

657-
// Assert - effectivePnL = (100 - 150) * 1 = -50
658-
// Look for negative P&L display (with - sign) - should show 50 (absolute value)
659-
const pnlElement = getByText(/-.*50/);
677+
// Assert - Now uses actual unrealizedPnl from position
678+
// Look for negative P&L display (with - sign) - should show 150 (absolute value)
679+
const pnlElement = getByText(/-.*150/);
660680
expect(pnlElement).toBeDefined();
661681
});
662682
});
@@ -2049,9 +2069,9 @@ describe('PerpsClosePositionView', () => {
20492069
expect(track).toHaveBeenCalled();
20502070

20512071
// Assert - Should call with expected parameters structure for full close
2052-
// Calculation: effectivePnL = (3000 - 2900) * 1.5 = 150
2053-
// effectiveMargin = 1450 + 150 = 1600
2054-
// receivedAmount = 1600 - 45 = 1555
2072+
// HyperLiquid's marginUsed already includes PnL
2073+
// receivedAmount = marginUsed - fees = 1450 - 45 = 1405
2074+
// realizedPnl = unrealizedPnl = 150 (from defaultPerpsPositionMock)
20552075
expect(handleClosePosition).toHaveBeenCalledWith(
20562076
defaultPerpsPositionMock,
20572077
'',
@@ -2060,7 +2080,7 @@ describe('PerpsClosePositionView', () => {
20602080
{
20612081
totalFee: 45,
20622082
marketPrice: 3000,
2063-
receivedAmount: 1555,
2083+
receivedAmount: 1405,
20642084
realizedPnl: 150,
20652085
metamaskFeeRate: 0,
20662086
metamaskFee: 0,
@@ -2369,6 +2389,81 @@ describe('PerpsClosePositionView', () => {
23692389
});
23702390

23712391
describe('Price Update Synchronization', () => {
2392+
it('recalculates effectivePnL when current price changes', async () => {
2393+
// Arrange - Position with entry price
2394+
const mockPosition = {
2395+
...defaultPerpsPositionMock,
2396+
size: '1', // 1 token long position
2397+
entryPrice: '100', // Entry at $100
2398+
marginUsed: '100',
2399+
unrealizedPnl: '0',
2400+
};
2401+
2402+
useRouteMock.mockReturnValue({
2403+
params: { position: mockPosition },
2404+
});
2405+
2406+
// Mock usePerpsLivePositions to return the test's mock position
2407+
usePerpsLivePositionsMock.mockReturnValue({
2408+
positions: [mockPosition],
2409+
isInitialLoading: false,
2410+
});
2411+
2412+
// Initially price equals entry price (no P&L)
2413+
usePerpsLivePricesMock.mockReturnValue({
2414+
ETH: { price: '100' }, // Current price = entry price
2415+
});
2416+
2417+
const { rerender, getByText, queryByText } = renderWithProvider(
2418+
<PerpsClosePositionView />,
2419+
{
2420+
state: STATE_MOCK,
2421+
},
2422+
true,
2423+
);
2424+
2425+
// Assert initial state - effectivePnL should be close to 0 when price = entry
2426+
// (100 - 100) * 1 = 0
2427+
// The margin displayed should be just the margin used ($100)
2428+
// P&L displayed should be $0 (or very close to it)
2429+
await waitFor(() => {
2430+
const marginLabel = getByText(strings('perps.close_position.margin'));
2431+
expect(marginLabel).toBeDefined();
2432+
// P&L should be ~$0 when price equals entry price
2433+
expect(queryByText(/\+.*\$0/)).toBeDefined();
2434+
});
2435+
2436+
// Act - Simulate live price increasing to $150
2437+
// Update both price and position to reflect the P&L change
2438+
const updatedPosition = {
2439+
...mockPosition,
2440+
unrealizedPnl: '50', // (150 - 100) * 1 = 50
2441+
marginUsed: '150', // 100 initial + 50 P&L
2442+
};
2443+
2444+
usePerpsLivePositionsMock.mockReturnValue({
2445+
positions: [updatedPosition],
2446+
isInitialLoading: false,
2447+
});
2448+
2449+
usePerpsLivePricesMock.mockReturnValue({
2450+
ETH: { price: '150' }, // Live price is $150 (50% profit)
2451+
});
2452+
2453+
// Force re-render with new price to trigger dependency recalculation
2454+
rerender(<PerpsClosePositionView />);
2455+
2456+
// Assert - effectivePnL should update to positive value
2457+
// (150 - 100) * 1 = 50 profit
2458+
// Margin should now include the P&L: 100 + 50 = 150
2459+
await waitFor(() => {
2460+
// Look for the positive P&L display in the "includes P&L" row
2461+
expect(queryByText(/\+.*\$50/)).toBeDefined();
2462+
// Look for the margin label to ensure we're checking the right section
2463+
expect(getByText(strings('perps.close_position.margin'))).toBeDefined();
2464+
});
2465+
});
2466+
23722467
it('syncs input amount when price updates from entry to live price', async () => {
23732468
// Arrange - Position with entry price
23742469
const mockPosition = {

0 commit comments

Comments
 (0)