Skip to content

fix(confirmations): scope MM Pay payment methods to deposit asset (TRAM-3838) - #34469

Draft
saustrie-consensys wants to merge 2 commits into
mainfrom
saustrie/tram-3838-context-payment-methods
Draft

fix(confirmations): scope MM Pay payment methods to deposit asset (TRAM-3838)#34469
saustrie-consensys wants to merge 2 commits into
mainfrom
saustrie/tram-3838-context-payment-methods

Conversation

@saustrie-consensys

@saustrie-consensys saustrie-consensys commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Description

Money account "Pay with" was listing Buy-scoped payment methods (for example Revolut Pay) that cannot be quoted against the deposit asset / headless provider set, which then surfaced alert_system.no_pay_token_quotes.

This PR points all four MM Pay payment-method consumers at a request-only RampsController.getPaymentMethodsForContext call scoped to the same deposit asset path TPC uses for quotes (confirmations_pay_fiat.assetPerTransactionType / defaults), with autoSelectProvider + restrictToKnownOrNativeProviders. Stale TPC fiatPayment.selectedPaymentMethodId values are cleared only after a successful fetch when the id is absent from that list.

Temporary preview dependency (blocked)

Consumes @metamask-previews/ramps-controller@20.0.0-preview-6e2b5fe from core draft PR MetaMask/core#9801 via an explicit npm alias dependency (not previewBuilds) so CocoaPods/autolinking lockfile resolution succeeds in CI. Do not merge until core lands a formal @metamask/ramps-controller release and this PR bumps off the preview. Also includes the addPrecreatedOrder non-empty chainId client guard required by ramps-controller 20.x.

Exp TestFlight consumer build

Successful exp upload for this branch:

Manual on-device QA of the Gherkin scenarios below is still required before formal core release.

Changelog

CHANGELOG entry: Fixed Money account Pay with offering payment methods that cannot produce a deposit quote

Related issues

Fixes: TRAM-3838
Refs: #34109
Refs: MetaMask/core#9801
Refs: TRAM-3856

Manual testing steps

Feature: Money account Pay with payment methods

  Scenario: Buy-only methods are not offered for Money deposit
    Given the user previously selected Revolut Pay (or Venmo / PIX / Interac / Instant ACH) in the standalone Buy flow
    And Money account fiat deposit is available in the user's region

    When the user opens Money account deposit and opens Pay with
    Then Revolut Pay and other methods that are not quotable for the deposit asset are absent
    And only deposit-context payment methods remain

  Scenario: Eligible method can still be selected and quoted
    Given Money account fiat deposit is available
    And at least one delay-eligible payment method exists for the deposit asset

    When the user selects that payment method and continues
    Then a deposit quote is returned without alert_system.no_pay_token_quotes

Install TestFlight build 8.8.0 (6393) from group MetaMask BETA & Release Candidates (workflow https://github.com/MetaMask/metamask-mobile/actions/runs/31185137884) and run the scenarios above on device. Manual QA is not complete until that validation is done.

Screenshots/Recordings

N/A: UI regression is validated on device via TestFlight build 8.8.0 (6393); screenshots/recordings will follow after that install and the manual scenarios above.

Before

N/A pending on-device TestFlight QA

After

N/A pending on-device TestFlight QA

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.

…text

Use RampsController.getPaymentMethodsForContext with the TPC deposit asset
and headless quote flags so Money Pay with no longer lists Buy-only methods
such as Revolut Pay. Temporary preview of @metamask/ramps-controller from
core #9801; also require chainId for addPrecreatedOrder (20.x breaking).
@saustrie-consensys saustrie-consensys added the team-money-movement issues related to Money Movement features label Aug 7, 2026
@saustrie-consensys saustrie-consensys self-assigned this Aug 7, 2026
@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.

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

@socket-security

socket-security Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addednpm/​@​metamask-previews/​ramps-controller@​20.0.0-preview-6e2b5fe781007998100

View full report

@socket-security

socket-security Bot commented Aug 7, 2026

Copy link
Copy Markdown

Caution

MetaMask internal reviewing guidelines:

  • Do not ignore-all
  • Each alert has instructions on how to review if you don't know what it means. If lost, ask your Security Liaison or the supply-chain group
  • Copy-paste ignore lines for specific packages or a group of one kind with a note on what research you did to deem it safe.
    @SocketSecurity ignore npm/PACKAGE@VERSION
Action Severity Alert  (click "▶" to expand/collapse)
Block Medium
Network access: npm @metamask-previews/ramps-controller in module globalThis["fetch"]

Module: globalThis["fetch"]

Location: Package overview

From: package.jsonnpm/@metamask-previews/ramps-controller@20.0.0-preview-6e2b5fe

ℹ Read more on: This package | This alert | What is network access?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should remove all network access that is functionally unnecessary. Consumers should audit network access to ensure legitimate use.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/@metamask-previews/ramps-controller@20.0.0-preview-6e2b5fe. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

@github-actions github-actions Bot added the size-L 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/Ramp/Views/Checkout/Checkout.test.tsx 0/86 0/243 0/361

AI-detected flaky patterns

app/components/UI/Ramp/Views/Checkout/Checkout.test.tsx

  • J4 — waitFor without a real assertion inside (high)
    • A negative assertion (not.toHaveBeenCalled()) inside waitFor is a vacuous pass: waitFor polls until the callback stops throwing, and a negative assertion never throws — it passes immediately on the very first poll, before any async work (e.g. a useEffect or a resolved promise) has had a chance to run. This means the test does not actually verify that mockAddPrecreatedOrder was never called after the async flow settled; it just verifies it wasn't called in the first synchronous tick. The test will always pass even if the production code later calls mockAddPrecreatedOrder asynchronously, making it a false-green that can mask regressions. The correct approach is to let the component settle first (e.g. by awaiting a positive condition that signals the async work is done), then assert the negative condition outside waitFor.
    • Suggested fix in app/components/UI/Ramp/Views/Checkout/Checkout.test.tsx:494:
      -      await waitFor(() => {
      -        expect(mockAddPrecreatedOrder).not.toHaveBeenCalled();
      -      });
      +      // Wait for the component to finish its async initialization before
      +      // asserting the negative condition. Replace the vacuous waitFor with
      +      // a positive sentinel assertion followed by the negative check.
      +      // If there is no reliable positive condition, use act() to flush
      +      // pending microtasks/promises instead.
      +      await act(async () => {
      +        // flush all pending state updates and effects
      +      });
      +      expect(mockAddPrecreatedOrder).not.toHaveBeenCalled();

This check is informational only and does not block merging.

CocoaPods/autolinking yarn resolution failed because previewBuilds type:breaking
removed the ^19.0.0 lockfile descriptor while package.json still declared it.
Pin the preview package with an explicit npm alias so the lockfile descriptor
matches what yarn resolves without the preview-builds plugin remap.
@saustrie-consensys
saustrie-consensys force-pushed the saustrie/tram-3838-context-payment-methods branch from d450d4e to ad611b5 Compare August 7, 2026 13:57
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-money-movement issues related to Money Movement features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant