feat(accounts-controller): add support for Snap keyring v2

packages/accounts-controller/package.json

@@ -44,8 +44,8 @@
     "build:docs": "typedoc",
     "changelog:update": "../../scripts/update-changelog.sh @metamask/accounts-controller",
     "changelog:validate": "../../scripts/validate-changelog.sh @metamask/accounts-controller",
-    "messenger-action-types:check": "tsx ../../packages/messenger-cli/src/cli.ts --check",
-    "messenger-action-types:generate": "tsx ../../packages/messenger-cli/src/cli.ts --generate",
+    "messenger-action-types:check": "tsx ../../packages/messenger-cli/src/cli.ts --formatter oxfmt --check",
+    "messenger-action-types:generate": "tsx ../../packages/messenger-cli/src/cli.ts --formatter oxfmt --generate",
     "since-latest-release": "../../scripts/since-latest-release.sh",
     "test": "NODE_OPTIONS=--experimental-vm-modules jest --reporters=jest-silent-reporter",
     "test:clean": "NODE_OPTIONS=--experimental-vm-modules jest --clearCache",

packages/accounts-controller/src/AccountsController.ts

@@ -4,12 +4,13 @@ import type {
   ControllerStateChangeEvent,
 } from '@metamask/base-controller';
 import { SnapKeyring } from '@metamask/eth-snap-keyring';
+import { SnapKeyring as SnapKeyringV2 } from '@metamask/eth-snap-keyring/v2';
 import type {
   SnapKeyringAccountAssetListUpdatedEvent,
   SnapKeyringAccountBalancesUpdatedEvent,
   SnapKeyringAccountTransactionsUpdatedEvent,
 } from '@metamask/eth-snap-keyring';
-import type { KeyringAccountEntropyOptions } from '@metamask/keyring-api';
+import type { KeyringAccount, KeyringAccountEntropyOptions } from '@metamask/keyring-api';
 import {
   EthAccountType,
   EthMethod,
@@ -23,6 +24,7 @@ import type {
   KeyringControllerStateChangeEvent,
   KeyringControllerGetStateAction,
   KeyringObject,
+  KeyringMetadata,
 } from '@metamask/keyring-controller';
 import type { InternalAccount } from '@metamask/keyring-internal-api';
 import { isScopeEqualToAny } from '@metamask/keyring-utils';
@@ -52,8 +54,11 @@ import {
   isHdSnapKeyringAccount,
   isMoneyKeyringType,
   isSnapKeyringType,
+  isSnapKeyringV2Type,
   keyringTypeToName,
 } from './utils';
+import { KeyringType } from '@metamask/keyring-api/v2';
+import { is } from '@metamask/superstruct';
 
 const controllerName = 'AccountsController';
 
@@ -847,6 +852,49 @@ export class AccountsController extends BaseController<
     return snapKeyring as SnapKeyring | undefined;
   }
 
+  /**
+   * Get an account from a Snap keyring v2.
+   *
+   * @param address - The address of the account to retrieve.
+   * @returns The Snap account if available.
+   */
+  #getAccountFromSnapKeyringV2(address: string): InternalAccount | undefined {
+    const keyrings = this.messenger.call(
+      'KeyringController:getKeyringsByType',
+      KeyringType.Snap,
+    );
+
+    // Snap keyring v2 are "per-Snaps", so we need to iterate over all of them to find the account.
+    for (const keyring of keyrings) {
+      if (keyring instanceof SnapKeyringV2) {
+        // We use the synchronous method here since this method is used during `:stateChange` that are
+        // use synchronous handlers.
+        const account = keyring.lookupByAddress(address);
+        if (account) {
+          return {
+            ...account,
+            // We still have to use internal account for now, so we inject some metadata.
+            metadata: {
+              name: '',
+              importTime: Date.now(),
+              lastSelected: 0,
+              keyring: {
+                type: KeyringType.Snap,
+              },
+              snap: {
+                name: keyring.snapId,
+                enabled: true,
+                id: keyring.snapId,
+              },
+            },
+          };
+        }
+      }
+    }
+
+    return undefined;
+  }
+
   /**
    * Re-publish an account event.
    *
@@ -887,32 +935,16 @@ export class AccountsController extends BaseController<
     log('Synchronizing accounts with keyrings (through :stateChange)...');
 
     // State patches.
-    const generatePatch = (): StatePatch => {
-      return {
-        previous: {},
-        added: [],
-        updated: [],
-        removed: [],
-      };
-    };
-    const patches = {
-      snap: generatePatch(),
-      normal: generatePatch(),
-    };
-
-    // Gets the patch object based on the keyring type (since Snap accounts and other accounts
-    // are handled differently).
-    const patchOf = (type: string): StatePatch => {
-      if (isSnapKeyringType(type)) {
-        return patches.snap;
-      }
-      return patches.normal;
+    const patch: StatePatch = {
+      previous: {},
+      added: [],
+      updated: [],
+      removed: [],
     };
 
     // Create a map (with lower-cased addresses) of all existing accounts.
     for (const account of this.listMultichainAccounts()) {
       const address = account.address.toLowerCase();
-      const patch = patchOf(account.metadata.keyring.type);
 
       patch.previous[address] = account;
     }
@@ -926,8 +958,6 @@ export class AccountsController extends BaseController<
         continue;
       }
 
-      const patch = patchOf(keyring.type);
-
       for (const accountAddress of keyring.accounts) {
         // Lower-case address to use it in the `previous` map.
         const address = accountAddress.toLowerCase();
@@ -951,12 +981,10 @@ export class AccountsController extends BaseController<
 
     // We might have accounts associated with removed keyrings, so we iterate
     // over all previous known accounts and check against the keyring addresses.
-    for (const patch of [patches.snap, patches.normal]) {
-      for (const [address, account] of Object.entries(patch.previous)) {
-        // If a previous address is not part of the new addesses, then it got removed.
-        if (!addresses.has(address)) {
-          patch.removed.push(account);
-        }
+    for (const [address, account] of Object.entries(patch.previous)) {
+      // If a previous address is not part of the new addesses, then it got removed.
+      if (!addresses.has(address)) {
+        patch.removed.push(account);
       }
     }
 
@@ -970,42 +998,40 @@ export class AccountsController extends BaseController<
       (state) => {
         const { internalAccounts, accountIdByAddress } = state;
 
-        for (const patch of [patches.snap, patches.normal]) {
-          for (const account of patch.removed) {
-            delete internalAccounts.accounts[account.id];
-            delete accountIdByAddress[account.address];
+        for (const account of patch.removed) {
+          delete internalAccounts.accounts[account.id];
+          delete accountIdByAddress[account.address];
 
-            diff.removed.push(account.id);
-          }
+          diff.removed.push(account.id);
+        }
+
+        for (const added of patch.added) {
+          const account = this.#getInternalAccountFromAddressAndType(
+            added.address,
+            added.keyring,
+          );
+
+          if (account) {
+            const accounts = Object.values(
+              internalAccounts.accounts,
+            ) as InternalAccount[];
+
+            // If it's the first account, we need to select it.
+            const lastSelected =
+              accounts.length === 0 ? this.#getLastSelectedIndex() : 0;
+
+            internalAccounts.accounts[account.id] = {
+              ...account,
+              metadata: {
+                ...account.metadata,
+                importTime: Date.now(),
+                lastSelected,
+              },
+            };
+
+            accountIdByAddress[account.address] = account.id;
 
-          for (const added of patch.added) {
-            const account = this.#getInternalAccountFromAddressAndType(
-              added.address,
-              added.keyring,
-            );
-
-            if (account) {
-              const accounts = Object.values(
-                internalAccounts.accounts,
-              ) as InternalAccount[];
-
-              // If it's the first account, we need to select it.
-              const lastSelected =
-                accounts.length === 0 ? this.#getLastSelectedIndex() : 0;
-
-              internalAccounts.accounts[account.id] = {
-                ...account,
-                metadata: {
-                  ...account.metadata,
-                  importTime: Date.now(),
-                  lastSelected,
-                },
-              };
-
-              accountIdByAddress[account.address] = account.id;
-
-              diff.added.push(internalAccounts.accounts[account.id]);
-            }
+            diff.added.push(internalAccounts.accounts[account.id]);
           }
         }
       },
@@ -1192,17 +1218,27 @@ export class AccountsController extends BaseController<
     address: string,
     keyring: KeyringObject,
   ): InternalAccount | undefined {
-    if (isSnapKeyringType(keyring.type)) {
-      const snapKeyring = this.#getSnapKeyring();
+    const isSnapKeyringV1 = isSnapKeyringType(keyring.type);
+    const isSnapKeyringV2 = isSnapKeyringV2Type(keyring.type);
+
+    if (isSnapKeyringV1 || isSnapKeyringV2) {
+      let account: InternalAccount | undefined;
+
+      if (isSnapKeyringV1) {
+        const snapKeyring = this.#getSnapKeyring();
+
+        // We need the Snap keyring to retrieve the account from its address.
+        if (!snapKeyring) {
+          return undefined;
+        }
 
-      // We need the Snap keyring to retrieve the account from its address.
-      if (!snapKeyring) {
-        return undefined;
+        // This might be undefined if the Snap deleted the account before
+        // reaching that point.
+        account = snapKeyring.getAccountByAddress(address);
+      } else {
+        account = this.#getAccountFromSnapKeyringV2(address);
       }
 
-      // This might be undefined if the Snap deleted the account before
-      // reaching that point.
-      let account = snapKeyring.getAccountByAddress(address);
       if (account) {
         // We force the copy here, to avoid mutating the reference returned by the Snap keyring.
         account = cloneDeep(account);

packages/accounts-controller/src/utils.ts

@@ -9,6 +9,7 @@ import type { V4Options } from 'uuid';
 import { v4 as uuid } from 'uuid';
 
 import type { AccountId } from './AccountsController';
+import { KeyringType } from '@metamask/keyring-api/v2';
 
 /**
  * Returns the name of the keyring type.
@@ -39,6 +40,7 @@ export function keyringTypeToName(keyringType: string): string {
     case KeyringTypes.qr: {
       return 'QR';
     }
+    case KeyringType.Snap:
     case KeyringTypes.snap: {
       return 'Snap Account';
     }
@@ -103,6 +105,16 @@ export function isSnapKeyringType(keyringType: KeyringTypes | string): boolean {
   return keyringType === (KeyringTypes.snap as string);
 }
 
+/**
+ * Check if a keyring type is a Snap keyring.
+ *
+ * @param keyringType - The account's keyring type.
+ * @returns True if the keyring type is considered a Snap keyring, false otherwise.
+ */
+export function isSnapKeyringV2Type(keyringType: KeyringTypes | KeyringType | string): boolean {
+  return keyringType === (KeyringType.Snap as string);
+}
+
 /**
  * Check if a keyring type is a simple keyring.
  *