Conversation
imyugioh
force-pushed
the
fix/ramps-geolocaiton-call-init-once
branch
from
2e9d637 to
e38a582
Compare
imyugioh
changed the title
imyugioh
force-pushed
the
fix/ramps-geolocaiton-call-init-once
branch
from
e38a582 to
e7946ce
Compare
7 tasks
imyugioh
force-pushed
the
fix/ramps-geolocaiton-call-init-once
branch
from
36a67a9 to
29bbf4c
Compare
imyugioh
changed the title
imyugioh
force-pushed
the
fix/ramps-geolocaiton-call-init-once
branch
from
ab25346 to
14dc03d
Compare
imyugioh
force-pushed
the
fix/ramps-geolocaiton-call-init-once
branch
from
a553f75 to
10a8a74
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. |
imyugioh
force-pushed
the
fix/ramps-geolocaiton-call-init-once
branch
from
10a8a74 to
4d05ac3
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
imyugioh
force-pushed
the
fix/ramps-geolocaiton-call-init-once
branch
6 times, most recently
from
6036f35 to
1d5c02c
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. |
…nge has error - fixed through passing provider object, same in paymentmethods as well
imyugioh
force-pushed
the
fix/ramps-geolocaiton-call-init-once
branch
from
1d5c02c to
2d102f8
Compare
amitabh94
reviewed
Apr 3, 2026
Contributor
There was a problem hiding this comment.
SHouldn't the changelog file just have the PR description and not all commits?
amitabh94
reviewed
Apr 3, 2026
| this.#setResourceLoading(resourceType, false); | ||
| } else { | ||
| this.#pendingResourceCount.set(resourceType, next); | ||
| const currentGeneration = |
Contributor
There was a problem hiding this comment.
Can you explain why this is needed?
Contributor
Author
There was a problem hiding this comment.
@amitabh94 good catch - this prevents a race condition during the rapid region switching - normal users will not do that, but hackers can do that :)
example: user taps R1 -> R2 quickly
- region A request starts -> pendingResourceCount =1 generation = 0
- user switches to region B -> clearPendingResourceCountForResource() deletes the count, bumps generation to 1
- region b request starts at this moment -> pendingResourceCount = 1, generation = 1
- Region A's request finishes -> its finally block runs. If we do not check generation, it would decrement count to 0 and set isLoading = false - > but at that moment region B is still loading.
with the generation check: region A captured generation 0 , count is 1 -> mismatch -> skip decrement -> isLoading stays true until region B finishes.
Contributor
Author
There was a problem hiding this comment.
generation counter only matters for only dependent resources, hmm - like providers, paymentmethods, tokens. these are resources that get their pending count cleared with setuserregion call
Contributor
Author
There was a problem hiding this comment.
And this is about the correctness - not about time.
the old request's finally block always runs (we do not abort it - because aborting would lose the executeRequest cache benefit)
amitabh94
previously approved these changes
Apr 3, 2026
amitabh94
approved these changes
Apr 3, 2026
github-merge-queue bot
pushed a commit
to MetaMask/metamask-mobile
that referenced
this pull request
Apr 3, 2026
…and payment methods (#28224) ## **Description** Payment methods, providers, and tokens were being fetched redundantly from two independent paths (controller `fireAndForget` + React Query), causing 2–3 duplicate API calls per user action with a ~10s spinner. This PR makes the mobile client the single fetch owner for all ramp data. React Query handles providers and payment methods with proper caching and invalidation. Tokens are fetched directly from RampsBootstrap. Screens that don't need payment methods no longer trigger fetches. Changes: 1. **`RampsBootstrap.tsx`** — Now fetches all three data sources at app root: `useRampsProviders` (React Query), `useRampsPaymentMethods` (React Query), and `getTokens` (direct controller call on region change). This follows the agreed flow: app loads → fetch providers and tokens → provider selected → fetch payment methods. 2. **`useRampsProviders.ts`** — Reads provider list from React Query cache (not controller state). Invalidates all ramp queries on region change to force fresh fetches. Passes full `Provider` object to `setSelectedProvider` for auto-selection (avoids crash when controller state is empty). 3. **`useRampsPaymentMethods.ts`** — Query key reduced to `[regionCode, providerId]` only. Token/fiat are passed to the API call but not the key, so token changes don't trigger refetches. Passes full `PaymentMethod` object to controller (avoids crash when controller state is empty). 4. **`paymentMethods.ts` (query config)** — `staleTime: 5min`. Query key only includes `regionCode` and `providerId`. 5. **`providers.ts` (query config)** — `staleTime: 15min`. Removed `forceRefresh: true` from queryFn (controller's own cache handles it). 6. **`SettingsModal.tsx`** — Uses `useRampsProviders` instead of `useRampsController` (no more payment methods fetch on settings screen). 7. **`TokenNotAvailableModal.tsx`** — Uses `useRampsProviders` + `useRampsTokens` instead of `useRampsController`. 8. **`RegionSelector.tsx`** — Uses `useRampsUserRegion` + `useRampsCountries` instead of `useRampsController`. 9. **`PaymentSelectionModal.tsx`** — Filters out payment methods with no available quote once quotes load, preventing dead-end options (e.g. Apple Pay shown but no provider returns a quote for it). 10. **Tests updated** — Removed `tokenSupportedByProvider` test, updated query key and staleTime assertions. ## **Changelog** CHANGELOG entry: Fixed duplicate payment method, provider, and token API calls during buy flow; React Query is now the single fetch owner for ramp data ## **Related issues** Fixes: [TRAM-3398](https://consensyssoftware.atlassian.net/browse/TRAM-3398) Depends on core PR: [MetaMask/core#8354](MetaMask/core#8354) (removes `fireAndForget` calls from controller) ## **Manual testing steps** ```gherkin Feature: Single-owner fetching for ramp data Scenario: App load fetches providers, tokens, and payment methods Given user opens the app (password screen) When the app loads Then getProviders, getTokens, and getPaymentMethods each fire once And providers, tokens, and payment methods are populated in state Scenario: Token selection does not trigger payment methods fetch Given user is on the token selection screen When user selects a token (e.g. Ethereum) Then getPaymentMethods is NOT called And the BuildQuote screen loads with cached payment methods Scenario: Provider change triggers a single payment methods fetch Given user is on the BuildQuote screen with a provider selected When user changes the provider (e.g. Transak -> Coinbase) Then getPaymentMethods fires exactly once for the new provider And the payment method pill auto-switches to Debit or Credit Scenario: Region switch refreshes all data Given user is on the settings screen When user changes region (e.g. France -> Finland -> France) Then getProviders and getPaymentMethods fire for each new region And switching back to a previous region also triggers fresh fetches Scenario: Settings screen does not trigger payment methods fetch Given user navigates to the Buy & Sell settings screen Then getPaymentMethods is NOT called And no unnecessary API calls appear in the debug dashboard Scenario: Payment selection modal hides dead-end options Given user taps the payment method pill When quotes load for all payment methods Then payment methods with no available quote are filtered out ``` ## **Screenshots/Recordings** ### **Before** <!-- 2-3 getPaymentMethods calls per token selection visible in debug dashboard --> ### **After** <!-- Single getPaymentMethods call only on provider change; no calls on settings/navigation --> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). ## **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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes ramp buy-flow data fetching/caching and controller interaction patterns (query keys, invalidation, and provider/payment-method selection), which could affect availability/performance across regions and providers. UI impact is limited but touches core buy-flow bootstrap and selection logic. > > **Overview** > **Makes the mobile client the single fetch owner for ramps buy data.** `RampsBootstrap` now preloads providers (with side effects), payment methods, and triggers token fetching on region availability so downstream screens don’t cause redundant requests. > > **Reworks React Query behavior for providers and payment methods.** `useRampsProviders` reads the provider list from the React Query cache, adds optional `enableSideEffects` to avoid duplicate invalidation/auto-selection, and invalidates all `ramps` queries on region changes. `useRampsPaymentMethods` simplifies the query to be provider-scoped (query key is `regionCode` + `providerId`, 5-min `staleTime`) and updates controller setters to accept full objects/null. > > **UI behavior tweaks and hook decoupling.** `PaymentSelectionModal` now hides payment methods that have no non-custom-action quote once quotes load, showing the “no payment methods available” state instead. Several screens (`SettingsModal`, `TokenNotAvailableModal`, `RegionSelector`) switch from `useRampsController` to narrower hooks (`useRampsProviders`, `useRampsTokens`, `useRampsUserRegion`, `useRampsCountries`) to prevent unrelated fetches. Dependency bump updates `@metamask/ramps-controller` to `^13.0.0`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 75f85ca. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
micaelae
pushed a commit
that referenced
this pull request
Apr 7, 2026
…t own data lifecycle (#8354) ## Explanation Payment methods, providers, and tokens were being fetched redundantly from two independent paths: the controller's `fireAndForget` calls (inside `setSelectedToken`, `setSelectedProvider`, and `setUserRegion`) and React Query on the mobile side. This caused 2–3 duplicate API calls per user action, with a ~10s spinner on the `/payments` endpoint due to cold DB connection pooling. This PR removes all `fireAndForget` data-fetching side effects from the controller. The mobile client (React Query + RampsBootstrap) is now the single owner of when providers, tokens, and payment methods are fetched. The controller only manages state updates and selections. Changes: 1. **`#runInit` (geolocation fix)** — `forceRefresh` no longer overrides a persisted `userRegion` with the geolocation endpoint. Geolocation is only used to seed the initial region when `userRegion` is null. 2. **`setSelectedToken`** — Removed `fireAndForget(getPaymentMethods(...))` and `resetResource('paymentMethods')`. Token change no longer triggers payment methods fetch or clears payment methods state. Payment methods are provider-scoped, not token-scoped. 3. **`setSelectedProvider`** — Removed `fireAndForget(getPaymentMethods(...))`, `resetResource('paymentMethods')`, and `tokenSupportedByProvider` gate. Now accepts a full `Provider` object (not just ID) to avoid dependency on `state.providers.data` being populated. Silently ignores when provider ID is not found instead of throwing. 4. **`setUserRegion`** — Removed `fireAndForget(getTokens(...))` and `fireAndForget(getProviders(...))`. The mobile client handles all data fetching via React Query (providers, payment methods) and direct controller calls (tokens) from RampsBootstrap. 5. **`setSelectedPaymentMethod`** — Now accepts a full `PaymentMethod` object (not just ID) to avoid dependency on `state.paymentMethods.data` being populated. Silently sets `null` when payment method ID is not found instead of throwing. 6. **`getPaymentMethods` response handler** — Always selects the first (highest-scored) payment method when new data arrives, preventing dead-end states where a payment method with no quotes stays selected after provider switch. 7. **`#fireAndForget`** — Removed (no remaining callers). 8. **`executeRequest` generation counter** — Added `#pendingResourceGeneration` map to prevent stale in-flight requests from corrupting `isLoading` state. When `setUserRegion` resets dependent resource counts, the generation is bumped. Orphaned finally blocks from the previous generation skip their decrement instead of prematurely clearing `isLoading`. ## Link to metamask-mobile Depends on mobile PR: [MetaMask/metamask-mobile#28224](MetaMask/metamask-mobile#28224) ## References [TRAM-3398](https://consensyssoftware.atlassian.net/browse/TRAM-3398) ## Changelog CHANGELOG entry: See `packages/ramps-controller/CHANGELOG.md` Unreleased section. ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Behavior changes in `RampsController` selection/region flows (no longer auto-fetching or clearing dependent data) could affect consumers that relied on controller-side side effects. Adds new stale-request invalidation for `executeRequest`, which impacts loading/error state handling across resources. > > **Overview** > **Shifts ramps data lifecycle ownership to the client** by removing controller-side `fireAndForget` fetching from `setUserRegion`, `setSelectedToken`, and `setSelectedProvider`, and by no longer clearing `paymentMethods` on token/provider changes. > > `setSelectedProvider` and `setSelectedPaymentMethod` now accept either an ID or a full object and **stop throwing when backing resource data isn’t loaded/not found** (silently leaving selection `null`). `init` is fixed to **never override a persisted `userRegion` with geolocation**, even on `forceRefresh`. > > Request tracking is hardened by adding a per-resource generation counter so stale in-flight requests can’t incorrectly decrement ref-counted `isLoading` after dependent-resource resets; tests are updated/added to cover these races and the new selection semantics. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 7073fe5. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
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
2 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
Payment methods, providers, and tokens were being fetched redundantly from two independent paths: the controller's
fireAndForgetcalls (insidesetSelectedToken,setSelectedProvider, andsetUserRegion) and React Query on the mobile side. This caused 2–3 duplicate API calls per user action, with a ~10s spinner on the/paymentsendpoint due to cold DB connection pooling.This PR removes all
fireAndForgetdata-fetching side effects from the controller. The mobile client (React Query + RampsBootstrap) is now the single owner of when providers, tokens, and payment methods are fetched. The controller only manages state updates and selections.Changes:
#runInit(geolocation fix) —forceRefreshno longer overrides a persisteduserRegionwith the geolocation endpoint. Geolocation is only used to seed the initial region whenuserRegionis null.setSelectedToken— RemovedfireAndForget(getPaymentMethods(...))andresetResource('paymentMethods'). Token change no longer triggers payment methods fetch or clears payment methods state. Payment methods are provider-scoped, not token-scoped.setSelectedProvider— RemovedfireAndForget(getPaymentMethods(...)),resetResource('paymentMethods'), andtokenSupportedByProvidergate. Now accepts a fullProviderobject (not just ID) to avoid dependency onstate.providers.databeing populated. Silently ignores when provider ID is not found instead of throwing.setUserRegion— RemovedfireAndForget(getTokens(...))andfireAndForget(getProviders(...)). The mobile client handles all data fetching via React Query (providers, payment methods) and direct controller calls (tokens) from RampsBootstrap.setSelectedPaymentMethod— Now accepts a fullPaymentMethodobject (not just ID) to avoid dependency onstate.paymentMethods.databeing populated. Silently setsnullwhen payment method ID is not found instead of throwing.getPaymentMethodsresponse handler — Always selects the first (highest-scored) payment method when new data arrives, preventing dead-end states where a payment method with no quotes stays selected after provider switch.#fireAndForget— Removed (no remaining callers).executeRequestgeneration counter — Added#pendingResourceGenerationmap to prevent stale in-flight requests from corruptingisLoadingstate. WhensetUserRegionresets dependent resource counts, the generation is bumped. Orphaned finally blocks from the previous generation skip their decrement instead of prematurely clearingisLoading.Link to metamask-mobile
Depends on mobile PR: MetaMask/metamask-mobile#28224
References
TRAM-3398
Changelog
CHANGELOG entry: See
packages/ramps-controller/CHANGELOG.mdUnreleased section.Checklist
Note
Medium Risk
Behavior changes in
RampsControllerselection/region flows (no longer auto-fetching or clearing dependent data) could affect consumers that relied on controller-side side effects. Adds new stale-request invalidation forexecuteRequest, which impacts loading/error state handling across resources.Overview
Shifts ramps data lifecycle ownership to the client by removing controller-side
fireAndForgetfetching fromsetUserRegion,setSelectedToken, andsetSelectedProvider, and by no longer clearingpaymentMethodson token/provider changes.setSelectedProviderandsetSelectedPaymentMethodnow accept either an ID or a full object and stop throwing when backing resource data isn’t loaded/not found (silently leaving selectionnull).initis fixed to never override a persisteduserRegionwith geolocation, even onforceRefresh.Request tracking is hardened by adding a per-resource generation counter so stale in-flight requests can’t incorrectly decrement ref-counted
isLoadingafter dependent-resource resets; tests are updated/added to cover these races and the new selection semantics.Written by Cursor Bugbot for commit 7073fe5. This will update automatically on new commits. Configure here.