feat: add getPaymentMethodsForContext to ramps-controller - #9801
Merged
saustrie-consensys merged 12 commits intoAug 28, 2026
Conversation
Add a context-scoped payment-method API aligned with getQuotes provider resolution so Headless/MM Pay can list request-eligible methods for a deposit asset without mutating the Buy payment-method catalog (TRAM-3838).
Contributor
Author
|
@metamaskbot publish-previews |
saustrie-consensys
force-pushed
the
saustrie/tram-3838-payment-methods-for-context
branch
from
August 7, 2026 13:25
0a0be4b to
1bbcc4b
Compare
saustrie-consensys
force-pushed
the
saustrie/tram-3838-payment-methods-for-context
branch
from
August 7, 2026 13:25
1bbcc4b to
8321b8c
Compare
Contributor
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
10 tasks
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@metamaskbot publish-previews |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
saustrie-consensys
marked this pull request as draft
August 25, 2026 15:18
saustrie-consensys
marked this pull request as ready for review
August 25, 2026 15:19
This was referenced Aug 26, 2026
Removes the per-request sequence counter that ordered `updateState: true` writes. The counter guarded a race between two same-context stateful requests that no caller can reach: the only consumer passes exactly one explicit provider when writing, so two such requests share a fetch cache key and collapse in `executeRequest`'s pending dedupe. It was also the source of both Bugbot findings on this PR, the second introduced by the fix for the first. The region, asset, and provider equality checks stay, since those close the gap legacy `getPaymentMethods` had. In its place, `updateState: true` throws when provider resolution yields more than one provider, making the unsafe shape impossible rather than ordered. Drops the field-level collision rules from `mergePaymentMethodsById`, keeping dedupe by canonical id with first seen winning. Payment method metadata is provider-invariant: the API serves it from a per-region catalog and the `provider` query narrows that catalog by id-set membership without rewriting fields, so colliding entries carry identical values. Promotes the local asset-id normalizer to an exported `normalizeRampsAssetId` and points `providerServesAsset` at it so the package ships one rule. That also stops non-EVM references being lowercased, which could report a provider as serving an asset it does not serve.
Contributor
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
saustrie-consensys
added a commit
to MetaMask/metamask-mobile
that referenced
this pull request
Aug 27, 2026
Picks up the Core review-response changes on MetaMask/core#9801: the payment-method write sequencing is gone, `updateState: true` now throws for a multi-provider fan-out, provider merging is dedupe-only, and `normalizeRampsAssetId` is exported.
Contributor
Author
mergePaymentMethodsById (and maybe the Promise.allSettled fan-out before it) could move to the backend. Either by modifying getPaymentsV2 to accept a repeated provider (drop assertAtMostOneFilterValuePerQueryKey and widen one type in getPaymentsWithRegions), or by using getRegionV2, which already takes a provider array and returns the same combined list. I looked into it out yesterday, might be worth doing it as a future follow up ticket, since this is becoming a lot of tech debt being handled for one Revolut ticket at this point. Handling more refactoring increases time by a lot since I'm carefully trying to minimize side effects. The rest wouldn't be easily moveable. #resolveProviderIdsForPaymentMethods and #resolveAllProvidersFlag need moneyHeadlessAllProviders and its allowlist. The updateState write path needs userRegion.regionCode, tokens.selected, providers.selected, and paymentMethods.data / .selected, all of which live in Core state. |
Cuts the added test block from 1,554 lines to 437 while keeping every behavioral guard. Most of the saving is a shared `withContext` fixture builder plus `stubPaymentMethods`, `deferPaymentMethods` and `expectBuyCatalogIntact`, replacing 31 sixteen-line state preambles, and hoisting the provider constants out of 38 duplicated local declarations. Deletes the seven `mergePaymentMethodsById` tests. Now that the function is a dedupe loop it stays at 100% from the controller tests alone, and it is not exported from `index.ts`. Removes tests that duplicated each other rather than covering distinct behavior: five variants of one selection-precedence chain, three fail-closed guard variants, two resolution-path variants, and a cache-behavior test whose final call passed `forceRefresh: true` and so could not fail for the reason its name claimed. Extracts `pickPaymentMethod` and `contextStillMatches` in the controller, replacing the selection chain that was written twice and the write guard that was written twice. The selection-precedence contract that needed 215 lines of controller tests is now a 16-line table test on the helper. Adds a direct assertion for `normalizeRampsAssetId` passing a Solana asset id through unchanged. That arm is the reason the function is not a plain `toLowerCase()`, and it was only covered incidentally before. Regenerates `RampsController-method-action-types.ts`, which was left stale by the previous commit's JSDoc change. Sets the package coverage thresholds to the post-trim values. New-code coverage is 91.45%; the package measures 99.72 / 97.52 / 100 / 99.71.
Contributor
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
Only `packages/ramps-controller/CHANGELOG.md` conflicted, where #9955 and #9945 added a `Changed` section alongside this branch's `Fixed` section. Resolved by keeping both in Keep a Changelog order. `RampsController.ts`, `RampsController.test.ts` and `index.ts` merged cleanly: #9955 widens `getProviders` to return the backend ranking metadata and adds two type exports, which is orthogonal to this branch's payment-method context work.
saustrie-consensys
added a commit
to MetaMask/metamask-mobile
that referenced
this pull request
Aug 27, 2026
Picks up the Core test trim and the two extracted helpers (`pickPaymentMethod`, `contextStillMatches`) from MetaMask/core#9801.
`contextStillMatches` took `RampsControllerState`, but both call sites pass Immer's `WritableDraft`. Checking that draft against the full state type exceeds TypeScript's instantiation depth limit during declaration emit, failing the build with TS2589. `tsc --noEmit` accepts it, so `lint:tsc` passed and only `yarn build` caught it. Narrows the parameter to a named `ContextGuardState` covering just the three fields the guard reads. Behaviour is unchanged.
Contributor
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
saustrie-consensys
enabled auto-merge
August 27, 2026 17:54
amitabh94
reviewed
Aug 27, 2026
| * @param assetId - CAIP-19 asset id, in either case form. | ||
| * @returns The asset id in the form the Ramps API uses. | ||
| */ | ||
| export function normalizeRampsAssetId(assetId: string): string { |
Contributor
There was a problem hiding this comment.
Isnt Ramps api giving out caip id?
amitabh94
previously approved these changes
Aug 27, 2026
The previous wording said the providers endpoint returns `supportedCryptoCurrencies` keys in both cases. Measured against prod across gb, us-ca, us-ny, fr, de and br, every `eip155` key there is lowercase; it is `topTokens` that returns four mainnet ids checksummed (USDT, USDC, WBTC, LINK), in both its `topTokens` and `allTokens` lists with no lowercase duplicate. Blaming the providers endpoint sent readers to check the one that is consistent. Also records what the change does and does not do: a plain `toLowerCase()` on both sides already matched EVM ids, so the only behavioral change is the non-EVM passthrough, and current API data has non-EVM ids in identical case on both endpoints, making that a guard against drift rather than a fix for an observed mismatch.
wenfix
approved these changes
Aug 28, 2026
saustrie-consensys
deleted the
saustrie/tram-3838-payment-methods-for-context
branch
August 28, 2026 13:34
saustrie-consensys
added a commit
to MetaMask/metamask-mobile
that referenced
this pull request
Aug 28, 2026
MetaMask/core#9801 exports `normalizeRampsAssetId`, the same rule this module has carried locally as `normalizeAssetIdForApi` since April (#29037). Consumes the Core export and deletes the local copy so the rule has one definition instead of two. The Core version is a superset for every string input: it trims, and its `/^eip155:/iu` namespace test widens the previous `startsWith('eip155:')` so an `EIP155:` prefix is also treated as EVM. Both call sites pass a value the types guarantee is a non-empty string, so Core's narrower `string` signature is safe where the local copy accepted `string | undefined`. The `.trim()` at the query call site is now redundant and dropped; `paymentMethods.test.ts` still covers it by passing a whitespace-padded id.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
Money account "Pay with" previously read Buy-scoped payment methods even though Money quotes use a deposit asset and a different eligible-provider set. That mismatch could offer methods such as Revolut Pay that cannot produce a Money deposit quote (TRAM-3838 / metamask-mobile#34109).
Shared context API and provenance
getPaymentMethodsForContextis a new shared Core API. It combines payment-method fetch and automatic-selection behavior that already existed in Mobile UB2 with Core's existing provider-resolution helpers, while allowing callers to choose whether the result updates Buy state.#resolveProviderIdsForPaymentMethodsis a new adapter over existing Core behavior. It reuses#resolveAllProvidersFlag,#filterProviderIdsBySupport,#getSupportingProvidersForRegion, and#resolveProviderIdsForQuoteinstead of creating a separate provider policy.mergePaymentMethodsByIdwas newly designed for multi-provider fan-out. It was not copied from Mobile.normalizeRampsAssetIdis not new behavior. Mobile has carried the same rule since April 2026 asnormalizeAssetIdForApi(metamask-mobile#29037). This PR promotes it into Core's public API so the rule lives in one place and Mobile can drop its copy.providerServesAssetnow uses it too, which is a behavior change: non-EVM CAIP-19 references are no longer lowercased, since Solana base58 and bitcoin bech32 references are case-sensitive and lowercasing both sides could report a provider as serving an asset it does not serve.Merge scope
mergePaymentMethodsByIdreturns a single provider list unchanged by identity. It preserves the provider's array, row references, ordering, length, and duplicate IDs, so single-provider consumers such as standalone Buy keep verbatim provider output just as legacygetPaymentMethodsdid.Deduping runs only when two or more provider lists are combined, and it keeps the first-seen entry per canonical ID. There is no field-level collision policy, because payment-method metadata is provider-invariant: the API serves it from a per-region catalog (
PaymentMethodsList.ts) and theproviderquery narrows that catalog by ID-set membership (RegionsV2Service.ts,providerPaymentIds.has(payment.id)) without rewriting fields. Two providers returning the same payment ID therefore return identicalscore,delay, andname. The same data is already deduped first-seen-wins by the API itself for its multi-region path.Provider fan-out does vary array order, because the API sorts by reliability per request. That affects ordering, not field values, and
mergePaymentMethodsByIdpreserves first-seen encounter order.Stateful write guards
updateState: truereproduces legacy Buy catalog and selection writes, but it is guarded because the controller mutates shared state from inside the query function. A consumer's query cache cannot undo that side effect after a stale request commits.The write requires the stored region, selected asset, and selected provider to match the request context. Stored region is trimmed and lowercased before comparison, and a missing region remains fail-closed. EVM asset IDs are compared case-insensitively while non-EVM asset case is preserved.
A region-only stale write is reachable when an older region request remains in flight, the user changes region, and the new region repopulates the same asset and provider before the older response returns. Legacy behavior checked asset and provider but not region. These guards close that latent gap rather than adding a cache-only safeguard.
Two same-context stateful requests cannot be ordered by those guards, because the context key falls back to the globally selected provider when more than one provider is passed. Rather than order them with a request sequence,
updateState: truenow throws when provider resolution yields more than one provider, before any fetch is issued. The shared Buy catalog has one slot per region, token, and selected provider, so a fan-out write has nowhere unambiguous to land. Callers wanting a multi-provider catalog passupdateState: falseand read the returnedmethods. UB2 already passes exactly one explicit selected provider, and MMPay deposit contexts are read-only, so no current caller is affected.An earlier revision of this PR ordered these writes with a per-request sequence counter instead. It was removed: the race it guarded is unreachable for a single-provider write (the state context key and the fetch cache key move together, so concurrent duplicates collapse in
executeRequest's pending dedupe), and the counter was the source of both Bugbot findings on this PR, the second introduced by the fix for the first.Catalog writers
paymentMethods.dataandpaymentMethods.selectedhave two writers in Core: the pre-existinggetPaymentMethodsand the newgetPaymentMethodsForContext. Neither one's write guard is aware of the other, so a client calling both for the same context has always been able to interleave them.The consumer PR removes Mobile's last call to
getPaymentMethods(it was inapp/components/UI/Ramp/queries/paymentMethods.tsonmain), so once both land,getPaymentMethodsForContextis Mobile's only writer of that state.getPaymentMethodsstays exported and unchanged for other consumers.Scope boundary
This PR does not include
getSmartSelectedQuoteorapplyQuoteContext. Payment-method context is the minimum shared boundary required by the implemented Money and UB2 consumers. Quote choice and quote-state application still have consumer-specific orchestration and remain deferred to the tracked UB2 dumbification follow-up (TRAM-3856).References
@metamask-previews/ramps-controller@20.0.0-preview-a2a428472a2a428472d39421ffb5aaa04789bfdc45f573bd1Checklist
Note
Medium Risk
Touches payment-method catalog state, provider eligibility matching, and new concurrent-write guards; behavior change for non-EVM asset matching could affect which providers are considered to serve an asset.
Overview
Adds
getPaymentMethodsForContext(and the messenger action) so Money/headless flows can load payment methods for a deposit asset and provider set using the same resolution rules asgetQuotes, without defaulting to the Buy catalog’s selected token/provider. By default the call is request-only; optionalupdateState: truewritespaymentMethodsonly when region, selected token, and selected provider still match, and throws if resolution yields more than one provider.Multi-provider fetches fan out, merge via new
mergePaymentMethodsById/pickPaymentMethod, and tolerate partial failures.normalizeRampsAssetIdis exported and used for comparisons;providerServesAsset/getProvidersServingAssetnow stop lowercasing non-EVM CAIP-19 ids so Solana/bitcoin references stay case-sensitive.Reviewed by Cursor Bugbot for commit aefeed7. Bugbot is set up for automated code reviews on this repo. Configure here.