Skip to content

refactor(confirmations): use canonical transaction approval preparation - #35754

Draft
pedronfigueiredo wants to merge 2 commits into
mainfrom
pnf/canonical-transaction-approval-preparation-handoff-implementation
Draft

refactor(confirmations): use canonical transaction approval preparation#35754
pedronfigueiredo wants to merge 2 commits into
mainfrom
pnf/canonical-transaction-approval-preparation-handoff-implementation

Conversation

@pedronfigueiredo

@pedronfigueiredo pedronfigueiredo commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Related pull requests

Description

Moves Mobile's pre-approval sponsorship and isExternalSign normalization to the canonical prepareTransactionForApproval API in @metamask/transaction-controller.

Mobile continues to own capability discovery and all client-specific behavior: signer-versus-payer handling, waitForResult, Smart Transaction fee-token batch construction, fee-token EIP-7702 external signing, hardware send deferral, ApprovalController acceptance, fiat branching, network enablement, and product navigation.

Characterization coverage was strengthened before the migration for hardware payer/software signer behavior, stale external signing, existing Smart Transaction batches, ignored fee tokens, and metadata passed to hardware signing.

Important

This PR is intentionally a draft and marked DO-NOT-MERGE. The Core implementation is not released yet, so the production dependency bump is not included. The branch was validated using an uncommitted local file: link to the Core worktree. Once Core publishes the API, this PR needs the released/preview dependency version and a final clean-install validation.

Changelog

CHANGELOG entry: null

Related issues

Refs: https://github.com/MetaMask/core

Cross-repository canonical transaction approval preparation handoff; the corresponding Core PR is pending.

Manual testing steps

Feature: Canonical transaction approval preparation

  Scenario: sponsored transaction from a supported software account
    Given a sponsored transaction is awaiting confirmation
    And the selected software account supports the sponsored publication path
    When the user confirms the transaction
    Then sponsorship metadata is retained
    And the existing external publication path is retained

  Scenario: sponsored transaction from an unsupported hardware signer
    Given simulation marked a hardware transaction as sponsored and externally signed
    And the hardware signer cannot use the sponsored relay path
    When the user confirms the transaction
    Then sponsorship is cleared
    And external signing is cleared before the transaction reaches the hardware handoff

  Scenario: hardware payer differs from the software signer
    Given a software signer uses a hardware funding account
    When the user confirms a fee-token transaction
    Then signer capability controls fee-token EIP-7702 preparation
    And the confirmation waits for the hardware payer result

Automated local-integration validation completed against the Core worktree:

  • useTransactionConfirm.test.ts: 54 passed
  • useConfirmActions.test.ts: 16 passed
  • useApprovalRequest.test.ts: 10 passed
  • Core preparation helper tests: 34 passed
  • Core dependency-aware build: passed
  • Changed-file Prettier and ESLint: passed

Full yarn lint:tsc is currently blocked by the pre-existing missing app/util/termsOfUse/termsOfUseContent module.

Screenshots/Recordings

N/A — headless transaction metadata refactor with no UI changes.

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

N/A — this refactor does not add runtime instrumentation or alter rendering/performance-sensitive UI.

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.

@pedronfigueiredo pedronfigueiredo added area-transactions DO-NOT-MERGE Pull requests that should not be merged team-confirmations Push issues to confirmations team labels Sep 4, 2026
@pedronfigueiredo pedronfigueiredo self-assigned this Sep 4, 2026
@github-actions

github-actions Bot commented Sep 4, 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.

@github-actions github-actions Bot added the size-M label Sep 4, 2026
@metamask-ci

metamask-ci Bot commented Sep 4, 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 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

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

E2E Test Selection:
The PR modifies useTransactionConfirm.ts, the core hook that handles transaction confirmation logic. Key changes:

  1. Gas fee sponsorship refactoring: Replaces inline shouldApplyGasFeeSponsorship logic with the new prepareTransactionForApproval function from @metamask/transaction-controller, which normalizes sponsorship state (isGasFeeSponsored, isExternalSign) in a canonical way.

  2. Hardware wallet payer support: New test cases cover scenarios where a hardware payer differs from the software signer, affecting waitForResult and isExternalSign behavior.

  3. EIP-7702 batch transaction handling: Tests verify fee-token transfers are correctly appended to existing batch transactions.

  4. Smart transaction routing: The sponsorship normalization affects how transactions are routed to smart transaction vs. gasless 7702 paths.

Tag selection rationale:

  • SmokeConfirmations: Direct impact — this hook is the core of the transaction confirmation flow. Gas fee sponsorship, hardware wallet signing, EIP-7702 batch transactions, and smart transaction routing are all tested in this suite.
  • SmokeSwap: Swap flows use gasless/smart transactions and go through the confirmation hook. Gasless swap paths are directly affected by the sponsorship normalization change.
  • SmokeStake: Stake flows also go through transaction confirmations and could be affected by the sponsorship logic changes.
  • SmokePerps: PerpsOrderView.tsx directly imports useTransactionConfirm, making it directly impacted by these changes.

The change is a refactoring that delegates sponsorship normalization to an upstream library function. While the intent is equivalent behavior, the new prepareTransactionForApproval function introduces a different code path that warrants validation across all flows that use transaction confirmations with gas fee sponsorship.

Performance Test Selection:
The changes are a refactoring of the gas fee sponsorship logic within the transaction confirmation hook. While this affects transaction flows, it does not impact app launch, asset loading, onboarding, login, or other performance-measured scenarios. The changes are functional/correctness-focused (sponsorship normalization, hardware wallet paths) rather than performance-sensitive (no rendering changes, no data fetching changes, no initialization changes). No performance test tags are warranted.

View GitHub Actions results

@github-actions

github-actions Bot commented Sep 4, 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/Views/confirmations/hooks/transactions/useTransactionConfirm.test.ts 0/144 0/240 0/437

AI-detected flaky patterns

app/components/Views/confirmations/hooks/transactions/useTransactionConfirm.test.ts

  • J3 — Missing jest.clearAllMocks() / jest.resetAllMocks() (high)
    • The test does not clear or reset mocks between test cases, which can lead to shared state across tests. This can cause tests to pass or fail unpredictably based on the order they are run, leading to flaky behavior.
    • Suggested fix in app/components/Views/confirmations/hooks/transactions/useTransactionConfirm.test.ts:134:
      -it('confirms approval request', async () => {
      -  const { result } = renderHook();
      -  await result.current.onConfirm();
      -  expect(onApprovalConfirm).toHaveBeenCalled();
      -});
      +beforeEach(() => {
      +  jest.clearAllMocks(); // Clears call counts, keeps implementations
      +});
      +it('confirms approval request', async () => {
      +  const { result } = renderHook();
      +  await result.current.onConfirm();
      +  expect(onApprovalConfirm).toHaveBeenCalled();
      +});

This check is informational only and does not block merging.

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

Labels

area-transactions DO-NOT-MERGE Pull requests that should not be merged size-M team-confirmations Push issues to confirmations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant