diff --git a/README.md b/README.md index 443b91c2b94..d1e5e829826 100644 --- a/README.md +++ b/README.md @@ -154,6 +154,7 @@ linkStyle default opacity:0.5 account_tree_controller --> multichain_account_service; account_tree_controller --> profile_sync_controller; accounts_controller --> base_controller; + accounts_controller --> messenger; accounts_controller --> controller_utils; accounts_controller --> keyring_controller; accounts_controller --> network_controller; diff --git a/packages/accounts-controller/CHANGELOG.md b/packages/accounts-controller/CHANGELOG.md index 6f3975077bf..b88829f19e7 100644 --- a/packages/accounts-controller/CHANGELOG.md +++ b/packages/accounts-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` ([#6426](https://github.com/MetaMask/core/pull/6426)) + - Previously, `AccountsController` accepted a `RestrictedMessenger` instance from `@metamask/base-controller`. + ## [33.2.0] ### Added diff --git a/packages/accounts-controller/package.json b/packages/accounts-controller/package.json index 14d01134071..5ad69009409 100644 --- a/packages/accounts-controller/package.json +++ b/packages/accounts-controller/package.json @@ -53,6 +53,7 @@ "@metamask/keyring-api": "^21.0.0", "@metamask/keyring-internal-api": "^9.0.0", "@metamask/keyring-utils": "^3.1.0", + "@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/accounts-controller/src/AccountsController.test.ts b/packages/accounts-controller/src/AccountsController.test.ts index 33165be78d6..45a54ebaa04 100644 --- a/packages/accounts-controller/src/AccountsController.test.ts +++ b/packages/accounts-controller/src/AccountsController.test.ts @@ -1,4 +1,4 @@ -import { Messenger, deriveStateFromMetadata } from '@metamask/base-controller'; +import { deriveStateFromMetadata } from '@metamask/base-controller/next'; import { InfuraNetworkType } from '@metamask/controller-utils'; import type { AccountAssetListUpdatedEventPayload, @@ -15,6 +15,13 @@ import { } from '@metamask/keyring-api'; import { KeyringTypes } from '@metamask/keyring-controller'; import type { InternalAccount } from '@metamask/keyring-internal-api'; +import { + MOCK_ANY_NAMESPACE, + Messenger, + type MessengerActions, + type MessengerEvents, + type MockAnyNamespace, +} from '@metamask/messenger'; import type { NetworkClientId } from '@metamask/network-controller'; import type { SnapControllerState } from '@metamask/snaps-controllers'; import { SnapStatus } from '@metamask/snaps-utils'; @@ -23,11 +30,8 @@ import type { V4Options } from 'uuid'; import * as uuid from 'uuid'; import type { - AccountsControllerActions, - AccountsControllerEvents, + AccountsControllerMessenger, AccountsControllerState, - AllowedActions, - AllowedEvents, } from './AccountsController'; import { AccountsController, EMPTY_ACCOUNT } from './AccountsController'; import { @@ -41,6 +45,17 @@ import { keyringTypeToName, } from './utils'; +type AllAccountsControllerActions = + MessengerActions; + +type AllAccountsControllerEvents = MessengerEvents; + +type RootMessenger = Messenger< + MockAnyNamespace, + AllAccountsControllerActions, + AllAccountsControllerEvents +>; + jest.mock('uuid'); const mockUUID = jest.spyOn(uuid, 'v4'); const actualUUID = jest.requireActual('uuid').v4; // We also use uuid.v4 in our mocks @@ -217,27 +232,37 @@ function setExpectedLastSelectedAsAny( } /** - * Builds a new instance of the Messenger class for the AccountsController. + * Builds a new instance of the root messenger. * - * @returns A new instance of the Messenger class for the AccountsController. + * @returns A new instance of the root messenger. */ -function buildMessenger() { - return new Messenger< - AccountsControllerActions | AllowedActions, - AccountsControllerEvents | AllowedEvents - >(); +function buildMessenger(): RootMessenger { + return new Messenger({ namespace: MOCK_ANY_NAMESPACE }); } /** - * Builds a restricted messenger for the AccountsController. + * Builds a messenger for the AccountsController. * - * @param messenger - The messenger to restrict. - * @returns The restricted messenger. + * @param rootMessenger - The root messenger. + * @returns The messenger for AccountsController. */ -function buildAccountsControllerMessenger(messenger = buildMessenger()) { - return messenger.getRestricted({ - name: 'AccountsController', - allowedEvents: [ +function buildAccountsControllerMessenger(rootMessenger = buildMessenger()) { + const accountsControllerMessenger = new Messenger< + 'AccountsController', + AllAccountsControllerActions, + AllAccountsControllerEvents, + typeof rootMessenger + >({ + namespace: 'AccountsController', + parent: rootMessenger, + }); + rootMessenger.delegate({ + messenger: accountsControllerMessenger, + actions: [ + 'KeyringController:getState', + 'KeyringController:getKeyringsByType', + ], + events: [ 'SnapController:stateChange', 'KeyringController:stateChange', 'SnapKeyring:accountAssetListUpdated', @@ -245,11 +270,8 @@ function buildAccountsControllerMessenger(messenger = buildMessenger()) { 'SnapKeyring:accountTransactionsUpdated', 'MultichainNetworkController:networkDidChange', ], - allowedActions: [ - 'KeyringController:getState', - 'KeyringController:getKeyringsByType', - ], }); + return accountsControllerMessenger; } /** @@ -257,7 +279,7 @@ function buildAccountsControllerMessenger(messenger = buildMessenger()) { * * @param options - The options object. * @param [options.initialState] - The initial state to use for the AccountsController. - * @param [options.messenger] - Messenger to use for the AccountsController. + * @param [options.messenger] - The root messenger to use for creating the AccountsController messenger. * @returns An instance of the AccountsController class. */ function setupAccountsController({ @@ -265,16 +287,11 @@ function setupAccountsController({ messenger = buildMessenger(), }: { initialState?: Partial; - messenger?: Messenger< - AccountsControllerActions | AllowedActions, - AccountsControllerEvents | AllowedEvents - >; -} = {}): { + messenger?: RootMessenger; +}): { accountsController: AccountsController; - messenger: Messenger< - AccountsControllerActions | AllowedActions, - AccountsControllerEvents | AllowedEvents - >; + messenger: RootMessenger; + accountsControllerMessenger: AccountsControllerMessenger; triggerMultichainNetworkChange: (id: NetworkClientId | CaipChainId) => void; } { const accountsControllerMessenger = @@ -288,7 +305,12 @@ function setupAccountsController({ const triggerMultichainNetworkChange = (id: NetworkClientId | CaipChainId) => messenger.publish('MultichainNetworkController:networkDidChange', id); - return { accountsController, messenger, triggerMultichainNetworkChange }; + return { + accountsController, + messenger, + accountsControllerMessenger, + triggerMultichainNetworkChange, + }; } describe('AccountsController', () => { @@ -1136,11 +1158,10 @@ describe('AccountsController', () => { it('publishes accountAdded event', async () => { const messenger = buildMessenger(); - const messengerSpy = jest.spyOn(messenger, 'publish'); mockUUIDWithNormalAccounts([mockAccount, mockAccount2]); - setupAccountsController({ + const { accountsControllerMessenger } = setupAccountsController({ initialState: { internalAccounts: { accounts: { @@ -1152,6 +1173,8 @@ describe('AccountsController', () => { messenger, }); + const messengerSpy = jest.spyOn(accountsControllerMessenger, 'publish'); + const mockNewKeyringState = { isUnlocked: true, keyrings: [ @@ -1172,11 +1195,10 @@ describe('AccountsController', () => { [], ); - // First call is 'KeyringController:stateChange' + // First call is 'AccountsController:stateChange' expect(messengerSpy).toHaveBeenNthCalledWith( - // 1. KeyringController:stateChange - // 2. AccountsController:stateChange - 3, + // 1. AccountsController:stateChange + 2, 'AccountsController:accountAdded', MockExpectedInternalAccountBuilder.from(mockAccount2) .setExpectedLastSelectedAsAny() @@ -1437,11 +1459,10 @@ describe('AccountsController', () => { it('publishes accountRemoved event', async () => { const messenger = buildMessenger(); - const messengerSpy = jest.spyOn(messenger, 'publish'); mockUUIDWithNormalAccounts([mockAccount, mockAccount2]); - setupAccountsController({ + const { accountsControllerMessenger } = setupAccountsController({ initialState: { internalAccounts: { accounts: { @@ -1454,6 +1475,8 @@ describe('AccountsController', () => { messenger, }); + const messengerSpy = jest.spyOn(accountsControllerMessenger, 'publish'); + const mockNewKeyringState = { isUnlocked: true, keyrings: [ @@ -1473,11 +1496,10 @@ describe('AccountsController', () => { [], ); - // First call is 'KeyringController:stateChange' + // First call is 'AccountsController:stateChange' expect(messengerSpy).toHaveBeenNthCalledWith( - // 1. KeyringController:stateChange - // 2. AccountsController:stateChange - 3, + // 1. AccountsController:stateChange + 2, 'AccountsController:accountRemoved', mockAccount3.id, ); @@ -3165,19 +3187,20 @@ describe('AccountsController', () => { }, type: BtcAccountType.P2wpkh, }); - const { accountsController, messenger } = setupAccountsController({ - initialState: { - internalAccounts: { - accounts: { - [mockAccount.id]: mockAccount, - [mockNonEvmAccount.id]: mockNonEvmAccount, + const { accountsController, accountsControllerMessenger } = + setupAccountsController({ + initialState: { + internalAccounts: { + accounts: { + [mockAccount.id]: mockAccount, + [mockNonEvmAccount.id]: mockNonEvmAccount, + }, + selectedAccount: mockAccount.id, }, - selectedAccount: mockAccount.id, }, - }, - }); + }); - const messengerSpy = jest.spyOn(messenger, 'publish'); + const messengerSpy = jest.spyOn(accountsControllerMessenger, 'publish'); accountsController.setSelectedAccount(mockNonEvmAccount.id); @@ -3271,10 +3294,10 @@ describe('AccountsController', () => { }); it('publishes the accountRenamed event', () => { - const { accountsController, messenger } = + const { accountsController, accountsControllerMessenger } = setupAccountsController(mockState); - const messengerSpy = jest.spyOn(messenger, 'publish'); + const messengerSpy = jest.spyOn(accountsControllerMessenger, 'publish'); accountsController.setAccountNameAndSelectAccount( mockAccount.id, @@ -3349,16 +3372,17 @@ describe('AccountsController', () => { }); it('publishes the accountRenamed event', () => { - const { accountsController, messenger } = setupAccountsController({ - initialState: { - internalAccounts: { - accounts: { [mockAccount.id]: mockAccount }, - selectedAccount: mockAccount.id, + const { accountsController, accountsControllerMessenger } = + setupAccountsController({ + initialState: { + internalAccounts: { + accounts: { [mockAccount.id]: mockAccount }, + selectedAccount: mockAccount.id, + }, }, - }, - }); + }); - const messengerSpy = jest.spyOn(messenger, 'publish'); + const messengerSpy = jest.spyOn(accountsControllerMessenger, 'publish'); accountsController.setAccountName(mockAccount.id, 'new name'); @@ -3922,19 +3946,19 @@ describe('AccountsController', () => { describe('metadata', () => { it('includes expected state in debug snapshots', () => { - const { accountsController: controller } = setupAccountsController(); + const { accountsController: controller } = setupAccountsController({}); expect( deriveStateFromMetadata( controller.state, controller.metadata, - 'anonymous', + 'includeInDebugSnapshot', ), ).toMatchInlineSnapshot(`Object {}`); }); it('includes expected state in state logs', () => { - const { accountsController: controller } = setupAccountsController(); + const { accountsController: controller } = setupAccountsController({}); expect( deriveStateFromMetadata( @@ -3953,7 +3977,7 @@ describe('AccountsController', () => { }); it('persists expected state', () => { - const { accountsController: controller } = setupAccountsController(); + const { accountsController: controller } = setupAccountsController({}); expect( deriveStateFromMetadata( @@ -3972,7 +3996,7 @@ describe('AccountsController', () => { }); it('exposes expected state to UI', () => { - const { accountsController: controller } = setupAccountsController(); + const { accountsController: controller } = setupAccountsController({}); expect( deriveStateFromMetadata( diff --git a/packages/accounts-controller/src/AccountsController.ts b/packages/accounts-controller/src/AccountsController.ts index 2bd0fb04af9..a4b7287d158 100644 --- a/packages/accounts-controller/src/AccountsController.ts +++ b/packages/accounts-controller/src/AccountsController.ts @@ -1,10 +1,8 @@ import { type ControllerGetStateAction, type ControllerStateChangeEvent, - type ExtractEventPayload, - type RestrictedMessenger, BaseController, -} from '@metamask/base-controller'; +} from '@metamask/base-controller/next'; import { type SnapKeyringAccountAssetListUpdatedEvent, type SnapKeyringAccountBalancesUpdatedEvent, @@ -29,6 +27,7 @@ import { } from '@metamask/keyring-controller'; import type { InternalAccount } from '@metamask/keyring-internal-api'; import { isScopeEqualToAny } from '@metamask/keyring-utils'; +import type { Messenger, ExtractEventPayload } from '@metamask/messenger'; import type { NetworkClientId } from '@metamask/network-controller'; import type { SnapControllerState, @@ -218,19 +217,17 @@ export type AccountsControllerEvents = | AccountsControllerAccountTransactionsUpdatedEvent | AccountsControllerAccountAssetListUpdatedEvent; -export type AccountsControllerMessenger = RestrictedMessenger< +export type AccountsControllerMessenger = Messenger< typeof controllerName, AccountsControllerActions | AllowedActions, - AccountsControllerEvents | AllowedEvents, - AllowedActions['type'], - AllowedEvents['type'] + AccountsControllerEvents | AllowedEvents >; const accountsControllerMetadata = { internalAccounts: { includeInStateLogs: true, persist: true, - anonymous: false, + includeInDebugSnapshot: false, usedInUi: true, }, }; @@ -502,7 +499,7 @@ export class AccountsController extends BaseController< state.internalAccounts.selectedAccount = account.id; }); - this.messagingSystem.publish( + this.messenger.publish( 'AccountsController:accountRenamed', internalAccount, ); @@ -546,7 +543,7 @@ export class AccountsController extends BaseController< }); if (metadata.name) { - this.messagingSystem.publish( + this.messenger.publish( 'AccountsController:accountRenamed', internalAccount, ); @@ -566,9 +563,7 @@ export class AccountsController extends BaseController< const internalAccounts: AccountsControllerState['internalAccounts']['accounts'] = {}; - const { keyrings } = this.messagingSystem.call( - 'KeyringController:getState', - ); + const { keyrings } = this.messenger.call('KeyringController:getState'); for (const keyring of keyrings) { const keyringTypeName = keyringTypeToName(keyring.type); @@ -725,7 +720,7 @@ export class AccountsController extends BaseController< * @returns The Snap keyring if available. */ #getSnapKeyring(): SnapKeyring | undefined { - const [snapKeyring] = this.messagingSystem.call( + const [snapKeyring] = this.messenger.call( 'KeyringController:getKeyringsByType', SnapKeyring.type, ); @@ -749,7 +744,7 @@ export class AccountsController extends BaseController< event: EventType, ...payload: ExtractEventPayload ): void { - this.messagingSystem.publish(event, ...payload); + this.messenger.publish(event, ...payload); } /** @@ -903,14 +898,11 @@ export class AccountsController extends BaseController< () => { // Now publish events for (const id of diff.removed) { - this.messagingSystem.publish('AccountsController:accountRemoved', id); + this.messenger.publish('AccountsController:accountRemoved', id); } for (const account of diff.added) { - this.messagingSystem.publish( - 'AccountsController:accountAdded', - account, - ); + this.messenger.publish('AccountsController:accountAdded', account); } }, ); @@ -971,12 +963,12 @@ export class AccountsController extends BaseController< // `selectedAccount` to be non-empty. if (account) { if (isEvmAccountType(account.type)) { - this.messagingSystem.publish( + this.messenger.publish( 'AccountsController:selectedEvmAccountChange', account, ); } - this.messagingSystem.publish( + this.messenger.publish( 'AccountsController:selectedAccountChange', account, ); @@ -1218,17 +1210,15 @@ export class AccountsController extends BaseController< * Subscribes to message events. */ #subscribeToMessageEvents() { - this.messagingSystem.subscribe( - 'SnapController:stateChange', - (snapStateState) => this.#handleOnSnapStateChange(snapStateState), + this.messenger.subscribe('SnapController:stateChange', (snapStateState) => + this.#handleOnSnapStateChange(snapStateState), ); - this.messagingSystem.subscribe( - 'KeyringController:stateChange', - (keyringState) => this.#handleOnKeyringStateChange(keyringState), + this.messenger.subscribe('KeyringController:stateChange', (keyringState) => + this.#handleOnKeyringStateChange(keyringState), ); - this.messagingSystem.subscribe( + this.messenger.subscribe( 'SnapKeyring:accountAssetListUpdated', (snapAccountEvent) => this.#handleOnSnapKeyringAccountEvent( @@ -1237,7 +1227,7 @@ export class AccountsController extends BaseController< ), ); - this.messagingSystem.subscribe( + this.messenger.subscribe( 'SnapKeyring:accountBalancesUpdated', (snapAccountEvent) => this.#handleOnSnapKeyringAccountEvent( @@ -1246,7 +1236,7 @@ export class AccountsController extends BaseController< ), ); - this.messagingSystem.subscribe( + this.messenger.subscribe( 'SnapKeyring:accountTransactionsUpdated', (snapAccountEvent) => this.#handleOnSnapKeyringAccountEvent( @@ -1256,7 +1246,7 @@ export class AccountsController extends BaseController< ); // Handle account change when multichain network is changed - this.messagingSystem.subscribe( + this.messenger.subscribe( 'MultichainNetworkController:networkDidChange', (id) => this.#handleOnMultichainNetworkDidChange(id), ); @@ -1266,67 +1256,67 @@ export class AccountsController extends BaseController< * Registers message handlers for the AccountsController. */ #registerMessageHandlers() { - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( `${controllerName}:setSelectedAccount`, this.setSelectedAccount.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( `${controllerName}:listAccounts`, this.listAccounts.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( `${controllerName}:listMultichainAccounts`, this.listMultichainAccounts.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( `${controllerName}:setAccountName`, this.setAccountName.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( `${controllerName}:setAccountNameAndSelectAccount`, this.setAccountNameAndSelectAccount.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( `${controllerName}:updateAccounts`, this.updateAccounts.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( `${controllerName}:getSelectedAccount`, this.getSelectedAccount.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( `${controllerName}:getSelectedMultichainAccount`, this.getSelectedMultichainAccount.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( `${controllerName}:getAccountByAddress`, this.getAccountByAddress.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( `${controllerName}:getNextAvailableAccountName`, this.getNextAvailableAccountName.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( `AccountsController:getAccount`, this.getAccount.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( `AccountsController:getAccounts`, this.getAccounts.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( `AccountsController:updateAccountMetadata`, this.updateAccountMetadata.bind(this), ); diff --git a/packages/accounts-controller/tsconfig.build.json b/packages/accounts-controller/tsconfig.build.json index 2ccd968d36d..d95a2cae72e 100644 --- a/packages/accounts-controller/tsconfig.build.json +++ b/packages/accounts-controller/tsconfig.build.json @@ -11,7 +11,8 @@ "path": "../base-controller/tsconfig.build.json" }, { "path": "../keyring-controller/tsconfig.build.json" }, - { "path": "../network-controller/tsconfig.build.json" } + { "path": "../network-controller/tsconfig.build.json" }, + { "path": "../messenger/tsconfig.build.json" } ], "include": ["../../types", "./src"] } diff --git a/packages/accounts-controller/tsconfig.json b/packages/accounts-controller/tsconfig.json index 12cd20ecb5c..d568399237d 100644 --- a/packages/accounts-controller/tsconfig.json +++ b/packages/accounts-controller/tsconfig.json @@ -10,7 +10,8 @@ { "path": "../keyring-controller" }, - { "path": "../network-controller" } + { "path": "../network-controller" }, + { "path": "../messenger" } ], "include": ["../../types", "./src", "src/tests"] } diff --git a/yarn.lock b/yarn.lock index 76b5b99bb9b..32d4fe097e1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2620,6 +2620,7 @@ __metadata: "@metamask/keyring-controller": "npm:^23.2.0" "@metamask/keyring-internal-api": "npm:^9.0.0" "@metamask/keyring-utils": "npm:^3.1.0" + "@metamask/messenger": "npm:^0.3.0" "@metamask/network-controller": "npm:^24.3.1" "@metamask/providers": "npm:^22.1.0" "@metamask/snaps-controllers": "npm:^14.0.1"