Skip to content

fix(ramps-controller): refresh countries catalog on app startup #9261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
amitabh94 wants to merge 5 commits into
base: main
Choose a base branch
from
+198 −17
Open

fix(ramps-controller): refresh countries catalog on app startup #9261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
931fefc
fix(ramps-controller): refresh countries catalog on app startup
amitabh94 Jun 25, 2026
fa3622c
chore(ramps-controller): fix Prettier in RampsController tests
amitabh94 Jun 25, 2026
27b4f2d
chore: fix changelog
wenfix Jun 25, 2026
ff0aa42
chore: link changelog entries to PR for changelog check
wenfix Jun 25, 2026
82c35e8
test(ramps-controller): cover catalog-sync guards; fix changelog afte…
wenfix Jun 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/ramps-controller/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Changed

- Stop persisting the countries catalog in `RampsController` state and refetch it on every app startup via `init()` ([#9261](https://github.com/MetaMask/core/pull/9261))
- Re-sync `userRegion` preset amounts from the countries catalog after each `getCountries()` call ([#9261](https://github.com/MetaMask/core/pull/9261))

## [15.0.0]

### Changed
Expand Down
2 changes: 1 addition & 1 deletion packages/ramps-controller/src/RampsController-method-action-types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export type RampsControllerSetSelectedProviderAction = {
* This should be called once at app startup to set up the initial region.
*
* Idempotent: subsequent calls return the same promise unless forceRefresh is set.
* Skips getCountries when countries are already loaded; skips geolocation when
* Always refetches the countries catalog on startup. Skips geolocation when
* userRegion already exists.
*
* @param options - Options for cache behavior. forceRefresh bypasses idempotency and re-runs the full flow.
Expand Down
170 changes: 162 additions & 8 deletions packages/ramps-controller/src/RampsController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -921,12 +921,6 @@ describe('RampsController', () => {
),
).toMatchInlineSnapshot(`
{
"countries": {
"data": [],
"error": null,
"isLoading": false,
"selected": null,
},
"orders": [],
"providerAutoSelected": false,
"userRegion": null,
Expand Down Expand Up @@ -1880,6 +1874,166 @@ describe('RampsController', () => {
});
});

it('re-syncs userRegion preset amounts after getCountries', async () => {
const staleCountries: Country[] = [
{
isoCode: 'CR',
id: '/regions/cr',
flag: '🇨🇷',
name: 'Costa Rica',
phone: {
prefix: '+506',
placeholder: '8312 3456',
template: 'XXXX XXXX',
},
currency: 'CRC',
supported: { buy: true, sell: true },
defaultAmount: 100,
quickAmounts: [20, 50, 100],
},
];
const freshCountries: Country[] = [
{
...staleCountries[0],
defaultAmount: 25000,
quickAmounts: [10000, 25000, 50000],
},
];

await withController(
{
options: {
state: {
userRegion: {
country: staleCountries[0],
state: null,
regionCode: 'cr',
},
},
},
},
async ({ controller, rootMessenger }) => {
rootMessenger.registerActionHandler(
'RampsService:getCountries',
async () => freshCountries,
);

await rootMessenger.call('RampsController:getCountries');

expect(controller.state.userRegion?.country.defaultAmount).toBe(
25000,
);
expect(
controller.state.userRegion?.country.quickAmounts,
).toStrictEqual([10000, 25000, 50000]);
},
);
});

it('leaves userRegion unchanged when the refreshed catalog is empty', async () => {
const userRegionCountry: Country = {
isoCode: 'CR',
id: '/regions/cr',
flag: '🇨🇷',
name: 'Costa Rica',
phone: {
prefix: '+506',
placeholder: '8312 3456',
template: 'XXXX XXXX',
},
currency: 'CRC',
supported: { buy: true, sell: true },
defaultAmount: 100,
quickAmounts: [20, 50, 100],
};

await withController(
{
options: {
state: {
userRegion: {
country: userRegionCountry,
state: null,
regionCode: 'cr',
},
},
},
},
async ({ controller, rootMessenger }) => {
rootMessenger.registerActionHandler(
'RampsService:getCountries',
async () => [],
);

await rootMessenger.call('RampsController:getCountries');

expect(controller.state.countries.data).toStrictEqual([]);
expect(controller.state.userRegion?.country.defaultAmount).toBe(100);
},
);
});

it('leaves userRegion unchanged when its region is absent from the refreshed catalog', async () => {
const userRegionCountry: Country = {
isoCode: 'CR',
id: '/regions/cr',
flag: '🇨🇷',
name: 'Costa Rica',
phone: {
prefix: '+506',
placeholder: '8312 3456',
template: 'XXXX XXXX',
},
currency: 'CRC',
supported: { buy: true, sell: true },
defaultAmount: 100,
quickAmounts: [20, 50, 100],
};
const otherCountries: Country[] = [
{
isoCode: 'US',
id: '/regions/us',
flag: '🇺🇸',
name: 'United States',
phone: {
prefix: '+1',
placeholder: '201 555 0123',
template: 'XXX XXX XXXX',
},
currency: 'USD',
supported: { buy: true, sell: true },
defaultAmount: 100,
quickAmounts: [100, 300, 1000],
},
];

await withController(
{
options: {
state: {
userRegion: {
country: userRegionCountry,
state: null,
regionCode: 'cr',
},
},
},
},
async ({ controller, rootMessenger }) => {
rootMessenger.registerActionHandler(
'RampsService:getCountries',
async () => otherCountries,
);

await rootMessenger.call('RampsController:getCountries');

expect(controller.state.countries.data).toStrictEqual(otherCountries);
expect(controller.state.userRegion?.country.isoCode).toBe('CR');
expect(controller.state.userRegion?.country.defaultAmount).toBe(100);
},
);
});

it('throws when updating resource field and resource is null', async () => {
const stateWithNullCountries = {
...getDefaultRampsControllerState(),
Expand Down Expand Up @@ -2110,7 +2264,7 @@ describe('RampsController', () => {
});
});

it('skips getCountries and geolocation when userRegion and countries exist', async () => {
it('refetches countries on init but skips geolocation when userRegion exists', async () => {
let getCountriesCalled = false;
let getGeolocationCalled = false;
await withController(
Expand Down Expand Up @@ -2148,7 +2302,7 @@ describe('RampsController', () => {

await rootMessenger.call('RampsController:init');

expect(getCountriesCalled).toBe(false);
expect(getCountriesCalled).toBe(true);
expect(getGeolocationCalled).toBe(false);
expect(controller.state.userRegion?.regionCode).toBe('us-ca');
},
Expand Down
38 changes: 30 additions & 8 deletions packages/ramps-controller/src/RampsController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ const rampsControllerMetadata = {
usedInUi: true,
},
countries: {
persist: true,
persist: false,
includeInDebugSnapshot: true,
includeInStateLogs: true,
usedInUi: true,
Expand Down Expand Up @@ -1383,7 +1383,7 @@ export class RampsController extends BaseController<
* This should be called once at app startup to set up the initial region.
*
* Idempotent: subsequent calls return the same promise unless forceRefresh is set.
* Skips getCountries when countries are already loaded; skips geolocation when
* Always refetches the countries catalog on startup. Skips geolocation when
* userRegion already exists.
*
* @param options - Options for cache behavior. forceRefresh bypasses idempotency and re-runs the full flow.
Expand Down Expand Up @@ -1412,12 +1412,7 @@ export class RampsController extends BaseController<
}

async #runInit(options?: ExecuteRequestOptions): Promise<void> {
const forceRefresh = options?.forceRefresh === true;
const hasCountries = this.state.countries.data.length > 0;

if (forceRefresh || !hasCountries) {
await this.getCountries(options);
}

@cursor cursor Bot Jun 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Init contradicts catalog sync grace

Medium Severity

After getCountries(), #syncUserRegionFromCountriesCatalog keeps a persisted userRegion when the refreshed catalog is empty or no longer contains that region. #runInit still always calls setUserRegion with the persisted code, which throws and runs #cleanupState, so startup can fail and wipe the saved region in cases the new tests treat as safe for getCountries alone.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 82c35e8. Configure here.

@wenfix wenfix Jun 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amitabh94, thoughts on the following?

Finding: init() can wipe the saved user region on app startup

Status: Flagged by Cursor Bugbot (Medium severity), confirmed by code review.

Summary

PR #9261 (part of the Ramps Hardening epic) changes the wallet so that, on every app
startup, it stops trusting the saved list of countries and re-downloads it fresh.
This fixes the original bug where Costa Rica users saw a stale default amount of 100
instead of 25,000.

However, it introduces a new edge case: if that startup download succeeds but comes
back empty — or comes back without the user's own country — the app will erase the user's
saved region entirely (it gets set to "none"). The user would effectively look like a
brand-new user with no region, until the next successful refresh re-establishes it.

This is the opposite of what the hardening work is trying to achieve (keeping the user's
region and its correct preset amounts stable across restarts).

When does it actually happen?

It does not happen on a normal failed network call (a true failure is caught earlier
and the saved region is left alone). It happens only in this narrower window:

  • The startup countries request succeeds (HTTP 200), but
    • the response body is an empty list, or
    • the list no longer contains the user's region (e.g. the region was delisted, or a
      partial/garbled-but-valid response).

In those cases the saved region is wiped.

Before this PR, this could not happen at startup because the previously-saved country list
(which contained the user's region) was reused. Now the list is always discarded and
re-fetched, so the empty/incomplete-response case is reachable on every cold start.

Technical detail

In RampsController.ts:

  • #runInit() (the startup routine):

    1. await this.getCountries({ forceRefresh: true }) — re-downloads the catalog. Inside
      this, #syncUserRegionFromCountriesCatalog() gracefully keeps the existing region
      if the catalog is empty or doesn't contain the region (it just returns, no change).
    2. Then it unconditionally calls await this.setUserRegion(persistedRegionCode).

  • setUserRegion() does the opposite of the grace logic: if countries.data is empty,
    or findRegionFromCode() returns nothing, it calls #cleanupState() and throws.

  • #cleanupState() → resetDependentResources(state, { clearUserRegionData: true }) →
    state.userRegion = null.

So the lenient guards added in #syncUserRegionFromCountriesCatalog() are immediately
undone by the setUserRegion() call in #runInit(). The net startup behavior in the
empty/region-missing case is: throw + null out userRegion.

The two unit tests added in the PR (leaves userRegion unchanged when the refreshed catalog is empty / ... region is absent ...) assert the grace behavior for getCountries() in
isolation, which is accurate — but they do not reflect what init() does end-to-end, so
they can give a false sense of safety.

Options

  1. Fix it (recommended). Make init() consistent with the grace logic: when a region is
    already saved, don't let a transient/incomplete fresh catalog wipe it; keep the strict
    "throw if no region" behavior only for a brand-new user (the geolocation path, where no
    valid region exists yet). Sketch:

    async #runInit(options) {
  await this.getCountries({ ...options, forceRefresh: true });

  const persistedRegionCode = this.state.userRegion?.regionCode;
  const regionCode =
    persistedRegionCode ?? (await this.messenger.call('RampsService:getGeolocation'));
  if (!regionCode) throw new Error('Failed to fetch geolocation. ...');

  // getCountries already re-synced an existing region from the fresh catalog.
  // Don't let a transient/incomplete catalog wipe a previously-valid region.
  if (persistedRegionCode) return;
  await this.setUserRegion(regionCode, options);
}

    Tradeoff: with this change, a forceRefresh init would no longer re-run
    resetDependentResources for an already-persisted region (today it does, because
    setUserRegion treats forceRefresh as a region change). Need product confirmation that
    that's acceptable.

  2. Accept as-is / out of scope. Decide the empty/region-missing-but-200 response is not
    a realistic case for this API, document the decision on the PR, and ship unchanged.

  3. Alternative hardening: keep calling setUserRegion, but change it so it does not null
    out a previously-valid region on a transient empty catalog (riskier — setUserRegion is
    also used for explicit user-driven region changes where throwing is intended).

Recommendation

Address it as Option 1 (small, contained change in #runInit plus an init-level test that
asserts the saved region survives an empty/region-missing startup refresh), pending
confirmation of the forceRefresh/resetDependentResources tradeoff noted above.

await this.getCountries({ ...options, forceRefresh: true });

// Always prefer the user's persisted region. Geolocation is only used to
// seed the initial value; once the user (or a prior init) has set a region
Expand All @@ -1434,6 +1429,31 @@ export class RampsController extends BaseController<
await this.setUserRegion(regionCode, options);
}

/**
* Re-applies `userRegion` from the current countries catalog so preset
* amounts and support flags stay in sync after a catalog refresh.
*/
#syncUserRegionFromCountriesCatalog(): void {
const regionCode = this.state.userRegion?.regionCode;
if (!regionCode) {
return;
}

const countriesData = this.state.countries.data;
if (!countriesData.length) {
return;
}

const userRegion = findRegionFromCode(regionCode, countriesData);
if (!userRegion) {
return;
}

this.update((state) => {
state.userRegion = userRegion;
});
}

/**
* Fetches the list of supported countries.
* The API returns countries with support information for both buy and sell actions.
Expand All @@ -1457,6 +1477,8 @@ export class RampsController extends BaseController<
state.countries.data = Array.isArray(countries) ? [...countries] : [];
});

this.#syncUserRegionFromCountriesCatalog();

return countries;
}

Expand Down
Loading