-
-
Notifications
You must be signed in to change notification settings - Fork 256
refactor: migrate AccountTreeController to @metamask/messenger
#6380
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
cf24808
83294c8
75b5398
33c4ea2
a8d9b51
b3df752
da25c0e
b419ecd
bee9e6f
a91b196
d3691a5
6746c38
8d8a971
a88ad38
7fe630b
03224dd
aa67cde
234d462
84be27c
e0cbe0e
102de9d
a185851
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ import { | |
| type AccountGroupId, | ||
| } from '@metamask/account-api'; | ||
| import type { AccountId } from '@metamask/accounts-controller'; | ||
| import { Messenger, deriveStateFromMetadata } from '@metamask/base-controller'; | ||
| import { deriveStateFromMetadata } from '@metamask/base-controller/next'; | ||
| import { | ||
| BtcAccountType, | ||
| EthAccountType, | ||
|
|
@@ -36,14 +36,11 @@ import type { BackupAndSyncAnalyticsEventPayload } from './backup-and-sync/analy | |
| import { BackupAndSyncService } from './backup-and-sync/service'; | ||
| import { isAccountGroupNameUnique } from './group'; | ||
| import { getAccountWalletNameFromKeyringType } from './rules/keyring'; | ||
| import { type AccountTreeControllerState } from './types'; | ||
| import { | ||
| type AccountTreeControllerMessenger, | ||
| type AccountTreeControllerActions, | ||
| type AccountTreeControllerEvents, | ||
| type AccountTreeControllerState, | ||
| type AllowedActions, | ||
| type AllowedEvents, | ||
| } from './types'; | ||
| getAccountTreeControllerMessenger, | ||
| getRootMessenger, | ||
| } from '../tests/mockMessenger'; | ||
|
|
||
| // Local mock of EMPTY_ACCOUNT to avoid circular dependency | ||
| const EMPTY_ACCOUNT_MOCK: InternalAccount = { | ||
|
|
@@ -233,53 +230,7 @@ const MOCK_HARDWARE_ACCOUNT_1: InternalAccount = { | |
| }, | ||
| }; | ||
|
|
||
| /** | ||
| * Creates a new root messenger instance for testing. | ||
| * | ||
| * @returns A new Messenger instance. | ||
| */ | ||
| function getRootMessenger() { | ||
| return new Messenger< | ||
| AccountTreeControllerActions | AllowedActions, | ||
| AccountTreeControllerEvents | AllowedEvents | ||
| >(); | ||
| } | ||
|
|
||
| /** | ||
| * Retrieves a restricted messenger for the AccountTreeController. | ||
| * | ||
| * @param messenger - The root messenger instance. Defaults to a new Messenger created by getRootMessenger(). | ||
| * @returns The restricted messenger for the AccountTreeController. | ||
| */ | ||
| function getAccountTreeControllerMessenger( | ||
| messenger = getRootMessenger(), | ||
| ): AccountTreeControllerMessenger { | ||
| return messenger.getRestricted({ | ||
| name: 'AccountTreeController', | ||
| allowedEvents: [ | ||
| 'AccountsController:accountAdded', | ||
| 'AccountsController:accountRemoved', | ||
| 'AccountsController:selectedAccountChange', | ||
| 'UserStorageController:stateChange', | ||
| 'MultichainAccountService:walletStatusChange', | ||
| ], | ||
| allowedActions: [ | ||
| 'AccountsController:listMultichainAccounts', | ||
| 'AccountsController:getAccount', | ||
| 'AccountsController:getSelectedMultichainAccount', | ||
| 'AccountsController:setSelectedAccount', | ||
| 'UserStorageController:getState', | ||
| 'UserStorageController:performGetStorage', | ||
| 'UserStorageController:performGetStorageAllFeatureEntries', | ||
| 'UserStorageController:performSetStorage', | ||
| 'UserStorageController:performBatchSetStorage', | ||
| 'AuthenticationController:getSessionProfile', | ||
| 'MultichainAccountService:createMultichainAccountGroup', | ||
| 'KeyringController:getState', | ||
| 'SnapController:get', | ||
| ], | ||
| }); | ||
| } | ||
| const mockGetSelectedMultichainAccountActionHandler = jest.fn(); | ||
|
|
||
| /** | ||
| * Sets up the AccountTreeController for testing. | ||
|
|
@@ -317,10 +268,7 @@ function setup({ | |
| }, | ||
| }: { | ||
| state?: Partial<AccountTreeControllerState>; | ||
| messenger?: Messenger< | ||
| AccountTreeControllerActions | AllowedActions, | ||
| AccountTreeControllerEvents | AllowedEvents | ||
| >; | ||
| messenger?: ReturnType<typeof getRootMessenger>; | ||
| accounts?: InternalAccount[]; | ||
| keyrings?: KeyringObject[]; | ||
| config?: { | ||
|
|
@@ -338,9 +286,9 @@ function setup({ | |
| }; | ||
| } = {}): { | ||
| controller: AccountTreeController; | ||
| messenger: Messenger< | ||
| AccountTreeControllerActions | AllowedActions, | ||
| AccountTreeControllerEvents | AllowedEvents | ||
| messenger: ReturnType<typeof getRootMessenger>; | ||
| accountTreeControllerMessenger: ReturnType< | ||
| typeof getAccountTreeControllerMessenger | ||
| >; | ||
| spies: { | ||
| consoleWarn: jest.SpyInstance; | ||
|
|
@@ -401,6 +349,7 @@ function setup({ | |
| mocks.AccountsController.listMultichainAccounts.mockImplementation( | ||
| () => mocks.AccountsController.accounts, | ||
| ); | ||
|
|
||
| messenger.registerActionHandler( | ||
| 'AccountsController:listMultichainAccounts', | ||
| mocks.AccountsController.listMultichainAccounts, | ||
|
|
@@ -474,8 +423,10 @@ function setup({ | |
| ); | ||
| } | ||
|
|
||
| const accountTreeControllerMessenger = | ||
| getAccountTreeControllerMessenger(messenger); | ||
| const controller = new AccountTreeController({ | ||
| messenger: getAccountTreeControllerMessenger(messenger), | ||
| messenger: accountTreeControllerMessenger, | ||
| state, | ||
| ...(config && { config }), | ||
| }); | ||
|
|
@@ -487,6 +438,7 @@ function setup({ | |
| return { | ||
| controller, | ||
| messenger, | ||
| accountTreeControllerMessenger, | ||
| spies: { consoleWarn: consoleWarnSpy }, | ||
| mocks, | ||
| }; | ||
|
|
@@ -1802,12 +1754,15 @@ describe('AccountTreeController', () => { | |
| }); | ||
|
|
||
| it('updates AccountsController selected account (with EVM account) when selectedAccountGroup changes', () => { | ||
| const { controller, messenger } = setup({ | ||
| const { controller, accountTreeControllerMessenger } = setup({ | ||
| accounts: [MOCK_HD_ACCOUNT_1, MOCK_HD_ACCOUNT_2], | ||
| keyrings: [MOCK_HD_KEYRING_1, MOCK_HD_KEYRING_2], | ||
| }); | ||
|
|
||
| const setSelectedAccountSpy = jest.spyOn(messenger, 'call'); | ||
| const setSelectedAccountSpy = jest.spyOn( | ||
| accountTreeControllerMessenger, | ||
| 'call', | ||
| ); | ||
|
|
||
| controller.init(); | ||
|
|
||
|
|
@@ -1839,15 +1794,18 @@ describe('AccountTreeController', () => { | |
| }, | ||
| }, | ||
| } as const; | ||
| const { controller, messenger } = setup({ | ||
| const { controller, accountTreeControllerMessenger } = setup({ | ||
| accounts: [ | ||
| MOCK_HD_ACCOUNT_1, | ||
| nonEvmAccount2, // Wallet 2 > Account 1. | ||
| ], | ||
| keyrings: [MOCK_HD_KEYRING_1, MOCK_HD_KEYRING_2], | ||
| }); | ||
|
|
||
| const setSelectedAccountSpy = jest.spyOn(messenger, 'call'); | ||
| const setSelectedAccountSpy = jest.spyOn( | ||
| accountTreeControllerMessenger, | ||
| 'call', | ||
| ); | ||
|
|
||
| controller.init(); | ||
|
|
||
|
|
@@ -1969,18 +1927,14 @@ describe('AccountTreeController', () => { | |
| }); | ||
|
|
||
| it('falls back to first wallet first group when AccountsController returns EMPTY_ACCOUNT', () => { | ||
| const { controller, messenger } = setup({ | ||
| const { controller } = setup({ | ||
| accounts: [MOCK_HD_ACCOUNT_1, MOCK_HD_ACCOUNT_2], | ||
| keyrings: [MOCK_HD_KEYRING_1, MOCK_HD_KEYRING_2], | ||
| }); | ||
|
|
||
| // Unregister existing handler and register new one BEFORE init | ||
| messenger.unregisterActionHandler( | ||
| 'AccountsController:getSelectedMultichainAccount', | ||
| ); | ||
| messenger.registerActionHandler( | ||
| 'AccountsController:getSelectedMultichainAccount', | ||
| () => EMPTY_ACCOUNT_MOCK, | ||
| // Mock action handler BEFORE init | ||
| mockGetSelectedMultichainAccountActionHandler.mockReturnValue( | ||
| EMPTY_ACCOUNT_MOCK, | ||
| ); | ||
|
|
||
| controller.init(); | ||
|
|
@@ -1998,7 +1952,7 @@ describe('AccountTreeController', () => { | |
| }); | ||
|
|
||
| it('falls back to first wallet first group when selected account is not in tree', () => { | ||
| const { controller, messenger } = setup({ | ||
| const { controller } = setup({ | ||
| accounts: [MOCK_HD_ACCOUNT_1, MOCK_HD_ACCOUNT_2], | ||
| keyrings: [MOCK_HD_KEYRING_1, MOCK_HD_KEYRING_2], | ||
| }); | ||
|
|
@@ -2009,12 +1963,8 @@ describe('AccountTreeController', () => { | |
| id: 'unknown-account-id', | ||
| }; | ||
|
|
||
| messenger.unregisterActionHandler( | ||
| 'AccountsController:getSelectedMultichainAccount', | ||
| ); | ||
| messenger.registerActionHandler( | ||
| 'AccountsController:getSelectedMultichainAccount', | ||
| () => unknownAccount, | ||
| mockGetSelectedMultichainAccountActionHandler.mockReturnValue( | ||
| unknownAccount, | ||
| ); | ||
|
|
||
| controller.init(); | ||
|
|
@@ -2032,18 +1982,14 @@ describe('AccountTreeController', () => { | |
| }); | ||
|
|
||
| it('returns empty string when no wallets exist and getSelectedMultichainAccount returns EMPTY_ACCOUNT', () => { | ||
| const { controller, messenger } = setup({ | ||
| const { controller } = setup({ | ||
| accounts: [], | ||
| keyrings: [], | ||
| }); | ||
|
|
||
| // Mock getSelectedMultichainAccount to return EMPTY_ACCOUNT_MOCK (id is '') BEFORE init | ||
| messenger.unregisterActionHandler( | ||
| 'AccountsController:getSelectedMultichainAccount', | ||
| ); | ||
| messenger.registerActionHandler( | ||
| 'AccountsController:getSelectedMultichainAccount', | ||
| () => EMPTY_ACCOUNT_MOCK, | ||
| // Mock getSelectedAccount to return EMPTY_ACCOUNT_MOCK (id is '') BEFORE init | ||
| mockGetSelectedMultichainAccountActionHandler.mockReturnValue( | ||
| EMPTY_ACCOUNT_MOCK, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Mock Handler Not Registered, Tests FailThe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Mock Handler Not RegisteredThe |
||
| ); | ||
|
|
||
| controller.init(); | ||
|
|
@@ -4328,7 +4274,7 @@ describe('AccountTreeController', () => { | |
| deriveStateFromMetadata( | ||
| controller.state, | ||
| controller.metadata, | ||
| 'anonymous', | ||
| 'includeInDebugSnapshot', | ||
| ), | ||
| ).toMatchInlineSnapshot(`Object {}`); | ||
| }); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Do we want to export a
RootMessengertype frommockMessenger.ts?