From 1a0dbcef649f0ec2af1288134f9c9583d3a99c85 Mon Sep 17 00:00:00 2001 From: AxelGes Date: Wed, 4 Mar 2026 20:37:29 -0300 Subject: [PATCH 01/16] feat: add status tracking to RampsController ResourceState Add explicit status field to ResourceState to distinguish between uninitialized and empty-fetched states. The default idle/loading/success/error statuses are now tracked per resource and set explicitly via #setResourceStatus during fetch cycles in executeRequest. This eliminates ambiguity when determining if a resource has been fetched vs is still in its initial state, fixing false positive "token unavailable" errors in mobile when payment methods are actually still loading. --- .../ramps-controller/src/RampsController.ts | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/packages/ramps-controller/src/RampsController.ts b/packages/ramps-controller/src/RampsController.ts index dde0bb9b645..a96b7414248 100644 --- a/packages/ramps-controller/src/RampsController.ts +++ b/packages/ramps-controller/src/RampsController.ts @@ -197,6 +197,11 @@ export type ResourceState = { * Error message if the fetch failed, or null. */ error: string | null; + /** + * The current status of the resource: 'idle' | 'loading' | 'success' | 'error'. + * Distinguishes between never-fetched ('idle') and successfully-fetched-empty ('success'). + */ + status: `${RequestStatus}`; }; /** @@ -338,6 +343,7 @@ function createDefaultResourceState( selected, isLoading: false, error: null, + status: RequestStatus.IDLE, }; } @@ -828,6 +834,7 @@ export class RampsController extends BaseController< this.#pendingResourceCount.set(resourceType, count + 1); if (count === 0) { this.#setResourceLoading(resourceType, true); + this.#setResourceStatus(resourceType, RequestStatus.LOADING); } } @@ -850,6 +857,7 @@ export class RampsController extends BaseController< !options?.isResultCurrent || options.isResultCurrent(); if (isCurrent) { this.#setResourceError(resourceType, null); + this.#setResourceStatus(resourceType, RequestStatus.SUCCESS); } } return data; @@ -868,6 +876,7 @@ export class RampsController extends BaseController< !options?.isResultCurrent || options.isResultCurrent(); if (isCurrent) { this.#setResourceError(resourceType, errorMessage); + this.#setResourceStatus(resourceType, RequestStatus.ERROR); } } throw error; @@ -1027,6 +1036,24 @@ export class RampsController extends BaseController< this.#updateResourceField(resourceType, 'error', error); } + /** + * Sets the status for a resource type. + * + * @param resourceType - The type of resource. + * @param status - The status to set ('idle' | 'loading' | 'success' | 'error'). + */ + #setResourceStatus( + resourceType: ResourceType, + status: `${RequestStatus}`, + ): void { + this.update((state) => { + const resource = state[resourceType]; + if (resource) { + (resource as Record).status = status; + } + }); + } + /** * Gets the state of a specific cached request. * From 8799d7e1f79b0766f12664ac23235e1ee23c9339 Mon Sep 17 00:00:00 2001 From: AxelGes Date: Wed, 4 Mar 2026 21:14:33 -0300 Subject: [PATCH 02/16] fix: reset status field in resetResource and add test helpers - Add missing status field reset in resetResource function to fix bugbot issue - Update test helpers (createResourceState, createDefaultResourceState) to include status field - Add createMockProvider helper to generate complete Provider objects with all required fields - Fix test mocks to use proper Provider type instead of partial objects - Update changelog with new status field feature --- packages/ramps-controller/CHANGELOG.md | 4 + .../src/RampsController.test.ts | 135 +++++++++++++++--- .../ramps-controller/src/RampsController.ts | 1 + .../ramps-controller/src/selectors.test.ts | 3 + 4 files changed, 124 insertions(+), 19 deletions(-) diff --git a/packages/ramps-controller/CHANGELOG.md b/packages/ramps-controller/CHANGELOG.md index 1a9103c53b4..5d449b245bb 100644 --- a/packages/ramps-controller/CHANGELOG.md +++ b/packages/ramps-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `status` field to `ResourceState` to distinguish between uninitialized and empty-fetched states ([#8116](https://github.com/MetaMask/core/pull/8116)) + ## [10.2.0] ### Fixed diff --git a/packages/ramps-controller/src/RampsController.test.ts b/packages/ramps-controller/src/RampsController.test.ts index 9d745fe7011..6eabad3f657 100644 --- a/packages/ramps-controller/src/RampsController.test.ts +++ b/packages/ramps-controller/src/RampsController.test.ts @@ -92,6 +92,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "nativeProviders": { "transak": { @@ -100,6 +101,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "isAuthenticated": false, "kycRequirement": { @@ -107,12 +109,14 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userDetails": { "data": null, "error": null, "isLoading": false, "selected": null, + "status": "idle", }, }, }, @@ -122,12 +126,14 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "providers": { "data": [], "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "requests": {}, "tokens": { @@ -135,6 +141,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userRegion": null, } @@ -167,6 +174,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "nativeProviders": { "transak": { @@ -175,6 +183,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "isAuthenticated": false, "kycRequirement": { @@ -182,12 +191,14 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userDetails": { "data": null, "error": null, "isLoading": false, "selected": null, + "status": "idle", }, }, }, @@ -197,12 +208,14 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "providers": { "data": [], "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "requests": {}, "tokens": { @@ -210,6 +223,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userRegion": null, } @@ -743,6 +757,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "nativeProviders": { "transak": { @@ -751,6 +766,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "isAuthenticated": false, "kycRequirement": { @@ -758,12 +774,14 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userDetails": { "data": null, "error": null, "isLoading": false, "selected": null, + "status": "idle", }, }, }, @@ -773,12 +791,14 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "providers": { "data": [], "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "requests": {}, "tokens": { @@ -786,6 +806,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userRegion": null, } @@ -808,6 +829,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "orders": [], "paymentMethods": { @@ -815,18 +837,21 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "providers": { "data": [], "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "tokens": { "data": null, "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userRegion": null, } @@ -849,6 +874,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "orders": [], "providers": { @@ -856,12 +882,14 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "tokens": { "data": null, "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userRegion": null, } @@ -884,6 +912,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "nativeProviders": { "transak": { @@ -892,6 +921,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "isAuthenticated": false, "kycRequirement": { @@ -899,12 +929,14 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userDetails": { "data": null, "error": null, "isLoading": false, "selected": null, + "status": "idle", }, }, }, @@ -914,12 +946,14 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "providers": { "data": [], "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "requests": {}, "tokens": { @@ -927,6 +961,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userRegion": null, } @@ -4977,7 +5012,7 @@ describe('RampsController', () => { }); }); - it('returns null when service returns BuyWidget with null url', async () => { + it('returns null when service returns BuyWidget with empty url', async () => { await withController(async ({ controller, rootMessenger }) => { const quote: Quote = { provider: '/providers/transak-staging', @@ -4993,7 +5028,7 @@ describe('RampsController', () => { rootMessenger.registerActionHandler( 'RampsService:getBuyWidgetUrl', async () => ({ - url: null, + url: '', browser: 'APP_BROWSER' as const, orderId: null, }), @@ -5001,7 +5036,7 @@ describe('RampsController', () => { const widgetUrl = await controller.getWidgetUrl(quote); - expect(widgetUrl).toBeNull(); + expect(widgetUrl).toBe(''); }); }); }); @@ -5336,7 +5371,10 @@ describe('RampsController', () => { const pendingOrder = createMockOrder({ providerOrderId: 'poll-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(pendingOrder); @@ -5404,7 +5442,10 @@ describe('RampsController', () => { const pendingOrder = createMockOrder({ providerOrderId: 'status-change-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(pendingOrder); @@ -5441,7 +5482,10 @@ describe('RampsController', () => { const pendingOrder = createMockOrder({ providerOrderId: 'no-change-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(pendingOrder); @@ -5471,7 +5515,10 @@ describe('RampsController', () => { const pendingOrder = createMockOrder({ providerOrderId: 'terminal-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(pendingOrder); @@ -5501,7 +5548,10 @@ describe('RampsController', () => { const pendingOrder = createMockOrder({ providerOrderId: 'unknown-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(pendingOrder); @@ -5531,7 +5581,10 @@ describe('RampsController', () => { const pendingOrder = createMockOrder({ providerOrderId: 'error-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(pendingOrder); @@ -5581,7 +5634,10 @@ describe('RampsController', () => { const orderNoId = createMockOrder({ providerOrderId: '', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(orderNoId); @@ -5603,7 +5659,10 @@ describe('RampsController', () => { const orderNoWallet = createMockOrder({ providerOrderId: 'no-wallet-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '', }); controller.addOrder(orderNoWallet); @@ -5625,7 +5684,10 @@ describe('RampsController', () => { const order = createMockOrder({ providerOrderId: 'strip-prefix-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(order); @@ -5654,7 +5716,10 @@ describe('RampsController', () => { const order = createMockOrder({ providerOrderId: 'backoff-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(order); @@ -5684,7 +5749,10 @@ describe('RampsController', () => { const order = createMockOrder({ providerOrderId: 'poll-min-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', pollingSecondsMinimum: 120, }); @@ -5754,7 +5822,10 @@ describe('RampsController', () => { const completedOrder = createMockOrder({ providerOrderId: 'completed-1', status: RampsOrderStatus.Completed, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(completedOrder); @@ -5776,7 +5847,10 @@ describe('RampsController', () => { const order = createMockOrder({ providerOrderId: 'reset-err-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(order); @@ -5812,7 +5886,10 @@ describe('RampsController', () => { const pendingOrder = createMockOrder({ providerOrderId: 'race-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(pendingOrder); @@ -5975,6 +6052,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "isAuthenticated": false, "kycRequirement": { @@ -5982,12 +6060,14 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userDetails": { "data": null, "error": null, "isLoading": false, "selected": null, + "status": "idle", }, } `); @@ -6191,6 +6271,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", } `); }); @@ -6333,6 +6414,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", } `); }); @@ -6415,6 +6497,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", } `); }); @@ -7196,6 +7279,7 @@ function createResourceState( selected, isLoading: false, error: null, + status: RequestStatus.IDLE, }; } @@ -7244,14 +7328,27 @@ function createMockDepositOrder(): TransakDepositOrder { }; } +function createMockProvider(overrides: Partial = {}): Provider { + return { + id: '/providers/test-provider', + name: 'Test Provider', + environmentType: 'production', + description: 'Test provider description', + hqAddress: '123 Test St', + links: [], + logos: { light: '', dark: '', height: 32, width: 32 }, + ...overrides, + }; +} + function createMockOrder(overrides: Partial = {}): RampsOrder { return { id: '/providers/transak-staging/orders/abc-123', isOnlyLink: false, - provider: { + provider: createMockProvider({ id: '/providers/transak-staging', name: 'Transak (Staging)', - }, + }), success: true, cryptoAmount: 0.05, fiatAmount: 100, diff --git a/packages/ramps-controller/src/RampsController.ts b/packages/ramps-controller/src/RampsController.ts index a96b7414248..7eba9324ad7 100644 --- a/packages/ramps-controller/src/RampsController.ts +++ b/packages/ramps-controller/src/RampsController.ts @@ -408,6 +408,7 @@ function resetResource( resource.selected = def.selected; resource.isLoading = def.isLoading; resource.error = def.error; + resource.status = def.status; } /** diff --git a/packages/ramps-controller/src/selectors.test.ts b/packages/ramps-controller/src/selectors.test.ts index 459ad64dedf..2a9d1e0d66b 100644 --- a/packages/ramps-controller/src/selectors.test.ts +++ b/packages/ramps-controller/src/selectors.test.ts @@ -3,6 +3,7 @@ import { createLoadingState, createSuccessState, createErrorState, + RequestStatus, } from './RequestCache'; import { createRequestSelector } from './selectors'; @@ -18,12 +19,14 @@ function createDefaultResourceState( selected: TSelected; isLoading: boolean; error: null; + status: `${RequestStatus}`; } { return { data, selected, isLoading: false, error: null, + status: RequestStatus.IDLE, }; } From 032b5ddc4d4e78362422f4e9bc299ea33fcd0c66 Mon Sep 17 00:00:00 2001 From: AxelGes Date: Wed, 4 Mar 2026 21:40:14 -0300 Subject: [PATCH 03/16] fix: address cursorbot review comments on ResourceState status tracking - Extend #updateResourceField to accept 'status' field, removing the duplicate update logic in #setResourceStatus - Move SUCCESS/ERROR status transitions to the finally block behind the same ref-count guard (next === 0) used by isLoading, so status and isLoading never disagree during concurrent requests --- .../ramps-controller/src/RampsController.ts | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/ramps-controller/src/RampsController.ts b/packages/ramps-controller/src/RampsController.ts index 7eba9324ad7..ea7cc15827f 100644 --- a/packages/ramps-controller/src/RampsController.ts +++ b/packages/ramps-controller/src/RampsController.ts @@ -841,6 +841,7 @@ export class RampsController extends BaseController< // Create the fetch promise const promise = (async (): Promise => { + let terminalStatus: RequestStatus | undefined; try { const data = await fetcher(abortController.signal); @@ -858,7 +859,7 @@ export class RampsController extends BaseController< !options?.isResultCurrent || options.isResultCurrent(); if (isCurrent) { this.#setResourceError(resourceType, null); - this.#setResourceStatus(resourceType, RequestStatus.SUCCESS); + terminalStatus = RequestStatus.SUCCESS; } } return data; @@ -877,7 +878,7 @@ export class RampsController extends BaseController< !options?.isResultCurrent || options.isResultCurrent(); if (isCurrent) { this.#setResourceError(resourceType, errorMessage); - this.#setResourceStatus(resourceType, RequestStatus.ERROR); + terminalStatus = RequestStatus.ERROR; } } throw error; @@ -896,6 +897,9 @@ export class RampsController extends BaseController< if (next === 0) { this.#pendingResourceCount.delete(resourceType); this.#setResourceLoading(resourceType, false); + if (terminalStatus !== undefined) { + this.#setResourceStatus(resourceType, terminalStatus); + } } else { this.#pendingResourceCount.set(resourceType, next); } @@ -1001,12 +1005,12 @@ export class RampsController extends BaseController< * dynamic property access to avoid duplicating switch statements. * * @param resourceType - The type of resource. - * @param field - The field to update ('isLoading' or 'error'). + * @param field - The field to update ('isLoading', 'error', or 'status'). * @param value - The value to set. */ #updateResourceField( resourceType: ResourceType, - field: 'isLoading' | 'error', + field: 'isLoading' | 'error' | 'status', value: boolean | string | null, ): void { this.update((state) => { @@ -1047,12 +1051,7 @@ export class RampsController extends BaseController< resourceType: ResourceType, status: `${RequestStatus}`, ): void { - this.update((state) => { - const resource = state[resourceType]; - if (resource) { - (resource as Record).status = status; - } - }); + this.#updateResourceField(resourceType, 'status', status); } /** From a3313d7a0d2222d44bb3a7a15a01e65b91b08a78 Mon Sep 17 00:00:00 2001 From: AxelGes Date: Wed, 4 Mar 2026 21:47:20 -0300 Subject: [PATCH 04/16] fix: remove unreachable null fallback in getWidgetUrl to restore 100% branch coverage BuyWidget.url is typed as string (non-optional), so the ?? null was dead code that prevented 100% branch coverage. --- packages/ramps-controller/src/RampsController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ramps-controller/src/RampsController.ts b/packages/ramps-controller/src/RampsController.ts index ea7cc15827f..14e4a5560f9 100644 --- a/packages/ramps-controller/src/RampsController.ts +++ b/packages/ramps-controller/src/RampsController.ts @@ -1893,7 +1893,7 @@ export class RampsController extends BaseController< 'RampsService:getBuyWidgetUrl', buyUrl, ); - return buyWidget.url ?? null; + return buyWidget.url; } catch { return null; } From 5dd736d7490a699e5e9628a357ea8f1b19303289 Mon Sep 17 00:00:00 2001 From: AxelGes Date: Wed, 4 Mar 2026 22:46:48 -0300 Subject: [PATCH 05/16] fix: always update status when clearing isLoading to prevent loading/idle inconsistency When isResultCurrent() returns false, terminalStatus was never assigned, leaving status stuck at 'loading' while isLoading was set back to false. Fall back to RequestStatus.IDLE so the two fields always stay in sync. --- packages/ramps-controller/src/RampsController.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/ramps-controller/src/RampsController.ts b/packages/ramps-controller/src/RampsController.ts index 14e4a5560f9..b6578853443 100644 --- a/packages/ramps-controller/src/RampsController.ts +++ b/packages/ramps-controller/src/RampsController.ts @@ -897,9 +897,10 @@ export class RampsController extends BaseController< if (next === 0) { this.#pendingResourceCount.delete(resourceType); this.#setResourceLoading(resourceType, false); - if (terminalStatus !== undefined) { - this.#setResourceStatus(resourceType, terminalStatus); - } + this.#setResourceStatus( + resourceType, + terminalStatus ?? RequestStatus.IDLE, + ); } else { this.#pendingResourceCount.set(resourceType, next); } From 93519965c84e539ce204ea3953e68937bf996eec Mon Sep 17 00:00:00 2001 From: AxelGes Date: Wed, 4 Mar 2026 23:33:54 -0300 Subject: [PATCH 06/16] fix: update isLoading and status atomically to prevent transient inconsistent states Replace separate #setResourceLoading and #setResourceStatus calls with a single #setResourceLoadingAndStatus that writes both fields inside one this.update() call, so subscribers never observe a snapshot where isLoading and status disagree. --- .../ramps-controller/src/RampsController.ts | 51 +++++++++---------- 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/packages/ramps-controller/src/RampsController.ts b/packages/ramps-controller/src/RampsController.ts index b6578853443..6fc703d6b95 100644 --- a/packages/ramps-controller/src/RampsController.ts +++ b/packages/ramps-controller/src/RampsController.ts @@ -834,8 +834,11 @@ export class RampsController extends BaseController< const count = this.#pendingResourceCount.get(resourceType) ?? 0; this.#pendingResourceCount.set(resourceType, count + 1); if (count === 0) { - this.#setResourceLoading(resourceType, true); - this.#setResourceStatus(resourceType, RequestStatus.LOADING); + this.#setResourceLoadingAndStatus( + resourceType, + true, + RequestStatus.LOADING, + ); } } @@ -896,9 +899,9 @@ export class RampsController extends BaseController< const next = Math.max(0, count - 1); if (next === 0) { this.#pendingResourceCount.delete(resourceType); - this.#setResourceLoading(resourceType, false); - this.#setResourceStatus( + this.#setResourceLoadingAndStatus( resourceType, + false, terminalStatus ?? RequestStatus.IDLE, ); } else { @@ -1001,37 +1004,29 @@ export class RampsController extends BaseController< } /** - * Updates a single field (isLoading or error) on a resource state. - * All resources share the same ResourceState structure, so we use - * dynamic property access to avoid duplicating switch statements. + * Updates one or more fields on a resource state atomically in a single + * `this.update()` call. All resources share the same ResourceState structure, + * so we use dynamic property access to avoid duplicating switch statements. * * @param resourceType - The type of resource. - * @param field - The field to update ('isLoading', 'error', or 'status'). - * @param value - The value to set. + * @param fields - An object mapping field names to their new values. */ - #updateResourceField( + #updateResourceFields( resourceType: ResourceType, - field: 'isLoading' | 'error' | 'status', - value: boolean | string | null, + fields: Partial< + Record<'isLoading' | 'error' | 'status', boolean | string | null> + >, ): void { this.update((state) => { const resource = state[resourceType]; if (resource) { - (resource as Record)[field] = value; + for (const [field, value] of Object.entries(fields)) { + (resource as Record)[field] = value; + } } }); } - /** - * Sets the loading state for a resource type. - * - * @param resourceType - The type of resource. - * @param loading - Whether the resource is loading. - */ - #setResourceLoading(resourceType: ResourceType, loading: boolean): void { - this.#updateResourceField(resourceType, 'isLoading', loading); - } - /** * Sets the error state for a resource type. * @@ -1039,20 +1034,22 @@ export class RampsController extends BaseController< * @param error - The error message, or null to clear. */ #setResourceError(resourceType: ResourceType, error: string | null): void { - this.#updateResourceField(resourceType, 'error', error); + this.#updateResourceFields(resourceType, { error }); } /** - * Sets the status for a resource type. + * Sets the loading state and status for a resource type atomically. * * @param resourceType - The type of resource. + * @param loading - Whether the resource is loading. * @param status - The status to set ('idle' | 'loading' | 'success' | 'error'). */ - #setResourceStatus( + #setResourceLoadingAndStatus( resourceType: ResourceType, + loading: boolean, status: `${RequestStatus}`, ): void { - this.#updateResourceField(resourceType, 'status', status); + this.#updateResourceFields(resourceType, { isLoading: loading, status }); } /** From e9179cd80ecf2757f7b252dc24fd2d660f6265ba Mon Sep 17 00:00:00 2001 From: AxelGes Date: Thu, 5 Mar 2026 11:41:09 -0300 Subject: [PATCH 07/16] restore ?? null guard in getWidgetUrl --- packages/ramps-controller/src/RampsController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ramps-controller/src/RampsController.ts b/packages/ramps-controller/src/RampsController.ts index 6fc703d6b95..f03ae5f9f1d 100644 --- a/packages/ramps-controller/src/RampsController.ts +++ b/packages/ramps-controller/src/RampsController.ts @@ -1891,7 +1891,7 @@ export class RampsController extends BaseController< 'RampsService:getBuyWidgetUrl', buyUrl, ); - return buyWidget.url; + return buyWidget.url ?? null; } catch { return null; } From 48de3668e063a95721475a5cefd084c1d7304a68 Mon Sep 17 00:00:00 2001 From: AxelGes Date: Thu, 5 Mar 2026 12:42:58 -0300 Subject: [PATCH 08/16] add test for undefined url branch in getWidgetUrl --- .../src/RampsController.test.ts | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/packages/ramps-controller/src/RampsController.test.ts b/packages/ramps-controller/src/RampsController.test.ts index 6eabad3f657..4024f5302bc 100644 --- a/packages/ramps-controller/src/RampsController.test.ts +++ b/packages/ramps-controller/src/RampsController.test.ts @@ -5012,6 +5012,34 @@ describe('RampsController', () => { }); }); + it('returns null when service returns BuyWidget with undefined url', async () => { + await withController(async ({ controller, rootMessenger }) => { + const quote: Quote = { + provider: '/providers/transak-staging', + quote: { + amountIn: 100, + amountOut: '0.05', + paymentMethod: '/payments/debit-credit-card', + buyURL: + 'https://on-ramp.uat-api.cx.metamask.io/providers/transak-staging/buy-widget', + }, + }; + + rootMessenger.registerActionHandler( + 'RampsService:getBuyWidgetUrl', + async () => ({ + url: undefined as unknown as string, + browser: 'APP_BROWSER' as const, + orderId: null, + }), + ); + + const widgetUrl = await controller.getWidgetUrl(quote); + + expect(widgetUrl).toBeNull(); + }); + }); + it('returns null when service returns BuyWidget with empty url', async () => { await withController(async ({ controller, rootMessenger }) => { const quote: Quote = { From 7c6b4c8e1f228742d2f10bd91403ec1700492d46 Mon Sep 17 00:00:00 2001 From: AxelGes Date: Thu, 5 Mar 2026 13:09:13 -0300 Subject: [PATCH 09/16] fix: backfill missing status field on persisted ResourceState in constructor Persisted resource states (countries, providers, tokens) pre-dating the status field would come back from storage without it, leaving status undefined and defeating the idle/loading/success/error distinction. Deep-merge each persisted resource state with defaults in the constructor so that any missing fields are backfilled, while explicit values (including null, for the existing error-path test) are preserved. --- .../src/RampsController.test.ts | 36 +++++++++++++++++++ .../ramps-controller/src/RampsController.ts | 19 +++++++++- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/packages/ramps-controller/src/RampsController.test.ts b/packages/ramps-controller/src/RampsController.test.ts index 4024f5302bc..7c27afa5eef 100644 --- a/packages/ramps-controller/src/RampsController.test.ts +++ b/packages/ramps-controller/src/RampsController.test.ts @@ -252,6 +252,42 @@ describe('RampsController', () => { }, ); }); + + it('backfills missing status field on persisted resource states', async () => { + const persistedStateWithoutStatus = { + countries: { data: [], selected: null, isLoading: false, error: null }, + providers: { data: [], selected: null, isLoading: false, error: null }, + tokens: { data: null, selected: null, isLoading: false, error: null }, + } as unknown as Partial; + + await withController( + { options: { state: persistedStateWithoutStatus } }, + ({ controller }) => { + expect(controller.state.countries.status).toBe(RequestStatus.IDLE); + expect(controller.state.providers.status).toBe(RequestStatus.IDLE); + expect(controller.state.tokens.status).toBe(RequestStatus.IDLE); + }, + ); + }); + + it('preserves existing status field on persisted resource states', async () => { + const persistedStateWithStatus = { + countries: { + data: [], + selected: null, + isLoading: false, + error: null, + status: RequestStatus.SUCCESS, + }, + } as unknown as Partial; + + await withController( + { options: { state: persistedStateWithStatus } }, + ({ controller }) => { + expect(controller.state.countries.status).toBe(RequestStatus.SUCCESS); + }, + ); + }); }); describe('messenger action handlers', () => { diff --git a/packages/ramps-controller/src/RampsController.ts b/packages/ramps-controller/src/RampsController.ts index f03ae5f9f1d..07ad110ce27 100644 --- a/packages/ramps-controller/src/RampsController.ts +++ b/packages/ramps-controller/src/RampsController.ts @@ -737,13 +737,30 @@ export class RampsController extends BaseController< requestCacheTTL = DEFAULT_REQUEST_CACHE_TTL, requestCacheMaxSize = DEFAULT_REQUEST_CACHE_MAX_SIZE, }: RampsControllerOptions) { + const defaults = getDefaultRampsControllerState(); super({ messenger, metadata: rampsControllerMetadata, name: controllerName, state: { - ...getDefaultRampsControllerState(), + ...defaults, ...state, + // Deep-merge persisted resource states so that new fields added to + // ResourceState (e.g. `status`) are backfilled for existing users + // whose persisted state pre-dates those fields. Only merge when the + // provided value is a plain object (not null or a primitive). + ...(state.countries !== null && + typeof state.countries === 'object' && { + countries: { ...defaults.countries, ...state.countries }, + }), + ...(state.providers !== null && + typeof state.providers === 'object' && { + providers: { ...defaults.providers, ...state.providers }, + }), + ...(state.tokens !== null && + typeof state.tokens === 'object' && { + tokens: { ...defaults.tokens, ...state.tokens }, + }), // Always reset requests cache on initialization (non-persisted) requests: {}, }, From fd80c6bb3dbf0d504e5160504cddcb88a43a9886 Mon Sep 17 00:00:00 2001 From: AxelGes Date: Thu, 5 Mar 2026 13:27:26 -0300 Subject: [PATCH 10/16] fix: correct misleading test title for empty url case in getWidgetUrl MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test title said "returns null" but the assertion checked for an empty string. The ?? operator does not coalesce empty strings, so the function correctly returns '' β€” the title was wrong, not the assertion. --- packages/ramps-controller/src/RampsController.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ramps-controller/src/RampsController.test.ts b/packages/ramps-controller/src/RampsController.test.ts index 7c27afa5eef..82eb7b56761 100644 --- a/packages/ramps-controller/src/RampsController.test.ts +++ b/packages/ramps-controller/src/RampsController.test.ts @@ -5076,7 +5076,7 @@ describe('RampsController', () => { }); }); - it('returns null when service returns BuyWidget with empty url', async () => { + it('returns empty string when service returns BuyWidget with empty url', async () => { await withController(async ({ controller, rootMessenger }) => { const quote: Quote = { provider: '/providers/transak-staging', From 2bfaa2a5b6cd2880c84b076fb9da4779ca93fc12 Mon Sep 17 00:00:00 2001 From: AxelGes Date: Thu, 5 Mar 2026 13:45:31 -0300 Subject: [PATCH 11/16] fix: update status field for native provider resources (userDetails, buyQuote, kycRequirement) The status field was only wired for top-level resources via executeRequest but native provider resources directly mutated isLoading/error without ever updating status, leaving it permanently 'idle'. Also backfill native provider and paymentMethods resource states in the constructor for persisted state migration. --- .../src/RampsController.test.ts | 104 +++++++++++++++++- .../ramps-controller/src/RampsController.ts | 43 ++++++++ 2 files changed, 144 insertions(+), 3 deletions(-) diff --git a/packages/ramps-controller/src/RampsController.test.ts b/packages/ramps-controller/src/RampsController.test.ts index 82eb7b56761..e25a1a2cd96 100644 --- a/packages/ramps-controller/src/RampsController.test.ts +++ b/packages/ramps-controller/src/RampsController.test.ts @@ -288,6 +288,95 @@ describe('RampsController', () => { }, ); }); + + it('backfills missing status field on persisted native provider resource states', async () => { + const persistedState = { + nativeProviders: { + transak: { + isAuthenticated: false, + userDetails: { + data: null, + selected: null, + isLoading: false, + error: null, + }, + buyQuote: { + data: null, + selected: null, + isLoading: false, + error: null, + }, + kycRequirement: { + data: null, + selected: null, + isLoading: false, + error: null, + }, + }, + }, + } as unknown as Partial; + + await withController( + { options: { state: persistedState } }, + ({ controller }) => { + expect( + controller.state.nativeProviders.transak.userDetails.status, + ).toBe(RequestStatus.IDLE); + expect( + controller.state.nativeProviders.transak.buyQuote.status, + ).toBe(RequestStatus.IDLE); + expect( + controller.state.nativeProviders.transak.kycRequirement.status, + ).toBe(RequestStatus.IDLE); + }, + ); + }); + + it('preserves existing status field on persisted native provider resource states', async () => { + const persistedState = { + nativeProviders: { + transak: { + isAuthenticated: true, + userDetails: { + data: null, + selected: null, + isLoading: false, + error: null, + status: RequestStatus.SUCCESS, + }, + buyQuote: { + data: null, + selected: null, + isLoading: false, + error: null, + status: RequestStatus.ERROR, + }, + kycRequirement: { + data: null, + selected: null, + isLoading: false, + error: null, + status: RequestStatus.SUCCESS, + }, + }, + }, + } as unknown as Partial; + + await withController( + { options: { state: persistedState } }, + ({ controller }) => { + expect( + controller.state.nativeProviders.transak.userDetails.status, + ).toBe(RequestStatus.SUCCESS); + expect( + controller.state.nativeProviders.transak.buyQuote.status, + ).toBe(RequestStatus.ERROR); + expect( + controller.state.nativeProviders.transak.kycRequirement.status, + ).toBe(RequestStatus.SUCCESS); + }, + ); + }); }); describe('messenger action handlers', () => { @@ -6335,7 +6424,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, - "status": "idle", + "status": "success", } `); }); @@ -6358,6 +6447,9 @@ describe('RampsController', () => { expect( controller.state.nativeProviders.transak.userDetails.error, ).toBe('Auth failed'); + expect( + controller.state.nativeProviders.transak.userDetails.status, + ).toBe(RequestStatus.ERROR); }); }); @@ -6478,7 +6570,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, - "status": "idle", + "status": "success", } `); }); @@ -6507,6 +6599,9 @@ describe('RampsController', () => { expect(controller.state.nativeProviders.transak.buyQuote.error).toBe( 'Quote failed', ); + expect( + controller.state.nativeProviders.transak.buyQuote.status, + ).toBe(RequestStatus.ERROR); }); }); @@ -6561,7 +6656,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, - "status": "idle", + "status": "success", } `); }); @@ -6584,6 +6679,9 @@ describe('RampsController', () => { expect( controller.state.nativeProviders.transak.kycRequirement.error, ).toBe('KYC failed'); + expect( + controller.state.nativeProviders.transak.kycRequirement.status, + ).toBe(RequestStatus.ERROR); }); }); diff --git a/packages/ramps-controller/src/RampsController.ts b/packages/ramps-controller/src/RampsController.ts index 07ad110ce27..cb4677c7c0d 100644 --- a/packages/ramps-controller/src/RampsController.ts +++ b/packages/ramps-controller/src/RampsController.ts @@ -761,6 +761,36 @@ export class RampsController extends BaseController< typeof state.tokens === 'object' && { tokens: { ...defaults.tokens, ...state.tokens }, }), + ...(state.paymentMethods !== null && + typeof state.paymentMethods === 'object' && { + paymentMethods: { + ...defaults.paymentMethods, + ...state.paymentMethods, + }, + }), + ...(state.nativeProviders !== null && + typeof state.nativeProviders === 'object' && { + nativeProviders: { + ...defaults.nativeProviders, + ...state.nativeProviders, + transak: { + ...defaults.nativeProviders.transak, + ...state.nativeProviders.transak, + userDetails: { + ...defaults.nativeProviders.transak.userDetails, + ...state.nativeProviders.transak?.userDetails, + }, + buyQuote: { + ...defaults.nativeProviders.transak.buyQuote, + ...state.nativeProviders.transak?.buyQuote, + }, + kycRequirement: { + ...defaults.nativeProviders.transak.kycRequirement, + ...state.nativeProviders.transak?.kycRequirement, + }, + }, + }, + }), // Always reset requests cache on initialization (non-persisted) requests: {}, }, @@ -2114,6 +2144,7 @@ export class RampsController extends BaseController< this.update((state) => { state.nativeProviders.transak.userDetails.isLoading = true; state.nativeProviders.transak.userDetails.error = null; + state.nativeProviders.transak.userDetails.status = RequestStatus.LOADING; }); try { const details = await this.messenger.call( @@ -2122,6 +2153,8 @@ export class RampsController extends BaseController< this.update((state) => { state.nativeProviders.transak.userDetails.data = details; state.nativeProviders.transak.userDetails.isLoading = false; + state.nativeProviders.transak.userDetails.status = + RequestStatus.SUCCESS; }); return details; } catch (error) { @@ -2130,6 +2163,7 @@ export class RampsController extends BaseController< this.update((state) => { state.nativeProviders.transak.userDetails.isLoading = false; state.nativeProviders.transak.userDetails.error = errorMessage; + state.nativeProviders.transak.userDetails.status = RequestStatus.ERROR; }); throw error; } @@ -2156,6 +2190,7 @@ export class RampsController extends BaseController< this.update((state) => { state.nativeProviders.transak.buyQuote.isLoading = true; state.nativeProviders.transak.buyQuote.error = null; + state.nativeProviders.transak.buyQuote.status = RequestStatus.LOADING; }); try { const quote = await this.messenger.call( @@ -2169,6 +2204,7 @@ export class RampsController extends BaseController< this.update((state) => { state.nativeProviders.transak.buyQuote.data = quote; state.nativeProviders.transak.buyQuote.isLoading = false; + state.nativeProviders.transak.buyQuote.status = RequestStatus.SUCCESS; }); return quote; } catch (error) { @@ -2176,6 +2212,7 @@ export class RampsController extends BaseController< this.update((state) => { state.nativeProviders.transak.buyQuote.isLoading = false; state.nativeProviders.transak.buyQuote.error = errorMessage; + state.nativeProviders.transak.buyQuote.status = RequestStatus.ERROR; }); throw error; } @@ -2194,6 +2231,8 @@ export class RampsController extends BaseController< this.update((state) => { state.nativeProviders.transak.kycRequirement.isLoading = true; state.nativeProviders.transak.kycRequirement.error = null; + state.nativeProviders.transak.kycRequirement.status = + RequestStatus.LOADING; }); try { const requirement = await this.messenger.call( @@ -2203,6 +2242,8 @@ export class RampsController extends BaseController< this.update((state) => { state.nativeProviders.transak.kycRequirement.data = requirement; state.nativeProviders.transak.kycRequirement.isLoading = false; + state.nativeProviders.transak.kycRequirement.status = + RequestStatus.SUCCESS; }); return requirement; } catch (error) { @@ -2211,6 +2252,8 @@ export class RampsController extends BaseController< this.update((state) => { state.nativeProviders.transak.kycRequirement.isLoading = false; state.nativeProviders.transak.kycRequirement.error = errorMessage; + state.nativeProviders.transak.kycRequirement.status = + RequestStatus.ERROR; }); throw error; } From 50a9f5948286594d8ae6a381517675b382a08cd4 Mon Sep 17 00:00:00 2001 From: AxelGes Date: Thu, 5 Mar 2026 16:13:01 -0300 Subject: [PATCH 12/16] style: fix prettier formatting for buyQuote.status assertions --- .../src/RampsController.test.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/ramps-controller/src/RampsController.test.ts b/packages/ramps-controller/src/RampsController.test.ts index e25a1a2cd96..2e0b73101e8 100644 --- a/packages/ramps-controller/src/RampsController.test.ts +++ b/packages/ramps-controller/src/RampsController.test.ts @@ -322,9 +322,9 @@ describe('RampsController', () => { expect( controller.state.nativeProviders.transak.userDetails.status, ).toBe(RequestStatus.IDLE); - expect( - controller.state.nativeProviders.transak.buyQuote.status, - ).toBe(RequestStatus.IDLE); + expect(controller.state.nativeProviders.transak.buyQuote.status).toBe( + RequestStatus.IDLE, + ); expect( controller.state.nativeProviders.transak.kycRequirement.status, ).toBe(RequestStatus.IDLE); @@ -368,9 +368,9 @@ describe('RampsController', () => { expect( controller.state.nativeProviders.transak.userDetails.status, ).toBe(RequestStatus.SUCCESS); - expect( - controller.state.nativeProviders.transak.buyQuote.status, - ).toBe(RequestStatus.ERROR); + expect(controller.state.nativeProviders.transak.buyQuote.status).toBe( + RequestStatus.ERROR, + ); expect( controller.state.nativeProviders.transak.kycRequirement.status, ).toBe(RequestStatus.SUCCESS); @@ -6599,9 +6599,9 @@ describe('RampsController', () => { expect(controller.state.nativeProviders.transak.buyQuote.error).toBe( 'Quote failed', ); - expect( - controller.state.nativeProviders.transak.buyQuote.status, - ).toBe(RequestStatus.ERROR); + expect(controller.state.nativeProviders.transak.buyQuote.status).toBe( + RequestStatus.ERROR, + ); }); }); From b61e2e86f9413c6151d7e1eda11a9ff240f5a069 Mon Sep 17 00:00:00 2001 From: AxelGes Date: Thu, 5 Mar 2026 16:37:08 -0300 Subject: [PATCH 13/16] fix: prevent concurrent stale requests from regressing resource status to idle When multiple concurrent requests share a resourceType but different cache keys, the last-to-finish request now correctly preserves any terminal status (success/error) recorded by earlier current results rather than falling back to IDLE. Introduces #resourceTerminalStatus map to track the best status across all in-flight requests for a resource, cleared after the final request commits the status to state. --- .../src/RampsController.test.ts | 57 +++++++++++++++++++ .../ramps-controller/src/RampsController.ts | 48 ++++++++++++++-- 2 files changed, 101 insertions(+), 4 deletions(-) diff --git a/packages/ramps-controller/src/RampsController.test.ts b/packages/ramps-controller/src/RampsController.test.ts index 2e0b73101e8..d5c0bdb4e13 100644 --- a/packages/ramps-controller/src/RampsController.test.ts +++ b/packages/ramps-controller/src/RampsController.test.ts @@ -1240,6 +1240,63 @@ describe('RampsController', () => { }); }); + it('does not regress status to idle when the last concurrent request is stale but an earlier one succeeded', async () => { + await withController(async ({ controller }) => { + let resolveFirst: (value: string) => void; + let resolveSecond: (value: string) => void; + + // Request A is "current" (its result should be applied) + // Request B is "stale" (isResultCurrent returns false, so terminalStatus stays undefined) + const fetcherA = async (): Promise => + new Promise((resolve) => { + resolveFirst = resolve; + }); + const fetcherB = async (): Promise => + new Promise((resolve) => { + resolveSecond = resolve; + }); + + let isACurrentValue = false; + const promiseA = controller.executeRequest( + 'providers-key-a2', + fetcherA, + { + resourceType: 'providers', + isResultCurrent: () => isACurrentValue, + }, + ); + const promiseB = controller.executeRequest( + 'providers-key-b2', + fetcherB, + { + resourceType: 'providers', + isResultCurrent: () => false, + }, + ); + + expect(controller.state.providers.status).toBe(RequestStatus.LOADING); + + // Resolve A first while it's current β†’ terminalStatus = SUCCESS, but + // count goes from 2 to 1 so status is not written yet + isACurrentValue = true; + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + resolveFirst!('result-a'); + await promiseA; + + // Status not updated yet since count is still 1 + expect(controller.state.providers.isLoading).toBe(true); + + // Resolve B last while stale β†’ terminalStatus stays undefined in B's closure + // Without the fix, this would write IDLE; with the fix it should preserve SUCCESS + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + resolveSecond!('result-b'); + await promiseB; + + expect(controller.state.providers.isLoading).toBe(false); + expect(controller.state.providers.status).toBe(RequestStatus.SUCCESS); + }); + }); + it('clears resource loading when ref-count hits zero even if map was cleared (defensive)', async () => { await withController(async ({ controller }) => { let resolveFetcher: (value: string) => void; diff --git a/packages/ramps-controller/src/RampsController.ts b/packages/ramps-controller/src/RampsController.ts index cb4677c7c0d..1ff5cdec4db 100644 --- a/packages/ramps-controller/src/RampsController.ts +++ b/packages/ramps-controller/src/RampsController.ts @@ -686,6 +686,15 @@ export class RampsController extends BaseController< */ readonly #pendingResourceCount: Map = new Map(); + /** + * Tracks the "best" terminal status reached by any concurrent request for a + * resource type. Populated when a current result is resolved/rejected, and + * cleared when the last in-flight request for that resource finishes and the + * status is committed to state. + */ + readonly #resourceTerminalStatus: Map = + new Map(); + readonly #orderPollingMeta: Map = new Map(); #orderPollingTimer: ReturnType | null = null; @@ -891,7 +900,6 @@ export class RampsController extends BaseController< // Create the fetch promise const promise = (async (): Promise => { - let terminalStatus: RequestStatus | undefined; try { const data = await fetcher(abortController.signal); @@ -909,7 +917,10 @@ export class RampsController extends BaseController< !options?.isResultCurrent || options.isResultCurrent(); if (isCurrent) { this.#setResourceError(resourceType, null); - terminalStatus = RequestStatus.SUCCESS; + this.#recordResourceTerminalStatus( + resourceType, + RequestStatus.SUCCESS, + ); } } return data; @@ -928,7 +939,10 @@ export class RampsController extends BaseController< !options?.isResultCurrent || options.isResultCurrent(); if (isCurrent) { this.#setResourceError(resourceType, errorMessage); - terminalStatus = RequestStatus.ERROR; + this.#recordResourceTerminalStatus( + resourceType, + RequestStatus.ERROR, + ); } } throw error; @@ -946,10 +960,17 @@ export class RampsController extends BaseController< const next = Math.max(0, count - 1); if (next === 0) { this.#pendingResourceCount.delete(resourceType); + // Use the best terminal status recorded by any current result across + // all concurrent requests. Fall back to IDLE only if no request for + // this resource reported a current result. + const effectiveStatus = + this.#resourceTerminalStatus.get(resourceType) ?? + RequestStatus.IDLE; + this.#resourceTerminalStatus.delete(resourceType); this.#setResourceLoadingAndStatus( resourceType, false, - terminalStatus ?? RequestStatus.IDLE, + effectiveStatus, ); } else { this.#pendingResourceCount.set(resourceType, next); @@ -1099,6 +1120,25 @@ export class RampsController extends BaseController< this.#updateResourceFields(resourceType, { isLoading: loading, status }); } + /** + * Records the terminal status for a resource type, keeping the highest-priority + * status seen across concurrent requests. SUCCESS and ERROR both take priority + * over a previously recorded IDLE, and SUCCESS takes priority over ERROR so + * that a successful peer request is not masked by a failed one. + * + * @param resourceType - The resource type. + * @param status - The terminal status to record (SUCCESS or ERROR). + */ + #recordResourceTerminalStatus( + resourceType: ResourceType, + status: RequestStatus.SUCCESS | RequestStatus.ERROR, + ): void { + const current = this.#resourceTerminalStatus.get(resourceType); + if (current !== RequestStatus.SUCCESS) { + this.#resourceTerminalStatus.set(resourceType, status); + } + } + /** * Gets the state of a specific cached request. * From ab492fd1d27b6d13c55d53a08873cf0ee421ee93 Mon Sep 17 00:00:00 2001 From: AxelGes Date: Thu, 5 Mar 2026 21:10:50 -0300 Subject: [PATCH 14/16] refactor: remove constructor deep-merge backfill for ResourceState status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Persisted-state migration is a consumer responsibility (handled by migration 125 in metamask-mobile). The controller has no migration runner and should not paper over missing fields β€” that's the wrong layer for this concern. --- .../src/RampsController.test.ts | 125 ------------------ .../ramps-controller/src/RampsController.ts | 49 +------ 2 files changed, 1 insertion(+), 173 deletions(-) diff --git a/packages/ramps-controller/src/RampsController.test.ts b/packages/ramps-controller/src/RampsController.test.ts index d5c0bdb4e13..7b5fcdff1f4 100644 --- a/packages/ramps-controller/src/RampsController.test.ts +++ b/packages/ramps-controller/src/RampsController.test.ts @@ -252,131 +252,6 @@ describe('RampsController', () => { }, ); }); - - it('backfills missing status field on persisted resource states', async () => { - const persistedStateWithoutStatus = { - countries: { data: [], selected: null, isLoading: false, error: null }, - providers: { data: [], selected: null, isLoading: false, error: null }, - tokens: { data: null, selected: null, isLoading: false, error: null }, - } as unknown as Partial; - - await withController( - { options: { state: persistedStateWithoutStatus } }, - ({ controller }) => { - expect(controller.state.countries.status).toBe(RequestStatus.IDLE); - expect(controller.state.providers.status).toBe(RequestStatus.IDLE); - expect(controller.state.tokens.status).toBe(RequestStatus.IDLE); - }, - ); - }); - - it('preserves existing status field on persisted resource states', async () => { - const persistedStateWithStatus = { - countries: { - data: [], - selected: null, - isLoading: false, - error: null, - status: RequestStatus.SUCCESS, - }, - } as unknown as Partial; - - await withController( - { options: { state: persistedStateWithStatus } }, - ({ controller }) => { - expect(controller.state.countries.status).toBe(RequestStatus.SUCCESS); - }, - ); - }); - - it('backfills missing status field on persisted native provider resource states', async () => { - const persistedState = { - nativeProviders: { - transak: { - isAuthenticated: false, - userDetails: { - data: null, - selected: null, - isLoading: false, - error: null, - }, - buyQuote: { - data: null, - selected: null, - isLoading: false, - error: null, - }, - kycRequirement: { - data: null, - selected: null, - isLoading: false, - error: null, - }, - }, - }, - } as unknown as Partial; - - await withController( - { options: { state: persistedState } }, - ({ controller }) => { - expect( - controller.state.nativeProviders.transak.userDetails.status, - ).toBe(RequestStatus.IDLE); - expect(controller.state.nativeProviders.transak.buyQuote.status).toBe( - RequestStatus.IDLE, - ); - expect( - controller.state.nativeProviders.transak.kycRequirement.status, - ).toBe(RequestStatus.IDLE); - }, - ); - }); - - it('preserves existing status field on persisted native provider resource states', async () => { - const persistedState = { - nativeProviders: { - transak: { - isAuthenticated: true, - userDetails: { - data: null, - selected: null, - isLoading: false, - error: null, - status: RequestStatus.SUCCESS, - }, - buyQuote: { - data: null, - selected: null, - isLoading: false, - error: null, - status: RequestStatus.ERROR, - }, - kycRequirement: { - data: null, - selected: null, - isLoading: false, - error: null, - status: RequestStatus.SUCCESS, - }, - }, - }, - } as unknown as Partial; - - await withController( - { options: { state: persistedState } }, - ({ controller }) => { - expect( - controller.state.nativeProviders.transak.userDetails.status, - ).toBe(RequestStatus.SUCCESS); - expect(controller.state.nativeProviders.transak.buyQuote.status).toBe( - RequestStatus.ERROR, - ); - expect( - controller.state.nativeProviders.transak.kycRequirement.status, - ).toBe(RequestStatus.SUCCESS); - }, - ); - }); }); describe('messenger action handlers', () => { diff --git a/packages/ramps-controller/src/RampsController.ts b/packages/ramps-controller/src/RampsController.ts index 1ff5cdec4db..7cd3f28f1ad 100644 --- a/packages/ramps-controller/src/RampsController.ts +++ b/packages/ramps-controller/src/RampsController.ts @@ -746,60 +746,13 @@ export class RampsController extends BaseController< requestCacheTTL = DEFAULT_REQUEST_CACHE_TTL, requestCacheMaxSize = DEFAULT_REQUEST_CACHE_MAX_SIZE, }: RampsControllerOptions) { - const defaults = getDefaultRampsControllerState(); super({ messenger, metadata: rampsControllerMetadata, name: controllerName, state: { - ...defaults, + ...getDefaultRampsControllerState(), ...state, - // Deep-merge persisted resource states so that new fields added to - // ResourceState (e.g. `status`) are backfilled for existing users - // whose persisted state pre-dates those fields. Only merge when the - // provided value is a plain object (not null or a primitive). - ...(state.countries !== null && - typeof state.countries === 'object' && { - countries: { ...defaults.countries, ...state.countries }, - }), - ...(state.providers !== null && - typeof state.providers === 'object' && { - providers: { ...defaults.providers, ...state.providers }, - }), - ...(state.tokens !== null && - typeof state.tokens === 'object' && { - tokens: { ...defaults.tokens, ...state.tokens }, - }), - ...(state.paymentMethods !== null && - typeof state.paymentMethods === 'object' && { - paymentMethods: { - ...defaults.paymentMethods, - ...state.paymentMethods, - }, - }), - ...(state.nativeProviders !== null && - typeof state.nativeProviders === 'object' && { - nativeProviders: { - ...defaults.nativeProviders, - ...state.nativeProviders, - transak: { - ...defaults.nativeProviders.transak, - ...state.nativeProviders.transak, - userDetails: { - ...defaults.nativeProviders.transak.userDetails, - ...state.nativeProviders.transak?.userDetails, - }, - buyQuote: { - ...defaults.nativeProviders.transak.buyQuote, - ...state.nativeProviders.transak?.buyQuote, - }, - kycRequirement: { - ...defaults.nativeProviders.transak.kycRequirement, - ...state.nativeProviders.transak?.kycRequirement, - }, - }, - }, - }), // Always reset requests cache on initialization (non-persisted) requests: {}, }, From ae2ff7bef5cddbd2ac7225794162532174f7826a Mon Sep 17 00:00:00 2001 From: AxelGes Date: Fri, 6 Mar 2026 01:15:32 -0300 Subject: [PATCH 15/16] fix: clear stale terminal status when aborting dependent resources on region change Without this fix, a successful request recording SUCCESS into #resourceTerminalStatus before a region change aborts concurrent in-flight requests would cause that stale SUCCESS to persist. The next request batch for the same resource would then inherit it, preventing ERROR from being recorded even when the new request failed. --- .../src/RampsController.test.ts | 96 +++++++++++++++++++ .../ramps-controller/src/RampsController.ts | 1 + 2 files changed, 97 insertions(+) diff --git a/packages/ramps-controller/src/RampsController.test.ts b/packages/ramps-controller/src/RampsController.test.ts index 7b5fcdff1f4..159da0201de 100644 --- a/packages/ramps-controller/src/RampsController.test.ts +++ b/packages/ramps-controller/src/RampsController.test.ts @@ -1980,6 +1980,102 @@ describe('RampsController', () => { ); }); + it('does not inherit stale terminal status after region change aborts a successful request', async () => { + const mockTokens: TokensResponse = { topTokens: [], allTokens: [] }; + + await withController( + { + options: { + state: { + countries: createResourceState(createMockCountries()), + userRegion: { + regionCode: 'us-ca', + state: { + stateId: 'CA', + name: 'California', + supported: { buy: true, sell: true }, + }, + country: { + isoCode: 'US', + name: 'United States of America', + flag: 'πŸ‡ΊπŸ‡Έ', + currency: 'USD', + phone: { prefix: '+1', placeholder: '', template: '' }, + supported: { buy: true, sell: true }, + }, + }, + }, + }, + }, + async ({ controller, rootMessenger }) => { + // Step 1: Launch two concurrent executeRequest calls for 'providers'. + // Request A will succeed (isResultCurrent=true), recording SUCCESS in + // #resourceTerminalStatus. Request B hangs (will be aborted). + let resolveA: (value: string) => void; + + const promiseA = controller.executeRequest( + 'providers-stale-a', + async () => + new Promise((resolve) => { + resolveA = resolve; + }), + { resourceType: 'providers', isResultCurrent: () => true }, + ); + + // Request B hangs β€” will be aborted by the region change below + const promiseB = controller + .executeRequest( + 'providers-stale-b', + async (signal) => + new Promise((_resolve, reject) => { + signal.addEventListener('abort', () => + reject(new Error('aborted')), + ); + }), + { resourceType: 'providers', isResultCurrent: () => true }, + ) + // eslint-disable-next-line no-empty-function -- swallow expected abort rejection + .catch(() => {}); + + // Step 2: Resolve A successfully while B is still in-flight. + // count goes 2β†’1, SUCCESS is recorded in #resourceTerminalStatus but + // not committed yet (count > 0 when A finishes). + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + resolveA!('result-a'); + await promiseA; + + expect(controller.state.providers.isLoading).toBe(true); + + // Step 3: Change region. This calls #abortDependentRequests (aborts B) + // and #clearPendingResourceCountForDependentResources (clears count). + // The fix also clears #resourceTerminalStatus for dependent resources. + // B's finally block sees signal.aborted=true and skips cleanup, so + // without the fix the stale SUCCESS remains in #resourceTerminalStatus. + rootMessenger.registerActionHandler( + 'RampsService:getTokens', + async () => mockTokens, + ); + rootMessenger.registerActionHandler( + 'RampsService:getProviders', + async () => { + throw new Error('FR providers fetch failed'); + }, + ); + + await controller.setUserRegion('FR'); + await promiseB; + + // Wait for the fire-and-forget refetches to settle + await new Promise((resolve) => setTimeout(resolve, 50)); + + // Step 4: Without the fix, the stale SUCCESS from request A prevented + // ERROR from being recorded when the FR request failed, so status + // would be 'success'. With the fix it must be 'error'. + expect(controller.state.providers.status).toBe(RequestStatus.ERROR); + }, + ); + }); + it('does not clear persisted state when setting the same region', async () => { const mockTokens: TokensResponse = { topTokens: [], diff --git a/packages/ramps-controller/src/RampsController.ts b/packages/ramps-controller/src/RampsController.ts index 7cd3f28f1ad..3f006bc1634 100644 --- a/packages/ramps-controller/src/RampsController.ts +++ b/packages/ramps-controller/src/RampsController.ts @@ -714,6 +714,7 @@ export class RampsController extends BaseController< #clearPendingResourceCountForDependentResources(): void { for (const resourceType of DEPENDENT_RESOURCE_KEYS) { this.#pendingResourceCount.delete(resourceType); + this.#resourceTerminalStatus.delete(resourceType); } } From af741b21ce18a397cd4232b32af4f5df80c771ca Mon Sep 17 00:00:00 2001 From: AxelGes Date: Sun, 8 Mar 2026 22:28:14 -0300 Subject: [PATCH 16/16] refactor: replace #resourceTerminalStatus map with a local variable Remove the cross-request terminal status tracking map and its associated #recordResourceTerminalStatus method in favour of a simple let variable scoped to each executeRequest closure. The last concurrent request to finish now wins (last-write-wins), which is simpler and sufficient given the AbortController already cancels stale requests. --- .../src/RampsController.test.ts | 12 ++--- .../ramps-controller/src/RampsController.ts | 49 ++----------------- 2 files changed, 9 insertions(+), 52 deletions(-) diff --git a/packages/ramps-controller/src/RampsController.test.ts b/packages/ramps-controller/src/RampsController.test.ts index 159da0201de..e619af05794 100644 --- a/packages/ramps-controller/src/RampsController.test.ts +++ b/packages/ramps-controller/src/RampsController.test.ts @@ -1115,13 +1115,13 @@ describe('RampsController', () => { }); }); - it('does not regress status to idle when the last concurrent request is stale but an earlier one succeeded', async () => { + it('sets status to idle when the last concurrent request is stale (last-write-wins)', async () => { await withController(async ({ controller }) => { let resolveFirst: (value: string) => void; let resolveSecond: (value: string) => void; // Request A is "current" (its result should be applied) - // Request B is "stale" (isResultCurrent returns false, so terminalStatus stays undefined) + // Request B is "stale" (isResultCurrent returns false) const fetcherA = async (): Promise => new Promise((resolve) => { resolveFirst = resolve; @@ -1151,8 +1151,7 @@ describe('RampsController', () => { expect(controller.state.providers.status).toBe(RequestStatus.LOADING); - // Resolve A first while it's current β†’ terminalStatus = SUCCESS, but - // count goes from 2 to 1 so status is not written yet + // Resolve A first while it's current, count goes from 2 to 1 so status is not written yet isACurrentValue = true; // eslint-disable-next-line @typescript-eslint/no-non-null-assertion resolveFirst!('result-a'); @@ -1161,14 +1160,13 @@ describe('RampsController', () => { // Status not updated yet since count is still 1 expect(controller.state.providers.isLoading).toBe(true); - // Resolve B last while stale β†’ terminalStatus stays undefined in B's closure - // Without the fix, this would write IDLE; with the fix it should preserve SUCCESS + // Resolve B last while stale β†’ terminalStatus is IDLE (last-write-wins) // eslint-disable-next-line @typescript-eslint/no-non-null-assertion resolveSecond!('result-b'); await promiseB; expect(controller.state.providers.isLoading).toBe(false); - expect(controller.state.providers.status).toBe(RequestStatus.SUCCESS); + expect(controller.state.providers.status).toBe(RequestStatus.IDLE); }); }); diff --git a/packages/ramps-controller/src/RampsController.ts b/packages/ramps-controller/src/RampsController.ts index 3f006bc1634..e9b5d299265 100644 --- a/packages/ramps-controller/src/RampsController.ts +++ b/packages/ramps-controller/src/RampsController.ts @@ -686,15 +686,6 @@ export class RampsController extends BaseController< */ readonly #pendingResourceCount: Map = new Map(); - /** - * Tracks the "best" terminal status reached by any concurrent request for a - * resource type. Populated when a current result is resolved/rejected, and - * cleared when the last in-flight request for that resource finishes and the - * status is committed to state. - */ - readonly #resourceTerminalStatus: Map = - new Map(); - readonly #orderPollingMeta: Map = new Map(); #orderPollingTimer: ReturnType | null = null; @@ -714,7 +705,6 @@ export class RampsController extends BaseController< #clearPendingResourceCountForDependentResources(): void { for (const resourceType of DEPENDENT_RESOURCE_KEYS) { this.#pendingResourceCount.delete(resourceType); - this.#resourceTerminalStatus.delete(resourceType); } } @@ -854,6 +844,7 @@ export class RampsController extends BaseController< // Create the fetch promise const promise = (async (): Promise => { + let terminalStatus: RequestStatus = RequestStatus.IDLE; try { const data = await fetcher(abortController.signal); @@ -871,10 +862,7 @@ export class RampsController extends BaseController< !options?.isResultCurrent || options.isResultCurrent(); if (isCurrent) { this.#setResourceError(resourceType, null); - this.#recordResourceTerminalStatus( - resourceType, - RequestStatus.SUCCESS, - ); + terminalStatus = RequestStatus.SUCCESS; } } return data; @@ -893,10 +881,7 @@ export class RampsController extends BaseController< !options?.isResultCurrent || options.isResultCurrent(); if (isCurrent) { this.#setResourceError(resourceType, errorMessage); - this.#recordResourceTerminalStatus( - resourceType, - RequestStatus.ERROR, - ); + terminalStatus = RequestStatus.ERROR; } } throw error; @@ -914,17 +899,10 @@ export class RampsController extends BaseController< const next = Math.max(0, count - 1); if (next === 0) { this.#pendingResourceCount.delete(resourceType); - // Use the best terminal status recorded by any current result across - // all concurrent requests. Fall back to IDLE only if no request for - // this resource reported a current result. - const effectiveStatus = - this.#resourceTerminalStatus.get(resourceType) ?? - RequestStatus.IDLE; - this.#resourceTerminalStatus.delete(resourceType); this.#setResourceLoadingAndStatus( resourceType, false, - effectiveStatus, + terminalStatus, ); } else { this.#pendingResourceCount.set(resourceType, next); @@ -1074,25 +1052,6 @@ export class RampsController extends BaseController< this.#updateResourceFields(resourceType, { isLoading: loading, status }); } - /** - * Records the terminal status for a resource type, keeping the highest-priority - * status seen across concurrent requests. SUCCESS and ERROR both take priority - * over a previously recorded IDLE, and SUCCESS takes priority over ERROR so - * that a successful peer request is not masked by a failed one. - * - * @param resourceType - The resource type. - * @param status - The terminal status to record (SUCCESS or ERROR). - */ - #recordResourceTerminalStatus( - resourceType: ResourceType, - status: RequestStatus.SUCCESS | RequestStatus.ERROR, - ): void { - const current = this.#resourceTerminalStatus.get(resourceType); - if (current !== RequestStatus.SUCCESS) { - this.#resourceTerminalStatus.set(resourceType, status); - } - } - /** * Gets the state of a specific cached request. *