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

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,
@@ -42,6 +47,7 @@ type Mocks = {
   // eslint-disable-next-line @typescript-eslint/naming-convention
   KeyringController: {
     getState: jest.MockedFunction<() => { keyrings: { type: string }[] }>;
+    withController: jest.Mock;
   };
 };
 
@@ -75,6 +81,7 @@ function getMessenger(
       'SnapController:getSnap',
       'SnapController:getRunnableSnaps',
       'KeyringController:getState',
+      'KeyringController:withController',
     ],
     events: [
       'SnapController:stateChange',
@@ -140,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<RestrictedController['addNewKeyring']>;
+} {
+  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.
  *
@@ -178,6 +230,7 @@ function setup({
     },
     KeyringController: {
       getState: jest.fn().mockReturnValue({ keyrings }),
+      withController: jest.fn(),
     },
   };
 
@@ -193,6 +246,10 @@ function setup({
     'KeyringController:getState',
     mocks.KeyringController.getState,
   );
+  rootMessenger.registerActionHandler(
+    'KeyringController:withController',
+    mocks.KeyringController.withController,
+  );
 
   const service = new SnapAccountService({ messenger, config });
 
@@ -358,4 +415,41 @@ describe('SnapAccountService', () => {
       expect(resolved).toBe(true);
     });
   });
+
+  describe('getLegacySnapKeyring', () => {
+    it('returns the existing Snap keyring when one is already present', async () => {
+      const { service, mocks } = setup();
+      const existing = buildKeyringEntry(KeyringTypes.snap);
+      const { addNewKeyring } = mockWithController(mocks, [
+        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, mocks } = setup();
+      const { addNewKeyring } = mockWithController(mocks, [
+        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, mocks } = setup();
+      mocks.KeyringController.withController.mockImplementation(async () => {
+        throw new Error('boom');
+      });
+
+      await expect(service.getLegacySnapKeyring()).rejects.toThrow('boom');
+    });
+  });
 });

packages/snap-account-service/src/SnapAccountService.ts

@@ -1,7 +1,12 @@
+import type { SnapKeyring as LegacySnapKeyring } from '@metamask/eth-snap-keyring';
 import type {
   KeyringControllerGetStateAction,
   KeyringControllerStateChangeEvent,
+  KeyringControllerWithControllerAction,
+  KeyringEntry,
 } from '@metamask/keyring-controller';
+import { KeyringTypes } from '@metamask/keyring-controller';
+import type { BaseKeyring } from '@metamask/keyring-utils';
 import type { Messenger } from '@metamask/messenger';
 import type {
   SnapControllerGetRunnableSnapsAction,
@@ -17,8 +22,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 +42,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 +63,8 @@ type AllowedActions =
   | SnapControllerGetStateAction
   | SnapControllerGetSnapAction
   | SnapControllerGetRunnableSnapsAction
-  | KeyringControllerGetStateAction;
+  | KeyringControllerGetStateAction
+  | KeyringControllerWithControllerAction;
 
 /**
  * Events that {@link SnapAccountService} exposes to other consumers.
@@ -96,6 +109,19 @@ export type SnapAccountServiceOptions = {
   config?: SnapAccountServiceConfig;
 };
 
+/**
+ * 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: {
+  type: BaseKeyring['type'];
+}): keyring is LegacySnapKeyring {
+  return keyring.type === KeyringTypes.snap;
+}
+
 /**
  * Service responsible for managing account management snaps.
  */
@@ -173,4 +199,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<LegacySnapKeyring> {
+    type Result = {
+      snapKeyring: LegacySnapKeyring;
+    };
+
+    // `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 }) =>
+          isLegacySnapKeyring(keyring),
+        );
+        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,7 +5433,9 @@ __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/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"