From 72d47827851ea7f484360408d9051d35e9845240 Mon Sep 17 00:00:00 2001 From: salimtb Date: Thu, 13 Aug 2026 14:10:06 +0200 Subject: [PATCH] fix: never serve Accounts API balances from cache AccountsApiDataSource now invalidates cached balance queries before every fetch and fetches v5/v6 balances with staleTime: 0, gcTime: 0, so balances always hit the network in every flow (polling, forced refresh, websocket-triggered) and are never retained for reuse. Previously non-forced polls could reuse balances cached for up to 60s and forceUpdate: true only shrank the cache window to 100ms, so pull-to-refresh and post-transaction refreshes could show stale balances. --- packages/assets-controller/CHANGELOG.md | 4 + .../src/AssetsController.test.ts | 3 + .../AccountsApiDataSource.test.ts | 91 +++++++--- .../src/data-sources/AccountsApiDataSource.ts | 22 ++- .../src/api/accounts/client.test.ts | 156 ++++++++++++++++++ 5 files changed, 248 insertions(+), 28 deletions(-) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index aefa1d0c02d..cf4e82deed2 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/transaction-controller` from `^69.5.1` to `^69.5.2` ([#9823](https://github.com/MetaMask/core/pull/9823)) +### Fixed + +- Never serve Accounts API balances from the TanStack cache in any flow (forced refresh, polling, websocket-triggered fetches): `AccountsApiDataSource` now invalidates cached balance queries before every fetch and fetches with `staleTime: 0, gcTime: 0`, so v5 and v6 balance requests always hit the network and their results are never retained for reuse — previously non-forced polls could reuse balances cached for up to 60s and even `forceUpdate: true` only shrank the cache window to 100ms, so `getAssets(..., { forceUpdate: true })` (pull-to-refresh, post-swap/post-transaction refresh) could still return stale cached balances + ## [13.1.2] ### Changed diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index 075b220955f..cac6b68f5ba 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -1409,6 +1409,7 @@ describe('AssetsController', () => { }), fetchV6MultiAccountBalances, fetchV5MultiAccountBalances, + invalidateBalances: jest.fn().mockResolvedValue(undefined), }, } as unknown as ApiPlatformClient; @@ -3044,6 +3045,7 @@ describe('AssetsController', () => { partialSupport: [], }), fetchV5MultiAccountBalances, + invalidateBalances: jest.fn().mockResolvedValue(undefined), }, } as unknown as ApiPlatformClient; @@ -3363,6 +3365,7 @@ describe('AssetsController', () => { balances: [], unprocessedNetworks: [], }), + invalidateBalances: jest.fn().mockResolvedValue(undefined), }, } as unknown as ApiPlatformClient; diff --git a/packages/assets-controller/src/data-sources/AccountsApiDataSource.test.ts b/packages/assets-controller/src/data-sources/AccountsApiDataSource.test.ts index 68bf5896a07..6d655356d81 100644 --- a/packages/assets-controller/src/data-sources/AccountsApiDataSource.test.ts +++ b/packages/assets-controller/src/data-sources/AccountsApiDataSource.test.ts @@ -43,6 +43,7 @@ type MockApiClient = { fetchV2SupportedNetworks: jest.Mock; fetchV5MultiAccountBalances: jest.Mock; fetchV6MultiAccountBalances: jest.Mock; + invalidateBalances: jest.Mock; }; }; @@ -87,6 +88,7 @@ function createMockApiClient( unprocessedNetworks, unprocessedIncludeAssetIds: [], }), + invalidateBalances: jest.fn().mockResolvedValue(undefined), }, }; } @@ -519,25 +521,74 @@ describe('AccountsApiDataSource', () => { expect(apiClient.accounts.fetchV5MultiAccountBalances).toHaveBeenCalledWith( [`eip155:1:${MOCK_ADDRESS}`], undefined, - undefined, + { staleTime: 0, gcTime: 0 }, ); controller.destroy(); }); - it('uses a short-lived TanStack cache window when forceUpdate is true', async () => { - const { controller, apiClient } = await setupController(); + // Balances must never be served from the TanStack cache in any flow. + it.each([ + { forceUpdate: undefined }, + { forceUpdate: false }, + { forceUpdate: true }, + ])( + 'always invalidates the balances cache and fetches with no-cache options (v5, forceUpdate: $forceUpdate)', + async ({ forceUpdate }) => { + const { controller, apiClient } = await setupController(); - await controller.fetch(createDataRequest({ forceUpdate: true })); + await controller.fetch(createDataRequest({ forceUpdate })); - expect(apiClient.accounts.fetchV5MultiAccountBalances).toHaveBeenCalledWith( - [`eip155:1:${MOCK_ADDRESS}`], - undefined, - { staleTime: 100, gcTime: 100 }, - ); + expect(apiClient.accounts.invalidateBalances).toHaveBeenCalledTimes(1); + expect( + apiClient.accounts.fetchV5MultiAccountBalances, + ).toHaveBeenCalledWith([`eip155:1:${MOCK_ADDRESS}`], undefined, { + staleTime: 0, + gcTime: 0, + }); + // Invalidation must precede the fetch so a fresh cache entry cannot be + // reused. + expect( + apiClient.accounts.invalidateBalances.mock.invocationCallOrder[0], + ).toBeLessThan( + apiClient.accounts.fetchV5MultiAccountBalances.mock + .invocationCallOrder[0], + ); - controller.destroy(); - }); + controller.destroy(); + }, + ); + + it.each([ + { forceUpdate: undefined }, + { forceUpdate: false }, + { forceUpdate: true }, + ])( + 'always invalidates the balances cache and fetches with no-cache options (v6, forceUpdate: $forceUpdate)', + async ({ forceUpdate }) => { + const { controller, apiClient } = await setupController({ + remoteFeatureFlags: { assetsAccountsApiV6: { value: true } }, + }); + + await controller.fetch(createDataRequest({ forceUpdate })); + + expect(apiClient.accounts.invalidateBalances).toHaveBeenCalledTimes(1); + expect( + apiClient.accounts.fetchV6MultiAccountBalances, + ).toHaveBeenCalledWith([`eip155:1:${MOCK_ADDRESS}`], undefined, { + staleTime: 0, + gcTime: 0, + }); + expect( + apiClient.accounts.invalidateBalances.mock.invocationCallOrder[0], + ).toBeLessThan( + apiClient.accounts.fetchV6MultiAccountBalances.mock + .invocationCallOrder[0], + ); + + controller.destroy(); + }, + ); it('fetch processes balance response', async () => { const balances = [ @@ -696,11 +747,10 @@ describe('AccountsApiDataSource', () => { expect( apiClient.accounts.fetchV6MultiAccountBalances, - ).toHaveBeenCalledWith( - [`eip155:1:${MOCK_ADDRESS}`], - undefined, - undefined, - ); + ).toHaveBeenCalledWith([`eip155:1:${MOCK_ADDRESS}`], undefined, { + staleTime: 0, + gcTime: 0, + }); expect( apiClient.accounts.fetchV5MultiAccountBalances, ).not.toHaveBeenCalled(); @@ -861,11 +911,10 @@ describe('AccountsApiDataSource', () => { expect( apiClient.accounts.fetchV6MultiAccountBalances, - ).toHaveBeenCalledWith( - [`eip155:1:${MOCK_ADDRESS}`], - undefined, - undefined, - ); + ).toHaveBeenCalledWith([`eip155:1:${MOCK_ADDRESS}`], undefined, { + staleTime: 0, + gcTime: 0, + }); controller.destroy(); }); diff --git a/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts b/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts index 3de2633e958..eaad7062548 100644 --- a/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts +++ b/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts @@ -427,9 +427,13 @@ export class AccountsApiDataSource extends AbstractDataSource< return response; } - const fetchOptions = request.forceUpdate - ? { staleTime: 100, gcTime: 100 } - : undefined; + // Balances must never be served from the TanStack cache in any flow + // (polling, forced refresh, websocket-triggered): invalidate cached + // balance queries first (fetchQuery refetches invalidated queries + // regardless of freshness), then fetch with staleTime/gcTime 0 so the + // response is neither read back nor retained. + const fetchOptions = { staleTime: 0, gcTime: 0 }; + await this.#apiClient.accounts.invalidateBalances(); // Feature-flagged: v6 endpoint with a fallback to legacy v5. The flag is // read here (not cached) so a runtime toggle can revert v6 -> v5. @@ -482,13 +486,15 @@ export class AccountsApiDataSource extends AbstractDataSource< * Fetch balances from the legacy v5 endpoint and process them. * * @param accountIds - CAIP-10 account IDs to fetch balances for. - * @param fetchOptions - Cache/fetch options (e.g. force update settings). + * @param fetchOptions - No-cache fetch options (balances are never cached). + * @param fetchOptions.staleTime - Time in ms before cached data is stale. + * @param fetchOptions.gcTime - Time in ms before cached data is removed. * @param request - The original data request containing accounts to map. * @returns Unprocessed networks and processed asset balances by account. */ async #fetchV5Balances( accountIds: string[], - fetchOptions: { staleTime: number; gcTime: number } | undefined, + fetchOptions: { staleTime: number; gcTime: number }, request: DataRequest, ): Promise<{ unprocessedNetworks: string[]; @@ -519,13 +525,15 @@ export class AccountsApiDataSource extends AbstractDataSource< * Fetch balances from the v6 endpoint and process them. * * @param accountIds - CAIP-10 account IDs to fetch balances for. - * @param fetchOptions - Cache/fetch options (e.g. force update settings). + * @param fetchOptions - No-cache fetch options (balances are never cached). + * @param fetchOptions.staleTime - Time in ms before cached data is stale. + * @param fetchOptions.gcTime - Time in ms before cached data is removed. * @param request - The original data request containing accounts to map. * @returns Unprocessed networks and processed asset balances by account. */ async #fetchV6Balances( accountIds: string[], - fetchOptions: { staleTime: number; gcTime: number } | undefined, + fetchOptions: { staleTime: number; gcTime: number }, request: DataRequest, ): Promise<{ unprocessedNetworks: string[]; diff --git a/packages/core-backend/src/api/accounts/client.test.ts b/packages/core-backend/src/api/accounts/client.test.ts index 2c02d7cba5a..1160bdf6c3d 100644 --- a/packages/core-backend/src/api/accounts/client.test.ts +++ b/packages/core-backend/src/api/accounts/client.test.ts @@ -336,6 +336,162 @@ describe('AccountsApiClient', () => { }); expect(mockFetch).not.toHaveBeenCalled(); }); + + describe('balances cache behavior', () => { + const v5Response = (balance: string): V5BalancesResponse => ({ + count: 1, + unprocessedNetworks: [], + balances: [ + { + object: 'token', + symbol: 'ETH', + name: 'Ethereum', + type: 'native', + decimals: 18, + assetId: 'eip155:1/slip44:60', + balance, + accountId: 'eip155:1:0x123', + }, + ], + }); + + const v6Response = (balance: string): V6BalancesResponse => ({ + unprocessedNetworks: [], + unprocessedIncludeAssetIds: [], + accounts: [ + { + accountId: 'eip155:1:0x123', + balances: [ + { + category: 'token', + assetId: 'eip155:1/slip44:60', + name: 'Ethereum', + symbol: 'ETH', + decimals: 18, + balance, + }, + ], + processingDefiPositions: false, + }, + ], + }); + + it('reuses a fresh cached v5 response when no fetch options are passed', async () => { + mockFetch.mockResolvedValueOnce(createMockResponse(v5Response('1'))); + + const first = await client.accounts.fetchV5MultiAccountBalances([ + 'eip155:1:0x123', + ]); + const second = await client.accounts.fetchV5MultiAccountBalances([ + 'eip155:1:0x123', + ]); + + expect(mockFetch).toHaveBeenCalledTimes(1); + expect(second).toStrictEqual(first); + }); + + it('refetches v5 balances over the network after invalidateBalances with no-cache options, even when a fresh cache entry exists', async () => { + mockFetch + .mockResolvedValueOnce(createMockResponse(v5Response('1'))) + .mockResolvedValueOnce(createMockResponse(v5Response('2'))); + + await client.accounts.fetchV5MultiAccountBalances(['eip155:1:0x123']); + await client.accounts.invalidateBalances(); + const result = await client.accounts.fetchV5MultiAccountBalances( + ['eip155:1:0x123'], + undefined, + { staleTime: 0, gcTime: 0 }, + ); + + expect(mockFetch).toHaveBeenCalledTimes(2); + expect(result.balances[0]?.balance).toBe('2'); + }); + + it('never reuses cached v5 balances across consecutive fetches with no-cache options', async () => { + mockFetch + .mockResolvedValueOnce(createMockResponse(v5Response('1'))) + .mockResolvedValueOnce(createMockResponse(v5Response('2'))) + .mockResolvedValueOnce(createMockResponse(v5Response('3'))); + + const noCache = { staleTime: 0, gcTime: 0 }; + await client.accounts.fetchV5MultiAccountBalances( + ['eip155:1:0x123'], + undefined, + noCache, + ); + await client.accounts.fetchV5MultiAccountBalances( + ['eip155:1:0x123'], + undefined, + noCache, + ); + const result = await client.accounts.fetchV5MultiAccountBalances( + ['eip155:1:0x123'], + undefined, + noCache, + ); + + expect(mockFetch).toHaveBeenCalledTimes(3); + expect(result.balances[0]?.balance).toBe('3'); + }); + + it('reuses a fresh cached v6 response when no fetch options are passed', async () => { + mockFetch.mockResolvedValueOnce(createMockResponse(v6Response('1'))); + + const first = await client.accounts.fetchV6MultiAccountBalances([ + 'eip155:1:0x123', + ]); + const second = await client.accounts.fetchV6MultiAccountBalances([ + 'eip155:1:0x123', + ]); + + expect(mockFetch).toHaveBeenCalledTimes(1); + expect(second).toStrictEqual(first); + }); + + it('refetches v6 balances over the network after invalidateBalances with no-cache options, even when a fresh cache entry exists', async () => { + mockFetch + .mockResolvedValueOnce(createMockResponse(v6Response('1'))) + .mockResolvedValueOnce(createMockResponse(v6Response('2'))); + + await client.accounts.fetchV6MultiAccountBalances(['eip155:1:0x123']); + await client.accounts.invalidateBalances(); + const result = await client.accounts.fetchV6MultiAccountBalances( + ['eip155:1:0x123'], + undefined, + { staleTime: 0, gcTime: 0 }, + ); + + expect(mockFetch).toHaveBeenCalledTimes(2); + expect(result.accounts[0]?.balances[0]?.balance).toBe('2'); + }); + + it('never reuses cached v6 balances across consecutive fetches with no-cache options', async () => { + mockFetch + .mockResolvedValueOnce(createMockResponse(v6Response('1'))) + .mockResolvedValueOnce(createMockResponse(v6Response('2'))) + .mockResolvedValueOnce(createMockResponse(v6Response('3'))); + + const noCache = { staleTime: 0, gcTime: 0 }; + await client.accounts.fetchV6MultiAccountBalances( + ['eip155:1:0x123'], + undefined, + noCache, + ); + await client.accounts.fetchV6MultiAccountBalances( + ['eip155:1:0x123'], + undefined, + noCache, + ); + const result = await client.accounts.fetchV6MultiAccountBalances( + ['eip155:1:0x123'], + undefined, + noCache, + ); + + expect(mockFetch).toHaveBeenCalledTimes(3); + expect(result.accounts[0]?.balances[0]?.balance).toBe('3'); + }); + }); }); describe('Transactions', () => {