From 622c8f064129cc85b8bb387c14e43144470f7bff Mon Sep 17 00:00:00 2001 From: George Weiler Date: Thu, 5 Mar 2026 12:26:56 -0700 Subject: [PATCH 1/4] feat: update state hydration --- packages/ramps-controller/CHANGELOG.md | 9 + .../src/RampsController.test.ts | 165 +++++++++++------- .../ramps-controller/src/RampsController.ts | 52 ++++-- 3 files changed, 151 insertions(+), 75 deletions(-) diff --git a/packages/ramps-controller/CHANGELOG.md b/packages/ramps-controller/CHANGELOG.md index 1a9103c53b4..fc90f4ebd55 100644 --- a/packages/ramps-controller/CHANGELOG.md +++ b/packages/ramps-controller/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Make `init()` idempotent: duplicate calls return the same promise; use `init({ forceRefresh: true })` to re-run +- Skip `getCountries` and geolocation in `init()` when `state.countries.data` and `state.userRegion` already exist (warm start) + +### Removed + +- Remove `hydrateState()` — use `init()` as the single entry point for controller hydration + ## [10.2.0] ### Fixed diff --git a/packages/ramps-controller/src/RampsController.test.ts b/packages/ramps-controller/src/RampsController.test.ts index 9d745fe7011..0e0b0e9d3d4 100644 --- a/packages/ramps-controller/src/RampsController.test.ts +++ b/packages/ramps-controller/src/RampsController.test.ts @@ -1652,98 +1652,139 @@ describe('RampsController', () => { }); }); }); - }); - describe('hydrateState', () => { - it('triggers fetching tokens and providers for user region', async () => { - await withController( - { - options: { - state: { - userRegion: createMockUserRegion('us-ca'), - }, + it('does not double-fetch when init() called twice concurrently', async () => { + await withController(async ({ controller, rootMessenger }) => { + let getCountriesCallCount = 0; + rootMessenger.registerActionHandler( + 'RampsService:getGeolocation', + async () => 'us-ca', + ); + rootMessenger.registerActionHandler( + 'RampsService:getCountries', + async () => { + getCountriesCallCount += 1; + return createMockCountries(); }, - }, - async ({ controller, rootMessenger }) => { - let tokensCalled = false; - let providersCalled = false; - - rootMessenger.registerActionHandler( - 'RampsService:getTokens', - async () => { - tokensCalled = true; - return { topTokens: [], allTokens: [] }; - }, - ); - rootMessenger.registerActionHandler( - 'RampsService:getProviders', - async () => { - providersCalled = true; - return { providers: [] }; - }, - ); - - controller.hydrateState(); - - await new Promise((resolve) => setTimeout(resolve, 10)); + ); + rootMessenger.registerActionHandler( + 'RampsService:getTokens', + async () => ({ topTokens: [], allTokens: [] }), + ); + rootMessenger.registerActionHandler( + 'RampsService:getProviders', + async () => ({ providers: [] }), + ); - expect(tokensCalled).toBe(true); - expect(providersCalled).toBe(true); - }, - ); + await Promise.all([controller.init(), controller.init()]); + expect(getCountriesCallCount).toBe(1); + }); }); - it('throws error when userRegion is not set', async () => { - await withController(async ({ controller }) => { - expect(() => controller.hydrateState()).toThrow( - 'Region is required. Cannot proceed without valid region information.', + it('returns immediately on second init() after first completes', async () => { + await withController(async ({ controller, rootMessenger }) => { + let getCountriesCallCount = 0; + rootMessenger.registerActionHandler( + 'RampsService:getGeolocation', + async () => 'us-ca', ); + rootMessenger.registerActionHandler( + 'RampsService:getCountries', + async () => { + getCountriesCallCount += 1; + return createMockCountries(); + }, + ); + rootMessenger.registerActionHandler( + 'RampsService:getTokens', + async () => ({ topTokens: [], allTokens: [] }), + ); + rootMessenger.registerActionHandler( + 'RampsService:getProviders', + async () => ({ providers: [] }), + ); + + await controller.init(); + await controller.init(); + expect(getCountriesCallCount).toBe(1); }); }); - it('calls getTokens and getProviders when hydrating even if state has data', async () => { - const existingProviders: Provider[] = [ - { - id: '/providers/test', - name: 'Test Provider', - environmentType: 'STAGING', - description: 'Test', - hqAddress: '123 Test St', - links: [], - logos: { light: '', dark: '', height: 24, width: 77 }, - }, - ]; + it('skips getCountries and geolocation when userRegion and countries exist', async () => { + let getCountriesCalled = false; + let getGeolocationCalled = false; await withController( { options: { state: { + countries: createResourceState(createMockCountries()), userRegion: createMockUserRegion('us-ca'), - providers: createResourceState(existingProviders, null), }, }, }, async ({ controller, rootMessenger }) => { - let providersCalled = false; + rootMessenger.registerActionHandler( + 'RampsService:getCountries', + async () => { + getCountriesCalled = true; + return createMockCountries(); + }, + ); + rootMessenger.registerActionHandler( + 'RampsService:getGeolocation', + async () => { + getGeolocationCalled = true; + return 'us-ca'; + }, + ); rootMessenger.registerActionHandler( 'RampsService:getTokens', async () => ({ topTokens: [], allTokens: [] }), ); rootMessenger.registerActionHandler( 'RampsService:getProviders', - async () => { - providersCalled = true; - return { providers: [] }; - }, + async () => ({ providers: [] }), ); - controller.hydrateState(); - - await new Promise((resolve) => setTimeout(resolve, 10)); + await controller.init(); - expect(providersCalled).toBe(true); + expect(getCountriesCalled).toBe(false); + expect(getGeolocationCalled).toBe(false); + expect(controller.state.userRegion?.regionCode).toBe('us-ca'); }, ); }); + + it('forceRefresh bypasses idempotency and re-runs full flow', async () => { + let getCountriesCallCount = 0; + await withController(async ({ controller, rootMessenger }) => { + rootMessenger.registerActionHandler( + 'RampsService:getGeolocation', + async () => 'us-ca', + ); + rootMessenger.registerActionHandler( + 'RampsService:getCountries', + async () => { + getCountriesCallCount += 1; + return createMockCountries(); + }, + ); + rootMessenger.registerActionHandler( + 'RampsService:getTokens', + async () => ({ topTokens: [], allTokens: [] }), + ); + rootMessenger.registerActionHandler( + 'RampsService:getProviders', + async () => ({ providers: [] }), + ); + + await controller.init(); + expect(getCountriesCallCount).toBe(1); + + await controller.init({ forceRefresh: true }); + expect(getCountriesCallCount).toBe(2); + }); + }); }); describe('setUserRegion', () => { diff --git a/packages/ramps-controller/src/RampsController.ts b/packages/ramps-controller/src/RampsController.ts index dde0bb9b645..0df6b65c8e8 100644 --- a/packages/ramps-controller/src/RampsController.ts +++ b/packages/ramps-controller/src/RampsController.ts @@ -685,6 +685,8 @@ export class RampsController extends BaseController< #isPolling = false; + #initPromise: Promise | null = null; + /** * Clears the pending resource count map. Used only in tests to exercise the * defensive path when get() returns undefined in the finally block. @@ -1211,17 +1213,48 @@ export class RampsController extends BaseController< * Initializes the controller by fetching the user's region from geolocation. * This should be called once at app startup to set up the initial region. * - * If a userRegion already exists (from persistence or manual selection), - * this method will skip geolocation fetch and use the existing region. + * Idempotent: subsequent calls return the same promise unless forceRefresh is set. + * Skips getCountries when countries are already loaded; skips geolocation when + * userRegion already exists. * - * @param options - Options for cache behavior. + * @param options - Options for cache behavior. forceRefresh bypasses idempotency and re-runs the full flow. * @returns Promise that resolves when initialization is complete. */ async init(options?: ExecuteRequestOptions): Promise { - await this.getCountries(options); + if (!options?.forceRefresh && this.#initPromise !== null) { + return this.#initPromise; + } + + if (options?.forceRefresh) { + this.#initPromise = null; + } + + const initPromise = this.#runInit(options).then( + () => undefined, + (error) => { + this.#initPromise = null; + throw error; + }, + ); + this.#initPromise = initPromise; + return initPromise; + } + + async #runInit(options?: ExecuteRequestOptions): Promise { + const forceRefresh = options?.forceRefresh === true; + const hasCountries = (this.state.countries.data?.length ?? 0) > 0; - let regionCode = this.state.userRegion?.regionCode; - regionCode ??= await this.messenger.call('RampsService:getGeolocation'); + if (forceRefresh || !hasCountries) { + await this.getCountries(options); + } + + let regionCode: string | undefined; + if (forceRefresh) { + regionCode = await this.messenger.call('RampsService:getGeolocation'); + } else { + regionCode = this.state.userRegion?.regionCode; + regionCode ??= await this.messenger.call('RampsService:getGeolocation'); + } if (!regionCode) { throw new Error( @@ -1232,13 +1265,6 @@ export class RampsController extends BaseController< await this.setUserRegion(regionCode, options); } - hydrateState(options?: ExecuteRequestOptions): void { - const regionCode = this.#requireRegion(); - - this.#fireAndForget(this.getTokens(regionCode, 'buy', options)); - this.#fireAndForget(this.getProviders(regionCode, options)); - } - /** * Fetches the list of supported countries. * The API returns countries with support information for both buy and sell actions. From 576349978f15988f3e454bea29a5e1cc67c5a271 Mon Sep 17 00:00:00 2001 From: George Weiler Date: Mon, 9 Mar 2026 20:28:08 -0600 Subject: [PATCH 2/4] chore: test nit --- packages/ramps-controller/src/RampsController.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/ramps-controller/src/RampsController.test.ts b/packages/ramps-controller/src/RampsController.test.ts index 0e0b0e9d3d4..cde4562c60f 100644 --- a/packages/ramps-controller/src/RampsController.test.ts +++ b/packages/ramps-controller/src/RampsController.test.ts @@ -7405,3 +7405,7 @@ async function withController( }); return await testFunction({ controller, rootMessenger, messenger }); } + + }); + return await testFunction({ controller, rootMessenger, messenger }); +} From 949e23b420e5475ae06f4ce1d500b70efa49a958 Mon Sep 17 00:00:00 2001 From: George Weiler Date: Mon, 9 Mar 2026 20:45:01 -0600 Subject: [PATCH 3/4] fix ramps controller hydration tests --- packages/ramps-controller/src/RampsController.test.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/ramps-controller/src/RampsController.test.ts b/packages/ramps-controller/src/RampsController.test.ts index cde4562c60f..0e0b0e9d3d4 100644 --- a/packages/ramps-controller/src/RampsController.test.ts +++ b/packages/ramps-controller/src/RampsController.test.ts @@ -7405,7 +7405,3 @@ async function withController( }); return await testFunction({ controller, rootMessenger, messenger }); } - - }); - return await testFunction({ controller, rootMessenger, messenger }); -} From ad48682cc0ec10350ff0412ae4894da1bd128774 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 10 Mar 2026 03:22:59 +0000 Subject: [PATCH 4/4] fix: update changelog and fix race condition in init error handler Co-authored-by: George Weiler --- packages/ramps-controller/CHANGELOG.md | 7 +------ packages/ramps-controller/src/RampsController.ts | 6 ++++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/ramps-controller/CHANGELOG.md b/packages/ramps-controller/CHANGELOG.md index fc90f4ebd55..2008526c68e 100644 --- a/packages/ramps-controller/CHANGELOG.md +++ b/packages/ramps-controller/CHANGELOG.md @@ -9,12 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- Make `init()` idempotent: duplicate calls return the same promise; use `init({ forceRefresh: true })` to re-run -- Skip `getCountries` and geolocation in `init()` when `state.countries.data` and `state.userRegion` already exist (warm start) - -### Removed - -- Remove `hydrateState()` — use `init()` as the single entry point for controller hydration +- **BREAKING:** Update state hydration to make `init()` idempotent and remove `hydrateState()` ([#8157](https://github.com/MetaMask/core/pull/8157)) ## [10.2.0] diff --git a/packages/ramps-controller/src/RampsController.ts b/packages/ramps-controller/src/RampsController.ts index 0df6b65c8e8..fbefe351737 100644 --- a/packages/ramps-controller/src/RampsController.ts +++ b/packages/ramps-controller/src/RampsController.ts @@ -1232,7 +1232,9 @@ export class RampsController extends BaseController< const initPromise = this.#runInit(options).then( () => undefined, (error) => { - this.#initPromise = null; + if (this.#initPromise === initPromise) { + this.#initPromise = null; + } throw error; }, ); @@ -1242,7 +1244,7 @@ export class RampsController extends BaseController< async #runInit(options?: ExecuteRequestOptions): Promise { const forceRefresh = options?.forceRefresh === true; - const hasCountries = (this.state.countries.data?.length ?? 0) > 0; + const hasCountries = this.state.countries.data.length > 0; if (forceRefresh || !hasCountries) { await this.getCountries(options);