refactor!: remove getLegacySnapKeyring (not usable after running the migration)

Author: ccharly

packages/snap-account-service/CHANGELOG.md

@@ -41,6 +41,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - Selected-account forwarding now targets v2 Snap keyrings.
   - The service messenger now requires the `KeyringController:withKeyringV2Unsafe`.
 
+### Removed
+
+- **BREAKING:** Removed `getLegacySnapKeyring` ([#8732](https://github.com/MetaMask/core/pull/8732))
+  - The legacy Snap keyring should not be used anymore after the migration has completed.
+
 ### Changed
 
 - Bump `@metamask/messenger` from `^1.1.1` to `^1.2.0` ([#8632](https://github.com/MetaMask/core/pull/8632))

packages/snap-account-service/src/SnapAccountService-method-action-types.ts

@@ -48,17 +48,6 @@ export type SnapAccountServiceMigrateAction = {
   handler: SnapAccountService['migrate'];
 };
 
-/**
- * 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'];
-};
-
 /**
  * Handle a message from a Snap.
  *
@@ -78,5 +67,4 @@ export type SnapAccountServiceMethodActions =
   | SnapAccountServiceGetSnapsAction
   | SnapAccountServiceEnsureReadyAction
   | SnapAccountServiceMigrateAction
-  | SnapAccountServiceGetLegacySnapKeyringAction
   | SnapAccountServiceHandleKeyringSnapMessageAction;

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

@@ -1,6 +1,6 @@
 import type { AccountGroupId } from '@metamask/account-api';
 import { SNAP_KEYRING_TYPE } from '@metamask/eth-snap-keyring';
-import type { SnapKeyring, SnapMessage } from '@metamask/eth-snap-keyring';
+import type { SnapMessage } from '@metamask/eth-snap-keyring';
 import type { SnapKeyring as SnapKeyringV2 } from '@metamask/eth-snap-keyring/v2';
 import { KeyringEvent } from '@metamask/keyring-api';
 import { KeyringType } from '@metamask/keyring-api/v2';
@@ -860,43 +860,6 @@ describe('SnapAccountService', () => {
     });
   });
 
-  describe('getLegacySnapKeyring', () => {
-    it('returns the existing Snap keyring when one is already present', async () => {
-      const { service, mocks } = await 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 } = await 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 } = await setup();
-      mocks.KeyringController.withController.mockImplementation(async () => {
-        throw new Error('boom');
-      });
-
-      await expect(service.getLegacySnapKeyring()).rejects.toThrow('boom');
-    });
-  });
-
   describe('handleKeyringSnapMessage', () => {
     const MOCK_MESSAGE = {
       method: KeyringEvent.AccountUpdated,

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

@@ -13,7 +13,6 @@ import type {
   KeyringControllerWithControllerAction,
   KeyringControllerWithKeyringV2Action,
   KeyringControllerWithKeyringV2UnsafeAction,
-  KeyringEntry,
 } from '@metamask/keyring-controller';
 import {
   isKeyringNotFoundError,
@@ -40,7 +39,6 @@ import type { Json } from '@metamask/utils';
 import { projectLogger as log } from './logger';
 import type {
   SnapAccountServiceEnsureReadyAction,
-  SnapAccountServiceGetLegacySnapKeyringAction,
   SnapAccountServiceGetSnapsAction,
   SnapAccountServiceHandleKeyringSnapMessageAction,
   SnapAccountServiceMigrateAction,
@@ -71,7 +69,6 @@ export const serviceName = 'SnapAccountService';
 const MESSENGER_EXPOSED_METHODS = [
   'ensureReady',
   'getSnaps',
-  'getLegacySnapKeyring',
   'handleKeyringSnapMessage',
   'migrate',
 ] as const;
@@ -82,7 +79,6 @@ const MESSENGER_EXPOSED_METHODS = [
 export type SnapAccountServiceActions =
   | SnapAccountServiceEnsureReadyAction
   | SnapAccountServiceGetSnapsAction
-  | SnapAccountServiceGetLegacySnapKeyringAction
   | SnapAccountServiceHandleKeyringSnapMessageAction
   | SnapAccountServiceMigrateAction;
 
@@ -535,53 +531,6 @@ export class SnapAccountService {
     );
   }
 
-  /**
-   * 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;
-  }
-
   /**
    * Handle a message from a Snap.
    *