diff --git a/README.md b/README.md index 0b377cff988..443b91c2b94 100644 --- a/README.md +++ b/README.md @@ -148,6 +148,7 @@ linkStyle default opacity:0.5 transaction_controller(["@metamask/transaction-controller"]); user_operation_controller(["@metamask/user-operation-controller"]); account_tree_controller --> base_controller; + account_tree_controller --> messenger; account_tree_controller --> accounts_controller; account_tree_controller --> keyring_controller; account_tree_controller --> multichain_account_service; @@ -216,6 +217,7 @@ linkStyle default opacity:0.5 earn_controller --> transaction_controller; eip_5792_middleware --> transaction_controller; eip_5792_middleware --> keyring_controller; + eip_7702_internal_rpc_middleware --> controller_utils; eip1193_permission_middleware --> chain_agnostic_permission; eip1193_permission_middleware --> controller_utils; eip1193_permission_middleware --> json_rpc_engine; diff --git a/packages/account-tree-controller/CHANGELOG.md b/packages/account-tree-controller/CHANGELOG.md index 9fd6c8488b1..57454190ab2 100644 --- a/packages/account-tree-controller/CHANGELOG.md +++ b/packages/account-tree-controller/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **BREAKING:** Use new `Messenger` from `@metamask/messenger` ([#6380](https://github.com/MetaMask/core/pull/6380)) + - Previously, `AccountTreeController` accepted a `RestrictedMessenger` instance from `@metamask/base-controller`. + ## [1.6.0] ### Changed diff --git a/packages/account-tree-controller/package.json b/packages/account-tree-controller/package.json index 4165b0e69fe..efe958fcd66 100644 --- a/packages/account-tree-controller/package.json +++ b/packages/account-tree-controller/package.json @@ -48,6 +48,7 @@ }, "dependencies": { "@metamask/base-controller": "^8.4.2", + "@metamask/messenger": "^0.3.0", "@metamask/snaps-sdk": "^9.0.0", "@metamask/snaps-utils": "^11.0.0", "@metamask/superstruct": "^3.1.0", diff --git a/packages/account-tree-controller/src/AccountTreeController.test.ts b/packages/account-tree-controller/src/AccountTreeController.test.ts index 08733672915..10ad0ede285 100644 --- a/packages/account-tree-controller/src/AccountTreeController.test.ts +++ b/packages/account-tree-controller/src/AccountTreeController.test.ts @@ -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; - messenger?: Messenger< - AccountTreeControllerActions | AllowedActions, - AccountTreeControllerEvents | AllowedEvents - >; + messenger?: ReturnType; accounts?: InternalAccount[]; keyrings?: KeyringObject[]; config?: { @@ -338,9 +286,9 @@ function setup({ }; } = {}): { controller: AccountTreeController; - messenger: Messenger< - AccountTreeControllerActions | AllowedActions, - AccountTreeControllerEvents | AllowedEvents + messenger: ReturnType; + 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,7 +1794,7 @@ describe('AccountTreeController', () => { }, }, } as const; - const { controller, messenger } = setup({ + const { controller, accountTreeControllerMessenger } = setup({ accounts: [ MOCK_HD_ACCOUNT_1, nonEvmAccount2, // Wallet 2 > Account 1. @@ -1847,7 +1802,10 @@ describe('AccountTreeController', () => { 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, ); controller.init(); @@ -4328,7 +4274,7 @@ describe('AccountTreeController', () => { deriveStateFromMetadata( controller.state, controller.metadata, - 'anonymous', + 'includeInDebugSnapshot', ), ).toMatchInlineSnapshot(`Object {}`); }); diff --git a/packages/account-tree-controller/src/AccountTreeController.ts b/packages/account-tree-controller/src/AccountTreeController.ts index 372d357a338..dbe6c966f46 100644 --- a/packages/account-tree-controller/src/AccountTreeController.ts +++ b/packages/account-tree-controller/src/AccountTreeController.ts @@ -8,8 +8,8 @@ import type { } from '@metamask/account-api'; import type { MultichainAccountWalletStatus } from '@metamask/account-api'; import { type AccountId } from '@metamask/accounts-controller'; -import type { StateMetadata } from '@metamask/base-controller'; -import { BaseController } from '@metamask/base-controller'; +import type { StateMetadata } from '@metamask/base-controller/next'; +import { BaseController } from '@metamask/base-controller/next'; import type { TraceCallback } from '@metamask/controller-utils'; import { isEvmAccountType } from '@metamask/keyring-api'; import type { InternalAccount } from '@metamask/keyring-internal-api'; @@ -49,31 +49,31 @@ const accountTreeControllerMetadata: StateMetadata = accountTree: { includeInStateLogs: true, persist: false, // We do re-recompute this state everytime. - anonymous: false, + includeInDebugSnapshot: false, usedInUi: true, }, isAccountTreeSyncingInProgress: { includeInStateLogs: false, persist: false, - anonymous: false, + includeInDebugSnapshot: false, usedInUi: true, }, hasAccountTreeSyncingSyncedAtLeastOnce: { includeInStateLogs: true, persist: true, - anonymous: false, + includeInDebugSnapshot: false, usedInUi: true, }, accountGroupsMetadata: { includeInStateLogs: true, persist: true, - anonymous: false, + includeInDebugSnapshot: false, usedInUi: true, }, accountWalletsMetadata: { includeInStateLogs: true, persist: true, - anonymous: false, + includeInDebugSnapshot: false, usedInUi: true, }, }; @@ -185,11 +185,11 @@ export class AccountTreeController extends BaseController< // Rules to apply to construct the wallets tree. this.#rules = [ // 1. We group by entropy-source - new EntropyRule(this.messagingSystem), + new EntropyRule(this.messenger), // 2. We group by Snap ID - new SnapRule(this.messagingSystem), + new SnapRule(this.messenger), // 3. We group by wallet type (this rule cannot fail and will group all non-matching accounts) - new KeyringRule(this.messagingSystem), + new KeyringRule(this.messenger), ]; // Initialize trace function @@ -213,28 +213,25 @@ export class AccountTreeController extends BaseController< this.#createBackupAndSyncContext(), ); - this.messagingSystem.subscribe( - 'AccountsController:accountAdded', - (account) => { - this.#handleAccountAdded(account); - }, - ); + this.messenger.subscribe('AccountsController:accountAdded', (account) => { + this.#handleAccountAdded(account); + }); - this.messagingSystem.subscribe( + this.messenger.subscribe( 'AccountsController:accountRemoved', (accountId) => { this.#handleAccountRemoved(accountId); }, ); - this.messagingSystem.subscribe( + this.messenger.subscribe( 'AccountsController:selectedAccountChange', (account) => { this.#handleSelectedAccountChange(account); }, ); - this.messagingSystem.subscribe( + this.messenger.subscribe( 'UserStorageController:stateChange', (userStorageControllerState) => { this.#backupAndSyncService.handleUserStorageStateChange( @@ -243,7 +240,7 @@ export class AccountTreeController extends BaseController< }, ); - this.messagingSystem.subscribe( + this.messenger.subscribe( 'MultichainAccountService:walletStatusChange', (walletId, status) => { this.#handleMultichainAccountWalletStatusChange(walletId, status); @@ -358,7 +355,7 @@ export class AccountTreeController extends BaseController< log( `Selected (initial) group is: [${this.state.accountTree.selectedAccountGroup}]`, ); - this.messagingSystem.publish( + this.messenger.publish( `${controllerName}:selectedAccountGroupChange`, this.state.accountTree.selectedAccountGroup, previousSelectedAccountGroup, @@ -488,10 +485,7 @@ export class AccountTreeController extends BaseController< let proposedName = ''; // Empty means there's no computed name for this group. for (const id of group.accounts) { - const account = this.messagingSystem.call( - 'AccountsController:getAccount', - id, - ); + const account = this.messenger.call('AccountsController:getAccount', id); if (!account || !account.metadata.name.length) { continue; } @@ -770,10 +764,7 @@ export class AccountTreeController extends BaseController< const accounts: InternalAccount[] = []; for (const id of group.accounts) { - const account = this.messagingSystem.call( - 'AccountsController:getAccount', - id, - ); + const account = this.messenger.call('AccountsController:getAccount', id); // For now, we're filtering undefined account, but I believe // throwing would be more appropriate here. @@ -834,7 +825,7 @@ export class AccountTreeController extends BaseController< } }); - this.messagingSystem.publish( + this.messenger.publish( `${controllerName}:accountTreeChange`, this.state.accountTree, ); @@ -892,14 +883,14 @@ export class AccountTreeController extends BaseController< } } }); - this.messagingSystem.publish( + this.messenger.publish( `${controllerName}:accountTreeChange`, this.state.accountTree, ); // Emit selectedAccountGroupChange event if the selected group changed if (selectedAccountGroupChanged) { - this.messagingSystem.publish( + this.messenger.publish( `${controllerName}:selectedAccountGroupChange`, this.state.accountTree.selectedAccountGroup, previousSelectedAccountGroup, @@ -1056,9 +1047,7 @@ export class AccountTreeController extends BaseController< * @returns The list of all internal accounts. */ #listAccounts(): InternalAccount[] { - return this.messagingSystem.call( - 'AccountsController:listMultichainAccounts', - ); + return this.messenger.call('AccountsController:listMultichainAccounts'); } /** @@ -1138,7 +1127,7 @@ export class AccountTreeController extends BaseController< `Selected group is now: [${this.state.accountTree.selectedAccountGroup}]`, ); - this.messagingSystem.publish( + this.messenger.publish( `${controllerName}:selectedAccountGroupChange`, groupId, previousSelectedAccountGroup, @@ -1146,7 +1135,7 @@ export class AccountTreeController extends BaseController< // Update AccountsController - this will trigger selectedAccountChange event, // but our handler is idempotent so it won't cause infinite loop - this.messagingSystem.call( + this.messenger.call( 'AccountsController:setSelectedAccount', accountToSelect, ); @@ -1161,7 +1150,7 @@ export class AccountTreeController extends BaseController< #getDefaultSelectedAccountGroup(wallets: { [walletId: AccountWalletId]: AccountWalletObject; }): AccountGroupId | '' { - const selectedAccount = this.messagingSystem.call( + const selectedAccount = this.messenger.call( 'AccountsController:getSelectedMultichainAccount', ); if (selectedAccount && selectedAccount.id) { @@ -1203,7 +1192,7 @@ export class AccountTreeController extends BaseController< this.update((state) => { state.accountTree.selectedAccountGroup = groupId; }); - this.messagingSystem.publish( + this.messenger.publish( `${controllerName}:selectedAccountGroupChange`, groupId, previousSelectedAccountGroup, @@ -1258,7 +1247,7 @@ export class AccountTreeController extends BaseController< if (group) { let candidate; for (const id of group.accounts) { - const account = this.messagingSystem.call( + const account = this.messenger.call( 'AccountsController:getAccount', id, ); @@ -1301,7 +1290,7 @@ export class AccountTreeController extends BaseController< } for (const id of group.accounts) { - const account = this.messagingSystem.call( + const account = this.messenger.call( 'AccountsController:getAccount', id, ); @@ -1544,37 +1533,37 @@ export class AccountTreeController extends BaseController< * Registers message handlers for the AccountTreeController. */ #registerMessageHandlers(): void { - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( `${controllerName}:getSelectedAccountGroup`, this.getSelectedAccountGroup.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( `${controllerName}:setSelectedAccountGroup`, this.setSelectedAccountGroup.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( `${controllerName}:getAccountsFromSelectedAccountGroup`, this.getAccountsFromSelectedAccountGroup.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( `${controllerName}:setAccountWalletName`, this.setAccountWalletName.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( `${controllerName}:setAccountGroupName`, this.setAccountGroupName.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( `${controllerName}:setAccountGroupPinned`, this.setAccountGroupPinned.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( `${controllerName}:setAccountGroupHidden`, this.setAccountGroupHidden.bind(this), ); @@ -1621,7 +1610,7 @@ export class AccountTreeController extends BaseController< return { ...this.#backupAndSyncConfig, controller: this, - messenger: this.messagingSystem, + messenger: this.messenger, controllerStateUpdateFn: this.update.bind(this), traceFn: this.#trace.bind(this), groupIdToWalletId: this.#groupIdToWalletId, diff --git a/packages/account-tree-controller/src/rule.test.ts b/packages/account-tree-controller/src/rule.test.ts index f370fbfdf5b..20318183277 100644 --- a/packages/account-tree-controller/src/rule.test.ts +++ b/packages/account-tree-controller/src/rule.test.ts @@ -4,7 +4,6 @@ import { toMultichainAccountGroupId, toMultichainAccountWalletId, } from '@metamask/account-api'; -import { Messenger } from '@metamask/base-controller'; import { EthAccountType, EthMethod, @@ -16,14 +15,11 @@ import type { InternalAccount } from '@metamask/keyring-internal-api'; import type { AccountGroupObject } from './group'; import { BaseRule } from './rule'; -import type { - AccountTreeControllerMessenger, - AccountTreeControllerActions, - AccountTreeControllerEvents, - AllowedActions, - AllowedEvents, -} from './types'; import type { AccountWalletObject } from './wallet'; +import { + getAccountTreeControllerMessenger, + getRootMessenger, +} from '../tests/mockMessenger'; const ETH_EOA_METHODS = [ EthMethod.PersonalSign, @@ -57,53 +53,15 @@ const MOCK_HD_ACCOUNT_1: Bip44Account = { }, }; -/** - * 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', - ], - allowedActions: [ - 'AccountsController:listMultichainAccounts', - 'AccountsController:getAccount', - 'AccountsController:getSelectedMultichainAccount', - 'AccountsController:setSelectedAccount', - 'KeyringController:getState', - 'SnapController:get', - ], - }); -} - describe('BaseRule', () => { describe('getComputedAccountGroupName', () => { it('returns empty string when account is not found', () => { - const rootMessenger = getRootMessenger(); - const messenger = getAccountTreeControllerMessenger(rootMessenger); - const rule = new BaseRule(messenger); + const messenger = getRootMessenger(); + const accountTreeControllerMessenger = + getAccountTreeControllerMessenger(messenger); + const rule = new BaseRule(accountTreeControllerMessenger); - rootMessenger.registerActionHandler( + messenger.registerActionHandler( 'AccountsController:getAccount', () => undefined, ); @@ -129,11 +87,12 @@ describe('BaseRule', () => { }); it('returns account name when account is found', () => { - const rootMessenger = getRootMessenger(); - const messenger = getAccountTreeControllerMessenger(rootMessenger); - const rule = new BaseRule(messenger); + const messenger = getRootMessenger(); + const accountTreeControllerMessenger = + getAccountTreeControllerMessenger(messenger); + const rule = new BaseRule(accountTreeControllerMessenger); - rootMessenger.registerActionHandler( + messenger.registerActionHandler( 'AccountsController:getAccount', () => MOCK_HD_ACCOUNT_1, ); diff --git a/packages/account-tree-controller/src/rules/entropy.test.ts b/packages/account-tree-controller/src/rules/entropy.test.ts index 6874ddc7abf..9023d344de4 100644 --- a/packages/account-tree-controller/src/rules/entropy.test.ts +++ b/packages/account-tree-controller/src/rules/entropy.test.ts @@ -4,7 +4,6 @@ import { toMultichainAccountGroupId, toMultichainAccountWalletId, } from '@metamask/account-api'; -import { Messenger } from '@metamask/base-controller'; import { EthAccountType, EthMethod, @@ -17,14 +16,11 @@ import type { InternalAccount } from '@metamask/keyring-internal-api'; import type { AccountWalletEntropyObject } from 'src/wallet'; import { EntropyRule } from './entropy'; +import { + getAccountTreeControllerMessenger, + getRootMessenger, +} from '../../tests/mockMessenger'; import type { AccountGroupObjectOf } from '../group'; -import type { - AccountTreeControllerMessenger, - AccountTreeControllerActions, - AccountTreeControllerEvents, - AllowedActions, - AllowedEvents, -} from '../types'; const ETH_EOA_METHODS = [ EthMethod.PersonalSign, @@ -64,53 +60,15 @@ const MOCK_HD_ACCOUNT_1: Bip44Account = { }, }; -/** - * 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', - ], - allowedActions: [ - 'AccountsController:listMultichainAccounts', - 'AccountsController:getAccount', - 'AccountsController:getSelectedMultichainAccount', - 'AccountsController:setSelectedAccount', - 'KeyringController:getState', - 'SnapController:get', - ], - }); -} - describe('EntropyRule', () => { describe('getComputedAccountGroupName', () => { it('uses BaseRule implementation', () => { - const rootMessenger = getRootMessenger(); - const messenger = getAccountTreeControllerMessenger(rootMessenger); - const rule = new EntropyRule(messenger); + const messenger = getRootMessenger(); + const accountTreeControllerMessenger = + getAccountTreeControllerMessenger(messenger); + const rule = new EntropyRule(accountTreeControllerMessenger); - rootMessenger.registerActionHandler( + messenger.registerActionHandler( 'AccountsController:getAccount', () => MOCK_HD_ACCOUNT_1, ); @@ -138,11 +96,12 @@ describe('EntropyRule', () => { }); it('returns empty string when account is not found', () => { - const rootMessenger = getRootMessenger(); - const messenger = getAccountTreeControllerMessenger(rootMessenger); - const rule = new EntropyRule(messenger); + const messenger = getRootMessenger(); + const accountTreeControllerMessenger = + getAccountTreeControllerMessenger(messenger); + const rule = new EntropyRule(accountTreeControllerMessenger); - rootMessenger.registerActionHandler( + messenger.registerActionHandler( 'AccountsController:getAccount', () => undefined, ); @@ -180,9 +139,10 @@ describe('EntropyRule', () => { }); it('getComputedAccountGroupName returns account name with EVM priority', () => { - const rootMessenger = getRootMessenger(); - const messenger = getAccountTreeControllerMessenger(rootMessenger); - const rule = new EntropyRule(messenger); + const messenger = getRootMessenger(); + const accountTreeControllerMessenger = + getAccountTreeControllerMessenger(messenger); + const rule = new EntropyRule(accountTreeControllerMessenger); const mockEvmAccount: InternalAccount = { ...MOCK_HD_ACCOUNT_1, @@ -194,7 +154,7 @@ describe('EntropyRule', () => { }, }; - rootMessenger.registerActionHandler( + messenger.registerActionHandler( 'AccountsController:getAccount', () => mockEvmAccount, ); @@ -220,11 +180,12 @@ describe('EntropyRule', () => { }); it('getComputedAccountGroupName returns empty string when no accounts found', () => { - const rootMessenger = getRootMessenger(); - const messenger = getAccountTreeControllerMessenger(rootMessenger); - const rule = new EntropyRule(messenger); + const messenger = getRootMessenger(); + const accountTreeControllerMessenger = + getAccountTreeControllerMessenger(messenger); + const rule = new EntropyRule(accountTreeControllerMessenger); - rootMessenger.registerActionHandler( + messenger.registerActionHandler( 'AccountsController:getAccount', () => undefined, ); @@ -250,9 +211,10 @@ describe('EntropyRule', () => { }); it('getComputedAccountGroupName returns empty string for non-EVM accounts to prevent chain-specific names', () => { - const rootMessenger = getRootMessenger(); - const messenger = getAccountTreeControllerMessenger(rootMessenger); - const rule = new EntropyRule(messenger); + const messenger = getRootMessenger(); + const accountTreeControllerMessenger = + getAccountTreeControllerMessenger(messenger); + const rule = new EntropyRule(accountTreeControllerMessenger); const mockSolanaAccount: InternalAccount = { ...MOCK_HD_ACCOUNT_1, @@ -264,7 +226,7 @@ describe('EntropyRule', () => { }, }; - rootMessenger.registerActionHandler( + messenger.registerActionHandler( 'AccountsController:getAccount', (accountId: string) => { const accounts: Record = { diff --git a/packages/account-tree-controller/src/rules/keyring.test.ts b/packages/account-tree-controller/src/rules/keyring.test.ts index 033b2c4239a..441c4652b14 100644 --- a/packages/account-tree-controller/src/rules/keyring.test.ts +++ b/packages/account-tree-controller/src/rules/keyring.test.ts @@ -4,20 +4,16 @@ import { toAccountWalletId, AccountWalletType, } from '@metamask/account-api'; -import { Messenger } from '@metamask/base-controller'; import { EthAccountType, EthMethod, EthScope } from '@metamask/keyring-api'; import { KeyringTypes } from '@metamask/keyring-controller'; import type { InternalAccount } from '@metamask/keyring-internal-api'; import { KeyringRule, getAccountWalletNameFromKeyringType } from './keyring'; +import { + getAccountTreeControllerMessenger, + getRootMessenger, +} from '../../tests/mockMessenger'; import type { AccountGroupObjectOf } from '../group'; -import type { - AccountTreeControllerMessenger, - AccountTreeControllerActions, - AccountTreeControllerEvents, - AllowedActions, - AllowedEvents, -} from '../types'; import type { AccountWalletKeyringObject, AccountWalletObjectOf, @@ -70,52 +66,14 @@ describe('keyring', () => { }, }; - /** - * 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', - ], - allowedActions: [ - 'AccountsController:listMultichainAccounts', - 'AccountsController:getAccount', - 'AccountsController:getSelectedMultichainAccount', - 'AccountsController:setSelectedAccount', - 'KeyringController:getState', - 'SnapController:get', - ], - }); - } - describe('getComputedAccountGroupName', () => { it('uses BaseRule implementation', () => { - const rootMessenger = getRootMessenger(); - const messenger = getAccountTreeControllerMessenger(rootMessenger); - const rule = new KeyringRule(messenger); + const messenger = getRootMessenger(); + const accountTreeControllerMessenger = + getAccountTreeControllerMessenger(messenger); + const rule = new KeyringRule(accountTreeControllerMessenger); - rootMessenger.registerActionHandler( + messenger.registerActionHandler( 'AccountsController:getAccount', () => MOCK_HARDWARE_ACCOUNT_1, ); @@ -140,11 +98,12 @@ describe('keyring', () => { }); it('returns empty string when account is not found', () => { - const rootMessenger = getRootMessenger(); - const messenger = getAccountTreeControllerMessenger(rootMessenger); - const rule = new KeyringRule(messenger); + const messenger = getRootMessenger(); + const accountTreeControllerMessenger = + getAccountTreeControllerMessenger(messenger); + const rule = new KeyringRule(accountTreeControllerMessenger); - rootMessenger.registerActionHandler( + messenger.registerActionHandler( 'AccountsController:getAccount', () => undefined, ); @@ -198,12 +157,13 @@ describe('keyring', () => { ); it('getComputedAccountGroupName returns computed name from base class', () => { - const rootMessenger = getRootMessenger(); - const messenger = getAccountTreeControllerMessenger(rootMessenger); - const rule = new KeyringRule(messenger); + const messenger = getRootMessenger(); + const accountTreeControllerMessenger = + getAccountTreeControllerMessenger(messenger); + const rule = new KeyringRule(accountTreeControllerMessenger); // Mock the AccountsController to always return the account - rootMessenger.registerActionHandler( + messenger.registerActionHandler( 'AccountsController:getAccount', () => MOCK_HARDWARE_ACCOUNT_1, ); @@ -231,12 +191,13 @@ describe('keyring', () => { }); it('getComputedAccountGroupName returns empty string when account not found', () => { - const rootMessenger = getRootMessenger(); - const messenger = getAccountTreeControllerMessenger(rootMessenger); - const rule = new KeyringRule(messenger); + const messenger = getRootMessenger(); + const accountTreeControllerMessenger = + getAccountTreeControllerMessenger(messenger); + const rule = new KeyringRule(accountTreeControllerMessenger); // Mock the AccountsController to return undefined (account not found) - rootMessenger.registerActionHandler( + messenger.registerActionHandler( 'AccountsController:getAccount', () => undefined, ); @@ -263,9 +224,10 @@ describe('keyring', () => { }); it('getDefaultAccountWalletName returns wallet name based on keyring type', () => { - const rootMessenger = getRootMessenger(); - const messenger = getAccountTreeControllerMessenger(rootMessenger); - const rule = new KeyringRule(messenger); + const messenger = getRootMessenger(); + const accountTreeControllerMessenger = + getAccountTreeControllerMessenger(messenger); + const rule = new KeyringRule(accountTreeControllerMessenger); const hdWallet: AccountWalletObjectOf = { id: toAccountWalletId(AccountWalletType.Keyring, KeyringTypes.hd), diff --git a/packages/account-tree-controller/src/rules/snap.test.ts b/packages/account-tree-controller/src/rules/snap.test.ts index 839d859856d..672f074db72 100644 --- a/packages/account-tree-controller/src/rules/snap.test.ts +++ b/packages/account-tree-controller/src/rules/snap.test.ts @@ -4,7 +4,6 @@ import { toAccountWalletId, AccountWalletType, } from '@metamask/account-api'; -import { Messenger } from '@metamask/base-controller'; import { EthAccountType, EthMethod, EthScope } from '@metamask/keyring-api'; import { KeyringTypes } from '@metamask/keyring-controller'; import type { InternalAccount } from '@metamask/keyring-internal-api'; @@ -12,14 +11,11 @@ import type { SnapId } from '@metamask/snaps-sdk'; import type { Snap } from '@metamask/snaps-utils'; import { SnapRule } from './snap'; +import { + getAccountTreeControllerMessenger, + getRootMessenger, +} from '../../tests/mockMessenger'; import type { AccountGroupObjectOf } from '../group'; -import type { - AccountTreeControllerMessenger, - AccountTreeControllerActions, - AccountTreeControllerEvents, - AllowedActions, - AllowedEvents, -} from '../types'; import type { AccountWalletObjectOf, AccountWalletSnapObject } from '../wallet'; const ETH_EOA_METHODS = [ @@ -58,55 +54,16 @@ const MOCK_SNAP_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', - 'MultichainAccountService:walletStatusChange', - ], - allowedActions: [ - 'AccountsController:listMultichainAccounts', - 'AccountsController:getAccount', - 'AccountsController:getSelectedMultichainAccount', - 'AccountsController:setSelectedAccount', - 'KeyringController:getState', - 'SnapController:get', - ], - }); -} - describe('SnapRule', () => { describe('getComputedAccountGroupName', () => { it('returns computed name from base class', () => { - const rootMessenger = getRootMessenger(); - const messenger = getAccountTreeControllerMessenger(rootMessenger); - const rule = new SnapRule(messenger); + const messenger = getRootMessenger(); + const accountTreeControllerMessenger = + getAccountTreeControllerMessenger(messenger); + const rule = new SnapRule(accountTreeControllerMessenger); // Mock the AccountsController to return an account - rootMessenger.registerActionHandler( + messenger.registerActionHandler( 'AccountsController:getAccount', () => MOCK_SNAP_ACCOUNT_1, ); @@ -131,12 +88,13 @@ describe('SnapRule', () => { }); it('returns empty string when account not found', () => { - const rootMessenger = getRootMessenger(); - const messenger = getAccountTreeControllerMessenger(rootMessenger); - const rule = new SnapRule(messenger); + const messenger = getRootMessenger(); + const accountTreeControllerMessenger = + getAccountTreeControllerMessenger(messenger); + const rule = new SnapRule(accountTreeControllerMessenger); // Mock the AccountsController to return undefined (account not found) - rootMessenger.registerActionHandler( + messenger.registerActionHandler( 'AccountsController:getAccount', () => undefined, ); @@ -174,12 +132,13 @@ describe('SnapRule', () => { describe('getDefaultAccountWalletName', () => { it('returns snap proposed name when available', () => { - const rootMessenger = getRootMessenger(); - const messenger = getAccountTreeControllerMessenger(rootMessenger); - const rule = new SnapRule(messenger); + const messenger = getRootMessenger(); + const accountTreeControllerMessenger = + getAccountTreeControllerMessenger(messenger); + const rule = new SnapRule(accountTreeControllerMessenger); // Mock SnapController to return snap with proposed name - rootMessenger.registerActionHandler( + messenger.registerActionHandler( 'SnapController:get', () => MOCK_SNAP_1 as unknown as Snap, ); @@ -199,9 +158,10 @@ describe('SnapRule', () => { }); it('returns cleaned snap ID when no proposed name available', () => { - const rootMessenger = getRootMessenger(); - const messenger = getAccountTreeControllerMessenger(rootMessenger); - const rule = new SnapRule(messenger); + const messenger = getRootMessenger(); + const accountTreeControllerMessenger = + getAccountTreeControllerMessenger(messenger); + const rule = new SnapRule(accountTreeControllerMessenger); const snapWithoutProposedName = { id: 'npm:@metamask/example-snap' as unknown as SnapId, @@ -215,7 +175,7 @@ describe('SnapRule', () => { }; // Mock SnapController to return snap without proposed name - rootMessenger.registerActionHandler( + messenger.registerActionHandler( 'SnapController:get', () => snapWithoutProposedName as unknown as Snap, ); @@ -239,15 +199,13 @@ describe('SnapRule', () => { }); it('returns cleaned snap ID when snap not found', () => { - const rootMessenger = getRootMessenger(); - const messenger = getAccountTreeControllerMessenger(rootMessenger); - const rule = new SnapRule(messenger); + const messenger = getRootMessenger(); + const accountTreeControllerMessenger = + getAccountTreeControllerMessenger(messenger); + const rule = new SnapRule(accountTreeControllerMessenger); // Mock SnapController to return undefined (snap not found) - rootMessenger.registerActionHandler( - 'SnapController:get', - () => undefined, - ); + messenger.registerActionHandler('SnapController:get', () => undefined); const snapId = 'npm:@metamask/missing-snap'; const wallet: AccountWalletObjectOf = { diff --git a/packages/account-tree-controller/src/types.ts b/packages/account-tree-controller/src/types.ts index 2f3b579e32e..da288a1b565 100644 --- a/packages/account-tree-controller/src/types.ts +++ b/packages/account-tree-controller/src/types.ts @@ -12,10 +12,10 @@ import type { import { type ControllerGetStateAction, type ControllerStateChangeEvent, - type RestrictedMessenger, } from '@metamask/base-controller'; import type { TraceCallback } from '@metamask/controller-utils'; import type { KeyringControllerGetStateAction } from '@metamask/keyring-controller'; +import type { Messenger } from '@metamask/messenger'; import type { MultichainAccountServiceCreateMultichainAccountGroupAction } from '@metamask/multichain-account-service'; import type { AuthenticationController, @@ -174,12 +174,10 @@ export type AccountTreeControllerEvents = | AccountTreeControllerAccountTreeChangeEvent | AccountTreeControllerSelectedAccountGroupChangeEvent; -export type AccountTreeControllerMessenger = RestrictedMessenger< +export type AccountTreeControllerMessenger = Messenger< typeof controllerName, AccountTreeControllerActions | AllowedActions, - AccountTreeControllerEvents | AllowedEvents, - AllowedActions['type'], - AllowedEvents['type'] + AccountTreeControllerEvents | AllowedEvents >; export type AccountTreeControllerConfig = { diff --git a/packages/account-tree-controller/tests/mockMessenger.ts b/packages/account-tree-controller/tests/mockMessenger.ts new file mode 100644 index 00000000000..10fa770a675 --- /dev/null +++ b/packages/account-tree-controller/tests/mockMessenger.ts @@ -0,0 +1,71 @@ +import { + Messenger, + MOCK_ANY_NAMESPACE, + type MockAnyNamespace, + type MessengerActions, + type MessengerEvents, +} from '@metamask/messenger'; + +import type { AccountTreeControllerMessenger } from '../src/types'; + +type AllAccountTreeControllerActions = + MessengerActions; + +type AllAccountTreeControllerEvents = + MessengerEvents; + +/** + * Creates a new root messenger instance for testing. + * + * @returns A new Messenger instance. + */ +export function getRootMessenger() { + return new Messenger< + MockAnyNamespace, + AllAccountTreeControllerActions, + AllAccountTreeControllerEvents + >({ namespace: MOCK_ANY_NAMESPACE }); +} + +/** + * Retrieves a messenger for the AccountTreeController. + * + * @param rootMessenger - The root messenger instance. + * @returns The messenger for the AccountTreeController. + */ +export function getAccountTreeControllerMessenger( + rootMessenger: ReturnType, +): AccountTreeControllerMessenger { + const accountTreeControllerMessenger = new Messenger< + 'AccountTreeController', + AllAccountTreeControllerActions, + AllAccountTreeControllerEvents, + typeof rootMessenger + >({ namespace: 'AccountTreeController', parent: rootMessenger }); + rootMessenger.delegate({ + messenger: accountTreeControllerMessenger, + events: [ + 'AccountsController:accountAdded', + 'AccountsController:accountRemoved', + 'AccountsController:selectedAccountChange', + 'UserStorageController:stateChange', + 'MultichainAccountService:walletStatusChange', + ], + actions: [ + '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', + ], + }); + return accountTreeControllerMessenger; +} diff --git a/packages/account-tree-controller/tsconfig.build.json b/packages/account-tree-controller/tsconfig.build.json index 707a559080c..d52110b5b4f 100644 --- a/packages/account-tree-controller/tsconfig.build.json +++ b/packages/account-tree-controller/tsconfig.build.json @@ -9,6 +9,7 @@ { "path": "../accounts-controller/tsconfig.build.json" }, { "path": "../base-controller/tsconfig.build.json" }, { "path": "../keyring-controller/tsconfig.build.json" }, + { "path": "../messenger/tsconfig.build.json" }, { "path": "../multichain-account-service/tsconfig.build.json" }, { "path": "../profile-sync-controller/tsconfig.build.json" } ], diff --git a/packages/account-tree-controller/tsconfig.json b/packages/account-tree-controller/tsconfig.json index ca31cc28bbc..80394fb0531 100644 --- a/packages/account-tree-controller/tsconfig.json +++ b/packages/account-tree-controller/tsconfig.json @@ -13,6 +13,9 @@ { "path": "../accounts-controller" }, + { + "path": "../messenger" + }, { "path": "../multichain-account-service" }, @@ -20,5 +23,5 @@ "path": "../profile-sync-controller" } ], - "include": ["../../types", "./src"] + "include": ["../../types", "./src", "./tests"] } diff --git a/yarn.lock b/yarn.lock index f67c9bc6cb0..76b5b99bb9b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2576,6 +2576,7 @@ __metadata: "@metamask/base-controller": "npm:^8.4.2" "@metamask/keyring-api": "npm:^21.0.0" "@metamask/keyring-controller": "npm:^23.2.0" + "@metamask/messenger": "npm:^0.3.0" "@metamask/multichain-account-service": "npm:^1.6.2" "@metamask/profile-sync-controller": "npm:^25.1.2" "@metamask/providers": "npm:^22.1.0"