diff --git a/packages/accounts-controller/CHANGELOG.md b/packages/accounts-controller/CHANGELOG.md index fbf98f64b2d..e68e2aebfb5 100644 --- a/packages/accounts-controller/CHANGELOG.md +++ b/packages/accounts-controller/CHANGELOG.md @@ -7,9 +7,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `init` method and `AccountsController:init` messenger action ([#10191](https://github.com/MetaMask/core/pull/10191)) + - Loads accounts from the current keyring state, intended to pair with `clearState` for wallet reset flows (`clearState` then `init`). + - Idempotent: subsequent calls before `clearState` are no-ops. + - Fires `AccountsController:accountsAdded` for accounts newly discovered during initialization. + - Fires `AccountsController:initialized` (with current state as payload) when initialization completes. +- Add `AccountsController:initialized` event - fired by `init()` when the controller finishes its first full account sync ([#10191](https://github.com/MetaMask/core/pull/10191)) +- Add `AccountsController:uninitialized` event - fired by `clearState()` to signal that `init()` must be called again ([#10191](https://github.com/MetaMask/core/pull/10191)) + ### Changed - Bump `uuid` from `^8.3.2` to `^9.0.1` ([#10117](https://github.com/MetaMask/core/pull/10117)) +- `clearState()` now resets the initialized flag and fires `AccountsController:uninitialized` ([#10191](https://github.com/MetaMask/core/pull/10191)) + +### Removed + +- **BREAKING:** Remove `updateAccounts` method and `AccountsController:updateAccounts` messenger action ([#10191](https://github.com/MetaMask/core/pull/10191)) + - Use `AccountsController:init` instead, it performs the same full keyring sync and additionally fires lifecycle events. ## [40.0.0] diff --git a/packages/accounts-controller/src/AccountsController-method-action-types.ts b/packages/accounts-controller/src/AccountsController-method-action-types.ts index ba571bac364..5605fbb1f66 100644 --- a/packages/accounts-controller/src/AccountsController-method-action-types.ts +++ b/packages/accounts-controller/src/AccountsController-method-action-types.ts @@ -151,16 +151,19 @@ export type AccountsControllerUpdateAccountMetadataAction = { }; /** - * Updates the internal accounts list by retrieving normal and snap accounts, - * removing duplicates, and updating the metadata of each account. + * Initializes the controller by loading accounts from the current keyring + * state. Any accounts present in the keyrings but missing from the + * controller's state are added; existing accounts are preserved. + * + * Intended to be called after {@link clearState} to restore accounts from + * the keyrings (i.e. `clearState` then `init`), or on first startup. * * @deprecated This method is deprecated and will be removed in a future version. * Use `AccountTreeController`, `MultichainAccountService`, or the Keyring API v2 instead. - * @returns A Promise that resolves when the accounts have been updated. */ -export type AccountsControllerUpdateAccountsAction = { - type: `AccountsController:updateAccounts`; - handler: AccountsController['updateAccounts']; +export type AccountsControllerInitAction = { + type: `AccountsController:init`; + handler: AccountsController['init']; }; /** @@ -201,6 +204,6 @@ export type AccountsControllerMethodActions = | AccountsControllerSetAccountNameAction | AccountsControllerSetAccountNameAndSelectAccountAction | AccountsControllerUpdateAccountMetadataAction - | AccountsControllerUpdateAccountsAction + | AccountsControllerInitAction | AccountsControllerLoadBackupAction | AccountsControllerClearStateAction; diff --git a/packages/accounts-controller/src/AccountsController.test.ts b/packages/accounts-controller/src/AccountsController.test.ts index b8deb353ba2..0f96e16b320 100644 --- a/packages/accounts-controller/src/AccountsController.test.ts +++ b/packages/accounts-controller/src/AccountsController.test.ts @@ -2331,7 +2331,7 @@ describe('AccountsController', () => { }); }); - describe('updateAccounts', () => { + describe('#sync', () => { const mockAddress1 = '0x123'; const mockAddress2 = '0x456'; let mockSnapAccount: InternalAccount; @@ -2434,7 +2434,7 @@ describe('AccountsController', () => { ]; mockUUIDWithNormalAccounts(expectedAccounts); - await accountsController.updateAccounts(); + accountsController.init(); expect(accountsController.listMultichainAccounts()).toStrictEqual( expectedAccounts, @@ -2505,7 +2505,7 @@ describe('AccountsController', () => { const expectedAccounts = [expectedAccount1, expectedAccount2]; - await accountsController.updateAccounts(); + accountsController.init(); expect( accountsController @@ -2539,7 +2539,7 @@ describe('AccountsController', () => { const expectedAccounts: InternalAccount[] = []; - await accountsController.updateAccounts(); + accountsController.init(); expect(accountsController.listMultichainAccounts()).toStrictEqual( expectedAccounts, @@ -2605,7 +2605,7 @@ describe('AccountsController', () => { ]; mockUUIDWithNormalAccounts(expectedAccounts); - await accountsController.updateAccounts(); + accountsController.init(); expect(accountsController.listMultichainAccounts()).toStrictEqual( expectedAccounts, @@ -2678,7 +2678,7 @@ describe('AccountsController', () => { }), ]; - await accountsController.updateAccounts(); + accountsController.init(); expect(accountsController.listMultichainAccounts()).toStrictEqual( expectedAccounts, @@ -2744,7 +2744,7 @@ describe('AccountsController', () => { }), ]; - await accountsController.updateAccounts(); + accountsController.init(); expect(accountsController.listMultichainAccounts()).toStrictEqual( expectedAccounts, @@ -2810,7 +2810,7 @@ describe('AccountsController', () => { }), ]; - await accountsController.updateAccounts(); + accountsController.init(); expect(accountsController.listMultichainAccounts()).toStrictEqual( expectedAccounts, @@ -2883,7 +2883,7 @@ describe('AccountsController', () => { }), ]; - await accountsController.updateAccounts(); + accountsController.init(); expect(accountsController.listMultichainAccounts()).toStrictEqual( expectedAccounts, @@ -2950,7 +2950,7 @@ describe('AccountsController', () => { }), ]; - await accountsController.updateAccounts(); + accountsController.init(); expect( accountsController @@ -2991,7 +2991,7 @@ describe('AccountsController', () => { messenger, }); - await expect(accountsController.updateAccounts()).rejects.toThrow( + expect(() => accountsController.init()).toThrow( 'Unknown keyring unknown', ); }); @@ -3094,7 +3094,7 @@ describe('AccountsController', () => { messenger, }); - await accountsController.updateAccounts(); + accountsController.init(); const selectedAccount = accountsController.getSelectedAccount(); @@ -3180,7 +3180,7 @@ describe('AccountsController', () => { }); // Will automatically re-create the internal account list. - await accountsController.updateAccounts(); + accountsController.init(); const account = accountsController.getAccount(mockHdSnapAccount.id); expect(account?.options).toStrictEqual({ @@ -3270,7 +3270,7 @@ describe('AccountsController', () => { }); // Will automatically re-create the internal account list. - await accountsController.updateAccounts(); + accountsController.init(); // This account has been skipped. expect( @@ -3334,7 +3334,7 @@ describe('AccountsController', () => { messenger, }); - await accountsController.updateAccounts(); + accountsController.init(); const accounts = accountsController.listMultichainAccounts(); expect(accounts).toHaveLength(1); @@ -3386,7 +3386,7 @@ describe('AccountsController', () => { messenger, }); - await accountsController.updateAccounts(); + accountsController.init(); expect(accountsController.listMultichainAccounts()).toStrictEqual([]); }); @@ -3396,6 +3396,206 @@ describe('AccountsController', () => { ); }); + describe('init', () => { + function setupInitTest(): ReturnType { + const messenger = buildMessenger(); + messenger.registerActionHandler( + 'KeyringController:getState', + mockGetState.mockReturnValue({ keyrings: [] }), + ); + messenger.registerActionHandler( + 'KeyringController:getKeyringsByType', + mockGetKeyringByType.mockReturnValue([]), + ); + return setupAccountsController({ messenger }); + } + + it('runs the internal sync', async () => { + const { accountsController } = setupInitTest(); + + // Verify init completes without throwing — the sync ran if no error occurs + expect(() => accountsController.init()).not.toThrow(); + }); + + it('is a no-op on subsequent calls', async () => { + const { accountsController, messenger } = setupInitTest(); + + const initializedListener = jest.fn(); + messenger.subscribe( + 'AccountsController:initialized', + initializedListener, + ); + + accountsController.init(); + accountsController.init(); + + // initialized event fires exactly once + expect(initializedListener).toHaveBeenCalledTimes(1); + }); + + it('fires accountsAdded for accounts not previously in state', async () => { + const messenger = buildMessenger(); + messenger.registerActionHandler( + 'KeyringController:getState', + mockGetState.mockReturnValue({ + keyrings: [ + { + type: KeyringTypes.hd, + accounts: [mockAccount.address], + metadata: { id: 'mock-id', name: 'mock-name' }, + }, + ], + }), + ); + messenger.registerActionHandler( + 'KeyringController:getKeyringsByType', + mockGetKeyringByType.mockReturnValue([]), + ); + mockUUIDWithNormalAccounts([mockAccount]); + + const { accountsController } = setupAccountsController({ + initialState: getDefaultAccountsControllerState(), + messenger, + }); + + const accountsAddedListener = jest.fn(); + messenger.subscribe( + 'AccountsController:accountsAdded', + accountsAddedListener, + ); + + accountsController.init(); + + expect(accountsAddedListener).toHaveBeenCalledTimes(1); + expect(accountsAddedListener.mock.calls[0][0]).toHaveLength(1); + }); + + it('does not fire accountsAdded for accounts already in state', async () => { + const messenger = buildMessenger(); + messenger.registerActionHandler( + 'KeyringController:getState', + mockGetState.mockReturnValue({ + keyrings: [ + { + type: KeyringTypes.hd, + accounts: [mockAccount.address], + metadata: { id: 'mock-id', name: 'mock-name' }, + }, + ], + }), + ); + messenger.registerActionHandler( + 'KeyringController:getKeyringsByType', + mockGetKeyringByType.mockReturnValue([]), + ); + mockUUIDWithNormalAccounts([mockAccount]); + + const { accountsController } = setupAccountsController({ + initialState: { + internalAccounts: { + accounts: { [mockAccount.id]: mockAccount }, + selectedAccount: mockAccount.id, + }, + accountIdByAddress: { [mockAccount.address]: mockAccount.id }, + }, + messenger, + }); + + const accountsAddedListener = jest.fn(); + messenger.subscribe( + 'AccountsController:accountsAdded', + accountsAddedListener, + ); + + accountsController.init(); + + expect(accountsAddedListener).not.toHaveBeenCalled(); + }); + + it('fires accountsRemoved for accounts dropped by the keyring rebuild', async () => { + const messenger = buildMessenger(); + messenger.registerActionHandler( + 'KeyringController:getState', + // keyrings are now empty — mockAccount is no longer in any keyring + mockGetState.mockReturnValue({ keyrings: [] }), + ); + messenger.registerActionHandler( + 'KeyringController:getKeyringsByType', + mockGetKeyringByType.mockReturnValue([]), + ); + + const { accountsController } = setupAccountsController({ + initialState: { + internalAccounts: { + accounts: { [mockAccount.id]: mockAccount }, + selectedAccount: mockAccount.id, + }, + accountIdByAddress: { [mockAccount.address]: mockAccount.id }, + }, + messenger, + }); + + const accountsRemovedListener = jest.fn(); + messenger.subscribe( + 'AccountsController:accountsRemoved', + accountsRemovedListener, + ); + + accountsController.init(); + + expect(accountsRemovedListener).toHaveBeenCalledTimes(1); + expect(accountsRemovedListener).toHaveBeenCalledWith([mockAccount.id]); + }); + + it('fires the initialized event with current state', async () => { + const { accountsController, messenger } = setupInitTest(); + + const initializedListener = jest.fn(); + messenger.subscribe( + 'AccountsController:initialized', + initializedListener, + ); + + accountsController.init(); + + expect(initializedListener).toHaveBeenCalledTimes(1); + expect(initializedListener).toHaveBeenCalledWith( + accountsController.state, + ); + }); + + it('does not fire initialized on subsequent calls', async () => { + const { accountsController, messenger } = setupInitTest(); + + const initializedListener = jest.fn(); + messenger.subscribe( + 'AccountsController:initialized', + initializedListener, + ); + + accountsController.init(); + accountsController.init(); + + expect(initializedListener).toHaveBeenCalledTimes(1); + }); + + it('runs again after clearState resets the flag', async () => { + const { accountsController, messenger } = setupInitTest(); + + const initializedListener = jest.fn(); + messenger.subscribe( + 'AccountsController:initialized', + initializedListener, + ); + + accountsController.init(); + accountsController.clearState(); + accountsController.init(); + + expect(initializedListener).toHaveBeenCalledTimes(2); + }); + }); + describe('clearState', () => { it('resets state to the default values', () => { const { accountsController } = setupAccountsController({ @@ -3452,6 +3652,20 @@ describe('AccountsController', () => { getDefaultAccountsControllerState(), ); }); + + it('fires the uninitialized event', () => { + const { accountsController, messenger } = setupAccountsController({}); + + const uninitializedListener = jest.fn(); + messenger.subscribe( + 'AccountsController:uninitialized', + uninitializedListener, + ); + + accountsController.clearState(); + + expect(uninitializedListener).toHaveBeenCalledTimes(1); + }); }); describe('loadBackup', () => { @@ -4511,7 +4725,6 @@ describe('AccountsController', () => { jest.spyOn(AccountsController.prototype, 'listAccounts'); jest.spyOn(AccountsController.prototype, 'listMultichainAccounts'); jest.spyOn(AccountsController.prototype, 'setAccountName'); - jest.spyOn(AccountsController.prototype, 'updateAccounts'); jest.spyOn(AccountsController.prototype, 'getAccountByAddress'); jest.spyOn(AccountsController.prototype, 'getSelectedAccount'); jest.spyOn(AccountsController.prototype, 'getAccount'); @@ -4677,8 +4890,8 @@ describe('AccountsController', () => { }); }); - describe('updateAccounts', () => { - it('update accounts', async () => { + describe('init', () => { + it('is callable via the messenger and fires initialized event', async () => { const messenger = buildMessenger(); messenger.registerActionHandler( 'KeyringController:getState', @@ -4689,7 +4902,7 @@ describe('AccountsController', () => { mockGetKeyringByType.mockReturnValueOnce([]), ); - const { accountsController } = setupAccountsController({ + setupAccountsController({ initialState: { internalAccounts: { accounts: { [mockAccount.id]: mockAccount }, @@ -4702,8 +4915,15 @@ describe('AccountsController', () => { messenger, }); - await messenger.call('AccountsController:updateAccounts'); - expect(accountsController.updateAccounts).toHaveBeenCalledWith(); + const initializedListener = jest.fn(); + messenger.subscribe( + 'AccountsController:initialized', + initializedListener, + ); + + messenger.call('AccountsController:init'); + + expect(initializedListener).toHaveBeenCalledTimes(1); }); }); diff --git a/packages/accounts-controller/src/AccountsController.ts b/packages/accounts-controller/src/AccountsController.ts index 50af609db83..845c41149ed 100644 --- a/packages/accounts-controller/src/AccountsController.ts +++ b/packages/accounts-controller/src/AccountsController.ts @@ -94,7 +94,7 @@ const MESSENGER_EXPOSED_METHODS = [ 'setAccountNameAndSelectAccount', 'listAccounts', 'listMultichainAccounts', - 'updateAccounts', + 'init', 'getSelectedAccount', 'getSelectedMultichainAccount', 'getAccountByAddress', @@ -212,6 +212,16 @@ export type AccountsControllerAccountAssetListUpdatedEvent = { payload: SnapKeyringAccountAssetListUpdatedEvent['payload']; }; +export type AccountsControllerInitializedEvent = { + type: `${typeof controllerName}:initialized`; + payload: [AccountsControllerState]; +}; + +export type AccountsControllerUninitializedEvent = { + type: `${typeof controllerName}:uninitialized`; + payload: []; +}; + /** * @deprecated This type is deprecated and will be removed in a future version. * Use `AccountTreeController`, `MultichainAccountService`, or the Keyring API v2 instead. @@ -238,7 +248,9 @@ export type AccountsControllerEvents = | AccountsControllerAccountRenamedEvent | AccountsControllerAccountBalancesUpdatesEvent | AccountsControllerAccountTransactionsUpdatedEvent - | AccountsControllerAccountAssetListUpdatedEvent; + | AccountsControllerAccountAssetListUpdatedEvent + | AccountsControllerInitializedEvent + | AccountsControllerUninitializedEvent; /** * @deprecated This type is deprecated and will be removed in a future version. @@ -330,6 +342,8 @@ export class AccountsController extends BaseController< AccountsControllerState, AccountsControllerMessenger > { + #initialized = false; + /** * Constructor for AccountsController. * @@ -662,14 +676,48 @@ export class AccountsController extends BaseController< } /** - * Updates the internal accounts list by retrieving normal and snap accounts, - * removing duplicates, and updating the metadata of each account. + * Initializes the controller by loading accounts from the current keyring + * state. Any accounts present in the keyrings but missing from the + * controller's state are added; existing accounts are preserved. + * + * Intended to be called after {@link clearState} to restore accounts from + * the keyrings (i.e. `clearState` then `init`), or on first startup. * * @deprecated This method is deprecated and will be removed in a future version. * Use `AccountTreeController`, `MultichainAccountService`, or the Keyring API v2 instead. - * @returns A Promise that resolves when the accounts have been updated. */ - async updateAccounts(): Promise { + init(): void { + if (this.#initialized) { + return; + } + + const previousAccounts = this.state.internalAccounts.accounts; + this.#sync(); + + const addedAccounts = Object.values( + this.state.internalAccounts.accounts, + ).filter((account) => !previousAccounts[account.id]); + + const removedAccountIds = Object.keys(previousAccounts).filter( + (id) => !this.state.internalAccounts.accounts[id], + ); + + if (addedAccounts.length > 0) { + this.messenger.publish('AccountsController:accountsAdded', addedAccounts); + } + + if (removedAccountIds.length > 0) { + this.messenger.publish( + 'AccountsController:accountsRemoved', + removedAccountIds, + ); + } + + this.#initialized = true; + this.messenger.publish('AccountsController:initialized', this.state); + } + + #sync(): void { log('Synchronizing accounts with keyrings...'); const keyringAccountIndexes = new Map(); @@ -763,9 +811,11 @@ export class AccountsController extends BaseController< * Use `AccountTreeController`, `MultichainAccountService`, or the Keyring API v2 instead. */ clearState(): void { + this.#initialized = false; this.update(() => { return getDefaultAccountsControllerState(); }); + this.messenger.publish('AccountsController:uninitialized'); } /** diff --git a/packages/accounts-controller/src/index.ts b/packages/accounts-controller/src/index.ts index 5907b8ef207..79fa685f920 100644 --- a/packages/accounts-controller/src/index.ts +++ b/packages/accounts-controller/src/index.ts @@ -15,6 +15,8 @@ export type { AccountsControllerAccountBalancesUpdatesEvent, AccountsControllerAccountTransactionsUpdatedEvent, AccountsControllerAccountAssetListUpdatedEvent, + AccountsControllerInitializedEvent, + AccountsControllerUninitializedEvent, AllowedEvents, AccountsControllerEvents, AccountsControllerMessenger, @@ -31,7 +33,7 @@ export type { AccountsControllerSetAccountNameAction, AccountsControllerSetAccountNameAndSelectAccountAction, AccountsControllerUpdateAccountMetadataAction, - AccountsControllerUpdateAccountsAction, + AccountsControllerInitAction, AccountsControllerLoadBackupAction, AccountsControllerClearStateAction, } from './AccountsController-method-action-types.js'; diff --git a/packages/wallet/src/Wallet.test.ts b/packages/wallet/src/Wallet.test.ts index 2ffc6db1922..f84cf54d468 100644 --- a/packages/wallet/src/Wallet.test.ts +++ b/packages/wallet/src/Wallet.test.ts @@ -173,7 +173,7 @@ describe('Wallet', () => { const results = await wallet.init(); - expect(results).toHaveLength(6); + expect(results).toHaveLength(7); }); it('disallows modifying the messenger', async () => {