Skip to content

Commit bbec92d

Browse files
authored
fix(perps): Incorrect PnL and order size displayed in perp market page after SL execution (#28593)
## **Description** The Activity page showed incorrect (lower) PnL and size for trades executed via multiple sub-fills (e.g. a Stop Loss split into 10 partial fills). The `mergedTransactions` useMemo in `usePerpsTransactionHistory` aggregated WebSocket fills into `PerpsTransaction` objects before merging with REST data, allowing the WS partial snapshot to overwrite the correctly aggregated REST result. The fix merges raw `OrderFill` objects at the fill level first (same pattern as `usePerpsHomeData`), then transforms once on the complete set. ## **Changelog** CHANGELOG entry: Fixed a bug where the Activity page showed incorrect PnL and order size for trades executed via multiple partial fills (e.g. Stop Loss). ## **Related issues** Fixes: [TAT-2483](https://consensyssoftware.atlassian.net/browse/TAT-2483) ## **Manual testing steps** ```gherkin Feature: Activity page PnL correctness after multi-fill trade Scenario: Stop Loss execution with multiple sub-fills shows correct aggregated PnL Given wallet is unlocked and perps feature is enabled And a Stop Loss order was executed via multiple partial sub-fills When user navigates to Activity page → Trades tab Then the trade row PnL matches the value shown on the market detail page And no TypeError appears in Metro logs And no BUG_MARKER fires in Metro logs ``` ## **Screenshots/Recordings** ### **Before** <!-- before.mp4 — Activity page showing incorrect PnL (WS partial snapshot overwrites REST aggregate) --> ### **After** <!-- after.mp4 — Activity page showing correct PnL after raw-fill-level merge --> ## **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 — TAT-2483: Activity page shows correct aggregated PnL after multi-fill trade</summary> ```json { "title": "TAT-2483: Activity page shows correct aggregated PnL after multi-fill trade", "jira": "TAT-2483", "acceptance_criteria": [ "Activity page Trades tab loads without error", "Trade transactions are present in the Trades tab", "No BUG_MARKER fires in metro logs (WS overwrite does not corrupt aggregated data)", "No TypeError or undefined errors in metro logs during Activity page render" ], "validate": { "static": ["yarn lint:tsc"], "workflow": { "pre_conditions": ["wallet.unlocked", "perps.feature_enabled"], "entry": "nav-activity", "nodes": { "nav-activity": { "action": "navigate", "target": "PerpsActivity", "params": { "redirectToPerpsTransactions": true }, "next": "wait-render" }, "wait-render": { "action": "wait_for", "route": "PerpsActivity", "next": "check-no-errors" }, "check-no-errors": { "action": "log_watch", "window_seconds": 5, "must_not_appear": ["BUG_MARKER", "TypeError", "undefined is not an object"], "watch_for": ["User fills received"], "next": "check-state" }, "check-state": { "action": "eval_sync", "expression": "JSON.stringify(Engine.context.PerpsController.state.accountState)", "assert": { "operator": "not_null" }, "next": "screenshot-activity" }, "screenshot-activity": { "action": "screenshot", "filename": "evidence-activity-trades.png", "next": "done" }, "done": { "action": "end", "status": "pass" } } } } } ``` </details> [TAT-2483]: https://consensyssoftware.atlassian.net/browse/TAT-2483?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes the perps activity/trade merging logic to merge raw REST and WebSocket fills before transforming, which affects user-facing PnL/size calculations and deduplication across multiple screens. > > **Overview** > Fixes incorrect trade PnL/size for multi-subfill executions (e.g., Stop Loss/Take Profit) by **merging REST and WebSocket data at the raw `OrderFill` level before transforming into `PerpsTransaction`s** in `usePerpsTransactionHistory`. > > Extracts the fill dedupe/merge behavior into a shared `mergeOrderFills` helper (preserving REST `detailedOrderType`/`liquidation` when WS lacks it) and reuses it in `usePerpsHomeData` and `usePerpsMarketFills`. Updates transaction history tests to exercise the real merge behavior and to assert ordering and `detailedOrderType` preservation. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 6645108. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent c3b271a commit bbec92d

5 files changed

Lines changed: 260 additions & 249 deletions

File tree

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

Lines changed: 8 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ import {
1616
type SortField,
1717
} from '@metamask/perps-controller';
1818
import type { PerpsTransaction } from '../types/transactionHistory';
19-
import { transformFillsToTransactions } from '../utils/transactionTransforms';
19+
import {
20+
transformFillsToTransactions,
21+
mergeOrderFills,
22+
} from '../utils/transactionTransforms';
2023
import Engine from '../../../../core/Engine';
2124
import { HOME_SCREEN_CONFIG } from '../constants/perpsConfig';
2225
import { selectPerpsWatchlistMarkets } from '../selectors/perpsController';
@@ -123,40 +126,10 @@ export const usePerpsHomeData = ({
123126
};
124127
}, [isConnected, isInitialized]);
125128

126-
// Merge REST + WebSocket fills with deduplication
127-
// Live fills take precedence over REST fills (more up-to-date)
128-
const mergedFills = useMemo(() => {
129-
// Use Map for efficient deduplication
130-
const fillsMap = new Map<string, OrderFill>();
131-
132-
// Add REST fills first
133-
for (const fill of restFills) {
134-
const key = `${fill.orderId}-${fill.timestamp}-${fill.size}-${fill.price}`;
135-
fillsMap.set(key, fill);
136-
}
137-
138-
// Add live fills (overwrites duplicates from REST - live data is fresher)
139-
// Preserve detailedOrderType from REST fills since WS fills lack it
140-
for (const fill of liveFills) {
141-
const key = `${fill.orderId}-${fill.timestamp}-${fill.size}-${fill.price}`;
142-
const existing = fillsMap.get(key);
143-
if (existing?.detailedOrderType && !fill.detailedOrderType) {
144-
fillsMap.set(key, {
145-
...fill,
146-
detailedOrderType: existing.detailedOrderType,
147-
...(existing.liquidation &&
148-
!fill.liquidation && { liquidation: existing.liquidation }),
149-
});
150-
} else {
151-
fillsMap.set(key, fill);
152-
}
153-
}
154-
155-
// Convert back to array and sort by timestamp descending (newest first)
156-
return Array.from(fillsMap.values()).sort(
157-
(a, b) => b.timestamp - a.timestamp,
158-
);
159-
}, [restFills, liveFills]);
129+
const mergedFills = useMemo(
130+
() => mergeOrderFills(restFills, liveFills),
131+
[restFills, liveFills],
132+
);
160133

161134
// Transform merged fills to PerpsTransaction format for activity display
162135
const tradesOnly = useMemo(

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

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
22
import { useSelector } from 'react-redux';
33
import { usePerpsLiveFills } from './stream';
44
import { PERPS_CONSTANTS, type OrderFill } from '@metamask/perps-controller';
5+
import { mergeOrderFills } from '../utils/transactionTransforms';
56
import Engine from '../../../../core/Engine';
67
import Logger from '../../../../util/Logger';
78
import { ensureError } from '../../../../util/errorUtils';
@@ -133,34 +134,14 @@ export const usePerpsMarketFills = ({
133134
}, [fetchRestFills]);
134135

135136
// Merge REST + WebSocket fills with deduplication, filtered by symbol
136-
// Live fills take precedence over REST fills (more up-to-date)
137-
const fills = useMemo(() => {
138-
// Use Map for efficient deduplication
139-
const fillsMap = new Map<string, OrderFill>();
140-
141-
// Add REST fills first
142-
for (const fill of restFills) {
143-
// Only add fills for the requested symbol
144-
if (fill.symbol === symbol) {
145-
const key = `${fill.orderId}-${fill.timestamp}-${fill.size}-${fill.price}`;
146-
fillsMap.set(key, fill);
147-
}
148-
}
149-
150-
// Add live fills (overwrites duplicates from REST - live data is fresher)
151-
for (const fill of liveFills) {
152-
// Only add fills for the requested symbol
153-
if (fill.symbol === symbol) {
154-
const key = `${fill.orderId}-${fill.timestamp}-${fill.size}-${fill.price}`;
155-
fillsMap.set(key, fill);
156-
}
157-
}
158-
159-
// Convert back to array and sort by timestamp descending (newest first)
160-
return Array.from(fillsMap.values()).sort(
161-
(a, b) => b.timestamp - a.timestamp,
162-
);
163-
}, [restFills, liveFills, symbol]);
137+
const fills = useMemo(
138+
() =>
139+
mergeOrderFills(
140+
restFills.filter((f) => f.symbol === symbol),
141+
liveFills.filter((f) => f.symbol === symbol),
142+
),
143+
[restFills, liveFills, symbol],
144+
);
164145

165146
return {
166147
fills,

0 commit comments

Comments
 (0)