From 0548318b63d9f07832fbe3f998e49f20f2599274 Mon Sep 17 00:00:00 2001 From: Xiaoming Wang Date: Mon, 19 Oct 2026 22:04:48 +0800 Subject: [PATCH 01/24] feat(wallet): add opt-in AssetsController initialization Export an AssetsController initialization config so clients can register it after NetworkEnablementController, which the wallet does not initialize yet. Co-authored-by: Cursor --- .github/CODEOWNERS | 1 + codeowners.ts | 1 + packages/wallet/CHANGELOG.md | 10 + packages/wallet/package.json | 1 + packages/wallet/src/index.ts | 2 + .../assets-controller.test.ts | 389 ++++++++++++++++++ .../assets-controller/assets-controller.ts | 100 +++++ .../instances/assets-controller/types.ts | 12 + packages/wallet/src/types.ts | 2 + packages/wallet/tsconfig.build.json | 3 + packages/wallet/tsconfig.json | 3 + packages/wallet/tsconfig.lint.json | 3 + yarn.lock | 1 + 13 files changed, 528 insertions(+) create mode 100644 packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts create mode 100644 packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts create mode 100644 packages/wallet/src/initialization/instances/assets-controller/types.ts diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 63dfec54bf6..aaf7a1a7688 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -744,6 +744,7 @@ /packages/wallet/src/initialization/instances/accounts-controller/ @MetaMask/accounts-engineers /packages/wallet/src/initialization/instances/address-book-controller/ @MetaMask/confirmations /packages/wallet/src/initialization/instances/approval-controller/ @MetaMask/confirmations +/packages/wallet/src/initialization/instances/assets-controller/ @MetaMask/metamask-assets /packages/wallet/src/initialization/instances/claims-controller/ @MetaMask/web3auth /packages/wallet/src/initialization/instances/claims-service/ @MetaMask/web3auth /packages/wallet/src/initialization/instances/client-controller/ @MetaMask/core-platform @MetaMask/extension-platform @MetaMask/mobile-platform diff --git a/codeowners.ts b/codeowners.ts index ea06b856a32..2b9e6a43a34 100644 --- a/codeowners.ts +++ b/codeowners.ts @@ -44,6 +44,7 @@ const config = { }, 'assets-controller': { teams: ['@MetaMask/metamask-assets'], + initializationPath: 'assets-controller', }, 'assets-controllers': { teams: ['@MetaMask/metamask-assets'], diff --git a/packages/wallet/CHANGELOG.md b/packages/wallet/CHANGELOG.md index 8f55b160f87..4c0a6720bb0 100644 --- a/packages/wallet/CHANGELOG.md +++ b/packages/wallet/CHANGELOG.md @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Export an `assetsController` initialization configuration for `AssetsController`, along with the `AssetsControllerInstanceOptions` type + - It is not part of the default initialization configurations, because the Wallet does not yet initialize several of its dependencies (e.g. `NetworkEnablementController`, `AccountTreeController`, `SnapController`). Pass it through `initializationConfigurations` after configurations that register `NetworkEnablementController:getState`. + - Configure it through the new optional `instanceOptions.assetsController`, which requires a `queryApiClient`. + +### Changed + +- Add `@metamask/assets-controller` as a dependency + ## [18.0.0] ### Added diff --git a/packages/wallet/package.json b/packages/wallet/package.json index e1b02a65f2e..e8bf1b4dd22 100644 --- a/packages/wallet/package.json +++ b/packages/wallet/package.json @@ -53,6 +53,7 @@ "@metamask/accounts-controller": "^40.1.0", "@metamask/address-book-controller": "^8.0.0", "@metamask/approval-controller": "^10.0.0", + "@metamask/assets-controller": "^19.1.0", "@metamask/base-controller": "^10.1.0", "@metamask/browser-passworder": "^6.0.0", "@metamask/claims-controller": "^1.0.3", diff --git a/packages/wallet/src/index.ts b/packages/wallet/src/index.ts index 50e9b004160..e5672faba00 100644 --- a/packages/wallet/src/index.ts +++ b/packages/wallet/src/index.ts @@ -1,6 +1,8 @@ export { Wallet } from './Wallet.js'; export { AlwaysOnlineAdapter } from './initialization/instances/connectivity-controller/always-online-adapter.js'; export { importSecretRecoveryPhrase } from './utilities.js'; +export { assetsController } from './initialization/instances/assets-controller/assets-controller.js'; +export type { AssetsControllerInstanceOptions } from './initialization/instances/assets-controller/types.js'; export type { WalletOptions } from './types.js'; export type { DefaultActions, diff --git a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts new file mode 100644 index 00000000000..cc7a8e31e88 --- /dev/null +++ b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts @@ -0,0 +1,389 @@ +import { + AssetsController, + getDefaultAssetsControllerState, +} from '@metamask/assets-controller'; +import type { AssetsControllerGetStateAction } from '@metamask/assets-controller'; +import { ConfigRegistryApiEnv } from '@metamask/config-registry-controller'; +import { Messenger } from '@metamask/messenger'; +import { InMemoryStorageAdapter } from '@metamask/storage-service'; + +import * as walletExports from '../../../index.js'; +import type { WalletOptions } from '../../../types.js'; +import { Wallet } from '../../../Wallet.js'; +import { defaultConfigurations } from '../../defaults.js'; +import type { + DefaultActions, + DefaultEvents, + RootMessenger, +} from '../../defaults.js'; +import type { AnyInitializationConfiguration } from '../../types.js'; +import { AlwaysOnlineAdapter } from '../connectivity-controller/always-online-adapter.js'; +import { assetsController } from './assets-controller.js'; +import type { AssetsControllerInstanceOptions } from './types.js'; + +const controllers: AssetsController[] = []; +const wallets: Wallet[] = []; + +const REMOTE_FEATURE_FLAG_OPTIONS = { + clientConfigApiService: { + fetchRemoteFeatureFlags: async (): Promise<{ + remoteFeatureFlags: Record; + cacheTimestamp: number; + }> => ({ remoteFeatureFlags: {}, cacheTimestamp: Date.now() }), + }, +}; + +const NETWORK_ENABLEMENT_STATE = { + enabledNetworkMap: { eip155: { '0x1': true } }, + nativeAssetIdentifiers: { 'eip155:1': 'eip155:1/slip44:60' }, +}; + +type ActionHandler = (...args: unknown[]) => unknown; + +type AnyMessenger = Messenger; + +describe('assetsController', () => { + afterEach(async () => { + for (const controller of controllers.splice(0)) { + controller.destroy(); + } + + await Promise.all(wallets.splice(0).map((wallet) => wallet.destroy())); + }); + + it('is not part of the default initialization configurations', () => { + expect(Object.values(defaultConfigurations)).not.toContain( + assetsController, + ); + }); + + it('is exported from the package entry point', () => { + expect(walletExports.assetsController).toBe(assetsController); + }); + + it('initializes an AssetsController with default state', () => { + const rootMessenger = getRootMessenger(); + const messenger = assetsController.getMessenger(rootMessenger); + + const instance = assetsController.init({ + state: undefined, + messenger, + options: { queryApiClient: createMockQueryApiClient() }, + }); + controllers.push(instance); + + expect(instance).toBeInstanceOf(AssetsController); + expect(instance.state).toStrictEqual(getDefaultAssetsControllerState()); + }); + + it('forwards the provided state to the controller', () => { + const rootMessenger = getRootMessenger(); + const messenger = assetsController.getMessenger(rootMessenger); + + const instance = assetsController.init({ + state: { + ...getDefaultAssetsControllerState(), + selectedCurrency: 'eur', + }, + messenger, + options: { queryApiClient: createMockQueryApiClient() }, + }); + controllers.push(instance); + + expect(instance.state.selectedCurrency).toBe('eur'); + }); + + it('forwards the instance options to the controller', () => { + const rootMessenger = getRootMessenger(); + const messenger = assetsController.getMessenger(rootMessenger); + const unsubscribe = jest.fn(); + const subscribeToBasicFunctionalityChange = jest + .fn() + .mockReturnValue(unsubscribe); + + const instance = assetsController.init({ + state: undefined, + messenger, + options: { + queryApiClient: createMockQueryApiClient(), + subscribeToBasicFunctionalityChange, + }, + }); + instance.destroy(); + + expect(subscribeToBasicFunctionalityChange).toHaveBeenCalledTimes(1); + expect(unsubscribe).toHaveBeenCalledTimes(1); + }); + + it('throws when no queryApiClient is provided', () => { + const rootMessenger = getRootMessenger(); + const messenger = assetsController.getMessenger(rootMessenger); + + expect(() => + assetsController.init({ + state: undefined, + messenger, + options: {} as AssetsControllerInstanceOptions, + }), + ).toThrow( + 'AssetsController requires `instanceOptions.assetsController.queryApiClient`.', + ); + }); + + it('exposes AssetsController actions over the root messenger', () => { + const rootMessenger = getRootMessenger(); + const messenger = assetsController.getMessenger(rootMessenger); + + const instance = assetsController.init({ + state: undefined, + messenger, + options: { queryApiClient: createMockQueryApiClient() }, + }); + controllers.push(instance); + + expect( + ( + rootMessenger as unknown as Messenger< + 'Root', + AssetsControllerGetStateAction + > + ).call('AssetsController:getState'), + ).toStrictEqual(getDefaultAssetsControllerState()); + }); + + it.each([ + 'AccountsController:getSelectedAccount', + 'AccountTreeController:getAccountsFromSelectedAccountGroup', + 'AccountTreeController:isInitialized', + 'ClientController:getState', + 'ConfigRegistryController:getNetworkConfigByCaip2ChainId', + 'KeyringController:isUnlocked', + 'NetworkController:getNetworkClientById', + 'NetworkController:getState', + 'NetworkEnablementController:getState', + 'PermissionController:getPermissions', + 'PhishingController:bulkScanTokens', + 'RemoteFeatureFlagController:getState', + 'SnapController:getRunnableSnaps', + 'SnapController:handleRequest', + ])('delegates the %s action', (actionType) => { + const rootMessenger = new Messenger<'Root', DefaultActions, DefaultEvents>({ + namespace: 'Root', + }); + const handler = jest.fn().mockReturnValue('result'); + registerActionHandler(rootMessenger, actionType, handler); + const messenger = assetsController.getMessenger(rootMessenger); + + const result = ( + messenger as unknown as { call(type: string): unknown } + ).call(actionType); + + expect(handler).toHaveBeenCalledTimes(1); + expect(result).toBe('result'); + }); + + it.each([ + 'AccountActivityService:balanceUpdated', + 'AccountActivityService:statusChanged', + 'AccountsController:accountBalancesUpdated', + 'AccountTreeController:initialized', + 'AccountTreeController:selectedAccountGroupChange', + 'AccountTreeController:uninitialized', + 'ClientController:stateChange', + 'KeyringController:lock', + 'KeyringController:unlock', + 'MultichainTransactionsController:transactionConfirmed', + 'NetworkController:networkAdded', + 'NetworkController:networkDidChange', + 'NetworkController:networkRemoved', + 'NetworkController:stateChange', + 'NetworkEnablementController:stateChange', + 'PermissionController:stateChange', + 'PreferencesController:stateChange', + 'RemoteFeatureFlagController:stateChange', + 'SnapController:snapInstalled', + 'TransactionController:transactionConfirmed', + 'TransactionController:unapprovedTransactionAdded', + ])('delegates the %s event', (eventType) => { + const rootMessenger = new Messenger<'Root', DefaultActions, DefaultEvents>({ + namespace: 'Root', + }); + const messenger = assetsController.getMessenger(rootMessenger); + const listener = jest.fn(); + ( + messenger as unknown as { + subscribe(type: string, handler: ActionHandler): void; + } + ).subscribe(eventType, listener); + + publishEvent(rootMessenger, eventType, 'payload'); + + expect(listener).toHaveBeenCalledWith('payload'); + }); + + it('is initialized by the Wallet when passed as an initialization configuration', async () => { + const wallet = new Wallet({ + initializationConfigurations: [ + getNetworkEnablementControllerStub(), + assetsController, + ], + instanceOptions: { + ...getInstanceOptions(), + assetsController: { queryApiClient: createMockQueryApiClient() }, + }, + }); + wallets.push(wallet); + + const instance = wallet.getInstance('AssetsController'); + + expect(instance).toBeInstanceOf(AssetsController); + expect( + ( + wallet.messenger as unknown as Messenger< + 'Root', + AssetsControllerGetStateAction + > + ).call('AssetsController:getState'), + ).toStrictEqual(getDefaultAssetsControllerState()); + }); + + it('is destroyed with the Wallet', async () => { + const wallet = new Wallet({ + initializationConfigurations: [ + getNetworkEnablementControllerStub(), + assetsController, + ], + instanceOptions: { + ...getInstanceOptions(), + assetsController: { queryApiClient: createMockQueryApiClient() }, + }, + }); + const destroy = jest.spyOn( + wallet.getInstance('AssetsController') as unknown as AssetsController, + 'destroy', + ); + + await wallet.destroy(); + + expect(destroy).toHaveBeenCalledTimes(1); + }); +}); + +/** + * Builds a minimal `queryApiClient` that serves the controller's native asset + * query from its own `queryFn`, so no API request is made. + * + * @returns The mock client. + */ +function createMockQueryApiClient(): AssetsControllerInstanceOptions['queryApiClient'] { + const cache = new Map(); + return { + getCachedData: jest.fn((key: string[]) => cache.get(JSON.stringify(key))), + setCachedData: jest.fn((key: string[], data: unknown) => + cache.set(JSON.stringify(key), data), + ), + queryClient: { + fetchQuery: jest.fn( + async (opts: { + queryKey: string[]; + queryFn: () => Promise; + }) => { + const data = await opts.queryFn(); + cache.set(JSON.stringify(opts.queryKey), data); + return data; + }, + ), + }, + } as unknown as AssetsControllerInstanceOptions['queryApiClient']; +} + +/** + * Builds a stand-in for the `NetworkEnablementController` configuration, + * which the Wallet does not yet initialize by default. + * + * @returns The initialization configuration. + */ +function getNetworkEnablementControllerStub(): AnyInitializationConfiguration { + return { + name: 'NetworkEnablementController', + getMessenger: (parent: RootMessenger) => { + registerActionHandler( + parent, + 'NetworkEnablementController:getState', + () => NETWORK_ENABLEMENT_STATE, + ); + return new Messenger({ namespace: 'NetworkEnablementControllerStub' }); + }, + init: (): object => ({}), + }; +} + +function getRootMessenger(): RootMessenger { + const rootMessenger = new Messenger<'Root', DefaultActions, DefaultEvents>({ + namespace: 'Root', + }); + + registerActionHandler( + rootMessenger, + 'NetworkEnablementController:getState', + () => NETWORK_ENABLEMENT_STATE, + ); + + return rootMessenger; +} + +function getInstanceOptions(): WalletOptions['instanceOptions'] { + return { + configRegistryApiService: { env: ConfigRegistryApiEnv.PRD }, + connectivityController: { + connectivityAdapter: new AlwaysOnlineAdapter(), + }, + gasFeeController: { + clientId: 'test', + }, + networkController: { + infuraProjectId: 'test-infura-project-id', + }, + selectedNetworkController: { + domainProxyMap: new Map(), + }, + storageService: { + storage: new InMemoryStorageAdapter(), + }, + remoteFeatureFlagController: REMOTE_FEATURE_FLAG_OPTIONS, + }; +} + +function registerActionHandler( + parent: RootMessenger, + actionType: string, + handler: ActionHandler, +): void { + const messenger = new Messenger({ + namespace: actionType.split(':')[0], + parent: parent as unknown as AnyMessenger, + }); + + ( + messenger as unknown as { + registerActionHandler(type: string, handler: ActionHandler): void; + } + ).registerActionHandler(actionType, handler); +} + +function publishEvent( + parent: RootMessenger, + eventType: string, + ...payload: unknown[] +): void { + const messenger = new Messenger({ + namespace: eventType.split(':')[0], + parent: parent as unknown as AnyMessenger, + }); + + ( + messenger as unknown as { + publish(type: string, ...payload: unknown[]): void; + } + ).publish(eventType, ...payload); +} diff --git a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts new file mode 100644 index 00000000000..b2e2a21771d --- /dev/null +++ b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts @@ -0,0 +1,100 @@ +import { AssetsController } from '@metamask/assets-controller'; +import type { AssetsControllerMessenger } from '@metamask/assets-controller'; +import { Messenger } from '@metamask/messenger'; +import type { MessengerActions, MessengerEvents } from '@metamask/messenger'; + +import type { InitializationConfiguration } from '../../types.js'; + +export type { AssetsControllerInstanceOptions } from './types.js'; + +/** + * Initialization configuration for `AssetsController`. + * + * This is not part of the default initialization configurations because the + * Wallet does not yet initialize several of the controllers it depends on + * (e.g. `NetworkEnablementController`, `AccountTreeController`, + * `SnapController`). Clients pass it through `initializationConfigurations`, + * after the configurations that register those dependencies; + * `NetworkEnablementController:getState` in particular must be registered + * before this configuration is initialized, as the controller reads it on + * construction. + */ +export const assetsController: InitializationConfiguration< + AssetsController, + AssetsControllerMessenger +> = { + name: 'AssetsController', + reference: AssetsController, + init: ({ state, messenger, options }) => { + if (!options.queryApiClient) { + throw new Error( + 'AssetsController requires `instanceOptions.assetsController.queryApiClient`.', + ); + } + + return new AssetsController({ + ...options, + messenger, + state, + }); + }, + getMessenger: (parent) => { + // Most of these dependencies are not initialized by the Wallet by default, + // so the root messenger is widened to the full set the controller uses. + const rootMessenger = parent as unknown as Messenger< + 'Root', + MessengerActions, + MessengerEvents + >; + + const messenger: AssetsControllerMessenger = new Messenger({ + namespace: 'AssetsController', + parent: rootMessenger, + }); + + rootMessenger.delegate({ + messenger, + actions: [ + 'AccountsController:getSelectedAccount', + 'AccountTreeController:getAccountsFromSelectedAccountGroup', + 'AccountTreeController:isInitialized', + 'ClientController:getState', + 'ConfigRegistryController:getNetworkConfigByCaip2ChainId', + 'KeyringController:isUnlocked', + 'NetworkController:getNetworkClientById', + 'NetworkController:getState', + 'NetworkEnablementController:getState', + 'PermissionController:getPermissions', + 'PhishingController:bulkScanTokens', + 'RemoteFeatureFlagController:getState', + 'SnapController:getRunnableSnaps', + 'SnapController:handleRequest', + ], + events: [ + 'AccountActivityService:balanceUpdated', + 'AccountActivityService:statusChanged', + 'AccountsController:accountBalancesUpdated', + 'AccountTreeController:initialized', + 'AccountTreeController:selectedAccountGroupChange', + 'AccountTreeController:uninitialized', + 'ClientController:stateChange', + 'KeyringController:lock', + 'KeyringController:unlock', + 'MultichainTransactionsController:transactionConfirmed', + 'NetworkController:networkAdded', + 'NetworkController:networkDidChange', + 'NetworkController:networkRemoved', + 'NetworkController:stateChange', + 'NetworkEnablementController:stateChange', + 'PermissionController:stateChange', + 'PreferencesController:stateChange', + 'RemoteFeatureFlagController:stateChange', + 'SnapController:snapInstalled', + 'TransactionController:transactionConfirmed', + 'TransactionController:unapprovedTransactionAdded', + ], + }); + + return messenger; + }, +}; diff --git a/packages/wallet/src/initialization/instances/assets-controller/types.ts b/packages/wallet/src/initialization/instances/assets-controller/types.ts new file mode 100644 index 00000000000..92f7042da2d --- /dev/null +++ b/packages/wallet/src/initialization/instances/assets-controller/types.ts @@ -0,0 +1,12 @@ +import type { AssetsControllerOptions } from '@metamask/assets-controller'; + +/** + * Per-instance options for the wallet's `AssetsController`. `queryApiClient` + * is required because each client constructs its own API Platform client + * (product name, bearer token, cache); every other field is optional and + * falls back to the controller's own default. + */ +export type AssetsControllerInstanceOptions = Omit< + AssetsControllerOptions, + 'messenger' | 'state' +>; diff --git a/packages/wallet/src/types.ts b/packages/wallet/src/types.ts index df08dde18a9..7696994a5c4 100644 --- a/packages/wallet/src/types.ts +++ b/packages/wallet/src/types.ts @@ -6,6 +6,7 @@ import type { RootMessenger, } from './initialization/defaults.js'; import type { ApprovalControllerInstanceOptions } from './initialization/instances/approval-controller/types.js'; +import type { AssetsControllerInstanceOptions } from './initialization/instances/assets-controller/types.js'; import type { ClaimsServiceInstanceOptions } from './initialization/instances/claims-service/types.js'; import type { ConfigRegistryApiServiceInstanceOptions } from './initialization/instances/config-registry-api-service/types.js'; import type { ConfigRegistryControllerInstanceOptions } from './initialization/instances/config-registry-controller/types.js'; @@ -35,6 +36,7 @@ export type WalletOptions = { export type InstanceSpecificOptions = { approvalController?: ApprovalControllerInstanceOptions; + assetsController?: AssetsControllerInstanceOptions; claimsService?: ClaimsServiceInstanceOptions; configRegistryApiService: ConfigRegistryApiServiceInstanceOptions; configRegistryController?: ConfigRegistryControllerInstanceOptions; diff --git a/packages/wallet/tsconfig.build.json b/packages/wallet/tsconfig.build.json index 68efc97ee72..8d42a9e4ca6 100644 --- a/packages/wallet/tsconfig.build.json +++ b/packages/wallet/tsconfig.build.json @@ -14,6 +14,9 @@ { "path": "../approval-controller/tsconfig.build.json" }, + { + "path": "../assets-controller/tsconfig.build.json" + }, { "path": "../claims-controller/tsconfig.build.json" }, diff --git a/packages/wallet/tsconfig.json b/packages/wallet/tsconfig.json index a8b09405b97..9fcdba74235 100644 --- a/packages/wallet/tsconfig.json +++ b/packages/wallet/tsconfig.json @@ -13,6 +13,9 @@ { "path": "../approval-controller" }, + { + "path": "../assets-controller" + }, { "path": "../claims-controller" }, diff --git a/packages/wallet/tsconfig.lint.json b/packages/wallet/tsconfig.lint.json index 34b96909506..5d9f9907216 100644 --- a/packages/wallet/tsconfig.lint.json +++ b/packages/wallet/tsconfig.lint.json @@ -20,6 +20,9 @@ { "path": "../approval-controller/tsconfig.lint.json" }, + { + "path": "../assets-controller/tsconfig.lint.json" + }, { "path": "../base-controller/tsconfig.lint.json" }, diff --git a/yarn.lock b/yarn.lock index 4fa8982f18e..9bb32d41b3c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9693,6 +9693,7 @@ __metadata: "@metamask/accounts-controller": "npm:^40.1.0" "@metamask/address-book-controller": "npm:^8.0.0" "@metamask/approval-controller": "npm:^10.0.0" + "@metamask/assets-controller": "npm:^19.1.0" "@metamask/auto-changelog": "npm:^6.2.1" "@metamask/base-controller": "npm:^10.1.0" "@metamask/browser-passworder": "npm:^6.0.0" From 3ef01a938a35420edd737e2cad904a62ce7559b8 Mon Sep 17 00:00:00 2001 From: Xiaoming Wang Date: Mon, 19 Oct 2026 22:08:35 +0800 Subject: [PATCH 02/24] docs(wallet): link AssetsController changelog entries to the PR Co-authored-by: Cursor --- packages/wallet/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/wallet/CHANGELOG.md b/packages/wallet/CHANGELOG.md index 4c0a6720bb0..ef0aa525d7a 100644 --- a/packages/wallet/CHANGELOG.md +++ b/packages/wallet/CHANGELOG.md @@ -9,13 +9,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Export an `assetsController` initialization configuration for `AssetsController`, along with the `AssetsControllerInstanceOptions` type +- Export an `assetsController` initialization configuration for `AssetsController`, along with the `AssetsControllerInstanceOptions` type ([#10885](https://github.com/MetaMask/core/pull/10885)) - It is not part of the default initialization configurations, because the Wallet does not yet initialize several of its dependencies (e.g. `NetworkEnablementController`, `AccountTreeController`, `SnapController`). Pass it through `initializationConfigurations` after configurations that register `NetworkEnablementController:getState`. - Configure it through the new optional `instanceOptions.assetsController`, which requires a `queryApiClient`. ### Changed -- Add `@metamask/assets-controller` as a dependency +- Add `@metamask/assets-controller` as a dependency ([#10885](https://github.com/MetaMask/core/pull/10885)) ## [18.0.0] From 3b6272aeb7f4e2c1a057e621726b89f197d31640 Mon Sep 17 00:00:00 2001 From: Xiaoming Wang Date: Wed, 28 Oct 2026 17:27:03 +0800 Subject: [PATCH 03/24] feat(assets-controller): read enabled chains after construction AssetsController can be constructed before NetworkEnablementController exists. init() and the stateChange subscription apply enabled chains once that state is available. Co-authored-by: Cursor --- packages/assets-controller/CHANGELOG.md | 1 + .../src/AssetsController.test.ts | 102 ++++++++++++++++++ .../assets-controller/src/AssetsController.ts | 48 ++++++++- packages/wallet/CHANGELOG.md | 2 +- .../assets-controller.test.ts | 45 ++++++++ .../assets-controller/assets-controller.ts | 10 +- 6 files changed, 198 insertions(+), 10 deletions(-) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 198ea6aa980..67fc8e36ae4 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Add `init()`, which reads enabled chains from `NetworkEnablementController:getState`. Construction no longer requires that action to be registered. Later enablement changes still arrive on `NetworkEnablementController:stateChange` ([#10885](https://github.com/MetaMask/core/pull/10885)) - Add `syncAssetsPrices(assetIds)`, exposed as the `AssetsController:syncAssetsPrices` messenger action. It force-fetches spot prices for an explicit CAIP-19 asset list through `PriceDataSource` and merges them into state ([#10892](https://github.com/MetaMask/core/pull/10892)) - Add support for Node.js 26 ([#10981](https://github.com/MetaMask/core/pull/10981)) diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index 4843c19f298..a0cd6c39dd7 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -746,6 +746,108 @@ describe('AssetsController', () => { ).not.toThrow(); }); + it('reads enabled chains from init and from NetworkEnablementController state changes when getState is not registered at construction', async () => { + const messenger: RootMessenger = new Messenger({ + namespace: MOCK_ANY_NAMESPACE, + }); + messenger.registerActionHandler( + 'AccountTreeController:getAccountsFromSelectedAccountGroup', + () => [], + ); + ( + messenger as { + registerActionHandler: (a: string, h: () => unknown) => void; + } + ).registerActionHandler('AccountsController:getSelectedAccount', () => + createMockInternalAccount(), + ); + ( + messenger as { + registerActionHandler: (a: string, h: () => unknown) => void; + } + ).registerActionHandler('NetworkController:getState', () => ({ + networkConfigurationsByChainId: {}, + networksMetadata: {}, + })); + ( + messenger as { + registerActionHandler: (a: string, h: () => unknown) => void; + } + ).registerActionHandler('NetworkController:getNetworkClientById', () => ({ + provider: {}, + })); + + const polygonNativeAssetId = 'eip155:137/slip44:966' as Caip19AssetId; + const nativeMetadata = { + type: 'native' as const, + symbol: 'ETH', + name: 'Ether', + decimals: 18, + }; + const controller = new AssetsController({ + messenger: messenger as unknown as AssetsControllerMessenger, + queryApiClient: createMockQueryApiClient(), + state: { + assetsBalance: { + [MOCK_ACCOUNT_ID]: { + [MOCK_NATIVE_ASSET_ID]: { amount: '1' }, + [polygonNativeAssetId]: { amount: '2' }, + }, + }, + assetsInfo: { + [MOCK_NATIVE_ASSET_ID]: nativeMetadata, + [polygonNativeAssetId]: { + ...nativeMetadata, + symbol: 'POL', + name: 'Polygon', + }, + }, + }, + }); + + const account = createMockInternalAccount(); + const beforeInit = await controller.getAssets([account]); + expect(Object.keys(beforeInit[MOCK_ACCOUNT_ID] ?? {})).toStrictEqual([]); + + messenger.registerActionHandler( + 'NetworkEnablementController:getState', + () => ({ + enabledNetworkMap: { + eip155: { '0x1': true }, + }, + nativeAssetIdentifiers: { + 'eip155:1': MOCK_NATIVE_ASSET_ID, + }, + }), + ); + await controller.init(); + + const afterInit = await controller.getAssets([account]); + expect(Object.keys(afterInit[MOCK_ACCOUNT_ID] ?? {})).toStrictEqual([ + MOCK_NATIVE_ASSET_ID, + ]); + + (messenger as unknown as { publish: LifecyclePublish }).publish( + 'NetworkEnablementController:stateChange', + { + enabledNetworkMap: { + eip155: { '0x1': true, '0x89': true }, + }, + nativeAssetIdentifiers: { + 'eip155:1': MOCK_NATIVE_ASSET_ID, + 'eip155:137': polygonNativeAssetId, + }, + }, + [], + ); + await new Promise(process.nextTick); + + const afterStateChange = await controller.getAssets([account]); + expect( + Object.keys(afterStateChange[MOCK_ACCOUNT_ID] ?? {}).sort(), + ).toStrictEqual([MOCK_NATIVE_ASSET_ID, polygonNativeAssetId].sort()); + }); + it('accepts isBasicFunctionality option and exposes handleBasicFunctionalityChange', async () => { await withController(async ({ controller }) => { expect(controller.handleBasicFunctionalityChange).toBeDefined(); diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index 7a046a342be..551038001ef 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -1061,8 +1061,10 @@ export class AssetsController extends BaseController< defaultUpdateInterval: this.#defaultUpdateInterval, }); - this.#initializeState(); + // Subscribe before the first read so a stateChange published while + // NetworkEnablementController is still starting is not missed. this.#subscribeToEvents(); + this.#initializeState(); this.#registerActionHandlers(); // Subscriptions start only when both UI is open and keyring unlocked -> #updateActive(). @@ -1110,10 +1112,36 @@ export class AssetsController extends BaseController< ); } + /** + * Read the current enabled chains from NetworkEnablementController. + * + * `Wallet` constructs additional controllers before default ones, so this + * controller can be constructed before `NetworkEnablementController:getState` + * is registered. `Wallet.init()` calls `init()` after that controller + * exists. Later enablement changes arrive on + * `NetworkEnablementController:stateChange`. + */ + async init(): Promise { + this.#initializeState(); + } + #initializeState(): void { - const { enabledNetworkMap } = this.messenger.call( - 'NetworkEnablementController:getState', - ); + let enabledNetworkMap: NetworkEnablementControllerState['enabledNetworkMap']; + try { + ({ enabledNetworkMap } = this.messenger.call( + 'NetworkEnablementController:getState', + )); + } catch (error) { + if (!isUnregisteredActionError(error)) { + throw error; + } + log( + 'NetworkEnablementController:getState is not registered yet. Enabled chains are applied from NetworkEnablementController:stateChange.', + { error }, + ); + return; + } + this.#enabledChains = this.#extractEnabledChains(enabledNetworkMap); log('Initialized state', { @@ -4481,3 +4509,15 @@ export class AssetsController extends BaseController< ); } } + +/** + * Whether a messenger call failed because no controller has registered the action yet. + * + * @param error - The error thrown by `messenger.call`. + * @returns True when the action handler is not registered. + */ +function isUnregisteredActionError(error: unknown): boolean { + return ( + error instanceof Error && error.message.includes('has not been registered') + ); +} diff --git a/packages/wallet/CHANGELOG.md b/packages/wallet/CHANGELOG.md index 6254a9c8e61..bea43075e89 100644 --- a/packages/wallet/CHANGELOG.md +++ b/packages/wallet/CHANGELOG.md @@ -13,7 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Delegates `MultichainNetworkController:getState`. `Wallet` does not construct `MultichainNetworkController`. Clients must register that action on the wallet messenger before calling `wallet.init()`. `NetworkEnablementController.init()` reads it during `Wallet.init()`, and `enableAllPopularNetworks` and `listPopularMultichainNetworks` read it later. - Also delegates `ConfigRegistryController:getState`, `NetworkController:getState`, `NetworkController:networkAdded`, `NetworkController:networkRemoved`, `NetworkController:stateChange`, and `TransactionController:transactionSubmitted`. `Wallet` constructs those controllers during default initialization. - Export an `assetsController` initialization configuration for `AssetsController`, along with the `AssetsControllerInstanceOptions` type ([#10885](https://github.com/MetaMask/core/pull/10885)) - - It is not part of the default initialization configurations, because the Wallet does not yet initialize several of its dependencies (e.g. `AccountTreeController`, `SnapController`). Pass it through `initializationConfigurations` after a configuration that registers `NetworkEnablementController:getState`. Additional configurations initialize before defaults, so the default `NetworkEnablementController` is not constructed yet. + - It is not part of the default initialization configurations, because the Wallet does not yet initialize several of its dependencies (e.g. `AccountTreeController`, `SnapController`). Pass it through `initializationConfigurations`. Those configurations are constructed before the default `NetworkEnablementController`. `AssetsController.init()` reads that controller's state during `wallet.init()`, and later enablement changes arrive on `NetworkEnablementController:stateChange`. - Configure it through the new optional `instanceOptions.assetsController`, which requires a `queryApiClient`. ### Changed diff --git a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts index 772a7cd4436..8c051a25bbe 100644 --- a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts +++ b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts @@ -222,6 +222,51 @@ describe('assetsController', () => { expect(listener).toHaveBeenCalledWith('payload'); }); + it('is constructed before the default NetworkEnablementController and reads its state during wallet init', async () => { + const accountId = 'account-1'; + const nativeAssetId = 'eip155:1/slip44:60'; + const wallet = new Wallet({ + state: { + AssetsController: { + assetsBalance: { + [accountId]: { + [nativeAssetId]: { amount: '1' }, + }, + }, + assetsInfo: { + [nativeAssetId]: { + type: 'native', + symbol: 'ETH', + name: 'Ether', + decimals: 18, + }, + }, + }, + }, + initializationConfigurations: [assetsController], + instanceOptions: { + ...getInstanceOptions(), + assetsController: { queryApiClient: createMockQueryApiClient() }, + }, + }); + wallets.push(wallet); + + const instance = wallet.getInstance( + 'AssetsController', + ) as unknown as AssetsController; + expect(instance).toBeInstanceOf(AssetsController); + + await instance.init(); + + // Default NetworkEnablementController state enables Ethereum mainnet. + // init() reads that state even though this controller was constructed first. + const assets = await instance.getAssets([ + { id: accountId } as Parameters[0][number], + ]); + + expect(Object.keys(assets[accountId] ?? {})).toStrictEqual([nativeAssetId]); + }); + it('is initialized by the Wallet when passed as an initialization configuration', async () => { const wallet = new Wallet({ initializationConfigurations: [ diff --git a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts index 2784dcfca31..8efc1f6ab1b 100644 --- a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts +++ b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts @@ -13,11 +13,11 @@ export type { AssetsControllerInstanceOptions } from './types.js'; * This is not part of the default initialization configurations because the * Wallet does not yet initialize several of the controllers it depends on * (e.g. `AccountTreeController`, `SnapController`). Clients pass it through - * `initializationConfigurations`, after the configurations that register those - * dependencies. `NetworkEnablementController:getState` must be registered - * before this configuration is initialized, as the controller reads it on - * construction. Additional configurations initialize before defaults, so the - * default `NetworkEnablementController` is not constructed yet. + * `initializationConfigurations`. Those configurations are constructed before + * the default `NetworkEnablementController`. `AssetsController` reads + * `NetworkEnablementController:getState` from `init()`, which `Wallet.init()` + * calls after every controller exists, and applies later enablement from + * `NetworkEnablementController:stateChange`. */ export const assetsController: InitializationConfiguration< AssetsController, From cd3925c8db1b31cd735f040894b6b06265376098 Mon Sep 17 00:00:00 2001 From: Xiaoming Wang Date: Wed, 28 Oct 2026 17:52:05 +0800 Subject: [PATCH 04/24] test(assets-controller): fix the enabled-chain fixture types The new test used a widened CAIP-19 id and an extra publish argument, which lint:tsc:check treats as unsuppressed errors. Co-authored-by: Cursor --- packages/assets-controller/src/AssetsController.test.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index a0cd6c39dd7..931e9a6fe41 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -816,7 +816,7 @@ describe('AssetsController', () => { eip155: { '0x1': true }, }, nativeAssetIdentifiers: { - 'eip155:1': MOCK_NATIVE_ASSET_ID, + 'eip155:1': 'eip155:1/slip44:60', }, }), ); @@ -834,11 +834,10 @@ describe('AssetsController', () => { eip155: { '0x1': true, '0x89': true }, }, nativeAssetIdentifiers: { - 'eip155:1': MOCK_NATIVE_ASSET_ID, - 'eip155:137': polygonNativeAssetId, + 'eip155:1': 'eip155:1/slip44:60', + 'eip155:137': 'eip155:137/slip44:966', }, }, - [], ); await new Promise(process.nextTick); From 5669a8ae022a697be00cee77a711584de90e0c44 Mon Sep 17 00:00:00 2001 From: Xiaoming Wang Date: Wed, 28 Oct 2026 18:02:46 +0800 Subject: [PATCH 05/24] test(assets-controller): satisfy oxlint in the enabled-chain tests The new assertions used a short cast parameter and nullish fallbacks, which oxlint reports as unsuppressed violations. Co-authored-by: Cursor --- .../src/AssetsController.test.ts | 25 +++++++++++++------ .../assets-controller.test.ts | 2 +- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index 931e9a6fe41..8cced76bc78 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -756,14 +756,20 @@ describe('AssetsController', () => { ); ( messenger as { - registerActionHandler: (a: string, h: () => unknown) => void; + registerActionHandler: ( + action: string, + handler: () => unknown, + ) => void; } ).registerActionHandler('AccountsController:getSelectedAccount', () => createMockInternalAccount(), ); ( messenger as { - registerActionHandler: (a: string, h: () => unknown) => void; + registerActionHandler: ( + action: string, + handler: () => unknown, + ) => void; } ).registerActionHandler('NetworkController:getState', () => ({ networkConfigurationsByChainId: {}, @@ -771,7 +777,10 @@ describe('AssetsController', () => { })); ( messenger as { - registerActionHandler: (a: string, h: () => unknown) => void; + registerActionHandler: ( + action: string, + handler: () => unknown, + ) => void; } ).registerActionHandler('NetworkController:getNetworkClientById', () => ({ provider: {}, @@ -807,7 +816,7 @@ describe('AssetsController', () => { const account = createMockInternalAccount(); const beforeInit = await controller.getAssets([account]); - expect(Object.keys(beforeInit[MOCK_ACCOUNT_ID] ?? {})).toStrictEqual([]); + expect(Object.keys(beforeInit[MOCK_ACCOUNT_ID])).toStrictEqual([]); messenger.registerActionHandler( 'NetworkEnablementController:getState', @@ -823,7 +832,7 @@ describe('AssetsController', () => { await controller.init(); const afterInit = await controller.getAssets([account]); - expect(Object.keys(afterInit[MOCK_ACCOUNT_ID] ?? {})).toStrictEqual([ + expect(Object.keys(afterInit[MOCK_ACCOUNT_ID])).toStrictEqual([ MOCK_NATIVE_ASSET_ID, ]); @@ -842,9 +851,9 @@ describe('AssetsController', () => { await new Promise(process.nextTick); const afterStateChange = await controller.getAssets([account]); - expect( - Object.keys(afterStateChange[MOCK_ACCOUNT_ID] ?? {}).sort(), - ).toStrictEqual([MOCK_NATIVE_ASSET_ID, polygonNativeAssetId].sort()); + expect(Object.keys(afterStateChange[MOCK_ACCOUNT_ID]).sort()).toStrictEqual( + [MOCK_NATIVE_ASSET_ID, polygonNativeAssetId].sort(), + ); }); it('accepts isBasicFunctionality option and exposes handleBasicFunctionalityChange', async () => { diff --git a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts index 8c051a25bbe..3b72572f6b8 100644 --- a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts +++ b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts @@ -264,7 +264,7 @@ describe('assetsController', () => { { id: accountId } as Parameters[0][number], ]); - expect(Object.keys(assets[accountId] ?? {})).toStrictEqual([nativeAssetId]); + expect(Object.keys(assets[accountId])).toStrictEqual([nativeAssetId]); }); it('is initialized by the Wallet when passed as an initialization configuration', async () => { From 91ce1929fb7616e2808beb32ca13c57474b8ceaf Mon Sep 17 00:00:00 2001 From: Xiaoming Wang Date: Wed, 28 Oct 2026 18:19:19 +0800 Subject: [PATCH 06/24] fix(assets-controller): keep the init note under Unreleased Changelog checks only accept the open pull request under Unreleased, and oxfmt needs the enabled-chain assertion wrapped. Co-authored-by: Cursor --- packages/assets-controller/CHANGELOG.md | 5 ++++- packages/assets-controller/src/AssetsController.test.ts | 6 +++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 67fc8e36ae4..e115512a58d 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -7,11 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `init()`, which reads enabled chains from `NetworkEnablementController:getState`. Construction no longer requires that action to be registered. Later enablement changes still arrive on `NetworkEnablementController:stateChange` ([#10885](https://github.com/MetaMask/core/pull/10885)) + ## [20.0.0] ### Added -- Add `init()`, which reads enabled chains from `NetworkEnablementController:getState`. Construction no longer requires that action to be registered. Later enablement changes still arrive on `NetworkEnablementController:stateChange` ([#10885](https://github.com/MetaMask/core/pull/10885)) - Add `syncAssetsPrices(assetIds)`, exposed as the `AssetsController:syncAssetsPrices` messenger action. It force-fetches spot prices for an explicit CAIP-19 asset list through `PriceDataSource` and merges them into state ([#10892](https://github.com/MetaMask/core/pull/10892)) - Add support for Node.js 26 ([#10981](https://github.com/MetaMask/core/pull/10981)) diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index 8cced76bc78..a887f12c951 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -851,9 +851,9 @@ describe('AssetsController', () => { await new Promise(process.nextTick); const afterStateChange = await controller.getAssets([account]); - expect(Object.keys(afterStateChange[MOCK_ACCOUNT_ID]).sort()).toStrictEqual( - [MOCK_NATIVE_ASSET_ID, polygonNativeAssetId].sort(), - ); + expect( + Object.keys(afterStateChange[MOCK_ACCOUNT_ID]).sort(), + ).toStrictEqual([MOCK_NATIVE_ASSET_ID, polygonNativeAssetId].sort()); }); it('accepts isBasicFunctionality option and exposes handleBasicFunctionalityChange', async () => { From 4fa64d59406bd962e600b67bfaf416257e3adff7 Mon Sep 17 00:00:00 2001 From: Xiaoming Wang Date: Thu, 29 Oct 2026 14:04:41 +0800 Subject: [PATCH 07/24] fix(wallet): require @metamask/assets-controller ^20.0.2 Co-authored-by: Cursor --- packages/wallet/package.json | 2 +- yarn.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/wallet/package.json b/packages/wallet/package.json index 64f54132d2f..8e6615dc6fc 100644 --- a/packages/wallet/package.json +++ b/packages/wallet/package.json @@ -54,7 +54,7 @@ "@metamask/address-book-controller": "^8.0.0", "@metamask/analytics-controller": "^4.1.0", "@metamask/approval-controller": "^10.0.0", - "@metamask/assets-controller": "^20.0.0", + "@metamask/assets-controller": "^20.0.2", "@metamask/base-controller": "^10.1.0", "@metamask/browser-passworder": "^6.0.0", "@metamask/claims-controller": "^1.1.0", diff --git a/yarn.lock b/yarn.lock index 78b591eacf7..57a304c0e96 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5485,7 +5485,7 @@ __metadata: languageName: node linkType: hard -"@metamask/assets-controller@npm:^20.0.0, @metamask/assets-controller@npm:^20.0.2, @metamask/assets-controller@workspace:packages/assets-controller": +"@metamask/assets-controller@npm:^20.0.2, @metamask/assets-controller@workspace:packages/assets-controller": version: 0.0.0-use.local resolution: "@metamask/assets-controller@workspace:packages/assets-controller" dependencies: @@ -9155,7 +9155,7 @@ __metadata: "@metamask/address-book-controller": "npm:^8.0.0" "@metamask/analytics-controller": "npm:^4.1.0" "@metamask/approval-controller": "npm:^10.0.0" - "@metamask/assets-controller": "npm:^20.0.0" + "@metamask/assets-controller": "npm:^20.0.2" "@metamask/auto-changelog": "npm:^6.2.1" "@metamask/base-controller": "npm:^10.1.0" "@metamask/browser-passworder": "npm:^6.0.0" From 40609032184619f0252dbf471ff28613fa64e161 Mon Sep 17 00:00:00 2001 From: Xiaoming Wang Date: Thu, 29 Oct 2026 17:56:34 +0800 Subject: [PATCH 08/24] fix(assets-controller): refresh data source active chains in init() RpcDataSource and StakedBalanceDataSource read NetworkController and NetworkEnablementController state only at construction. When the Wallet constructs AssetsController before those controllers, both reads fail and active chains stay empty until a later stateChange event. AssetsController.init() now also refreshes both data sources' active chains. Adds StakedBalanceDataSource.refreshActiveChainsFromNetworkEnablementState() to match the existing RpcDataSource.refreshActiveChainsFromNetworkState(). Co-Authored-By: Claude Opus 5.5 --- packages/assets-controller/CHANGELOG.md | 1 + .../src/AssetsController.test.ts | 27 +++++++++++++++++++ .../assets-controller/src/AssetsController.ts | 12 ++++++--- .../StakedBalanceDataSource.test.ts | 25 +++++++++++++++++ .../data-sources/StakedBalanceDataSource.ts | 9 +++++++ 5 files changed, 70 insertions(+), 4 deletions(-) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 98b1f900e4d..23ea6ea1032 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add `init()`, which reads enabled chains from `NetworkEnablementController:getState`. Construction no longer requires that action to be registered. Later enablement changes still arrive on `NetworkEnablementController:stateChange` ([#10885](https://github.com/MetaMask/core/pull/10885)) + - `init()` also refreshes the RPC and staked balance data sources' active chains from `NetworkController:getState` and `NetworkEnablementController:getState`, which they otherwise read only at construction. ### Changed diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index a887f12c951..576a9d5f065 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -29,6 +29,7 @@ import { AccountsApiDataSource } from './data-sources/AccountsApiDataSource.js'; import type { PriceDataSourceConfig } from './data-sources/PriceDataSource.js'; import { PriceDataSource } from './data-sources/PriceDataSource.js'; import { RpcDataSource } from './data-sources/RpcDataSource.js'; +import { StakedBalanceDataSource } from './data-sources/StakedBalanceDataSource.js'; import { TokenDataSource } from './data-sources/TokenDataSource.js'; import { buildDefaultAssetsInfo, @@ -856,6 +857,32 @@ describe('AssetsController', () => { ).toStrictEqual([MOCK_NATIVE_ASSET_ID, polygonNativeAssetId].sort()); }); + it('refreshes data source active chains from init', async () => { + const refreshRpcSpy = vi.spyOn( + RpcDataSource.prototype, + 'refreshActiveChainsFromNetworkState', + ); + const refreshStakedSpy = vi.spyOn( + StakedBalanceDataSource.prototype, + 'refreshActiveChainsFromNetworkEnablementState', + ); + + try { + await withController(async ({ controller }) => { + refreshRpcSpy.mockClear(); + refreshStakedSpy.mockClear(); + + await controller.init(); + + expect(refreshRpcSpy).toHaveBeenCalledTimes(1); + expect(refreshStakedSpy).toHaveBeenCalledTimes(1); + }); + } finally { + refreshRpcSpy.mockRestore(); + refreshStakedSpy.mockRestore(); + } + }); + it('accepts isBasicFunctionality option and exposes handleBasicFunctionalityChange', async () => { await withController(async ({ controller }) => { expect(controller.handleBasicFunctionalityChange).toBeDefined(); diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index 551038001ef..1499f2a1320 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -1113,16 +1113,20 @@ export class AssetsController extends BaseController< } /** - * Read the current enabled chains from NetworkEnablementController. + * Read the current enabled chains from NetworkEnablementController, and + * refresh the active chains of data sources that read + * NetworkEnablementController or NetworkController state at construction. * * `Wallet` constructs additional controllers before default ones, so this * controller can be constructed before `NetworkEnablementController:getState` - * is registered. `Wallet.init()` calls `init()` after that controller - * exists. Later enablement changes arrive on - * `NetworkEnablementController:stateChange`. + * and `NetworkController:getState` are registered. `Wallet.init()` calls + * `init()` after those controllers exist. Later changes arrive on their + * `stateChange` events. */ async init(): Promise { this.#initializeState(); + this.#rpcDataSource.refreshActiveChainsFromNetworkState(); + this.#stakedBalanceDataSource.refreshActiveChainsFromNetworkEnablementState(); } #initializeState(): void { diff --git a/packages/assets-controller/src/data-sources/StakedBalanceDataSource.test.ts b/packages/assets-controller/src/data-sources/StakedBalanceDataSource.test.ts index 6d17b5fee5c..f8e502ae1d1 100644 --- a/packages/assets-controller/src/data-sources/StakedBalanceDataSource.test.ts +++ b/packages/assets-controller/src/data-sources/StakedBalanceDataSource.test.ts @@ -219,6 +219,31 @@ describe('StakedBalanceDataSource', () => { }); }); + describe('refreshActiveChainsFromNetworkEnablementState', () => { + it('re-reads NetworkEnablementController state and updates active chains', async () => { + await withController( + { enabledNetworkMap: {} }, + async ({ controller, mockMessengerCall, onActiveChainsUpdated }) => { + expect(await controller.getActiveChains()).toStrictEqual([]); + + mockMessengerCall.mockReturnValueOnce({ + enabledNetworkMap: { eip155: { [MAINNET_CHAIN_ID_HEX]: true } }, + }); + controller.refreshActiveChainsFromNetworkEnablementState(); + + expect(await controller.getActiveChains()).toStrictEqual([ + MAINNET_CHAIN_ID_CAIP, + ]); + expect(onActiveChainsUpdated).toHaveBeenLastCalledWith( + 'StakedBalanceDataSource', + [MAINNET_CHAIN_ID_CAIP], + [], + ); + }, + ); + }); + }); + describe('getName', () => { it('returns the data source name', async () => { await withController(({ controller }) => { diff --git a/packages/assets-controller/src/data-sources/StakedBalanceDataSource.ts b/packages/assets-controller/src/data-sources/StakedBalanceDataSource.ts index 65d5a0c1528..84511b98a16 100644 --- a/packages/assets-controller/src/data-sources/StakedBalanceDataSource.ts +++ b/packages/assets-controller/src/data-sources/StakedBalanceDataSource.ts @@ -219,6 +219,15 @@ export class StakedBalanceDataSource extends AbstractDataSource< this.#initializeActiveChains(); } + /** + * Re-read NetworkEnablementController state and refresh staking + * `activeChains` (e.g. when that controller was not registered yet at + * construction). + */ + refreshActiveChainsFromNetworkEnablementState(): void { + this.#initializeActiveChains(); + } + /** * When NetworkController state changes (e.g. RPC endpoints or network clients * reconfigured), clear the provider cache so subsequent fetches use fresh From a8d1c7b3e1aded3970f62cc4522df927b564e915 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 29 Oct 2026 13:13:36 +0000 Subject: [PATCH 09/24] fix(assets-controller): read enabled chains in init instead of the constructor Keep NetworkEnablementController:getState out of AssetsController construction so Wallet can finish constructing every controller before that read. Co-authored-by: Xiaoming Wang --- packages/assets-controller/CHANGELOG.md | 3 ++- ...n-filtering.accounts-api-v6.integration.test.ts | 1 + ...er.bsc-spam-token-filtering.integration.test.ts | 1 + ...setsController.spam-cleanup.integration.test.ts | 1 + .../src/AssetsController.spam-cleanup.test.ts | 1 + ...ler.stale-balance-scenarios.integration.test.ts | 1 + .../assets-controller/src/AssetsController.test.ts | 5 +++++ packages/assets-controller/src/AssetsController.ts | 14 ++++++-------- ...Controller.ws-price-updates.integration.test.ts | 1 + 9 files changed, 19 insertions(+), 9 deletions(-) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 23ea6ea1032..052e9a55f70 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -9,11 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Add `init()`, which reads enabled chains from `NetworkEnablementController:getState`. Construction no longer requires that action to be registered. Later enablement changes still arrive on `NetworkEnablementController:stateChange` ([#10885](https://github.com/MetaMask/core/pull/10885)) +- Add `init()`, which reads enabled chains from `NetworkEnablementController:getState` after every controller has been constructed. Later enablement changes still arrive on `NetworkEnablementController:stateChange` ([#10885](https://github.com/MetaMask/core/pull/10885)) - `init()` also refreshes the RPC and staked balance data sources' active chains from `NetworkController:getState` and `NetworkEnablementController:getState`, which they otherwise read only at construction. ### Changed +- **BREAKING:** The constructor no longer calls `NetworkEnablementController:getState`. Call `init()` after the other controllers exist. `Wallet.init()` does this. Until `init()` runs, enabled chains stay empty unless a `NetworkEnablementController:stateChange` event arrives ([#10885](https://github.com/MetaMask/core/pull/10885)) - Use hand-rolled Multicall3 `aggregate3` codecs for balance batches, while retaining the Ethers codecs for byte-identical comparison ([#11078](https://github.com/MetaMask/core/pull/11078)) ## [20.0.1] diff --git a/packages/assets-controller/src/AssetsController.bsc-spam-token-filtering.accounts-api-v6.integration.test.ts b/packages/assets-controller/src/AssetsController.bsc-spam-token-filtering.accounts-api-v6.integration.test.ts index fffe15e853a..2c5c39b3416 100644 --- a/packages/assets-controller/src/AssetsController.bsc-spam-token-filtering.accounts-api-v6.integration.test.ts +++ b/packages/assets-controller/src/AssetsController.bsc-spam-token-filtering.accounts-api-v6.integration.test.ts @@ -82,6 +82,7 @@ async function withController( queryApiClient, isBasicFunctionality: (): boolean => true, }); + await controller.init(); try { return await fn({ controller, messenger: rootMessenger }); diff --git a/packages/assets-controller/src/AssetsController.bsc-spam-token-filtering.integration.test.ts b/packages/assets-controller/src/AssetsController.bsc-spam-token-filtering.integration.test.ts index a22e8d32317..8067992b755 100644 --- a/packages/assets-controller/src/AssetsController.bsc-spam-token-filtering.integration.test.ts +++ b/packages/assets-controller/src/AssetsController.bsc-spam-token-filtering.integration.test.ts @@ -70,6 +70,7 @@ async function withController( queryApiClient, isBasicFunctionality: (): boolean => true, }); + await controller.init(); try { return await fn({ controller, messenger: rootMessenger }); diff --git a/packages/assets-controller/src/AssetsController.spam-cleanup.integration.test.ts b/packages/assets-controller/src/AssetsController.spam-cleanup.integration.test.ts index b20f349ee67..7d24ff61182 100644 --- a/packages/assets-controller/src/AssetsController.spam-cleanup.integration.test.ts +++ b/packages/assets-controller/src/AssetsController.spam-cleanup.integration.test.ts @@ -102,6 +102,7 @@ async function withController( isBasicFunctionality, captureException, }); + await controller.init(); try { return await fn({ controller, messenger: rootMessenger }); diff --git a/packages/assets-controller/src/AssetsController.spam-cleanup.test.ts b/packages/assets-controller/src/AssetsController.spam-cleanup.test.ts index 066369674f4..6e64a006373 100644 --- a/packages/assets-controller/src/AssetsController.spam-cleanup.test.ts +++ b/packages/assets-controller/src/AssetsController.spam-cleanup.test.ts @@ -120,6 +120,7 @@ async function withController( isBasicFunctionality, captureException, }); + await controller.init(); try { return await fn({ controller, messenger: rootMessenger }); diff --git a/packages/assets-controller/src/AssetsController.stale-balance-scenarios.integration.test.ts b/packages/assets-controller/src/AssetsController.stale-balance-scenarios.integration.test.ts index c7c9425ed77..63e03243524 100644 --- a/packages/assets-controller/src/AssetsController.stale-balance-scenarios.integration.test.ts +++ b/packages/assets-controller/src/AssetsController.stale-balance-scenarios.integration.test.ts @@ -142,6 +142,7 @@ async function withStaleBalanceController( queryApiClient, isBasicFunctionality: (): boolean => true, }); + await controller.init(); try { // `AccountsApiDataSource` loads `/v2/supportedNetworks` asynchronously diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index 576a9d5f065..5269881bc8d 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -441,6 +441,7 @@ async function withController( ...incomingPriceDataSourceConfig, }, }); + await controller.init(); try { return await fn({ controller, messenger, getSelectedAccountsMock }); @@ -816,6 +817,7 @@ describe('AssetsController', () => { }); const account = createMockInternalAccount(); + await controller.init(); const beforeInit = await controller.getAssets([account]); expect(Object.keys(beforeInit[MOCK_ACCOUNT_ID])).toStrictEqual([]); @@ -5635,6 +5637,7 @@ describe('AssetsController', () => { /* no-op */ }, }); + await controller.init(); const getAssetsSpy = vi.spyOn(controller, 'getAssets'); @@ -5774,6 +5777,7 @@ describe('AssetsController', () => { /* no-op */ }, }); + await controller.init(); let nativePresentAtPriceFetch: boolean | undefined; const fetchSpy = vi @@ -6291,6 +6295,7 @@ describe('AssetsController', () => { /* no-op */ }, }); + await controller.init(); const getAssetsSpy = vi.spyOn(controller, 'getAssets'); diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index 1499f2a1320..02fb3e4ddd7 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -1061,10 +1061,10 @@ export class AssetsController extends BaseController< defaultUpdateInterval: this.#defaultUpdateInterval, }); - // Subscribe before the first read so a stateChange published while - // NetworkEnablementController is still starting is not missed. + // Subscribe during construction so a stateChange published before init() + // is not missed. Enabled chains are read in init(), after every + // controller has been constructed. this.#subscribeToEvents(); - this.#initializeState(); this.#registerActionHandlers(); // Subscriptions start only when both UI is open and keyring unlocked -> #updateActive(). @@ -1117,11 +1117,9 @@ export class AssetsController extends BaseController< * refresh the active chains of data sources that read * NetworkEnablementController or NetworkController state at construction. * - * `Wallet` constructs additional controllers before default ones, so this - * controller can be constructed before `NetworkEnablementController:getState` - * and `NetworkController:getState` are registered. `Wallet.init()` calls - * `init()` after those controllers exist. Later changes arrive on their - * `stateChange` events. + * This controller does not call those actions in its constructor. `Wallet` + * constructs every controller first, then `Wallet.init()` calls `init()`. + * Later changes arrive on `stateChange` events. */ async init(): Promise { this.#initializeState(); diff --git a/packages/assets-controller/src/AssetsController.ws-price-updates.integration.test.ts b/packages/assets-controller/src/AssetsController.ws-price-updates.integration.test.ts index ebbaf01d5ca..cb3da104a05 100644 --- a/packages/assets-controller/src/AssetsController.ws-price-updates.integration.test.ts +++ b/packages/assets-controller/src/AssetsController.ws-price-updates.integration.test.ts @@ -113,6 +113,7 @@ async function withController( queryApiClient, isBasicFunctionality: (): boolean => true, }); + await controller.init(); try { return await fn({ controller, messenger: rootMessenger }); From 3a7ec4aa9ffb581b645d141a394efa87b6bdf8c5 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 2 Nov 2026 13:37:54 +0000 Subject: [PATCH 10/24] feat(wallet): initialize AssetsController by default Construct AssetsController with the other Wallet defaults and require instanceOptions.assetsController.queryApiClient. Account-group reads fall back to AccountsController, and snap discovery stays empty, until AccountTreeController and SnapController are registered. Move the remaining constructor messenger reads into init paths that AssetsController.init() runs after every controller exists. Co-authored-by: Xiaoming Wang --- packages/assets-controller/CHANGELOG.md | 12 +- .../src/AssetsController.test.ts | 198 ++++++++++++++++++ .../assets-controller/src/AssetsController.ts | 46 ++-- .../AccountActivityDataSource.test.ts | 68 +++++- .../data-sources/AccountActivityDataSource.ts | 28 ++- .../AccountsApiDataSource.test.ts | 48 ++++- .../src/data-sources/AccountsApiDataSource.ts | 15 +- .../src/data-sources/RpcDataSource.test.ts | 28 +++ .../src/data-sources/RpcDataSource.ts | 8 +- .../src/data-sources/SnapDataSource.test.ts | 26 +++ .../src/data-sources/SnapDataSource.ts | 11 +- .../StakedBalanceDataSource.test.ts | 25 +++ .../data-sources/StakedBalanceDataSource.ts | 11 +- packages/wallet/CHANGELOG.md | 9 +- packages/wallet/src/Wallet.test.ts | 16 +- .../assets-controller.test.ts | 93 +++----- .../assets-controller/assets-controller.ts | 27 +-- .../gas-fee-controller.test.ts | 14 ++ .../src/initialization/instances/index.ts | 1 + .../transaction-controller.test.ts | 14 ++ packages/wallet/src/types.ts | 2 +- 21 files changed, 579 insertions(+), 121 deletions(-) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 85dda576f6b..8f64a1627c4 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -10,11 +10,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add `init()`, which reads enabled chains from `NetworkEnablementController:getState` after every controller has been constructed. Later enablement changes still arrive on `NetworkEnablementController:stateChange` ([#10885](https://github.com/MetaMask/core/pull/10885)) - - `init()` also refreshes the RPC and staked balance data sources' active chains from `NetworkController:getState` and `NetworkEnablementController:getState`, which they otherwise read only at construction. + - `init()` refreshes data sources that read other controllers through the messenger: `RpcDataSource.refreshActiveChainsFromNetworkState()` (`NetworkController:getState`), `StakedBalanceDataSource.refreshActiveChainsFromNetworkEnablementState()` (`NetworkEnablementController:getState`), `SnapDataSource.init()` (snap discovery), and `AccountsApiDataSource.init()` (supported networks and `RemoteFeatureFlagController:getState`). +- Add `SnapDataSource.init()` and `AccountsApiDataSource.init()` ([#10885](https://github.com/MetaMask/core/pull/10885)) ### Changed -- **BREAKING:** The constructor no longer calls `NetworkEnablementController:getState`. Call `init()` after the other controllers exist. `Wallet.init()` does this. Until `init()` runs, enabled chains stay empty unless a `NetworkEnablementController:stateChange` event arrives ([#10885](https://github.com/MetaMask/core/pull/10885)) +- **BREAKING:** Constructors no longer call messenger actions. Call `AssetsController.init()` after the other controllers exist. `Wallet.init()` does this. Until `init()` runs, enabled chains and data-source active chains stay empty unless a later state change arrives ([#10885](https://github.com/MetaMask/core/pull/10885)) + - `AssetsController` no longer calls `NetworkEnablementController:getState` in its constructor. + - `RpcDataSource` no longer calls `NetworkController:getState` in its constructor. Call `refreshActiveChainsFromNetworkState()`, or `AssetsController.init()`. + - `StakedBalanceDataSource` no longer calls `NetworkEnablementController:getState` in its constructor. Call `refreshActiveChainsFromNetworkEnablementState()`, or `AssetsController.init()`. + - `SnapDataSource` no longer calls `SnapController:getRunnableSnaps` in its constructor. Call `init()`. When `SnapController` is not registered, discovery leaves active chains empty. + - `AccountsApiDataSource` no longer starts its supported-network fetch, which calls `RemoteFeatureFlagController:getState`, from the constructor. Call `init()`. +- When `AccountTreeController` actions are not registered, selected-account reads fall back to `AccountsController:getSelectedAccount`, and asset tracking does not wait for account-tree initialization ([#10885](https://github.com/MetaMask/core/pull/10885)) + - `AccountActivityDataSource` uses the same fallback when resolving the account for a balance update. ## [20.1.0] diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index 5269881bc8d..4fccba39efa 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -29,6 +29,7 @@ import { AccountsApiDataSource } from './data-sources/AccountsApiDataSource.js'; import type { PriceDataSourceConfig } from './data-sources/PriceDataSource.js'; import { PriceDataSource } from './data-sources/PriceDataSource.js'; import { RpcDataSource } from './data-sources/RpcDataSource.js'; +import { SnapDataSource } from './data-sources/SnapDataSource.js'; import { StakedBalanceDataSource } from './data-sources/StakedBalanceDataSource.js'; import { TokenDataSource } from './data-sources/TokenDataSource.js'; import { @@ -868,23 +869,220 @@ describe('AssetsController', () => { StakedBalanceDataSource.prototype, 'refreshActiveChainsFromNetworkEnablementState', ); + const snapInitSpy = vi.spyOn(SnapDataSource.prototype, 'init'); + const accountsApiInitSpy = vi.spyOn( + AccountsApiDataSource.prototype, + 'init', + ); try { await withController(async ({ controller }) => { refreshRpcSpy.mockClear(); refreshStakedSpy.mockClear(); + snapInitSpy.mockClear(); + accountsApiInitSpy.mockClear(); await controller.init(); expect(refreshRpcSpy).toHaveBeenCalledTimes(1); expect(refreshStakedSpy).toHaveBeenCalledTimes(1); + expect(snapInitSpy).toHaveBeenCalledTimes(1); + expect(accountsApiInitSpy).toHaveBeenCalledTimes(1); }); } finally { refreshRpcSpy.mockRestore(); refreshStakedSpy.mockRestore(); + snapInitSpy.mockRestore(); + accountsApiInitSpy.mockRestore(); } }); + it('does not call messenger actions in the constructor', async () => { + const messenger: RootMessenger = new Messenger({ + namespace: MOCK_ANY_NAMESPACE, + }); + messenger.registerActionHandler( + 'NetworkEnablementController:getState', + () => ({ + enabledNetworkMap: { eip155: { '0x1': true } }, + nativeAssetIdentifiers: {}, + }), + ); + ( + messenger as { + registerActionHandler: ( + action: string, + handler: () => unknown, + ) => void; + } + ).registerActionHandler('NetworkController:getState', () => ({ + networkConfigurationsByChainId: {}, + networksMetadata: {}, + })); + ( + messenger as { + registerActionHandler: ( + action: string, + handler: () => unknown, + ) => void; + } + ).registerActionHandler('RemoteFeatureFlagController:getState', () => ({ + remoteFeatureFlags: {}, + cacheTimestamp: 0, + })); + + const callSpy = vi.spyOn(messenger, 'call'); + const controller = new AssetsController({ + messenger: messenger as unknown as AssetsControllerMessenger, + queryApiClient: createMockQueryApiClient(), + }); + + expect(callSpy).not.toHaveBeenCalled(); + + await controller.init(); + + expect(callSpy).toHaveBeenCalledWith( + 'NetworkEnablementController:getState', + ); + expect(callSpy).toHaveBeenCalledWith('NetworkController:getState'); + + controller.destroy(); + }); + + it('tracks the selected account when AccountTreeController and SnapController are absent', async () => { + const messenger: RootMessenger = new Messenger({ + namespace: MOCK_ANY_NAMESPACE, + }); + const account = createMockInternalAccount(); + registerKeyringUnlockMock(messenger); + ( + messenger as { + registerActionHandler: ( + action: string, + handler: () => unknown, + ) => void; + } + ).registerActionHandler( + 'AccountsController:getSelectedAccount', + () => account, + ); + messenger.registerActionHandler( + 'NetworkEnablementController:getState', + () => ({ + enabledNetworkMap: { eip155: { '0x1': true } }, + nativeAssetIdentifiers: { + 'eip155:1': 'eip155:1/slip44:60', + }, + }), + ); + ( + messenger as { + registerActionHandler: ( + action: string, + handler: () => unknown, + ) => void; + } + ).registerActionHandler('NetworkController:getState', () => ({ + networkConfigurationsByChainId: {}, + networksMetadata: {}, + })); + ( + messenger as { + registerActionHandler: ( + action: string, + handler: () => unknown, + ) => void; + } + ).registerActionHandler('ClientController:getState', () => ({ + isUiOpen: true, + })); + ( + messenger as { + registerActionHandler: ( + action: string, + handler: () => unknown, + ) => void; + } + ).registerActionHandler('RemoteFeatureFlagController:getState', () => ({ + remoteFeatureFlags: {}, + cacheTimestamp: 0, + })); + + const controller = new AssetsController({ + messenger: messenger as unknown as AssetsControllerMessenger, + queryApiClient: createMockQueryApiClient(), + }); + await controller.init(); + + const getAssetsSpy = vi.spyOn(controller, 'getAssets'); + messenger.publish('KeyringController:unlock'); + await flushPromises(); + + expect(getAssetsSpy).toHaveBeenCalledWith( + [expect.objectContaining({ id: account.id })], + expect.objectContaining({ forceUpdate: true }), + ); + + controller.destroy(); + }); + + it('rethrows AccountTreeController failures that are not a missing handler', async () => { + await withController(async ({ controller, messenger }) => { + messenger.unregisterActionHandler( + 'AccountTreeController:getAccountsFromSelectedAccountGroup', + ); + messenger.registerActionHandler( + 'AccountTreeController:getAccountsFromSelectedAccountGroup', + () => { + throw new Error('account tree failed'); + }, + ); + + expect(() => controller.getStateForTransactionPay()).toThrow( + 'account tree failed', + ); + }); + }); + + it('rethrows AccountTreeController:isInitialized failures that are not a missing handler', async () => { + await withController( + { clientControllerState: { isUiOpen: true } }, + async ({ messenger }) => { + messenger.publish('KeyringController:unlock'); + await flushPromises(); + + messenger.unregisterActionHandler( + 'AccountTreeController:isInitialized', + ); + messenger.registerActionHandler( + 'AccountTreeController:isInitialized', + () => { + throw new Error('account tree init failed'); + }, + ); + + const consoleError = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined); + try { + (messenger as unknown as { publish: LifecyclePublish }).publish( + 'ClientController:stateChange', + { isUiOpen: true }, + [], + ); + + expect(consoleError).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'account tree init failed', + }), + ); + } finally { + consoleError.mockRestore(); + } + }, + ); + }); + it('accepts isBasicFunctionality option and exposes handleBasicFunctionalityChange', async () => { await withController(async ({ controller }) => { expect(controller.handleBasicFunctionalityChange).toBeDefined(); diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index 02fb3e4ddd7..ec2732a1766 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -815,15 +815,25 @@ export class AssetsController extends BaseController< * This includes all accounts in the same group as the selected account * (EVM, Bitcoin, Solana, Tron, etc. that belong to the same logical account group). * + * When `AccountTreeController` is not registered, falls back to + * `AccountsController:getSelectedAccount`. + * * @returns Array of InternalAccount objects from the selected account group. */ #getSelectedAccounts(): InternalAccount[] { - const accounts = this.messenger.call( - 'AccountTreeController:getAccountsFromSelectedAccountGroup', - ); - if (accounts.length > 0) { - return accounts; + try { + const accounts = this.messenger.call( + 'AccountTreeController:getAccountsFromSelectedAccountGroup', + ); + if (accounts.length > 0) { + return accounts; + } + } catch (error) { + if (!isUnregisteredActionError(error)) { + throw error; + } } + const selectedAccount = this.messenger.call( 'AccountsController:getSelectedAccount', ); @@ -836,10 +846,20 @@ export class AssetsController extends BaseController< /** * Whether `AccountTreeController.init()` has completed and the tree is ready. * - * @returns True when the account tree has been initialized. + * When `AccountTreeController` is not registered, returns true so tracking + * is not blocked on a controller the host has not installed. + * + * @returns True when the account tree has been initialized, or when that controller is absent. */ #isAccountTreeInitialized(): boolean { - return this.messenger.call('AccountTreeController:isInitialized'); + try { + return this.messenger.call('AccountTreeController:isInitialized'); + } catch (error) { + if (!isUnregisteredActionError(error)) { + throw error; + } + return true; + } } /** @@ -1114,17 +1134,19 @@ export class AssetsController extends BaseController< /** * Read the current enabled chains from NetworkEnablementController, and - * refresh the active chains of data sources that read - * NetworkEnablementController or NetworkController state at construction. + * refresh data sources that would otherwise read other controllers through + * the messenger. * - * This controller does not call those actions in its constructor. `Wallet` - * constructs every controller first, then `Wallet.init()` calls `init()`. - * Later changes arrive on `stateChange` events. + * This controller does not call messenger actions in its constructor. + * `Wallet` constructs every controller first, then `Wallet.init()` calls + * `init()`. Later enablement changes arrive on `stateChange` events. */ async init(): Promise { this.#initializeState(); this.#rpcDataSource.refreshActiveChainsFromNetworkState(); this.#stakedBalanceDataSource.refreshActiveChainsFromNetworkEnablementState(); + this.#snapDataSource.init(); + await this.#accountsApiDataSource.init(); } #initializeState(): void { diff --git a/packages/assets-controller/src/data-sources/AccountActivityDataSource.test.ts b/packages/assets-controller/src/data-sources/AccountActivityDataSource.test.ts index 12559520fd1..fac5e97358e 100644 --- a/packages/assets-controller/src/data-sources/AccountActivityDataSource.test.ts +++ b/packages/assets-controller/src/data-sources/AccountActivityDataSource.test.ts @@ -96,6 +96,7 @@ function createBalanceUpdate(overrides?: { type SetupOptions = { groupAccounts?: InternalAccount[]; selectedAccount?: InternalAccount | null; + registerAccountTree?: boolean; onAssetsUpdate?: Mock; onActiveChainsUpdated?: Mock; state?: { activeChains?: ChainId[] }; @@ -129,6 +130,7 @@ function setup(options: SetupOptions = {}): SetupResult { const { groupAccounts = [createMockAccount()], selectedAccount = null, + registerAccountTree = true, onAssetsUpdate = vi.fn().mockResolvedValue(undefined), onActiveChainsUpdated = vi.fn(), state, @@ -155,10 +157,12 @@ function setup(options: SetupOptions = {}): SetupResult { ], }); - rootMessenger.registerActionHandler( - 'AccountTreeController:getAccountsFromSelectedAccountGroup', - () => groupAccounts, - ); + if (registerAccountTree) { + rootMessenger.registerActionHandler( + 'AccountTreeController:getAccountsFromSelectedAccountGroup', + () => groupAccounts, + ); + } rootMessenger.registerActionHandler( 'AccountsController:getSelectedAccount', () => selectedAccount as InternalAccount, @@ -300,6 +304,62 @@ describe('AccountActivityDataSource', () => { cleanup(); }); + it('falls back to the selected account when AccountTreeController is not registered', async () => { + const account = createMockAccount(); + const { onAssetsUpdate, triggerBalanceUpdated, cleanup } = setup({ + registerAccountTree: false, + selectedAccount: account, + }); + + triggerBalanceUpdated({ + address: EVM_ADDRESS, + chain: CHAIN_MAINNET, + updates: [createBalanceUpdate()], + }); + + await waitFor(() => { + expect(onAssetsUpdate).toHaveBeenCalledTimes(1); + }); + expect(onAssetsUpdate.mock.calls[0][1]).toStrictEqual({ + accountsWithSupportedChains: [ + { account, supportedChains: [CHAIN_MAINNET] }, + ], + chainIds: [CHAIN_MAINNET], + dataTypes: ['balance', 'metadata', 'price'], + }); + + cleanup(); + }); + + it('drops the balance update when AccountTreeController throws', async () => { + const account = createMockAccount(); + const { onAssetsUpdate, triggerBalanceUpdated, rootMessenger, cleanup } = + setup({ + groupAccounts: [account], + }); + rootMessenger.unregisterActionHandler( + 'AccountTreeController:getAccountsFromSelectedAccountGroup', + ); + rootMessenger.registerActionHandler( + 'AccountTreeController:getAccountsFromSelectedAccountGroup', + () => { + throw new Error('account tree failed'); + }, + ); + + triggerBalanceUpdated({ + address: EVM_ADDRESS, + chain: CHAIN_MAINNET, + updates: [createBalanceUpdate()], + }); + + await new Promise(process.nextTick); + + expect(onAssetsUpdate).not.toHaveBeenCalled(); + + cleanup(); + }); + it('converts a hex postBalance to a human-readable amount', async () => { const account = createMockAccount(); const { onAssetsUpdate, triggerBalanceUpdated, cleanup } = setup({ diff --git a/packages/assets-controller/src/data-sources/AccountActivityDataSource.ts b/packages/assets-controller/src/data-sources/AccountActivityDataSource.ts index cea27c29b5e..720ab29c0cc 100644 --- a/packages/assets-controller/src/data-sources/AccountActivityDataSource.ts +++ b/packages/assets-controller/src/data-sources/AccountActivityDataSource.ts @@ -321,11 +321,17 @@ export class AccountActivityDataSource extends AbstractDataSource< * @returns The accounts of the selected account group. */ #getWalletAccounts(): InternalAccount[] { - const accounts = this.#messenger.call( - 'AccountTreeController:getAccountsFromSelectedAccountGroup', - ); - if (accounts.length > 0) { - return accounts; + try { + const accounts = this.#messenger.call( + 'AccountTreeController:getAccountsFromSelectedAccountGroup', + ); + if (accounts.length > 0) { + return accounts; + } + } catch (error) { + if (!isUnregisteredActionError(error)) { + throw error; + } } const selectedAccount = this.#messenger.call( @@ -413,6 +419,18 @@ export class AccountActivityDataSource extends AbstractDataSource< } } +/** + * Whether a messenger call failed because no controller has registered the action. + * + * @param error - The error thrown by `messenger.call`. + * @returns True when the action handler is not registered. + */ +function isUnregisteredActionError(error: unknown): boolean { + return ( + error instanceof Error && error.message.includes('has not been registered') + ); +} + // ============================================================================ // FACTORY FUNCTION // ============================================================================ diff --git a/packages/assets-controller/src/data-sources/AccountsApiDataSource.test.ts b/packages/assets-controller/src/data-sources/AccountsApiDataSource.test.ts index d84eee998ca..329a6403765 100644 --- a/packages/assets-controller/src/data-sources/AccountsApiDataSource.test.ts +++ b/packages/assets-controller/src/data-sources/AccountsApiDataSource.test.ts @@ -199,6 +199,7 @@ async function setupController( remoteFeatureFlags?: Record; getAssetsState?: () => AssetsControllerState; tokenDetectionEnabled?: () => boolean; + initialize?: boolean; } = {}, ): Promise { const { @@ -212,6 +213,7 @@ async function setupController( remoteFeatureFlags = {}, getAssetsState = (): AssetsControllerState => createAssetsState(), tokenDetectionEnabled, + initialize = true, } = options; const rootMessenger = new Messenger({ @@ -277,7 +279,9 @@ async function setupController( ...(tokenDetectionEnabled === undefined ? {} : { tokenDetectionEnabled }), }); - // Wait for async initialization + if (initialize) { + await controller.init(); + } await new Promise(process.nextTick); return { @@ -319,6 +323,30 @@ describe('AccountsApiDataSource', () => { vi.clearAllMocks(); }); + it('does not call RemoteFeatureFlagController in the constructor', async () => { + const { controller, apiClient, activeChainsUpdateHandler } = + await setupController({ + initialize: false, + supportedChains: [1], + }); + + expect(apiClient.accounts.fetchV2SupportedNetworks).not.toHaveBeenCalled(); + expect(await controller.getActiveChains()).toStrictEqual([]); + + await controller.init(); + + expect(apiClient.accounts.fetchV2SupportedNetworks).toHaveBeenCalledTimes( + 1, + ); + expect(activeChainsUpdateHandler).toHaveBeenCalledWith( + 'AccountsApiDataSource', + [CHAIN_MAINNET], + [], + ); + + controller.destroy(); + }); + it('initializes with correct name', async () => { const { controller } = await setupController(); expect(controller.getName()).toBe('AccountsApiDataSource'); @@ -339,6 +367,22 @@ describe('AccountsApiDataSource', () => { controller.destroy(); }); + it('refreshes supported networks on a second init', async () => { + const { controller, apiClient } = await setupController({ + supportedChains: [1], + }); + + apiClient.accounts.fetchV2SupportedNetworks.mockClear(); + + await controller.init(); + + expect(apiClient.accounts.fetchV2SupportedNetworks).toHaveBeenCalledTimes( + 1, + ); + + controller.destroy(); + }); + it('refreshActiveChains re-fetches supported networks and updates activeChains', async () => { const { controller, apiClient, activeChainsUpdateHandler } = await setupController({ supportedChains: [1] }); @@ -2068,7 +2112,7 @@ describe('AccountsApiDataSource', () => { const controller = new AccountsApiDataSource(controllerOptions); - // Wait for async initialization + await controller.init(); await new Promise(process.nextTick); return { diff --git a/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts b/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts index 1894998a418..7902a2aa015 100644 --- a/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts +++ b/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts @@ -314,8 +314,21 @@ export class AccountsApiDataSource extends AbstractDataSource< // return a primitive rather than a fresh object. (state) => getMigrationStages(state.remoteFeatureFlags).join(','), ); + } - this.#initializeActiveChains().catch(console.error); + /** + * Fetch supported networks and start the active-chain refresh timer. + * + * The constructor does not call messenger actions. `AssetsController.init()` + * calls this after every controller has been constructed. A later call + * refreshes the chain list without starting another timer. + */ + async init(): Promise { + if (this.#chainsRefreshTimer) { + await this.#refreshActiveChains(); + return; + } + await this.#initializeActiveChains(); } /** diff --git a/packages/assets-controller/src/data-sources/RpcDataSource.test.ts b/packages/assets-controller/src/data-sources/RpcDataSource.test.ts index 300f6170667..447058e10ce 100644 --- a/packages/assets-controller/src/data-sources/RpcDataSource.test.ts +++ b/packages/assets-controller/src/data-sources/RpcDataSource.test.ts @@ -269,6 +269,8 @@ async function withController( ...options, getAssetsState, }); + // Active chains are read in init(), not the constructor. + controller.refreshActiveChainsFromNetworkState(); try { return await fn({ @@ -374,6 +376,32 @@ describe('RpcDataSource', () => { ); }); + it('does not call messenger actions in the constructor', () => { + const { assetsControllerMessenger } = createMockMessengers({ + registerCustomRootActions: (rootMessenger) => + registerRpcDataSourceActions(rootMessenger, { + networkState: createMockNetworkState(), + }), + }); + const callSpy = vi.spyOn(assetsControllerMessenger, 'call'); + const source = new RpcDataSource({ + messenger: assetsControllerMessenger, + getAssetsState: getDefaultAssetsControllerState, + onActiveChainsUpdated: vi.fn(), + getNativeAssetForChain: vi.fn(), + getAssetType: vi.fn(), + getAssetVisibility: vi.fn(), + queryApiClient: createTestApiClient(), + }); + + expect(callSpy).not.toHaveBeenCalled(); + + source.refreshActiveChainsFromNetworkState(); + + expect(callSpy).toHaveBeenCalledWith('NetworkController:getState'); + source.destroy(); + }); + it('reports active chains on initialization', async () => { await withController(async ({ onActiveChainsUpdated }) => { expect(onActiveChainsUpdated).toHaveBeenCalledWith( diff --git a/packages/assets-controller/src/data-sources/RpcDataSource.ts b/packages/assets-controller/src/data-sources/RpcDataSource.ts index 36a7aa13578..1e6ef66009c 100644 --- a/packages/assets-controller/src/data-sources/RpcDataSource.ts +++ b/packages/assets-controller/src/data-sources/RpcDataSource.ts @@ -347,7 +347,6 @@ export class RpcDataSource extends AbstractDataSource< this.#subscribeToNetworkController(); this.#subscribeToTransactionEvents(); - this.#initializeFromNetworkController(); } /** @@ -875,8 +874,11 @@ export class RpcDataSource extends AbstractDataSource< } /** - * Re-read NetworkController state and refresh Rpc `activeChains` (e.g. when - * network availability metadata changes after an EVM network switch). + * Read `NetworkController:getState` and refresh Rpc `activeChains`. + * + * The constructor does not call messenger actions. `AssetsController.init()` + * calls this after every controller has been constructed. Later network + * changes still arrive on `NetworkController:stateChange`. */ refreshActiveChainsFromNetworkState(): void { this.#initializeFromNetworkController(); diff --git a/packages/assets-controller/src/data-sources/SnapDataSource.test.ts b/packages/assets-controller/src/data-sources/SnapDataSource.test.ts index 2dd0d7b719d..bb0a62f6bed 100644 --- a/packages/assets-controller/src/data-sources/SnapDataSource.test.ts +++ b/packages/assets-controller/src/data-sources/SnapDataSource.test.ts @@ -248,6 +248,7 @@ function setupController( isBalanceV6Enabled?: () => boolean; getAssetVisibility?: GetAssetVisibility; getAssetsState?: () => AssetsControllerState; + initialize?: boolean; } = {}, ): SetupResult { const { @@ -339,6 +340,9 @@ function setupController( }; const controller = new SnapDataSource(controllerOptions); + if (options.initialize !== false) { + controller.init(); + } // Subscribe so that balance updates are reported via onAssetsUpdate const chainIdsFromSnaps = Object.values(installedSnaps).flatMap( @@ -433,6 +437,28 @@ describe('SnapDataSource helper functions', () => { }); describe('SnapDataSource', () => { + it('does not call SnapController in the constructor', () => { + const { controller, mockGetRunnableSnaps, mockGetPermissions, cleanup } = + setupController({ + installedSnaps: { + [SOLANA_SNAP_ID]: { version: '1.0.0', chainIds: [SOLANA_MAINNET] }, + }, + initialize: false, + }); + + expect(mockGetRunnableSnaps).not.toHaveBeenCalled(); + expect(mockGetPermissions).not.toHaveBeenCalled(); + expect(controller.getActiveChainsSync()).toStrictEqual([]); + + controller.init(); + + expect(mockGetRunnableSnaps).toHaveBeenCalledTimes(1); + expect(mockGetPermissions).toHaveBeenCalledWith(SOLANA_SNAP_ID); + expect(controller.getActiveChainsSync()).toContain(SOLANA_MAINNET); + + cleanup(); + }); + it('initializes with correct name', async () => { const { controller, cleanup } = setupController(); expect(controller.getName()).toBe('SnapDataSource'); diff --git a/packages/assets-controller/src/data-sources/SnapDataSource.ts b/packages/assets-controller/src/data-sources/SnapDataSource.ts index 1d62da6d57e..0ca743c719b 100644 --- a/packages/assets-controller/src/data-sources/SnapDataSource.ts +++ b/packages/assets-controller/src/data-sources/SnapDataSource.ts @@ -265,8 +265,17 @@ export class SnapDataSource extends AbstractDataSource< this.#discoverKeyringSnaps.bind(this); this.#subscribeToEvents(); + } - // Discover keyring-capable snaps and populate activeChains dynamically + /** + * Discover keyring snaps and set `activeChains`. + * + * The constructor does not call messenger actions. `AssetsController.init()` + * calls this after every controller has been constructed. When + * `SnapController` is not registered, discovery leaves `activeChains` empty. + * Later snap installs and permission changes rediscover. + */ + init(): void { this.#discoverKeyringSnaps(); } diff --git a/packages/assets-controller/src/data-sources/StakedBalanceDataSource.test.ts b/packages/assets-controller/src/data-sources/StakedBalanceDataSource.test.ts index 1ee597f82ef..10fdc59853e 100644 --- a/packages/assets-controller/src/data-sources/StakedBalanceDataSource.test.ts +++ b/packages/assets-controller/src/data-sources/StakedBalanceDataSource.test.ts @@ -142,6 +142,8 @@ async function withController( ...options, pollInterval: 1000, }); + // Active chains are read in init(), not the constructor. + controller.refreshActiveChainsFromNetworkEnablementState(); try { return await fn({ @@ -187,6 +189,29 @@ describe('StakedBalanceDataSource', () => { ); }); + it('does not call messenger actions in the constructor', async () => { + const { assetsControllerMessenger } = createMockMessengers({ + registerCustomRootActions: (messenger) => + registerStakedMessengerActions(messenger, { + enabledNetworkMap: { eip155: { [MAINNET_CHAIN_ID_HEX]: true } }, + }), + }); + const callSpy = vi.spyOn(assetsControllerMessenger, 'call'); + const source = new StakedBalanceDataSource({ + messenger: assetsControllerMessenger, + onActiveChainsUpdated: vi.fn(), + }); + + expect(callSpy).not.toHaveBeenCalled(); + + source.refreshActiveChainsFromNetworkEnablementState(); + + expect(callSpy).toHaveBeenCalledWith( + 'NetworkEnablementController:getState', + ); + source.destroy(); + }); + it('calls onActiveChainsUpdated with active staking chains when mainnet is enabled', async () => { await withController(({ onActiveChainsUpdated }) => { expect(onActiveChainsUpdated).toHaveBeenCalledWith( diff --git a/packages/assets-controller/src/data-sources/StakedBalanceDataSource.ts b/packages/assets-controller/src/data-sources/StakedBalanceDataSource.ts index 84511b98a16..513662af54d 100644 --- a/packages/assets-controller/src/data-sources/StakedBalanceDataSource.ts +++ b/packages/assets-controller/src/data-sources/StakedBalanceDataSource.ts @@ -215,14 +215,15 @@ export class StakedBalanceDataSource extends AbstractDataSource< 'NetworkEnablementController:stateChange', this.#onNetworkEnablementControllerStateChange.bind(this), ); - - this.#initializeActiveChains(); } /** - * Re-read NetworkEnablementController state and refresh staking - * `activeChains` (e.g. when that controller was not registered yet at - * construction). + * Read `NetworkEnablementController:getState` and refresh staking + * `activeChains`. + * + * The constructor does not call messenger actions. `AssetsController.init()` + * calls this after every controller has been constructed. Later enablement + * changes still arrive on `NetworkEnablementController:stateChange`. */ refreshActiveChainsFromNetworkEnablementState(): void { this.#initializeActiveChains(); diff --git a/packages/wallet/CHANGELOG.md b/packages/wallet/CHANGELOG.md index dd2b3b5286d..58dd281c593 100644 --- a/packages/wallet/CHANGELOG.md +++ b/packages/wallet/CHANGELOG.md @@ -9,9 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Export an `assetsController` initialization configuration for `AssetsController`, along with the `AssetsControllerInstanceOptions` type ([#10885](https://github.com/MetaMask/core/pull/10885)) - - It is not part of the default initialization configurations, because the Wallet does not yet initialize several of its dependencies (e.g. `AccountTreeController`, `SnapController`). Pass it through `initializationConfigurations`. Those configurations are constructed before the default `NetworkEnablementController`. `AssetsController.init()` reads that controller's state during `wallet.init()`, and later enablement changes arrive on `NetworkEnablementController:stateChange`. - - Configure it through the new optional `instanceOptions.assetsController`, which requires a `queryApiClient`. +- **BREAKING:** Add `AssetsController` to the default initialization configurations, and export `assetsController` with the `AssetsControllerInstanceOptions` type ([#10885](https://github.com/MetaMask/core/pull/10885)) + - Requires `instanceOptions.assetsController.queryApiClient`. + - `Wallet.init()` calls `AssetsController.init()`, which reads `NetworkEnablementController:getState` after every controller has been constructed. Later enablement changes arrive on `NetworkEnablementController:stateChange`. + - Delegates `AccountTreeController` and `SnapController` actions and events. `Wallet` does not construct those controllers. Until a client registers them, account-group reads fall back to `AccountsController:getSelectedAccount`, asset tracking does not wait for the account tree, and snap discovery leaves snap chains empty. + - Also delegates the other external `AssetsControllerMessenger` actions and events (accounts, client, keyring, network, network enablement, permission, phishing, remote feature flags, preferences, and transactions). `Wallet` constructs the ones it already initializes by default. + - Hosts that already register `AssetsController` on the wallet root messenger must remove that registration before upgrading, or messenger registration will collide. ### Changed diff --git a/packages/wallet/src/Wallet.test.ts b/packages/wallet/src/Wallet.test.ts index 1cc3dbad7a3..84e24c69bca 100644 --- a/packages/wallet/src/Wallet.test.ts +++ b/packages/wallet/src/Wallet.test.ts @@ -42,8 +42,22 @@ const REMOTE_FEATURE_FLAG_OPTIONS = { const TEST_ANALYTICS_ID = '11111111-1111-4111-8111-111111111111'; +function createMockQueryApiClient(): WalletOptions['instanceOptions']['assetsController']['queryApiClient'] { + const cache = new Map(); + return { + getCachedData: vi.fn((key: string[]) => cache.get(JSON.stringify(key))), + setCachedData: vi.fn((key: string[], data: unknown) => { + cache.set(JSON.stringify(key), data); + }), + queryClient: { + fetchQuery: vi.fn().mockResolvedValue({}), + }, + } as unknown as WalletOptions['instanceOptions']['assetsController']['queryApiClient']; +} + function getInstanceOptions(): WalletOptions['instanceOptions'] { return { + assetsController: { queryApiClient: createMockQueryApiClient() }, analyticsController: { platformAdapter: { track: vi.fn(), @@ -199,7 +213,7 @@ describe('Wallet', () => { const results = await wallet.init(); - expect(results).toHaveLength(10); + expect(results).toHaveLength(11); }); it('disallows modifying the messenger', async () => { diff --git a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts index 3b72572f6b8..99fbe4b858b 100644 --- a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts +++ b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts @@ -17,7 +17,6 @@ import type { DefaultEvents, RootMessenger, } from '../../defaults.js'; -import type { AnyInitializationConfiguration } from '../../types.js'; import { AlwaysOnlineAdapter } from '../connectivity-controller/always-online-adapter.js'; import { assetsController } from './assets-controller.js'; import type { AssetsControllerInstanceOptions } from './types.js'; @@ -52,10 +51,8 @@ describe('assetsController', () => { await Promise.all(wallets.splice(0).map((wallet) => wallet.destroy())); }); - it('is not part of the default initialization configurations', () => { - expect(Object.values(defaultConfigurations)).not.toContain( - assetsController, - ); + it('is part of the default initialization configurations', () => { + expect(Object.values(defaultConfigurations)).toContain(assetsController); }); it('is exported from the package entry point', () => { @@ -222,7 +219,7 @@ describe('assetsController', () => { expect(listener).toHaveBeenCalledWith('payload'); }); - it('is constructed before the default NetworkEnablementController and reads its state during wallet init', async () => { + it('is constructed by default while AccountTreeController and SnapController are absent', async () => { const accountId = 'account-1'; const nativeAssetId = 'eip155:1/slip44:60'; const wallet = new Wallet({ @@ -243,23 +240,32 @@ describe('assetsController', () => { }, }, }, - initializationConfigurations: [assetsController], - instanceOptions: { - ...getInstanceOptions(), - assetsController: { queryApiClient: createMockQueryApiClient() }, - }, + instanceOptions: getInstanceOptions(), }); wallets.push(wallet); - const instance = wallet.getInstance( - 'AssetsController', - ) as unknown as AssetsController; + const instance = wallet.getInstance('AssetsController'); expect(instance).toBeInstanceOf(AssetsController); - await instance.init(); + expect(() => + ( + wallet.messenger as unknown as { + call(action: string): unknown; + } + ).call('AccountTreeController:isInitialized'), + ).toThrow(/AccountTreeController:isInitialized has not been/u); + expect(() => + ( + wallet.messenger as unknown as { + call(action: string): unknown; + } + ).call('SnapController:getRunnableSnaps'), + ).toThrow(/SnapController:getRunnableSnaps has not been/u); + + await wallet.init(); - // Default NetworkEnablementController state enables Ethereum mainnet. - // init() reads that state even though this controller was constructed first. + // AssetsController is constructed before NetworkEnablementController. + // init() reads that controller's default state, which enables mainnet. const assets = await instance.getAssets([ { id: accountId } as Parameters[0][number], ]); @@ -267,22 +273,15 @@ describe('assetsController', () => { expect(Object.keys(assets[accountId])).toStrictEqual([nativeAssetId]); }); - it('is initialized by the Wallet when passed as an initialization configuration', async () => { + it('exposes AssetsController state on the wallet messenger by default', () => { const wallet = new Wallet({ - initializationConfigurations: [ - getNetworkEnablementControllerStub(), - assetsController, - ], - instanceOptions: { - ...getInstanceOptions(), - assetsController: { queryApiClient: createMockQueryApiClient() }, - }, + instanceOptions: getInstanceOptions(), }); wallets.push(wallet); - const instance = wallet.getInstance('AssetsController'); - - expect(instance).toBeInstanceOf(AssetsController); + expect(wallet.getInstance('AssetsController')).toBeInstanceOf( + AssetsController, + ); expect( ( wallet.messenger as unknown as Messenger< @@ -295,19 +294,9 @@ describe('assetsController', () => { it('is destroyed with the Wallet', async () => { const wallet = new Wallet({ - initializationConfigurations: [ - getNetworkEnablementControllerStub(), - assetsController, - ], - instanceOptions: { - ...getInstanceOptions(), - assetsController: { queryApiClient: createMockQueryApiClient() }, - }, + instanceOptions: getInstanceOptions(), }); - const destroy = vi.spyOn( - wallet.getInstance('AssetsController') as unknown as AssetsController, - 'destroy', - ); + const destroy = vi.spyOn(wallet.getInstance('AssetsController'), 'destroy'); await wallet.destroy(); @@ -343,27 +332,6 @@ function createMockQueryApiClient(): AssetsControllerInstanceOptions['queryApiCl } as unknown as AssetsControllerInstanceOptions['queryApiClient']; } -/** - * Builds a stand-in for the `NetworkEnablementController` configuration, - * which the Wallet does not yet initialize by default. - * - * @returns The initialization configuration. - */ -function getNetworkEnablementControllerStub(): AnyInitializationConfiguration { - return { - name: 'NetworkEnablementController', - getMessenger: (parent: RootMessenger) => { - registerActionHandler( - parent, - 'NetworkEnablementController:getState', - () => NETWORK_ENABLEMENT_STATE, - ); - return new Messenger({ namespace: 'NetworkEnablementControllerStub' }); - }, - init: (): object => ({}), - }; -} - function getRootMessenger(): RootMessenger { const rootMessenger = new Messenger<'Root', DefaultActions, DefaultEvents>({ namespace: 'Root', @@ -380,6 +348,7 @@ function getRootMessenger(): RootMessenger { function getInstanceOptions(): WalletOptions['instanceOptions'] { return { + assetsController: { queryApiClient: createMockQueryApiClient() }, analyticsController: { platformAdapter: { track: vi.fn(), diff --git a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts index 8efc1f6ab1b..6db2b054747 100644 --- a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts +++ b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts @@ -1,7 +1,6 @@ import { AssetsController } from '@metamask/assets-controller'; import type { AssetsControllerMessenger } from '@metamask/assets-controller'; import { Messenger } from '@metamask/messenger'; -import type { MessengerActions, MessengerEvents } from '@metamask/messenger'; import type { InitializationConfiguration } from '../../types.js'; @@ -10,14 +9,12 @@ export type { AssetsControllerInstanceOptions } from './types.js'; /** * Initialization configuration for `AssetsController`. * - * This is not part of the default initialization configurations because the - * Wallet does not yet initialize several of the controllers it depends on - * (e.g. `AccountTreeController`, `SnapController`). Clients pass it through - * `initializationConfigurations`. Those configurations are constructed before - * the default `NetworkEnablementController`. `AssetsController` reads - * `NetworkEnablementController:getState` from `init()`, which `Wallet.init()` - * calls after every controller exists, and applies later enablement from - * `NetworkEnablementController:stateChange`. + * `AccountTreeController` and `SnapController` are not Wallet defaults. + * Construction does not call them. Account-group reads fall back to + * `AccountsController` until `AccountTreeController` is registered, and snap + * discovery leaves snap chains empty until `SnapController` is registered. + * `AssetsController.init()` reads `NetworkEnablementController` during + * `Wallet.init()`, after every default controller exists. */ export const assetsController: InitializationConfiguration< AssetsController, @@ -39,20 +36,12 @@ export const assetsController: InitializationConfiguration< }); }, getMessenger: (parent) => { - // Most of these dependencies are not initialized by the Wallet by default, - // so the root messenger is widened to the full set the controller uses. - const rootMessenger = parent as unknown as Messenger< - 'Root', - MessengerActions, - MessengerEvents - >; - const messenger: AssetsControllerMessenger = new Messenger({ namespace: 'AssetsController', - parent: rootMessenger, + parent, }); - rootMessenger.delegate({ + parent.delegate({ messenger, actions: [ 'AccountsController:getSelectedAccount', diff --git a/packages/wallet/src/initialization/instances/gas-fee-controller/gas-fee-controller.test.ts b/packages/wallet/src/initialization/instances/gas-fee-controller/gas-fee-controller.test.ts index 17611f8ae39..cc8d905d623 100644 --- a/packages/wallet/src/initialization/instances/gas-fee-controller/gas-fee-controller.test.ts +++ b/packages/wallet/src/initialization/instances/gas-fee-controller/gas-fee-controller.test.ts @@ -203,8 +203,22 @@ function getRootMessenger(): RootMessenger { return rootMessenger; } +function createMockQueryApiClient(): WalletOptions['instanceOptions']['assetsController']['queryApiClient'] { + const cache = new Map(); + return { + getCachedData: vi.fn((key: string[]) => cache.get(JSON.stringify(key))), + setCachedData: vi.fn((key: string[], data: unknown) => { + cache.set(JSON.stringify(key), data); + }), + queryClient: { + fetchQuery: vi.fn().mockResolvedValue({}), + }, + } as unknown as WalletOptions['instanceOptions']['assetsController']['queryApiClient']; +} + function getInstanceOptions(): WalletOptions['instanceOptions'] { return { + assetsController: { queryApiClient: createMockQueryApiClient() }, analyticsController: { platformAdapter: { track: vi.fn(), diff --git a/packages/wallet/src/initialization/instances/index.ts b/packages/wallet/src/initialization/instances/index.ts index 4055cad6ada..55e4d7d1c0a 100644 --- a/packages/wallet/src/initialization/instances/index.ts +++ b/packages/wallet/src/initialization/instances/index.ts @@ -2,6 +2,7 @@ export { accountsController } from './accounts-controller/accounts-controller.js export { addressBookController } from './address-book-controller/address-book-controller.js'; export { analyticsController } from './analytics-controller/analytics-controller.js'; export { approvalController } from './approval-controller/approval-controller.js'; +export { assetsController } from './assets-controller/assets-controller.js'; export { claimsController } from './claims-controller/claims-controller.js'; export { claimsService } from './claims-service/claims-service.js'; export { clientController } from './client-controller/client-controller.js'; diff --git a/packages/wallet/src/initialization/instances/transaction-controller/transaction-controller.test.ts b/packages/wallet/src/initialization/instances/transaction-controller/transaction-controller.test.ts index 05d8f4c3552..9d4dd6fb2e8 100644 --- a/packages/wallet/src/initialization/instances/transaction-controller/transaction-controller.test.ts +++ b/packages/wallet/src/initialization/instances/transaction-controller/transaction-controller.test.ts @@ -139,8 +139,22 @@ function getRootMessenger(): RootMessenger { return rootMessenger; } +function createMockQueryApiClient(): WalletOptions['instanceOptions']['assetsController']['queryApiClient'] { + const cache = new Map(); + return { + getCachedData: vi.fn((key: string[]) => cache.get(JSON.stringify(key))), + setCachedData: vi.fn((key: string[], data: unknown) => { + cache.set(JSON.stringify(key), data); + }), + queryClient: { + fetchQuery: vi.fn().mockResolvedValue({}), + }, + } as unknown as WalletOptions['instanceOptions']['assetsController']['queryApiClient']; +} + function getInstanceOptions(): WalletOptions['instanceOptions'] { return { + assetsController: { queryApiClient: createMockQueryApiClient() }, analyticsController: { platformAdapter: { track: vi.fn(), diff --git a/packages/wallet/src/types.ts b/packages/wallet/src/types.ts index 7868234caf1..397877f129a 100644 --- a/packages/wallet/src/types.ts +++ b/packages/wallet/src/types.ts @@ -39,7 +39,7 @@ export type WalletOptions = { export type InstanceSpecificOptions = { analyticsController: AnalyticsControllerInstanceOptions; approvalController?: ApprovalControllerInstanceOptions; - assetsController?: AssetsControllerInstanceOptions; + assetsController: AssetsControllerInstanceOptions; claimsService?: ClaimsServiceInstanceOptions; configRegistryApiService: ConfigRegistryApiServiceInstanceOptions; configRegistryController?: ConfigRegistryControllerInstanceOptions; From 4025d6d6575577ca7c78ba9df6aab984ba7d4b29 Mon Sep 17 00:00:00 2001 From: Xiaoming Wang Date: Mon, 2 Nov 2026 22:06:16 +0800 Subject: [PATCH 11/24] Removed the anti pattern code and fix the test file. --- .cursor/hooks/state/continual-learning.json | 8 ++++++++ packages/wallet/CHANGELOG.md | 2 +- packages/wallet/src/index.ts | 2 -- .../instances/assets-controller/assets-controller.test.ts | 5 ----- 4 files changed, 9 insertions(+), 8 deletions(-) create mode 100644 .cursor/hooks/state/continual-learning.json diff --git a/.cursor/hooks/state/continual-learning.json b/.cursor/hooks/state/continual-learning.json new file mode 100644 index 00000000000..01debbefc1c --- /dev/null +++ b/.cursor/hooks/state/continual-learning.json @@ -0,0 +1,8 @@ +{ + "version": 1, + "lastRunAtMs": 0, + "turnsSinceLastRun": 5, + "lastTranscriptMtimeMs": null, + "lastProcessedGenerationId": "806e592b-b4df-4d6b-b6f5-a1653f1acd1c", + "trialStartedAtMs": null +} diff --git a/packages/wallet/CHANGELOG.md b/packages/wallet/CHANGELOG.md index 58dd281c593..15888ed0f48 100644 --- a/packages/wallet/CHANGELOG.md +++ b/packages/wallet/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **BREAKING:** Add `AssetsController` to the default initialization configurations, and export `assetsController` with the `AssetsControllerInstanceOptions` type ([#10885](https://github.com/MetaMask/core/pull/10885)) +- **BREAKING:** Add `AssetsController` to the default initialization configurations ([#10885](https://github.com/MetaMask/core/pull/10885)) - Requires `instanceOptions.assetsController.queryApiClient`. - `Wallet.init()` calls `AssetsController.init()`, which reads `NetworkEnablementController:getState` after every controller has been constructed. Later enablement changes arrive on `NetworkEnablementController:stateChange`. - Delegates `AccountTreeController` and `SnapController` actions and events. `Wallet` does not construct those controllers. Until a client registers them, account-group reads fall back to `AccountsController:getSelectedAccount`, asset tracking does not wait for the account tree, and snap discovery leaves snap chains empty. diff --git a/packages/wallet/src/index.ts b/packages/wallet/src/index.ts index e5672faba00..50e9b004160 100644 --- a/packages/wallet/src/index.ts +++ b/packages/wallet/src/index.ts @@ -1,8 +1,6 @@ export { Wallet } from './Wallet.js'; export { AlwaysOnlineAdapter } from './initialization/instances/connectivity-controller/always-online-adapter.js'; export { importSecretRecoveryPhrase } from './utilities.js'; -export { assetsController } from './initialization/instances/assets-controller/assets-controller.js'; -export type { AssetsControllerInstanceOptions } from './initialization/instances/assets-controller/types.js'; export type { WalletOptions } from './types.js'; export type { DefaultActions, diff --git a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts index 99fbe4b858b..e57a12bf9bc 100644 --- a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts +++ b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts @@ -8,7 +8,6 @@ import { Messenger } from '@metamask/messenger'; import { InMemoryStorageAdapter } from '@metamask/storage-service'; import { afterEach, describe, expect, it, vi } from 'vitest'; -import * as walletExports from '../../../index.js'; import type { WalletOptions } from '../../../types.js'; import { Wallet } from '../../../Wallet.js'; import { defaultConfigurations } from '../../defaults.js'; @@ -55,10 +54,6 @@ describe('assetsController', () => { expect(Object.values(defaultConfigurations)).toContain(assetsController); }); - it('is exported from the package entry point', () => { - expect(walletExports.assetsController).toBe(assetsController); - }); - it('initializes an AssetsController with default state', () => { const rootMessenger = getRootMessenger(); const messenger = assetsController.getMessenger(rootMessenger); From aad2df58e49304db5f9b1540ac1d96bb1eeee523 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 2 Nov 2026 14:08:45 +0000 Subject: [PATCH 12/24] chore: remove local cursor hook state file Co-authored-by: Xiaoming Wang --- .cursor/hooks/state/continual-learning.json | 8 -------- 1 file changed, 8 deletions(-) delete mode 100644 .cursor/hooks/state/continual-learning.json diff --git a/.cursor/hooks/state/continual-learning.json b/.cursor/hooks/state/continual-learning.json deleted file mode 100644 index 01debbefc1c..00000000000 --- a/.cursor/hooks/state/continual-learning.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "version": 1, - "lastRunAtMs": 0, - "turnsSinceLastRun": 5, - "lastTranscriptMtimeMs": null, - "lastProcessedGenerationId": "806e592b-b4df-4d6b-b6f5-a1653f1acd1c", - "trialStartedAtMs": null -} From 67efa4ce129cd1fccc193f520baf66980be83621 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 2 Nov 2026 14:11:19 +0000 Subject: [PATCH 13/24] fix(assets-controller): refresh assets on account switch without account tree When AccountTreeController is not registered, selected-account reads already fall back to AccountsController, but switches were ignored until another lifecycle event. Subscribe to AccountsController:selectedAccountChange in that case, and ignore it while AccountTreeController is registered so a group switch is refreshed only once. Co-authored-by: Xiaoming Wang --- packages/assets-controller/CHANGELOG.md | 3 + .../src/AssetsController.test.ts | 187 ++++++++++++++++++ .../assets-controller/src/AssetsController.ts | 78 +++++++- packages/assets-controller/src/README.md | 17 +- .../MockAssetControllerMessenger.ts | 1 + packages/wallet/CHANGELOG.md | 1 + .../assets-controller.test.ts | 1 + .../assets-controller/assets-controller.ts | 7 +- 8 files changed, 283 insertions(+), 12 deletions(-) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 8f64a1627c4..799de7cec29 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -15,6 +15,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **BREAKING:** Subscribe to `AccountsController:selectedAccountChange` so a selected-account switch refreshes tracked assets when `AccountTreeController` is not registered ([#10885](https://github.com/MetaMask/core/pull/10885)) + - Hosts that restrict which events flow through the `AssetsController` messenger must also delegate `AccountsController:selectedAccountChange`. + - When `AccountTreeController` is registered, that event is ignored. Group switches still refresh only through `AccountTreeController:selectedAccountGroupChange`, so the two events are not both handled. - **BREAKING:** Constructors no longer call messenger actions. Call `AssetsController.init()` after the other controllers exist. `Wallet.init()` does this. Until `init()` runs, enabled chains and data-source active chains stay empty unless a later state change arrives ([#10885](https://github.com/MetaMask/core/pull/10885)) - `AssetsController` no longer calls `NetworkEnablementController:getState` in its constructor. - `RpcDataSource` no longer calls `NetworkController:getState` in its constructor. Call `refreshActiveChainsFromNetworkState()`, or `AssetsController.init()`. diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index 4fccba39efa..b1db655df83 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -6125,6 +6125,193 @@ describe('AssetsController', () => { getAssetsSpy.mockRestore(); }); }); + + it('ignores AccountsController:selectedAccountChange while AccountTreeController is registered', async () => { + await withController(async ({ controller, messenger }) => { + const getAssetsSpy = vi + .spyOn(controller, 'getAssets') + .mockResolvedValue({}); + + await activateTracking(messenger); + getAssetsSpy.mockClear(); + + messenger.publish( + 'AccountsController:selectedAccountChange', + createMockInternalAccount({ id: 'mock-account-id-2' }), + ); + await flushPromises(); + + expect(getAssetsSpy).not.toHaveBeenCalled(); + + messenger.publish( + 'AccountTreeController:selectedAccountGroupChange', + 'entropy:mock-keyring-id-1/1', + 'entropy:mock-keyring-id-1/0', + ); + await flushPromises(); + + expect(getAssetsSpy).toHaveBeenCalledTimes(1); + getAssetsSpy.mockRestore(); + }); + }); + + it('does not refresh again when selectedAccountChange follows a group switch', async () => { + await withController(async ({ controller, messenger }) => { + const getAssetsSpy = vi + .spyOn(controller, 'getAssets') + .mockResolvedValue({}); + + await activateTracking(messenger); + getAssetsSpy.mockClear(); + + messenger.publish( + 'AccountTreeController:selectedAccountGroupChange', + 'entropy:mock-keyring-id-1/1', + 'entropy:mock-keyring-id-1/0', + ); + messenger.publish( + 'AccountsController:selectedAccountChange', + createMockInternalAccount({ id: 'mock-account-id-2' }), + ); + await flushPromises(); + + expect(getAssetsSpy).toHaveBeenCalledTimes(1); + getAssetsSpy.mockRestore(); + }); + }); + + it('does not treat AccountTreeController:isInitialized failures as a missing controller', async () => { + await withController(async ({ controller, messenger }) => { + const getAssetsSpy = vi + .spyOn(controller, 'getAssets') + .mockResolvedValue({}); + + await activateTracking(messenger); + getAssetsSpy.mockClear(); + + messenger.unregisterActionHandler( + 'AccountTreeController:isInitialized', + ); + messenger.registerActionHandler( + 'AccountTreeController:isInitialized', + () => { + throw new Error('account tree init failed'); + }, + ); + + const consoleError = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined); + try { + messenger.publish( + 'AccountsController:selectedAccountChange', + createMockInternalAccount({ id: 'mock-account-id-2' }), + ); + await flushPromises(); + + expect(getAssetsSpy).not.toHaveBeenCalled(); + expect(consoleError).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'account tree init failed', + }), + ); + } finally { + consoleError.mockRestore(); + } + getAssetsSpy.mockRestore(); + }); + }); + }); + + describe('selected account changes without AccountTreeController', () => { + /** + * Drops AccountTreeController actions and serves the selected account from + * AccountsController, matching a host that has not installed the tree. + * + * @param messenger - Root messenger from `withController`. + * @param selectedAccount - Account returned by `getSelectedAccount`. + * @returns A setter for the account served on later reads. + */ + function useAccountsControllerSelection( + messenger: RootMessenger, + selectedAccount: InternalAccount, + ): (account: InternalAccount) => void { + messenger.unregisterActionHandler( + 'AccountTreeController:getAccountsFromSelectedAccountGroup', + ); + messenger.unregisterActionHandler('AccountTreeController:isInitialized'); + + let current = selectedAccount; + messenger.unregisterActionHandler( + 'AccountsController:getSelectedAccount', + ); + ( + messenger as { + registerActionHandler: ( + action: string, + handler: () => InternalAccount, + ) => void; + } + ).registerActionHandler( + 'AccountsController:getSelectedAccount', + () => current, + ); + + return (account: InternalAccount) => { + current = account; + }; + } + + it('refreshes tracked assets when the selected account changes', async () => { + await withController(async ({ controller, messenger }) => { + const setSelectedAccount = useAccountsControllerSelection( + messenger, + createMockInternalAccount({ id: 'account-a' }), + ); + const getAssetsSpy = vi + .spyOn(controller, 'getAssets') + .mockResolvedValue({}); + + await activateTracking(messenger); + getAssetsSpy.mockClear(); + + const nextAccount = createMockInternalAccount({ id: 'account-b' }); + setSelectedAccount(nextAccount); + messenger.publish( + 'AccountsController:selectedAccountChange', + nextAccount, + ); + await flushPromises(); + + expect(getAssetsSpy).toHaveBeenCalledTimes(1); + expect(getAssetsSpy).toHaveBeenCalledWith( + [expect.objectContaining({ id: 'account-b' })], + expect.objectContaining({ forceUpdate: true }), + ); + getAssetsSpy.mockRestore(); + }); + }); + + it('does not refresh a selected-account change before tracking starts', async () => { + await withController(async ({ controller, messenger }) => { + useAccountsControllerSelection( + messenger, + createMockInternalAccount({ id: 'account-a' }), + ); + const getAssetsSpy = vi + .spyOn(controller, 'getAssets') + .mockResolvedValue({}); + + messenger.publish( + 'AccountsController:selectedAccountChange', + createMockInternalAccount({ id: 'account-b' }), + ); + await flushPromises(); + + expect(getAssetsSpy).not.toHaveBeenCalled(); + getAssetsSpy.mockRestore(); + }); + }); }); describe('assets loading status', () => { diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index ec2732a1766..bc629b77193 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -5,7 +5,10 @@ import type { AccountTreeControllerSelectedAccountGroupChangeEvent, AccountTreeControllerUninitializedEvent, } from '@metamask/account-tree-controller'; -import type { AccountsControllerGetSelectedAccountAction } from '@metamask/accounts-controller'; +import type { + AccountsControllerGetSelectedAccountAction, + AccountsControllerSelectedAccountChangeEvent, +} from '@metamask/accounts-controller'; import { BaseController } from '@metamask/base-controller'; import type { ControllerGetStateAction, @@ -336,10 +339,15 @@ type AllowedActions = | RemoteFeatureFlagControllerGetStateAction; type AllowedEvents = - // AssetsController — account tree lifecycle and group switches + // AssetsController — account tree lifecycle and group switches. + // `AccountsController:selectedAccountChange` is the fallback when + // `AccountTreeController` is not registered. It is ignored while that + // controller is registered, because the tree republishes the switch as + // `selectedAccountGroupChange`. | AccountTreeControllerSelectedAccountGroupChangeEvent | AccountTreeControllerInitializedEvent | AccountTreeControllerUninitializedEvent + | AccountsControllerSelectedAccountChangeEvent | ClientControllerStateChangeEvent | KeyringControllerLockEvent | KeyringControllerUnlockEvent @@ -862,6 +870,28 @@ export class AssetsController extends BaseController< } } + /** + * Whether `AccountTreeController` has registered its actions. + * + * Selected-account switches are published by `AccountsController` in both + * setups. When the tree controller is registered it turns those switches + * into `selectedAccountGroupChange`, so handling both events would refresh + * tracked assets twice. + * + * @returns True when `AccountTreeController:isInitialized` is registered. + */ + #isAccountTreeControllerRegistered(): boolean { + try { + this.messenger.call('AccountTreeController:isInitialized'); + return true; + } catch (error) { + if (!isUnregisteredActionError(error)) { + throw error; + } + return false; + } + } + /** * Whether the client (UI) is open, read from ClientController state. * @@ -1236,6 +1266,13 @@ export class AssetsController extends BaseController< }, ); + // Fallback for hosts that do not register AccountTreeController. + // Ignored while that controller is registered so a switch is not + // refreshed here and again on `selectedAccountGroupChange`. + this.messenger.subscribe('AccountsController:selectedAccountChange', () => { + this.#handleSelectedAccountChanged().catch(console.error); + }); + this.messenger.subscribe('AccountTreeController:initialized', () => { this.#updateActive(); }); @@ -4035,6 +4072,43 @@ export class AssetsController extends BaseController< previousGroupId, }); + await this.#refreshTrackedAccounts(accounts); + } + + /** + * Refresh tracked assets after `AccountsController:selectedAccountChange` + * when `AccountTreeController` is not registered. + * + * Ignored while the account tree controller is registered, because that + * controller republishes the switch as `selectedAccountGroupChange`. + * Also ignored until tracking has started, matching group switches, so the + * first start stays owned by unlock and account-tree initialization. + */ + async #handleSelectedAccountChanged(): Promise { + if (this.#isAccountTreeControllerRegistered()) { + return; + } + if (this.#activeSubscriptions.size === 0) { + return; + } + + const accounts = this.#getSelectedAccounts(); + + log('Selected account changed', { + accountCount: accounts.length, + accountIds: accounts.map((account) => account.id), + }); + + await this.#refreshTrackedAccounts(accounts); + } + + /** + * Force-fetch and resubscribe for the accounts that should be tracked now. + * + * @param accounts - Accounts to refresh. An empty list skips the fetch and + * still resubscribes, matching a switch onto an empty selection. + */ + async #refreshTrackedAccounts(accounts: InternalAccount[]): Promise { const releaseLock = await this.#accountRefreshMutex.acquire(); try { this.#ensureNativeBalancesDefaultZero(); diff --git a/packages/assets-controller/src/README.md b/packages/assets-controller/src/README.md index 8cd99a71f61..32a70584555 100644 --- a/packages/assets-controller/src/README.md +++ b/packages/assets-controller/src/README.md @@ -66,14 +66,15 @@ get selectedAccounts(): InternalAccount[] #### 1.3 Subscribe to External Events -| Event | Handler | Purpose | -| -------------------------------------------------- | -------------------------------- | -------------------------------------- | -| `AccountTreeController:selectedAccountGroupChange` | `handleAccountGroupChanged()` | Refresh when the user switches groups | -| `AccountTreeController:initialized` | `updateActive()` | Start tracking once the tree is built | -| `AccountTreeController:uninitialized` | `updateActive()` | Stop tracking when the tree is cleared | -| `NetworkEnablementController:stateChange` | `handleEnabledNetworksChanged()` | Track enabled chains | -| `KeyringController:unlock` | `start()` | Start subscriptions | -| `KeyringController:lock` | `stop()` | Stop subscriptions | +| Event | Handler | Purpose | +| -------------------------------------------------- | -------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- | +| `AccountTreeController:selectedAccountGroupChange` | `handleAccountGroupChanged()` | Refresh when the user switches groups | +| `AccountsController:selectedAccountChange` | `handleSelectedAccountChanged()` | Refresh when the selected account changes and `AccountTreeController` is absent. Ignored while that controller is registered | +| `AccountTreeController:initialized` | `updateActive()` | Start tracking once the tree is built | +| `AccountTreeController:uninitialized` | `updateActive()` | Stop tracking when the tree is cleared | +| `NetworkEnablementController:stateChange` | `handleEnabledNetworksChanged()` | Track enabled chains | +| `KeyringController:unlock` | `start()` | Start subscriptions | +| `KeyringController:lock` | `stop()` | Stop subscriptions | #### 1.4 Register Action Handlers diff --git a/packages/assets-controller/src/__fixtures__/MockAssetControllerMessenger.ts b/packages/assets-controller/src/__fixtures__/MockAssetControllerMessenger.ts index 52383681d09..f2fb2bf5961 100644 --- a/packages/assets-controller/src/__fixtures__/MockAssetControllerMessenger.ts +++ b/packages/assets-controller/src/__fixtures__/MockAssetControllerMessenger.ts @@ -127,6 +127,7 @@ export function createMockAssetsControllerMessenger( 'NetworkEnablementController:stateChange', // SnapDataSource 'AccountsController:accountBalancesUpdated', + 'AccountsController:selectedAccountChange', 'PermissionController:stateChange', 'SnapController:snapInstalled', // AccountActivityService (real-time balances + chain status) diff --git a/packages/wallet/CHANGELOG.md b/packages/wallet/CHANGELOG.md index 58dd281c593..a2d6ff74ea4 100644 --- a/packages/wallet/CHANGELOG.md +++ b/packages/wallet/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Requires `instanceOptions.assetsController.queryApiClient`. - `Wallet.init()` calls `AssetsController.init()`, which reads `NetworkEnablementController:getState` after every controller has been constructed. Later enablement changes arrive on `NetworkEnablementController:stateChange`. - Delegates `AccountTreeController` and `SnapController` actions and events. `Wallet` does not construct those controllers. Until a client registers them, account-group reads fall back to `AccountsController:getSelectedAccount`, asset tracking does not wait for the account tree, and snap discovery leaves snap chains empty. + - Delegates `AccountsController:selectedAccountChange`. When `AccountTreeController` is not registered, that event refreshes tracked assets. When it is registered, `AssetsController` ignores the event and still refreshes only on `AccountTreeController:selectedAccountGroupChange`. - Also delegates the other external `AssetsControllerMessenger` actions and events (accounts, client, keyring, network, network enablement, permission, phishing, remote feature flags, preferences, and transactions). `Wallet` constructs the ones it already initializes by default. - Hosts that already register `AssetsController` on the wallet root messenger must remove that registration before upgrading, or messenger registration will collide. diff --git a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts index 99fbe4b858b..0ad213e5960 100644 --- a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts +++ b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts @@ -184,6 +184,7 @@ describe('assetsController', () => { 'AccountActivityService:balanceUpdated', 'AccountActivityService:statusChanged', 'AccountsController:accountBalancesUpdated', + 'AccountsController:selectedAccountChange', 'AccountTreeController:initialized', 'AccountTreeController:selectedAccountGroupChange', 'AccountTreeController:uninitialized', diff --git a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts index 6db2b054747..d018ea03035 100644 --- a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts +++ b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts @@ -11,8 +11,10 @@ export type { AssetsControllerInstanceOptions } from './types.js'; * * `AccountTreeController` and `SnapController` are not Wallet defaults. * Construction does not call them. Account-group reads fall back to - * `AccountsController` until `AccountTreeController` is registered, and snap - * discovery leaves snap chains empty until `SnapController` is registered. + * `AccountsController` until `AccountTreeController` is registered, and + * `AccountsController:selectedAccountChange` refreshes tracked assets only + * in that fallback. Snap discovery leaves snap chains empty until + * `SnapController` is registered. * `AssetsController.init()` reads `NetworkEnablementController` during * `Wallet.init()`, after every default controller exists. */ @@ -63,6 +65,7 @@ export const assetsController: InitializationConfiguration< 'AccountActivityService:balanceUpdated', 'AccountActivityService:statusChanged', 'AccountsController:accountBalancesUpdated', + 'AccountsController:selectedAccountChange', 'AccountTreeController:initialized', 'AccountTreeController:selectedAccountGroupChange', 'AccountTreeController:uninitialized', From 472c8e8e342d0f9b3119a0a846c4c9e4ca29cea8 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 2 Nov 2026 14:25:44 +0000 Subject: [PATCH 14/24] fix(assets-controller): correct deferred-init test types Allow the second event payload on test publishes, and let staked messenger fixtures omit mockProvider so the constructor test can override only the enabled network map. Co-authored-by: Xiaoming Wang --- .../src/AssetsController.test.ts | 6 ++++- .../MockAssetControllerMessenger.ts | 26 +++++++++++-------- tsc-suppressions.json | 3 --- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index b1db655df83..5c8b9c14b4b 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -218,7 +218,11 @@ const [MOCK_DEFAULT_TRACKED_ASSET_ID] = * * @param messenger - The root messenger used to publish lifecycle events. */ -type LifecyclePublish = (topic: string, payload?: unknown) => void; +type LifecyclePublish = ( + topic: string, + payload?: unknown, + secondPayload?: unknown, +) => void; async function activateTracking(messenger: RootMessenger): Promise { (messenger as unknown as { publish: LifecyclePublish }).publish( diff --git a/packages/assets-controller/src/__fixtures__/MockAssetControllerMessenger.ts b/packages/assets-controller/src/__fixtures__/MockAssetControllerMessenger.ts index f2fb2bf5961..d5f018b503e 100644 --- a/packages/assets-controller/src/__fixtures__/MockAssetControllerMessenger.ts +++ b/packages/assets-controller/src/__fixtures__/MockAssetControllerMessenger.ts @@ -182,21 +182,25 @@ export function registerAssetsControllerStateMock( export function registerStakedMessengerActions( rootMessenger: MockRootMessenger, - opts = { - enabledNetworkMap: { eip155: { [MAINNET_CHAIN_ID_HEX]: true } } as Record< - string, - Record - >, - mockProvider: createMockWeb3Provider({ + opts: { + enabledNetworkMap?: Record>; + mockProvider?: ReturnType; + } = {}, +): void { + const enabledNetworkMap = opts.enabledNetworkMap ?? { + eip155: { [MAINNET_CHAIN_ID_HEX]: true }, + }; + const mockProvider = + opts.mockProvider ?? + createMockWeb3Provider({ sharesWei: '1000000000000000000', assetsWei: '1500000000000000000', - }), - }, -): void { + }); + rootMessenger.registerActionHandler( 'NetworkEnablementController:getState', () => ({ - enabledNetworkMap: opts.enabledNetworkMap, + enabledNetworkMap, nativeAssetIdentifiers: {}, }), ); @@ -205,7 +209,7 @@ export function registerStakedMessengerActions( 'NetworkController:getNetworkClientById', () => ({ - provider: opts.mockProvider, + provider: mockProvider, configuration: { chainId: MAINNET_CHAIN_ID_HEX }, }) as TestMockType, ); diff --git a/tsc-suppressions.json b/tsc-suppressions.json index 3b22ef89c3e..20d0dcb1c72 100644 --- a/tsc-suppressions.json +++ b/tsc-suppressions.json @@ -113,9 +113,6 @@ "TS2532": { "count": 1 }, - "TS2554": { - "count": 9 - }, "TS2739": { "count": 4 }, From fc535b2c136cee9122d2d23a13fa8984afee4aed Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 2 Nov 2026 14:30:22 +0000 Subject: [PATCH 15/24] fix(assets-controller): refresh account switches during startup Co-authored-by: Xiaoming Wang --- packages/assets-controller/CHANGELOG.md | 4 + .../src/AssetsController.test.ts | 76 +++++++++++++++++++ .../assets-controller/src/AssetsController.ts | 29 +++++++ 3 files changed, 109 insertions(+) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 799de7cec29..9e7eb9208a2 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -27,6 +27,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - When `AccountTreeController` actions are not registered, selected-account reads fall back to `AccountsController:getSelectedAccount`, and asset tracking does not wait for account-tree initialization ([#10885](https://github.com/MetaMask/core/pull/10885)) - `AccountActivityDataSource` uses the same fallback when resolving the account for a balance update. +### Fixed + +- Refresh the newly selected account after startup completes when it changes while the initial asset fetch is in progress ([#10885](https://github.com/MetaMask/core/pull/10885)) + ## [20.1.0] ### Added diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index 5c8b9c14b4b..84f780b412e 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -6036,6 +6036,43 @@ describe('AssetsController', () => { }); describe('account group changes', () => { + it('refreshes the latest group when it changes during startup', async () => { + const { client, arm, release, getGatedCallCount } = + createGatedQueryApiClient(); + + await withController( + { queryApiClient: client }, + async ({ controller, messenger, getSelectedAccountsMock }) => { + const accountA = createMockInternalAccount({ id: 'account-a' }); + const accountB = createMockInternalAccount({ id: 'account-b' }); + getSelectedAccountsMock.mockReturnValue([accountA]); + const getAssetsSpy = vi.spyOn(controller, 'getAssets'); + + arm(); + await activateTracking(messenger); + await waitFor(() => expect(getGatedCallCount()).toBeGreaterThan(0)); + + getSelectedAccountsMock.mockReturnValue([accountB]); + messenger.publish( + 'AccountTreeController:selectedAccountGroupChange', + 'entropy:mock-keyring-id-1/1', + 'entropy:mock-keyring-id-1/0', + ); + + release(); + + await waitFor(() => + expect(getAssetsSpy).toHaveBeenCalledWith( + [expect.objectContaining({ id: accountB.id })], + expect.objectContaining({ forceUpdate: true }), + ), + ); + expect(getAssetsSpy).toHaveBeenCalledTimes(2); + getAssetsSpy.mockRestore(); + }, + ); + }); + it('refreshes assets when the selected group changes while tracking', async () => { await withController(async ({ controller, messenger }) => { const getAssetsSpy = vi @@ -6296,6 +6333,45 @@ describe('AssetsController', () => { }); }); + it('refreshes the latest account when it changes during startup', async () => { + const { client, arm, release, getGatedCallCount } = + createGatedQueryApiClient(); + + await withController( + { queryApiClient: client }, + async ({ controller, messenger }) => { + const accountA = createMockInternalAccount({ id: 'account-a' }); + const accountB = createMockInternalAccount({ id: 'account-b' }); + const setSelectedAccount = useAccountsControllerSelection( + messenger, + accountA, + ); + const getAssetsSpy = vi.spyOn(controller, 'getAssets'); + + arm(); + await activateTracking(messenger); + await waitFor(() => expect(getGatedCallCount()).toBeGreaterThan(0)); + + setSelectedAccount(accountB); + messenger.publish( + 'AccountsController:selectedAccountChange', + accountB, + ); + + release(); + + await waitFor(() => + expect(getAssetsSpy).toHaveBeenCalledWith( + [expect.objectContaining({ id: accountB.id })], + expect.objectContaining({ forceUpdate: true }), + ), + ); + expect(getAssetsSpy).toHaveBeenCalledTimes(2); + getAssetsSpy.mockRestore(); + }, + ); + }); + it('does not refresh a selected-account change before tracking starts', async () => { await withController(async ({ controller, messenger }) => { useAccountsControllerSelection( diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index bc629b77193..e0249cc2aef 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -815,6 +815,14 @@ export class AssetsController extends BaseController< */ #startInFlight = false; + /** + * Whether an account switch occurred while startup was force-fetching. + * + * The startup subscription skips its initial Accounts API fetch, so a + * selection made in that window must be refreshed once startup completes. + */ + #pendingAccountRefresh = false; + /** Currently enabled chains from NetworkEnablementController */ #enabledChains: Set = new Set(); @@ -3565,6 +3573,21 @@ export class AssetsController extends BaseController< }) .finally(() => { this.#startInFlight = false; + const shouldRefreshPendingAccount = + this.#pendingAccountRefresh && + this.#isUiOpen() && + this.#isKeyringUnlocked() && + this.#isAccountTreeInitialized(); + this.#pendingAccountRefresh = false; + if (shouldRefreshPendingAccount) { + this.#refreshTrackedAccounts(this.#getSelectedAccounts()).catch( + (error: unknown) => { + log('Failed to refresh assets after startup account switch', { + error, + }); + }, + ); + } }); } @@ -4057,6 +4080,9 @@ export class AssetsController extends BaseController< // (including when the group did not change) so late subscribers can catch // up — ignore those until we are already tracking. if (this.#activeSubscriptions.size === 0) { + if (this.#startInFlight && groupId !== previousGroupId) { + this.#pendingAccountRefresh = true; + } return; } if (groupId === previousGroupId) { @@ -4089,6 +4115,9 @@ export class AssetsController extends BaseController< return; } if (this.#activeSubscriptions.size === 0) { + if (this.#startInFlight) { + this.#pendingAccountRefresh = true; + } return; } From a895268e1aadac2da80fc4baa45c8d2e797afac5 Mon Sep 17 00:00:00 2001 From: Xiaoming Wang <7315988+dawnseeker8@users.noreply.github.com> Date: Tue, 3 Nov 2026 18:34:06 +0800 Subject: [PATCH 16/24] Update packages/assets-controller/src/data-sources/AccountsApiDataSource.ts Co-authored-by: Christian Tran --- .../src/data-sources/AccountsApiDataSource.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts b/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts index 7902a2aa015..a27a57da1ad 100644 --- a/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts +++ b/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts @@ -324,10 +324,6 @@ export class AccountsApiDataSource extends AbstractDataSource< * refreshes the chain list without starting another timer. */ async init(): Promise { - if (this.#chainsRefreshTimer) { - await this.#refreshActiveChains(); - return; - } await this.#initializeActiveChains(); } From 5abb39b5c16907fd8e18b3f2c91c5944d1865b56 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 3 Nov 2026 10:35:21 +0000 Subject: [PATCH 17/24] fix: make Accounts API data source init idempotent Co-authored-by: Xiaoming Wang --- .../AccountsApiDataSource.test.ts | 13 +++++++++++++ .../src/data-sources/AccountsApiDataSource.ts | 19 ++++++++++++------- yarn.lock | 2 +- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/packages/assets-controller/src/data-sources/AccountsApiDataSource.test.ts b/packages/assets-controller/src/data-sources/AccountsApiDataSource.test.ts index 329a6403765..3762a9c765f 100644 --- a/packages/assets-controller/src/data-sources/AccountsApiDataSource.test.ts +++ b/packages/assets-controller/src/data-sources/AccountsApiDataSource.test.ts @@ -383,6 +383,19 @@ describe('AccountsApiDataSource', () => { controller.destroy(); }); + it('starts one refresh timer when init is called concurrently', async () => { + const { controller } = await setupController({ + initialize: false, + supportedChains: [1], + }); + + await Promise.all([controller.init(), controller.init()]); + + expect(activeTimers).toHaveLength(1); + + controller.destroy(); + }); + it('refreshActiveChains re-fetches supported networks and updates activeChains', async () => { const { controller, apiClient, activeChainsUpdateHandler } = await setupController({ supportedChains: [1] }); diff --git a/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts b/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts index 7902a2aa015..a4c83d87df5 100644 --- a/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts +++ b/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts @@ -317,17 +317,13 @@ export class AccountsApiDataSource extends AbstractDataSource< } /** - * Fetch supported networks and start the active-chain refresh timer. + * Fetch supported networks and ensure the active-chain refresh timer runs. * * The constructor does not call messenger actions. `AssetsController.init()` - * calls this after every controller has been constructed. A later call - * refreshes the chain list without starting another timer. + * calls this after every controller has been constructed. Repeated calls + * refresh the chain list without starting another timer. */ async init(): Promise { - if (this.#chainsRefreshTimer) { - await this.#refreshActiveChains(); - return; - } await this.#initializeActiveChains(); } @@ -352,6 +348,11 @@ export class AccountsApiDataSource extends AbstractDataSource< // ============================================================================ async #initializeActiveChains(): Promise { + if (this.#chainsRefreshTimer) { + await this.#refreshActiveChains(); + return; + } + try { const chains = await this.#fetchActiveChains(); const previous = [...this.state.activeChains]; @@ -359,6 +360,10 @@ export class AccountsApiDataSource extends AbstractDataSource< this.#onActiveChainsUpdated(this.getName(), updatedChains, previous), ); + if (this.#chainsRefreshTimer) { + return; + } + // Periodically refresh active chains (every 20 minutes) this.#chainsRefreshTimer = setInterval( () => { diff --git a/yarn.lock b/yarn.lock index a088229c0c7..2d7d95d75f1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5487,7 +5487,7 @@ __metadata: languageName: node linkType: hard -"@metamask/assets-controller@npm:^20.2.0, @metamask/assets-controller@workspace:packages/assets-controller": +"@metamask/assets-controller@npm:^20.1.0, @metamask/assets-controller@npm:^20.2.0, @metamask/assets-controller@workspace:packages/assets-controller": version: 0.0.0-use.local resolution: "@metamask/assets-controller@workspace:packages/assets-controller" dependencies: From 7bbff7dbfe3f8d0e3b1ef21ea20b455e9c06ecb2 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 3 Nov 2026 10:42:28 +0000 Subject: [PATCH 18/24] fix: align wallet assets controller dependency Co-authored-by: Xiaoming Wang --- packages/wallet/CHANGELOG.md | 2 +- packages/wallet/package.json | 2 +- yarn.lock | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/wallet/CHANGELOG.md b/packages/wallet/CHANGELOG.md index 8812abdd25e..d82b83464a5 100644 --- a/packages/wallet/CHANGELOG.md +++ b/packages/wallet/CHANGELOG.md @@ -19,7 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- Add `@metamask/assets-controller` as a dependency ([#10885](https://github.com/MetaMask/core/pull/10885)) +- Add `@metamask/assets-controller` `^20.2.0` as a dependency ([#10885](https://github.com/MetaMask/core/pull/10885)) ## [21.0.0] diff --git a/packages/wallet/package.json b/packages/wallet/package.json index c859f65ea7e..dd880a8bd93 100644 --- a/packages/wallet/package.json +++ b/packages/wallet/package.json @@ -54,7 +54,7 @@ "@metamask/address-book-controller": "^8.0.0", "@metamask/analytics-controller": "^4.1.0", "@metamask/approval-controller": "^10.0.0", - "@metamask/assets-controller": "^20.1.0", + "@metamask/assets-controller": "^20.2.0", "@metamask/base-controller": "^10.1.0", "@metamask/browser-passworder": "^6.0.0", "@metamask/claims-controller": "^1.1.0", diff --git a/yarn.lock b/yarn.lock index 2d7d95d75f1..633d6e71d30 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5487,7 +5487,7 @@ __metadata: languageName: node linkType: hard -"@metamask/assets-controller@npm:^20.1.0, @metamask/assets-controller@npm:^20.2.0, @metamask/assets-controller@workspace:packages/assets-controller": +"@metamask/assets-controller@npm:^20.2.0, @metamask/assets-controller@workspace:packages/assets-controller": version: 0.0.0-use.local resolution: "@metamask/assets-controller@workspace:packages/assets-controller" dependencies: @@ -9210,7 +9210,7 @@ __metadata: "@metamask/address-book-controller": "npm:^8.0.0" "@metamask/analytics-controller": "npm:^4.1.0" "@metamask/approval-controller": "npm:^10.0.0" - "@metamask/assets-controller": "npm:^20.1.0" + "@metamask/assets-controller": "npm:^20.2.0" "@metamask/auto-changelog": "npm:^7.1.0" "@metamask/base-controller": "npm:^10.1.0" "@metamask/browser-passworder": "npm:^6.0.0" From f864c38bf1b6b0118138a0728531ead7d5959771 Mon Sep 17 00:00:00 2001 From: Xiaoming Wang Date: Tue, 3 Nov 2026 18:52:45 +0800 Subject: [PATCH 19/24] fix(wallet): drop stale assets-controller lockfile descriptor Co-authored-by: Cursor --- yarn.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn.lock b/yarn.lock index 19e7ce2206a..633d6e71d30 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5487,7 +5487,7 @@ __metadata: languageName: node linkType: hard -"@metamask/assets-controller@npm:^20.1.0, @metamask/assets-controller@npm:^20.2.0, @metamask/assets-controller@workspace:packages/assets-controller": +"@metamask/assets-controller@npm:^20.2.0, @metamask/assets-controller@workspace:packages/assets-controller": version: 0.0.0-use.local resolution: "@metamask/assets-controller@workspace:packages/assets-controller" dependencies: From fbb37663c29fee2c645b93c6b7b75a117e836d41 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 3 Nov 2026 12:00:19 +0000 Subject: [PATCH 20/24] fix(wallet): drop redundant AssetsController queryApiClient guard AssetsControllerInstanceOptions already requires queryApiClient, and WalletOptions requires instanceOptions.assetsController, so omitting the client is a type error. Remove the runtime throw and the test that asserted it. Co-authored-by: Xiaoming Wang --- .../assets-controller/assets-controller.test.ts | 15 --------------- .../assets-controller/assets-controller.ts | 13 +++---------- 2 files changed, 3 insertions(+), 25 deletions(-) diff --git a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts index b555ed567c0..c2e7e0e0ade 100644 --- a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts +++ b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts @@ -108,21 +108,6 @@ describe('assetsController', () => { expect(unsubscribe).toHaveBeenCalledTimes(1); }); - it('throws when no queryApiClient is provided', () => { - const rootMessenger = getRootMessenger(); - const messenger = assetsController.getMessenger(rootMessenger); - - expect(() => - assetsController.init({ - state: undefined, - messenger, - options: {} as AssetsControllerInstanceOptions, - }), - ).toThrow( - 'AssetsController requires `instanceOptions.assetsController.queryApiClient`.', - ); - }); - it('exposes AssetsController actions over the root messenger', () => { const rootMessenger = getRootMessenger(); const messenger = assetsController.getMessenger(rootMessenger); diff --git a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts index d018ea03035..65d9abc13b9 100644 --- a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts +++ b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts @@ -24,19 +24,12 @@ export const assetsController: InitializationConfiguration< > = { name: 'AssetsController', reference: AssetsController, - init: ({ state, messenger, options }) => { - if (!options.queryApiClient) { - throw new Error( - 'AssetsController requires `instanceOptions.assetsController.queryApiClient`.', - ); - } - - return new AssetsController({ + init: ({ state, messenger, options }) => + new AssetsController({ ...options, messenger, state, - }); - }, + }), getMessenger: (parent) => { const messenger: AssetsControllerMessenger = new Messenger({ namespace: 'AssetsController', From 636b56dc88db1e03f333fc3d30a3c9d228bc56f8 Mon Sep 17 00:00:00 2001 From: Xiaoming Wang Date: Wed, 4 Nov 2026 16:10:13 +0800 Subject: [PATCH 21/24] refactor(assets-controller): create data sources in start() instead of the constructor The constructor no longer calls messenger actions, creates data sources or subscribes to external events, so construction does not depend on the order in which the Wallet constructs controllers. A new public `start()` method (also the `AssetsController:start` messenger action) reads enabled networks, creates the data sources, subscribes to external events and basic-functionality changes, runs the unlock cleanups when the keyring is already unlocked, and starts asset tracking. Clients call it once their own controllers (AccountTreeController, SnapController, ...) are registered, since `wallet.init()` runs before those exist. Data sources are unchanged from main: their constructors still read other controllers, only later. This replaces the per-data-source `init()` methods and the "controller not registered" fallbacks, which are removed. Entry points that can run before `start()` are guarded. Co-Authored-By: Claude Opus 5.5 --- packages/assets-controller/CHANGELOG.md | 23 +- .../AssetsController-method-action-types.ts | 42 + ...tering.accounts-api-v6.integration.test.ts | 2 +- ...c-spam-token-filtering.integration.test.ts | 2 +- ...er.launchpad-filtering.integration.test.ts | 2 +- ...ontroller.spam-cleanup.integration.test.ts | 2 +- .../src/AssetsController.spam-cleanup.test.ts | 2 +- ...tale-balance-scenarios.integration.test.ts | 2 +- .../src/AssetsController.test.ts | 878 ++++++------------ .../assets-controller/src/AssetsController.ts | 584 +++++++----- ...oller.ws-price-updates.integration.test.ts | 2 +- packages/assets-controller/src/README.md | 44 +- .../MockAssetControllerMessenger.ts | 27 +- .../AccountActivityDataSource.test.ts | 68 +- .../data-sources/AccountActivityDataSource.ts | 28 +- .../AccountsApiDataSource.test.ts | 61 +- .../src/data-sources/AccountsApiDataSource.ts | 20 +- .../src/data-sources/RpcDataSource.test.ts | 28 - .../src/data-sources/RpcDataSource.ts | 8 +- .../src/data-sources/SnapDataSource.test.ts | 26 - .../src/data-sources/SnapDataSource.ts | 11 +- .../StakedBalanceDataSource.test.ts | 50 - .../data-sources/StakedBalanceDataSource.ts | 10 - packages/assets-controller/src/index.ts | 1 + packages/wallet/CHANGELOG.md | 5 +- packages/wallet/src/Wallet.test.ts | 2 +- .../assets-controller.test.ts | 55 +- .../assets-controller/assets-controller.ts | 17 +- tsc-suppressions.json | 3 + 29 files changed, 799 insertions(+), 1206 deletions(-) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 144494c4db4..39350a8d203 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -9,27 +9,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Add `init()`, which reads enabled chains from `NetworkEnablementController:getState` after every controller has been constructed. Later enablement changes still arrive on `NetworkEnablementController:stateChange` ([#10885](https://github.com/MetaMask/core/pull/10885)) - - `init()` refreshes data sources that read other controllers through the messenger: `RpcDataSource.refreshActiveChainsFromNetworkState()` (`NetworkController:getState`), `StakedBalanceDataSource.refreshActiveChainsFromNetworkEnablementState()` (`NetworkEnablementController:getState`), `SnapDataSource.init()` (snap discovery), and `AccountsApiDataSource.init()` (supported networks and `RemoteFeatureFlagController:getState`). -- Add `SnapDataSource.init()` and `AccountsApiDataSource.init()` ([#10885](https://github.com/MetaMask/core/pull/10885)) +- Add `start()` and the `AssetsController:start` messenger action ([#10885](https://github.com/MetaMask/core/pull/10885)) + - `start()` reads enabled networks from `NetworkEnablementController:getState`, creates the data sources, subscribes to external events and to `subscribeToBasicFunctionalityChange`, runs the unlock cleanups when the keyring is already unlocked, and starts asset tracking when the UI is open, the keyring is unlocked and the account tree is initialized. + - Call it once the client has constructed and initialized the controllers `AssetsController` depends on, such as `AccountTreeController` and `SnapController`. Repeated calls return the same promise; calls after `destroy()` do nothing. +- Export the `AssetsControllerStartAction` type ([#10885](https://github.com/MetaMask/core/pull/10885)) ### Changed -- **BREAKING:** Subscribe to `AccountsController:selectedAccountChange` so a selected-account switch refreshes tracked assets when `AccountTreeController` is not registered ([#10885](https://github.com/MetaMask/core/pull/10885)) - - Hosts that restrict which events flow through the `AssetsController` messenger must also delegate `AccountsController:selectedAccountChange`. - - When `AccountTreeController` is registered, that event is ignored. Group switches still refresh only through `AccountTreeController:selectedAccountGroupChange`, so the two events are not both handled. -- **BREAKING:** Constructors no longer call messenger actions. Call `AssetsController.init()` after the other controllers exist. `Wallet.init()` does this. Until `init()` runs, enabled chains and data-source active chains stay empty unless a later state change arrives ([#10885](https://github.com/MetaMask/core/pull/10885)) - - `AssetsController` no longer calls `NetworkEnablementController:getState` in its constructor. - - `RpcDataSource` no longer calls `NetworkController:getState` in its constructor. Call `refreshActiveChainsFromNetworkState()`, or `AssetsController.init()`. - - `StakedBalanceDataSource` no longer calls `NetworkEnablementController:getState` in its constructor. Call `refreshActiveChainsFromNetworkEnablementState()`, or `AssetsController.init()`. - - `SnapDataSource` no longer calls `SnapController:getRunnableSnaps` in its constructor. Call `init()`. When `SnapController` is not registered, discovery leaves active chains empty. - - `AccountsApiDataSource` no longer starts its supported-network fetch, which calls `RemoteFeatureFlagController:getState`, from the constructor. Call `init()`. -- When `AccountTreeController` actions are not registered, selected-account reads fall back to `AccountsController:getSelectedAccount`, and asset tracking does not wait for account-tree initialization ([#10885](https://github.com/MetaMask/core/pull/10885)) - - `AccountActivityDataSource` uses the same fallback when resolving the account for a balance update. +- **BREAKING:** The `AssetsController` constructor no longer calls messenger actions, creates data sources or subscribes to external events; asset tracking does not run until `start()` is called ([#10885](https://github.com/MetaMask/core/pull/10885)) + - Call `start()` (or the `AssetsController:start` action) after the controllers `AssetsController` depends on are registered. Data sources are created in `start()`, so their constructors still read `NetworkController`, `NetworkEnablementController`, `SnapController`, `PermissionController` and `RemoteFeatureFlagController` as before, only later. + - `subscribeToBasicFunctionalityChange` is invoked by `start()` instead of the constructor. + - Before `start()`, `getAssets` (and the methods built on it) with `forceUpdate: true` return what is already in state without fetching and without changing `assetsLoadingStatus`; `handleAssetsUpdate`, `syncAssetsPrices`, `subscribeAssetsPrice` and `handleBasicFunctionalityChange` do nothing; `setSelectedCurrency` updates state without refreshing prices. ### Fixed -- Refresh the newly selected account after startup completes when it changes while the initial asset fetch is in progress ([#10885](https://github.com/MetaMask/core/pull/10885)) - Stop removing launchpad tokens as spam: EVM ERC-20s below the occurrence floor are now kept when Token API `/v2/assets` identifies them as launchpad tokens and their security data does not classify them as malicious ([#11251](https://github.com/MetaMask/core/pull/11251)) - Applies to metadata enrichment, websocket balance updates, and the unlock spam cleanup. diff --git a/packages/assets-controller/src/AssetsController-method-action-types.ts b/packages/assets-controller/src/AssetsController-method-action-types.ts index f456418eb3b..5914febd083 100644 --- a/packages/assets-controller/src/AssetsController-method-action-types.ts +++ b/packages/assets-controller/src/AssetsController-method-action-types.ts @@ -5,6 +5,47 @@ import type { AssetsController } from './AssetsController.js'; +/** + * Start the controller once every controller it depends on is registered. + * + * The constructor does not touch other controllers. `start()` reads enabled + * networks, creates the data sources (whose constructors read network, snap + * and feature-flag state), subscribes to external events and to + * basic-functionality changes, and begins asset tracking when the UI is + * open, the keyring is unlocked and the account tree is initialized. + * + * Call it after the client has constructed and initialized its own + * controllers, such as `AccountTreeController` and `SnapController`. + * Until then, asset tracking does not run: forced fetches return what is + * already in state, and pushed updates are ignored. + * + * Repeated calls return the same promise. Calls after `destroy()` do + * nothing. + * + * @returns A promise that resolves once the controller has started. + */ +export type AssetsControllerStartAction = { + type: `AssetsController:start`; + handler: AssetsController['start']; +}; + +/** + * Get assets for the given accounts, optionally force-fetching them first. + * + * Before `start()` there are no data sources, so a forced fetch is skipped + * and the assets already in state are returned. The loading status of the + * accounts is left untouched, so the first fetch after `start()` still + * reports them as loading. + * + * @param accounts - Accounts to get assets for. + * @param options - Fetch options. + * @param options.chainIds - Chains to include. Defaults to the enabled chains. + * @param options.assetTypes - Asset types to include. Defaults to fungible. + * @param options.forceUpdate - Fetch from the data sources before reading state. + * @param options.bypassServerCache - Also bypass server-side HTTP caches. + * @param options.dataTypes - Data types to fetch. + * @returns Assets keyed by account ID and asset ID. + */ export type AssetsControllerGetAssetsAction = { type: `AssetsController:getAssets`; handler: AssetsController['getAssets']; @@ -217,6 +258,7 @@ export type AssetsControllerSetSelectedCurrencyAction = { * Union of all AssetsController action types. */ export type AssetsControllerMethodActions = + | AssetsControllerStartAction | AssetsControllerGetAssetsAction | AssetsControllerSyncAssetsPricesAction | AssetsControllerGetAssetsBalanceAction diff --git a/packages/assets-controller/src/AssetsController.bsc-spam-token-filtering.accounts-api-v6.integration.test.ts b/packages/assets-controller/src/AssetsController.bsc-spam-token-filtering.accounts-api-v6.integration.test.ts index 2c5c39b3416..f7416ec8d30 100644 --- a/packages/assets-controller/src/AssetsController.bsc-spam-token-filtering.accounts-api-v6.integration.test.ts +++ b/packages/assets-controller/src/AssetsController.bsc-spam-token-filtering.accounts-api-v6.integration.test.ts @@ -82,7 +82,7 @@ async function withController( queryApiClient, isBasicFunctionality: (): boolean => true, }); - await controller.init(); + await controller.start(); try { return await fn({ controller, messenger: rootMessenger }); diff --git a/packages/assets-controller/src/AssetsController.bsc-spam-token-filtering.integration.test.ts b/packages/assets-controller/src/AssetsController.bsc-spam-token-filtering.integration.test.ts index 8067992b755..8d9cbfaf677 100644 --- a/packages/assets-controller/src/AssetsController.bsc-spam-token-filtering.integration.test.ts +++ b/packages/assets-controller/src/AssetsController.bsc-spam-token-filtering.integration.test.ts @@ -70,7 +70,7 @@ async function withController( queryApiClient, isBasicFunctionality: (): boolean => true, }); - await controller.init(); + await controller.start(); try { return await fn({ controller, messenger: rootMessenger }); diff --git a/packages/assets-controller/src/AssetsController.launchpad-filtering.integration.test.ts b/packages/assets-controller/src/AssetsController.launchpad-filtering.integration.test.ts index 565762604a0..39c9548ea44 100644 --- a/packages/assets-controller/src/AssetsController.launchpad-filtering.integration.test.ts +++ b/packages/assets-controller/src/AssetsController.launchpad-filtering.integration.test.ts @@ -109,7 +109,7 @@ async function withController( queryApiClient, isBasicFunctionality: (): boolean => true, }); - await controller.init(); + await controller.start(); try { return await fn({ controller, messenger: rootMessenger }); diff --git a/packages/assets-controller/src/AssetsController.spam-cleanup.integration.test.ts b/packages/assets-controller/src/AssetsController.spam-cleanup.integration.test.ts index 7d24ff61182..85444f6bcdf 100644 --- a/packages/assets-controller/src/AssetsController.spam-cleanup.integration.test.ts +++ b/packages/assets-controller/src/AssetsController.spam-cleanup.integration.test.ts @@ -102,7 +102,7 @@ async function withController( isBasicFunctionality, captureException, }); - await controller.init(); + await controller.start(); try { return await fn({ controller, messenger: rootMessenger }); diff --git a/packages/assets-controller/src/AssetsController.spam-cleanup.test.ts b/packages/assets-controller/src/AssetsController.spam-cleanup.test.ts index 6e64a006373..087ee81502b 100644 --- a/packages/assets-controller/src/AssetsController.spam-cleanup.test.ts +++ b/packages/assets-controller/src/AssetsController.spam-cleanup.test.ts @@ -120,7 +120,7 @@ async function withController( isBasicFunctionality, captureException, }); - await controller.init(); + await controller.start(); try { return await fn({ controller, messenger: rootMessenger }); diff --git a/packages/assets-controller/src/AssetsController.stale-balance-scenarios.integration.test.ts b/packages/assets-controller/src/AssetsController.stale-balance-scenarios.integration.test.ts index 63e03243524..b6de49e016b 100644 --- a/packages/assets-controller/src/AssetsController.stale-balance-scenarios.integration.test.ts +++ b/packages/assets-controller/src/AssetsController.stale-balance-scenarios.integration.test.ts @@ -142,7 +142,7 @@ async function withStaleBalanceController( queryApiClient, isBasicFunctionality: (): boolean => true, }); - await controller.init(); + await controller.start(); try { // `AccountsApiDataSource` loads `/v2/supportedNetworks` asynchronously diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index 84f780b412e..c6cc53ff2fb 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -29,8 +29,6 @@ import { AccountsApiDataSource } from './data-sources/AccountsApiDataSource.js'; import type { PriceDataSourceConfig } from './data-sources/PriceDataSource.js'; import { PriceDataSource } from './data-sources/PriceDataSource.js'; import { RpcDataSource } from './data-sources/RpcDataSource.js'; -import { SnapDataSource } from './data-sources/SnapDataSource.js'; -import { StakedBalanceDataSource } from './data-sources/StakedBalanceDataSource.js'; import { TokenDataSource } from './data-sources/TokenDataSource.js'; import { buildDefaultAssetsInfo, @@ -218,11 +216,7 @@ const [MOCK_DEFAULT_TRACKED_ASSET_ID] = * * @param messenger - The root messenger used to publish lifecycle events. */ -type LifecyclePublish = ( - topic: string, - payload?: unknown, - secondPayload?: unknown, -) => void; +type LifecyclePublish = (topic: string, payload?: unknown) => void; async function activateTracking(messenger: RootMessenger): Promise { (messenger as unknown as { publish: LifecyclePublish }).publish( @@ -284,6 +278,11 @@ type WithControllerOptions = { * Required for tests that rely on asset tracking running (e.g. trace on unlock). */ clientControllerState?: { isUiOpen: boolean }; + /** + * When true, the controller is constructed but `start()` is not called, so + * no data source exists yet. Defaults to false. + */ + skipStart?: boolean; /** * When set, registers RemoteFeatureFlagController:getState so the controller can * read feature flags (e.g. `assetsAccountsApiV6` gating the balances endpoint). @@ -297,6 +296,9 @@ type WithControllerOptions = { isEnabled: () => boolean; captureException: (error: Error) => void; tempMigrateAssetsInfoMetadataAssets3346: () => Assets3346MigrationState; + subscribeToBasicFunctionalityChange: ( + onChange: (isBasic: boolean) => void, + ) => void | (() => void); }>; }; @@ -330,6 +332,7 @@ async function withController( remoteFeatureFlags = {}, queryApiClient = createMockQueryApiClient(), controllerOptions = {}, + skipStart = false, }, fn, ]: [WithControllerOptions, WithControllerCallback] = @@ -446,7 +449,9 @@ async function withController( ...incomingPriceDataSourceConfig, }, }); - await controller.init(); + if (!skipStart) { + await controller.start(); + } try { return await fn({ controller, messenger, getSelectedAccountsMock }); @@ -753,365 +758,343 @@ describe('AssetsController', () => { ).not.toThrow(); }); - it('reads enabled chains from init and from NetworkEnablementController state changes when getState is not registered at construction', async () => { + it('accepts isBasicFunctionality option and exposes handleBasicFunctionalityChange', async () => { + await withController(async ({ controller }) => { + expect(controller.handleBasicFunctionalityChange).toBeDefined(); + expect(() => + controller.handleBasicFunctionalityChange(true), + ).not.toThrow(); + }); + }); + + it('works with isBasicFunctionality false (RPC-only mode)', async () => { + await withController( + { state: {}, isBasicFunctionality: () => false }, + async ({ controller }) => { + const accounts = [createMockInternalAccount()]; + const assets = await controller.getAssets(accounts, { + forceUpdate: true, + }); + expect(assets).toBeDefined(); + expect(assets[MOCK_ACCOUNT_ID]).toBeDefined(); + expect(() => + controller.handleBasicFunctionalityChange(false), + ).not.toThrow(); + }, + ); + }); + }); + + describe('start', () => { + it('does not call any messenger action during construction', async () => { const messenger: RootMessenger = new Messenger({ namespace: MOCK_ANY_NAMESPACE, }); - messenger.registerActionHandler( - 'AccountTreeController:getAccountsFromSelectedAccountGroup', - () => [], - ); - ( - messenger as { - registerActionHandler: ( - action: string, - handler: () => unknown, - ) => void; - } - ).registerActionHandler('AccountsController:getSelectedAccount', () => - createMockInternalAccount(), - ); - ( - messenger as { - registerActionHandler: ( - action: string, - handler: () => unknown, - ) => void; - } - ).registerActionHandler('NetworkController:getState', () => ({ - networkConfigurationsByChainId: {}, - networksMetadata: {}, - })); - ( - messenger as { - registerActionHandler: ( - action: string, - handler: () => unknown, - ) => void; - } - ).registerActionHandler('NetworkController:getNetworkClientById', () => ({ - provider: {}, - })); + const callSpy = vi.spyOn(messenger, 'call'); - const polygonNativeAssetId = 'eip155:137/slip44:966' as Caip19AssetId; - const nativeMetadata = { - type: 'native' as const, - symbol: 'ETH', - name: 'Ether', - decimals: 18, - }; const controller = new AssetsController({ messenger: messenger as unknown as AssetsControllerMessenger, queryApiClient: createMockQueryApiClient(), - state: { - assetsBalance: { - [MOCK_ACCOUNT_ID]: { - [MOCK_NATIVE_ASSET_ID]: { amount: '1' }, - [polygonNativeAssetId]: { amount: '2' }, - }, - }, - assetsInfo: { - [MOCK_NATIVE_ASSET_ID]: nativeMetadata, - [polygonNativeAssetId]: { - ...nativeMetadata, - symbol: 'POL', - name: 'Polygon', - }, - }, - }, }); + await flushPromises(); - const account = createMockInternalAccount(); - await controller.init(); - const beforeInit = await controller.getAssets([account]); - expect(Object.keys(beforeInit[MOCK_ACCOUNT_ID])).toStrictEqual([]); + expect(callSpy).not.toHaveBeenCalled(); + controller.destroy(); + }); - messenger.registerActionHandler( - 'NetworkEnablementController:getState', - () => ({ - enabledNetworkMap: { - eip155: { '0x1': true }, - }, - nativeAssetIdentifiers: { - 'eip155:1': 'eip155:1/slip44:60', - }, - }), - ); - await controller.init(); + it('does not invoke subscribeToBasicFunctionalityChange until started', async () => { + const subscribeToBasicFunctionalityChange = vi.fn(); + await withController( + { + skipStart: true, + controllerOptions: { subscribeToBasicFunctionalityChange }, + }, + async ({ controller }) => { + expect(subscribeToBasicFunctionalityChange).not.toHaveBeenCalled(); - const afterInit = await controller.getAssets([account]); - expect(Object.keys(afterInit[MOCK_ACCOUNT_ID])).toStrictEqual([ - MOCK_NATIVE_ASSET_ID, - ]); + await controller.start(); - (messenger as unknown as { publish: LifecyclePublish }).publish( - 'NetworkEnablementController:stateChange', + expect(subscribeToBasicFunctionalityChange).toHaveBeenCalledTimes(1); + }, + ); + }); + + it('refreshes subscriptions on basic functionality changes and unsubscribes on destroy', async () => { + let notifyBasicFunctionalityChange: + | ((isBasic: boolean) => void) + | undefined; + const unsubscribe = vi.fn(); + await withController( { - enabledNetworkMap: { - eip155: { '0x1': true, '0x89': true }, - }, - nativeAssetIdentifiers: { - 'eip155:1': 'eip155:1/slip44:60', - 'eip155:137': 'eip155:137/slip44:966', + controllerOptions: { + subscribeToBasicFunctionalityChange: (onChange) => { + notifyBasicFunctionalityChange = onChange; + return unsubscribe; + }, }, }, - ); - await new Promise(process.nextTick); + async ({ controller }) => { + const handleSpy = vi.spyOn( + controller, + 'handleBasicFunctionalityChange', + ); - const afterStateChange = await controller.getAssets([account]); - expect( - Object.keys(afterStateChange[MOCK_ACCOUNT_ID]).sort(), - ).toStrictEqual([MOCK_NATIVE_ASSET_ID, polygonNativeAssetId].sort()); - }); + notifyBasicFunctionalityChange?.(false); + expect(handleSpy).toHaveBeenCalledWith(false); - it('refreshes data source active chains from init', async () => { - const refreshRpcSpy = vi.spyOn( - RpcDataSource.prototype, - 'refreshActiveChainsFromNetworkState', - ); - const refreshStakedSpy = vi.spyOn( - StakedBalanceDataSource.prototype, - 'refreshActiveChainsFromNetworkEnablementState', + controller.destroy(); + expect(unsubscribe).toHaveBeenCalledTimes(1); + }, ); - const snapInitSpy = vi.spyOn(SnapDataSource.prototype, 'init'); - const accountsApiInitSpy = vi.spyOn( - AccountsApiDataSource.prototype, - 'init', + }); + + it('does not start tracking when destroyed while starting', async () => { + await withController( + { + skipStart: true, + clientControllerState: { isUiOpen: true }, + }, + async ({ controller, messenger }) => { + messenger.publish('AccountTreeController:initialized', {} as never); + messenger.publish('KeyringController:unlock'); + const getAssetsSpy = vi.spyOn(controller, 'getAssets'); + + const startPromise = controller.start(); + controller.destroy(); + await startPromise; + await flushPromises(); + + expect(getAssetsSpy).not.toHaveBeenCalled(); + }, ); + }); - try { - await withController(async ({ controller }) => { - refreshRpcSpy.mockClear(); - refreshStakedSpy.mockClear(); - snapInitSpy.mockClear(); - accountsApiInitSpy.mockClear(); + it('reads other controllers only once started', async () => { + await withController( + { skipStart: true }, + async ({ controller, messenger }) => { + const callSpy = vi.spyOn(messenger, 'call'); - await controller.init(); + await controller.start(); - expect(refreshRpcSpy).toHaveBeenCalledTimes(1); - expect(refreshStakedSpy).toHaveBeenCalledTimes(1); - expect(snapInitSpy).toHaveBeenCalledTimes(1); - expect(accountsApiInitSpy).toHaveBeenCalledTimes(1); - }); - } finally { - refreshRpcSpy.mockRestore(); - refreshStakedSpy.mockRestore(); - snapInitSpy.mockRestore(); - accountsApiInitSpy.mockRestore(); - } + const calledActions = callSpy.mock.calls.map(([action]) => action); + expect(calledActions).toContain( + 'NetworkEnablementController:getState', + ); + expect(calledActions).toContain('NetworkController:getState'); + }, + ); }); - it('does not call messenger actions in the constructor', async () => { - const messenger: RootMessenger = new Messenger({ - namespace: MOCK_ANY_NAMESPACE, - }); - messenger.registerActionHandler( - 'NetworkEnablementController:getState', - () => ({ - enabledNetworkMap: { eip155: { '0x1': true } }, - nativeAssetIdentifiers: {}, - }), + it('is idempotent', async () => { + await withController( + { skipStart: true }, + async ({ controller, messenger }) => { + const callSpy = vi.spyOn(messenger, 'call'); + + await Promise.all([controller.start(), controller.start()]); + const callCountAfterFirstStart = callSpy.mock.calls.length; + await controller.start(); + + expect(callSpy.mock.calls).toHaveLength(callCountAfterFirstStart); + }, ); - ( - messenger as { - registerActionHandler: ( - action: string, - handler: () => unknown, - ) => void; - } - ).registerActionHandler('NetworkController:getState', () => ({ - networkConfigurationsByChainId: {}, - networksMetadata: {}, - })); - ( - messenger as { - registerActionHandler: ( - action: string, - handler: () => unknown, - ) => void; - } - ).registerActionHandler('RemoteFeatureFlagController:getState', () => ({ - remoteFeatureFlags: {}, - cacheTimestamp: 0, - })); + }); - const callSpy = vi.spyOn(messenger, 'call'); - const controller = new AssetsController({ - messenger: messenger as unknown as AssetsControllerMessenger, - queryApiClient: createMockQueryApiClient(), + it('is exposed as the AssetsController:start messenger action', async () => { + await withController({ skipStart: true }, async ({ messenger }) => { + const callSpy = vi.spyOn(messenger, 'call'); + + await messenger.call('AssetsController:start'); + + expect(callSpy.mock.calls.map(([action]) => action)).toContain( + 'NetworkEnablementController:getState', + ); }); + }); - expect(callSpy).not.toHaveBeenCalled(); + it('does nothing after the controller is destroyed', async () => { + await withController( + { skipStart: true }, + async ({ controller, messenger }) => { + controller.destroy(); + const callSpy = vi.spyOn(messenger, 'call'); - await controller.init(); + await controller.start(); - expect(callSpy).toHaveBeenCalledWith( - 'NetworkEnablementController:getState', + expect(callSpy).not.toHaveBeenCalled(); + }, ); - expect(callSpy).toHaveBeenCalledWith('NetworkController:getState'); + }); - controller.destroy(); + it('ignores external events published before start', async () => { + await withController( + { skipStart: true, clientControllerState: { isUiOpen: true } }, + async ({ messenger }) => { + const callSpy = vi.spyOn(messenger, 'call'); + + messenger.publish('AccountTreeController:initialized', {} as never); + messenger.publish('KeyringController:unlock'); + await flushPromises(); + + expect(callSpy.mock.calls.map(([action]) => action)).not.toContain( + 'AccountTreeController:getAccountsFromSelectedAccountGroup', + ); + }, + ); }); - it('tracks the selected account when AccountTreeController and SnapController are absent', async () => { - const messenger: RootMessenger = new Messenger({ - namespace: MOCK_ANY_NAMESPACE, - }); - const account = createMockInternalAccount(); - registerKeyringUnlockMock(messenger); - ( - messenger as { - registerActionHandler: ( - action: string, - handler: () => unknown, - ) => void; - } - ).registerActionHandler( - 'AccountsController:getSelectedAccount', - () => account, + it('starts tracking when the UI is open, the keyring unlocked and the account tree initialized', async () => { + await withController( + { skipStart: true, clientControllerState: { isUiOpen: true } }, + async ({ controller, messenger }) => { + messenger.publish('AccountTreeController:initialized', {} as never); + messenger.publish('KeyringController:unlock'); + await flushPromises(); + const getAssetsSpy = vi.spyOn(controller, 'getAssets'); + + await controller.start(); + await flushPromises(); + + expect(getAssetsSpy).toHaveBeenCalledWith( + [expect.objectContaining({ id: MOCK_ACCOUNT_ID })], + expect.objectContaining({ forceUpdate: true }), + ); + }, ); - messenger.registerActionHandler( - 'NetworkEnablementController:getState', - () => ({ - enabledNetworkMap: { eip155: { '0x1': true } }, - nativeAssetIdentifiers: { - 'eip155:1': 'eip155:1/slip44:60', + }); + + it('drops balances outside account scopes on start when already unlocked', async () => { + const solanaAssetId = + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501' as Caip19AssetId; + await withController( + { + skipStart: true, + state: { + assetsBalance: { + [MOCK_ACCOUNT_ID]: { + [MOCK_NATIVE_ASSET_ID]: { amount: '1' }, + [solanaAssetId]: { amount: '2' }, + }, + }, }, - }), + }, + async ({ controller, messenger }) => { + messenger.publish('KeyringController:unlock'); + + await controller.start(); + + expect( + controller.state.assetsBalance[MOCK_ACCOUNT_ID][solanaAssetId], + ).toBeUndefined(); + expect( + controller.state.assetsBalance[MOCK_ACCOUNT_ID][ + MOCK_NATIVE_ASSET_ID + ], + ).toStrictEqual({ amount: '1' }); + }, ); - ( - messenger as { - registerActionHandler: ( - action: string, - handler: () => unknown, - ) => void; - } - ).registerActionHandler('NetworkController:getState', () => ({ - networkConfigurationsByChainId: {}, - networksMetadata: {}, - })); - ( - messenger as { - registerActionHandler: ( - action: string, - handler: () => unknown, - ) => void; - } - ).registerActionHandler('ClientController:getState', () => ({ - isUiOpen: true, - })); - ( - messenger as { - registerActionHandler: ( - action: string, - handler: () => unknown, - ) => void; - } - ).registerActionHandler('RemoteFeatureFlagController:getState', () => ({ - remoteFeatureFlags: {}, - cacheTimestamp: 0, - })); + }); - const controller = new AssetsController({ - messenger: messenger as unknown as AssetsControllerMessenger, - queryApiClient: createMockQueryApiClient(), + describe('before start', () => { + it('returns assets from state without fetching on a forced getAssets', async () => { + const queryApiClient = createMockQueryApiClient(); + await withController( + { + skipStart: true, + queryApiClient, + state: { + assetsInfo: { + [MOCK_NATIVE_ASSET_ID]: { + type: 'native', + symbol: 'ETH', + name: 'Ether', + decimals: 18, + }, + }, + assetsBalance: { + [MOCK_ACCOUNT_ID]: { + [MOCK_NATIVE_ASSET_ID]: { amount: '1' }, + }, + }, + }, + }, + async ({ controller }) => { + const assets = await controller.getAssets( + [createMockInternalAccount()], + { chainIds: ['eip155:1'], forceUpdate: true }, + ); + + expect( + assets[MOCK_ACCOUNT_ID][MOCK_NATIVE_ASSET_ID].balance, + ).toStrictEqual({ amount: '1' }); + }, + ); }); - await controller.init(); - - const getAssetsSpy = vi.spyOn(controller, 'getAssets'); - messenger.publish('KeyringController:unlock'); - await flushPromises(); - expect(getAssetsSpy).toHaveBeenCalledWith( - [expect.objectContaining({ id: account.id })], - expect.objectContaining({ forceUpdate: true }), - ); + it('ignores pushed asset updates', async () => { + await withController({ skipStart: true }, async ({ controller }) => { + await controller.handleAssetsUpdate( + { + assetsBalance: { + [MOCK_ACCOUNT_ID]: { [MOCK_NATIVE_ASSET_ID]: { amount: '5' } }, + }, + }, + 'AccountsApiDataSource', + ); - controller.destroy(); - }); + expect(controller.state.assetsBalance).toStrictEqual({}); + }); + }); - it('rethrows AccountTreeController failures that are not a missing handler', async () => { - await withController(async ({ controller, messenger }) => { - messenger.unregisterActionHandler( - 'AccountTreeController:getAccountsFromSelectedAccountGroup', - ); - messenger.registerActionHandler( - 'AccountTreeController:getAccountsFromSelectedAccountGroup', - () => { - throw new Error('account tree failed'); - }, - ); + it('does not fetch prices on syncAssetsPrices', async () => { + await withController({ skipStart: true }, async ({ controller }) => { + await controller.syncAssetsPrices([MOCK_NATIVE_ASSET_ID]); - expect(() => controller.getStateForTransactionPay()).toThrow( - 'account tree failed', - ); + expect(controller.state.assetsPrice).toStrictEqual({}); + }); }); - }); - it('rethrows AccountTreeController:isInitialized failures that are not a missing handler', async () => { - await withController( - { clientControllerState: { isUiOpen: true } }, - async ({ messenger }) => { - messenger.publish('KeyringController:unlock'); + it('updates the selected currency without fetching prices', async () => { + await withController({ skipStart: true }, async ({ controller }) => { + controller.setSelectedCurrency('eur'); await flushPromises(); - messenger.unregisterActionHandler( - 'AccountTreeController:isInitialized', - ); - messenger.registerActionHandler( - 'AccountTreeController:isInitialized', - () => { - throw new Error('account tree init failed'); - }, - ); + expect(controller.state.selectedCurrency).toBe('eur'); + }); + }); - const consoleError = vi - .spyOn(console, 'error') - .mockImplementation(() => undefined); - try { - (messenger as unknown as { publish: LifecyclePublish }).publish( - 'ClientController:stateChange', - { isUiOpen: true }, - [], + it('does not throw on subscription and basic functionality calls', async () => { + await withController({ skipStart: true }, async ({ controller }) => { + expect(() => { + controller.subscribeAssetsPrice( + [createMockInternalAccount()], + ['eip155:1'], ); + controller.unsubscribeAssetsPrice(); + controller.handleBasicFunctionalityChange(true); + }).not.toThrow(); + }); + }); - expect(consoleError).toHaveBeenCalledWith( - expect.objectContaining({ - message: 'account tree init failed', - }), + it('ignores data source active chain updates', async () => { + await withController( + { skipStart: true, clientControllerState: { isUiOpen: true } }, + async ({ controller, messenger }) => { + messenger.publish('KeyringController:unlock'); + const callSpy = vi.spyOn(messenger, 'call'); + + controller.getOnActiveChainsUpdated()( + 'RpcDataSource', + ['eip155:1'], + [], ); - } finally { - consoleError.mockRestore(); - } - }, - ); - }); - it('accepts isBasicFunctionality option and exposes handleBasicFunctionalityChange', async () => { - await withController(async ({ controller }) => { - expect(controller.handleBasicFunctionalityChange).toBeDefined(); - expect(() => - controller.handleBasicFunctionalityChange(true), - ).not.toThrow(); + expect(callSpy).not.toHaveBeenCalled(); + }, + ); }); }); - - it('works with isBasicFunctionality false (RPC-only mode)', async () => { - await withController( - { state: {}, isBasicFunctionality: () => false }, - async ({ controller }) => { - const accounts = [createMockInternalAccount()]; - const assets = await controller.getAssets(accounts, { - forceUpdate: true, - }); - expect(assets).toBeDefined(); - expect(assets[MOCK_ACCOUNT_ID]).toBeDefined(); - expect(() => - controller.handleBasicFunctionalityChange(false), - ).not.toThrow(); - }, - ); - }); }); describe('addCustomAsset', () => { @@ -5839,7 +5822,7 @@ describe('AssetsController', () => { /* no-op */ }, }); - await controller.init(); + await controller.start(); const getAssetsSpy = vi.spyOn(controller, 'getAssets'); @@ -5979,7 +5962,7 @@ describe('AssetsController', () => { /* no-op */ }, }); - await controller.init(); + await controller.start(); let nativePresentAtPriceFetch: boolean | undefined; const fetchSpy = vi @@ -6036,43 +6019,6 @@ describe('AssetsController', () => { }); describe('account group changes', () => { - it('refreshes the latest group when it changes during startup', async () => { - const { client, arm, release, getGatedCallCount } = - createGatedQueryApiClient(); - - await withController( - { queryApiClient: client }, - async ({ controller, messenger, getSelectedAccountsMock }) => { - const accountA = createMockInternalAccount({ id: 'account-a' }); - const accountB = createMockInternalAccount({ id: 'account-b' }); - getSelectedAccountsMock.mockReturnValue([accountA]); - const getAssetsSpy = vi.spyOn(controller, 'getAssets'); - - arm(); - await activateTracking(messenger); - await waitFor(() => expect(getGatedCallCount()).toBeGreaterThan(0)); - - getSelectedAccountsMock.mockReturnValue([accountB]); - messenger.publish( - 'AccountTreeController:selectedAccountGroupChange', - 'entropy:mock-keyring-id-1/1', - 'entropy:mock-keyring-id-1/0', - ); - - release(); - - await waitFor(() => - expect(getAssetsSpy).toHaveBeenCalledWith( - [expect.objectContaining({ id: accountB.id })], - expect.objectContaining({ forceUpdate: true }), - ), - ); - expect(getAssetsSpy).toHaveBeenCalledTimes(2); - getAssetsSpy.mockRestore(); - }, - ); - }); - it('refreshes assets when the selected group changes while tracking', async () => { await withController(async ({ controller, messenger }) => { const getAssetsSpy = vi @@ -6166,232 +6112,6 @@ describe('AssetsController', () => { getAssetsSpy.mockRestore(); }); }); - - it('ignores AccountsController:selectedAccountChange while AccountTreeController is registered', async () => { - await withController(async ({ controller, messenger }) => { - const getAssetsSpy = vi - .spyOn(controller, 'getAssets') - .mockResolvedValue({}); - - await activateTracking(messenger); - getAssetsSpy.mockClear(); - - messenger.publish( - 'AccountsController:selectedAccountChange', - createMockInternalAccount({ id: 'mock-account-id-2' }), - ); - await flushPromises(); - - expect(getAssetsSpy).not.toHaveBeenCalled(); - - messenger.publish( - 'AccountTreeController:selectedAccountGroupChange', - 'entropy:mock-keyring-id-1/1', - 'entropy:mock-keyring-id-1/0', - ); - await flushPromises(); - - expect(getAssetsSpy).toHaveBeenCalledTimes(1); - getAssetsSpy.mockRestore(); - }); - }); - - it('does not refresh again when selectedAccountChange follows a group switch', async () => { - await withController(async ({ controller, messenger }) => { - const getAssetsSpy = vi - .spyOn(controller, 'getAssets') - .mockResolvedValue({}); - - await activateTracking(messenger); - getAssetsSpy.mockClear(); - - messenger.publish( - 'AccountTreeController:selectedAccountGroupChange', - 'entropy:mock-keyring-id-1/1', - 'entropy:mock-keyring-id-1/0', - ); - messenger.publish( - 'AccountsController:selectedAccountChange', - createMockInternalAccount({ id: 'mock-account-id-2' }), - ); - await flushPromises(); - - expect(getAssetsSpy).toHaveBeenCalledTimes(1); - getAssetsSpy.mockRestore(); - }); - }); - - it('does not treat AccountTreeController:isInitialized failures as a missing controller', async () => { - await withController(async ({ controller, messenger }) => { - const getAssetsSpy = vi - .spyOn(controller, 'getAssets') - .mockResolvedValue({}); - - await activateTracking(messenger); - getAssetsSpy.mockClear(); - - messenger.unregisterActionHandler( - 'AccountTreeController:isInitialized', - ); - messenger.registerActionHandler( - 'AccountTreeController:isInitialized', - () => { - throw new Error('account tree init failed'); - }, - ); - - const consoleError = vi - .spyOn(console, 'error') - .mockImplementation(() => undefined); - try { - messenger.publish( - 'AccountsController:selectedAccountChange', - createMockInternalAccount({ id: 'mock-account-id-2' }), - ); - await flushPromises(); - - expect(getAssetsSpy).not.toHaveBeenCalled(); - expect(consoleError).toHaveBeenCalledWith( - expect.objectContaining({ - message: 'account tree init failed', - }), - ); - } finally { - consoleError.mockRestore(); - } - getAssetsSpy.mockRestore(); - }); - }); - }); - - describe('selected account changes without AccountTreeController', () => { - /** - * Drops AccountTreeController actions and serves the selected account from - * AccountsController, matching a host that has not installed the tree. - * - * @param messenger - Root messenger from `withController`. - * @param selectedAccount - Account returned by `getSelectedAccount`. - * @returns A setter for the account served on later reads. - */ - function useAccountsControllerSelection( - messenger: RootMessenger, - selectedAccount: InternalAccount, - ): (account: InternalAccount) => void { - messenger.unregisterActionHandler( - 'AccountTreeController:getAccountsFromSelectedAccountGroup', - ); - messenger.unregisterActionHandler('AccountTreeController:isInitialized'); - - let current = selectedAccount; - messenger.unregisterActionHandler( - 'AccountsController:getSelectedAccount', - ); - ( - messenger as { - registerActionHandler: ( - action: string, - handler: () => InternalAccount, - ) => void; - } - ).registerActionHandler( - 'AccountsController:getSelectedAccount', - () => current, - ); - - return (account: InternalAccount) => { - current = account; - }; - } - - it('refreshes tracked assets when the selected account changes', async () => { - await withController(async ({ controller, messenger }) => { - const setSelectedAccount = useAccountsControllerSelection( - messenger, - createMockInternalAccount({ id: 'account-a' }), - ); - const getAssetsSpy = vi - .spyOn(controller, 'getAssets') - .mockResolvedValue({}); - - await activateTracking(messenger); - getAssetsSpy.mockClear(); - - const nextAccount = createMockInternalAccount({ id: 'account-b' }); - setSelectedAccount(nextAccount); - messenger.publish( - 'AccountsController:selectedAccountChange', - nextAccount, - ); - await flushPromises(); - - expect(getAssetsSpy).toHaveBeenCalledTimes(1); - expect(getAssetsSpy).toHaveBeenCalledWith( - [expect.objectContaining({ id: 'account-b' })], - expect.objectContaining({ forceUpdate: true }), - ); - getAssetsSpy.mockRestore(); - }); - }); - - it('refreshes the latest account when it changes during startup', async () => { - const { client, arm, release, getGatedCallCount } = - createGatedQueryApiClient(); - - await withController( - { queryApiClient: client }, - async ({ controller, messenger }) => { - const accountA = createMockInternalAccount({ id: 'account-a' }); - const accountB = createMockInternalAccount({ id: 'account-b' }); - const setSelectedAccount = useAccountsControllerSelection( - messenger, - accountA, - ); - const getAssetsSpy = vi.spyOn(controller, 'getAssets'); - - arm(); - await activateTracking(messenger); - await waitFor(() => expect(getGatedCallCount()).toBeGreaterThan(0)); - - setSelectedAccount(accountB); - messenger.publish( - 'AccountsController:selectedAccountChange', - accountB, - ); - - release(); - - await waitFor(() => - expect(getAssetsSpy).toHaveBeenCalledWith( - [expect.objectContaining({ id: accountB.id })], - expect.objectContaining({ forceUpdate: true }), - ), - ); - expect(getAssetsSpy).toHaveBeenCalledTimes(2); - getAssetsSpy.mockRestore(); - }, - ); - }); - - it('does not refresh a selected-account change before tracking starts', async () => { - await withController(async ({ controller, messenger }) => { - useAccountsControllerSelection( - messenger, - createMockInternalAccount({ id: 'account-a' }), - ); - const getAssetsSpy = vi - .spyOn(controller, 'getAssets') - .mockResolvedValue({}); - - messenger.publish( - 'AccountsController:selectedAccountChange', - createMockInternalAccount({ id: 'account-b' }), - ); - await flushPromises(); - - expect(getAssetsSpy).not.toHaveBeenCalled(); - getAssetsSpy.mockRestore(); - }); - }); }); describe('assets loading status', () => { @@ -6760,7 +6480,7 @@ describe('AssetsController', () => { /* no-op */ }, }); - await controller.init(); + await controller.start(); const getAssetsSpy = vi.spyOn(controller, 'getAssets'); diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index e0249cc2aef..2f7540800dc 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -5,10 +5,7 @@ import type { AccountTreeControllerSelectedAccountGroupChangeEvent, AccountTreeControllerUninitializedEvent, } from '@metamask/account-tree-controller'; -import type { - AccountsControllerGetSelectedAccountAction, - AccountsControllerSelectedAccountChangeEvent, -} from '@metamask/accounts-controller'; +import type { AccountsControllerGetSelectedAccountAction } from '@metamask/accounts-controller'; import { BaseController } from '@metamask/base-controller'; import type { ControllerGetStateAction, @@ -210,6 +207,7 @@ const CONTROLLER_NAME = 'AssetsController' as const; /** Method names exposed as messenger actions (AssetsController:getAssets, etc.) */ const MESSENGER_EXPOSED_METHODS = [ + 'start', 'getAssets', 'getAssetsBalance', 'getAssetMetadata', @@ -339,15 +337,10 @@ type AllowedActions = | RemoteFeatureFlagControllerGetStateAction; type AllowedEvents = - // AssetsController — account tree lifecycle and group switches. - // `AccountsController:selectedAccountChange` is the fallback when - // `AccountTreeController` is not registered. It is ignored while that - // controller is registered, because the tree republishes the switch as - // `selectedAccountGroupChange`. + // AssetsController — account tree lifecycle and group switches | AccountTreeControllerSelectedAccountGroupChangeEvent | AccountTreeControllerInitializedEvent | AccountTreeControllerUninitializedEvent - | AccountsControllerSelectedAccountChangeEvent | ClientControllerStateChangeEvent | KeyringControllerLockEvent | KeyringControllerUnlockEvent @@ -705,6 +698,37 @@ function effectiveAccountBalancesV6( * - Data sources are responsible for their own update mechanisms (WebSocket, polling, events) * - The controller does NOT manage polling - it simply receives pushed updates */ +/** + * Data sources and middlewares created by {@link AssetsController.start}. + */ +type AssetsControllerSources = { + accountActivityDataSource: AccountActivityDataSource; + accountsApiDataSource: AccountsApiDataSource; + snapDataSource: SnapDataSource; + rpcDataSource: RpcDataSource; + stakedBalanceDataSource: StakedBalanceDataSource; + tokenDataSource: TokenDataSource; + priceDataSource: PriceDataSource; + customAssetGraduationMiddleware: CustomAssetGraduationMiddleware; + rpcFallbackMiddleware: RpcFallbackMiddleware; +}; + +/** + * Constructor options kept until {@link AssetsController.start} creates the + * data sources. + */ +type AssetsControllerSourceOptions = Pick< + AssetsControllerOptions, + | 'queryApiClient' + | 'accountsApiDataSourceConfig' + | 'priceDataSourceConfig' + | 'stakedBalanceDataSourceConfig' + | 'isOnboarded' + | 'subscribeToBasicFunctionalityChange' +> & { + rpcDataSourceConfig: RpcDataSourceConfig; +}; + export class AssetsController extends BaseController< typeof CONTROLLER_NAME, AssetsControllerState, @@ -815,14 +839,6 @@ export class AssetsController extends BaseController< */ #startInFlight = false; - /** - * Whether an account switch occurred while startup was force-fetching. - * - * The startup subscription skips its initial Accounts API fetch, so a - * selection made in that window must be refreshed once startup completes. - */ - #pendingAccountRefresh = false; - /** Currently enabled chains from NetworkEnablementController */ #enabledChains: Set = new Set(); @@ -831,25 +847,15 @@ export class AssetsController extends BaseController< * This includes all accounts in the same group as the selected account * (EVM, Bitcoin, Solana, Tron, etc. that belong to the same logical account group). * - * When `AccountTreeController` is not registered, falls back to - * `AccountsController:getSelectedAccount`. - * * @returns Array of InternalAccount objects from the selected account group. */ #getSelectedAccounts(): InternalAccount[] { - try { - const accounts = this.messenger.call( - 'AccountTreeController:getAccountsFromSelectedAccountGroup', - ); - if (accounts.length > 0) { - return accounts; - } - } catch (error) { - if (!isUnregisteredActionError(error)) { - throw error; - } + const accounts = this.messenger.call( + 'AccountTreeController:getAccountsFromSelectedAccountGroup', + ); + if (accounts.length > 0) { + return accounts; } - const selectedAccount = this.messenger.call( 'AccountsController:getSelectedAccount', ); @@ -862,42 +868,10 @@ export class AssetsController extends BaseController< /** * Whether `AccountTreeController.init()` has completed and the tree is ready. * - * When `AccountTreeController` is not registered, returns true so tracking - * is not blocked on a controller the host has not installed. - * - * @returns True when the account tree has been initialized, or when that controller is absent. + * @returns True when the account tree has been initialized. */ #isAccountTreeInitialized(): boolean { - try { - return this.messenger.call('AccountTreeController:isInitialized'); - } catch (error) { - if (!isUnregisteredActionError(error)) { - throw error; - } - return true; - } - } - - /** - * Whether `AccountTreeController` has registered its actions. - * - * Selected-account switches are published by `AccountsController` in both - * setups. When the tree controller is registered it turns those switches - * into `selectedAccountGroupChange`, so handling both events would refresh - * tracked assets twice. - * - * @returns True when `AccountTreeController:isInitialized` is registered. - */ - #isAccountTreeControllerRegistered(): boolean { - try { - this.messenger.call('AccountTreeController:isInitialized'); - return true; - } catch (error) { - if (!isUnregisteredActionError(error)) { - throw error; - } - return false; - } + return this.messenger.call('AccountTreeController:isInitialized'); } /** @@ -920,15 +894,57 @@ export class AssetsController extends BaseController< return this.messenger.call('KeyringController:isUnlocked'); } - readonly #accountActivityDataSource: AccountActivityDataSource; + /** + * Data sources and middlewares. Created by {@link AssetsController.start}, + * not by the constructor: their constructors read other controllers + * (network, snaps, feature flags), which may not be registered yet when this + * controller is constructed. + */ + #sources: AssetsControllerSources | undefined; + + /** Options forwarded to the data sources when `start()` creates them. */ + readonly #sourceOptions: AssetsControllerSourceOptions; + + /** The pending or settled `start()` call, so repeated calls share it. */ + #startPromise: Promise | undefined; + + /** Whether `destroy()` has been called. */ + #isDestroyed = false; + + /** + * Get the data sources, throwing if `start()` has not created them yet. + * Entry points that can run before `start()` check `#sources` first. + * + * @returns The data sources and middlewares. + */ + #getSources(): AssetsControllerSources { + if (!this.#sources) { + throw new Error( + 'AssetsController has not been started. Call `start()` first.', + ); + } + return this.#sources; + } + + get #accountActivityDataSource(): AccountActivityDataSource { + return this.#getSources().accountActivityDataSource; + } - readonly #accountsApiDataSource: AccountsApiDataSource; + get #accountsApiDataSource(): AccountsApiDataSource { + return this.#getSources().accountsApiDataSource; + } - readonly #snapDataSource: SnapDataSource; + get #snapDataSource(): SnapDataSource { + return this.#getSources().snapDataSource; + } - readonly #rpcDataSource: RpcDataSource; + get #rpcDataSource(): RpcDataSource { + return this.#getSources().rpcDataSource; + } - readonly #stakedBalanceDataSource: StakedBalanceDataSource; + get #stakedBalanceDataSource(): StakedBalanceDataSource { + return this.#getSources().stakedBalanceDataSource; + } /** * All balance data sources in priority order for chain-claiming and cleanup. @@ -951,13 +967,21 @@ export class AssetsController extends BaseController< ]; } - readonly #priceDataSource: PriceDataSource; + get #priceDataSource(): PriceDataSource { + return this.#getSources().priceDataSource; + } - readonly #customAssetGraduationMiddleware: CustomAssetGraduationMiddleware; + get #customAssetGraduationMiddleware(): CustomAssetGraduationMiddleware { + return this.#getSources().customAssetGraduationMiddleware; + } - readonly #rpcFallbackMiddleware: RpcFallbackMiddleware; + get #rpcFallbackMiddleware(): RpcFallbackMiddleware { + return this.#getSources().rpcFallbackMiddleware; + } - readonly #tokenDataSource: TokenDataSource; + get #tokenDataSource(): TokenDataSource { + return this.#getSources().tokenDataSource; + } #unsubscribeBasicFunctionality: (() => void) | null = null; @@ -1033,17 +1057,117 @@ export class AssetsController extends BaseController< } }; - this.#accountActivityDataSource = new AccountActivityDataSource({ - messenger: this.messenger, - onActiveChainsUpdated: this.#onActiveChainsUpdated, - onAssetsUpdate: (response, request): Promise => - this.handleAssetsUpdate( - response, - this.#accountActivityDataSource.getName(), - request, - ), + this.#sourceOptions = { + queryApiClient, + rpcDataSourceConfig: rpcConfig, + accountsApiDataSourceConfig, + priceDataSourceConfig, + stakedBalanceDataSourceConfig, + isOnboarded, + subscribeToBasicFunctionalityChange, + }; + + log('Initializing AssetsController', { + defaultUpdateInterval: this.#defaultUpdateInterval, }); - this.#accountsApiDataSource = new AccountsApiDataSource({ + + // The constructor does not call messenger actions, so construction does + // not depend on the order in which controllers are constructed. Reading + // other controllers, creating data sources and subscribing to events all + // happen in `start()`. + this.#registerActionHandlers(); + } + + // ============================================================================ + // INITIALIZATION + // ============================================================================ + + /** + * Start the controller once every controller it depends on is registered. + * + * The constructor does not touch other controllers. `start()` reads enabled + * networks, creates the data sources (whose constructors read network, snap + * and feature-flag state), subscribes to external events and to + * basic-functionality changes, and begins asset tracking when the UI is + * open, the keyring is unlocked and the account tree is initialized. + * + * Call it after the client has constructed and initialized its own + * controllers, such as `AccountTreeController` and `SnapController`. + * Until then, asset tracking does not run: forced fetches return what is + * already in state, and pushed updates are ignored. + * + * Repeated calls return the same promise. Calls after `destroy()` do + * nothing. + * + * @returns A promise that resolves once the controller has started. + */ + async start(): Promise { + this.#startPromise ??= this.#runStart(); + return this.#startPromise; + } + + async #runStart(): Promise { + if (this.#isDestroyed) { + return; + } + + log('Starting AssetsController'); + + this.#initializeState(); + this.#sources = this.#createSources(); + this.#subscribeToEvents(); + this.#subscribeToBasicFunctionality(); + + // The keyring may already be unlocked when `start()` runs, in which case + // the `KeyringController:unlock` handler that runs these cleanups was not + // subscribed in time. + if (this.#isKeyringUnlocked()) { + try { + this.#runOutOfScopeBalanceCleanup(); + } catch { + /* Do nothing */ + } + await this.#runSpamCleanup(); + } + + if (this.#isDestroyed) { + return; + } + this.#updateActive(); + } + + /** + * Create every data source and middleware. + * + * Data source constructors read other controllers and may report active + * chains synchronously. `#handleActiveChainsUpdate` ignores those reports + * until `#sources` is assigned, and `start()` subscribes afterwards with + * every source's chains. + * + * @returns The data sources and middlewares. + */ + #createSources(): AssetsControllerSources { + const { + queryApiClient, + rpcDataSourceConfig, + accountsApiDataSourceConfig, + priceDataSourceConfig, + stakedBalanceDataSourceConfig, + isOnboarded, + } = this.#sourceOptions; + + const accountActivityDataSource: AccountActivityDataSource = + new AccountActivityDataSource({ + messenger: this.messenger, + onActiveChainsUpdated: this.#onActiveChainsUpdated, + onAssetsUpdate: (response, request): Promise => + this.handleAssetsUpdate( + response, + accountActivityDataSource.getName(), + request, + ), + }); + const accountsApiDataSource = new AccountsApiDataSource({ messenger: this.messenger, queryApiClient, onActiveChainsUpdated: this.#onActiveChainsUpdated, @@ -1052,7 +1176,7 @@ export class AssetsController extends BaseController< getAssetsState: (): AssetsControllerState => this.state, getAssetVisibility: this.#getAssetVisibility.bind(this), }); - this.#snapDataSource = new SnapDataSource({ + const snapDataSource = new SnapDataSource({ messenger: this.messenger, onActiveChainsUpdated: this.#onActiveChainsUpdated, onAssetsUpdate: (response): Promise => @@ -1061,7 +1185,7 @@ export class AssetsController extends BaseController< getAssetVisibility: this.#getAssetVisibility.bind(this), getAssetsState: (): AssetsControllerState => this.state, }); - this.#rpcDataSource = new RpcDataSource({ + const rpcDataSource = new RpcDataSource({ messenger: this.messenger, getAssetsState: (): AssetsControllerState => this.state, isBalanceV6Enabled: (): boolean => this.#isBalanceV6Enabled(), @@ -1070,17 +1194,17 @@ export class AssetsController extends BaseController< getNativeAssetForChain: (chainId: ChainId): Caip19AssetId | undefined => this.#getNativeAssetForChain(chainId), queryApiClient, - ...rpcConfig, - isOnboarded: rpcConfig.isOnboarded ?? isOnboarded, + ...rpcDataSourceConfig, + isOnboarded: rpcDataSourceConfig.isOnboarded ?? isOnboarded, getAssetType: (assetId: Caip19AssetId): 'native' | 'erc20' | 'spl' => this.#getAssetType(assetId), }); - this.#stakedBalanceDataSource = new StakedBalanceDataSource({ + const stakedBalanceDataSource = new StakedBalanceDataSource({ messenger: this.messenger, onActiveChainsUpdated: this.#onActiveChainsUpdated, ...stakedBalanceDataSourceConfig, }); - this.#tokenDataSource = new TokenDataSource(this.messenger, { + const tokenDataSource = new TokenDataSource(this.messenger, { queryApiClient, getNativeAssetIds: (): Caip19AssetId[] => { return this.#getNativeAssetIdsForEnabledChains(); @@ -1089,13 +1213,13 @@ export class AssetsController extends BaseController< this.#getAssetType(assetId), getAssetsState: (): AssetsControllerState => this.state, }); - this.#priceDataSource = new PriceDataSource({ + const priceDataSource = new PriceDataSource({ queryApiClient, getSelectedCurrency: (): SupportedCurrency => this.state.selectedCurrency, getAssetsState: (): AssetsControllerState => this.state, ...priceDataSourceConfig, }); - this.#customAssetGraduationMiddleware = new CustomAssetGraduationMiddleware( + const customAssetGraduationMiddleware = new CustomAssetGraduationMiddleware( { getSelectedAccountId: (): AccountId | undefined => { try { @@ -1109,39 +1233,43 @@ export class AssetsController extends BaseController< getAssetsState: (): AssetsControllerState => this.state, }, ); - this.#rpcFallbackMiddleware = new RpcFallbackMiddleware({ - rpcDataSource: this.#rpcDataSource, + const rpcFallbackMiddleware = new RpcFallbackMiddleware({ + rpcDataSource, isBalanceV6Enabled: (): boolean => this.#isBalanceV6Enabled(), getAssetsState: (): AssetsControllerState => this.state, }); - log('Initializing AssetsController', { - defaultUpdateInterval: this.#defaultUpdateInterval, - }); - - // Subscribe during construction so a stateChange published before init() - // is not missed. Enabled chains are read in init(), after every - // controller has been constructed. - this.#subscribeToEvents(); - this.#registerActionHandlers(); - // Subscriptions start only when both UI is open and keyring unlocked -> #updateActive(). + return { + accountActivityDataSource, + accountsApiDataSource, + snapDataSource, + rpcDataSource, + stakedBalanceDataSource, + tokenDataSource, + priceDataSource, + customAssetGraduationMiddleware, + rpcFallbackMiddleware, + }; + } - // Subscribe to basic-functionality changes after construction so a synchronous - // onChange during subscribe cannot run before data sources are initialized. - if (subscribeToBasicFunctionalityChange) { - const unsubscribe = subscribeToBasicFunctionalityChange((isBasic) => - this.handleBasicFunctionalityChange(isBasic), - ); - if (typeof unsubscribe === 'function') { - this.#unsubscribeBasicFunctionality = unsubscribe; - } + /** + * Subscribe to the host's basic-functionality setting, if the host provided + * a way to. Runs from `start()`, after the data sources exist, so a + * synchronous `onChange` cannot run before they are created. + */ + #subscribeToBasicFunctionality(): void { + const { subscribeToBasicFunctionalityChange } = this.#sourceOptions; + if (!subscribeToBasicFunctionalityChange) { + return; + } + const unsubscribe = subscribeToBasicFunctionalityChange((isBasic) => + this.handleBasicFunctionalityChange(isBasic), + ); + if (typeof unsubscribe === 'function') { + this.#unsubscribeBasicFunctionality = unsubscribe; } } - // ============================================================================ - // INITIALIZATION - // ============================================================================ - #initializeNativeAssetsMap(queryApiClient: ApiPlatformClient): void { queryApiClient.queryClient .fetchQuery({ @@ -1170,40 +1298,10 @@ export class AssetsController extends BaseController< ); } - /** - * Read the current enabled chains from NetworkEnablementController, and - * refresh data sources that would otherwise read other controllers through - * the messenger. - * - * This controller does not call messenger actions in its constructor. - * `Wallet` constructs every controller first, then `Wallet.init()` calls - * `init()`. Later enablement changes arrive on `stateChange` events. - */ - async init(): Promise { - this.#initializeState(); - this.#rpcDataSource.refreshActiveChainsFromNetworkState(); - this.#stakedBalanceDataSource.refreshActiveChainsFromNetworkEnablementState(); - this.#snapDataSource.init(); - await this.#accountsApiDataSource.init(); - } - #initializeState(): void { - let enabledNetworkMap: NetworkEnablementControllerState['enabledNetworkMap']; - try { - ({ enabledNetworkMap } = this.messenger.call( - 'NetworkEnablementController:getState', - )); - } catch (error) { - if (!isUnregisteredActionError(error)) { - throw error; - } - log( - 'NetworkEnablementController:getState is not registered yet. Enabled chains are applied from NetworkEnablementController:stateChange.', - { error }, - ); - return; - } - + const { enabledNetworkMap } = this.messenger.call( + 'NetworkEnablementController:getState', + ); this.#enabledChains = this.#extractEnabledChains(enabledNetworkMap); log('Initialized state', { @@ -1274,13 +1372,6 @@ export class AssetsController extends BaseController< }, ); - // Fallback for hosts that do not register AccountTreeController. - // Ignored while that controller is registered so a switch is not - // refreshed here and again on `selectedAccountGroupChange`. - this.messenger.subscribe('AccountsController:selectedAccountChange', () => { - this.#handleSelectedAccountChanged().catch(console.error); - }); - this.messenger.subscribe('AccountTreeController:initialized', () => { this.#updateActive(); }); @@ -1550,9 +1641,9 @@ export class AssetsController extends BaseController< this.#isKeyringUnlocked() && this.#isAccountTreeInitialized(); if (shouldRun) { - this.#start(); + this.#startTracking(); } else { - this.#stop(); + this.#stopTracking(); } } @@ -1613,6 +1704,11 @@ export class AssetsController extends BaseController< activeChains: ChainId[], previousChains: ChainId[], ): void { + // Data source constructors report their chains while `start()` is still + // creating them. `start()` subscribes afterwards with every source's chains. + if (!this.#sources) { + return; + } if (!this.#isUiOpen() || !this.#isKeyringUnlocked() || !this.#isEnabled()) { return; } @@ -1688,7 +1784,23 @@ export class AssetsController extends BaseController< // PUBLIC API: QUERY METHODS // ============================================================================ - @trackAssetsLoading + /** + * Get assets for the given accounts, optionally force-fetching them first. + * + * Before `start()` there are no data sources, so a forced fetch is skipped + * and the assets already in state are returned. The loading status of the + * accounts is left untouched, so the first fetch after `start()` still + * reports them as loading. + * + * @param accounts - Accounts to get assets for. + * @param options - Fetch options. + * @param options.chainIds - Chains to include. Defaults to the enabled chains. + * @param options.assetTypes - Asset types to include. Defaults to fungible. + * @param options.forceUpdate - Fetch from the data sources before reading state. + * @param options.bypassServerCache - Also bypass server-side HTTP caches. + * @param options.dataTypes - Data types to fetch. + * @returns Assets keyed by account ID and asset ID. + */ async getAssets( accounts: InternalAccount[], options?: { @@ -1704,6 +1816,33 @@ export class AssetsController extends BaseController< bypassServerCache?: boolean; dataTypes?: DataType[]; }, + ): Promise>> { + if (options?.forceUpdate && !this.#sources) { + log('Skipping forced asset fetch: controller has not been started'); + return this.#getAssetsTracked(accounts, { + ...options, + forceUpdate: false, + }); + } + return this.#getAssetsTracked(accounts, options); + } + + @trackAssetsLoading + async #getAssetsTracked( + accounts: InternalAccount[], + options?: { + chainIds?: ChainId[]; + assetTypes?: AssetType[]; + forceUpdate?: boolean; + /** + * Also bypass server-side HTTP caches (e.g. the Accounts API's 60s + * cache, via a random `bypassServerCache` query param). Only meaningful + * together with `forceUpdate`. Use sparingly — e.g. right after a + * transaction confirms, when the API's cached snapshot is known stale. + */ + bypassServerCache?: boolean; + dataTypes?: DataType[]; + }, ): Promise>> { const chainIds = options?.chainIds ?? [...this.#enabledChains]; const assetTypes = options?.assetTypes ?? ['fungible']; @@ -1744,7 +1883,11 @@ export class AssetsController extends BaseController< * @param assetIds - CAIP-19 asset IDs to sync. */ async syncAssetsPrices(assetIds: Caip19AssetId[]): Promise { - if (!this.#isBasicFunctionality() || assetIds.length === 0) { + if ( + !this.#sources || + !this.#isBasicFunctionality() || + assetIds.length === 0 + ) { return; } @@ -2649,7 +2792,7 @@ export class AssetsController extends BaseController< selectedCurrency, }); - if (!this.#isBasicFunctionality()) { + if (!this.#sources || !this.#isBasicFunctionality()) { return; } @@ -2677,7 +2820,7 @@ export class AssetsController extends BaseController< * @param chainIds - Chain IDs to filter prices for. */ subscribeAssetsPrice(accounts: InternalAccount[], chainIds: ChainId[]): void { - if (!this.#isBasicFunctionality()) { + if (!this.#sources || !this.#isBasicFunctionality()) { return; } const subscriptionKey = 'ds:PriceDataSource'; @@ -3549,7 +3692,7 @@ export class AssetsController extends BaseController< * Idempotent — returns early if accounts/chains are not yet available or * subscriptions are already active. */ - #start(): void { + #startTracking(): void { const accounts = this.#getSelectedAccounts(); const chainIds = [...this.#enabledChains]; @@ -3573,21 +3716,6 @@ export class AssetsController extends BaseController< }) .finally(() => { this.#startInFlight = false; - const shouldRefreshPendingAccount = - this.#pendingAccountRefresh && - this.#isUiOpen() && - this.#isKeyringUnlocked() && - this.#isAccountTreeInitialized(); - this.#pendingAccountRefresh = false; - if (shouldRefreshPendingAccount) { - this.#refreshTrackedAccounts(this.#getSelectedAccounts()).catch( - (error: unknown) => { - log('Failed to refresh assets after startup account switch', { - error, - }); - }, - ); - } }); } @@ -3595,7 +3723,7 @@ export class AssetsController extends BaseController< * Stop asset tracking: unsubscribe from all updates. * Called when app closes or keyring locks. */ - #stop(): void { + #stopTracking(): void { log('Stopping asset tracking', { activeSubscriptionCount: this.#activeSubscriptions.size, hasPriceSubscription: this.#activeSubscriptions.has('ds:PriceDataSource'), @@ -3604,6 +3732,10 @@ export class AssetsController extends BaseController< this.#firstInitFetchReported = false; this.#stateSizeReported = false; + if (!this.#sources) { + return; + } + // Stop price subscription first (uses direct messenger call) this.unsubscribeAssetsPrice(); @@ -3639,7 +3771,10 @@ export class AssetsController extends BaseController< * @param _isBasic - The new value (for call-site clarity; the getter is the source of truth). */ handleBasicFunctionalityChange(_isBasic: boolean): void { - this.#stop(); + if (!this.#sources) { + return; + } + this.#stopTracking(); this.#subscribeAssets(); } @@ -4080,9 +4215,6 @@ export class AssetsController extends BaseController< // (including when the group did not change) so late subscribers can catch // up — ignore those until we are already tracking. if (this.#activeSubscriptions.size === 0) { - if (this.#startInFlight && groupId !== previousGroupId) { - this.#pendingAccountRefresh = true; - } return; } if (groupId === previousGroupId) { @@ -4098,46 +4230,6 @@ export class AssetsController extends BaseController< previousGroupId, }); - await this.#refreshTrackedAccounts(accounts); - } - - /** - * Refresh tracked assets after `AccountsController:selectedAccountChange` - * when `AccountTreeController` is not registered. - * - * Ignored while the account tree controller is registered, because that - * controller republishes the switch as `selectedAccountGroupChange`. - * Also ignored until tracking has started, matching group switches, so the - * first start stays owned by unlock and account-tree initialization. - */ - async #handleSelectedAccountChanged(): Promise { - if (this.#isAccountTreeControllerRegistered()) { - return; - } - if (this.#activeSubscriptions.size === 0) { - if (this.#startInFlight) { - this.#pendingAccountRefresh = true; - } - return; - } - - const accounts = this.#getSelectedAccounts(); - - log('Selected account changed', { - accountCount: accounts.length, - accountIds: accounts.map((account) => account.id), - }); - - await this.#refreshTrackedAccounts(accounts); - } - - /** - * Force-fetch and resubscribe for the accounts that should be tracked now. - * - * @param accounts - Accounts to refresh. An empty list skips the fetch and - * still resubscribes, matching a switch onto an empty selection. - */ - async #refreshTrackedAccounts(accounts: InternalAccount[]): Promise { const releaseLock = await this.#accountRefreshMutex.acquire(); try { this.#ensureNativeBalancesDefaultZero(); @@ -4379,6 +4471,13 @@ export class AssetsController extends BaseController< assetsPrice: response.assetsPrice, }); + if (!this.#sources) { + log('Ignoring assets update: controller has not been started', { + sourceId, + }); + return; + } + if (this.#isBalanceV6Enabled()) { await this.#handleAssetsUpdateV6(response, sourceId, request); return; @@ -4584,19 +4683,23 @@ export class AssetsController extends BaseController< destroy(): void { log('Destroying AssetsController', { - dataSourceCount: this.#allBalanceDataSources.length, + isStarted: this.#sources !== undefined, subscriptionCount: this.#activeSubscriptions.size, }); - // Destroy instantiated data sources - this.#accountActivityDataSource?.destroy?.(); - this.#accountsApiDataSource?.destroy?.(); - this.#snapDataSource?.destroy?.(); - this.#rpcDataSource?.destroy?.(); - this.#stakedBalanceDataSource?.destroy?.(); + this.#isDestroyed = true; + + // Destroy instantiated data sources (none exist before `start()`) + if (this.#sources) { + this.#sources.accountActivityDataSource.destroy(); + this.#sources.accountsApiDataSource.destroy(); + this.#sources.snapDataSource.destroy(); + this.#sources.rpcDataSource.destroy(); + this.#sources.stakedBalanceDataSource.destroy(); + } // Stop all active subscriptions - this.#stop(); + this.#stopTracking(); if (this.#unsubscribeBasicFunctionality) { this.#unsubscribeBasicFunctionality(); @@ -4604,6 +4707,7 @@ export class AssetsController extends BaseController< } // Unregister action handlers + this.messenger.unregisterActionHandler('AssetsController:start'); this.messenger.unregisterActionHandler('AssetsController:getAssets'); this.messenger.unregisterActionHandler('AssetsController:getAssetsBalance'); this.messenger.unregisterActionHandler('AssetsController:getAssetMetadata'); @@ -4636,15 +4740,3 @@ export class AssetsController extends BaseController< ); } } - -/** - * Whether a messenger call failed because no controller has registered the action yet. - * - * @param error - The error thrown by `messenger.call`. - * @returns True when the action handler is not registered. - */ -function isUnregisteredActionError(error: unknown): boolean { - return ( - error instanceof Error && error.message.includes('has not been registered') - ); -} diff --git a/packages/assets-controller/src/AssetsController.ws-price-updates.integration.test.ts b/packages/assets-controller/src/AssetsController.ws-price-updates.integration.test.ts index cb3da104a05..01d16e7de20 100644 --- a/packages/assets-controller/src/AssetsController.ws-price-updates.integration.test.ts +++ b/packages/assets-controller/src/AssetsController.ws-price-updates.integration.test.ts @@ -113,7 +113,7 @@ async function withController( queryApiClient, isBasicFunctionality: (): boolean => true, }); - await controller.init(); + await controller.start(); try { return await fn({ controller, messenger: rootMessenger }); diff --git a/packages/assets-controller/src/README.md b/packages/assets-controller/src/README.md index 32a70584555..31b4497113e 100644 --- a/packages/assets-controller/src/README.md +++ b/packages/assets-controller/src/README.md @@ -39,9 +39,14 @@ The `AssetsController` is a unified asset management system that provides real-t ### 1. Initialization -When the `AssetsController` constructor is called, it performs the following steps **synchronously**: +Initialization has two phases, so that construction does not depend on the order in which controllers are constructed: -#### 1.1 Base Controller Setup +- **Constructor**: sets up state, stores options and registers action handlers. It does not call messenger actions, create data sources or subscribe to external events. +- **`start()`** (also the `AssetsController:start` action): called by the client once the controllers `AssetsController` depends on (for example `AccountTreeController` and `SnapController`) are registered. It performs steps 1.2, 1.3 and 1.5, subscribes to `subscribeToBasicFunctionalityChange`, runs the unlock cleanups if the keyring is already unlocked, and starts tracking (section 2) when the UI is open, the keyring is unlocked and the account tree is initialized. Repeated calls return the same promise. + +Before `start()`, forced fetches return what is already in state, and pushed updates are ignored. + +#### 1.1 Base Controller Setup (constructor) ```typescript super({ @@ -52,7 +57,7 @@ super({ }); ``` -#### 1.2 Initialize Internal State +#### 1.2 Initialize Internal State (`start()`) ```typescript initializeState() @@ -64,22 +69,22 @@ get selectedAccounts(): InternalAccount[] └── return messenger.call('AccountTreeController:getAccountsFromSelectedAccountGroup') ``` -#### 1.3 Subscribe to External Events +#### 1.3 Subscribe to External Events (`start()`) -| Event | Handler | Purpose | -| -------------------------------------------------- | -------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- | -| `AccountTreeController:selectedAccountGroupChange` | `handleAccountGroupChanged()` | Refresh when the user switches groups | -| `AccountsController:selectedAccountChange` | `handleSelectedAccountChanged()` | Refresh when the selected account changes and `AccountTreeController` is absent. Ignored while that controller is registered | -| `AccountTreeController:initialized` | `updateActive()` | Start tracking once the tree is built | -| `AccountTreeController:uninitialized` | `updateActive()` | Stop tracking when the tree is cleared | -| `NetworkEnablementController:stateChange` | `handleEnabledNetworksChanged()` | Track enabled chains | -| `KeyringController:unlock` | `start()` | Start subscriptions | -| `KeyringController:lock` | `stop()` | Stop subscriptions | +| Event | Handler | Purpose | +| -------------------------------------------------- | -------------------------------- | -------------------------------------- | +| `AccountTreeController:selectedAccountGroupChange` | `handleAccountGroupChanged()` | Refresh when the user switches groups | +| `AccountTreeController:initialized` | `updateActive()` | Start tracking once the tree is built | +| `AccountTreeController:uninitialized` | `updateActive()` | Stop tracking when the tree is cleared | +| `NetworkEnablementController:stateChange` | `handleEnabledNetworksChanged()` | Track enabled chains | +| `KeyringController:unlock` | `startTracking()` | Start subscriptions | +| `KeyringController:lock` | `stopTracking()` | Stop subscriptions | -#### 1.4 Register Action Handlers +#### 1.4 Register Action Handlers (constructor) ```typescript registerActionHandlers() +├── AssetsController:start ├── AssetsController:getAssets ├── AssetsController:getAssetsBalance ├── AssetsController:getAssetMetadata @@ -88,7 +93,7 @@ registerActionHandlers() └── AssetsController:assetsUpdate // Data sources push asset updates ``` -#### 1.5 Balance data source priority +#### 1.5 Balance data source priority (data sources created in `start()`) Built-in balance data sources are fixed and processed in priority order: AccountActivityDataSource, AccountsApiDataSource, SnapDataSource, RpcDataSource. Earlier sources get first pick for chain assignment; later sources act as fallbacks. Data sources report active chains via the `onActiveChainsUpdated` callback passed at construction. @@ -126,7 +131,7 @@ executeMiddlewares([ When the keyring unlocks: ``` -start() // Called by KeyringController:unlock +startTracking() // Called by KeyringController:unlock │ ├── subscribeAssets() │ │ @@ -240,7 +245,7 @@ Result: Earlier registered sources get first pick; later sources act as fallback When the keyring locks: ``` -stop() // Called by KeyringController:lock +stopTracking() // Called by KeyringController:lock │ ├── For each activeSubscription: │ └── subscription.unsubscribe() @@ -266,7 +271,7 @@ destroy() │ └── dataSources.clear() │ ├── Stop all active subscriptions -│ └── stop() +│ └── stopTracking() │ └── Unregister action handlers ├── AssetsController:getAssets @@ -891,6 +896,9 @@ const assetsController = new AssetsController({ state: existingState, // Optional: restore persisted state defaultUpdateInterval: 30_000, // Optional: polling hint (30s default) }); + +// After the client has registered AccountTreeController, SnapController, etc. +await assetsController.start(); ``` ### Basic Usage (Controllers) diff --git a/packages/assets-controller/src/__fixtures__/MockAssetControllerMessenger.ts b/packages/assets-controller/src/__fixtures__/MockAssetControllerMessenger.ts index d5f018b503e..52383681d09 100644 --- a/packages/assets-controller/src/__fixtures__/MockAssetControllerMessenger.ts +++ b/packages/assets-controller/src/__fixtures__/MockAssetControllerMessenger.ts @@ -127,7 +127,6 @@ export function createMockAssetsControllerMessenger( 'NetworkEnablementController:stateChange', // SnapDataSource 'AccountsController:accountBalancesUpdated', - 'AccountsController:selectedAccountChange', 'PermissionController:stateChange', 'SnapController:snapInstalled', // AccountActivityService (real-time balances + chain status) @@ -182,25 +181,21 @@ export function registerAssetsControllerStateMock( export function registerStakedMessengerActions( rootMessenger: MockRootMessenger, - opts: { - enabledNetworkMap?: Record>; - mockProvider?: ReturnType; - } = {}, -): void { - const enabledNetworkMap = opts.enabledNetworkMap ?? { - eip155: { [MAINNET_CHAIN_ID_HEX]: true }, - }; - const mockProvider = - opts.mockProvider ?? - createMockWeb3Provider({ + opts = { + enabledNetworkMap: { eip155: { [MAINNET_CHAIN_ID_HEX]: true } } as Record< + string, + Record + >, + mockProvider: createMockWeb3Provider({ sharesWei: '1000000000000000000', assetsWei: '1500000000000000000', - }); - + }), + }, +): void { rootMessenger.registerActionHandler( 'NetworkEnablementController:getState', () => ({ - enabledNetworkMap, + enabledNetworkMap: opts.enabledNetworkMap, nativeAssetIdentifiers: {}, }), ); @@ -209,7 +204,7 @@ export function registerStakedMessengerActions( 'NetworkController:getNetworkClientById', () => ({ - provider: mockProvider, + provider: opts.mockProvider, configuration: { chainId: MAINNET_CHAIN_ID_HEX }, }) as TestMockType, ); diff --git a/packages/assets-controller/src/data-sources/AccountActivityDataSource.test.ts b/packages/assets-controller/src/data-sources/AccountActivityDataSource.test.ts index fac5e97358e..12559520fd1 100644 --- a/packages/assets-controller/src/data-sources/AccountActivityDataSource.test.ts +++ b/packages/assets-controller/src/data-sources/AccountActivityDataSource.test.ts @@ -96,7 +96,6 @@ function createBalanceUpdate(overrides?: { type SetupOptions = { groupAccounts?: InternalAccount[]; selectedAccount?: InternalAccount | null; - registerAccountTree?: boolean; onAssetsUpdate?: Mock; onActiveChainsUpdated?: Mock; state?: { activeChains?: ChainId[] }; @@ -130,7 +129,6 @@ function setup(options: SetupOptions = {}): SetupResult { const { groupAccounts = [createMockAccount()], selectedAccount = null, - registerAccountTree = true, onAssetsUpdate = vi.fn().mockResolvedValue(undefined), onActiveChainsUpdated = vi.fn(), state, @@ -157,12 +155,10 @@ function setup(options: SetupOptions = {}): SetupResult { ], }); - if (registerAccountTree) { - rootMessenger.registerActionHandler( - 'AccountTreeController:getAccountsFromSelectedAccountGroup', - () => groupAccounts, - ); - } + rootMessenger.registerActionHandler( + 'AccountTreeController:getAccountsFromSelectedAccountGroup', + () => groupAccounts, + ); rootMessenger.registerActionHandler( 'AccountsController:getSelectedAccount', () => selectedAccount as InternalAccount, @@ -304,62 +300,6 @@ describe('AccountActivityDataSource', () => { cleanup(); }); - it('falls back to the selected account when AccountTreeController is not registered', async () => { - const account = createMockAccount(); - const { onAssetsUpdate, triggerBalanceUpdated, cleanup } = setup({ - registerAccountTree: false, - selectedAccount: account, - }); - - triggerBalanceUpdated({ - address: EVM_ADDRESS, - chain: CHAIN_MAINNET, - updates: [createBalanceUpdate()], - }); - - await waitFor(() => { - expect(onAssetsUpdate).toHaveBeenCalledTimes(1); - }); - expect(onAssetsUpdate.mock.calls[0][1]).toStrictEqual({ - accountsWithSupportedChains: [ - { account, supportedChains: [CHAIN_MAINNET] }, - ], - chainIds: [CHAIN_MAINNET], - dataTypes: ['balance', 'metadata', 'price'], - }); - - cleanup(); - }); - - it('drops the balance update when AccountTreeController throws', async () => { - const account = createMockAccount(); - const { onAssetsUpdate, triggerBalanceUpdated, rootMessenger, cleanup } = - setup({ - groupAccounts: [account], - }); - rootMessenger.unregisterActionHandler( - 'AccountTreeController:getAccountsFromSelectedAccountGroup', - ); - rootMessenger.registerActionHandler( - 'AccountTreeController:getAccountsFromSelectedAccountGroup', - () => { - throw new Error('account tree failed'); - }, - ); - - triggerBalanceUpdated({ - address: EVM_ADDRESS, - chain: CHAIN_MAINNET, - updates: [createBalanceUpdate()], - }); - - await new Promise(process.nextTick); - - expect(onAssetsUpdate).not.toHaveBeenCalled(); - - cleanup(); - }); - it('converts a hex postBalance to a human-readable amount', async () => { const account = createMockAccount(); const { onAssetsUpdate, triggerBalanceUpdated, cleanup } = setup({ diff --git a/packages/assets-controller/src/data-sources/AccountActivityDataSource.ts b/packages/assets-controller/src/data-sources/AccountActivityDataSource.ts index 720ab29c0cc..cea27c29b5e 100644 --- a/packages/assets-controller/src/data-sources/AccountActivityDataSource.ts +++ b/packages/assets-controller/src/data-sources/AccountActivityDataSource.ts @@ -321,17 +321,11 @@ export class AccountActivityDataSource extends AbstractDataSource< * @returns The accounts of the selected account group. */ #getWalletAccounts(): InternalAccount[] { - try { - const accounts = this.#messenger.call( - 'AccountTreeController:getAccountsFromSelectedAccountGroup', - ); - if (accounts.length > 0) { - return accounts; - } - } catch (error) { - if (!isUnregisteredActionError(error)) { - throw error; - } + const accounts = this.#messenger.call( + 'AccountTreeController:getAccountsFromSelectedAccountGroup', + ); + if (accounts.length > 0) { + return accounts; } const selectedAccount = this.#messenger.call( @@ -419,18 +413,6 @@ export class AccountActivityDataSource extends AbstractDataSource< } } -/** - * Whether a messenger call failed because no controller has registered the action. - * - * @param error - The error thrown by `messenger.call`. - * @returns True when the action handler is not registered. - */ -function isUnregisteredActionError(error: unknown): boolean { - return ( - error instanceof Error && error.message.includes('has not been registered') - ); -} - // ============================================================================ // FACTORY FUNCTION // ============================================================================ diff --git a/packages/assets-controller/src/data-sources/AccountsApiDataSource.test.ts b/packages/assets-controller/src/data-sources/AccountsApiDataSource.test.ts index 3762a9c765f..d84eee998ca 100644 --- a/packages/assets-controller/src/data-sources/AccountsApiDataSource.test.ts +++ b/packages/assets-controller/src/data-sources/AccountsApiDataSource.test.ts @@ -199,7 +199,6 @@ async function setupController( remoteFeatureFlags?: Record; getAssetsState?: () => AssetsControllerState; tokenDetectionEnabled?: () => boolean; - initialize?: boolean; } = {}, ): Promise { const { @@ -213,7 +212,6 @@ async function setupController( remoteFeatureFlags = {}, getAssetsState = (): AssetsControllerState => createAssetsState(), tokenDetectionEnabled, - initialize = true, } = options; const rootMessenger = new Messenger({ @@ -279,9 +277,7 @@ async function setupController( ...(tokenDetectionEnabled === undefined ? {} : { tokenDetectionEnabled }), }); - if (initialize) { - await controller.init(); - } + // Wait for async initialization await new Promise(process.nextTick); return { @@ -323,30 +319,6 @@ describe('AccountsApiDataSource', () => { vi.clearAllMocks(); }); - it('does not call RemoteFeatureFlagController in the constructor', async () => { - const { controller, apiClient, activeChainsUpdateHandler } = - await setupController({ - initialize: false, - supportedChains: [1], - }); - - expect(apiClient.accounts.fetchV2SupportedNetworks).not.toHaveBeenCalled(); - expect(await controller.getActiveChains()).toStrictEqual([]); - - await controller.init(); - - expect(apiClient.accounts.fetchV2SupportedNetworks).toHaveBeenCalledTimes( - 1, - ); - expect(activeChainsUpdateHandler).toHaveBeenCalledWith( - 'AccountsApiDataSource', - [CHAIN_MAINNET], - [], - ); - - controller.destroy(); - }); - it('initializes with correct name', async () => { const { controller } = await setupController(); expect(controller.getName()).toBe('AccountsApiDataSource'); @@ -367,35 +339,6 @@ describe('AccountsApiDataSource', () => { controller.destroy(); }); - it('refreshes supported networks on a second init', async () => { - const { controller, apiClient } = await setupController({ - supportedChains: [1], - }); - - apiClient.accounts.fetchV2SupportedNetworks.mockClear(); - - await controller.init(); - - expect(apiClient.accounts.fetchV2SupportedNetworks).toHaveBeenCalledTimes( - 1, - ); - - controller.destroy(); - }); - - it('starts one refresh timer when init is called concurrently', async () => { - const { controller } = await setupController({ - initialize: false, - supportedChains: [1], - }); - - await Promise.all([controller.init(), controller.init()]); - - expect(activeTimers).toHaveLength(1); - - controller.destroy(); - }); - it('refreshActiveChains re-fetches supported networks and updates activeChains', async () => { const { controller, apiClient, activeChainsUpdateHandler } = await setupController({ supportedChains: [1] }); @@ -2125,7 +2068,7 @@ describe('AccountsApiDataSource', () => { const controller = new AccountsApiDataSource(controllerOptions); - await controller.init(); + // Wait for async initialization await new Promise(process.nextTick); return { diff --git a/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts b/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts index a4c83d87df5..1894998a418 100644 --- a/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts +++ b/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts @@ -314,17 +314,8 @@ export class AccountsApiDataSource extends AbstractDataSource< // return a primitive rather than a fresh object. (state) => getMigrationStages(state.remoteFeatureFlags).join(','), ); - } - /** - * Fetch supported networks and ensure the active-chain refresh timer runs. - * - * The constructor does not call messenger actions. `AssetsController.init()` - * calls this after every controller has been constructed. Repeated calls - * refresh the chain list without starting another timer. - */ - async init(): Promise { - await this.#initializeActiveChains(); + this.#initializeActiveChains().catch(console.error); } /** @@ -348,11 +339,6 @@ export class AccountsApiDataSource extends AbstractDataSource< // ============================================================================ async #initializeActiveChains(): Promise { - if (this.#chainsRefreshTimer) { - await this.#refreshActiveChains(); - return; - } - try { const chains = await this.#fetchActiveChains(); const previous = [...this.state.activeChains]; @@ -360,10 +346,6 @@ export class AccountsApiDataSource extends AbstractDataSource< this.#onActiveChainsUpdated(this.getName(), updatedChains, previous), ); - if (this.#chainsRefreshTimer) { - return; - } - // Periodically refresh active chains (every 20 minutes) this.#chainsRefreshTimer = setInterval( () => { diff --git a/packages/assets-controller/src/data-sources/RpcDataSource.test.ts b/packages/assets-controller/src/data-sources/RpcDataSource.test.ts index 447058e10ce..300f6170667 100644 --- a/packages/assets-controller/src/data-sources/RpcDataSource.test.ts +++ b/packages/assets-controller/src/data-sources/RpcDataSource.test.ts @@ -269,8 +269,6 @@ async function withController( ...options, getAssetsState, }); - // Active chains are read in init(), not the constructor. - controller.refreshActiveChainsFromNetworkState(); try { return await fn({ @@ -376,32 +374,6 @@ describe('RpcDataSource', () => { ); }); - it('does not call messenger actions in the constructor', () => { - const { assetsControllerMessenger } = createMockMessengers({ - registerCustomRootActions: (rootMessenger) => - registerRpcDataSourceActions(rootMessenger, { - networkState: createMockNetworkState(), - }), - }); - const callSpy = vi.spyOn(assetsControllerMessenger, 'call'); - const source = new RpcDataSource({ - messenger: assetsControllerMessenger, - getAssetsState: getDefaultAssetsControllerState, - onActiveChainsUpdated: vi.fn(), - getNativeAssetForChain: vi.fn(), - getAssetType: vi.fn(), - getAssetVisibility: vi.fn(), - queryApiClient: createTestApiClient(), - }); - - expect(callSpy).not.toHaveBeenCalled(); - - source.refreshActiveChainsFromNetworkState(); - - expect(callSpy).toHaveBeenCalledWith('NetworkController:getState'); - source.destroy(); - }); - it('reports active chains on initialization', async () => { await withController(async ({ onActiveChainsUpdated }) => { expect(onActiveChainsUpdated).toHaveBeenCalledWith( diff --git a/packages/assets-controller/src/data-sources/RpcDataSource.ts b/packages/assets-controller/src/data-sources/RpcDataSource.ts index 1e6ef66009c..36a7aa13578 100644 --- a/packages/assets-controller/src/data-sources/RpcDataSource.ts +++ b/packages/assets-controller/src/data-sources/RpcDataSource.ts @@ -347,6 +347,7 @@ export class RpcDataSource extends AbstractDataSource< this.#subscribeToNetworkController(); this.#subscribeToTransactionEvents(); + this.#initializeFromNetworkController(); } /** @@ -874,11 +875,8 @@ export class RpcDataSource extends AbstractDataSource< } /** - * Read `NetworkController:getState` and refresh Rpc `activeChains`. - * - * The constructor does not call messenger actions. `AssetsController.init()` - * calls this after every controller has been constructed. Later network - * changes still arrive on `NetworkController:stateChange`. + * Re-read NetworkController state and refresh Rpc `activeChains` (e.g. when + * network availability metadata changes after an EVM network switch). */ refreshActiveChainsFromNetworkState(): void { this.#initializeFromNetworkController(); diff --git a/packages/assets-controller/src/data-sources/SnapDataSource.test.ts b/packages/assets-controller/src/data-sources/SnapDataSource.test.ts index bb0a62f6bed..2dd0d7b719d 100644 --- a/packages/assets-controller/src/data-sources/SnapDataSource.test.ts +++ b/packages/assets-controller/src/data-sources/SnapDataSource.test.ts @@ -248,7 +248,6 @@ function setupController( isBalanceV6Enabled?: () => boolean; getAssetVisibility?: GetAssetVisibility; getAssetsState?: () => AssetsControllerState; - initialize?: boolean; } = {}, ): SetupResult { const { @@ -340,9 +339,6 @@ function setupController( }; const controller = new SnapDataSource(controllerOptions); - if (options.initialize !== false) { - controller.init(); - } // Subscribe so that balance updates are reported via onAssetsUpdate const chainIdsFromSnaps = Object.values(installedSnaps).flatMap( @@ -437,28 +433,6 @@ describe('SnapDataSource helper functions', () => { }); describe('SnapDataSource', () => { - it('does not call SnapController in the constructor', () => { - const { controller, mockGetRunnableSnaps, mockGetPermissions, cleanup } = - setupController({ - installedSnaps: { - [SOLANA_SNAP_ID]: { version: '1.0.0', chainIds: [SOLANA_MAINNET] }, - }, - initialize: false, - }); - - expect(mockGetRunnableSnaps).not.toHaveBeenCalled(); - expect(mockGetPermissions).not.toHaveBeenCalled(); - expect(controller.getActiveChainsSync()).toStrictEqual([]); - - controller.init(); - - expect(mockGetRunnableSnaps).toHaveBeenCalledTimes(1); - expect(mockGetPermissions).toHaveBeenCalledWith(SOLANA_SNAP_ID); - expect(controller.getActiveChainsSync()).toContain(SOLANA_MAINNET); - - cleanup(); - }); - it('initializes with correct name', async () => { const { controller, cleanup } = setupController(); expect(controller.getName()).toBe('SnapDataSource'); diff --git a/packages/assets-controller/src/data-sources/SnapDataSource.ts b/packages/assets-controller/src/data-sources/SnapDataSource.ts index 0ca743c719b..1d62da6d57e 100644 --- a/packages/assets-controller/src/data-sources/SnapDataSource.ts +++ b/packages/assets-controller/src/data-sources/SnapDataSource.ts @@ -265,17 +265,8 @@ export class SnapDataSource extends AbstractDataSource< this.#discoverKeyringSnaps.bind(this); this.#subscribeToEvents(); - } - /** - * Discover keyring snaps and set `activeChains`. - * - * The constructor does not call messenger actions. `AssetsController.init()` - * calls this after every controller has been constructed. When - * `SnapController` is not registered, discovery leaves `activeChains` empty. - * Later snap installs and permission changes rediscover. - */ - init(): void { + // Discover keyring-capable snaps and populate activeChains dynamically this.#discoverKeyringSnaps(); } diff --git a/packages/assets-controller/src/data-sources/StakedBalanceDataSource.test.ts b/packages/assets-controller/src/data-sources/StakedBalanceDataSource.test.ts index 10fdc59853e..ecb6409b8e7 100644 --- a/packages/assets-controller/src/data-sources/StakedBalanceDataSource.test.ts +++ b/packages/assets-controller/src/data-sources/StakedBalanceDataSource.test.ts @@ -142,8 +142,6 @@ async function withController( ...options, pollInterval: 1000, }); - // Active chains are read in init(), not the constructor. - controller.refreshActiveChainsFromNetworkEnablementState(); try { return await fn({ @@ -189,29 +187,6 @@ describe('StakedBalanceDataSource', () => { ); }); - it('does not call messenger actions in the constructor', async () => { - const { assetsControllerMessenger } = createMockMessengers({ - registerCustomRootActions: (messenger) => - registerStakedMessengerActions(messenger, { - enabledNetworkMap: { eip155: { [MAINNET_CHAIN_ID_HEX]: true } }, - }), - }); - const callSpy = vi.spyOn(assetsControllerMessenger, 'call'); - const source = new StakedBalanceDataSource({ - messenger: assetsControllerMessenger, - onActiveChainsUpdated: vi.fn(), - }); - - expect(callSpy).not.toHaveBeenCalled(); - - source.refreshActiveChainsFromNetworkEnablementState(); - - expect(callSpy).toHaveBeenCalledWith( - 'NetworkEnablementController:getState', - ); - source.destroy(); - }); - it('calls onActiveChainsUpdated with active staking chains when mainnet is enabled', async () => { await withController(({ onActiveChainsUpdated }) => { expect(onActiveChainsUpdated).toHaveBeenCalledWith( @@ -240,31 +215,6 @@ describe('StakedBalanceDataSource', () => { }); }); - describe('refreshActiveChainsFromNetworkEnablementState', () => { - it('re-reads NetworkEnablementController state and updates active chains', async () => { - await withController( - { enabledNetworkMap: {} }, - async ({ controller, mockMessengerCall, onActiveChainsUpdated }) => { - expect(await controller.getActiveChains()).toStrictEqual([]); - - mockMessengerCall.mockReturnValueOnce({ - enabledNetworkMap: { eip155: { [MAINNET_CHAIN_ID_HEX]: true } }, - }); - controller.refreshActiveChainsFromNetworkEnablementState(); - - expect(await controller.getActiveChains()).toStrictEqual([ - MAINNET_CHAIN_ID_CAIP, - ]); - expect(onActiveChainsUpdated).toHaveBeenLastCalledWith( - 'StakedBalanceDataSource', - [MAINNET_CHAIN_ID_CAIP], - [], - ); - }, - ); - }); - }); - describe('getName', () => { it('returns the data source name', async () => { await withController(({ controller }) => { diff --git a/packages/assets-controller/src/data-sources/StakedBalanceDataSource.ts b/packages/assets-controller/src/data-sources/StakedBalanceDataSource.ts index 513662af54d..65d5a0c1528 100644 --- a/packages/assets-controller/src/data-sources/StakedBalanceDataSource.ts +++ b/packages/assets-controller/src/data-sources/StakedBalanceDataSource.ts @@ -215,17 +215,7 @@ export class StakedBalanceDataSource extends AbstractDataSource< 'NetworkEnablementController:stateChange', this.#onNetworkEnablementControllerStateChange.bind(this), ); - } - /** - * Read `NetworkEnablementController:getState` and refresh staking - * `activeChains`. - * - * The constructor does not call messenger actions. `AssetsController.init()` - * calls this after every controller has been constructed. Later enablement - * changes still arrive on `NetworkEnablementController:stateChange`. - */ - refreshActiveChainsFromNetworkEnablementState(): void { this.#initializeActiveChains(); } diff --git a/packages/assets-controller/src/index.ts b/packages/assets-controller/src/index.ts index dd56b5ee3c7..2224eed60da 100644 --- a/packages/assets-controller/src/index.ts +++ b/packages/assets-controller/src/index.ts @@ -39,6 +39,7 @@ export type { AssetsControllerEvents, } from './AssetsController.js'; export type { + AssetsControllerStartAction, AssetsControllerGetAssetsAction, AssetsControllerGetAssetsBalanceAction, AssetsControllerGetAssetMetadataAction, diff --git a/packages/wallet/CHANGELOG.md b/packages/wallet/CHANGELOG.md index d82b83464a5..dc3ef36678e 100644 --- a/packages/wallet/CHANGELOG.md +++ b/packages/wallet/CHANGELOG.md @@ -11,9 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **BREAKING:** Add `AssetsController` to the default initialization configurations ([#10885](https://github.com/MetaMask/core/pull/10885)) - Requires `instanceOptions.assetsController.queryApiClient`. - - `Wallet.init()` calls `AssetsController.init()`, which reads `NetworkEnablementController:getState` after every controller has been constructed. Later enablement changes arrive on `NetworkEnablementController:stateChange`. - - Delegates `AccountTreeController` and `SnapController` actions and events. `Wallet` does not construct those controllers. Until a client registers them, account-group reads fall back to `AccountsController:getSelectedAccount`, asset tracking does not wait for the account tree, and snap discovery leaves snap chains empty. - - Delegates `AccountsController:selectedAccountChange`. When `AccountTreeController` is not registered, that event refreshes tracked assets. When it is registered, `AssetsController` ignores the event and still refreshes only on `AccountTreeController:selectedAccountGroupChange`. + - `AssetsController` does not track assets until the client calls `AssetsController:start` (or `AssetsController.start()`). Call it after the client has registered and initialized the controllers `AssetsController` depends on that `Wallet` does not construct, such as `AccountTreeController` and `SnapController`. `Wallet.init()` does not start it. + - Delegates `AccountTreeController` and `SnapController` actions and events, which the client registers. - Also delegates the other external `AssetsControllerMessenger` actions and events (accounts, client, keyring, network, network enablement, permission, phishing, remote feature flags, preferences, and transactions). `Wallet` constructs the ones it already initializes by default. - Hosts that already register `AssetsController` on the wallet root messenger must remove that registration before upgrading, or messenger registration will collide. diff --git a/packages/wallet/src/Wallet.test.ts b/packages/wallet/src/Wallet.test.ts index 84e24c69bca..69f96e40fc8 100644 --- a/packages/wallet/src/Wallet.test.ts +++ b/packages/wallet/src/Wallet.test.ts @@ -213,7 +213,7 @@ describe('Wallet', () => { const results = await wallet.init(); - expect(results).toHaveLength(11); + expect(results).toHaveLength(10); }); it('disallows modifying the messenger', async () => { diff --git a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts index c2e7e0e0ade..95e8d15cadf 100644 --- a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts +++ b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.test.ts @@ -86,23 +86,23 @@ describe('assetsController', () => { expect(instance.state.selectedCurrency).toBe('eur'); }); - it('forwards the instance options to the controller', () => { - const rootMessenger = getRootMessenger(); - const messenger = assetsController.getMessenger(rootMessenger); + it('forwards the instance options to the controller', async () => { const unsubscribe = vi.fn(); const subscribeToBasicFunctionalityChange = vi .fn() .mockReturnValue(unsubscribe); - - const instance = assetsController.init({ - state: undefined, - messenger, - options: { - queryApiClient: createMockQueryApiClient(), - subscribeToBasicFunctionalityChange, + const wallet = new Wallet({ + instanceOptions: { + ...getInstanceOptions(), + assetsController: { + queryApiClient: createMockQueryApiClient(), + subscribeToBasicFunctionalityChange, + }, }, }); - instance.destroy(); + + await wallet.getInstance('AssetsController').start(); + await wallet.destroy(); expect(subscribeToBasicFunctionalityChange).toHaveBeenCalledTimes(1); expect(unsubscribe).toHaveBeenCalledTimes(1); @@ -164,7 +164,6 @@ describe('assetsController', () => { 'AccountActivityService:balanceUpdated', 'AccountActivityService:statusChanged', 'AccountsController:accountBalancesUpdated', - 'AccountsController:selectedAccountChange', 'AccountTreeController:initialized', 'AccountTreeController:selectedAccountGroupChange', 'AccountTreeController:uninitialized', @@ -200,7 +199,7 @@ describe('assetsController', () => { expect(listener).toHaveBeenCalledWith('payload'); }); - it('is constructed by default while AccountTreeController and SnapController are absent', async () => { + it('is constructed and initialized by default while AccountTreeController and SnapController are absent', async () => { const accountId = 'account-1'; const nativeAssetId = 'eip155:1/slip44:60'; const wallet = new Wallet({ @@ -245,15 +244,35 @@ describe('assetsController', () => { await wallet.init(); - // AssetsController is constructed before NetworkEnablementController. - // init() reads that controller's default state, which enables mainnet. - const assets = await instance.getAssets([ - { id: accountId } as Parameters[0][number], - ]); + // Without `start()`, a forced fetch reads what is already in state. + const assets = await instance.getAssets( + [ + { id: accountId } as Parameters< + AssetsController['getAssets'] + >[0][number], + ], + { chainIds: ['eip155:1'], forceUpdate: true }, + ); expect(Object.keys(assets[accountId])).toStrictEqual([nativeAssetId]); }); + it('exposes AssetsController:start on the wallet messenger', async () => { + const wallet = new Wallet({ + instanceOptions: getInstanceOptions(), + }); + wallets.push(wallet); + await wallet.init(); + + expect( + await ( + wallet.messenger as unknown as { + call(action: 'AssetsController:start'): Promise; + } + ).call('AssetsController:start'), + ).toBeUndefined(); + }); + it('exposes AssetsController state on the wallet messenger by default', () => { const wallet = new Wallet({ instanceOptions: getInstanceOptions(), diff --git a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts index 65d9abc13b9..6f2672837cb 100644 --- a/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts +++ b/packages/wallet/src/initialization/instances/assets-controller/assets-controller.ts @@ -9,14 +9,14 @@ export type { AssetsControllerInstanceOptions } from './types.js'; /** * Initialization configuration for `AssetsController`. * - * `AccountTreeController` and `SnapController` are not Wallet defaults. - * Construction does not call them. Account-group reads fall back to - * `AccountsController` until `AccountTreeController` is registered, and - * `AccountsController:selectedAccountChange` refreshes tracked assets only - * in that fallback. Snap discovery leaves snap chains empty until - * `SnapController` is registered. - * `AssetsController.init()` reads `NetworkEnablementController` during - * `Wallet.init()`, after every default controller exists. + * The controller's constructor does not call other controllers, so the + * Wallet can construct it before controllers it depends on that are not + * Wallet defaults yet, such as `AccountTreeController` and `SnapController`. + * + * Asset tracking does not start on its own. Once the client has registered + * and initialized those controllers, it must call `AssetsController:start` + * (or `AssetsController.start()`). That reads enabled networks, creates the + * data sources and subscribes to external events. */ export const assetsController: InitializationConfiguration< AssetsController, @@ -58,7 +58,6 @@ export const assetsController: InitializationConfiguration< 'AccountActivityService:balanceUpdated', 'AccountActivityService:statusChanged', 'AccountsController:accountBalancesUpdated', - 'AccountsController:selectedAccountChange', 'AccountTreeController:initialized', 'AccountTreeController:selectedAccountGroupChange', 'AccountTreeController:uninitialized', diff --git a/tsc-suppressions.json b/tsc-suppressions.json index 4dea4dfd106..cb90870ce24 100644 --- a/tsc-suppressions.json +++ b/tsc-suppressions.json @@ -113,6 +113,9 @@ "TS2532": { "count": 1 }, + "TS2554": { + "count": 9 + }, "TS2739": { "count": 4 }, From 8093c7c840b8998cc884cfaacc38a94ea234597c Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 4 Nov 2026 08:35:42 +0000 Subject: [PATCH 22/24] fix(assets-controller): wait for Accounts API chains before the first fetch start() created AccountsApiDataSource and force-fetched before its supported-networks request settled, so the first fetch ran with an empty activeChains list and skipped the Accounts API. Co-authored-by: Xiaoming Wang --- packages/assets-controller/CHANGELOG.md | 3 +- .../src/AssetsController.test.ts | 72 +++++++++++++++++++ .../assets-controller/src/AssetsController.ts | 67 +++++++++++------ .../src/data-sources/AccountsApiDataSource.ts | 24 ++++++- 4 files changed, 144 insertions(+), 22 deletions(-) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 39350a8d203..61556ea9a04 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add `start()` and the `AssetsController:start` messenger action ([#10885](https://github.com/MetaMask/core/pull/10885)) - - `start()` reads enabled networks from `NetworkEnablementController:getState`, creates the data sources, subscribes to external events and to `subscribeToBasicFunctionalityChange`, runs the unlock cleanups when the keyring is already unlocked, and starts asset tracking when the UI is open, the keyring is unlocked and the account tree is initialized. + - `start()` reads enabled networks from `NetworkEnablementController:getState`, creates the data sources, subscribes to external events and to `subscribeToBasicFunctionalityChange`, runs the unlock cleanups when the keyring is already unlocked, waits for the Accounts API supported-networks fetch to settle, and starts asset tracking when the UI is open, the keyring is unlocked and the account tree is initialized. - Call it once the client has constructed and initialized the controllers `AssetsController` depends on, such as `AccountTreeController` and `SnapController`. Repeated calls return the same promise; calls after `destroy()` do nothing. - Export the `AssetsControllerStartAction` type ([#10885](https://github.com/MetaMask/core/pull/10885)) @@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Wait for `AccountsApiDataSource` to finish its initial supported-networks fetch before `start()` force-fetches balances, so the first fetch can use the Accounts API ([#10885](https://github.com/MetaMask/core/pull/10885)) - Stop removing launchpad tokens as spam: EVM ERC-20s below the occurrence floor are now kept when Token API `/v2/assets` identifies them as launchpad tokens and their security data does not classify them as malicious ([#11251](https://github.com/MetaMask/core/pull/11251)) - Applies to metadata enrichment, websocket balance updates, and the unlock spam cleanup. diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index c6cc53ff2fb..bf6b6d59a3a 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -964,6 +964,78 @@ describe('AssetsController', () => { ); }); + it('waits for Accounts API active chains before the first forced fetch', async () => { + let resolveSupportedNetworks: (value: { + fullSupport: number[]; + partialSupport: number[]; + }) => void = () => undefined; + const supportedNetworks = new Promise<{ + fullSupport: number[]; + partialSupport: number[]; + }>((resolve) => { + resolveSupportedNetworks = resolve; + }); + const fetchV5MultiAccountBalances = vi.fn().mockResolvedValue({ + balances: [], + unprocessedNetworks: [], + }); + const queryApiClient = { + ...createMockQueryApiClient(), + accounts: { + fetchV2SupportedNetworks: vi.fn(() => supportedNetworks), + fetchV5MultiAccountBalances, + fetchV6MultiAccountBalances: vi.fn().mockResolvedValue({ + accounts: [], + unprocessedNetworks: [], + unprocessedIncludeAssetIds: [], + }), + }, + } as unknown as ApiPlatformClient; + + await withController( + { + skipStart: true, + queryApiClient, + clientControllerState: { isUiOpen: true }, + }, + async ({ controller, messenger }) => { + messenger.publish('AccountTreeController:initialized', {} as never); + messenger.publish('KeyringController:unlock'); + await flushPromises(); + + const getAssetsSpy = vi.spyOn(controller, 'getAssets'); + let started = false; + const startPromise = controller.start().then(() => { + started = true; + }); + await flushPromises(); + + // Supported networks is still in flight. The startup force-fetch + // must not run yet, or it assigns chains while Accounts API + // `activeChains` is empty and skips that source. + expect(started).toBe(false); + expect(getAssetsSpy).not.toHaveBeenCalled(); + expect(fetchV5MultiAccountBalances).not.toHaveBeenCalled(); + + resolveSupportedNetworks({ fullSupport: [1], partialSupport: [] }); + await startPromise; + await flushPromises(); + + expect(getAssetsSpy).toHaveBeenCalledWith( + [expect.objectContaining({ id: MOCK_ACCOUNT_ID })], + expect.objectContaining({ forceUpdate: true }), + ); + expect(fetchV5MultiAccountBalances).toHaveBeenCalledWith( + expect.arrayContaining([ + 'eip155:1:0x1234567890123456789012345678901234567890', + ]), + undefined, + expect.anything(), + ); + }, + ); + }); + it('drops balances outside account scopes on start when already unlocked', async () => { const solanaAssetId = 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501' as Caip19AssetId; diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index 2f7540800dc..c32ffbbd7a3 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -908,6 +908,12 @@ export class AssetsController extends BaseController< /** The pending or settled `start()` call, so repeated calls share it. */ #startPromise: Promise | undefined; + /** + * Set while `start()` is waiting to assign chains. Chain reports during + * that wait must not subscribe, or `#startTracking` skips its forced fetch. + */ + #deferActiveChainSubscriptions = false; + /** Whether `destroy()` has been called. */ #isDestroyed = false; @@ -1088,8 +1094,9 @@ export class AssetsController extends BaseController< * The constructor does not touch other controllers. `start()` reads enabled * networks, creates the data sources (whose constructors read network, snap * and feature-flag state), subscribes to external events and to - * basic-functionality changes, and begins asset tracking when the UI is - * open, the keyring is unlocked and the account tree is initialized. + * basic-functionality changes, waits for the Accounts API supported-networks + * fetch to settle, and begins asset tracking when the UI is open, the + * keyring is unlocked and the account tree is initialized. * * Call it after the client has constructed and initialized its own * controllers, such as `AccountTreeController` and `SnapController`. @@ -1113,21 +1120,33 @@ export class AssetsController extends BaseController< log('Starting AssetsController'); - this.#initializeState(); - this.#sources = this.#createSources(); - this.#subscribeToEvents(); - this.#subscribeToBasicFunctionality(); + // Accounts API reports its chains asynchronously. Hold those reports + // until the wait below finishes, then let `#updateActive` assign chains + // and force-fetch with `activeChains` already populated. + this.#deferActiveChainSubscriptions = true; + try { + this.#initializeState(); + this.#sources = this.#createSources(); + this.#subscribeToEvents(); + this.#subscribeToBasicFunctionality(); + + // The keyring may already be unlocked when `start()` runs, in which case + // the `KeyringController:unlock` handler that runs these cleanups was not + // subscribed in time. + if (this.#isKeyringUnlocked()) { + try { + this.#runOutOfScopeBalanceCleanup(); + } catch { + /* Do nothing */ + } + await this.#runSpamCleanup(); + } - // The keyring may already be unlocked when `start()` runs, in which case - // the `KeyringController:unlock` handler that runs these cleanups was not - // subscribed in time. - if (this.#isKeyringUnlocked()) { - try { - this.#runOutOfScopeBalanceCleanup(); - } catch { - /* Do nothing */ + if (!this.#isDestroyed) { + await this.#sources.accountsApiDataSource.whenActiveChainsInitialized(); } - await this.#runSpamCleanup(); + } finally { + this.#deferActiveChainSubscriptions = false; } if (this.#isDestroyed) { @@ -1141,8 +1160,9 @@ export class AssetsController extends BaseController< * * Data source constructors read other controllers and may report active * chains synchronously. `#handleActiveChainsUpdate` ignores those reports - * until `#sources` is assigned, and `start()` subscribes afterwards with - * every source's chains. + * until `#sources` is assigned. The Accounts API report arrives later, + * after its supported-networks fetch; `start()` waits for that fetch, then + * subscribes with every source's chains. * * @returns The data sources and middlewares. */ @@ -1704,9 +1724,16 @@ export class AssetsController extends BaseController< activeChains: ChainId[], previousChains: ChainId[], ): void { - // Data source constructors report their chains while `start()` is still - // creating them. `start()` subscribes afterwards with every source's chains. - if (!this.#sources) { + // Constructors report chains before `#sources` is assigned, and the + // Accounts API reports them while `start()` is still waiting on supported + // networks. `start()` subscribes afterwards, once every source's chains + // are known. Ignore reports after `destroy()` too: the in-flight + // supported-networks fetch can still resolve. + if ( + !this.#sources || + this.#isDestroyed || + this.#deferActiveChainSubscriptions + ) { return; } if (!this.#isUiOpen() || !this.#isKeyringUnlocked() || !this.#isEnabled()) { diff --git a/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts b/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts index 1894998a418..e54fc766322 100644 --- a/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts +++ b/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts @@ -281,6 +281,12 @@ export class AccountsApiDataSource extends AbstractDataSource< /** Chains refresh timer */ #chainsRefreshTimer: ReturnType | null = null; + /** + * Resolves when the first supported-networks fetch has settled. + * `activeChains` stays empty until then. + */ + readonly #activeChainsInitialized: Promise; + constructor(options: AccountsApiDataSourceOptions) { super(CONTROLLER_NAME, { ...defaultState, @@ -315,7 +321,23 @@ export class AccountsApiDataSource extends AbstractDataSource< (state) => getMigrationStages(state.remoteFeatureFlags).join(','), ); - this.#initializeActiveChains().catch(console.error); + // Settles on success or failure. Callers that fetch before this resolves + // see an empty `activeChains` and skip the Accounts API. + this.#activeChainsInitialized = this.#initializeActiveChains().catch( + console.error, + ); + } + + /** + * Resolves once the initial supported-networks fetch has settled. + * + * `AssetsController.start` awaits this before the first forced fetch so + * chain assignment sees the chains this source can actually serve. + * + * @returns A promise that resolves when initialization has settled. + */ + whenActiveChainsInitialized(): Promise { + return this.#activeChainsInitialized; } /** From 8b8e85c46b2cf80ce3171846fc5a997b1178d576 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 4 Nov 2026 08:40:14 +0000 Subject: [PATCH 23/24] Revert "fix(assets-controller): wait for Accounts API chains before the first fetch" This reverts commit 8093c7c840b8998cc884cfaacc38a94ea234597c. Co-authored-by: Xiaoming Wang --- packages/assets-controller/CHANGELOG.md | 3 +- .../src/AssetsController.test.ts | 72 ------------------- .../assets-controller/src/AssetsController.ts | 67 ++++++----------- .../src/data-sources/AccountsApiDataSource.ts | 24 +------ 4 files changed, 22 insertions(+), 144 deletions(-) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 61556ea9a04..39350a8d203 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add `start()` and the `AssetsController:start` messenger action ([#10885](https://github.com/MetaMask/core/pull/10885)) - - `start()` reads enabled networks from `NetworkEnablementController:getState`, creates the data sources, subscribes to external events and to `subscribeToBasicFunctionalityChange`, runs the unlock cleanups when the keyring is already unlocked, waits for the Accounts API supported-networks fetch to settle, and starts asset tracking when the UI is open, the keyring is unlocked and the account tree is initialized. + - `start()` reads enabled networks from `NetworkEnablementController:getState`, creates the data sources, subscribes to external events and to `subscribeToBasicFunctionalityChange`, runs the unlock cleanups when the keyring is already unlocked, and starts asset tracking when the UI is open, the keyring is unlocked and the account tree is initialized. - Call it once the client has constructed and initialized the controllers `AssetsController` depends on, such as `AccountTreeController` and `SnapController`. Repeated calls return the same promise; calls after `destroy()` do nothing. - Export the `AssetsControllerStartAction` type ([#10885](https://github.com/MetaMask/core/pull/10885)) @@ -23,7 +23,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Wait for `AccountsApiDataSource` to finish its initial supported-networks fetch before `start()` force-fetches balances, so the first fetch can use the Accounts API ([#10885](https://github.com/MetaMask/core/pull/10885)) - Stop removing launchpad tokens as spam: EVM ERC-20s below the occurrence floor are now kept when Token API `/v2/assets` identifies them as launchpad tokens and their security data does not classify them as malicious ([#11251](https://github.com/MetaMask/core/pull/11251)) - Applies to metadata enrichment, websocket balance updates, and the unlock spam cleanup. diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index bf6b6d59a3a..c6cc53ff2fb 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -964,78 +964,6 @@ describe('AssetsController', () => { ); }); - it('waits for Accounts API active chains before the first forced fetch', async () => { - let resolveSupportedNetworks: (value: { - fullSupport: number[]; - partialSupport: number[]; - }) => void = () => undefined; - const supportedNetworks = new Promise<{ - fullSupport: number[]; - partialSupport: number[]; - }>((resolve) => { - resolveSupportedNetworks = resolve; - }); - const fetchV5MultiAccountBalances = vi.fn().mockResolvedValue({ - balances: [], - unprocessedNetworks: [], - }); - const queryApiClient = { - ...createMockQueryApiClient(), - accounts: { - fetchV2SupportedNetworks: vi.fn(() => supportedNetworks), - fetchV5MultiAccountBalances, - fetchV6MultiAccountBalances: vi.fn().mockResolvedValue({ - accounts: [], - unprocessedNetworks: [], - unprocessedIncludeAssetIds: [], - }), - }, - } as unknown as ApiPlatformClient; - - await withController( - { - skipStart: true, - queryApiClient, - clientControllerState: { isUiOpen: true }, - }, - async ({ controller, messenger }) => { - messenger.publish('AccountTreeController:initialized', {} as never); - messenger.publish('KeyringController:unlock'); - await flushPromises(); - - const getAssetsSpy = vi.spyOn(controller, 'getAssets'); - let started = false; - const startPromise = controller.start().then(() => { - started = true; - }); - await flushPromises(); - - // Supported networks is still in flight. The startup force-fetch - // must not run yet, or it assigns chains while Accounts API - // `activeChains` is empty and skips that source. - expect(started).toBe(false); - expect(getAssetsSpy).not.toHaveBeenCalled(); - expect(fetchV5MultiAccountBalances).not.toHaveBeenCalled(); - - resolveSupportedNetworks({ fullSupport: [1], partialSupport: [] }); - await startPromise; - await flushPromises(); - - expect(getAssetsSpy).toHaveBeenCalledWith( - [expect.objectContaining({ id: MOCK_ACCOUNT_ID })], - expect.objectContaining({ forceUpdate: true }), - ); - expect(fetchV5MultiAccountBalances).toHaveBeenCalledWith( - expect.arrayContaining([ - 'eip155:1:0x1234567890123456789012345678901234567890', - ]), - undefined, - expect.anything(), - ); - }, - ); - }); - it('drops balances outside account scopes on start when already unlocked', async () => { const solanaAssetId = 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501' as Caip19AssetId; diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index c32ffbbd7a3..2f7540800dc 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -908,12 +908,6 @@ export class AssetsController extends BaseController< /** The pending or settled `start()` call, so repeated calls share it. */ #startPromise: Promise | undefined; - /** - * Set while `start()` is waiting to assign chains. Chain reports during - * that wait must not subscribe, or `#startTracking` skips its forced fetch. - */ - #deferActiveChainSubscriptions = false; - /** Whether `destroy()` has been called. */ #isDestroyed = false; @@ -1094,9 +1088,8 @@ export class AssetsController extends BaseController< * The constructor does not touch other controllers. `start()` reads enabled * networks, creates the data sources (whose constructors read network, snap * and feature-flag state), subscribes to external events and to - * basic-functionality changes, waits for the Accounts API supported-networks - * fetch to settle, and begins asset tracking when the UI is open, the - * keyring is unlocked and the account tree is initialized. + * basic-functionality changes, and begins asset tracking when the UI is + * open, the keyring is unlocked and the account tree is initialized. * * Call it after the client has constructed and initialized its own * controllers, such as `AccountTreeController` and `SnapController`. @@ -1120,33 +1113,21 @@ export class AssetsController extends BaseController< log('Starting AssetsController'); - // Accounts API reports its chains asynchronously. Hold those reports - // until the wait below finishes, then let `#updateActive` assign chains - // and force-fetch with `activeChains` already populated. - this.#deferActiveChainSubscriptions = true; - try { - this.#initializeState(); - this.#sources = this.#createSources(); - this.#subscribeToEvents(); - this.#subscribeToBasicFunctionality(); - - // The keyring may already be unlocked when `start()` runs, in which case - // the `KeyringController:unlock` handler that runs these cleanups was not - // subscribed in time. - if (this.#isKeyringUnlocked()) { - try { - this.#runOutOfScopeBalanceCleanup(); - } catch { - /* Do nothing */ - } - await this.#runSpamCleanup(); - } + this.#initializeState(); + this.#sources = this.#createSources(); + this.#subscribeToEvents(); + this.#subscribeToBasicFunctionality(); - if (!this.#isDestroyed) { - await this.#sources.accountsApiDataSource.whenActiveChainsInitialized(); + // The keyring may already be unlocked when `start()` runs, in which case + // the `KeyringController:unlock` handler that runs these cleanups was not + // subscribed in time. + if (this.#isKeyringUnlocked()) { + try { + this.#runOutOfScopeBalanceCleanup(); + } catch { + /* Do nothing */ } - } finally { - this.#deferActiveChainSubscriptions = false; + await this.#runSpamCleanup(); } if (this.#isDestroyed) { @@ -1160,9 +1141,8 @@ export class AssetsController extends BaseController< * * Data source constructors read other controllers and may report active * chains synchronously. `#handleActiveChainsUpdate` ignores those reports - * until `#sources` is assigned. The Accounts API report arrives later, - * after its supported-networks fetch; `start()` waits for that fetch, then - * subscribes with every source's chains. + * until `#sources` is assigned, and `start()` subscribes afterwards with + * every source's chains. * * @returns The data sources and middlewares. */ @@ -1724,16 +1704,9 @@ export class AssetsController extends BaseController< activeChains: ChainId[], previousChains: ChainId[], ): void { - // Constructors report chains before `#sources` is assigned, and the - // Accounts API reports them while `start()` is still waiting on supported - // networks. `start()` subscribes afterwards, once every source's chains - // are known. Ignore reports after `destroy()` too: the in-flight - // supported-networks fetch can still resolve. - if ( - !this.#sources || - this.#isDestroyed || - this.#deferActiveChainSubscriptions - ) { + // Data source constructors report their chains while `start()` is still + // creating them. `start()` subscribes afterwards with every source's chains. + if (!this.#sources) { return; } if (!this.#isUiOpen() || !this.#isKeyringUnlocked() || !this.#isEnabled()) { diff --git a/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts b/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts index e54fc766322..1894998a418 100644 --- a/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts +++ b/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts @@ -281,12 +281,6 @@ export class AccountsApiDataSource extends AbstractDataSource< /** Chains refresh timer */ #chainsRefreshTimer: ReturnType | null = null; - /** - * Resolves when the first supported-networks fetch has settled. - * `activeChains` stays empty until then. - */ - readonly #activeChainsInitialized: Promise; - constructor(options: AccountsApiDataSourceOptions) { super(CONTROLLER_NAME, { ...defaultState, @@ -321,23 +315,7 @@ export class AccountsApiDataSource extends AbstractDataSource< (state) => getMigrationStages(state.remoteFeatureFlags).join(','), ); - // Settles on success or failure. Callers that fetch before this resolves - // see an empty `activeChains` and skip the Accounts API. - this.#activeChainsInitialized = this.#initializeActiveChains().catch( - console.error, - ); - } - - /** - * Resolves once the initial supported-networks fetch has settled. - * - * `AssetsController.start` awaits this before the first forced fetch so - * chain assignment sees the chains this source can actually serve. - * - * @returns A promise that resolves when initialization has settled. - */ - whenActiveChainsInitialized(): Promise { - return this.#activeChainsInitialized; + this.#initializeActiveChains().catch(console.error); } /** From 713f7d75932b6fcf7c0d62e20ac19dd0e695af32 Mon Sep 17 00:00:00 2001 From: Xiaoming Wang Date: Wed, 4 Nov 2026 16:44:49 +0800 Subject: [PATCH 24/24] fix(assets-controller): wait for Accounts API chains before the first fetch `start()` created `AccountsApiDataSource` and could begin tracking before the source's async supported-networks fetch resolved. The first forced fetch then saw no Accounts API chains and routed every chain to the Snap/RPC lane. `start()` now awaits `AccountsApiDataSource.refreshActiveChains()` (which shares the in-flight `fetchQuery` request) alongside the unlock cleanups, and ignores data source chain updates until it has finished, so an early update cannot subscribe ahead of the startup refresh. Co-Authored-By: Claude Opus 5.5 --- packages/assets-controller/CHANGELOG.md | 2 +- .../src/AssetsController.test.ts | 48 ++++++++++++++++++ .../assets-controller/src/AssetsController.ts | 50 +++++++++++++------ 3 files changed, 85 insertions(+), 15 deletions(-) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 39350a8d203..d76a8da86d8 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add `start()` and the `AssetsController:start` messenger action ([#10885](https://github.com/MetaMask/core/pull/10885)) - - `start()` reads enabled networks from `NetworkEnablementController:getState`, creates the data sources, subscribes to external events and to `subscribeToBasicFunctionalityChange`, runs the unlock cleanups when the keyring is already unlocked, and starts asset tracking when the UI is open, the keyring is unlocked and the account tree is initialized. + - `start()` reads enabled networks from `NetworkEnablementController:getState`, creates the data sources, subscribes to external events and to `subscribeToBasicFunctionalityChange`, waits for the Accounts API supported networks, runs the unlock cleanups when the keyring is already unlocked, and starts asset tracking when the UI is open, the keyring is unlocked and the account tree is initialized. - Call it once the client has constructed and initialized the controllers `AssetsController` depends on, such as `AccountTreeController` and `SnapController`. Repeated calls return the same promise; calls after `destroy()` do nothing. - Export the `AssetsControllerStartAction` type ([#10885](https://github.com/MetaMask/core/pull/10885)) diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index c6cc53ff2fb..c10f365bcc8 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -964,6 +964,54 @@ describe('AssetsController', () => { ); }); + it('waits for Accounts API supported networks before the first forced fetch', async () => { + let supportedNetworksResolved = false; + const queryApiClient = { + ...createMockQueryApiClient(), + accounts: { + fetchV2SupportedNetworks: vi.fn( + () => + new Promise((resolve) => { + setTimeout(() => { + supportedNetworksResolved = true; + resolve({ fullSupport: [1], partialSupport: [] }); + }, 10); + }), + ), + fetchV5MultiAccountBalances: vi.fn().mockResolvedValue({ + balances: [], + unprocessedNetworks: [], + }), + }, + } as unknown as ApiPlatformClient; + await withController( + { + skipStart: true, + queryApiClient, + clientControllerState: { isUiOpen: true }, + }, + async ({ controller, messenger }) => { + messenger.publish('AccountTreeController:initialized', {} as never); + messenger.publish('KeyringController:unlock'); + const supportedNetworksResolvedAtFirstFetch: boolean[] = []; + const getAssets = controller.getAssets.bind(controller); + vi.spyOn(controller, 'getAssets').mockImplementation( + async (...args) => { + supportedNetworksResolvedAtFirstFetch.push( + supportedNetworksResolved, + ); + return getAssets(...args); + }, + ); + + await controller.start(); + await flushPromises(); + + expect(supportedNetworksResolvedAtFirstFetch[0]).toBe(true); + }, + ); + }); + it('drops balances outside account scopes on start when already unlocked', async () => { const solanaAssetId = 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501' as Caip19AssetId; diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index 2f7540800dc..bd7c2cc30dc 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -908,6 +908,9 @@ export class AssetsController extends BaseController< /** The pending or settled `start()` call, so repeated calls share it. */ #startPromise: Promise | undefined; + /** Whether `start()` has finished creating sources and subscribing. */ + #isStarted = false; + /** Whether `destroy()` has been called. */ #isDestroyed = false; @@ -1118,24 +1121,42 @@ export class AssetsController extends BaseController< this.#subscribeToEvents(); this.#subscribeToBasicFunctionality(); - // The keyring may already be unlocked when `start()` runs, in which case - // the `KeyringController:unlock` handler that runs these cleanups was not - // subscribed in time. - if (this.#isKeyringUnlocked()) { - try { - this.#runOutOfScopeBalanceCleanup(); - } catch { - /* Do nothing */ - } - await this.#runSpamCleanup(); - } + await Promise.all([ + // AccountsApiDataSource fetches its supported networks asynchronously + // from its constructor. Wait for them so the first forced fetch can use + // the Accounts API instead of routing every chain to the slow lane. + // `fetchQuery` shares the in-flight request, so this does not issue a + // second one. Failures are logged by the data source and do not reject. + this.#getSources().accountsApiDataSource.refreshActiveChains(), + this.#runUnlockCleanupsIfUnlocked(), + ]); if (this.#isDestroyed) { return; } + // Chain updates reported while starting are ignored; `#updateActive` + // subscribes with every source's current chains. + this.#isStarted = true; this.#updateActive(); } + /** + * Run the unlock cleanups when the keyring is already unlocked. `start()` + * subscribes to `KeyringController:unlock` too late to see an unlock that + * happened before it. + */ + async #runUnlockCleanupsIfUnlocked(): Promise { + if (!this.#isKeyringUnlocked()) { + return; + } + try { + this.#runOutOfScopeBalanceCleanup(); + } catch { + /* Do nothing */ + } + await this.#runSpamCleanup(); + } + /** * Create every data source and middleware. * @@ -1704,9 +1725,10 @@ export class AssetsController extends BaseController< activeChains: ChainId[], previousChains: ChainId[], ): void { - // Data source constructors report their chains while `start()` is still - // creating them. `start()` subscribes afterwards with every source's chains. - if (!this.#sources) { + // Data sources report their chains while `start()` is still running. + // `start()` subscribes afterwards with every source's chains. Subscribing + // here first would make `#startTracking` skip the startup refresh. + if (!this.#isStarted) { return; } if (!this.#isUiOpen() || !this.#isKeyringUnlocked() || !this.#isEnabled()) {