feat(snap-account-service): add getLegacySnapKeyring

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
 

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",

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;

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<RestrictedController['addNewKeyring']> {
+      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');
+    });
+  });
 });

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,
@@ -17,8 +21,10 @@ import type {
 } from '@metamask/snaps-controllers';
 import { SnapId } from '@metamask/snaps-sdk';
 
+import { projectLogger as log } from './logger';
 import type {
   SnapAccountServiceEnsureReadyAction,
+  SnapAccountServiceGetLegacySnapKeyringAction,
   SnapAccountServiceGetSnapsAction,
 } from './SnapAccountService-method-action-types';
 import { SnapPlatformWatcher } from './SnapPlatformWatcher';
@@ -35,14 +41,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 +62,8 @@ type AllowedActions =
   | SnapControllerGetStateAction
   | SnapControllerGetSnapAction
   | SnapControllerGetRunnableSnapsAction
-  | KeyringControllerGetStateAction;
+  | KeyringControllerGetStateAction
+  | KeyringControllerWithControllerAction;
 
 /**
  * Events that {@link SnapAccountService} exposes to other consumers.
@@ -173,4 +185,51 @@ 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<SnapKeyring> {
+    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<Result> => {
+        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,
+            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.
+        return { snapKeyring } as unknown as Result;
+      },
+    );
+
+    return (result as Result).snapKeyring;
+  }
 }

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"