Skip to content

feat: adopt optimized Money Account deposit quote pipeline#33433

Draft
pedronfigueiredo wants to merge 3 commits into
pnf/profile-lag-regressionfrom
pnf/money-account-quote-latency-poc
Draft

feat: adopt optimized Money Account deposit quote pipeline#33433
pedronfigueiredo wants to merge 3 commits into
pnf/profile-lag-regressionfrom
pnf/money-account-quote-latency-poc

Conversation

@pedronfigueiredo

@pedronfigueiredo pedronfigueiredo commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Description

This builds on the Money Account loading-state work in MetaMask/metamask-mobile#33325 and is the first Mobile adoption of the generic amount-update-and-quote pipeline introduced by MetaMask/core#9543.

Money Account deposits are the initial scope because one amount change coherently updates required assets plus approval and deposit calldata, making the duplicated parent rebuild/quote-generation cost especially visible. The core APIs remain generic and can be adopted by other Transaction Pay flows after their correctness and strategy requirements are validated.

This PR:

  • adds a Money Account amount-preparation callback that preserves previewDeposit, the existing 0.2% minimum-mint protection, amount precision, required assets, and approval/deposit calldata;
  • wires the callback and new controller messenger action into Engine;
  • routes only generic/convert Money Account deposits through TransactionPayController.updateAmount when the version-gated moneyAccountDepositQuotePipeline flag or local override is enabled;
  • keeps explicit addMusd and card intents on the existing pipeline until their max/gas-station and multi-stage fiat behavior is validated separately;
  • hands the optimized path into the continuous loading-state behavior provided by the base PR and keeps confirmation disabled until the matching quote is executable;
  • retains the existing path as the disabled/default behavior and kill switch;
  • adds focused tests for preparation, callback wiring, feature-flag behavior, and legacy/new-path selection.

Benchmark

Using the same anchors in two iOS simulator React Native DevTools traces:

Segment Baseline PoC Change
Update/Done → Relay request 3,393.546 ms 1,512.891 ms -55.4%
Relay response → executable quote 1,001.045 ms 805.860 ms -19.5%
Client-controlled total 4,394.591 ms 2,318.751 ms -47.2%

The observed total was 8.503 s → 7.437 s, but Relay independently varied from 4.108 s to 5.119 s. Holding Relay constant at the baseline gives a normalized PoC total of 6.427 s, a 2.076 s / 24.4% end-to-end improvement. These are individual simulator traces, not production percentiles.

Dependency and rollout status

This draft depends on the controller APIs in MetaMask/core#9543. Package versions must be updated after those packages are released; the local validation below used local builds from that core branch.

The new remote flag is registered with a disabled production default.

Security/privacy approval is required before rollout to confirm Sentinel/vendor-disclosure ordering. A Sentinel-blocked revision must never become executable.

Future work

  • Collect alternating old/new samples and physical-device release profiles.
  • Move remaining balance/capability work earlier where correctness permits.
  • Reduce response-to-executable publication/render cycles.
  • Validate Android and broader Transaction Pay adoption separately.

Changelog

CHANGELOG entry: Improved Money Account deposit quote loading performance

Related issues

Refs: MetaMask/core#9543

Refs: #33325

Manual testing steps

Feature: Faster Money Account deposit amount updates

  Scenario: update a Money Account deposit amount through the optimized pipeline
    Given a Money Account deposit confirmation is open
    And the moneyAccountDepositQuotePipeline feature flag is enabled

    When the user changes the deposit amount and taps Update/Done
    Then the required asset and approval/deposit calldata match the new amount
    And only the optimized quote pipeline is used
    And confirmation remains disabled until the matching quote and local preparation complete

  Scenario: use the legacy kill-switch path
    Given a Money Account deposit confirmation is open
    And the moneyAccountDepositQuotePipeline feature flag is disabled

    When the user changes the deposit amount and taps Update/Done
    Then the existing Money Account amount-update path is used

Automated validation:

  • Four focused Mobile suites for this adoption diff: 62 tests passed
  • yarn lint:tsc
  • ESLint for all changed TypeScript files

Screenshots/Recordings

Before

N/A — no visual design change. Baseline trace measured 8.503 seconds from Update/Done to executable quote.

After

N/A — no visual design change. PoC trace measured 7.437 seconds observed / 6.427 seconds normalized to the baseline Relay duration.

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
  • I've tested with a power user scenario
  • I've instrumented key operations with Sentry traces for production performance metrics

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.

@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 16, 2026
@metamask-ci metamask-ci Bot added the team-confirmations Push issues to confirmations team label Jul 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Feature Flag Registry Check

This PR introduces feature flag references that are not yet registered in the
feature flag registry.

Unregistered flags

Flag Referenced in
moneyAccountDepositQuotePipeline app/selectors/featureFlagController/moneyAccount/index.ts
How to fix

Add an entry for each flag in tests/feature-flags/feature-flag-registry.ts:

myNewFlag: {
  name: 'myNewFlag',
  type: FeatureFlagType.Remote,
  inProd: false,
  productionDefault: false,
  status: FeatureFlagStatus.Active,
},

Set inProd and productionDefault to match the current production values from the
client-config API.

If you access the flag via a constant (e.g. remoteFeatureFlags[MY_CONSTANT]), also add the constant to
.github/scripts/known-feature-flag-constants.ts
so the CI check can resolve it.

@github-actions

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/pay/useUpdateTransactionPayAmount.test.ts 0/132 0/159 0/371

AI-detected flaky patterns

app/components/Views/confirmations/hooks/pay/useUpdateTransactionPayAmount.test.ts

  • J6 — Arbitrary sleep / Promise-flush used as synchronization barrier (high)
    • The flushPromises helper drains the microtask queue by awaiting two resolved Promises in sequence. This is a well-known anti-pattern: it relies on the assumption that all async work triggered by the test completes within exactly two microtask ticks. Under CI load, or if the production code adds an extra async hop (e.g. an extra await inside the hook), the flush may resolve before the side-effects have run, causing assertions like expect(updateAtomicBatchDataMock).toHaveBeenCalledTimes(2) to see 0 or 1 calls instead of 2. The correct fix is to await the actual condition using waitFor, which polls until the assertion passes or a timeout is reached.
    • Suggested fix in app/components/Views/confirmations/hooks/pay/useUpdateTransactionPayAmount.test.ts:130:
      -  async function flushPromises() {
      -    await Promise.resolve();
      -    await Promise.resolve();
      -  }
      +// Remove flushPromises entirely and replace each call site with waitFor:
      +
      +// Before (in each test):
      +// result.current.updateTransactionPayAmount('1.23');
      +// await flushPromises();
      +// expect(updateAtomicBatchDataMock).toHaveBeenCalledTimes(2);
      +
      +// After:
      +import { waitFor } from '@testing-library/react-native';
      +
      +// ...
      +
      +  it('calls updateAtomicBatchData for each update returned from updateMoneyAccountDepositTokenAmount', async () => {
      +    updateMoneyAccountDepositTokenAmountMock.mockResolvedValue([
      +      { nestedTransactionIndex: 0, transactionData: '0xaaaa' },
      +      { nestedTransactionIndex: 2, transactionData: '0xbbbb' },
      +    ]);
      +
      +    const { result } = runHook({ transactionMeta: moneyAccountDepositMeta });
      +
      +    result.current.updateTransactionPayAmount('1.23');
      +
      +    await waitFor(() => {
      +      expect(updateAtomicBatchDataMock).toHaveBeenCalledTimes(2);
      +    });
      +
      +    expect(updateAtomicBatchDataMock).toHaveBeenNthCalledWith(1, {
      +      transactionId: expect.any(String),
      +      transactionIndex: 0,
      +      transactionData: '0xaaaa',
      +    });
      +    // ... remaining assertions unchanged
      +  });

This check is informational only and does not block merging.

@pedronfigueiredo
pedronfigueiredo changed the base branch from main to pnf/profile-lag-regression July 16, 2026 16:19
@github-actions github-actions Bot added size-L and removed size-XL labels Jul 16, 2026
@pedronfigueiredo
pedronfigueiredo force-pushed the pnf/money-account-quote-latency-poc branch from ae400c9 to bd3f5ea Compare July 16, 2026 19:01
@pedronfigueiredo
pedronfigueiredo force-pushed the pnf/money-account-quote-latency-poc branch from bd3f5ea to d183541 Compare July 16, 2026 19:10
@sonarqubecloud

Copy link
Copy Markdown

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

Labels

pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. size-L team-confirmations Push issues to confirmations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant