refactor!: HdKeyring implements Keyring

packages/keyring-eth-hd/CHANGELOG.md

@@ -10,6 +10,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - **BREAKING:** The `HdKeyring` is not exported as default anymore ([#161](https://github.com/MetaMask/accounts/pull/161))
+- **BREAKING:** The method signature for `signTypedData` has been changed ([#224](https://github.com/MetaMask/accounts/pull/224))
+  - The method now accepts a `TypedDataV1` object when `SigntypedDataVersion.V1` is passed in the options, and `TypedMessage<Types>` when other versions are requested
+- **BREAKING:** The `HdKeyring` class now extends `Keyring` from `@metamask/keyring-utils`
+  - The `deserialize` method does not accept `Buffer` mnemonic anymore
 
 ## [10.0.1]
 

packages/keyring-eth-hd/package.json

@@ -56,6 +56,7 @@
     "@lavamoat/preinstall-always-fail": "^2.1.0",
     "@metamask/auto-changelog": "^3.4.4",
     "@metamask/bip39": "^4.0.0",
+    "@metamask/keyring-utils": "workspace:^",
     "@ts-bridge/cli": "^0.6.3",
     "@types/jest": "^29.5.12",
     "deepmerge": "^4.2.2",

packages/keyring-eth-hd/src/hd-keyring.test.ts

@@ -21,7 +21,6 @@ import {
   type MessageTypes,
   type EIP7702Authorization,
 } from '@metamask/eth-sig-util';
-import { wordlist } from '@metamask/scure-bip39/dist/wordlists/english';
 import { assert, type Hex } from '@metamask/utils';
 import { webcrypto } from 'crypto';
 import { keccak256 } from 'ethereum-cryptography/keccak';
@@ -41,8 +40,11 @@ const secondAcct = '0x1b00aed43a693f3a957f9feb5cc08afa031e37a0';
 
 const notKeyringAddress = '0xbD20F6F5F1616947a39E11926E78ec94817B3931';
 
-const getAddressAtIndex = (keyring: HdKeyring, index: number): Hex => {
-  const accounts = keyring.getAccounts();
+const getAddressAtIndex = async (
+  keyring: HdKeyring,
+  index: number,
+): Promise<Hex> => {
+  const accounts = await keyring.getAccounts();
   assert(accounts[index], `Account not found at index ${index}`);
   return accounts[index];
 };
@@ -64,14 +66,14 @@ describe('hd-keyring', () => {
           mnemonics.map(async (mnemonic) => {
             const newHDKeyring = new HdKeyring();
             await newHDKeyring.deserialize({
-              mnemonic,
+              mnemonic: mnemonic.toJSON(),
               numberOfAccounts: 3,
             });
             const oldHDKeyring = new OldHdKeyring({
               mnemonic,
               numberOfAccounts: 3,
             });
-            const newAccounts = newHDKeyring.getAccounts();
+            const newAccounts = await newHDKeyring.getAccounts();
             const oldAccounts = await oldHDKeyring.getAccounts();
             expect(newAccounts[0]).toStrictEqual(oldAccounts[0]);
 
@@ -90,7 +92,7 @@ describe('hd-keyring', () => {
       'Eth-Hd-Keyring: Secret recovery phrase already provided';
     it('double generateRandomMnemonic', async () => {
       const keyring = new HdKeyring();
-      await keyring.deserialize();
+      await keyring.deserialize({});
       await keyring.generateRandomMnemonic();
       await expect(keyring.generateRandomMnemonic()).rejects.toThrow(
         alreadyProvidedError,
@@ -174,39 +176,20 @@ describe('hd-keyring', () => {
         numberOfAccounts: 2,
       });
 
-      const accounts = keyring.getAccounts();
-      expect(accounts[0]).toStrictEqual(firstAcct);
-      expect(accounts[1]).toStrictEqual(secondAcct);
-    });
-
-    it('deserializes with a typeof buffer mnemonic', async () => {
-      const keyring = new HdKeyring();
-
-      await keyring.deserialize({
-        mnemonic: Buffer.from(sampleMnemonic, 'utf8'),
-        numberOfAccounts: 2,
-      });
-
-      const accounts = keyring.getAccounts();
+      const accounts = await keyring.getAccounts();
       expect(accounts[0]).toStrictEqual(firstAcct);
       expect(accounts[1]).toStrictEqual(secondAcct);
     });
 
-    it('deserializes with a typeof Uint8Array mnemonic', async () => {
-      const indices = sampleMnemonic
-        .split(' ')
-        .map((word) => wordlist.indexOf(word));
-      const uInt8ArrayOfMnemonic = new Uint8Array(
-        new Uint16Array(indices).buffer,
-      );
+    it('deserializes with a typeof serialized buffer mnemonic', async () => {
       const keyring = new HdKeyring();
 
       await keyring.deserialize({
-        mnemonic: uInt8ArrayOfMnemonic,
+        mnemonic: Buffer.from(sampleMnemonic, 'utf8').toJSON(),
         numberOfAccounts: 2,
       });
 
-      const accounts = keyring.getAccounts();
+      const accounts = await keyring.getAccounts();
       expect(accounts[0]).toStrictEqual(firstAcct);
       expect(accounts[1]).toStrictEqual(secondAcct);
     });
@@ -250,7 +233,7 @@ describe('hd-keyring', () => {
         numberOfAccounts: 2,
       });
 
-      const accounts = keyring.getAccounts();
+      const accounts = await keyring.getAccounts();
       expect(accounts[0]).toStrictEqual(firstAcct);
       expect(accounts[1]).toStrictEqual(secondAcct);
       expect(cryptographicFunctions.pbkdf2Sha512).toHaveBeenCalledTimes(1);
@@ -287,11 +270,11 @@ describe('hd-keyring', () => {
         mnemonic: sampleMnemonic,
         numberOfAccounts: 1,
       });
-      const accountsFirstCheck = keyring.getAccounts();
+      const accountsFirstCheck = await keyring.getAccounts();
 
       expect(accountsFirstCheck).toHaveLength(1);
       await keyring.addAccounts(1);
-      const accountsSecondCheck = keyring.getAccounts();
+      const accountsSecondCheck = await keyring.getAccounts();
       expect(accountsSecondCheck[0]).toStrictEqual(firstAcct);
       expect(accountsSecondCheck[1]).toStrictEqual(secondAcct);
       expect(accountsSecondCheck).toHaveLength(2);
@@ -306,10 +289,10 @@ describe('hd-keyring', () => {
     describe('with no arguments', () => {
       it('creates a single wallet', async () => {
         const keyring = new HdKeyring();
-        await keyring.deserialize();
+        await keyring.deserialize({});
         await keyring.generateRandomMnemonic();
         await keyring.addAccounts();
-        const accounts = keyring.getAccounts();
+        const accounts = await keyring.getAccounts();
         expect(accounts).toHaveLength(1);
       });
 
@@ -324,10 +307,10 @@ describe('hd-keyring', () => {
     describe('with a numeric argument', () => {
       it('creates that number of wallets', async () => {
         const keyring = new HdKeyring();
-        await keyring.deserialize();
+        await keyring.deserialize({});
         await keyring.generateRandomMnemonic();
         await keyring.addAccounts(3);
-        const accounts = keyring.getAccounts();
+        const accounts = await keyring.getAccounts();
         expect(accounts).toHaveLength(3);
       });
     });
@@ -367,10 +350,10 @@ describe('hd-keyring', () => {
           value: 'Hi, Alice!',
         },
       ];
-      await keyring.deserialize();
+      await keyring.deserialize({});
       await keyring.generateRandomMnemonic();
       await keyring.addAccounts(1);
-      const address = getAddressAtIndex(keyring, 0);
+      const address = await getAddressAtIndex(keyring, 0);
       const signature = await keyring.signTypedData(address, typedData);
       const restored = recoverTypedSignature({
         data: typedData,
@@ -392,10 +375,10 @@ describe('hd-keyring', () => {
 
     it('signs in a compliant and recoverable way', async () => {
       const keyring = new HdKeyring();
-      await keyring.deserialize();
+      await keyring.deserialize({});
       await keyring.generateRandomMnemonic();
       await keyring.addAccounts(1);
-      const address = getAddressAtIndex(keyring, 0);
+      const address = await getAddressAtIndex(keyring, 0);
       const signature = await keyring.signTypedData(address, typedData, {
         version: SignTypedDataVersion.V1,
       });
@@ -424,7 +407,7 @@ describe('hd-keyring', () => {
         mnemonic: sampleMnemonic,
         numberOfAccounts: 1,
       });
-      const address = getAddressAtIndex(keyring, 0);
+      const address = await getAddressAtIndex(keyring, 0);
       const signature = await keyring.signTypedData(address, typedData, {
         version: SignTypedDataVersion.V3,
       });
@@ -478,10 +461,10 @@ describe('hd-keyring', () => {
         },
       };
 
-      await keyring.deserialize();
+      await keyring.deserialize({});
       await keyring.generateRandomMnemonic();
       await keyring.addAccounts(1);
-      const address = getAddressAtIndex(keyring, 0);
+      const address = await getAddressAtIndex(keyring, 0);
       const signature = await keyring.signTypedData(address, typedData, {
         version: SignTypedDataVersion.V3,
       });
@@ -503,7 +486,7 @@ describe('hd-keyring', () => {
         numberOfAccounts: 1,
         hdPath: hdPathString,
       });
-      const addresses = keyring.getAccounts();
+      const addresses = await keyring.getAccounts();
       expect(addresses[0]).toStrictEqual(firstAcct);
       const serialized = await keyring.serialize();
       expect(serialized.hdPath).toStrictEqual(hdPathString);
@@ -517,7 +500,7 @@ describe('hd-keyring', () => {
         numberOfAccounts: 1,
         hdPath: hdPathString,
       });
-      const addresses = keyring.getAccounts();
+      const addresses = await keyring.getAccounts();
       expect(addresses[0]).not.toBe(firstAcct);
       const serialized = await keyring.serialize();
       expect(serialized.hdPath).toStrictEqual(hdPathString);
@@ -646,7 +629,7 @@ describe('hd-keyring', () => {
         Buffer.from(keccak256(Buffer.from(localMessage))),
       );
       await keyring.addAccounts(9);
-      const addresses = keyring.getAccounts();
+      const addresses = await keyring.getAccounts();
       const signatures = await Promise.all(
         addresses.map(async (accountAddress) => {
           return await keyring.signMessage(accountAddress, msgHashHex);
@@ -773,10 +756,10 @@ describe('hd-keyring', () => {
 
     describe('if the account exists', function () {
       it('should remove that account', async function () {
-        const addresses = keyring.getAccounts();
+        const addresses = await keyring.getAccounts();
         expect(addresses).toHaveLength(1);
-        keyring.removeAccount(getAddressAtIndex(keyring, 0));
-        const addressesAfterRemoval = keyring.getAccounts();
+        keyring.removeAccount(await getAddressAtIndex(keyring, 0));
+        const addressesAfterRemoval = await keyring.getAccounts();
         expect(addressesAfterRemoval).toHaveLength(0);
       });
     });
@@ -985,7 +968,7 @@ describe('hd-keyring', () => {
         },
       };
 
-      const address = getAddressAtIndex(keyring, 0);
+      const address = await getAddressAtIndex(keyring, 0);
 
       const signature = await keyring.signTypedData(address, typedData, {
         version: SignTypedDataVersion.V4,

packages/keyring-eth-hd/src/hd-keyring.ts

@@ -25,6 +25,7 @@ import {
   type CryptographicFunctions,
   mnemonicToSeed,
 } from '@metamask/key-tree';
+import type { Keyring } from '@metamask/keyring-utils';
 import { generateMnemonic } from '@metamask/scure-bip39';
 import { wordlist } from '@metamask/scure-bip39/dist/wordlists/english';
 import {
@@ -53,12 +54,27 @@ export type HDKeyringOptions = {
  * @property numberOfAccounts - The number of accounts in the keyring.
  * @property hdPath - The HD path used to derive accounts.
  */
-export type HDKeyringState = {
-  mnemonic: number[] | Uint8Array | Buffer | string;
+export type SerializedHDKeyringState = {
+  mnemonic: number[];
   numberOfAccounts: number;
   hdPath: string;
 };
 
+/**
+ * An object that can be passed to the Keyring.deserialize method to initialize
+ * an `HDKeyring` instance.
+ *
+ * @property mnemonic - The mnemonic seed phrase as an array of numbers.
+ * @property numberOfAccounts - The number of accounts in the keyring.
+ * @property hdPath - The HD path used to derive accounts.
+ */
+export type DeserializableHDKeyringState = Omit<
+  SerializedHDKeyringState,
+  'mnemonic'
+> & {
+  mnemonic: number[] | SerializedBuffer | string;
+};
+
 /**
  * Options for selecting an account from an `HDKeyring` instance.
  *
@@ -88,7 +104,7 @@ function isSerializedBuffer(value: unknown): value is SerializedBuffer {
   );
 }
 
-export class HdKeyring {
+export class HdKeyring implements Keyring {
   static type: string = type;
 
   type: string = type;
@@ -124,7 +140,7 @@ export class HdKeyring {
    *
    * @returns The serialized state of the keyring.
    */
-  async serialize(): Promise<HDKeyringState> {
+  async serialize(): Promise<SerializedHDKeyringState> {
     let mnemonic: number[] = [];
 
     if (this.mnemonic) {
@@ -145,7 +161,9 @@ export class HdKeyring {
    * @param opts - The serialized state of the keyring.
    * @returns An empty array.
    */
-  async deserialize(opts: Partial<HDKeyringState> = {}): Promise<string[]> {
+  async deserialize(
+    opts: Partial<DeserializableHDKeyringState>,
+  ): Promise<void> {
     if (opts.numberOfAccounts && !opts.mnemonic) {
       throw new Error(
         'Eth-Hd-Keyring: Deserialize method cannot be called with an opts value for numberOfAccounts and no menmonic',
@@ -167,10 +185,8 @@ export class HdKeyring {
     }
 
     if (opts.numberOfAccounts) {
-      return this.addAccounts(opts.numberOfAccounts);
+      await this.addAccounts(opts.numberOfAccounts);
     }
-
-    return [];
   }
 
   /**
@@ -204,7 +220,7 @@ export class HdKeyring {
    *
    * @returns The addresses of all accounts in the keyring.
    */
-  getAccounts(): Hex[] {
+  async getAccounts(): Promise<Hex[]> {
     return this.#wallets.map((wallet) => {
       assert(wallet.publicKey, 'Expected public key to be set');
       return this.#addressfromPublicKey(wallet.publicKey);
@@ -349,16 +365,19 @@ export class HdKeyring {
    * @param opts - The options for signing the message.
    * @returns The signature of the message.
    */
-  async signTypedData<Types extends MessageTypes>(
+  async signTypedData<
+    Version extends SignTypedDataVersion,
+    Types extends MessageTypes,
+    Options extends { version: Version },
+  >(
     withAccount: Hex,
-    typedData: TypedDataV1 | TypedMessage<Types>,
-    opts: HDKeyringAccountSelectionOptions & {
-      version: SignTypedDataVersion;
-    } = { version: SignTypedDataVersion.V1 },
+    typedData: Version extends 'V1' ? TypedDataV1 : TypedMessage<Types>,
+    opts?: HDKeyringAccountSelectionOptions & Options,
   ): Promise<string> {
+    const options = opts ?? { version: SignTypedDataVersion.V1 };
     // Treat invalid versions as "V1"
-    const version = Object.keys(SignTypedDataVersion).includes(opts.version)
-      ? opts.version
+    const version = Object.keys(SignTypedDataVersion).includes(options.version)
+      ? options.version
       : SignTypedDataVersion.V1;
 
     const privateKey = this.#getPrivateKeyFor(withAccount, opts);
@@ -584,7 +603,7 @@ export class HdKeyring {
    * passed as type buffer or array of UTF-8 bytes must be NFKD normalized.
    */
   async #initFromMnemonic(
-    mnemonic: string | number[] | Buffer | Uint8Array,
+    mnemonic: string | number[] | SerializedBuffer | Buffer | Uint8Array,
   ): Promise<void> {
     if (this.root) {
       throw new Error(

packages/keyring-eth-hd/src/index.ts

@@ -1,6 +1,7 @@
 export { HdKeyring } from './hd-keyring';
 export type {
-  HDKeyringState,
+  SerializedHDKeyringState,
+  DeserializableHDKeyringState,
   HDKeyringOptions,
   HDKeyringAccountSelectionOptions,
 } from './hd-keyring';