Skip to content

fix(perps): use recorded fees for historical orders - #34514

Open
geositta wants to merge 8 commits into
mainfrom
fix/TAT-3693-use-recorded-fees-perps-history
Open

fix(perps): use recorded fees for historical orders#34514
geositta wants to merge 8 commits into
mainfrom
fix/TAT-3693-use-recorded-fees-perps-history

Conversation

@geositta

@geositta geositta commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Description

Historical Perps order details recalculated fees using the current fee schedule, which could differ from the fees charged when the order executed.

This change associates orders with their execution fills and displays the summed recorded fee. It also replaces the estimated MetaMask/Hyperliquid breakdown with one accurate Total fee row and prevents loading failures from appearing as $0.

Fees are derived from matching fills rather than order status because canceled orders may have partial fills that incurred fees, while triggered orders may not use the literal Filled status.

Expected QA behavior: Recent fully filled orders may show the same total as main — that is expected when current rates match execution-time rates. The clearest regression to verify is a partially filled, then canceled order: main showed $0 fees; this branch should show the sum of fees from those partial fills. Larger divergence is more likely on older orders or after VIP/builder fee tier changes.

  • Orders outside the 90-day fill window now show — when no fills match.
  • Covered orders without fills still show $0.
  • Matching recorded fills are summed regardless of age.
  • Both detail views now pass the transaction timestamp.

Changelog

CHANGELOG entry: Fixed historical Perps orders displaying fees calculated from current rates.

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/TAT-3693

Manual testing steps

Feature: Historical Perps order fees

  Scenario: user views a historical Perps order
    Given the order has one or more recorded execution fills

    When user opens the order details
    Then the total fee equals the sum of the recorded fill fees

Screenshots/Recordings

Before

Simulator Screenshot - iPhone 17 Pro - 2026-08-07 at 19 50 13 Simulator Screenshot - iPhone 17 Pro - 2026-08-07 at 19 50 49

After

Simulator Screenshot - iPhone 17 Pro - 2026-08-07 at 19 50 13 Simulator Screenshot - iPhone 17 Pro - 2026-08-07 at 19 50 49

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
    • Ideally on a mid-range device; emulator is acceptable
  • I've tested with a power user scenario
    • Use these power-user SRPs to import wallets with many accounts and tokens
  • I've instrumented key operations with Sentry traces for production performance metrics

For performance guidelines and tooling, see the Performance Guide.

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 how historical trading fees are sourced and displayed, which affects user-facing financial accuracy. Also tightens fill-history fetch lifecycle around connection and stale requests.

Overview
Historical Perps order details now show the actual fees charged at execution instead of recalculating from the current fee schedule.

Adds usePerpsRecordedOrderFees, which sums fees from fills matching an order ID. Both PerpsOrderTransactionView and PerpsDetails switch to this hook and collapse the MetaMask/Hyperliquid breakdown into a single Total fee row. Loading or unavailable lookups show instead of $0.

usePerpsMarketFills gains a restHistoryStatus and waits for connection readiness, so fee lookups can distinguish pending, ready, and failed history. Orders now carry orderId so fills can be correlated, including partial fills on canceled orders.

Reviewed by Cursor Bugbot for commit 1640437. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
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.

@metamask-ci metamask-ci Bot added the team-perps Perps team label Aug 7, 2026
@metamask-ci

metamask-ci Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

PR template — items to address before "Ready for review"

Warnings — informational, address before merging:

  • Pre-merge author checklist has unchecked items (e.g. "I've tested on Android"). Every box must be consciously checked — see docs/readme/ready-for-review.md.

See docs/readme/ready-for-review.md for the full Definition of Ready for Review.

@github-actions github-actions Bot added the size-M label Aug 7, 2026
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

🧪 Flaky unit test detection

Run history flaky detection

View recent run history

Historical failure rate is a hint, not proof — review each suggestion in context. See the flaky-test-detection skill for the full pattern reference and manual audit workflow.

Failures / runs sampled per window:

File 7d 15d 30d
app/components/UI/Perps/components/PerpsMarketTradesList/PerpsMarketTradesList.test.tsx 0/90 0/167 0/344
app/components/UI/Perps/hooks/usePerpsMarketFills.test.ts 0/90 0/167 0/344
app/components/UI/Perps/utils/transactionTransforms.test.ts 0/90 0/167 0/344

AI-detected flaky patterns

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

  • J3 — Missing jest.clearAllMocks() / jest.resetAllMocks() (high)
    • The file uses jest.clearAllMocks() in beforeEach AND jest.resetAllMocks() in afterEach. resetAllMocks wipes all mock implementations, which is why the beforeEach must re-establish the analytics mock every time. However, there is no top-level jest.mock('../../../../hooks/useAnalytics/useAnalytics', ...) call in this file — the mock is only set up via jest.requireMock inside beforeEach. Without a jest.mock(...) factory at module scope, jest.requireMock returns the real module (or an auto-mock), not a controllable jest.fn(). This means useAnalytics.mockReturnValue(...) will throw or silently fail, and the analytics assertions in the 'Analytics Tracking' describe block will be non-deterministic. The fix is to add a proper jest.mock factory at the top of the file so the module is replaced before any test runs.
    • Suggested fix in app/components/UI/Perps/components/PerpsMarketTradesList/PerpsMarketTradesList.test.tsx:196:
      -  beforeEach(() => {
      -    jest.clearAllMocks();
      -    const { useNavigation } = jest.requireMock('@react-navigation/native');
      -    useNavigation.mockReturnValue({
      -      navigate: mockNavigate,
      -    });
      -    // Set default mock for usePerpsMarketFills
      -    mockUsePerpsMarketFills.mockReturnValue(createMockFillsReturn());
      -
      -    // Re-set up analytics mock (resetAllMocks in afterEach clears implementations)
      -    const { useAnalytics } = jest.requireMock(
      -      '../../../../hooks/useAnalytics/useAnalytics',
      -    );
      -    mockAddProperties.mockReturnValue({ build: mockBuild });
      -    mockCreateEventBuilder.mockReturnValue({
      -      addProperties: mockAddProperties,
      -      build: mockBuild,
      -    });
      -    useAnalytics.mockReturnValue({
      -      trackEvent: mockTrackEvent,
      -      createEventBuilder: mockCreateEventBuilder,
      -    });
      -  });
      -
      -  afterEach(() => {
      -    jest.resetAllMocks();
      -  });
      +// Add at the top of the file alongside the other jest.mock() calls:
      +jest.mock('../../../../hooks/useAnalytics/useAnalytics', () => ({
      +  useAnalytics: jest.fn(),
      +}));
      +
      +// The beforeEach setup of useAnalytics then works correctly:
      +beforeEach(() => {
      +  jest.clearAllMocks();
      +  const { useNavigation } = jest.requireMock('@react-navigation/native');
      +  useNavigation.mockReturnValue({
      +    navigate: mockNavigate,
      +  });
      +  mockUsePerpsMarketFills.mockReturnValue(createMockFillsReturn());
      +
      +  const { useAnalytics } = jest.requireMock(
      +    '../../../../hooks/useAnalytics/useAnalytics',
      +  );
      +  mockAddProperties.mockReturnValue({ build: mockBuild });
      +  mockCreateEventBuilder.mockReturnValue({
      +    addProperties: mockAddProperties,
      +    build: mockBuild,
      +  });
      +  useAnalytics.mockReturnValue({
      +    trackEvent: mockTrackEvent,
      +    createEventBuilder: mockCreateEventBuilder,
      +  });
      +});
      +
      +// Change afterEach to use clearAllMocks instead of resetAllMocks
      +// to avoid wiping implementations that are set once at module level:
      +afterEach(() => {
      +  jest.clearAllMocks(); // clears call counts; implementations survive to next beforeEach
      +});

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

  • J10 — jest.spyOn without restoreAllMocks() (high)
    • The file calls jest.spyOn(Date, 'now') in at least two individual test bodies ('fetches REST fills on mount with 3-month lookback' and 'uses FILLS_LOOKBACK_MS constant for 3-month window') and jest.spyOn(console, 'error') in 'resets isRefreshing on refresh error'. There is no afterEach(() => { jest.restoreAllMocks(); }) in this file — only jest.clearAllMocks() in beforeEach. clearAllMocks resets call counts and return values but does NOT restore the original implementation of a spy. This means the Date.now spy installed in one test persists into subsequent tests, causing them to see a pinned timestamp instead of the real clock. Similarly, the console.error spy leaks into later tests, silencing real errors. The order in which Jest runs tests can vary (especially with --randomize), making failures intermittent.
    • Suggested fix in app/components/UI/Perps/hooks/usePerpsMarketFills.test.ts:1:
      -describe('usePerpsMarketFills', () => {
      -  // ...
      -  beforeEach(() => {
      -    jest.clearAllMocks();
      -    // ... (no afterEach with restoreAllMocks)
      -  });
      -
      -  // Inside 'REST API fetching' describe:
      -  it('fetches REST fills on mount with 3-month lookback', async () => {
      -    const mockNow = 1700000000000;
      -    jest.spyOn(Date, 'now').mockReturnValue(mockNow);
      -    // ...
      -  });
      -
      -  it('uses FILLS_LOOKBACK_MS constant for 3-month window', async () => {
      -    const mockNow = 1700000000000;
      -    jest.spyOn(Date, 'now').mockReturnValue(mockNow);
      -    // ...
      -  });
      -
      -  // Inside 'refresh functionality' describe:
      -  it('resets isRefreshing on refresh error', async () => {
      -    const consoleError = jest
      -      .spyOn(console, 'error')
      -      .mockImplementation(jest.fn());
      -    // ...
      -  });
      +describe('usePerpsMarketFills', () => {
      +  // ...
      +  beforeEach(() => {
      +    jest.clearAllMocks();
      +    // ...
      +  });
      +
      +  afterEach(() => {
      +    jest.restoreAllMocks(); // restores Date.now, console.error, and any other spies
      +  });
      +
      +  // Individual tests remain unchanged — spyOn calls inside them
      +  // are now automatically restored after each test.
      +});

app/components/UI/Perps/utils/transactionTransforms.test.ts

  • J10 — jest.spyOn without restoreAllMocks() (medium)
    • In the 'silently skips Spot Dust Conversion fills without console.error' test, errorSpy is created via jest.spyOn(console, 'error').mockImplementation() but is never restored — only warnSpy.mockRestore() is called. This means console.error remains silenced for all subsequent tests in the file. Any test that relies on console.error being the real implementation (e.g. to detect unexpected errors) will silently swallow them, masking real failures. The file has no afterEach(() => jest.restoreAllMocks()) guard. The other two similar tests do restore errorSpy, but the first one does not.
    • Suggested fix in app/components/UI/Perps/utils/transactionTransforms.test.ts:
      -it('silently skips Spot Dust Conversion fills without console.error', () => {
      -      const warnSpy = jest.spyOn(console, 'warn').mockImplementation();
      -      const errorSpy = jest.spyOn(console, 'error').mockImplementation();
      -      // ...
      -      warnSpy.mockRestore();
      -      // errorSpy is never restored
      -    });
      -
      -    it('emits console.warn for unknown fill directions instead of console.error', () => {
      -      const warnSpy = jest.spyOn(console, 'warn').mockImplementation();
      -      const errorSpy = jest.spyOn(console, 'error').mockImplementation();
      -      // ...
      -      warnSpy.mockRestore();
      -      errorSpy.mockRestore();
      -    });
      -
      -    it('emits console.warn for empty direction instead of console.error', () => {
      -      const warnSpy = jest.spyOn(console, 'warn').mockImplementation();
      -      const errorSpy = jest.spyOn(console, 'error').mockImplementation();
      -      // ...
      -      warnSpy.mockRestore();
      -      errorSpy.mockRestore();
      -    });
      +it('silently skips Spot Dust Conversion fills without console.error', () => {
      +      const warnSpy = jest.spyOn(console, 'warn').mockImplementation();
      +      const errorSpy = jest.spyOn(console, 'error').mockImplementation();
      +      const dustFill = {
      +        ...mockFill,
      +        direction: 'Spot Dust Conversion',
      +      };
      +      const result = transformFillsToTransactions([dustFill]);
      +      expect(result).toHaveLength(0);
      +      expect(errorSpy).not.toHaveBeenCalled();
      +      expect(warnSpy).not.toHaveBeenCalled();
      +      warnSpy.mockRestore();
      +      errorSpy.mockRestore(); // ← add this line
      +    });

This check is informational only and does not block merging.

@github-actions github-actions Bot added size-L and removed size-M labels Aug 7, 2026
@geositta
geositta force-pushed the fix/TAT-3693-use-recorded-fees-perps-history branch from b43cc91 to 29a4028 Compare August 7, 2026 22:33
@geositta
geositta marked this pull request as ready for review August 7, 2026 22:38
@geositta
geositta requested a review from a team as a code owner August 7, 2026 22:38
Comment thread app/components/UI/Perps/hooks/usePerpsRecordedOrderFees.ts
@github-actions github-actions Bot added the risk:low AI analysis: low risk label Aug 7, 2026
@geositta
geositta force-pushed the fix/TAT-3693-use-recorded-fees-perps-history branch from 29a4028 to fadc851 Compare August 8, 2026 01:07
@github-actions github-actions Bot added risk:medium AI analysis: medium risk and removed risk:low AI analysis: low risk labels Aug 8, 2026
@geositta
geositta force-pushed the fix/TAT-3693-use-recorded-fees-perps-history branch from cce6dca to 09045da Compare August 10, 2026 17:43
Comment thread app/components/UI/Perps/hooks/usePerpsMarketFills.ts Outdated
Comment thread app/components/UI/Perps/hooks/usePerpsMarketFills.ts Outdated
@geositta
geositta force-pushed the fix/TAT-3693-use-recorded-fees-perps-history branch from 89e9c31 to 75f4a25 Compare August 11, 2026 14:58
Comment thread app/components/UI/Perps/hooks/usePerpsMarketFills.ts Outdated
@geositta
geositta force-pushed the fix/TAT-3693-use-recorded-fees-perps-history branch from 75f4a25 to a23957e Compare August 11, 2026 16:31
@geositta
geositta force-pushed the fix/TAT-3693-use-recorded-fees-perps-history branch from a23957e to dbee7fd Compare August 11, 2026 16:49
@github-actions github-actions Bot added size-XL and removed size-L labels Aug 11, 2026
@geositta
geositta force-pushed the fix/TAT-3693-use-recorded-fees-perps-history branch from 6979f8c to 72452de Compare August 11, 2026 19:09

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 72452de. Configure here.

Comment thread app/components/UI/Perps/hooks/usePerpsMarketFills.ts
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokePerps, SmokeWalletPlatform, SmokeConfirmations
  • Selected Performance tags: @PerformancePreps
  • Risk Level: medium
  • AI Confidence: 88%
click to see 🤖 AI reasoning details

E2E Test Selection:
The PR modifies Perps transaction history and order details functionality:

  1. New usePerpsRecordedOrderFees hook: Replaces the estimated fee calculation (usePerpsOrderFees) with actual recorded fees from fill history. This is a behavioral change in how fees are displayed in order transaction views.

  2. usePerpsMarketFills enhancement: Added restHistoryStatus state ('pending'/'loading'/'ready'/'error') and connection-aware fetching. This affects when and how historical fill data is loaded, which is used in the Perps transaction history views.

  3. PerpsOrderTransactionView and PerpsDetails.tsx: Both now show a single "Total fee" row (from recorded fills) instead of the previous three-row breakdown (MetaMask fee, Hyperliquid fee, Total fee). This is a visible UI change in the order transaction detail screens.

  4. transactionTransforms.ts: Added orderId to transformed order objects, enabling the fee lookup by order ID.

Tag selection rationale:

  • SmokePerps: Directly affected - perps order transaction views, fee display, and fill history hooks are all changed. Perps smoke tests cover position management and order flows that would exercise these code paths.
  • SmokeWalletPlatform: Required per SmokePerps tag description - "Perps is also a section inside the Trending tab (SmokeWalletPlatform); changes to Perps views affect Trending."
  • SmokeConfirmations: Required per SmokePerps tag description - "When selecting SmokePerps, also select SmokeConfirmations (Add Funds deposits are on-chain transactions)."

No other feature areas (accounts, network, browser, snaps, swap, etc.) are affected by these changes.

Performance Test Selection:
The changes to usePerpsMarketFills add connection-aware fetching with status tracking and a new request cancellation pattern (requestIdRef). This affects how and when historical fill data is loaded in the Perps feature. The @PerformancePreps tag covers perps market loading and position management flows, which would exercise the modified usePerpsMarketFills hook. The new connection-dependency check (isConnected, isInitialized, isConnecting) could affect the timing of data loading in performance scenarios.

View GitHub Actions results

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.92857% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.17%. Comparing base (cbe5e23) to head (1640437).
⚠️ Report is 74 commits behind head on main.

Files with missing lines Patch % Lines
...p/components/UI/Perps/hooks/usePerpsMarketFills.ts 79.41% 5 Missing and 2 partials ⚠️
...onents/UI/Perps/hooks/usePerpsRecordedOrderFees.ts 93.75% 0 Missing and 1 partial ⚠️
...s/Views/ActivityDetails/templates/PerpsDetails.tsx 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #34514      +/-   ##
==========================================
+ Coverage   85.12%   85.17%   +0.04%     
==========================================
  Files        6350     6397      +47     
  Lines      173080   174329    +1249     
  Branches    42804    43214     +410     
==========================================
+ Hits       147333   148483    +1150     
- Misses      15708    15722      +14     
- Partials    10039    10124      +85     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

⚡ Performance Test Results

ℹ️ Performance test results are currently non-blocking and will not block this PR.

All tests passed · 2 tests · 1 device

📱 Devices tested (1)

Android: Google Pixel 8 Pro (v14.0)

✅ Passed Tests (2)
Test Platform Device Duration Team Recording
Perps add funds Android Google Pixel 8 Pro (v14.0) 9.29s @mm-perps-engineering-team 📹 Watch
Perps open position and close it Android Google Pixel 8 Pro (v14.0) 15.29s @mm-perps-engineering-team 📹 Watch

Branch: fix/TAT-3693-use-recorded-fees-perps-history · Build: E2E · Commit: e8e1fb3 · View full run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:medium AI analysis: medium risk size-XL team-perps Perps team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants