Skip to content

fix(perps): Incorrect PnL and order size displayed in perp market page after SL execution#27906

Open
abretonc7s wants to merge 4 commits intomainfrom
fix/perps/tat-2483-0325-1840
Open

fix(perps): Incorrect PnL and order size displayed in perp market page after SL execution#27906
abretonc7s wants to merge 4 commits intomainfrom
fix/perps/tat-2483-0325-1840

Conversation

@abretonc7s
Copy link
Contributor

@abretonc7s abretonc7s commented Mar 25, 2026

Description

Multi-fill trades (e.g., stop-loss orders split across multiple price levels by HyperLiquid) showed incorrect PnL and order size on the Perp Market screen and Perps Home screen. The deduplication key orderId-timestamp used to merge REST and WebSocket fills collapsed distinct fills with the same orderId+timestamp into one entry, losing size/PnL data. Fixed by extending the dedup key to orderId-timestamp-size-price, which preserves all distinct fills while still deduplicating identical entries from both sources. The Activity page was already correct (no Map-based dedup).

Changelog

Fixed incorrect PnL and order size for multi-fill trades on the Perp Market screen and Home screen recent activity

Related issues

Fixes: TAT-2483

Manual testing steps

Feature: Multi-fill trade aggregation consistency
  Scenario: SL execution with multiple fills shows correct aggregated values
    Given the user has an account with SL trades that executed as multiple fills
    When the user navigates to the ETH market page trades tab
    Then the PnL and order size match the Activity page values
    And the PnL and order size match the HyperLiquid UI aggregated view

Screenshots/Recordings

Before

See .task/fix/tat-2483-0325-1840/artifacts/before.mp4

After

See .task/fix/tat-2483-0325-1840/artifacts/after.mp4

Validation Recipe

Automated validation recipe (validate-recipe.sh)
{
  "pr": "27906",
  "title": "Verify aggregated PnL and order size consistency across all trade history surfaces",
  "jira": "TAT-2483",
  "acceptance_criteria": [
    "Perp Market screen displays correct aggregated PnL and order size for multi-fill trades",
    "Perps Home screen recent activity shows identical aggregated values",
    "Activity page shows the same aggregated values as the other two surfaces",
    "Multi-fill trades (same orderId + timestamp) are properly aggregated, not collapsed by dedup"
  ],
  "validate": {
    "static": ["yarn lint:tsc"],
    "runtime": {
      "pre_conditions": ["wallet.unlocked", "perps.feature_enabled"],
      "steps": [
        {
          "id": "check_multi_fills_exist",
          "description": "Verify account has multi-fill trades (prerequisite for the bug)",
          "action": "eval_async",
          "expression": "Engine.context.PerpsController.getActiveProviderOrNull().getOrderFills({aggregateByTime: false}).then(function(fills) { var grouped = {}; fills.forEach(function(f) { var key = f.orderId + '_' + f.timestamp; if (!grouped[key]) grouped[key] = 0; grouped[key] = grouped[key] + 1; }); var multiCount = 0; Object.keys(grouped).forEach(function(k) { if (grouped[k] > 1) multiCount = multiCount + 1; }); return JSON.stringify({totalFills: fills.length, multiFillKeys: multiCount}); })",
          "assert": {
            "operator": "gt",
            "field": "multiFillKeys",
            "value": 0
          }
        },
        {
          "id": "verify_new_dedup_preserves_fills",
          "description": "Assert that the fixed dedup key (orderId+timestamp+size+price) preserves all fills - this fails with old key",
          "action": "eval_async",
          "expression": "Engine.context.PerpsController.getActiveProviderOrNull().getOrderFills({aggregateByTime: false}).then(function(fills) { var oldKeyMap = {}; var newKeyMap = {}; fills.forEach(function(f) { var oldKey = f.orderId + '_' + f.timestamp; var newKey = f.orderId + '_' + f.timestamp + '_' + f.size + '_' + f.price; oldKeyMap[oldKey] = f; newKeyMap[newKey] = f; }); var oldCount = Object.keys(oldKeyMap).length; var newCount = Object.keys(newKeyMap).length; return JSON.stringify({totalFills: fills.length, oldKeyUnique: oldCount, newKeyUnique: newCount, oldKeyLost: fills.length - oldCount, newKeyLost: fills.length - newCount}); })",
          "assert": {
            "operator": "eq",
            "field": "newKeyLost",
            "value": 0
          }
        },
        {
          "id": "nav_market_eth",
          "description": "Navigate to ETH market page (has multi-fill SL trades)",
          "action": "flow_ref",
          "ref": "market-discovery",
          "params": { "symbol": "ETH" }
        },
        {
          "id": "wait_market_render",
          "description": "Wait for trades list to render with REST fills",
          "action": "wait",
          "ms": 5000
        },
        {
          "id": "screenshot_market",
          "description": "Capture market trades list for visual review",
          "action": "screenshot",
          "filename": "market-trades-eth"
        },
        {
          "id": "nav_activity",
          "description": "Navigate to activity page trades tab",
          "action": "flow_ref",
          "ref": "activity-view",
          "params": { "tab": "trades" }
        },
        {
          "id": "wait_activity_render",
          "description": "Wait for activity data to load",
          "action": "wait",
          "ms": 3000
        },
        {
          "id": "screenshot_activity",
          "description": "Capture activity trades for visual comparison",
          "action": "screenshot",
          "filename": "activity-trades"
        }
      ]
    }
  }
}

Pre-merge author checklist

  • I've followed MetaMask Contributor Docs and Coding Standards
  • I've completed the PR template to the best of my ability
  • I've included tests if applicable
  • I've documented my code using JSDoc format if applicable
  • I've applied the right labels on the PR

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.

Note

Medium Risk
Changes fill deduplication/merging used to compute displayed trade size and PnL on the Perps Home and Market pages; mistakes could cause missing or duplicated fills in activity lists. Scope is limited to client-side aggregation plus added test coverage.

Overview
Fixes Perps Home (usePerpsHomeData) and Market (usePerpsMarketFills) fill merging so multi-fill executions (same orderId + timestamp) are no longer collapsed into a single fill.

Deduplication now keys on orderId-timestamp-size-price, still allowing exact REST/WS duplicates to be overwritten by live data while preserving distinct partial fills; new tests cover multi-fill preservation and exact-duplicate preference.

Written by Cursor Bugbot for commit e586bf1. This will update automatically on new commits. Configure here.

@abretonc7s abretonc7s added DO-NOT-MERGE Pull requests that should not be merged team-perps Perps team labels Mar 25, 2026
@github-actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@github-actions
Copy link
Contributor

🔍 Smart E2E Test Selection

⏭️ Smart E2E selection skipped - draft PR

All E2E tests pre-selected.

View GitHub Actions results

@sonarqubecloud
Copy link

@abretonc7s
Copy link
Contributor Author

Report: TAT-2483 — Incorrect PnL and order size on Perp Market page after SL execution

Summary

Multi-fill trades (e.g., stop-loss orders split across multiple price levels) displayed incorrect PnL and order size on the Perp Market screen and Perps Home screen. The deduplication key used to merge REST and WebSocket fills collapsed distinct fills into one, losing data. The Activity page was unaffected because it doesn't use Map-based dedup.

Root Cause

File: app/components/UI/Perps/hooks/usePerpsMarketFills.ts:145,154
File: app/components/UI/Perps/hooks/usePerpsHomeData.ts:134,140

The dedup key ${fill.orderId}-${fill.timestamp} was insufficient for multi-fill trades. When HyperLiquid splits a SL/TP order across multiple fills (same orderId and timestamp but different sizes/prices), the Map kept only the last fill per key. CDP eval confirmed: 901 total fills, 13 orderId+timestamp keys had multiple fills — old key produced 888 unique keys (13 fills lost), new key orderId-timestamp-size-price produced 901 (0 lost).

Reproduction Commit

SHA: cea4fe6840debug(pr-27906): add reproduction marker for multi-fill dedup collapse

Metro log excerpt (BUG_MARKER detection):

[PR-27906] BUG_MARKER: fills collapsed by dedup - input=N output=M symbol=ETH

Changes

File Change
usePerpsMarketFills.ts Changed dedup key from orderId-timestamp to orderId-timestamp-size-price (2 occurrences)
usePerpsHomeData.ts Same dedup key fix (2 occurrences)
usePerpsMarketFills.test.ts Updated duplicate test to use identical fills; added multi-fill preservation test
usePerpsHomeData.test.ts Added multi-fill preservation test verifying aggregated size

Test Plan

Automated

  • yarn jest usePerpsMarketFills.test.ts — 20/20 passed
  • yarn jest usePerpsHomeData.test.ts — 53/53 passed
  • yarn lint:tsc — no errors
  • validate-recipe.sh — 8/8 steps passed
  • Recipe verifies: multi-fill keys exist, new dedup key loses 0 fills, market screen renders, activity screen renders

Manual

Feature: Multi-fill trade aggregation
  Scenario: SL execution with multiple fills shows correct PnL
    Given user has an account with SL trades that executed as multiple fills
    When user navigates to the ETH market page trades tab
    Then PnL and size match the Activity page values
    And PnL and size match the HyperLiquid UI aggregated view

Evidence

  • before.mp4 — recipe run on buggy code (dedup key = orderId-timestamp)
  • after.mp4 — recipe run on fixed code (dedup key = orderId-timestamp-size-price)
  • Screenshots captured by recipe: market-trades-eth.png, activity-trades.png

Ticket

@abretonc7s abretonc7s marked this pull request as ready for review March 25, 2026 11:47
@abretonc7s abretonc7s requested a review from a team as a code owner March 25, 2026 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DO-NOT-MERGE Pull requests that should not be merged team-perps Perps team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant