Skip to content

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

Open
geositta wants to merge 7 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 7 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 users see historical trading fees (user-facing money display) and adds async fill lookup edge cases, but scope is limited to Perps order/activity detail UI and improves accuracy versus estimates.

Overview
Fixes incorrect historical order fees by replacing usePerpsOrderFees (current schedule estimates) with usePerpsRecordedOrderFees, which sums fee on fills sharing the order’s orderId. Order history mapping now carries orderId so details views can correlate partial fills (e.g. canceled orders that still paid fees).

PerpsOrderTransactionView and activity PerpsDetails drop the MetaMask/Hyperliquid breakdown and show one Total fee row. Values use while fills load, on history errors, or when the order is outside the fill lookback; covered orders with no matching fills still show $0.

usePerpsMarketFills gains isHistoryLoading / historyError, waits for Perps connection readiness before REST backfill, and ignores stale in-flight fetches on account/symbol changes or refresh overlap.

Reviewed by Cursor Bugbot for commit 72452de. 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/Views/PerpsMarketDetailsView/PerpsMarketDetailsView.test.tsx 0/83 0/162 0/344
app/components/UI/Perps/hooks/usePerpsMarketFills.test.ts 0/83 0/162 0/344
app/components/UI/Perps/utils/transactionTransforms.test.ts 0/83 0/162 0/344

AI-detected flaky patterns

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

  • J8 — jest.useFakeTimers() combined with waitFor (high)
    • Three tests call jest.useFakeTimers() inline without a matching jest.useRealTimers() in afterEach. The afterEach only calls jest.restoreAllMocks(), which does NOT restore real timers. If any of these tests runs before a test that uses waitFor, the fake-timer state leaks into that test and waitFor's internal setTimeout-based polling never fires — causing the subsequent test to hang or time out intermittently depending on test execution order. The file also uses waitFor in other tests (e.g. tooltip tests, pull-to-refresh tests), making this a real cross-test contamination risk.
    • Suggested fix in app/components/UI/Perps/Views/PerpsMarketDetailsView/PerpsMarketDetailsView.test.tsx:
      -it('plays the shimmer before switching from Lite to Pro', async () => {
      -    jest.useFakeTimers();
      -    enableProModeFlag();
      -    const { getByTestId } = renderWithProvider(
      -      <PerpsConnectionProvider>
      -    // ...
      -  it('plays the shimmer before switching from Pro to Lite', async () => {
      -    jest.useFakeTimers();
      -    // ...
      -  it('opens the mode chooser from the pill when the chooser has not been completed', async () => {
      -    jest.useFakeTimers();
      -    // ...
      +afterEach(() => {
      +  jest.restoreAllMocks();
      +  jest.useRealTimers(); // ← add this to prevent fake-timer state from leaking
      +  mockComplianceGate.mockImplementation((action: () => Promise<unknown>) =>
      +    action(),
      +  );
      +});

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

  • J10 — jest.spyOn without restoreAllMocks() (medium)
    • The 'resets isRefreshing on refresh error' test installs a spy on console.error via jest.spyOn(console, 'error').mockImplementation(jest.fn()) but never calls consoleError.mockRestore(). The file's beforeEach only calls jest.clearAllMocks(), which clears call counts but does NOT restore spied-on implementations. As a result, console.error remains silenced for all tests that run after this one (in the same file or in the same Jest worker if module state leaks). This can hide real errors in subsequent tests. An afterEach(() => jest.restoreAllMocks()) guard would prevent this.
    • Suggested fix in app/components/UI/Perps/hooks/usePerpsMarketFills.test.ts:
      -it('resets isRefreshing on refresh error', async () => {
      -      const consoleError = jest
      -        .spyOn(console, 'error')
      -        .mockImplementation(jest.fn());
      -      // ... test body
      -      // consoleError is never restored
      -    });
      +// Add to the describe('usePerpsMarketFills') block:
      +afterEach(() => {
      +  jest.restoreAllMocks();
      +});
      +
      +// Or restore inline at the end of the test:
      +it('resets isRefreshing on refresh error', async () => {
      +  const consoleError = jest
      +    .spyOn(console, 'error')
      +    .mockImplementation(jest.fn());
      +  // ... test body ...
      +  consoleError.mockRestore(); // ← add this
      +});

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
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
@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 changes are focused on the Perps transaction history and order fee display system:

  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 isHistoryLoading/historyError states and connection-aware fetching via usePerpsConnection. This changes the loading behavior of fill history.

  3. PerpsOrderTransactionView: Replaced fee hook, simplified from 3 fee rows (MetaMask fee, Hyperliquid fee, Total fee) to 1 row (Total fee). Shows '—' placeholder during loading/error states.

  4. PerpsDetails.tsx (ActivityDetails template): Same fee hook replacement and simplified display.

  5. transactionTransforms.ts: Added orderId to order data - critical for the new fee lookup to work correctly.

  6. Type changes: PerpsTransaction.order.orderId field added.

SmokePerps: Directly affected - the Perps order transaction views and fee display are changed. Smoke tests cover limit orders, fills, and cancels which exercise the transaction history and fee display.

SmokeWalletPlatform: Required by SmokePerps description - "When selecting SmokePerps, also select SmokeWalletPlatform (Trending section)" since Perps is embedded in Trending.

SmokeConfirmations: Required by SmokePerps description - "When selecting SmokePerps, also select SmokeConfirmations (Add Funds deposits are on-chain transactions)".

All other tags (SmokeAccounts, SmokeNetworkAbstractions, SmokeNetworkExpansion, SmokeSwap, SmokeStake, SmokeMoney, SmokeMultiChainAPI, SmokePredictions, SmokeSeedlessOnboarding, SmokeBrowser, SmokeSnaps, SmokeMMConnect) are unaffected as the changes are isolated to Perps transaction history and fee display.

Performance Test Selection:
The changes affect the Perps order transaction view rendering and fee display logic. The @PerformancePreps tag covers 'perps market loading, position management, add funds flow, and order execution' - the fee display changes in PerpsOrderTransactionView and PerpsDetails could affect rendering performance in order details views. The new hook (usePerpsRecordedOrderFees) introduces additional async loading states (isHistoryLoading) that could impact render timing. Selecting conservatively to validate no performance regression in Perps flows.

View GitHub Actions results

@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.


setRestFills([]);
fetchRestFills();
}, [fetchRestFills, selectedAddress]);

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.

False zero fees without provider

Medium Severity

When getActiveProviderOrNull() is null, historical fill loading ends as a quiet success (isHistoryLoading false, historyError null, empty REST fills). usePerpsRecordedOrderFees then treats in-window orders with no matches as a confirmed $0, so order details can show $0 instead of even though fills were never fetched. That conflicts with this PR’s goal of not presenting failed or incomplete lookups as zero fees.

Additional Locations (1)
Fix in Cursor Fix in Web

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

@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) 420.00s @mm-perps-engineering-team
Perps open position and close it Android Google Pixel 8 Pro (v14.0) 17.67s @mm-perps-engineering-team 📹 Watch

Branch: fix/TAT-3693-use-recorded-fees-perps-history · Build: E2E · Commit: fe91c0a · 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.

1 participant