From 05e48153dd3ea758cf47d53c266bda5d911e89ed Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Tue, 7 Oct 2025 12:24:30 +0200 Subject: [PATCH 1/2] refactor: migrate {Authentication,UserStorage}Controller to @metamask/messenger --- packages/profile-sync-controller/CHANGELOG.md | 5 ++ packages/profile-sync-controller/package.json | 1 + .../AuthenticationController.test.ts | 67 +++++++++++++---- .../AuthenticationController.ts | 58 +++++++-------- .../user-storage/UserStorageController.ts | 72 +++++++++---------- .../__fixtures__/mockMessenger.ts | 51 ++++++++++--- .../tsconfig.build.json | 3 +- .../profile-sync-controller/tsconfig.json | 3 +- yarn.lock | 1 + 9 files changed, 164 insertions(+), 97 deletions(-) diff --git a/packages/profile-sync-controller/CHANGELOG.md b/packages/profile-sync-controller/CHANGELOG.md index 2ce826ff3a2..b675a2b3c86 100644 --- a/packages/profile-sync-controller/CHANGELOG.md +++ b/packages/profile-sync-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` ([#6533](https://github.com/MetaMask/core/pull/6533)) + - Previously, `AuthenticationController` and `UserStorageController` accepted a `RestrictedMessenger` instance from `@metamask/base-controller`. + ## [25.1.0] ### Changed diff --git a/packages/profile-sync-controller/package.json b/packages/profile-sync-controller/package.json index 33d3dd634f8..6d147875d8e 100644 --- a/packages/profile-sync-controller/package.json +++ b/packages/profile-sync-controller/package.json @@ -101,6 +101,7 @@ }, "dependencies": { "@metamask/base-controller": "^8.4.0", + "@metamask/messenger": "^0.3.0", "@metamask/snaps-sdk": "^9.0.0", "@metamask/snaps-utils": "^11.0.0", "@metamask/utils": "^11.8.1", diff --git a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts index 50933b12378..ef7f1622431 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts @@ -1,9 +1,15 @@ -import { Messenger, deriveStateFromMetadata } from '@metamask/base-controller'; +import { deriveStateFromMetadata } from '@metamask/base-controller/next'; +import { + Messenger, + MOCK_ANY_NAMESPACE, + type MessengerActions, + type MessengerEvents, + type MockAnyNamespace, +} from '@metamask/messenger'; import AuthenticationController from './AuthenticationController'; import type { - AllowedActions, - AllowedEvents, + AuthenticationControllerMessenger, AuthenticationControllerState, } from './AuthenticationController'; import { @@ -560,7 +566,7 @@ describe('metadata', () => { deriveStateFromMetadata( controller.state, controller.metadata, - 'anonymous', + 'includeInDebugSnapshot', ), ).toMatchInlineSnapshot(` Object { @@ -726,23 +732,52 @@ describe('metadata', () => { }); }); +type AllAuthenticationControllerActions = + MessengerActions; + +type AllAuthenticationControllerEvents = + MessengerEvents; + +type RootMessenger = Messenger< + MockAnyNamespace, + AllAuthenticationControllerActions, + AllAuthenticationControllerEvents +>; + +/** + * Constructs the root messenger. + * + * @returns A root messenger. + */ +function getRootMessenger(): RootMessenger { + return new Messenger({ namespace: MOCK_ANY_NAMESPACE }); +} + +const controllerName = 'AuthenticationController'; + /** * Jest Test Utility - create Auth Messenger * * @returns Auth Messenger */ function createAuthenticationMessenger() { - const baseMessenger = new Messenger(); - const messenger = baseMessenger.getRestricted({ - name: 'AuthenticationController', - allowedActions: [ - 'KeyringController:getState', - 'SnapController:handleRequest', - ], - allowedEvents: ['KeyringController:lock', 'KeyringController:unlock'], + const rootMessenger = getRootMessenger(); + const messenger = new Messenger< + typeof controllerName, + AllAuthenticationControllerActions, + AllAuthenticationControllerEvents, + RootMessenger + >({ + namespace: controllerName, + parent: rootMessenger, + }); + rootMessenger.delegate({ + messenger, + actions: ['KeyringController:getState', 'SnapController:handleRequest'], + events: ['KeyringController:lock', 'KeyringController:unlock'], }); - return { messenger, baseMessenger }; + return { messenger, baseMessenger: rootMessenger }; } /** @@ -771,6 +806,12 @@ function createMockAuthenticationMessenger() { mockCall.mockImplementation((...args) => { const [actionType, params] = args; if (actionType === 'SnapController:handleRequest') { + if (typeof params === 'string') { + throw new Error( + `MOCK_FAIL - unsupported SnapController:handleRequest call: ${params}`, + ); + } + if (params?.request.method === 'getPublicKey') { return mockSnapGetPublicKey(); } diff --git a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts index 7e9a62443a8..f807a68a6e3 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts @@ -1,15 +1,15 @@ -import type { - ControllerGetStateAction, - ControllerStateChangeEvent, - RestrictedMessenger, - StateMetadata, -} from '@metamask/base-controller'; -import { BaseController } from '@metamask/base-controller'; +import { + BaseController, + type ControllerGetStateAction, + type ControllerStateChangeEvent, + type StateMetadata, +} from '@metamask/base-controller/next'; import type { KeyringControllerGetStateAction, KeyringControllerLockEvent, KeyringControllerUnlockEvent, } from '@metamask/keyring-controller'; +import type { Messenger } from '@metamask/messenger'; import type { HandleSnapRequest } from '@metamask/snaps-controllers'; import type { Json } from '@metamask/utils'; @@ -46,7 +46,7 @@ const metadata: StateMetadata = { isSignedIn: { includeInStateLogs: true, persist: true, - anonymous: true, + includeInDebugSnapshot: true, usedInUi: true, }, srpSessionData: { @@ -74,7 +74,7 @@ const metadata: StateMetadata = { ); }, persist: true, - anonymous: false, + includeInDebugSnapshot: false, usedInUi: true, }, }; @@ -125,21 +125,15 @@ export type AuthenticationControllerStateChangeEvent = export type Events = AuthenticationControllerStateChangeEvent; // Allowed Actions -export type AllowedActions = - | HandleSnapRequest - | KeyringControllerGetStateAction; +type AllowedActions = HandleSnapRequest | KeyringControllerGetStateAction; -export type AllowedEvents = - | KeyringControllerLockEvent - | KeyringControllerUnlockEvent; +type AllowedEvents = KeyringControllerLockEvent | KeyringControllerUnlockEvent; // Messenger -export type AuthenticationControllerMessenger = RestrictedMessenger< +export type AuthenticationControllerMessenger = Messenger< typeof controllerName, Actions | AllowedActions, - Events | AllowedEvents, - AllowedActions['type'], - AllowedEvents['type'] + Events | AllowedEvents >; /** @@ -163,16 +157,14 @@ export default class AuthenticationController extends BaseController< readonly #keyringController = { setupLockedStateSubscriptions: () => { - const { isUnlocked } = this.messagingSystem.call( - 'KeyringController:getState', - ); + const { isUnlocked } = this.messenger.call('KeyringController:getState'); this.#isUnlocked = isUnlocked; - this.messagingSystem.subscribe('KeyringController:unlock', () => { + this.messenger.subscribe('KeyringController:unlock', () => { this.#isUnlocked = true; }); - this.messagingSystem.subscribe('KeyringController:lock', () => { + this.messenger.subscribe('KeyringController:lock', () => { this.#isUnlocked = false; }); }, @@ -239,32 +231,32 @@ export default class AuthenticationController extends BaseController< * actions. */ #registerMessageHandlers(): void { - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( 'AuthenticationController:getBearerToken', this.getBearerToken.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( 'AuthenticationController:getSessionProfile', this.getSessionProfile.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( 'AuthenticationController:isSignedIn', this.isSignedIn.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( 'AuthenticationController:performSignIn', this.performSignIn.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( 'AuthenticationController:performSignOut', this.performSignOut.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( 'AuthenticationController:getUserProfileLineage', this.getUserProfileLineage.bind(this), ); @@ -388,7 +380,7 @@ export default class AuthenticationController extends BaseController< async #snapGetPublicKey(entropySourceId?: string): Promise { this.#assertIsUnlocked('#snapGetPublicKey'); - const result = (await this.messagingSystem.call( + const result = (await this.messenger.call( 'SnapController:handleRequest', createSnapPublicKeyRequest(entropySourceId), )) as string; @@ -404,7 +396,7 @@ export default class AuthenticationController extends BaseController< async #snapGetAllPublicKeys(): Promise<[string, string][]> { this.#assertIsUnlocked('#snapGetAllPublicKeys'); - const result = (await this.messagingSystem.call( + const result = (await this.messenger.call( 'SnapController:handleRequest', createSnapAllPublicKeysRequest(), )) as [string, string][]; @@ -434,7 +426,7 @@ export default class AuthenticationController extends BaseController< this.#assertIsUnlocked('#snapSignMessage'); - const result = (await this.messagingSystem.call( + const result = (await this.messenger.call( 'SnapController:handleRequest', createSnapSignMessageRequest(message, entropySourceId), )) as string; diff --git a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts index 7d231a4c5b8..972972e2dd4 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts @@ -6,13 +6,12 @@ import type { AddressBookControllerSetAction, AddressBookControllerDeleteAction, } from '@metamask/address-book-controller'; -import type { - ControllerGetStateAction, - ControllerStateChangeEvent, - RestrictedMessenger, - StateMetadata, -} from '@metamask/base-controller'; -import { BaseController } from '@metamask/base-controller'; +import { + BaseController, + type ControllerGetStateAction, + type ControllerStateChangeEvent, + type StateMetadata, +} from '@metamask/base-controller/next'; import type { TraceCallback, TraceContext, @@ -24,6 +23,7 @@ import { type KeyringControllerLockEvent, type KeyringControllerUnlockEvent, } from '@metamask/keyring-controller'; +import type { Messenger } from '@metamask/messenger'; import type { HandleSnapRequest } from '@metamask/snaps-controllers'; import { BACKUPANDSYNC_FEATURES } from './constants'; @@ -83,31 +83,31 @@ const metadata: StateMetadata = { isBackupAndSyncEnabled: { includeInStateLogs: true, persist: true, - anonymous: true, + includeInDebugSnapshot: true, usedInUi: true, }, isBackupAndSyncUpdateLoading: { includeInStateLogs: false, persist: false, - anonymous: false, + includeInDebugSnapshot: false, usedInUi: true, }, isAccountSyncingEnabled: { includeInStateLogs: true, persist: true, - anonymous: true, + includeInDebugSnapshot: true, usedInUi: true, }, isContactSyncingEnabled: { includeInStateLogs: true, persist: true, - anonymous: true, + includeInDebugSnapshot: true, usedInUi: true, }, isContactSyncingInProgress: { includeInStateLogs: false, persist: false, - anonymous: false, + includeInDebugSnapshot: false, usedInUi: true, }, }; @@ -209,12 +209,10 @@ export type AllowedEvents = | AddressBookControllerContactDeletedEvent; // Messenger -export type UserStorageControllerMessenger = RestrictedMessenger< +export type UserStorageControllerMessenger = Messenger< typeof controllerName, Actions | AllowedActions, - Events | AllowedEvents, - AllowedActions['type'], - AllowedEvents['type'] + Events | AllowedEvents >; /** @@ -234,17 +232,17 @@ export default class UserStorageController extends BaseController< readonly #auth = { getProfileId: async (entropySourceId?: string) => { - const sessionProfile = await this.messagingSystem.call( + const sessionProfile = await this.messenger.call( 'AuthenticationController:getSessionProfile', entropySourceId, ); return sessionProfile?.profileId; }, isSignedIn: () => { - return this.messagingSystem.call('AuthenticationController:isSignedIn'); + return this.messenger.call('AuthenticationController:isSignedIn'); }, signIn: async () => { - return await this.messagingSystem.call( + return await this.messenger.call( 'AuthenticationController:performSignIn', ); }, @@ -262,16 +260,14 @@ export default class UserStorageController extends BaseController< readonly #keyringController = { setupLockedStateSubscriptions: () => { - const { isUnlocked } = this.messagingSystem.call( - 'KeyringController:getState', - ); + const { isUnlocked } = this.messenger.call('KeyringController:getState'); this.#isUnlocked = isUnlocked; - this.messagingSystem.subscribe('KeyringController:unlock', () => { + this.messenger.subscribe('KeyringController:unlock', () => { this.#isUnlocked = true; }); - this.messagingSystem.subscribe('KeyringController:lock', () => { + this.messenger.subscribe('KeyringController:lock', () => { this.#isUnlocked = false; }); }, @@ -322,12 +318,12 @@ export default class UserStorageController extends BaseController< env: this.#config.env, auth: { getAccessToken: (entropySourceId?: string) => - this.messagingSystem.call( + this.messenger.call( 'AuthenticationController:getBearerToken', entropySourceId, ), getUserProfile: async (entropySourceId?: string) => { - return await this.messagingSystem.call( + return await this.messenger.call( 'AuthenticationController:getSessionProfile', entropySourceId, ); @@ -357,7 +353,7 @@ export default class UserStorageController extends BaseController< // Contact Syncing setupContactSyncingSubscriptions({ getUserStorageControllerInstance: () => this, - getMessenger: () => this.messagingSystem, + getMessenger: () => this.messenger, trace: this.#trace, }); } @@ -367,37 +363,37 @@ export default class UserStorageController extends BaseController< * actions. */ #registerMessageHandlers(): void { - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( 'UserStorageController:performGetStorage', this.performGetStorage.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( 'UserStorageController:performGetStorageAllFeatureEntries', this.performGetStorageAllFeatureEntries.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( 'UserStorageController:performSetStorage', this.performSetStorage.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( 'UserStorageController:performBatchSetStorage', this.performBatchSetStorage.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( 'UserStorageController:performDeleteStorage', this.performDeleteStorage.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( 'UserStorageController:performBatchDeleteStorage', this.performBatchDeleteStorage.bind(this), ); - this.messagingSystem.registerActionHandler( + this.messenger.registerActionHandler( 'UserStorageController:getStorageKey', this.getStorageKey.bind(this), ); @@ -565,9 +561,7 @@ export default class UserStorageController extends BaseController< ); } - const { keyrings } = this.messagingSystem.call( - 'KeyringController:getState', - ); + const { keyrings } = this.messenger.call('KeyringController:getState'); return keyrings .filter((keyring) => keyring.type === KeyringTypes.hd.toString()) .map((keyring) => keyring.metadata.id); @@ -598,7 +592,7 @@ export default class UserStorageController extends BaseController< ); } - const result = (await this.messagingSystem.call( + const result = (await this.messenger.call( 'SnapController:handleRequest', createSnapSignMessageRequest(message, entropySourceId), )) as string; @@ -697,7 +691,7 @@ export default class UserStorageController extends BaseController< }; await syncContactsWithUserStorage(config, { - getMessenger: () => this.messagingSystem, + getMessenger: () => this.messenger, getUserStorageControllerInstance: () => this, trace: this.#trace, }); diff --git a/packages/profile-sync-controller/src/controllers/user-storage/__fixtures__/mockMessenger.ts b/packages/profile-sync-controller/src/controllers/user-storage/__fixtures__/mockMessenger.ts index 399f1dc6535..1856d732b20 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/__fixtures__/mockMessenger.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/__fixtures__/mockMessenger.ts @@ -1,5 +1,11 @@ -import type { NotNamespacedBy } from '@metamask/base-controller'; -import { Messenger } from '@metamask/base-controller'; +import { + Messenger, + MOCK_ANY_NAMESPACE, + type MockAnyNamespace, + type MessengerActions, + type MessengerEvents, + type NotNamespacedBy, +} from '@metamask/messenger'; import type { AllowedActions, @@ -9,6 +15,8 @@ import type { import { MOCK_LOGIN_RESPONSE } from '../../authentication/mocks'; import { MOCK_STORAGE_KEY_SIGNATURE } from '../mocks'; +const controllerName = 'UserStorageController'; + type GetHandler = Extract< AllowedActions, { type: ActionType } @@ -33,10 +41,22 @@ const typedMockFn = < ) => jest.fn, Parameters>(); type ExternalEvents = NotNamespacedBy< - 'UserStorageController', + typeof controllerName, AllowedEvents['type'] >; +type AllUserStorageControllerActions = + MessengerActions; + +type AllUserStorageControllerEvents = + MessengerEvents; + +type RootMessenger = Messenger< + MockAnyNamespace, + AllUserStorageControllerActions, + AllUserStorageControllerEvents +>; + /** * creates a custom user storage messenger, in case tests need different permissions * @@ -47,10 +67,21 @@ type ExternalEvents = NotNamespacedBy< export function createCustomUserStorageMessenger(props?: { overrideEvents?: ExternalEvents[]; }) { - const baseMessenger = new Messenger(); - const messenger = baseMessenger.getRestricted({ - name: 'UserStorageController', - allowedActions: [ + const rootMessenger: RootMessenger = new Messenger({ + namespace: MOCK_ANY_NAMESPACE, + }); + const messenger = new Messenger< + typeof controllerName, + AllUserStorageControllerActions, + AllUserStorageControllerEvents, + RootMessenger + >({ + namespace: controllerName, + parent: rootMessenger, + }); + rootMessenger.delegate({ + messenger, + actions: [ 'KeyringController:getState', 'SnapController:handleRequest', 'AuthenticationController:getBearerToken', @@ -58,7 +89,7 @@ export function createCustomUserStorageMessenger(props?: { 'AuthenticationController:isSignedIn', 'AuthenticationController:performSignIn', ], - allowedEvents: props?.overrideEvents ?? [ + events: props?.overrideEvents ?? [ 'KeyringController:lock', 'KeyringController:unlock', 'AddressBookController:contactUpdated', @@ -67,13 +98,13 @@ export function createCustomUserStorageMessenger(props?: { }); return { - baseMessenger, + baseMessenger: rootMessenger, messenger, }; } type OverrideMessengers = { - baseMessenger: Messenger; + baseMessenger: RootMessenger; messenger: UserStorageControllerMessenger; }; diff --git a/packages/profile-sync-controller/tsconfig.build.json b/packages/profile-sync-controller/tsconfig.build.json index ca9500d8729..df960063ed8 100644 --- a/packages/profile-sync-controller/tsconfig.build.json +++ b/packages/profile-sync-controller/tsconfig.build.json @@ -9,7 +9,8 @@ "references": [ { "path": "../base-controller/tsconfig.build.json" }, { "path": "../keyring-controller/tsconfig.build.json" }, - { "path": "../address-book-controller/tsconfig.build.json" } + { "path": "../address-book-controller/tsconfig.build.json" }, + { "path": "../messenger/tsconfig.build.json" } ], "include": ["../../types", "./src"], "exclude": [ diff --git a/packages/profile-sync-controller/tsconfig.json b/packages/profile-sync-controller/tsconfig.json index bbd45ba561c..e6966e7a7c7 100644 --- a/packages/profile-sync-controller/tsconfig.json +++ b/packages/profile-sync-controller/tsconfig.json @@ -6,7 +6,8 @@ "references": [ { "path": "../base-controller" }, { "path": "../keyring-controller" }, - { "path": "../address-book-controller" } + { "path": "../address-book-controller" }, + { "path": "../messenger" } ], "include": ["../../types", "./src"] } diff --git a/yarn.lock b/yarn.lock index 4c490b218f9..81c5809da98 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4301,6 +4301,7 @@ __metadata: "@metamask/keyring-api": "npm:^21.0.0" "@metamask/keyring-controller": "npm:^23.1.0" "@metamask/keyring-internal-api": "npm:^9.0.0" + "@metamask/messenger": "npm:^0.3.0" "@metamask/providers": "npm:^22.1.0" "@metamask/snaps-controllers": "npm:^14.0.1" "@metamask/snaps-sdk": "npm:^9.0.0" From 6b692a0f9e707a987b58dd1fa90c7788cc67c9d0 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Wed, 15 Oct 2025 16:07:41 +0200 Subject: [PATCH 2/2] fix: UserStorageController tests --- .../controllers/user-storage/UserStorageController.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.test.ts b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.test.ts index 54d70c58800..5c79d3a97e8 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.test.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.test.ts @@ -1,4 +1,4 @@ -import { deriveStateFromMetadata } from '@metamask/base-controller'; +import { deriveStateFromMetadata } from '@metamask/base-controller/next'; import type nock from 'nock'; import { mockUserStorageMessenger } from './__fixtures__/mockMessenger'; @@ -766,7 +766,7 @@ describe('metadata', () => { deriveStateFromMetadata( controller.state, controller.metadata, - 'anonymous', + 'includeInDebugSnapshot', ), ).toMatchInlineSnapshot(` Object {