diff --git a/packages/snap-account-service/CHANGELOG.md b/packages/snap-account-service/CHANGELOG.md index 43c2339e37..363e6fcbf2 100644 --- a/packages/snap-account-service/CHANGELOG.md +++ b/packages/snap-account-service/CHANGELOG.md @@ -10,17 +10,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add `SnapAccountService` ([#8414](https://github.com/MetaMask/core/pull/8414)) -- Add `SnapPlatformWatcher`, which waits for the Snap platform to be ready and for a Snap keyring to appear in `KeyringController` state before allowing Snap account operations ([#8715](https://github.com/MetaMask/core/pull/8715)) +- Add `SnapPlatformWatcher` and `SnapAccountService.ensureReady` ([#8715](https://github.com/MetaMask/core/pull/8715)), ([#8725](https://github.com/MetaMask/core/pull/8725)) + - Waits for the Snap platform to be ready and for a Snap keyring to appear in `KeyringController` state before allowing Snap account operations. + - Callers must ensure `init()` has run and the Snap is currently installed, enabled, non-blocked, and declares `endowment:keyring`. - `SnapAccountService.ensureReady` now awaits the watcher, so it only resolves once both conditions hold (or rejects if the Snap keyring does not appear within the configured timeout). + - `SnapAccountService.ensureReady` now throws `Unknown snap: ""` when called with a Snap ID that isn't tracked as an account-management Snap. - Add `config` option to `SnapAccountService` constructor with a `snapPlatformWatcher` field exposing `ensureOnboardingComplete` and `snapKeyringWaitTimeoutMs` ([#8715](https://github.com/MetaMask/core/pull/8715)) - Export `SnapAccountServiceConfig` and `SnapPlatformWatcherConfig` types. - Add `@metamask/keyring-controller` dependency ([#8715](https://github.com/MetaMask/core/pull/8715)) - The service messenger now requires the `KeyringController:getState` action and `KeyringController:stateChange` event. +- Add `getSnaps` action to `SnapAccountService`, returning the IDs of installed, enabled, non-blocked Snaps that declare the `endowment:keyring` permission ([#8725](https://github.com/MetaMask/core/pull/8725)) + - 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. ### Changed - Bump `@metamask/messenger` from `^1.1.1` to `^1.2.0` ([#8632](https://github.com/MetaMask/core/pull/8632)) -- **BREAKING:** Remove the top-level `ensureOnboardingComplete` option from `SnapAccountServiceOptions` ([#8715](https://github.com/MetaMask/core/pull/8715)) - - Pass the callback via `config.snapPlatformWatcher.ensureOnboardingComplete` instead. [Unreleased]: https://github.com/MetaMask/core/ diff --git a/packages/snap-account-service/package.json b/packages/snap-account-service/package.json index 419dc6bc03..d6984ab9fe 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/eth-snap-keyring": "^22.0.1", + "@metamask/keyring-api": "^23.1.0", "@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 a206cc87a4..971420f049 100644 --- a/packages/snap-account-service/src/SnapAccountService-method-action-types.ts +++ b/packages/snap-account-service/src/SnapAccountService-method-action-types.ts @@ -5,21 +5,57 @@ import type { SnapAccountService } from './SnapAccountService'; +/** + * Returns the IDs of all currently tracked account-management Snaps — + * Snaps that are installed, enabled, not blocked, and have the + * `endowment:keyring` permission. + * + * @returns The IDs of tracked account-management Snaps. + */ +export type SnapAccountServiceGetSnapsAction = { + type: `SnapAccountService:getSnaps`; + handler: SnapAccountService['getSnaps']; +}; + /** * Ensures everything is ready to use Snap accounts for the given Snap. - * 1. Waits for the Snap platform to be fully started. + * 1. Validates that `snapId` is a tracked account-management Snap. + * 2. Runs the legacy -> v2 Snap keyring migration (cached — no-op if + * already done). + * 3. Atomically creates the v2 keyring for this Snap if it doesn't exist + * yet. + * 4. Waits for the Snap platform to be fully started. * * Safe to call concurrently — each step is idempotent or mutex-protected. * - * @param _snapId - ID of the Snap to ensure readiness for. + * @param snapId - ID of the Snap to ensure readiness for. + * @throws If `snapId` is not a tracked account-management Snap. */ export type SnapAccountServiceEnsureReadyAction = { type: `SnapAccountService:ensureReady`; handler: SnapAccountService['ensureReady']; }; +/** + * Handle a message from a Snap. + * + * Only `AccountCreated` triggers lazy keyring creation, since that is the + * single entry point for the v1 event-driven flow. All other events from + * unknown Snaps throw an error. + * + * @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 = - SnapAccountServiceEnsureReadyAction; + | SnapAccountServiceGetSnapsAction + | SnapAccountServiceEnsureReadyAction + | SnapAccountServiceHandleKeyringSnapMessageAction; diff --git a/packages/snap-account-service/src/SnapAccountService.test.ts b/packages/snap-account-service/src/SnapAccountService.test.ts index d4c401e2f2..713efdd8d4 100644 --- a/packages/snap-account-service/src/SnapAccountService.test.ts +++ b/packages/snap-account-service/src/SnapAccountService.test.ts @@ -1,3 +1,5 @@ +import { SNAP_KEYRING_TYPE } from '@metamask/eth-snap-keyring'; +import { KeyringType } from '@metamask/keyring-api/v2'; import { KeyringTypes } from '@metamask/keyring-controller'; import { Messenger, MOCK_ANY_NAMESPACE } from '@metamask/messenger'; import type { @@ -6,6 +8,8 @@ import type { MessengerEvents, } from '@metamask/messenger'; import type { SnapControllerState } from '@metamask/snaps-controllers'; +import type { SnapId } from '@metamask/snaps-sdk'; +import type { TruncatedSnap } from '@metamask/snaps-utils'; import type { SnapAccountServiceMessenger, @@ -30,10 +34,23 @@ type Mocks = { // eslint-disable-next-line @typescript-eslint/naming-convention SnapController: { getState: jest.MockedFunction<() => SnapControllerState>; + getRunnableSnaps: jest.MockedFunction<() => TruncatedSnap[]>; }; // eslint-disable-next-line @typescript-eslint/naming-convention KeyringController: { getState: jest.MockedFunction<() => { keyrings: { type: string }[] }>; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + withController: jest.MockedFunction< + (callback: (controller: any) => Promise) => Promise + >; + withKeyringV2Unsafe: jest.MockedFunction< + ( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + selector: any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + operation: (selected: { keyring: any }) => Promise, + ) => Promise + >; }; }; @@ -62,8 +79,22 @@ function getMessenger( }); rootMessenger.delegate({ messenger, - actions: ['SnapController:getState', 'KeyringController:getState'], - events: ['SnapController:stateChange', 'KeyringController:stateChange'], + actions: [ + 'SnapController:getState', + 'SnapController:getRunnableSnaps', + 'KeyringController:getState', + 'KeyringController:withController', + 'KeyringController:withKeyringV2Unsafe', + ], + events: [ + 'SnapController:stateChange', + 'SnapController:snapInstalled', + 'SnapController:snapEnabled', + 'SnapController:snapDisabled', + 'SnapController:snapBlocked', + 'SnapController:snapUninstalled', + 'KeyringController:stateChange', + ], }); return messenger; } @@ -103,22 +134,105 @@ function publishKeyrings( ); } +/** + * Builds a minimal `TruncatedSnap` for tests. + * + * @param id - The Snap ID. + * @param hasKeyring - Whether the Snap declares the `endowment:keyring` initial permission. + * @returns A minimal `TruncatedSnap`. + */ +function buildSnap(id: string, hasKeyring: boolean): TruncatedSnap { + return { + id: id as SnapId, + initialPermissions: hasKeyring ? { 'endowment:keyring': {} } : {}, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any; +} + +/** + * Publishes a `SnapController:snapInstalled` event on the root messenger. + * + * @param rootMessenger - The root messenger. + * @param snap - The Snap that was installed. + */ +function publishSnapInstalled( + rootMessenger: RootMessenger, + snap: TruncatedSnap, +): void { + rootMessenger.publish('SnapController:snapInstalled', snap, 'origin', false); +} + +/** + * Publishes a `SnapController:snapEnabled` event on the root messenger. + * + * @param rootMessenger - The root messenger. + * @param snap - The Snap that was enabled. + */ +function publishSnapEnabled( + rootMessenger: RootMessenger, + snap: TruncatedSnap, +): void { + rootMessenger.publish('SnapController:snapEnabled', snap); +} + +/** + * Publishes a `SnapController:snapDisabled` event on the root messenger. + * + * @param rootMessenger - The root messenger. + * @param snap - The Snap that was disabled. + */ +function publishSnapDisabled( + rootMessenger: RootMessenger, + snap: TruncatedSnap, +): void { + rootMessenger.publish('SnapController:snapDisabled', snap); +} + +/** + * Publishes a `SnapController:snapBlocked` event on the root messenger. + * + * @param rootMessenger - The root messenger. + * @param snapId - The ID of the Snap that was blocked. + */ +function publishSnapBlocked( + rootMessenger: RootMessenger, + snapId: string, +): void { + rootMessenger.publish('SnapController:snapBlocked', snapId); +} + +/** + * Publishes a `SnapController:snapUninstalled` event on the root messenger. + * + * @param rootMessenger - The root messenger. + * @param snap - The Snap that was uninstalled. + */ +function publishSnapUninstalled( + rootMessenger: RootMessenger, + snap: TruncatedSnap, +): void { + rootMessenger.publish('SnapController:snapUninstalled', snap); +} + /** * Constructs the service under test with sensible defaults. * * @param args - The arguments to this function. * @param args.snapIsReady - Initial value of `SnapController.isReady`. * @param args.keyrings - Initial keyrings returned by `KeyringController:getState`. + * @param args.runnableSnaps - Snaps returned by `SnapController:getRunnableSnaps`. * @param args.config - Optional service config. * @returns The new service, root messenger, service messenger, and mocks. */ function setup({ snapIsReady = true, keyrings = [{ type: KeyringTypes.snap }], + runnableSnaps = [], config, }: { snapIsReady?: boolean; keyrings?: { type: string }[]; + runnableSnaps?: TruncatedSnap[]; config?: SnapAccountServiceOptions['config']; } = {}): { service: SnapAccountService; @@ -134,9 +248,21 @@ function setup({ getState: jest .fn() .mockReturnValue({ isReady: snapIsReady } as SnapControllerState), + getRunnableSnaps: jest.fn().mockReturnValue(runnableSnaps), }, KeyringController: { getState: jest.fn().mockReturnValue({ keyrings }), + withController: jest + .fn() + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .mockImplementation(async (callback: (controller: any) => Promise) => + callback({ + keyrings: [], + addNewKeyring: jest.fn().mockResolvedValue(undefined), + removeKeyring: jest.fn().mockResolvedValue(undefined), + }), + ), + withKeyringV2Unsafe: jest.fn(), }, }; @@ -144,17 +270,32 @@ function setup({ 'SnapController:getState', mocks.SnapController.getState, ); + rootMessenger.registerActionHandler( + 'SnapController:getRunnableSnaps', + mocks.SnapController.getRunnableSnaps, + ); rootMessenger.registerActionHandler( 'KeyringController:getState', mocks.KeyringController.getState, ); + rootMessenger.registerActionHandler( + 'KeyringController:withController', + // eslint-disable-next-line @typescript-eslint/no-explicit-any + mocks.KeyringController.withController as any, + ); + rootMessenger.registerActionHandler( + 'KeyringController:withKeyringV2Unsafe', + // eslint-disable-next-line @typescript-eslint/no-explicit-any + mocks.KeyringController.withKeyringV2Unsafe as any, + ); const service = new SnapAccountService({ messenger, config }); return { service, rootMessenger, messenger, mocks }; } -const MOCK_SNAP_ID = 'npm:@metamask/mock-snap' as const; +const MOCK_SNAP_ID = 'npm:@metamask/mock-snap' as SnapId; +const MOCK_OTHER_SNAP_ID = 'npm:@metamask/other-snap' as SnapId; describe('SnapAccountService', () => { describe('init', () => { @@ -163,17 +304,309 @@ describe('SnapAccountService', () => { expect(await service.init()).toBeUndefined(); }); + + it('seeds tracked Snaps from getRunnableSnaps, filtering out non-keyring Snaps', async () => { + const { service } = setup({ + runnableSnaps: [ + buildSnap(MOCK_SNAP_ID, true), + buildSnap(MOCK_OTHER_SNAP_ID, false), + ], + }); + + await service.init(); + + expect(service.getSnaps()).toStrictEqual([MOCK_SNAP_ID]); + }); + }); + + describe('getSnaps', () => { + it('returns an empty array before init', () => { + const { service } = setup({ + runnableSnaps: [buildSnap(MOCK_SNAP_ID, true)], + }); + + expect(service.getSnaps()).toStrictEqual([]); + }); + }); + + describe('lifecycle events', () => { + it('ignores add events received before init', async () => { + const { service, rootMessenger } = setup(); + + publishSnapInstalled(rootMessenger, buildSnap(MOCK_SNAP_ID, true)); + + await service.init(); + + expect(service.getSnaps()).toStrictEqual([]); + }); + + it('ignores remove events received before init', async () => { + const { service, rootMessenger } = setup({ + runnableSnaps: [buildSnap(MOCK_SNAP_ID, true)], + }); + + // Publish a removal event *before* init: it must be ignored, so once + // init seeds from `getRunnableSnaps` the Snap is still tracked. + publishSnapUninstalled(rootMessenger, buildSnap(MOCK_SNAP_ID, true)); + + await service.init(); + + expect(service.getSnaps()).toStrictEqual([MOCK_SNAP_ID]); + }); + + it('adds a Snap on snapInstalled when it has the keyring endowment', async () => { + const { service, rootMessenger } = setup(); + + await service.init(); + publishSnapInstalled(rootMessenger, buildSnap(MOCK_SNAP_ID, true)); + + expect(service.getSnaps()).toStrictEqual([MOCK_SNAP_ID]); + }); + + it('does not add a Snap on snapInstalled when it lacks the keyring endowment', async () => { + const { service, rootMessenger } = setup(); + + await service.init(); + publishSnapInstalled(rootMessenger, buildSnap(MOCK_SNAP_ID, false)); + + expect(service.getSnaps()).toStrictEqual([]); + }); + + it('adds a Snap on snapEnabled when it has the keyring endowment', async () => { + const { service, rootMessenger } = setup(); + + await service.init(); + publishSnapEnabled(rootMessenger, buildSnap(MOCK_SNAP_ID, true)); + + expect(service.getSnaps()).toStrictEqual([MOCK_SNAP_ID]); + }); + + it('removes a Snap on snapDisabled', async () => { + const { service, rootMessenger } = setup({ + runnableSnaps: [buildSnap(MOCK_SNAP_ID, true)], + }); + + await service.init(); + expect(service.getSnaps()).toStrictEqual([MOCK_SNAP_ID]); + + publishSnapDisabled(rootMessenger, buildSnap(MOCK_SNAP_ID, true)); + + expect(service.getSnaps()).toStrictEqual([]); + }); + + it('removes a Snap on snapBlocked', async () => { + const { service, rootMessenger } = setup({ + runnableSnaps: [buildSnap(MOCK_SNAP_ID, true)], + }); + + await service.init(); + publishSnapBlocked(rootMessenger, MOCK_SNAP_ID); + + expect(service.getSnaps()).toStrictEqual([]); + }); + + it('removes a Snap on snapUninstalled', async () => { + const { service, rootMessenger } = setup({ + runnableSnaps: [buildSnap(MOCK_SNAP_ID, true)], + }); + + await service.init(); + publishSnapUninstalled(rootMessenger, buildSnap(MOCK_SNAP_ID, true)); + + expect(service.getSnaps()).toStrictEqual([]); + }); + }); + + describe('migrate', () => { + it('runs the migration only once when called concurrently', async () => { + const { service, mocks } = setup(); + + await Promise.all([service.migrate(), service.migrate()]); + + expect(mocks.KeyringController.withController).toHaveBeenCalledTimes(1); + }); + + it('is a no-op when no legacy Snap keyring is present', async () => { + const addNewKeyring = jest.fn().mockResolvedValue(undefined); + const removeKeyring = jest.fn().mockResolvedValue(undefined); + const { service, mocks } = setup(); + mocks.KeyringController.withController.mockImplementation( + async (callback) => + callback({ keyrings: [], addNewKeyring, removeKeyring }), + ); + + await service.migrate(); + + expect(addNewKeyring).not.toHaveBeenCalled(); + expect(removeKeyring).not.toHaveBeenCalled(); + }); + + it('migrates accounts from the legacy Snap keyring to per-Snap v2 keyrings and removes the legacy entry', async () => { + const addNewKeyring = jest.fn().mockResolvedValue(undefined); + const removeKeyring = jest.fn().mockResolvedValue(undefined); + const legacyKeyringId = 'legacy-keyring-id'; + const account1 = { + id: 'account-1', + address: '0x1', + metadata: { snap: { id: MOCK_SNAP_ID } }, + }; + const account2 = { + id: 'account-2', + address: '0x2', + metadata: { snap: { id: MOCK_SNAP_ID } }, + }; + const account3 = { + id: 'account-3', + address: '0x3', + metadata: { snap: { id: MOCK_OTHER_SNAP_ID } }, + }; + const orphanAccount = { + id: 'orphan', + address: '0x4', + metadata: {}, + }; + const legacyKeyring = { + type: SNAP_KEYRING_TYPE, + listAccounts: jest + .fn() + .mockReturnValue([account1, account2, account3, orphanAccount]), + }; + const { service, mocks } = setup(); + mocks.KeyringController.withController.mockImplementation( + async (callback) => + callback({ + keyrings: [ + { keyring: legacyKeyring, metadata: { id: legacyKeyringId } }, + ], + addNewKeyring, + removeKeyring, + }), + ); + + await service.migrate(); + + expect(addNewKeyring).toHaveBeenCalledTimes(2); + expect(addNewKeyring).toHaveBeenCalledWith(KeyringType.Snap, { + snapId: MOCK_SNAP_ID, + accounts: { + [account1.id]: { id: account1.id, address: account1.address }, + [account2.id]: { id: account2.id, address: account2.address }, + }, + }); + expect(addNewKeyring).toHaveBeenCalledWith(KeyringType.Snap, { + snapId: MOCK_OTHER_SNAP_ID, + accounts: { + [account3.id]: { id: account3.id, address: account3.address }, + }, + }); + expect(removeKeyring).toHaveBeenCalledWith(legacyKeyringId); + }); }); describe('ensureReady', () => { - it('resolves when platform is already ready', async () => { + it('throws when the Snap is not tracked', async () => { const { service } = setup(); + await service.init(); + + await expect(service.ensureReady(MOCK_SNAP_ID)).rejects.toThrow( + `Unknown snap: "${MOCK_SNAP_ID}"`, + ); + }); + + it('throws before init even for runnable Snaps', async () => { + const { service } = setup({ + runnableSnaps: [buildSnap(MOCK_SNAP_ID, true)], + }); + + await expect(service.ensureReady(MOCK_SNAP_ID)).rejects.toThrow( + `Unknown snap: "${MOCK_SNAP_ID}"`, + ); + }); + + it('resolves when platform is already ready', async () => { + const { service } = setup({ + runnableSnaps: [buildSnap(MOCK_SNAP_ID, true)], + }); + + await service.init(); + expect(await service.ensureReady(MOCK_SNAP_ID)).toBeUndefined(); }); + it('runs the migration before checking platform readiness', async () => { + const { service, mocks } = setup({ + runnableSnaps: [buildSnap(MOCK_SNAP_ID, true)], + }); + + await service.init(); + // `init` runs migration once + ensureReady runs the keyring-creation + // step (the cached migration call is a no-op). + await service.ensureReady(MOCK_SNAP_ID); + + expect(mocks.KeyringController.withController).toHaveBeenCalledTimes(2); + }); + + it('creates a v2 keyring for the Snap when one does not exist yet', async () => { + const addNewKeyring = jest.fn().mockResolvedValue(undefined); + const removeKeyring = jest.fn().mockResolvedValue(undefined); + const { service, mocks } = setup({ + runnableSnaps: [buildSnap(MOCK_SNAP_ID, true)], + }); + mocks.KeyringController.withController.mockImplementation( + async (callback) => + callback({ keyrings: [], addNewKeyring, removeKeyring }), + ); + + await service.init(); + await service.ensureReady(MOCK_SNAP_ID); + + expect(addNewKeyring).toHaveBeenCalledWith(KeyringType.Snap, { + snapId: MOCK_SNAP_ID, + accounts: {}, + }); + }); + + it('does not create a v2 keyring when one already exists for the Snap', async () => { + const addNewKeyring = jest.fn().mockResolvedValue(undefined); + const removeKeyring = jest.fn().mockResolvedValue(undefined); + const { service, mocks } = setup({ + runnableSnaps: [buildSnap(MOCK_SNAP_ID, true)], + }); + // First call: migration (no legacy snap keyring → early return). + // Second call: ensureReady keyring check (snap keyring already exists). + mocks.KeyringController.withController + .mockImplementationOnce(async (callback) => + callback({ keyrings: [], addNewKeyring, removeKeyring }), + ) + .mockImplementationOnce(async (callback) => + callback({ + keyrings: [ + { + keyringV2: { + type: KeyringType.Snap, + snapId: MOCK_SNAP_ID, + }, + }, + ], + addNewKeyring, + removeKeyring, + }), + ); + + await service.init(); + await service.ensureReady(MOCK_SNAP_ID); + + expect(addNewKeyring).not.toHaveBeenCalled(); + }); + it('waits for the Snap platform to become ready', async () => { - const { service, rootMessenger } = setup({ snapIsReady: false }); + const { service, rootMessenger } = setup({ + snapIsReady: false, + runnableSnaps: [buildSnap(MOCK_SNAP_ID, true)], + }); + + await service.init(); let resolved = false; const ensurePromise = service.ensureReady(MOCK_SNAP_ID).then(() => { @@ -190,7 +623,12 @@ describe('SnapAccountService', () => { }); it('waits for the Snap keyring to appear via KeyringController:stateChange', async () => { - const { service, rootMessenger } = setup({ keyrings: [] }); + const { service, rootMessenger } = setup({ + keyrings: [], + runnableSnaps: [buildSnap(MOCK_SNAP_ID, true)], + }); + + await service.init(); let resolved = false; const ensurePromise = service.ensureReady(MOCK_SNAP_ID).then(() => { @@ -198,9 +636,9 @@ describe('SnapAccountService', () => { return undefined; }); - // Flush microtasks so #waitForSnapKeyring subscribes. - await Promise.resolve(); - await Promise.resolve(); + // Flush microtasks so migration completes and #waitForSnapKeyring + // subscribes. + await new Promise((resolve) => setImmediate(resolve)); expect(resolved).toBe(false); @@ -213,11 +651,14 @@ describe('SnapAccountService', () => { it('rejects if the Snap keyring does not appear within snapKeyringWaitTimeoutMs', async () => { const { service } = setup({ keyrings: [], + runnableSnaps: [buildSnap(MOCK_SNAP_ID, true)], config: { snapPlatformWatcher: { snapKeyringWaitTimeoutMs: 1_000 }, }, }); + await service.init(); + jest.useFakeTimers(); const ensurePromise = service.ensureReady(MOCK_SNAP_ID); // Attach rejection handler before advancing timers to avoid unhandled rejection. @@ -242,16 +683,19 @@ describe('SnapAccountService', () => { ); const { service } = setup({ + runnableSnaps: [buildSnap(MOCK_SNAP_ID, true)], config: { snapPlatformWatcher: { ensureOnboardingComplete } }, }); + await service.init(); + let resolved = false; const ensurePromise = service.ensureReady(MOCK_SNAP_ID).then(() => { resolved = true; return undefined; }); - await Promise.resolve(); + await new Promise((resolve) => setImmediate(resolve)); expect(ensureOnboardingComplete).toHaveBeenCalledTimes(1); expect(resolved).toBe(false); @@ -261,4 +705,101 @@ describe('SnapAccountService', () => { expect(resolved).toBe(true); }); }); + + describe('handleKeyringSnapMessage', () => { + it('returns an empty array for `getSelectedAccounts`', async () => { + const { service, mocks } = setup(); + + const result = await service.handleKeyringSnapMessage(MOCK_SNAP_ID, { + method: 'getSelectedAccounts', + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any); + + expect(result).toStrictEqual([]); + expect(mocks.KeyringController.withController).not.toHaveBeenCalled(); + expect( + mocks.KeyringController.withKeyringV2Unsafe, + ).not.toHaveBeenCalled(); + }); + + it('throws for non-AccountCreated events', async () => { + const { service } = setup(); + + await expect( + service.handleKeyringSnapMessage(MOCK_SNAP_ID, { + method: 'accountUpdated', + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any), + ).rejects.toThrow( + 'Cannot delegate keyring Snap message, keyring does not exist yet.', + ); + }); + + it('auto-creates a Snap keyring on AccountCreated when one does not exist, then delegates', async () => { + const addNewKeyring = jest.fn().mockResolvedValue(undefined); + const removeKeyring = jest.fn().mockResolvedValue(undefined); + const handleSnapMessage = jest.fn().mockResolvedValue('ok'); + const { service, mocks } = setup(); + mocks.KeyringController.withController.mockImplementation( + async (callback) => + callback({ keyrings: [], addNewKeyring, removeKeyring }), + ); + mocks.KeyringController.withKeyringV2Unsafe.mockImplementation( + async (_selector, operation) => + operation({ + keyring: { handleKeyringSnapMessage: handleSnapMessage }, + }), + ); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const message = { method: 'notify:accountCreated' } as any; + const result = await service.handleKeyringSnapMessage( + MOCK_SNAP_ID, + message, + ); + + expect(addNewKeyring).toHaveBeenCalledWith(KeyringType.Snap, { + snapId: MOCK_SNAP_ID, + accounts: {}, + }); + expect(handleSnapMessage).toHaveBeenCalledWith(message); + expect(result).toBe('ok'); + }); + + it('does not re-create the Snap keyring on AccountCreated when one already exists', async () => { + const addNewKeyring = jest.fn().mockResolvedValue(undefined); + const removeKeyring = jest.fn().mockResolvedValue(undefined); + const handleSnapMessage = jest.fn().mockResolvedValue('ok'); + const { service, mocks } = setup(); + mocks.KeyringController.withController.mockImplementation( + async (callback) => + callback({ + keyrings: [ + { + keyringV2: { + type: KeyringType.Snap, + snapId: MOCK_SNAP_ID, + }, + }, + ], + addNewKeyring, + removeKeyring, + }), + ); + mocks.KeyringController.withKeyringV2Unsafe.mockImplementation( + async (_selector, operation) => + operation({ + keyring: { handleKeyringSnapMessage: handleSnapMessage }, + }), + ); + + await service.handleKeyringSnapMessage(MOCK_SNAP_ID, { + method: 'notify:accountCreated', + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any); + + expect(addNewKeyring).not.toHaveBeenCalled(); + expect(handleSnapMessage).toHaveBeenCalled(); + }); + }); }); diff --git a/packages/snap-account-service/src/SnapAccountService.ts b/packages/snap-account-service/src/SnapAccountService.ts index e43cb700b8..f99f11a7cd 100644 --- a/packages/snap-account-service/src/SnapAccountService.ts +++ b/packages/snap-account-service/src/SnapAccountService.ts @@ -1,18 +1,67 @@ +import { + SNAP_KEYRING_TYPE, + SnapKeyring as LegacySnapKeyring, +} from '@metamask/eth-snap-keyring'; +import type { + SnapKeyring, + SnapKeyringState, +} from '@metamask/eth-snap-keyring/v2'; +import type { SnapMessage } from '@metamask/eth-snap-keyring'; +import { KeyringEvent } from '@metamask/keyring-api'; +import type { Keyring } from '@metamask/keyring-api/v2'; +import { KeyringType } from '@metamask/keyring-api/v2'; import type { KeyringControllerGetStateAction, KeyringControllerStateChangeEvent, + KeyringControllerWithControllerAction, + KeyringControllerWithKeyringV2UnsafeAction, } from '@metamask/keyring-controller'; import type { Messenger } from '@metamask/messenger'; import type { + SnapControllerGetRunnableSnapsAction, SnapControllerGetStateAction, + SnapControllerSnapBlockedEvent, + SnapControllerSnapDisabledEvent, + SnapControllerSnapEnabledEvent, + SnapControllerSnapInstalledEvent, + SnapControllerSnapUninstalledEvent, SnapControllerStateChangeEvent, } from '@metamask/snaps-controllers'; +import type { Json } from '@metamask/snaps-sdk'; import { SnapId } from '@metamask/snaps-sdk'; +import type { TruncatedSnap } from '@metamask/snaps-utils'; -import type { SnapAccountServiceEnsureReadyAction } from './SnapAccountService-method-action-types'; +import { projectLogger as log } from './logger'; +import type { + SnapAccountServiceEnsureReadyAction, + SnapAccountServiceGetSnapsAction, + SnapAccountServiceHandleKeyringSnapMessageAction, +} from './SnapAccountService-method-action-types'; import { SnapPlatformWatcher } from './SnapPlatformWatcher'; import type { SnapPlatformWatcherConfig } from './SnapPlatformWatcher'; +/** + * Checks if a given Snap is an account management Snap. + * + * @param snap - The Snap to check. + * @returns True if the Snap declares the `endowment:keyring` initial + * permission. + */ +function isAccountManagementSnap(snap: TruncatedSnap): boolean { + return snap.initialPermissions['endowment:keyring'] !== undefined; +} + +/** + * 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 isSnapKeyring(keyring: Keyring): keyring is SnapKeyring { + // Using `KeyringType.Snap` (used for v2). + return keyring.type === KeyringType.Snap; +} + /** * The name of the {@link SnapAccountService}, used to namespace the service's * actions and events. @@ -23,19 +72,29 @@ export const serviceName = 'SnapAccountService'; * All of the methods within {@link SnapAccountService} that are exposed via * the messenger. */ -const MESSENGER_EXPOSED_METHODS = ['ensureReady'] as const; +const MESSENGER_EXPOSED_METHODS = [ + 'ensureReady', + 'getSnaps', + 'handleKeyringSnapMessage', +] as const; /** * Actions that {@link SnapAccountService} exposes to other consumers. */ -export type SnapAccountServiceActions = SnapAccountServiceEnsureReadyAction; +export type SnapAccountServiceActions = + | SnapAccountServiceEnsureReadyAction + | SnapAccountServiceGetSnapsAction + | SnapAccountServiceHandleKeyringSnapMessageAction; /** * Actions from other messengers that {@link SnapAccountService} calls. */ type AllowedActions = | SnapControllerGetStateAction - | KeyringControllerGetStateAction; + | SnapControllerGetRunnableSnapsAction + | KeyringControllerGetStateAction + | KeyringControllerWithControllerAction + | KeyringControllerWithKeyringV2UnsafeAction; /** * Events that {@link SnapAccountService} exposes to other consumers. @@ -47,6 +106,11 @@ export type SnapAccountServiceEvents = never; */ type AllowedEvents = | SnapControllerStateChangeEvent + | SnapControllerSnapInstalledEvent + | SnapControllerSnapEnabledEvent + | SnapControllerSnapDisabledEvent + | SnapControllerSnapBlockedEvent + | SnapControllerSnapUninstalledEvent | KeyringControllerStateChangeEvent; /** @@ -87,6 +151,12 @@ export class SnapAccountService { readonly #watcher: SnapPlatformWatcher; + readonly #snaps: Set = new Set(); + + #initialized = false; + + #migratePromise: Promise | null = null; + /** * Constructs a new {@link SnapAccountService}. * @@ -102,6 +172,22 @@ export class SnapAccountService { config?.snapPlatformWatcher, ); + this.#messenger.subscribe('SnapController:snapInstalled', (snap) => + this.#handleSnapAdded(snap, 'installed'), + ); + this.#messenger.subscribe('SnapController:snapEnabled', (snap) => + this.#handleSnapAdded(snap, 'enabled'), + ); + this.#messenger.subscribe('SnapController:snapDisabled', (snap) => + this.#handleSnapRemoved(snap.id, 'disabled'), + ); + this.#messenger.subscribe('SnapController:snapBlocked', (snapId) => + this.#handleSnapRemoved(snapId as SnapId, 'blocked'), + ); + this.#messenger.subscribe('SnapController:snapUninstalled', (snap) => + this.#handleSnapRemoved(snap.id, 'uninstalled'), + ); + this.#messenger.registerMethodActionHandlers( this, MESSENGER_EXPOSED_METHODS, @@ -110,22 +196,291 @@ export class SnapAccountService { /** * Initializes the snap account service. + * + * Seeds the internal set of account-management Snaps from + * `SnapController:getRunnableSnaps`, runs the legacy → v2 Snap keyring + * migration, then starts processing lifecycle events. */ async init(): Promise { - // TODO: Add initialization logic here. + const runnable = this.#messenger.call('SnapController:getRunnableSnaps'); + for (const snap of runnable) { + if (isAccountManagementSnap(snap) && !this.#snaps.has(snap.id)) { + log(`Found account management Snap: ${snap.id} (initialization)`); + this.#snaps.add(snap.id); + } + } + + await this.migrate(); + + this.#initialized = true; + } + + /** + * Migrate the old (big) legacy Snap keyring to the new (per-snap) Snap + * keyring v2. Safe to call concurrently — the migration runs only once; + * all callers await the same promise. + * + * @returns A promise that resolves when the migration is complete. + */ + async migrate(): Promise { + if (!this.#migratePromise) { + this.#migratePromise = this.#migrate(); + } + return await this.#migratePromise; + } + + async #migrate(): Promise { + log('Migration started...'); + + await this.#messenger.call( + 'KeyringController:withController', + async (controller) => { + const { keyrings } = controller; + + const legacySnapKeyringEntry = keyrings.find( + ({ keyring }) => keyring.type === SNAP_KEYRING_TYPE, + ); + if (!legacySnapKeyringEntry) { + log('No legacy Snap keyring found. Migration not required.'); + return; + } + + // The legacy Snap keyring has never been a true `EthKeyring` so we + // need to cast it to `unknown` first. + const legacySnapKeyring = + legacySnapKeyringEntry.keyring as unknown as LegacySnapKeyring; + + // Compute the account list for each Snap, grouped by snap ID. + const states = new Map(); + for (const internalAccount of legacySnapKeyring.listAccounts()) { + // Convert `InternalAccount` to `KeyringAccount` since the Snap + // keyring (v2) expects accounts in that format and will verify it + // with `superstruct` when adding the keyring. + const { metadata, ...account } = internalAccount; + + const snap = metadata?.snap; + if (snap) { + const snapId = snap.id as SnapId; + + let state = states.get(snapId); + if (!state) { + state = { snapId, accounts: {} }; + states.set(snapId, state); + } + state.accounts[account.id] = account; + } + } + + // Create the new Snap keyring (v2) for each Snap and migrate the + // accounts over. + for (const state of states.values()) { + log(`Migrating accounts for Snap "${state.snapId}"...`); + await controller.addNewKeyring( + // IMPORTANT: The Snap keyring (v2) can also be used as a v1 + // keyring. So the builder associated with the v2 keyring type is + // able to build both v1 and v2 keyrings. + KeyringType.Snap, + state, + ); + } + + // Remove the legacy Snap keyring after migration. + log('Removing legacy Snap keyring...'); + await controller.removeKeyring(legacySnapKeyringEntry.metadata.id); + }, + ); + + log('Migration completed!'); + } + + /** + * Creates a Snap keyring for the given Snap if it doesn't exist yet using + * a safe "get or create" pattern. Safe to call concurrently. + * + * @param snapId - The Snap ID to create the keyring for. + */ + async #createKeyringForSnap(snapId: SnapId): Promise { + await this.#messenger.call( + 'KeyringController:withController', + async (controller) => { + const hasKeyring = controller.keyrings.some( + ({ keyringV2 }) => + keyringV2 && + isSnapKeyring(keyringV2) && + keyringV2.snapId === snapId, + ); + + if (!hasKeyring) { + log(`Creating v2 keyring for Snap "${snapId}"...`); + await controller.addNewKeyring(KeyringType.Snap, { + snapId, + accounts: {}, + }); + } + }, + ); + } + + /** + * Returns the IDs of all currently tracked account-management Snaps — + * Snaps that are installed, enabled, not blocked, and have the + * `endowment:keyring` permission. + * + * @returns The IDs of tracked account-management Snaps. + */ + getSnaps(): SnapId[] { + return [...this.#snaps]; } /** * Ensures everything is ready to use Snap accounts for the given Snap. - * 1. Waits for the Snap platform to be fully started. + * 1. Validates that `snapId` is a tracked account-management Snap. + * 2. Runs the legacy -> v2 Snap keyring migration (cached — no-op if + * already done). + * 3. Atomically creates the v2 keyring for this Snap if it doesn't exist + * yet. + * 4. Waits for the Snap platform to be fully started. * * Safe to call concurrently — each step is idempotent or mutex-protected. * - * @param _snapId - ID of the Snap to ensure readiness for. + * @param snapId - ID of the Snap to ensure readiness for. + * @throws If `snapId` is not a tracked account-management Snap. */ - async ensureReady(_snapId: SnapId): Promise { - // Lastly, before doing anything with our Snap, we need to make sure the - // platform is ready to process requests. + async ensureReady(snapId: SnapId): Promise { + if (!this.#snaps.has(snapId)) { + throw new Error(`Unknown snap: "${snapId}"`); + } + // Migrate from the global v1 Snap keyring to the per-Snap v2 keyring + // before doing anything else. + await this.migrate(); + + // We still try to create the keyring for the Snap here, since we might + // want to use a new Snap that never had accounts before. + await this.#createKeyringForSnap(snapId); + + // Before doing anything with our Snap, we need to make sure the platform + // is ready to process requests. await this.#watcher.ensureCanUseSnapPlatform(); } + + /** + * Handle a message from a Snap. + * + * Only `AccountCreated` triggers lazy keyring creation, since that is the + * single entry point for the v1 event-driven flow. All other events from + * unknown Snaps throw an error. + * + * @param snapId - ID of the Snap. + * @param message - Message sent by the Snap. + * @returns The execution result. + */ + async handleKeyringSnapMessage( + snapId: SnapId, + message: SnapMessage, + ): Promise { + // We assume the Snap platform always sends a valid `KeyringEvent` here. + const event = message.method as KeyringEvent; + + if (message.method === 'getSelectedAccounts') { + return []; + } + + const isSnapKeyringForThisSnap = ( + keyring: Keyring, + ): keyring is SnapKeyring => + isSnapKeyring(keyring) && keyring.snapId === snapId; + + // We can create a new keyring if the message is an AccountCreated event. + const isAccountCreatedMessage = event === KeyringEvent.AccountCreated; + + // Create the Snap keyring if it doesn't exist yet (in an atomic way). We + // cannot assume the keyring exists (e.g for the MMI Snap). + // NOTE: We only auto-create it for v1 account creation flows. + if (isAccountCreatedMessage) { + await this.#messenger.call( + 'KeyringController:withController', + async (controller) => { + const hasSnapKeyring = controller.keyrings.some( + ({ keyringV2 }) => keyringV2 && isSnapKeyringForThisSnap(keyringV2), + ); + + if (!hasSnapKeyring) { + log(`Auto-creating Snap keyring for "${snapId}"...`); + + await controller.addNewKeyring(KeyringType.Snap, { + snapId, + accounts: {}, + }); + + log(`Snap keyring for "${snapId}" is ready!`); + } + }, + ); + } else { + log( + `No Snap keyring found for snap "${snapId}". Cannot handle message with method "${event}".`, + ); + + throw new Error( + 'Cannot delegate keyring Snap message, keyring does not exist yet.', + ); + } + + // This part of the flow relies on v1 flows, so we have to go with + // "unsafe" to avoid deadlocks. The keyring persistence will be done in a + // later stage of `handleKeyringSnapMessage` after the message is handled, + // so we don't have to worry about that here. + const result = await this.#messenger.call( + 'KeyringController:withKeyringV2Unsafe', + { filter: (keyring) => isSnapKeyringForThisSnap(keyring) }, + async ({ keyring }) => { + const snapKeyring = keyring as SnapKeyring; + return snapKeyring.handleKeyringSnapMessage(message); + }, + ); + + log( + `Snap keyring for "${snapId}" handled keyring message "${event}" successfully!`, + ); + + return result as Json; + } + + /** + * Handles a Snap being added (installed or enabled). If the Snap is an + * account-management Snap, adds it to the internal set of tracked Snaps. + * + * @param snap - The Snap that was installed or enabled. + * @param reason - The reason the Snap was added. + */ + #handleSnapAdded(snap: TruncatedSnap, reason: string): void { + if (!this.#initialized) { + return; + } + + if (isAccountManagementSnap(snap) && !this.#snaps.has(snap.id)) { + log(`Added account management Snap: ${snap.id} (${reason})`); + + this.#snaps.add(snap.id); + } + } + + /** + * Handles a Snap being removed (disabled, blocked, or uninstalled). If the Snap is an + * account-management Snap, removes it from the internal set of tracked Snaps. + * + * @param snapId - The Snap ID that was disabled, blocked, or uninstalled. + * @param reason - The reason the Snap was removed. + */ + #handleSnapRemoved(snapId: SnapId, reason: string): void { + if (!this.#initialized) { + return; + } + + if (this.#snaps.has(snapId)) { + log(`Removed account management Snap: ${snapId} (${reason})`); + + this.#snaps.delete(snapId); + } + } } diff --git a/packages/snap-account-service/src/index.ts b/packages/snap-account-service/src/index.ts index 557b5fdf99..8f8022612a 100644 --- a/packages/snap-account-service/src/index.ts +++ b/packages/snap-account-service/src/index.ts @@ -6,6 +6,10 @@ export type { SnapAccountServiceMessenger, SnapAccountServiceOptions, } from './SnapAccountService'; -export type { SnapAccountServiceEnsureReadyAction } from './SnapAccountService-method-action-types'; +export type { + SnapAccountServiceEnsureReadyAction, + SnapAccountServiceGetSnapsAction, + SnapAccountServiceHandleKeyringSnapMessageAction, +} from './SnapAccountService-method-action-types'; export { SnapPlatformWatcher } from './SnapPlatformWatcher'; export type { SnapPlatformWatcherConfig } from './SnapPlatformWatcher'; diff --git a/yarn.lock b/yarn.lock index 0a955bf5f9..731ccba3d7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5432,6 +5432,8 @@ __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-api": "npm:^23.1.0" "@metamask/keyring-controller": "npm:^25.5.0" "@metamask/messenger": "npm:^1.2.0" "@metamask/snaps-controllers": "npm:^19.0.0"