From 080458c3694cbe444e2f740b02a421ee8da3786c Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Wed, 3 Jul 2024 08:34:13 +0800 Subject: [PATCH 1/6] feat: add accountAdded and accountRemoved events --- .../src/AccountsController.test.ts | 87 +++++++++++++++++++ .../src/AccountsController.ts | 33 ++++++- packages/accounts-controller/src/index.ts | 4 + 3 files changed, 121 insertions(+), 3 deletions(-) diff --git a/packages/accounts-controller/src/AccountsController.test.ts b/packages/accounts-controller/src/AccountsController.test.ts index cf8a1b1a4d9..e331061ede9 100644 --- a/packages/accounts-controller/src/AccountsController.test.ts +++ b/packages/accounts-controller/src/AccountsController.test.ts @@ -880,6 +880,49 @@ describe('AccountsController', () => { ]); expect(accountsController.getSelectedAccount().id).toBe(mockAccount.id); }); + + it('publishes accountAdded event', async () => { + const messenger = buildMessenger(); + const messengerSpy = jest.spyOn(messenger, 'publish'); + mockUUID + .mockReturnValueOnce('mock-id') // call to check if its a new account + .mockReturnValueOnce('mock-id2') // call to check if its a new account + .mockReturnValueOnce('mock-id2'); // call to add account + + const mockNewKeyringState = { + isUnlocked: true, + keyrings: [ + { + type: KeyringTypes.hd, + accounts: [mockAccount.address, mockAccount2.address], + }, + ], + }; + setupAccountsController({ + initialState: { + internalAccounts: { + accounts: { + [mockAccount.id]: mockAccount, + }, + selectedAccount: mockAccount.id, + }, + }, + messenger, + }); + + messenger.publish( + 'KeyringController:stateChange', + mockNewKeyringState, + [], + ); + + // First call is 'KeyringController:stateChange' + expect(messengerSpy).toHaveBeenNthCalledWith( + 2, + 'AccountsController:accountAdded', + setLastSelectedAsAny(mockAccount2), + ); + }); }); describe('deleting account', () => { @@ -1107,6 +1150,50 @@ describe('AccountsController', () => { mockAccountWithoutLastSelected, ); }); + + it('publishes accountRemoved event', async () => { + const messenger = buildMessenger(); + const messengerSpy = jest.spyOn(messenger, 'publish'); + mockUUID + .mockReturnValueOnce('mock-id') // call to check if its a new account + .mockReturnValueOnce('mock-id2') // call to check if its a new account + .mockReturnValueOnce('mock-id2'); // call to add account + + const mockNewKeyringState = { + isUnlocked: true, + keyrings: [ + { + type: KeyringTypes.hd, + accounts: [mockAccount.address, mockAccount2.address], + }, + ], + }; + setupAccountsController({ + initialState: { + internalAccounts: { + accounts: { + [mockAccount.id]: mockAccount, + [mockAccount3.id]: mockAccount3, + }, + selectedAccount: mockAccount.id, + }, + }, + messenger, + }); + + messenger.publish( + 'KeyringController:stateChange', + mockNewKeyringState, + [], + ); + + // First call is 'KeyringController:stateChange' + expect(messengerSpy).toHaveBeenNthCalledWith( + 2, + 'AccountsController:accountRemoved', + 'mock-id3', + ); + }); }); it('handle keyring reinitialization', async () => { diff --git a/packages/accounts-controller/src/AccountsController.ts b/packages/accounts-controller/src/AccountsController.ts index ccc6c76f0f4..683a2c6ca73 100644 --- a/packages/accounts-controller/src/AccountsController.ts +++ b/packages/accounts-controller/src/AccountsController.ts @@ -42,9 +42,11 @@ import { const controllerName = 'AccountsController'; +export type AccountId = string; + export type AccountsControllerState = { internalAccounts: { - accounts: Record; + accounts: Record; selectedAccount: string; // id of the selected account }; }; @@ -137,12 +139,24 @@ export type AccountsControllerSelectedEvmAccountChangeEvent = { payload: [InternalAccount]; }; +export type AccountsControllerAccountAddedEvent = { + type: `${typeof controllerName}:accountAdded`; + payload: [InternalAccount]; +}; + +export type AccountsControllerAccountRemovedEvent = { + type: `${typeof controllerName}:accountRemoved`; + payload: [AccountId]; +}; + export type AllowedEvents = SnapStateChange | KeyringControllerStateChangeEvent; export type AccountsControllerEvents = | AccountsControllerChangeEvent | AccountsControllerSelectedAccountChangeEvent - | AccountsControllerSelectedEvmAccountChangeEvent; + | AccountsControllerSelectedEvmAccountChangeEvent + | AccountsControllerAccountAddedEvent + | AccountsControllerAccountRemovedEvent; export type AccountsControllerMessenger = RestrictedControllerMessenger< typeof controllerName, @@ -419,6 +433,7 @@ export class AccountsController extends BaseController< ...account, metadata: { ...account.metadata, name: accountName }, }; + // @ts-ignore currentState.internalAccounts.accounts[accountId] = internalAccount; }); } @@ -941,7 +956,7 @@ export class AccountsController extends BaseController< Object.values(accountsState), ); - accountsState[newAccount.id] = { + const newAccountWithUpdatedMetadata = { ...newAccount, metadata: { ...newAccount.metadata, @@ -951,6 +966,13 @@ export class AccountsController extends BaseController< }, }; + accountsState[newAccount.id] = newAccountWithUpdatedMetadata; + + this.messagingSystem.publish( + 'AccountsController:accountAdded', + newAccountWithUpdatedMetadata, + ); + return accountsState; } @@ -965,6 +987,11 @@ export class AccountsController extends BaseController< accountId: string, ): AccountsControllerState['internalAccounts']['accounts'] { delete accountsState[accountId]; + + this.messagingSystem.publish( + 'AccountsController:accountRemoved', + accountId, + ); return accountsState; } diff --git a/packages/accounts-controller/src/index.ts b/packages/accounts-controller/src/index.ts index c07b5b03f26..bbe4db142f1 100644 --- a/packages/accounts-controller/src/index.ts +++ b/packages/accounts-controller/src/index.ts @@ -8,10 +8,14 @@ export type { AccountsControllerGetSelectedAccountAction, AccountsControllerGetAccountByAddressAction, AccountsControllerGetAccountAction, + AccountsControllerGetNextAvailableAccountNameAction, + AccountsControllerGetSelectedMultichainAccountAction, AccountsControllerActions, AccountsControllerChangeEvent, AccountsControllerSelectedAccountChangeEvent, AccountsControllerSelectedEvmAccountChangeEvent, + AccountsControllerAccountAddedEvent, + AccountsControllerAccountRemovedEvent, AccountsControllerEvents, AccountsControllerMessenger, } from './AccountsController'; From e4450524bdca6d31bca77fcc6c7156ea02e7bd66 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Wed, 3 Jul 2024 17:41:42 +0800 Subject: [PATCH 2/6] fix: lint --- packages/accounts-controller/src/AccountsController.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/accounts-controller/src/AccountsController.ts b/packages/accounts-controller/src/AccountsController.ts index 683a2c6ca73..1652ee5b8d0 100644 --- a/packages/accounts-controller/src/AccountsController.ts +++ b/packages/accounts-controller/src/AccountsController.ts @@ -433,7 +433,6 @@ export class AccountsController extends BaseController< ...account, metadata: { ...account.metadata, name: accountName }, }; - // @ts-ignore currentState.internalAccounts.accounts[accountId] = internalAccount; }); } From a4777b72a9a475024bdcb6c9d31e8b6898c8a6cd Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Wed, 3 Jul 2024 18:59:44 +0800 Subject: [PATCH 3/6] Apply suggestions from code review Co-authored-by: Charly Chevalier --- .../src/AccountsController.test.ts | 12 ++++++------ .../accounts-controller/src/AccountsController.ts | 1 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/accounts-controller/src/AccountsController.test.ts b/packages/accounts-controller/src/AccountsController.test.ts index e331061ede9..dc3976b59a2 100644 --- a/packages/accounts-controller/src/AccountsController.test.ts +++ b/packages/accounts-controller/src/AccountsController.test.ts @@ -885,9 +885,9 @@ describe('AccountsController', () => { const messenger = buildMessenger(); const messengerSpy = jest.spyOn(messenger, 'publish'); mockUUID - .mockReturnValueOnce('mock-id') // call to check if its a new account - .mockReturnValueOnce('mock-id2') // call to check if its a new account - .mockReturnValueOnce('mock-id2'); // call to add account + .mockReturnValueOnce(mockAccount.id) // call to check if its a new account + .mockReturnValueOnce(mockAccount2.id) // call to check if its a new account + .mockReturnValueOnce(mockAccount2.id); // call to add account const mockNewKeyringState = { isUnlocked: true, @@ -1155,9 +1155,9 @@ describe('AccountsController', () => { const messenger = buildMessenger(); const messengerSpy = jest.spyOn(messenger, 'publish'); mockUUID - .mockReturnValueOnce('mock-id') // call to check if its a new account - .mockReturnValueOnce('mock-id2') // call to check if its a new account - .mockReturnValueOnce('mock-id2'); // call to add account + .mockReturnValueOnce(mockAccount.id) // call to check if its a new account + .mockReturnValueOnce(mockAccount2.id) // call to check if its a new account + .mockReturnValueOnce(mockAccount2.id); // call to add account const mockNewKeyringState = { isUnlocked: true, diff --git a/packages/accounts-controller/src/AccountsController.ts b/packages/accounts-controller/src/AccountsController.ts index 1652ee5b8d0..700cf28d5d7 100644 --- a/packages/accounts-controller/src/AccountsController.ts +++ b/packages/accounts-controller/src/AccountsController.ts @@ -991,6 +991,7 @@ export class AccountsController extends BaseController< 'AccountsController:accountRemoved', accountId, ); + return accountsState; } From c7c7e692ad96d8dac57f77717b16cd399dbb79da Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Wed, 3 Jul 2024 19:00:02 +0800 Subject: [PATCH 4/6] fix: remove exports --- packages/accounts-controller/src/index.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/accounts-controller/src/index.ts b/packages/accounts-controller/src/index.ts index bbe4db142f1..188caf3fedf 100644 --- a/packages/accounts-controller/src/index.ts +++ b/packages/accounts-controller/src/index.ts @@ -8,8 +8,6 @@ export type { AccountsControllerGetSelectedAccountAction, AccountsControllerGetAccountByAddressAction, AccountsControllerGetAccountAction, - AccountsControllerGetNextAvailableAccountNameAction, - AccountsControllerGetSelectedMultichainAccountAction, AccountsControllerActions, AccountsControllerChangeEvent, AccountsControllerSelectedAccountChangeEvent, From d7bf78802caf98e5ac5773d4ef48b14712628e46 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Wed, 3 Jul 2024 19:02:41 +0800 Subject: [PATCH 5/6] fix: nit --- .../src/AccountsController.test.ts | 39 ++++++++++--------- .../src/AccountsController.ts | 1 - 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/packages/accounts-controller/src/AccountsController.test.ts b/packages/accounts-controller/src/AccountsController.test.ts index dc3976b59a2..ada7a9ae376 100644 --- a/packages/accounts-controller/src/AccountsController.test.ts +++ b/packages/accounts-controller/src/AccountsController.test.ts @@ -889,15 +889,6 @@ describe('AccountsController', () => { .mockReturnValueOnce(mockAccount2.id) // call to check if its a new account .mockReturnValueOnce(mockAccount2.id); // call to add account - const mockNewKeyringState = { - isUnlocked: true, - keyrings: [ - { - type: KeyringTypes.hd, - accounts: [mockAccount.address, mockAccount2.address], - }, - ], - }; setupAccountsController({ initialState: { internalAccounts: { @@ -910,6 +901,16 @@ describe('AccountsController', () => { messenger, }); + const mockNewKeyringState = { + isUnlocked: true, + keyrings: [ + { + type: KeyringTypes.hd, + accounts: [mockAccount.address, mockAccount2.address], + }, + ], + }; + messenger.publish( 'KeyringController:stateChange', mockNewKeyringState, @@ -1159,15 +1160,6 @@ describe('AccountsController', () => { .mockReturnValueOnce(mockAccount2.id) // call to check if its a new account .mockReturnValueOnce(mockAccount2.id); // call to add account - const mockNewKeyringState = { - isUnlocked: true, - keyrings: [ - { - type: KeyringTypes.hd, - accounts: [mockAccount.address, mockAccount2.address], - }, - ], - }; setupAccountsController({ initialState: { internalAccounts: { @@ -1181,6 +1173,15 @@ describe('AccountsController', () => { messenger, }); + const mockNewKeyringState = { + isUnlocked: true, + keyrings: [ + { + type: KeyringTypes.hd, + accounts: [mockAccount.address, mockAccount2.address], + }, + ], + }; messenger.publish( 'KeyringController:stateChange', mockNewKeyringState, @@ -1191,7 +1192,7 @@ describe('AccountsController', () => { expect(messengerSpy).toHaveBeenNthCalledWith( 2, 'AccountsController:accountRemoved', - 'mock-id3', + mockAccount3.id, ); }); }); diff --git a/packages/accounts-controller/src/AccountsController.ts b/packages/accounts-controller/src/AccountsController.ts index 700cf28d5d7..06f8df24124 100644 --- a/packages/accounts-controller/src/AccountsController.ts +++ b/packages/accounts-controller/src/AccountsController.ts @@ -964,7 +964,6 @@ export class AccountsController extends BaseController< lastSelected: 0, }, }; - accountsState[newAccount.id] = newAccountWithUpdatedMetadata; this.messagingSystem.publish( From 4b5c190e5381b93db5f9cc6d9336ffb6cbbc9eef Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Wed, 3 Jul 2024 19:15:05 +0800 Subject: [PATCH 6/6] fix: lint --- packages/accounts-controller/src/AccountsController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/accounts-controller/src/AccountsController.ts b/packages/accounts-controller/src/AccountsController.ts index 06f8df24124..d756479b1a2 100644 --- a/packages/accounts-controller/src/AccountsController.ts +++ b/packages/accounts-controller/src/AccountsController.ts @@ -990,7 +990,7 @@ export class AccountsController extends BaseController< 'AccountsController:accountRemoved', accountId, ); - + return accountsState; }