From b6100beed5796b44eb32714212b15521ff4be114 Mon Sep 17 00:00:00 2001 From: local Date: Mon, 26 Oct 2026 10:30:45 +0100 Subject: [PATCH] fix(assets-controller): revalidate quiet snap accounts and treat an empty balance map as a failure Snap accounts that are not pushing balance updates are re-checked after 90 s so a chain whose snap keeps failing is flagged unavailable instead of keeping a stale value. An empty balance map from the snap counts as a failed fetch. Includes the stale-balance tests (flagged unavailable, Retry asks the snap, recovery clears it). --- .../src/data-sources/SnapDataSource.test.ts | 226 ++++++++++++++++++ .../src/data-sources/SnapDataSource.ts | 212 +++++++++++++++- 2 files changed, 437 insertions(+), 1 deletion(-) diff --git a/packages/assets-controller/src/data-sources/SnapDataSource.test.ts b/packages/assets-controller/src/data-sources/SnapDataSource.test.ts index 766697e0698..cea44d470b0 100644 --- a/packages/assets-controller/src/data-sources/SnapDataSource.test.ts +++ b/packages/assets-controller/src/data-sources/SnapDataSource.test.ts @@ -32,6 +32,8 @@ import { KEYRING_PERMISSION, ASSETS_PERMISSION, getSnapBalanceRetryDelay, + SNAP_BALANCE_REVALIDATE_AFTER_MS, + SNAP_EMPTY_BALANCES_ERROR, } from './SnapDataSource.js'; // Test chain IDs @@ -210,6 +212,7 @@ function setupController( getAssetsState?: () => AssetsControllerState; onBalanceFetchStatusChange?: SnapDataSourceOptions['onBalanceFetchStatusChange']; retryBackoff?: SnapDataSourceOptions['retryBackoff']; + revalidation?: SnapDataSourceOptions['revalidation']; } = {}, ): SetupResult { const { @@ -300,6 +303,7 @@ function setupController( getAssetsState, onBalanceFetchStatusChange: options.onBalanceFetchStatusChange, retryBackoff: options.retryBackoff, + revalidation: options.revalidation, }; const controller = new SnapDataSource(controllerOptions); @@ -1398,6 +1402,7 @@ describe('SnapDataSource', () => { isBalanceV6Enabled?: () => boolean; retryBackoff?: SnapDataSourceOptions['retryBackoff']; onBalanceFetchStatusChange?: jest.Mock; + revalidation?: SnapDataSourceOptions['revalidation']; } = {}, ): SetupResult & { failing: { value: boolean }; @@ -1417,6 +1422,7 @@ describe('SnapDataSource', () => { onBalanceFetchStatusChange: onStatus, // Long default so automatic retries never fire unless a test wants them. retryBackoff: options.retryBackoff ?? { initialDelayMs: 60_000 }, + revalidation: options.revalidation, }); setup.mockHandleRequest.mockImplementation(failingHandler(failing)); return { ...setup, failing, onStatus }; @@ -1724,6 +1730,226 @@ describe('SnapDataSource', () => { cleanup(); }); + + describe('revalidation of subscribed accounts (v3)', () => { + const balanceCalls = (mock: jest.Mock): number => + mock.mock.calls.filter( + ([params]) => params.request?.method === 'keyring_getAccountBalances', + ).length; + + const subscribe = async (controller: SnapDataSource): Promise => { + await controller.subscribe({ + request: createDataRequest(), + subscriptionId: 'reval-sub', + isUpdate: false, + onAssetsUpdate: jest.fn(), + }); + }; + + it('flags a quiet, failing snap account unavailable after the revalidation window (TRX case)', async () => { + const { controller, failing, onStatus, mockHandleRequest, cleanup } = + setupFailing(); + failing.value = false; + await flush(); + await subscribe(controller); + expect(controller.getAccountBalanceErrors()).toStrictEqual({}); + + // The snap's own syncs start failing: it stops pushing balances. + failing.value = true; + const before = balanceCalls(mockHandleRequest); + + // Still within the window: nothing is asked. + await controller.revalidateStaleAccounts( + Date.now() + SNAP_BALANCE_REVALIDATE_AFTER_MS - 10_000, + ); + expect(balanceCalls(mockHandleRequest)).toBe(before); + + // Past the window: the snap is asked again, and the failure flags the + // cached balance unavailable. + await controller.revalidateStaleAccounts( + Date.now() + SNAP_BALANCE_REVALIDATE_AFTER_MS + 1, + ); + expect(balanceCalls(mockHandleRequest)).toBe(before + 1); + expect(controller.getAccountBalanceErrors()).toStrictEqual({ + 'mock-account-id': { chainIds: [SOLANA_MAINNET], attempts: 0 }, + }); + expect(lastError(onStatus).error).toBe( + 'the account has not been synchronized yet', + ); + + cleanup(); + }); + + it('flags a synced account unavailable when the snap reports its balance as stale, Retry asks the snap, and recovery clears it (v6)', async () => { + const STALE_MESSAGE = + 'Account balance unavailable: synchronization has been failing since the last successful sync'; + const snapState = { stale: false }; + const { controller, onStatus, mockHandleRequest, cleanup } = + setupFailing(); + mockHandleRequest.mockImplementation( + (params: { request?: { method: string } }): Promise => { + if (params.request?.method === 'keyring_listAccountAssets') { + return Promise.resolve([MOCK_SOL_ASSET]); + } + if (params.request?.method === 'keyring_getAccountBalances') { + // As re-created by the keyring SDK: a plain Error with the message. + return snapState.stale + ? Promise.reject(new Error(STALE_MESSAGE)) + : Promise.resolve({ [MOCK_SOL_ASSET]: SOL_BALANCE }); + } + return Promise.resolve(null); + }, + ); + await flush(); + await subscribe(controller); + expect(controller.getAccountBalanceErrors()).toStrictEqual({}); + + // The connection breaks after the account was synchronized: ~90 s + // later the snap answers with the stale-balance error. + snapState.stale = true; + await controller.revalidateStaleAccounts( + Date.now() + SNAP_BALANCE_REVALIDATE_AFTER_MS + 1, + ); + expect(controller.getAccountBalanceErrors()).toStrictEqual({ + 'mock-account-id': { chainIds: [SOLANA_MAINNET], attempts: 0 }, + }); + expect(lastError(onStatus).error).toBe(STALE_MESSAGE); + + // Retry while still failing: the snap is asked again (it schedules a + // sync) and the account stays unavailable. + const before = balanceCalls(mockHandleRequest); + await controller.retryFailedAccounts(['mock-account-id']); + expect(balanceCalls(mockHandleRequest)).toBe(before + 1); + expect(controller.getAccountBalanceErrors()).toHaveProperty( + 'mock-account-id', + ); + + // The snap syncs again: the next Retry clears the error. + snapState.stale = false; + await controller.retryFailedAccounts(['mock-account-id']); + expect(controller.getAccountBalanceErrors()).toStrictEqual({}); + expect(onStatus).toHaveBeenLastCalledWith( + expect.objectContaining({ + accountId: 'mock-account-id', + error: null, + }), + ); + + cleanup(); + }); + + it('does not ask again when the snap pushed balances recently', async () => { + const { + controller, + failing, + mockHandleRequest, + triggerBalancesUpdated, + cleanup, + } = setupFailing(); + failing.value = false; + await flush(); + await subscribe(controller); + const start = Date.now(); + + await flush(5); + triggerBalancesUpdated({ + balances: { 'mock-account-id': { [MOCK_SOL_ASSET]: SOL_BALANCE } }, + }); + const before = balanceCalls(mockHandleRequest); + + // 90 s after the subscription, but less than 90 s after the push. + await controller.revalidateStaleAccounts( + start + SNAP_BALANCE_REVALIDATE_AFTER_MS + 1, + ); + + expect(balanceCalls(mockHandleRequest)).toBe(before); + cleanup(); + }); + + it('leaves failing accounts to their scheduled retries', async () => { + const { controller, mockHandleRequest, cleanup } = setupFailing(); + await flush(); + await subscribe(controller); + expect(controller.getAccountBalanceErrors()).toHaveProperty( + 'mock-account-id', + ); + const before = balanceCalls(mockHandleRequest); + + await controller.revalidateStaleAccounts(Date.now() + 10 * 60_000); + + expect(balanceCalls(mockHandleRequest)).toBe(before); + cleanup(); + }); + + it('runs on its own timer, and stops after unsubscribe', async () => { + const { controller, failing, mockHandleRequest, cleanup } = + setupFailing({ revalidation: { afterMs: 20, checkIntervalMs: 10 } }); + failing.value = false; + await flush(); + await subscribe(controller); + const before = balanceCalls(mockHandleRequest); + + await flush(80); + expect(balanceCalls(mockHandleRequest)).toBeGreaterThan(before); + + await controller.unsubscribe('reval-sub'); + await controller.unsubscribe('test-sub'); + await flush(5); + const afterUnsubscribe = balanceCalls(mockHandleRequest); + await flush(60); + expect(balanceCalls(mockHandleRequest)).toBe(afterUnsubscribe); + + cleanup(); + }); + + it('can be disabled', async () => { + const { controller, failing, mockHandleRequest, cleanup } = + setupFailing({ revalidation: false }); + failing.value = false; + await flush(); + await subscribe(controller); + const before = balanceCalls(mockHandleRequest); + + await controller.revalidateStaleAccounts(Date.now() + 10 * 60_000); + + expect(balanceCalls(mockHandleRequest)).toBe(before); + cleanup(); + }); + + it.each([ + ['v6', true], + ['v5', false], + ])( + 'an empty balance map for requested assets is a failure, not a confirmed balance (SOL case, %s)', + async (_label, v6) => { + const { controller, mockHandleRequest, onStatus, cleanup } = + setupFailing({ isBalanceV6Enabled: (): boolean => v6 }); + mockHandleRequest.mockImplementation( + (params: { request?: { method: string } }) => { + if (params.request?.method === 'keyring_listAccountAssets') { + return Promise.resolve([MOCK_SOL_ASSET]); + } + if (params.request?.method === 'keyring_getAccountBalances') { + return Promise.resolve({}); + } + return Promise.resolve(null); + }, + ); + await flush(); + + const response = await controller.fetch(createDataRequest()); + + expect(response.assetsBalance).toStrictEqual({}); + expect(controller.getAccountBalanceErrors()).toHaveProperty( + 'mock-account-id', + ); + expect(lastError(onStatus).error).toContain( + SNAP_EMPTY_BALANCES_ERROR, + ); + cleanup(); + }, + ); + }); }); it('createSnapDataSource factory creates instance', async () => { diff --git a/packages/assets-controller/src/data-sources/SnapDataSource.ts b/packages/assets-controller/src/data-sources/SnapDataSource.ts index 0093532e5c4..78fbe472e4b 100644 --- a/packages/assets-controller/src/data-sources/SnapDataSource.ts +++ b/packages/assets-controller/src/data-sources/SnapDataSource.ts @@ -86,6 +86,23 @@ export const SNAP_BALANCE_RETRY_INITIAL_DELAY_MS = 30_000; /** Maximum automatic retry delay after repeated snap balance fetch failures. */ export const SNAP_BALANCE_RETRY_MAX_DELAY_MS = 5 * 60_000; +/** + * How long a subscribed snap account may go without a confirmed balance (a + * successful fetch or a balance pushed by the snap) before the data source + * asks the snap again. Snaps push balances only after a successful sync, so a + * snap whose syncs keep failing goes quiet: without this revalidation the + * client would keep showing the cached balance until something else fetches + * it. Healthy snaps push every 30-60 s, so they are not asked again. + */ +export const SNAP_BALANCE_REVALIDATE_AFTER_MS = 90_000; + +/** How often subscribed snap accounts are checked for revalidation. */ +export const SNAP_BALANCE_REVALIDATE_CHECK_INTERVAL_MS = 15_000; + +/** Error recorded when a snap returns no balance for the requested assets. */ +export const SNAP_EMPTY_BALANCES_ERROR = + 'Snap returned no balances for the requested assets'; + /** * Exponential backoff for automatic snap balance retries. * @@ -232,6 +249,11 @@ export type SnapDataSourceOptions = { onBalanceFetchStatusChange?: (update: SnapBalanceFetchStatusUpdate) => void; /** Automatic retry backoff for failed per-account fetches. */ retryBackoff?: { initialDelayMs?: number; maxDelayMs?: number }; + /** + * Revalidation of subscribed accounts whose balance was not confirmed + * recently (see `SNAP_BALANCE_REVALIDATE_AFTER_MS`). `false` disables it. + */ + revalidation?: { afterMs?: number; checkIntervalMs?: number } | false; /** Initial state */ state?: Partial; }; @@ -262,6 +284,13 @@ type FailedAccountEntry = { retryInProgress: 'scheduled' | 'manual' | null; }; +/** + * A subscribed snap account, with the narrowed request used to revalidate it. + */ +type WatchedAccountEntry = { + request: DataRequest; +}; + /** * Unified Snap data source that routes requests to the appropriate wallet snap * based on the chain ID prefix. @@ -324,6 +353,25 @@ export class SnapDataSource extends AbstractDataSource< */ readonly #failedAccounts: Map = new Map(); + /** Subscribed snap accounts per subscription ID. */ + readonly #watchedAccounts: Map> = + new Map(); + + /** + * Last time each account's balance was confirmed: a successful fetch or a + * balance pushed by its snap. + */ + readonly #lastConfirmedAt: Map = new Map(); + + /** Accounts whose revalidation fetch is in progress. */ + readonly #revalidating: Set = new Set(); + + readonly #revalidateAfterMs: number | null; + + readonly #revalidateCheckIntervalMs: number; + + #revalidationTimer: ReturnType | null = null; + #destroyed = false; constructor(options: SnapDataSourceOptions) { @@ -344,6 +392,15 @@ export class SnapDataSource extends AbstractDataSource< SNAP_BALANCE_RETRY_INITIAL_DELAY_MS; this.#retryMaxDelayMs = options.retryBackoff?.maxDelayMs ?? SNAP_BALANCE_RETRY_MAX_DELAY_MS; + this.#revalidateAfterMs = + options.revalidation === false + ? null + : (options.revalidation?.afterMs ?? SNAP_BALANCE_REVALIDATE_AFTER_MS); + this.#revalidateCheckIntervalMs = + (options.revalidation === false + ? undefined + : options.revalidation?.checkIntervalMs) ?? + SNAP_BALANCE_REVALIDATE_CHECK_INTERVAL_MS; // Bind handlers for cleanup in destroy() this.#handleSnapBalancesUpdatedBound = @@ -622,8 +679,21 @@ export class SnapDataSource extends AbstractDataSource< const balances: Record = await client.getAccountBalances(accountId, accountAssets); + // The snap listed assets but returned no balance for any of them: + // that is a failed fetch (snaps that swallow network errors answer + // with an empty map), not a confirmed balance. + if ( + !balances || + typeof balances !== 'object' || + Object.keys(balances).length === 0 + ) { + throw new Error( + `${SNAP_EMPTY_BALANCES_ERROR}: ${accountAssets.length} asset(s) requested`, + ); + } + // Transform keyring response to DataResponse format - if (balances && typeof balances === 'object' && results.assetsBalance) { + if (results.assetsBalance) { for (const [assetId, balance] of Object.entries(balances)) { results.assetsBalance[accountId] ??= {}; const accountBalances = results.assetsBalance[accountId]; @@ -714,6 +784,14 @@ export class SnapDataSource extends AbstractDataSource< typeof balances !== 'object' || Object.keys(balances).length === 0 ) { + if (assetsToFetch.length > 0) { + // Assets were requested but none came back: the snap swallowed + // a failed fetch. Record it so the cached balance is flagged + // unavailable instead of being shown as current. + throw new Error( + `${SNAP_EMPTY_BALANCES_ERROR}: ${assetsToFetch.length} asset(s) requested`, + ); + } continue; } @@ -951,9 +1029,15 @@ export class SnapDataSource extends AbstractDataSource< ); if (supportedChains.length === 0) { + this.#unwatchSubscription(subscriptionId); return; } + this.#watchSubscription(subscriptionId, { + ...request, + chainIds: supportedChains, + }); + try { const fetchResponse = await this.fetch({ ...request, @@ -1008,7 +1092,126 @@ export class SnapDataSource extends AbstractDataSource< ); } + async unsubscribe(subscriptionId: string): Promise { + this.#unwatchSubscription(subscriptionId); + await super.unsubscribe(subscriptionId); + } + + // ============================================================================ + // REVALIDATION OF SUBSCRIBED ACCOUNTS + // ============================================================================ + + /** + * Remember the snap accounts of a subscription, so that their balance is + * revalidated when it is not confirmed for a while. + * + * @param subscriptionId - The subscription ID. + * @param request - The subscription request, narrowed to snap chains. + */ + #watchSubscription(subscriptionId: string, request: DataRequest): void { + if (this.#revalidateAfterMs === null || this.#destroyed) { + return; + } + const now = Date.now(); + const watched = new Map(); + for (const entry of request.accountsWithSupportedChains ?? []) { + const snapId = entry.account.metadata.snap?.id; + if (!snapId) { + continue; + } + const chainIds = request.chainIds.filter( + (chainId) => this.state.chainToSnap[chainId] === snapId, + ); + if (chainIds.length === 0) { + continue; + } + const accountId = entry.account.id; + watched.set(accountId, { + request: { + ...request, + chainIds, + accountsWithSupportedChains: [entry], + forceUpdate: true, + }, + }); + // The subscription fetches it right away: start the clock now. + if (!this.#lastConfirmedAt.has(accountId)) { + this.#lastConfirmedAt.set(accountId, now); + } + } + if (watched.size === 0) { + this.#unwatchSubscription(subscriptionId); + return; + } + this.#watchedAccounts.set(subscriptionId, watched); + if (!this.#revalidationTimer) { + this.#revalidationTimer = setInterval(() => { + this.revalidateStaleAccounts().catch((error) => + log('Snap balance revalidation failed', { error }), + ); + }, this.#revalidateCheckIntervalMs); + // Do not keep a Node process (tests, scripts) alive for revalidation. + (this.#revalidationTimer as { unref?: () => void }).unref?.(); + } + } + + #unwatchSubscription(subscriptionId: string): void { + this.#watchedAccounts.delete(subscriptionId); + if (this.#watchedAccounts.size === 0 && this.#revalidationTimer) { + clearInterval(this.#revalidationTimer); + this.#revalidationTimer = null; + } + } + + /** + * Ask the snaps again for subscribed accounts whose balance was not + * confirmed (fetched or pushed) within the revalidation window. A failure + * flags the balance unavailable and starts the retry backoff; accounts that + * are already failing are left to their scheduled retries. + * + * @param now - Current time (for tests). + */ + async revalidateStaleAccounts(now: number = Date.now()): Promise { + if (this.#revalidateAfterMs === null || this.#destroyed) { + return; + } + const due = new Map(); + for (const watched of this.#watchedAccounts.values()) { + for (const [accountId, entry] of watched) { + if ( + due.has(accountId) || + this.#failedAccounts.has(accountId) || + this.#revalidating.has(accountId) + ) { + continue; + } + const confirmedAt = this.#lastConfirmedAt.get(accountId) ?? 0; + if (now - confirmedAt >= this.#revalidateAfterMs) { + due.set(accountId, entry); + } + } + } + + await Promise.all( + Array.from(due.entries()).map(async ([accountId, entry]) => { + this.#revalidating.add(accountId); + try { + log('Revalidating snap balance', { accountId }); + // `fetch` records the outcome: success confirms the balance, a + // failure flags it unavailable and schedules a retry. + const response = await this.fetch(entry.request); + if (Object.keys(response.assetsBalance ?? {}).length > 0) { + await this.#onAssetsUpdate(response); + } + } finally { + this.#revalidating.delete(accountId); + } + }), + ); + } + #markAccountSucceeded(accountId: string): void { + this.#lastConfirmedAt.set(accountId, Date.now()); const failure = this.#failedAccounts.get(accountId); if (!failure) { return; @@ -1246,6 +1449,13 @@ export class SnapDataSource extends AbstractDataSource< // Clear keyring client cache this.#keyringClientCache.clear(); + // Stop revalidation + if (this.#revalidationTimer) { + clearInterval(this.#revalidationTimer); + this.#revalidationTimer = null; + } + this.#watchedAccounts.clear(); + // Stop pending retries this.#destroyed = true; for (const failure of this.#failedAccounts.values()) {