From 2a3827ba6373a9df78b4dcb3773208fc5a700a04 Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Mon, 11 May 2026 11:16:09 +0200 Subject: [PATCH 01/18] feat(snap-account-service): add getLegacySnapKeyring --- packages/snap-account-service/package.json | 1 + .../SnapAccountService-method-action-types.ts | 14 ++- .../src/SnapAccountService.test.ts | 91 +++++++++++++++++++ .../src/SnapAccountService.ts | 61 ++++++++++++- yarn.lock | 1 + 5 files changed, 164 insertions(+), 4 deletions(-) diff --git a/packages/snap-account-service/package.json b/packages/snap-account-service/package.json index 419dc6bc03..cbfd140420 100644 --- a/packages/snap-account-service/package.json +++ b/packages/snap-account-service/package.json @@ -53,6 +53,7 @@ "test:watch": "NODE_OPTIONS=--experimental-vm-modules jest --watch" }, "dependencies": { + "@metamask/eth-snap-keyring": "^22.0.1", "@metamask/keyring-controller": "^25.5.0", "@metamask/messenger": "^1.2.0", "@metamask/snaps-controllers": "^19.0.0", diff --git a/packages/snap-account-service/src/SnapAccountService-method-action-types.ts b/packages/snap-account-service/src/SnapAccountService-method-action-types.ts index 1be1270a0a..d39f3cfb8b 100644 --- a/packages/snap-account-service/src/SnapAccountService-method-action-types.ts +++ b/packages/snap-account-service/src/SnapAccountService-method-action-types.ts @@ -32,9 +32,21 @@ export type SnapAccountServiceEnsureReadyAction = { handler: SnapAccountService['ensureReady']; }; +/** + * Atomically gets-or-creates the legacy (v1) Snap keyring — the keyring + * associated with {@link KeyringTypes.snap}. + * + * @returns The existing or newly-created Snap keyring instance. + */ +export type SnapAccountServiceGetLegacySnapKeyringAction = { + type: `SnapAccountService:getLegacySnapKeyring`; + handler: SnapAccountService['getLegacySnapKeyring']; +}; + /** * Union of all SnapAccountService action types. */ export type SnapAccountServiceMethodActions = | SnapAccountServiceGetSnapsAction - | SnapAccountServiceEnsureReadyAction; + | SnapAccountServiceEnsureReadyAction + | SnapAccountServiceGetLegacySnapKeyringAction; diff --git a/packages/snap-account-service/src/SnapAccountService.test.ts b/packages/snap-account-service/src/SnapAccountService.test.ts index ebf32b2473..4fb0ba78b0 100644 --- a/packages/snap-account-service/src/SnapAccountService.test.ts +++ b/packages/snap-account-service/src/SnapAccountService.test.ts @@ -1,7 +1,12 @@ +import type { SnapKeyring } from '@metamask/eth-snap-keyring'; import { KeyringControllerState, KeyringTypes, } from '@metamask/keyring-controller'; +import type { + KeyringEntry, + RestrictedController, +} from '@metamask/keyring-controller'; import { Messenger, MOCK_ANY_NAMESPACE } from '@metamask/messenger'; import type { MockAnyNamespace, @@ -75,6 +80,7 @@ function getMessenger( 'SnapController:getSnap', 'SnapController:getRunnableSnaps', 'KeyringController:getState', + 'KeyringController:withController', ], events: [ 'SnapController:stateChange', @@ -358,4 +364,89 @@ describe('SnapAccountService', () => { expect(resolved).toBe(true); }); }); + + describe('getLegacySnapKeyring', () => { + /** + * Builds a fake {@link KeyringEntry} with the given type. + * + * @param type - The keyring type. + * @returns A minimal KeyringEntry for tests. + */ + function buildKeyringEntry(type: string): KeyringEntry { + return { + keyring: { type } as KeyringEntry['keyring'], + metadata: { id: `id-${type}`, name: type }, + }; + } + + /** + * Registers a fake `KeyringController:withController` handler that calls + * the operation with a controllable {@link RestrictedController}. + * + * @param rootMessenger - The root messenger. + * @param initialEntries - Entries exposed via `controller.keyrings`. + * @returns The mocked `addNewKeyring` jest fn for assertions. + */ + function registerWithController( + rootMessenger: RootMessenger, + initialEntries: KeyringEntry[], + ): jest.MockedFunction { + const entries = [...initialEntries]; + const addNewKeyring = jest.fn(async (type: string) => { + const entry = buildKeyringEntry(type); + entries.push(entry); + return entry; + }); + rootMessenger.registerActionHandler( + 'KeyringController:withController', + async (operation) => + operation({ + get keyrings() { + return Object.freeze([...entries]); + }, + addNewKeyring, + removeKeyring: jest.fn(), + }), + ); + return addNewKeyring; + } + + it('returns the existing Snap keyring when one is already present', async () => { + const { service, rootMessenger } = setup(); + const existing = buildKeyringEntry(KeyringTypes.snap); + const addNewKeyring = registerWithController(rootMessenger, [ + buildKeyringEntry(KeyringTypes.hd), + existing, + ]); + + const result = await service.getLegacySnapKeyring(); + + expect(result).toBe(existing.keyring as unknown as SnapKeyring); + expect(addNewKeyring).not.toHaveBeenCalled(); + }); + + it('creates a new Snap keyring when none exists', async () => { + const { service, rootMessenger } = setup(); + const addNewKeyring = registerWithController(rootMessenger, [ + buildKeyringEntry(KeyringTypes.hd), + ]); + + const result = await service.getLegacySnapKeyring(); + + expect(addNewKeyring).toHaveBeenCalledWith(KeyringTypes.snap); + expect(result.type).toBe(KeyringTypes.snap); + }); + + it('propagates errors thrown by withController', async () => { + const { service, rootMessenger } = setup(); + rootMessenger.registerActionHandler( + 'KeyringController:withController', + async () => { + throw new Error('boom'); + }, + ); + + await expect(service.getLegacySnapKeyring()).rejects.toThrow('boom'); + }); + }); }); diff --git a/packages/snap-account-service/src/SnapAccountService.ts b/packages/snap-account-service/src/SnapAccountService.ts index 56b6a389e5..d9ae5483c6 100644 --- a/packages/snap-account-service/src/SnapAccountService.ts +++ b/packages/snap-account-service/src/SnapAccountService.ts @@ -1,7 +1,11 @@ +import type { SnapKeyring } from '@metamask/eth-snap-keyring'; import type { KeyringControllerGetStateAction, KeyringControllerStateChangeEvent, + KeyringControllerWithControllerAction, + KeyringEntry, } from '@metamask/keyring-controller'; +import { KeyringTypes } from '@metamask/keyring-controller'; import type { Messenger } from '@metamask/messenger'; import type { SnapControllerGetRunnableSnapsAction, @@ -19,6 +23,7 @@ import { SnapId } from '@metamask/snaps-sdk'; import type { SnapAccountServiceEnsureReadyAction, + SnapAccountServiceGetLegacySnapKeyringAction, SnapAccountServiceGetSnapsAction, } from './SnapAccountService-method-action-types'; import { SnapPlatformWatcher } from './SnapPlatformWatcher'; @@ -35,14 +40,19 @@ export const serviceName = 'SnapAccountService'; * All of the methods within {@link SnapAccountService} that are exposed via * the messenger. */ -const MESSENGER_EXPOSED_METHODS = ['ensureReady', 'getSnaps'] as const; +const MESSENGER_EXPOSED_METHODS = [ + 'ensureReady', + 'getSnaps', + 'getLegacySnapKeyring', +] as const; /** * Actions that {@link SnapAccountService} exposes to other consumers. */ export type SnapAccountServiceActions = | SnapAccountServiceEnsureReadyAction - | SnapAccountServiceGetSnapsAction; + | SnapAccountServiceGetSnapsAction + | SnapAccountServiceGetLegacySnapKeyringAction; /** * Actions from other messengers that {@link SnapAccountService} calls. @@ -51,7 +61,8 @@ type AllowedActions = | SnapControllerGetStateAction | SnapControllerGetSnapAction | SnapControllerGetRunnableSnapsAction - | KeyringControllerGetStateAction; + | KeyringControllerGetStateAction + | KeyringControllerWithControllerAction; /** * Events that {@link SnapAccountService} exposes to other consumers. @@ -173,4 +184,48 @@ export class SnapAccountService { // is ready to process requests. await this.#watcher.ensureCanUseSnapPlatform(); } + + /** + * Atomically gets-or-creates the legacy (v1) Snap keyring — the keyring + * associated with {@link KeyringTypes.snap}. + * + * @returns The existing or newly-created Snap keyring instance. + */ + async getLegacySnapKeyring(): Promise { + type Result = { + snapKeyring: SnapKeyring; + }; + + // `KeyringController:withController` forbids returning a direct keyring + // reference (it checks the result via `Object.is`), so we smuggle the + // instance out wrapped in an object and unwrap it after the call. + // NOTE: This violates the abstraction of `KeyringController:withController`, but this + // is how we currently interact with the legacy Snap keyring. Once we migrate it to + // the Snap keyring v2, we won't be using the same pattern. + const result = await this.#messenger.call( + 'KeyringController:withController', + async (controller): Promise => { + let snapKeyring: KeyringEntry['keyring'] | undefined; + + const found = controller.keyrings.find( + ({ keyring }) => keyring.type === KeyringTypes.snap, + ); + if (found) { + snapKeyring = found.keyring; + } + + if (!snapKeyring) { + const { keyring: newSnapKeyring } = await controller.addNewKeyring( + KeyringTypes.snap, + ); + snapKeyring = newSnapKeyring; + } + + // The legacy Snap keyring is not compatible with `EthKeyring`, so we need to cast here. + return { snapKeyring } as unknown as Result; + }, + ); + + return (result as Result).snapKeyring; + } } diff --git a/yarn.lock b/yarn.lock index 9ac90e0c2f..7e1d57916b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5433,6 +5433,7 @@ __metadata: resolution: "@metamask/snap-account-service@workspace:packages/snap-account-service" dependencies: "@metamask/auto-changelog": "npm:^6.1.0" + "@metamask/eth-snap-keyring": "npm:^22.0.1" "@metamask/keyring-controller": "npm:^25.5.0" "@metamask/messenger": "npm:^1.2.0" "@metamask/snaps-controllers": "npm:^19.0.0" From 8ab376723f6b981be2e3abb45e0f374d28e0c08c Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Mon, 11 May 2026 11:20:45 +0200 Subject: [PATCH 02/18] chore: changelog --- packages/snap-account-service/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/snap-account-service/CHANGELOG.md b/packages/snap-account-service/CHANGELOG.md index 363e6fcbf2..f6eb4e7e14 100644 --- a/packages/snap-account-service/CHANGELOG.md +++ b/packages/snap-account-service/CHANGELOG.md @@ -23,6 +23,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Export `SnapAccountServiceGetSnapsAction` type. - The service now seeds its internal set from `SnapController:getRunnableSnaps` during `init()` and keeps it in sync via `SnapController` lifecycle events (`snapInstalled`, `snapEnabled`, `snapDisabled`, `snapBlocked`, `snapUninstalled`). - The service messenger now requires the `SnapController:getRunnableSnaps` action and the five lifecycle events listed above. +- Add `getLegacySnapKeyring` ([#8757](https://github.com/MetaMask/core/pull/8757)) + - This is a concurrent-safe variant of the existing `getSnapKeyring` function that exist on clients. + - The service messenger now requires the `KeyringController:withController` action. ### Changed From f222f71c6bc6befc00dc70eda0e87e95f0b1f692 Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Mon, 11 May 2026 11:42:40 +0200 Subject: [PATCH 03/18] chore: add logs --- .../snap-account-service/src/SnapAccountService.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/snap-account-service/src/SnapAccountService.ts b/packages/snap-account-service/src/SnapAccountService.ts index d9ae5483c6..8b221698ad 100644 --- a/packages/snap-account-service/src/SnapAccountService.ts +++ b/packages/snap-account-service/src/SnapAccountService.ts @@ -21,6 +21,7 @@ import type { } from '@metamask/snaps-controllers'; import { SnapId } from '@metamask/snaps-sdk'; +import { projectLogger as log } from './logger'; import type { SnapAccountServiceEnsureReadyAction, SnapAccountServiceGetLegacySnapKeyringAction, @@ -215,10 +216,13 @@ export class SnapAccountService { } if (!snapKeyring) { - const { keyring: newSnapKeyring } = await controller.addNewKeyring( - KeyringTypes.snap, - ); + const { + keyring: newSnapKeyring, + metadata: { id }, + } = await controller.addNewKeyring(KeyringTypes.snap); snapKeyring = newSnapKeyring; + + log(`Legacy Snap keyring created. ("${id}")`); } // The legacy Snap keyring is not compatible with `EthKeyring`, so we need to cast here. From 298ac2d0abe985e3d02466b0478f44cc08108f9d Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Mon, 11 May 2026 11:49:41 +0200 Subject: [PATCH 04/18] refactor: add LegacySnapKeyring type + helper --- packages/snap-account-service/package.json | 1 + .../src/SnapAccountService.ts | 23 +++++++++++++++---- yarn.lock | 1 + 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/packages/snap-account-service/package.json b/packages/snap-account-service/package.json index cbfd140420..cc1e974237 100644 --- a/packages/snap-account-service/package.json +++ b/packages/snap-account-service/package.json @@ -63,6 +63,7 @@ }, "devDependencies": { "@metamask/auto-changelog": "^6.1.0", + "@metamask/keyring-utils": "^3.2.1", "@ts-bridge/cli": "^0.6.4", "@types/jest": "^29.5.14", "deepmerge": "^4.2.2", diff --git a/packages/snap-account-service/src/SnapAccountService.ts b/packages/snap-account-service/src/SnapAccountService.ts index 8b221698ad..67f6c3ba13 100644 --- a/packages/snap-account-service/src/SnapAccountService.ts +++ b/packages/snap-account-service/src/SnapAccountService.ts @@ -1,4 +1,4 @@ -import type { SnapKeyring } from '@metamask/eth-snap-keyring'; +import type { SnapKeyring as LegacySnapKeyring } from '@metamask/eth-snap-keyring'; import type { KeyringControllerGetStateAction, KeyringControllerStateChangeEvent, @@ -6,6 +6,7 @@ import type { KeyringEntry, } from '@metamask/keyring-controller'; import { KeyringTypes } from '@metamask/keyring-controller'; +import type { EthKeyring } from '@metamask/keyring-utils'; import type { Messenger } from '@metamask/messenger'; import type { SnapControllerGetRunnableSnapsAction, @@ -108,6 +109,18 @@ export type SnapAccountServiceOptions = { config?: SnapAccountServiceConfig; }; +/** + * Checks if a given keyring is a Snap keyring (v2). + * + * @param keyring - The keyring to check. + * @returns `true` if the keyring is a Snap keyring (v2), `false` otherwise. + */ +function isLegacySnapKeyring( + keyring: EthKeyring, +): keyring is LegacySnapKeyring { + return keyring.type === KeyringTypes.snap; +} + /** * Service responsible for managing account management snaps. */ @@ -192,9 +205,9 @@ export class SnapAccountService { * * @returns The existing or newly-created Snap keyring instance. */ - async getLegacySnapKeyring(): Promise { + async getLegacySnapKeyring(): Promise { type Result = { - snapKeyring: SnapKeyring; + snapKeyring: LegacySnapKeyring; }; // `KeyringController:withController` forbids returning a direct keyring @@ -208,8 +221,8 @@ export class SnapAccountService { async (controller): Promise => { let snapKeyring: KeyringEntry['keyring'] | undefined; - const found = controller.keyrings.find( - ({ keyring }) => keyring.type === KeyringTypes.snap, + const found = controller.keyrings.find(({ keyring }) => + isLegacySnapKeyring(keyring), ); if (found) { snapKeyring = found.keyring; diff --git a/yarn.lock b/yarn.lock index 7e1d57916b..c8033b704e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5435,6 +5435,7 @@ __metadata: "@metamask/auto-changelog": "npm:^6.1.0" "@metamask/eth-snap-keyring": "npm:^22.0.1" "@metamask/keyring-controller": "npm:^25.5.0" + "@metamask/keyring-utils": "npm:^3.2.1" "@metamask/messenger": "npm:^1.2.0" "@metamask/snaps-controllers": "npm:^19.0.0" "@metamask/snaps-sdk": "npm:^11.0.0" From a32593047f3958d04da02125426d2c9153a1e7e9 Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Mon, 11 May 2026 12:00:02 +0200 Subject: [PATCH 05/18] fix: fix helper --- packages/snap-account-service/src/SnapAccountService.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/snap-account-service/src/SnapAccountService.ts b/packages/snap-account-service/src/SnapAccountService.ts index 67f6c3ba13..ba9f073d8a 100644 --- a/packages/snap-account-service/src/SnapAccountService.ts +++ b/packages/snap-account-service/src/SnapAccountService.ts @@ -6,7 +6,7 @@ import type { KeyringEntry, } from '@metamask/keyring-controller'; import { KeyringTypes } from '@metamask/keyring-controller'; -import type { EthKeyring } from '@metamask/keyring-utils'; +import type { BaseKeyring } from '@metamask/keyring-utils'; import type { Messenger } from '@metamask/messenger'; import type { SnapControllerGetRunnableSnapsAction, @@ -113,11 +113,12 @@ export type SnapAccountServiceOptions = { * Checks if a given keyring is a Snap keyring (v2). * * @param keyring - The keyring to check. + * @param keyring.type - The type of the keyring. * @returns `true` if the keyring is a Snap keyring (v2), `false` otherwise. */ -function isLegacySnapKeyring( - keyring: EthKeyring, -): keyring is LegacySnapKeyring { +function isLegacySnapKeyring(keyring: { + type: BaseKeyring['type']; +}): keyring is LegacySnapKeyring { return keyring.type === KeyringTypes.snap; } From 5255d4306a6460d234dedf646f26a4548097e4c1 Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Mon, 11 May 2026 12:29:56 +0200 Subject: [PATCH 06/18] test: refactor --- .../src/SnapAccountService.test.ts | 44 ++++++++++--------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/packages/snap-account-service/src/SnapAccountService.test.ts b/packages/snap-account-service/src/SnapAccountService.test.ts index 4fb0ba78b0..f9005fd49d 100644 --- a/packages/snap-account-service/src/SnapAccountService.test.ts +++ b/packages/snap-account-service/src/SnapAccountService.test.ts @@ -47,6 +47,7 @@ type Mocks = { // eslint-disable-next-line @typescript-eslint/naming-convention KeyringController: { getState: jest.MockedFunction<() => { keyrings: { type: string }[] }>; + withController: jest.Mock; }; }; @@ -184,6 +185,7 @@ function setup({ }, KeyringController: { getState: jest.fn().mockReturnValue({ keyrings }), + withController: jest.fn(), }, }; @@ -199,6 +201,10 @@ function setup({ 'KeyringController:getState', mocks.KeyringController.getState, ); + rootMessenger.registerActionHandler( + 'KeyringController:withController', + mocks.KeyringController.withController, + ); const service = new SnapAccountService({ messenger, config }); @@ -380,25 +386,26 @@ describe('SnapAccountService', () => { } /** - * Registers a fake `KeyringController:withController` handler that calls - * the operation with a controllable {@link RestrictedController}. + * Configures `mocks.KeyringController.withController` to invoke the + * operation with a controllable {@link RestrictedController}. * - * @param rootMessenger - The root messenger. + * @param mocks - The mocks object from {@link setup}. * @param initialEntries - Entries exposed via `controller.keyrings`. * @returns The mocked `addNewKeyring` jest fn for assertions. */ - function registerWithController( - rootMessenger: RootMessenger, + function mockWithController( + mocks: Mocks, initialEntries: KeyringEntry[], - ): jest.MockedFunction { + ): { + addNewKeyring: jest.MockedFunction; + } { const entries = [...initialEntries]; const addNewKeyring = jest.fn(async (type: string) => { const entry = buildKeyringEntry(type); entries.push(entry); return entry; }); - rootMessenger.registerActionHandler( - 'KeyringController:withController', + mocks.KeyringController.withController.mockImplementation( async (operation) => operation({ get keyrings() { @@ -408,13 +415,13 @@ describe('SnapAccountService', () => { removeKeyring: jest.fn(), }), ); - return addNewKeyring; + return { addNewKeyring }; } it('returns the existing Snap keyring when one is already present', async () => { - const { service, rootMessenger } = setup(); + const { service, mocks } = setup(); const existing = buildKeyringEntry(KeyringTypes.snap); - const addNewKeyring = registerWithController(rootMessenger, [ + const { addNewKeyring } = mockWithController(mocks, [ buildKeyringEntry(KeyringTypes.hd), existing, ]); @@ -426,8 +433,8 @@ describe('SnapAccountService', () => { }); it('creates a new Snap keyring when none exists', async () => { - const { service, rootMessenger } = setup(); - const addNewKeyring = registerWithController(rootMessenger, [ + const { service, mocks } = setup(); + const { addNewKeyring } = mockWithController(mocks, [ buildKeyringEntry(KeyringTypes.hd), ]); @@ -438,13 +445,10 @@ describe('SnapAccountService', () => { }); it('propagates errors thrown by withController', async () => { - const { service, rootMessenger } = setup(); - rootMessenger.registerActionHandler( - 'KeyringController:withController', - async () => { - throw new Error('boom'); - }, - ); + const { service, mocks } = setup(); + mocks.KeyringController.withController.mockImplementation(async () => { + throw new Error('boom'); + }); await expect(service.getLegacySnapKeyring()).rejects.toThrow('boom'); }); From 74136a7d7309bf740d20633f4da6aaf0a32cda13 Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Mon, 11 May 2026 12:32:46 +0200 Subject: [PATCH 07/18] test: refactor --- .../src/SnapAccountService.test.ts | 91 +++++++++---------- 1 file changed, 45 insertions(+), 46 deletions(-) diff --git a/packages/snap-account-service/src/SnapAccountService.test.ts b/packages/snap-account-service/src/SnapAccountService.test.ts index f9005fd49d..e1addaa540 100644 --- a/packages/snap-account-service/src/SnapAccountService.test.ts +++ b/packages/snap-account-service/src/SnapAccountService.test.ts @@ -147,6 +147,51 @@ function buildSnap(id: string, hasKeyring: boolean): TruncatedSnap { } as MockTruncatedSnap as TruncatedSnap; } +/** + * Builds a fake {@link KeyringEntry} with the given type. + * + * @param type - The keyring type. + * @returns A minimal KeyringEntry for tests. + */ +function buildKeyringEntry(type: string): KeyringEntry { + return { + keyring: { type } as KeyringEntry['keyring'], + metadata: { id: `id-${type}`, name: type }, + }; +} + +/** + * Configures `mocks.KeyringController.withController` to invoke the + * operation with a controllable {@link RestrictedController}. + * + * @param mocks - The mocks object from {@link setup}. + * @param initialEntries - Entries exposed via `controller.keyrings`. + * @returns The mocked `addNewKeyring` jest fn for assertions. + */ +function mockWithController( + mocks: Mocks, + initialEntries: KeyringEntry[], +): { + addNewKeyring: jest.MockedFunction; +} { + const entries = [...initialEntries]; + const addNewKeyring = jest.fn(async (type: string) => { + const entry = buildKeyringEntry(type); + entries.push(entry); + return entry; + }); + mocks.KeyringController.withController.mockImplementation(async (operation) => + operation({ + get keyrings() { + return Object.freeze([...entries]); + }, + addNewKeyring, + removeKeyring: jest.fn(), + }), + ); + return { addNewKeyring }; +} + /** * Constructs the service under test with sensible defaults. * @@ -372,52 +417,6 @@ describe('SnapAccountService', () => { }); describe('getLegacySnapKeyring', () => { - /** - * Builds a fake {@link KeyringEntry} with the given type. - * - * @param type - The keyring type. - * @returns A minimal KeyringEntry for tests. - */ - function buildKeyringEntry(type: string): KeyringEntry { - return { - keyring: { type } as KeyringEntry['keyring'], - metadata: { id: `id-${type}`, name: type }, - }; - } - - /** - * Configures `mocks.KeyringController.withController` to invoke the - * operation with a controllable {@link RestrictedController}. - * - * @param mocks - The mocks object from {@link setup}. - * @param initialEntries - Entries exposed via `controller.keyrings`. - * @returns The mocked `addNewKeyring` jest fn for assertions. - */ - function mockWithController( - mocks: Mocks, - initialEntries: KeyringEntry[], - ): { - addNewKeyring: jest.MockedFunction; - } { - const entries = [...initialEntries]; - const addNewKeyring = jest.fn(async (type: string) => { - const entry = buildKeyringEntry(type); - entries.push(entry); - return entry; - }); - mocks.KeyringController.withController.mockImplementation( - async (operation) => - operation({ - get keyrings() { - return Object.freeze([...entries]); - }, - addNewKeyring, - removeKeyring: jest.fn(), - }), - ); - return { addNewKeyring }; - } - it('returns the existing Snap keyring when one is already present', async () => { const { service, mocks } = setup(); const existing = buildKeyringEntry(KeyringTypes.snap); From 00a26d29fd14dc903a7011f74e0922063912f12b Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Mon, 11 May 2026 12:46:06 +0200 Subject: [PATCH 08/18] feat(snap-account-service): add handleKeyringSnapMessage --- .../SnapAccountService-method-action-types.ts | 15 ++- .../src/SnapAccountService.test.ts | 99 ++++++++++++++++++- .../src/SnapAccountService.ts | 26 ++++- 3 files changed, 136 insertions(+), 4 deletions(-) diff --git a/packages/snap-account-service/src/SnapAccountService-method-action-types.ts b/packages/snap-account-service/src/SnapAccountService-method-action-types.ts index d39f3cfb8b..15af5c939f 100644 --- a/packages/snap-account-service/src/SnapAccountService-method-action-types.ts +++ b/packages/snap-account-service/src/SnapAccountService-method-action-types.ts @@ -43,10 +43,23 @@ export type SnapAccountServiceGetLegacySnapKeyringAction = { handler: SnapAccountService['getLegacySnapKeyring']; }; +/** + * Handle a message from a Snap. + * + * @param snapId - ID of the Snap. + * @param message - Message sent by the Snap. + * @returns The execution result. + */ +export type SnapAccountServiceHandleKeyringSnapMessageAction = { + type: `SnapAccountService:handleKeyringSnapMessage`; + handler: SnapAccountService['handleKeyringSnapMessage']; +}; + /** * Union of all SnapAccountService action types. */ export type SnapAccountServiceMethodActions = | SnapAccountServiceGetSnapsAction | SnapAccountServiceEnsureReadyAction - | SnapAccountServiceGetLegacySnapKeyringAction; + | SnapAccountServiceGetLegacySnapKeyringAction + | SnapAccountServiceHandleKeyringSnapMessageAction; diff --git a/packages/snap-account-service/src/SnapAccountService.test.ts b/packages/snap-account-service/src/SnapAccountService.test.ts index e1addaa540..4b85f2a863 100644 --- a/packages/snap-account-service/src/SnapAccountService.test.ts +++ b/packages/snap-account-service/src/SnapAccountService.test.ts @@ -1,4 +1,4 @@ -import type { SnapKeyring } from '@metamask/eth-snap-keyring'; +import type { SnapKeyring, SnapMessage } from '@metamask/eth-snap-keyring'; import { KeyringControllerState, KeyringTypes, @@ -192,6 +192,44 @@ function mockWithController( return { addNewKeyring }; } +/** + * Configures `mocks.KeyringController.withController` to expose a single + * legacy Snap keyring with the provided mocked methods. + * + * @param mocks - The mocks object from {@link setup}. + * @param keyring - The mocked Snap keyring methods. + * @param keyring.handleKeyringSnapMessage - The mocked implementation. + */ +function mockLegacySnapKeyring( + mocks: Mocks, + { + handleKeyringSnapMessage, + }: { + handleKeyringSnapMessage: jest.MockedFunction< + SnapKeyring['handleKeyringSnapMessage'] + >; + }, +): void { + const snapKeyring = { + type: KeyringTypes.snap, + handleKeyringSnapMessage, + }; + mocks.KeyringController.withController.mockImplementation(async (operation) => + operation({ + get keyrings() { + return Object.freeze([ + { + keyring: snapKeyring as unknown as KeyringEntry['keyring'], + metadata: { id: 'id-snap', name: KeyringTypes.snap }, + }, + ]); + }, + addNewKeyring: jest.fn(), + removeKeyring: jest.fn(), + }), + ); +} + /** * Constructs the service under test with sensible defaults. * @@ -452,4 +490,63 @@ describe('SnapAccountService', () => { await expect(service.getLegacySnapKeyring()).rejects.toThrow('boom'); }); }); + + describe('handleKeyringSnapMessage', () => { + const MOCK_MESSAGE = { + method: 'keyring_listAccounts', + params: {}, + } as unknown as SnapMessage; + + it('forwards the call to the legacy Snap keyring and returns its result', async () => { + const { service, mocks } = setup(); + const handleKeyringSnapMessage = jest + .fn() + .mockResolvedValue({ ok: true }); + mockLegacySnapKeyring(mocks, { handleKeyringSnapMessage }); + + const result = await service.handleKeyringSnapMessage( + MOCK_SNAP_ID, + MOCK_MESSAGE, + ); + + expect(handleKeyringSnapMessage).toHaveBeenCalledWith( + MOCK_SNAP_ID, + MOCK_MESSAGE, + ); + expect(result).toStrictEqual({ ok: true }); + }); + + it('propagates errors thrown by the Snap keyring', async () => { + const { service, mocks } = setup(); + const error = new Error('snap boom'); + const handleKeyringSnapMessage = jest.fn().mockRejectedValue(error); + mockLegacySnapKeyring(mocks, { handleKeyringSnapMessage }); + + await expect( + service.handleKeyringSnapMessage(MOCK_SNAP_ID, MOCK_MESSAGE), + ).rejects.toThrow(error); + }); + + it('is exposed as a messenger action', async () => { + const { service, mocks, messenger } = setup(); + const handleKeyringSnapMessage = jest.fn().mockResolvedValue('pong'); + mockLegacySnapKeyring(mocks, { handleKeyringSnapMessage }); + + // Reference `service` so it isn't flagged as unused; constructing it + // registers the messenger action under test. + expect(service).toBeDefined(); + + const result = await messenger.call( + 'SnapAccountService:handleKeyringSnapMessage', + MOCK_SNAP_ID, + MOCK_MESSAGE, + ); + + expect(handleKeyringSnapMessage).toHaveBeenCalledWith( + MOCK_SNAP_ID, + MOCK_MESSAGE, + ); + expect(result).toBe('pong'); + }); + }); }); diff --git a/packages/snap-account-service/src/SnapAccountService.ts b/packages/snap-account-service/src/SnapAccountService.ts index ba9f073d8a..6f67c358c1 100644 --- a/packages/snap-account-service/src/SnapAccountService.ts +++ b/packages/snap-account-service/src/SnapAccountService.ts @@ -1,4 +1,7 @@ -import type { SnapKeyring as LegacySnapKeyring } from '@metamask/eth-snap-keyring'; +import type { + SnapKeyring as LegacySnapKeyring, + SnapMessage, +} from '@metamask/eth-snap-keyring'; import type { KeyringControllerGetStateAction, KeyringControllerStateChangeEvent, @@ -21,12 +24,14 @@ import type { SnapControllerStateChangeEvent, } from '@metamask/snaps-controllers'; import { SnapId } from '@metamask/snaps-sdk'; +import type { Json } from '@metamask/utils'; import { projectLogger as log } from './logger'; import type { SnapAccountServiceEnsureReadyAction, SnapAccountServiceGetLegacySnapKeyringAction, SnapAccountServiceGetSnapsAction, + SnapAccountServiceHandleKeyringSnapMessageAction, } from './SnapAccountService-method-action-types'; import { SnapPlatformWatcher } from './SnapPlatformWatcher'; import type { SnapPlatformWatcherConfig } from './SnapPlatformWatcher'; @@ -46,6 +51,7 @@ const MESSENGER_EXPOSED_METHODS = [ 'ensureReady', 'getSnaps', 'getLegacySnapKeyring', + 'handleKeyringSnapMessage', ] as const; /** @@ -54,7 +60,8 @@ const MESSENGER_EXPOSED_METHODS = [ export type SnapAccountServiceActions = | SnapAccountServiceEnsureReadyAction | SnapAccountServiceGetSnapsAction - | SnapAccountServiceGetLegacySnapKeyringAction; + | SnapAccountServiceGetLegacySnapKeyringAction + | SnapAccountServiceHandleKeyringSnapMessageAction; /** * Actions from other messengers that {@link SnapAccountService} calls. @@ -246,4 +253,19 @@ export class SnapAccountService { return (result as Result).snapKeyring; } + + /** + * Handle a message from a Snap. + * + * @param snapId - ID of the Snap. + * @param message - Message sent by the Snap. + * @returns The execution result. + */ + async handleKeyringSnapMessage( + snapId: SnapId, + message: SnapMessage, + ): Promise { + const snapKeyring = await this.getLegacySnapKeyring(); + return snapKeyring.handleKeyringSnapMessage(snapId, message); + } } From 1b5f6ba156c6fb2c53b0c261aa90d5d543b60ebc Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Mon, 11 May 2026 13:49:17 +0200 Subject: [PATCH 09/18] chore: changelog --- packages/snap-account-service/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/snap-account-service/CHANGELOG.md b/packages/snap-account-service/CHANGELOG.md index f6eb4e7e14..6dbce3c074 100644 --- a/packages/snap-account-service/CHANGELOG.md +++ b/packages/snap-account-service/CHANGELOG.md @@ -26,6 +26,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `getLegacySnapKeyring` ([#8757](https://github.com/MetaMask/core/pull/8757)) - This is a concurrent-safe variant of the existing `getSnapKeyring` function that exist on clients. - The service messenger now requires the `KeyringController:withController` action. +- Add `handleKeyringSnapMessage` ([#8758](https://github.com/MetaMask/core/pull/8758)) + - This will be the new entry point for consumer that needs to forward keyring events to a account management Snap (instead of using the legacy Snap keyring instance directly). ### Changed From ee06aff24655dca6f93fb55f798c3c04af91a8fe Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Mon, 11 May 2026 14:57:22 +0200 Subject: [PATCH 10/18] feat(snap-account-service): handle :selectedAccountGroupChange event + forward to Snap keyring --- packages/snap-account-service/package.json | 2 + .../src/SnapAccountService.test.ts | 149 +++++++++++++++++- .../src/SnapAccountService.ts | 44 +++++- .../snap-account-service/tsconfig.build.json | 1 + packages/snap-account-service/tsconfig.json | 1 + yarn.lock | 2 + 6 files changed, 196 insertions(+), 3 deletions(-) diff --git a/packages/snap-account-service/package.json b/packages/snap-account-service/package.json index cc1e974237..a566d7e575 100644 --- a/packages/snap-account-service/package.json +++ b/packages/snap-account-service/package.json @@ -53,6 +53,8 @@ "test:watch": "NODE_OPTIONS=--experimental-vm-modules jest --watch" }, "dependencies": { + "@metamask/account-api": "^1.0.4", + "@metamask/account-tree-controller": "^7.3.0", "@metamask/eth-snap-keyring": "^22.0.1", "@metamask/keyring-controller": "^25.5.0", "@metamask/messenger": "^1.2.0", diff --git a/packages/snap-account-service/src/SnapAccountService.test.ts b/packages/snap-account-service/src/SnapAccountService.test.ts index 4b85f2a863..d3c118597e 100644 --- a/packages/snap-account-service/src/SnapAccountService.test.ts +++ b/packages/snap-account-service/src/SnapAccountService.test.ts @@ -1,3 +1,5 @@ +import type { AccountGroupId } from '@metamask/account-api'; +import type { AccountGroupObject } from '@metamask/account-tree-controller'; import type { SnapKeyring, SnapMessage } from '@metamask/eth-snap-keyring'; import { KeyringControllerState, @@ -49,6 +51,12 @@ type Mocks = { getState: jest.MockedFunction<() => { keyrings: { type: string }[] }>; withController: jest.Mock; }; + // eslint-disable-next-line @typescript-eslint/naming-convention + AccountTreeController: { + getAccountGroupObject: jest.MockedFunction< + (groupId: AccountGroupId) => AccountGroupObject | undefined + >; + }; }; /** @@ -82,6 +90,7 @@ function getMessenger( 'SnapController:getRunnableSnaps', 'KeyringController:getState', 'KeyringController:withController', + 'AccountTreeController:getAccountGroupObject', ], events: [ 'SnapController:stateChange', @@ -92,6 +101,7 @@ function getMessenger( 'SnapController:snapUnblocked', 'SnapController:snapUninstalled', 'KeyringController:stateChange', + 'AccountTreeController:selectedAccountGroupChange', ], }); return messenger; @@ -131,6 +141,36 @@ function publishKeyrings( ); } +/** + * Flushes pending microtasks so that chained `await`s in fire-and-forget + * handlers resolve before assertions run. + * + * @returns A promise that resolves once the event loop has drained. + */ +async function flushMicrotasks(): Promise { + await new Promise((resolve) => setImmediate(resolve)); +} + +/** + * Publishes an AccountTreeController selectedAccountGroupChange event on the + * root messenger. + * + * @param rootMessenger - The root messenger. + * @param next - The newly selected account group ID (or ''). + * @param previous - The previously selected account group ID (or ''). + */ +function publishSelectedAccountGroupChange( + rootMessenger: RootMessenger, + next: AccountGroupId | '', + previous: AccountGroupId | '' = '', +): void { + rootMessenger.publish( + 'AccountTreeController:selectedAccountGroupChange', + next, + previous, + ); +} + /** * Builds a minimal `TruncatedSnap` for tests. * @@ -199,20 +239,26 @@ function mockWithController( * @param mocks - The mocks object from {@link setup}. * @param keyring - The mocked Snap keyring methods. * @param keyring.handleKeyringSnapMessage - The mocked implementation. + * @param keyring.setSelectedAccounts - The mocked implementation. */ function mockLegacySnapKeyring( mocks: Mocks, { handleKeyringSnapMessage, + setSelectedAccounts, }: { - handleKeyringSnapMessage: jest.MockedFunction< + handleKeyringSnapMessage?: jest.MockedFunction< SnapKeyring['handleKeyringSnapMessage'] >; + setSelectedAccounts?: jest.MockedFunction< + SnapKeyring['setSelectedAccounts'] + >; }, ): void { const snapKeyring = { type: KeyringTypes.snap, handleKeyringSnapMessage, + setSelectedAccounts, }; mocks.KeyringController.withController.mockImplementation(async (operation) => operation({ @@ -270,6 +316,9 @@ function setup({ getState: jest.fn().mockReturnValue({ keyrings }), withController: jest.fn(), }, + AccountTreeController: { + getAccountGroupObject: jest.fn().mockReturnValue(undefined), + }, }; rootMessenger.registerActionHandler( @@ -288,6 +337,10 @@ function setup({ 'KeyringController:withController', mocks.KeyringController.withController, ); + rootMessenger.registerActionHandler( + 'AccountTreeController:getAccountGroupObject', + mocks.AccountTreeController.getAccountGroupObject, + ); const service = new SnapAccountService({ messenger, config }); @@ -549,4 +602,98 @@ describe('SnapAccountService', () => { expect(result).toBe('pong'); }); }); + + describe('on AccountTreeController:selectedAccountGroupChange', () => { + const MOCK_GROUP_ID = 'keyring:01JABC/group-1' as AccountGroupId; + const MOCK_ACCOUNTS = [ + '00000000-0000-0000-0000-000000000001', + '00000000-0000-0000-0000-000000000002', + ]; + + /** + * Builds a minimal `AccountGroupObject` for tests. + * + * @param accounts - The list of account IDs in the group. + * @returns A minimal `AccountGroupObject`. + */ + function buildGroup(accounts: string[]): AccountGroupObject { + return { accounts } as unknown as AccountGroupObject; + } + + it('forwards the selected accounts to the Snap keyring', async () => { + const { service, rootMessenger, mocks } = setup(); + const setSelectedAccounts = jest.fn().mockResolvedValue(undefined); + mockLegacySnapKeyring(mocks, { setSelectedAccounts }); + mocks.AccountTreeController.getAccountGroupObject.mockReturnValue( + buildGroup(MOCK_ACCOUNTS), + ); + expect(service).toBeDefined(); + + publishSelectedAccountGroupChange(rootMessenger, MOCK_GROUP_ID); + await flushMicrotasks(); + + expect( + mocks.AccountTreeController.getAccountGroupObject, + ).toHaveBeenCalledWith(MOCK_GROUP_ID); + expect(setSelectedAccounts).toHaveBeenCalledWith(MOCK_ACCOUNTS); + }); + + it('does nothing when the new group ID is empty', async () => { + const { service, rootMessenger, mocks } = setup(); + const setSelectedAccounts = jest.fn().mockResolvedValue(undefined); + mockLegacySnapKeyring(mocks, { setSelectedAccounts }); + expect(service).toBeDefined(); + + publishSelectedAccountGroupChange(rootMessenger, ''); + await flushMicrotasks(); + + expect( + mocks.AccountTreeController.getAccountGroupObject, + ).not.toHaveBeenCalled(); + expect(setSelectedAccounts).not.toHaveBeenCalled(); + }); + + it('does nothing when the account group is not found', async () => { + const { service, rootMessenger, mocks } = setup(); + const setSelectedAccounts = jest.fn().mockResolvedValue(undefined); + mockLegacySnapKeyring(mocks, { setSelectedAccounts }); + mocks.AccountTreeController.getAccountGroupObject.mockReturnValue( + undefined, + ); + expect(service).toBeDefined(); + + publishSelectedAccountGroupChange(rootMessenger, MOCK_GROUP_ID); + await flushMicrotasks(); + + expect( + mocks.AccountTreeController.getAccountGroupObject, + ).toHaveBeenCalledWith(MOCK_GROUP_ID); + expect(setSelectedAccounts).not.toHaveBeenCalled(); + }); + + it('logs an error when forwarding to the Snap keyring fails', async () => { + const { service, rootMessenger, mocks } = setup(); + const error = new Error('forward boom'); + const setSelectedAccounts = jest.fn().mockRejectedValue(error); + mockLegacySnapKeyring(mocks, { setSelectedAccounts }); + mocks.AccountTreeController.getAccountGroupObject.mockReturnValue( + buildGroup(MOCK_ACCOUNTS), + ); + const consoleErrorSpy = jest + .spyOn(console, 'error') + .mockImplementation(() => undefined); + expect(service).toBeDefined(); + + publishSelectedAccountGroupChange(rootMessenger, MOCK_GROUP_ID); + await flushMicrotasks(); + + expect(setSelectedAccounts).toHaveBeenCalledWith(MOCK_ACCOUNTS); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Error handling selected account group change:', + error, + ); + + consoleErrorSpy.mockRestore(); + }); + }); }); diff --git a/packages/snap-account-service/src/SnapAccountService.ts b/packages/snap-account-service/src/SnapAccountService.ts index 6f67c358c1..39ec9db017 100644 --- a/packages/snap-account-service/src/SnapAccountService.ts +++ b/packages/snap-account-service/src/SnapAccountService.ts @@ -1,3 +1,6 @@ +import { AccountGroupId } from '@metamask/account-api'; +import { AccountTreeControllerSelectedAccountGroupChangeEvent } from '@metamask/account-tree-controller'; +import { AccountTreeControllerGetAccountGroupObjectAction } from '@metamask/account-tree-controller'; import type { SnapKeyring as LegacySnapKeyring, SnapMessage, @@ -71,7 +74,8 @@ type AllowedActions = | SnapControllerGetSnapAction | SnapControllerGetRunnableSnapsAction | KeyringControllerGetStateAction - | KeyringControllerWithControllerAction; + | KeyringControllerWithControllerAction + | AccountTreeControllerGetAccountGroupObjectAction; /** * Events that {@link SnapAccountService} exposes to other consumers. @@ -89,7 +93,8 @@ type AllowedEvents = | SnapControllerSnapBlockedEvent | SnapControllerSnapUnblockedEvent | SnapControllerSnapUninstalledEvent - | KeyringControllerStateChangeEvent; + | KeyringControllerStateChangeEvent + | AccountTreeControllerSelectedAccountGroupChangeEvent; /** * The messenger which is restricted to actions and events accessed by @@ -164,6 +169,15 @@ export class SnapAccountService { this, MESSENGER_EXPOSED_METHODS, ); + + this.#messenger.subscribe( + 'AccountTreeController:selectedAccountGroupChange', + (groupId) => { + this.#handleSelectedAccountGroupChange(groupId).catch((error) => { + console.error('Error handling selected account group change:', error); + }); + }, + ); } /** @@ -268,4 +282,30 @@ export class SnapAccountService { const snapKeyring = await this.getLegacySnapKeyring(); return snapKeyring.handleKeyringSnapMessage(snapId, message); } + + /** + * Handles changes to the selected account group. Forwards accounts from the selected account group object + * to te Snap keyring. + * + * @param groupId - The ID of the newly selected account group. + */ + async #handleSelectedAccountGroupChange( + groupId: AccountGroupId | '', + ): Promise { + if (groupId) { + const group = this.#messenger.call( + 'AccountTreeController:getAccountGroupObject', + groupId, + ); + + if (group) { + log( + `Forwarding selected accounts (from "${groupId}") to Snap keyring: ${group.accounts.join(', ')}`, + ); + + const snapKeyring = await this.getLegacySnapKeyring(); + await snapKeyring.setSelectedAccounts(group.accounts); + } + } + } } diff --git a/packages/snap-account-service/tsconfig.build.json b/packages/snap-account-service/tsconfig.build.json index 59c9b7a1df..17c397c85e 100644 --- a/packages/snap-account-service/tsconfig.build.json +++ b/packages/snap-account-service/tsconfig.build.json @@ -6,6 +6,7 @@ "rootDir": "./src" }, "references": [ + { "path": "../account-tree-controller/tsconfig.build.json" }, { "path": "../keyring-controller/tsconfig.build.json" }, { "path": "../messenger/tsconfig.build.json" } ], diff --git a/packages/snap-account-service/tsconfig.json b/packages/snap-account-service/tsconfig.json index a0556a87d1..b5367cb8ec 100644 --- a/packages/snap-account-service/tsconfig.json +++ b/packages/snap-account-service/tsconfig.json @@ -4,6 +4,7 @@ "baseUrl": "./" }, "references": [ + { "path": "../account-tree-controller" }, { "path": "../keyring-controller" }, { "path": "../messenger" } ], diff --git a/yarn.lock b/yarn.lock index c8033b704e..1b5964bd25 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5432,6 +5432,8 @@ __metadata: version: 0.0.0-use.local resolution: "@metamask/snap-account-service@workspace:packages/snap-account-service" dependencies: + "@metamask/account-api": "npm:^1.0.4" + "@metamask/account-tree-controller": "npm:^7.3.0" "@metamask/auto-changelog": "npm:^6.1.0" "@metamask/eth-snap-keyring": "npm:^22.0.1" "@metamask/keyring-controller": "npm:^25.5.0" From 017c5f55f4a0a9a17976df61a07ae497fc37943a Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Mon, 11 May 2026 15:10:13 +0200 Subject: [PATCH 11/18] feat(snap-account-service): also handle :unlock --- .../src/SnapAccountService.test.ts | 113 ++++++++++++++++-- .../src/SnapAccountService.ts | 58 +++++++-- 2 files changed, 149 insertions(+), 22 deletions(-) diff --git a/packages/snap-account-service/src/SnapAccountService.test.ts b/packages/snap-account-service/src/SnapAccountService.test.ts index d3c118597e..27affa6798 100644 --- a/packages/snap-account-service/src/SnapAccountService.test.ts +++ b/packages/snap-account-service/src/SnapAccountService.test.ts @@ -56,6 +56,7 @@ type Mocks = { getAccountGroupObject: jest.MockedFunction< (groupId: AccountGroupId) => AccountGroupObject | undefined >; + getSelectedAccountGroup: jest.MockedFunction<() => AccountGroupId | ''>; }; }; @@ -91,6 +92,7 @@ function getMessenger( 'KeyringController:getState', 'KeyringController:withController', 'AccountTreeController:getAccountGroupObject', + 'AccountTreeController:getSelectedAccountGroup', ], events: [ 'SnapController:stateChange', @@ -101,6 +103,7 @@ function getMessenger( 'SnapController:snapUnblocked', 'SnapController:snapUninstalled', 'KeyringController:stateChange', + 'KeyringController:unlock', 'AccountTreeController:selectedAccountGroupChange', ], }); @@ -171,6 +174,15 @@ function publishSelectedAccountGroupChange( ); } +/** + * Publishes a KeyringController unlock event on the root messenger. + * + * @param rootMessenger - The root messenger. + */ +function publishUnlock(rootMessenger: RootMessenger): void { + rootMessenger.publish('KeyringController:unlock'); +} + /** * Builds a minimal `TruncatedSnap` for tests. * @@ -187,6 +199,16 @@ function buildSnap(id: string, hasKeyring: boolean): TruncatedSnap { } as MockTruncatedSnap as TruncatedSnap; } +/** + * Builds a minimal `AccountGroupObject` for tests. + * + * @param accounts - The list of account IDs in the group. + * @returns A minimal `AccountGroupObject`. + */ +function buildGroup(accounts: string[]): AccountGroupObject { + return { accounts } as unknown as AccountGroupObject; +} + /** * Builds a fake {@link KeyringEntry} with the given type. * @@ -318,6 +340,7 @@ function setup({ }, AccountTreeController: { getAccountGroupObject: jest.fn().mockReturnValue(undefined), + getSelectedAccountGroup: jest.fn().mockReturnValue(''), }, }; @@ -341,6 +364,10 @@ function setup({ 'AccountTreeController:getAccountGroupObject', mocks.AccountTreeController.getAccountGroupObject, ); + rootMessenger.registerActionHandler( + 'AccountTreeController:getSelectedAccountGroup', + mocks.AccountTreeController.getSelectedAccountGroup, + ); const service = new SnapAccountService({ messenger, config }); @@ -610,16 +637,6 @@ describe('SnapAccountService', () => { '00000000-0000-0000-0000-000000000002', ]; - /** - * Builds a minimal `AccountGroupObject` for tests. - * - * @param accounts - The list of account IDs in the group. - * @returns A minimal `AccountGroupObject`. - */ - function buildGroup(accounts: string[]): AccountGroupObject { - return { accounts } as unknown as AccountGroupObject; - } - it('forwards the selected accounts to the Snap keyring', async () => { const { service, rootMessenger, mocks } = setup(); const setSelectedAccounts = jest.fn().mockResolvedValue(undefined); @@ -696,4 +713,80 @@ describe('SnapAccountService', () => { consoleErrorSpy.mockRestore(); }); }); + + describe('on KeyringController:unlock', () => { + const MOCK_GROUP_ID = 'keyring:01JABC/group-1' as AccountGroupId; + const MOCK_ACCOUNTS = [ + '00000000-0000-0000-0000-000000000001', + '00000000-0000-0000-0000-000000000002', + ]; + + it('forwards the currently selected account group to the Snap keyring', async () => { + const { service, rootMessenger, mocks } = setup(); + const setSelectedAccounts = jest.fn().mockResolvedValue(undefined); + mockLegacySnapKeyring(mocks, { setSelectedAccounts }); + mocks.AccountTreeController.getSelectedAccountGroup.mockReturnValue( + MOCK_GROUP_ID, + ); + mocks.AccountTreeController.getAccountGroupObject.mockReturnValue( + buildGroup(MOCK_ACCOUNTS), + ); + expect(service).toBeDefined(); + + publishUnlock(rootMessenger); + await flushMicrotasks(); + + expect( + mocks.AccountTreeController.getSelectedAccountGroup, + ).toHaveBeenCalledTimes(1); + expect( + mocks.AccountTreeController.getAccountGroupObject, + ).toHaveBeenCalledWith(MOCK_GROUP_ID); + expect(setSelectedAccounts).toHaveBeenCalledWith(MOCK_ACCOUNTS); + }); + + it('does nothing when no account group is selected', async () => { + const { service, rootMessenger, mocks } = setup(); + const setSelectedAccounts = jest.fn().mockResolvedValue(undefined); + mockLegacySnapKeyring(mocks, { setSelectedAccounts }); + mocks.AccountTreeController.getSelectedAccountGroup.mockReturnValue(''); + expect(service).toBeDefined(); + + publishUnlock(rootMessenger); + await flushMicrotasks(); + + expect( + mocks.AccountTreeController.getAccountGroupObject, + ).not.toHaveBeenCalled(); + expect(setSelectedAccounts).not.toHaveBeenCalled(); + }); + + it('logs an error when forwarding to the Snap keyring fails', async () => { + const { service, rootMessenger, mocks } = setup(); + const error = new Error('forward boom'); + const setSelectedAccounts = jest.fn().mockRejectedValue(error); + mockLegacySnapKeyring(mocks, { setSelectedAccounts }); + mocks.AccountTreeController.getSelectedAccountGroup.mockReturnValue( + MOCK_GROUP_ID, + ); + mocks.AccountTreeController.getAccountGroupObject.mockReturnValue( + buildGroup(MOCK_ACCOUNTS), + ); + const consoleErrorSpy = jest + .spyOn(console, 'error') + .mockImplementation(() => undefined); + expect(service).toBeDefined(); + + publishUnlock(rootMessenger); + await flushMicrotasks(); + + expect(setSelectedAccounts).toHaveBeenCalledWith(MOCK_ACCOUNTS); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Error forwarding selected account group on unlock:', + error, + ); + + consoleErrorSpy.mockRestore(); + }); + }); }); diff --git a/packages/snap-account-service/src/SnapAccountService.ts b/packages/snap-account-service/src/SnapAccountService.ts index 39ec9db017..ca69864e2a 100644 --- a/packages/snap-account-service/src/SnapAccountService.ts +++ b/packages/snap-account-service/src/SnapAccountService.ts @@ -1,6 +1,9 @@ import { AccountGroupId } from '@metamask/account-api'; -import { AccountTreeControllerSelectedAccountGroupChangeEvent } from '@metamask/account-tree-controller'; -import { AccountTreeControllerGetAccountGroupObjectAction } from '@metamask/account-tree-controller'; +import { + AccountTreeControllerGetAccountGroupObjectAction, + AccountTreeControllerGetSelectedAccountGroupAction, + AccountTreeControllerSelectedAccountGroupChangeEvent, +} from '@metamask/account-tree-controller'; import type { SnapKeyring as LegacySnapKeyring, SnapMessage, @@ -8,6 +11,7 @@ import type { import type { KeyringControllerGetStateAction, KeyringControllerStateChangeEvent, + KeyringControllerUnlockEvent, KeyringControllerWithControllerAction, KeyringEntry, } from '@metamask/keyring-controller'; @@ -75,7 +79,8 @@ type AllowedActions = | SnapControllerGetRunnableSnapsAction | KeyringControllerGetStateAction | KeyringControllerWithControllerAction - | AccountTreeControllerGetAccountGroupObjectAction; + | AccountTreeControllerGetAccountGroupObjectAction + | AccountTreeControllerGetSelectedAccountGroupAction; /** * Events that {@link SnapAccountService} exposes to other consumers. @@ -94,6 +99,7 @@ type AllowedEvents = | SnapControllerSnapUnblockedEvent | SnapControllerSnapUninstalledEvent | KeyringControllerStateChangeEvent + | KeyringControllerUnlockEvent | AccountTreeControllerSelectedAccountGroupChangeEvent; /** @@ -172,12 +178,40 @@ export class SnapAccountService { this.#messenger.subscribe( 'AccountTreeController:selectedAccountGroupChange', - (groupId) => { - this.#handleSelectedAccountGroupChange(groupId).catch((error) => { - console.error('Error handling selected account group change:', error); - }); - }, + (groupId) => this.#handleSelectedAccountGroupChange(groupId), + ); + + this.#messenger.subscribe('KeyringController:unlock', () => + this.#handleUnlock(), + ); + } + + /** + * Handles changes to the selected account group by forwarding the new + * group's accounts to the Snap keyring. + * + * @param groupId - The ID of the newly selected account group. + */ + #handleSelectedAccountGroupChange(groupId: AccountGroupId | ''): void { + this.#forwardSelectedAccountGroup(groupId).catch((error) => { + console.error('Error handling selected account group change:', error); + }); + } + + /** + * Handles the keyring controller unlock event by forwarding the currently + * selected account group's accounts to the Snap keyring. + */ + #handleUnlock(): void { + const groupId = this.#messenger.call( + 'AccountTreeController:getSelectedAccountGroup', ); + this.#forwardSelectedAccountGroup(groupId).catch((error) => { + console.error( + 'Error forwarding selected account group on unlock:', + error, + ); + }); } /** @@ -284,12 +318,12 @@ export class SnapAccountService { } /** - * Handles changes to the selected account group. Forwards accounts from the selected account group object - * to te Snap keyring. + * Forwards the accounts of the given account group to the Snap keyring. * - * @param groupId - The ID of the newly selected account group. + * @param groupId - The ID of the account group whose accounts should be + * forwarded. If empty, this is a no-op. */ - async #handleSelectedAccountGroupChange( + async #forwardSelectedAccountGroup( groupId: AccountGroupId | '', ): Promise { if (groupId) { From 662df3f402801c56fde3655b3b5154f9219598f5 Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Mon, 11 May 2026 15:15:07 +0200 Subject: [PATCH 12/18] chore: changelog --- packages/snap-account-service/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/snap-account-service/CHANGELOG.md b/packages/snap-account-service/CHANGELOG.md index 6dbce3c074..8aae4645b9 100644 --- a/packages/snap-account-service/CHANGELOG.md +++ b/packages/snap-account-service/CHANGELOG.md @@ -28,6 +28,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The service messenger now requires the `KeyringController:withController` action. - Add `handleKeyringSnapMessage` ([#8758](https://github.com/MetaMask/core/pull/8758)) - This will be the new entry point for consumer that needs to forward keyring events to a account management Snap (instead of using the legacy Snap keyring instance directly). +- Forward selected account group accounts ([#8763](https://github.com/MetaMask/core/pull/8763)) + - This logic used to live on the clients. + - The service messenger now requires the `KeyringController:unlock`, `AccountTreeController:selectedAccountGroupChange` events and `AccountTreeController:getAccountGroupObject` action. ### Changed From 1e6cc237c6dae0275dd07ce5a58298d868c14f1b Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Mon, 11 May 2026 15:21:09 +0200 Subject: [PATCH 13/18] chore: readme --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index b163294787..d2810cadd5 100644 --- a/README.md +++ b/README.md @@ -509,6 +509,7 @@ linkStyle default opacity:0.5 signature_controller --> logging_controller; signature_controller --> messenger; signature_controller --> network_controller; + snap_account_service --> account_tree_controller; snap_account_service --> keyring_controller; snap_account_service --> messenger; social_controllers --> base_controller; From fe7dce40a8a5ebf739df1a900dbec86f09f5c739 Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Mon, 11 May 2026 16:42:12 +0200 Subject: [PATCH 14/18] build: fix (breaking cycle) --- packages/snap-account-service/package.json | 1 - .../src/SnapAccountService.test.ts | 2 +- .../snap-account-service/src/SnapAccountService.ts | 10 +++++----- packages/snap-account-service/tsconfig.build.json | 1 - packages/snap-account-service/tsconfig.json | 1 - yarn.lock | 1 - 6 files changed, 6 insertions(+), 10 deletions(-) diff --git a/packages/snap-account-service/package.json b/packages/snap-account-service/package.json index a566d7e575..9413001719 100644 --- a/packages/snap-account-service/package.json +++ b/packages/snap-account-service/package.json @@ -54,7 +54,6 @@ }, "dependencies": { "@metamask/account-api": "^1.0.4", - "@metamask/account-tree-controller": "^7.3.0", "@metamask/eth-snap-keyring": "^22.0.1", "@metamask/keyring-controller": "^25.5.0", "@metamask/messenger": "^1.2.0", diff --git a/packages/snap-account-service/src/SnapAccountService.test.ts b/packages/snap-account-service/src/SnapAccountService.test.ts index 27affa6798..871206f85d 100644 --- a/packages/snap-account-service/src/SnapAccountService.test.ts +++ b/packages/snap-account-service/src/SnapAccountService.test.ts @@ -1,5 +1,4 @@ import type { AccountGroupId } from '@metamask/account-api'; -import type { AccountGroupObject } from '@metamask/account-tree-controller'; import type { SnapKeyring, SnapMessage } from '@metamask/eth-snap-keyring'; import { KeyringControllerState, @@ -24,6 +23,7 @@ import type { SnapAccountServiceOptions, } from './SnapAccountService'; import { SnapAccountService } from './SnapAccountService'; +import type { AccountGroupObject } from './types'; type RootMessenger = Messenger< MockAnyNamespace, diff --git a/packages/snap-account-service/src/SnapAccountService.ts b/packages/snap-account-service/src/SnapAccountService.ts index ca69864e2a..ec2a285bec 100644 --- a/packages/snap-account-service/src/SnapAccountService.ts +++ b/packages/snap-account-service/src/SnapAccountService.ts @@ -1,9 +1,4 @@ import { AccountGroupId } from '@metamask/account-api'; -import { - AccountTreeControllerGetAccountGroupObjectAction, - AccountTreeControllerGetSelectedAccountGroupAction, - AccountTreeControllerSelectedAccountGroupChangeEvent, -} from '@metamask/account-tree-controller'; import type { SnapKeyring as LegacySnapKeyring, SnapMessage, @@ -43,6 +38,11 @@ import type { import { SnapPlatformWatcher } from './SnapPlatformWatcher'; import type { SnapPlatformWatcherConfig } from './SnapPlatformWatcher'; import { SnapTracker } from './SnapTracker'; +import type { + AccountTreeControllerGetAccountGroupObjectAction, + AccountTreeControllerGetSelectedAccountGroupAction, + AccountTreeControllerSelectedAccountGroupChangeEvent, +} from './types'; /** * The name of the {@link SnapAccountService}, used to namespace the service's diff --git a/packages/snap-account-service/tsconfig.build.json b/packages/snap-account-service/tsconfig.build.json index 17c397c85e..59c9b7a1df 100644 --- a/packages/snap-account-service/tsconfig.build.json +++ b/packages/snap-account-service/tsconfig.build.json @@ -6,7 +6,6 @@ "rootDir": "./src" }, "references": [ - { "path": "../account-tree-controller/tsconfig.build.json" }, { "path": "../keyring-controller/tsconfig.build.json" }, { "path": "../messenger/tsconfig.build.json" } ], diff --git a/packages/snap-account-service/tsconfig.json b/packages/snap-account-service/tsconfig.json index b5367cb8ec..a0556a87d1 100644 --- a/packages/snap-account-service/tsconfig.json +++ b/packages/snap-account-service/tsconfig.json @@ -4,7 +4,6 @@ "baseUrl": "./" }, "references": [ - { "path": "../account-tree-controller" }, { "path": "../keyring-controller" }, { "path": "../messenger" } ], diff --git a/yarn.lock b/yarn.lock index 1b5964bd25..569c00d96f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5433,7 +5433,6 @@ __metadata: resolution: "@metamask/snap-account-service@workspace:packages/snap-account-service" dependencies: "@metamask/account-api": "npm:^1.0.4" - "@metamask/account-tree-controller": "npm:^7.3.0" "@metamask/auto-changelog": "npm:^6.1.0" "@metamask/eth-snap-keyring": "npm:^22.0.1" "@metamask/keyring-controller": "npm:^25.5.0" From 2c98bc214860641a0aed30a7066d4f80fe604434 Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Mon, 11 May 2026 17:17:19 +0200 Subject: [PATCH 15/18] fix: add missing types.ts --- packages/snap-account-service/src/types.ts | 52 ++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 packages/snap-account-service/src/types.ts diff --git a/packages/snap-account-service/src/types.ts b/packages/snap-account-service/src/types.ts new file mode 100644 index 0000000000..2ffb5aaae1 --- /dev/null +++ b/packages/snap-account-service/src/types.ts @@ -0,0 +1,52 @@ +import type { AccountGroupId } from '@metamask/account-api'; + +/* + * NOTE: The types below intentionally duplicate definitions from + * `@metamask/account-tree-controller`. They are mirrored here to break a + * package-level circular dependency: + * + * account-tree-controller -> multichain-account-service -> snap-account-service + * -> account-tree-controller + * + * `multichain-account-service` legitimately depends on `snap-account-service` + * (for `:ensureReady`), and `account-tree-controller` legitimately depends on + * `multichain-account-service`. Importing `AccountTreeController` types here + * would close the cycle and crash `ts-bridge` with a stack overflow during + * project-reference builds. + * + * Keep these signatures in sync with `@metamask/account-tree-controller`. + */ + +/** + * Minimal shape of an account group object as consumed by + * {@link SnapAccountService}. Only the `accounts` field is read at runtime; + * the rest of the structure is intentionally omitted to keep this mirror + * narrow. See the note above. + */ +export type AccountGroupObject = { + accounts: string[]; +}; + +/** + * Mirror of `AccountTreeControllerGetAccountGroupObjectAction`. + */ +export type AccountTreeControllerGetAccountGroupObjectAction = { + type: `AccountTreeController:getAccountGroupObject`; + handler: (groupId: AccountGroupId) => AccountGroupObject | undefined; +}; + +/** + * Mirror of `AccountTreeControllerGetSelectedAccountGroupAction`. + */ +export type AccountTreeControllerGetSelectedAccountGroupAction = { + type: `AccountTreeController:getSelectedAccountGroup`; + handler: () => AccountGroupId | ''; +}; + +/** + * Mirror of `AccountTreeControllerSelectedAccountGroupChangeEvent`. + */ +export type AccountTreeControllerSelectedAccountGroupChangeEvent = { + type: `AccountTreeController:selectedAccountGroupChange`; + payload: [AccountGroupId | '', AccountGroupId | '']; +}; From 2fa826c1582b2319cf7eb7b6f43fb9537be93d4e Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Mon, 11 May 2026 17:37:14 +0200 Subject: [PATCH 16/18] build: still use account-tree-controller dep --- packages/snap-account-service/package.json | 1 + yarn.lock | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/snap-account-service/package.json b/packages/snap-account-service/package.json index 9413001719..a566d7e575 100644 --- a/packages/snap-account-service/package.json +++ b/packages/snap-account-service/package.json @@ -54,6 +54,7 @@ }, "dependencies": { "@metamask/account-api": "^1.0.4", + "@metamask/account-tree-controller": "^7.3.0", "@metamask/eth-snap-keyring": "^22.0.1", "@metamask/keyring-controller": "^25.5.0", "@metamask/messenger": "^1.2.0", diff --git a/yarn.lock b/yarn.lock index 569c00d96f..1b5964bd25 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5433,6 +5433,7 @@ __metadata: resolution: "@metamask/snap-account-service@workspace:packages/snap-account-service" dependencies: "@metamask/account-api": "npm:^1.0.4" + "@metamask/account-tree-controller": "npm:^7.3.0" "@metamask/auto-changelog": "npm:^6.1.0" "@metamask/eth-snap-keyring": "npm:^22.0.1" "@metamask/keyring-controller": "npm:^25.5.0" From d4676207f99d05bcdfcdd45b5fd715f314a85f92 Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Mon, 11 May 2026 17:50:15 +0200 Subject: [PATCH 17/18] chore: cosmetic --- packages/snap-account-service/src/types.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/snap-account-service/src/types.ts b/packages/snap-account-service/src/types.ts index 2ffb5aaae1..9e175a1b7e 100644 --- a/packages/snap-account-service/src/types.ts +++ b/packages/snap-account-service/src/types.ts @@ -1,4 +1,5 @@ import type { AccountGroupId } from '@metamask/account-api'; +import { AccountId } from '@metamask/keyring-utils'; /* * NOTE: The types below intentionally duplicate definitions from @@ -24,7 +25,7 @@ import type { AccountGroupId } from '@metamask/account-api'; * narrow. See the note above. */ export type AccountGroupObject = { - accounts: string[]; + accounts: AccountId[]; }; /** From 1e129f1d57c762b8d28e48a1811201a9825996e5 Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Mon, 11 May 2026 17:50:22 +0200 Subject: [PATCH 18/18] build: add comment --- packages/snap-account-service/tsconfig.build.json | 9 +++++++++ packages/snap-account-service/tsconfig.json | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/packages/snap-account-service/tsconfig.build.json b/packages/snap-account-service/tsconfig.build.json index 59c9b7a1df..f5d033dda3 100644 --- a/packages/snap-account-service/tsconfig.build.json +++ b/packages/snap-account-service/tsconfig.build.json @@ -6,6 +6,15 @@ "rootDir": "./src" }, "references": [ + // READ THIS CAREFULLY: + // `account-tree-controller` is intentionally NOT referenced here to break + // a project-reference cycle: + // account-tree-controller -> multichain-account-service + // -> snap-account-service + // -> account-tree-controller + // The types we'd otherwise pull from `@metamask/account-tree-controller` + // are mirrored locally in `./src/types.ts` — keep them in sync. + // { "path": "../account-tree-controller/tsconfig.build.json" }, { "path": "../keyring-controller/tsconfig.build.json" }, { "path": "../messenger/tsconfig.build.json" } ], diff --git a/packages/snap-account-service/tsconfig.json b/packages/snap-account-service/tsconfig.json index a0556a87d1..4fd7b9ded3 100644 --- a/packages/snap-account-service/tsconfig.json +++ b/packages/snap-account-service/tsconfig.json @@ -4,6 +4,15 @@ "baseUrl": "./" }, "references": [ + // READ THIS CAREFULLY: + // `account-tree-controller` is intentionally NOT referenced here to break + // a project-reference cycle: + // account-tree-controller -> multichain-account-service + // -> snap-account-service + // -> account-tree-controller + // The types we'd otherwise pull from `@metamask/account-tree-controller` + // are mirrored locally in `./src/types.ts` — keep them in sync. + // { "path": "../account-tree-controller" }, { "path": "../keyring-controller" }, { "path": "../messenger" } ],