Skip to content

fix(bridge): restore hardware wallet post-trade modal after swap - #33865

Merged
bfullam merged 7 commits into
mainfrom
swaps-4831-restore-hw-post-trade-modal
Aug 3, 2026
Merged

fix(bridge): restore hardware wallet post-trade modal after swap#33865
bfullam merged 7 commits into
mainfrom
swaps-4831-restore-hw-post-trade-modal

Conversation

@bfullam

@bfullam bfullam commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Description

Hardware Bridge swaps stopped opening the post-trade modal after the software graduation path landed in #31688. HW completion kept navigating to Activity with a toast instead of the modal.

This restores HW Bridge post-trade by:

  • Carrying source/dest token and amount context on existing HW submissionParams
  • Capturing Bridge submit settlement (id/hash) and waiting for it before opening the modal
  • Registering the HW Bridge screen as a post-trade notification surface while focused
  • Keeping Send QR scanner-owned completion (completeOnScan) and Send toast + Activity behavior
  • Deferring QR Bridge modal navigation until the scanner return transitionEnd to avoid Android view-insert races
  • Falling back to legacy toast + Activity on remount when settlement metadata is unavailable

Changelog

CHANGELOG entry: Fixed hardware wallet Bridge swaps not opening the post-trade modal after completion

Related issues

Refs: SWAPS-4831

Manual testing steps

Feature: Hardware wallet Bridge post-trade modal

  Background:
    Given I am logged into MetaMask Mobile with a hardware wallet account
    And Bridge is available for that account

  Scenario: Ledger Bridge swap opens post-trade modal
    Given I am on the Bridge screen with a valid quote
    When I confirm the Bridge swap
    And I complete all Ledger signing steps
    Then the post-trade modal should open with the submitted transaction
    And I should not see only the Activity toast path for Bridge

  Scenario: QR Bridge swap opens post-trade after final scan return
    Given I am completing a Bridge swap with a QR hardware wallet
    When I complete the final QR signature scan
    And the scanner returns to the HW progress screen
    Then the post-trade modal should open after the return transition
    And the modal should include transaction id/hash context

  Scenario: Send QR final scan keeps Activity completion
    Given I am completing a Send flow with a QR hardware wallet
    When I complete the final QR signature scan
    Then I should see the submitted toast
    And I should navigate to Activity
    And the post-trade modal should not open

Screenshots/Recordings

Before

N/A

After

N/A

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 navigation, timing, and completion paths across Bridge vs Send HW flows; QR transition gating and remount fallbacks need careful regression on Android and Ledger/QR devices.

Overview
Hardware Bridge swaps again open the post-trade modal instead of only toast + Activity after HW signing completes.

useBridgeConfirm now passes postTradeModalParams (amounts and tokens) on HW submissionParams. The HW lifecycle waits for submitBridgeTx settlement (submittedTransaction), suppresses duplicate post-trade notifications during submit, and registers a notification surface while the HW Bridge screen is focused. On success it resets bridge state, navigates to Bridge view, then opens POST_TRADE_MODAL with transaction id/hash—matching the software path.

QR Bridge no longer finishes on the last scan in HwQrScanner; it **goBack()**s so completion runs on the progress screen. Modal navigation waits for the scanner return transitionEnd to avoid Android view-insert races. Send QR keeps scanner-owned completion via optional completeOnScan.

Remount with already-signed steps and no local submit metadata still falls back to legacy toast + Activity and does not resubmit.

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

@bfullam bfullam self-assigned this Jul 27, 2026
@github-actions

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.

@github-actions github-actions Bot added the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label Jul 27, 2026
@metamask-ci metamask-ci Bot added the team-swaps-and-bridge Swaps and Bridge team label Jul 27, 2026
@github-actions

github-actions Bot commented Jul 27, 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/HardwareWallet/Swaps/HardwareWalletsSwaps.test.tsx 0/146 0/282 0/390
app/components/UI/HardwareWallet/Swaps/HwQrScanner.test.tsx 0/146 0/282 0/390

AI-detected flaky patterns

app/components/UI/HardwareWallet/Swaps/HardwareWalletsSwaps.test.tsx

  • J6 — Arbitrary setTimeout/sleep used as a synchronization barrier (high)
    • The helper flushPromises uses setTimeout(resolve, 0) inside act() as a barrier to flush promises after dispatches and state updates. This is called in multiple tests including the modified 'retries in place on try again after rejection dispatch' test. This is a classic J6 pattern known to be flaky under load/CI. The diff shows continued usage of this helper in updated tests.
    • Suggested fix in app/components/UI/HardwareWallet/Swaps/HardwareWalletsSwaps.test.tsx:
      -async function flushPromises() {
      -  await act(async () => {
      -    await new Promise((resolve) => setTimeout(resolve, 0));
      -  });
      -}
      -
      -const STATUS_RENDERING_CASES = [
      -  {
      -    name: 'submitted',
      -    state: SUBMITTED_STATE,
      -    expectedTitle: 'Transaction submitted',
      -
      +async function flushPromises() {
      +  await act(async () => {
      +    await Promise.resolve();
      +  });
      +}
      +
      +const STATUS_RENDERING_CASES = [
      +  {
      +    name: 'submitted',
      +    state: SUBMITTED_STATE,
      +    expectedTitle: 'Transaction submitted',
      +

app/components/UI/HardwareWallet/Swaps/HwQrScanner.test.tsx

  • J10 — jest.spyOn() without restoreAllMocks()/mockRestore() afterward (medium)
    • The beforeEach clears mocks but there is no afterEach with restoreAllMocks. Tests use jest.spyOn(Linking, 'openURL').mockResolvedValue and jest.spyOn(Linking, 'openSettings') without restoring the original methods. This matches J10 and can cause leakage. The modified tests for scan success do not introduce new spies but the pattern exists in the file.
    • Suggested fix in app/components/UI/HardwareWallet/Swaps/HwQrScanner.test.tsx:
      -describe('HwQrScanner', () => {
      -  beforeEach(() => {
      -    jest.clearAllMocks();
      -    mockCreateEventBuilder.mockReturnValue({
      -      addProperties: mockAddProperties,
      -    });
      -    mockAddProperties.mockReturnValue({ build: mockBuild });
      -    mockBuild.mockReturnValue({ event: 'hardware-wallet-error' });
      -    capturedOnScanSuccess = undefined;
      -    jest.requireMock('uuid').stringify.mockReturnValue('test-request-id');
      -    jest
      -      .requireMock('@keystonehq/bc-ur-registry-eth')
      -      .ETHSignature.fromCBOR.mockReturnValue({
      -        getRequestId: jest.fn(() => Buffer.from('test-request-id')),
      -      });
      -    mockUseRoute.mockReturnValue({ params: { currentStep: 1, totalSteps: 2 } });
      -    mockUseAnimatedQrScanner.mockImplementation(
      -      (options: { onScanSuccess: typeof capturedOnScanSuccess }) => {
      -        capturedOnScanSuccess = options.onScanSuccess;
      -        return mockScannerResult;
      -      },
      -    );
      -    mockUseHardwareWallet.mockReturnValue({
      -      walletType: 'qr',
      -      qr: {
      -        pendingScanRequest: {
      -          type: 'sign',
      -          request: {
      -            requestId: 'test-request-id',
      -            payload: { type: 'eth-sign-request', cbor: 'aabbccdd' },
      -          },
      -        },
      -        isSigningQRObject: true,
      -        cancelQRScanRequestIfPresent: mockCancelQRScanRequestIfPresent,
      -        setRequestCompleted: mockSetRequestCompleted,
      -        isRequestCompleted: false,
      -      },
      -    });
      -  });
      -
      +describe('HwQrScanner', () => {
      +  beforeEach(() => {
      +    jest.clearAllMocks();
      +    mockCreateEventBuilder.mockReturnValue({
      +      addProperties: mockAddProperties,
      +    });
      +    mockAddProperties.mockReturnValue({ build: mockBuild });
      +    mockBuild.mockReturnValue({ event: 'hardware-wallet-error' });
      +    capturedOnScanSuccess = undefined;
      +    jest.requireMock('uuid').stringify.mockReturnValue('test-request-id');
      +    jest
      +      .requireMock('@keystonehq/bc-ur-registry-eth')
      +      .ETHSignature.fromCBOR.mockReturnValue({
      +        getRequestId: jest.fn(() => Buffer.from('test-request-id')),
      +      });
      +    mockUseRoute.mockReturnValue({ params: { currentStep: 1, totalSteps: 2 } });
      +    mockUseAnimatedQrScanner.mockImplementation(
      +      (options: { onScanSuccess: typeof capturedOnScanSuccess }) => {
      +        capturedOnScanSuccess = options.onScanSuccess;
      +        return mockScannerResult;
      +      },
      +    );
      +    mockUseHardwareWallet.mockReturnValue({
      +      walletType: 'qr',
      +      qr: {
      +        pendingScanRequest: {
      +          type: 'sign',
      +          request: {
      +            requestId: 'test-request-id',
      +            payload: { type: 'eth-sign-request', cbor: 'aabbccdd' },
      +          },
      +        },
      +        isSigningQRObject: true,
      +        cancelQRScanRequestIfPresent: mockCancelQRScanRequestIfPresent,
      +        setRequestCompleted: mockSetRequestCompleted,
      +        isRequestCompleted: false,
      +      },
      +    });
      +  });
      +
      +  afterEach(() => {
      +    jest.restoreAllMocks();
      +  });
      +

This check is informational only and does not block merging.

@bfullam bfullam removed the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label Jul 27, 2026
@bfullam
bfullam marked this pull request as ready for review July 27, 2026 17:56
@bfullam
bfullam requested review from a team as code owners July 27, 2026 17:56
@github-actions github-actions Bot added the risk:medium AI analysis: medium risk label Jul 27, 2026
Comment thread app/components/UI/HardwareWallet/Swaps/useHwSwapLifecycle.ts
@bfullam
bfullam requested a review from a team as a code owner July 28, 2026 09:05
bfullam added 4 commits July 28, 2026 11:08
Hardware Bridge swaps stopped opening the post-trade modal after the
software graduation path landed. Carry modal token context through HW
submission, wait for settlement (and QR return transition), then open
post-trade while keeping Send QR scanner-owned completion.
The prior commit routed Bridge Done through completeSignedFlow, which no-oped
while submission was pending and left a dead button. Restore the original
Activity navigation and keep Bridge Done coverage on the default renderer.
Use AppStackNavigationProp so transitionEnd is a valid event, and type the
test listener mock so mock.calls indexing is not never[].
@bfullam
bfullam force-pushed the swaps-4831-restore-hw-post-trade-modal branch from ed630eb to ec3c8b0 Compare July 28, 2026 09: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 ec3c8b0. Configure here.

Comment thread app/components/UI/HardwareWallet/Swaps/useHwSwapLifecycle.ts
bfullam added 2 commits July 28, 2026 11:19
Run the all-signed remount path before the Waiting-only submit gate so
Submitted remounts still fall back to legacy completion instead of hanging.
@bfullam
bfullam enabled auto-merge July 31, 2026 10:56
GeorgeGkas
GeorgeGkas previously approved these changes Jul 31, 2026
ccharly
ccharly previously approved these changes Jul 31, 2026

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

Code LGTM!

@bfullam
bfullam added this pull request to the merge queue Jul 31, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 31, 2026
@bfullam
bfullam added this pull request to the merge queue Jul 31, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 31, 2026
@bfullam
bfullam added this pull request to the merge queue Jul 31, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 31, 2026
@bfullam
bfullam added this pull request to the merge queue Aug 3, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 3, 2026
@bfullam
bfullam dismissed stale reviews from ccharly and GeorgeGkas via 8712054 August 3, 2026 11:38
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeSwap, SmokeConfirmations, SmokeAccounts
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: medium
  • AI Confidence: 82%
click to see 🤖 AI reasoning details

E2E Test Selection:
The PR modifies the Bridge + Hardware Wallet signing flow in several interconnected ways:

  1. useBridgeConfirm/index.ts: Adds postTradeModalParams (source/dest token amounts) to the navigation params when routing to the hardware wallet signing screen for bridge transactions. This is part of the bridge confirmation flow.

  2. HwQrScanner.tsx: Introduces a completeOnScan param. Bridge flows now call goBack() on the last scan (delegating completion to useHwSwapLifecycle), while Send flows retain scanner-owned completion via completeOnScan: true.

  3. HardwareWalletsSwaps.tsx: Passes completeOnScan: true only for Send flows when navigating to the QR scanner.

  4. flowStrategy.ts: Adds postTradeModalParams to the SubmissionParams interface.

  5. useHardwareWalletSubmit.ts: Exposes submittedTransaction state, adds withPostTradeNotificationSuppression for bridge flows, and returns transaction metadata after submission.

  6. useHwSwapLifecycle.ts: Major refactor - Bridge flows now navigate to POST_TRADE_MODAL instead of TRANSACTIONS_VIEW after signing completes. Adds QR transition tracking (isQrReturnTransitionEnded), post-trade notification surface management, and didStartBridgeSubmitRef to distinguish remounts.

Tag Rationale:

  • SmokeSwap: Directly impacts bridge/swap flows with hardware wallets. The useBridgeConfirm hook and useHwSwapLifecycle changes affect how bridge transactions complete after hardware wallet signing. Bridge is part of the swap/bridge trading flow.
  • SmokeConfirmations: The hardware wallet signing flow is a confirmation flow. Changes to how signing completes (post-trade modal vs. transactions view navigation) directly affect transaction confirmation UX. Also selected per SmokeSwap dependency guidance.
  • SmokeAccounts: Hardware wallet accounts (QR/Ledger) are the primary subject of these changes. The QR scanner behavior change and the lifecycle changes affect hardware wallet account signing flows.

Not selected:

  • SmokeNetworkAbstractions: No network management UI changes
  • SmokeWalletPlatform: No activity/transaction history display changes (the navigation destination changed but no history component changes)
  • SmokePerps/SmokePredictions/SmokeMoney: Not affected by these bridge HW wallet changes

Performance Test Selection:
The changes are focused on the hardware wallet signing flow for bridge transactions - specifically the post-signing navigation (opening a post-trade modal instead of navigating to transactions view). These are UI flow/navigation changes that don't affect app launch, asset loading, account list rendering, onboarding, login, or swap quote performance. No performance-sensitive code paths (rendering loops, data fetching, state initialization) are modified. No performance spec files were changed.

View GitHub Actions results

@sonarqubecloud

sonarqubecloud Bot commented Aug 3, 2026

Copy link
Copy Markdown

@bfullam
bfullam enabled auto-merge August 3, 2026 14:26
@bfullam
bfullam added this pull request to the merge queue Aug 3, 2026
Merged via the queue into main with commit 3aa4425 Aug 3, 2026
407 of 414 checks passed
@bfullam
bfullam deleted the swaps-4831-restore-hw-post-trade-modal branch August 3, 2026 19:40
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 3, 2026
@metamask-ci metamask-ci Bot added the release-8.7.0 Issue or pull request that will be included in release 8.7.0 label Aug 3, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-8.7.0 Issue or pull request that will be included in release 8.7.0 risk:medium AI analysis: medium risk size-M team-swaps-and-bridge Swaps and Bridge team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants