From cfc72a6fc23050a5d95000714cb52f3e73d878bd Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Fri, 21 Feb 2025 15:07:08 +0100 Subject: [PATCH 1/8] fix: `Keyring.signTypedData` accepts types for V1 --- .../src/ledger-keyring.ts | 12 +++++++---- .../keyring-eth-simple/src/simple-keyring.ts | 21 +++++++++++++------ .../keyring-eth-trezor/src/trezor-keyring.ts | 10 ++++++--- packages/keyring-utils/src/keyring.ts | 2 +- 4 files changed, 31 insertions(+), 14 deletions(-) diff --git a/packages/keyring-eth-ledger-bridge/src/ledger-keyring.ts b/packages/keyring-eth-ledger-bridge/src/ledger-keyring.ts index 4155a384..ed5d6208 100644 --- a/packages/keyring-eth-ledger-bridge/src/ledger-keyring.ts +++ b/packages/keyring-eth-ledger-bridge/src/ledger-keyring.ts @@ -484,12 +484,16 @@ export class LedgerKeyring implements Keyring { return hdPath; } - async signTypedData( + async signTypedData< + Version extends SignTypedDataVersion.V4, + Types extends MessageTypes, + Options extends { version?: Version }, + >( withAccount: Hex, - data: TypedMessage, - options: { version?: string } = {}, + data: TypedMessage, + options: Options, ): Promise { - const isV4 = options.version === 'V4'; + const isV4 = options?.version === 'V4'; if (!isV4) { throw new Error( 'Ledger: Only version 4 of typed data signing is supported', diff --git a/packages/keyring-eth-simple/src/simple-keyring.ts b/packages/keyring-eth-simple/src/simple-keyring.ts index 8b7410bd..dc46e9ad 100644 --- a/packages/keyring-eth-simple/src/simple-keyring.ts +++ b/packages/keyring-eth-simple/src/simple-keyring.ts @@ -10,6 +10,9 @@ import { toBuffer, } from '@ethereumjs/util'; import { + type TypedDataV1, + type MessageTypes, + type TypedMessage, concatSig, decrypt, EIP7702Authorization, @@ -147,17 +150,23 @@ export default class SimpleKeyring implements Keyring { return decrypt({ privateKey, encryptedData }); } - // personal_signTypedData, signs data along with the schema - async signTypedData( + async signTypedData< + Version extends SignTypedDataVersion, + Types extends MessageTypes, + Options extends { version: Version } & KeyringOpt, + >( address: Hex, - typedData: any, - opts: KeyringOpt = { version: SignTypedDataVersion.V1 }, + typedData: Version extends SignTypedDataVersion.V1 + ? TypedDataV1 + : TypedMessage, + opts?: Options, ): Promise { + const options = opts ?? { version: SignTypedDataVersion.V1 }; // Treat invalid versions as "V1" let version = SignTypedDataVersion.V1; - if (opts.version && isSignTypedDataVersion(opts.version)) { - version = SignTypedDataVersion[opts.version]; + if (options.version && isSignTypedDataVersion(options.version)) { + version = SignTypedDataVersion[options.version]; } const privateKey = this.#getPrivateKeyFor(address, opts); diff --git a/packages/keyring-eth-trezor/src/trezor-keyring.ts b/packages/keyring-eth-trezor/src/trezor-keyring.ts index 203864c1..ca3d7111 100644 --- a/packages/keyring-eth-trezor/src/trezor-keyring.ts +++ b/packages/keyring-eth-trezor/src/trezor-keyring.ts @@ -453,10 +453,14 @@ export class TrezorKeyring implements Keyring { } // EIP-712 Sign Typed Data - async signTypedData( + async signTypedData< + Version extends SignTypedDataVersion.V3 | SignTypedDataVersion.V4, + Types extends MessageTypes, + Options extends { version?: Version }, + >( address: Hex, - data: TypedMessage, - { version }: { version: SignTypedDataVersion }, + data: TypedMessage, + { version }: Options, ): Promise { const dataWithHashes = transformTypedData( data, diff --git a/packages/keyring-utils/src/keyring.ts b/packages/keyring-utils/src/keyring.ts index a523e3ce..9859d128 100644 --- a/packages/keyring-utils/src/keyring.ts +++ b/packages/keyring-utils/src/keyring.ts @@ -229,7 +229,7 @@ export type Keyring = { */ signTypedData?( address: Hex, - typedData: Record, + typedData: unknown[] | Record, options?: Record, ): Promise; From affd252f1f75f48910850caf30f2df2710c10819 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Fri, 21 Feb 2025 15:16:27 +0100 Subject: [PATCH 2/8] refactor: change hdkeyring type --- packages/keyring-eth-hd/src/index.ts | 17 ++++++++++------- .../keyring-eth-simple/src/simple-keyring.ts | 4 +--- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/keyring-eth-hd/src/index.ts b/packages/keyring-eth-hd/src/index.ts index f340593d..3febef5e 100644 --- a/packages/keyring-eth-hd/src/index.ts +++ b/packages/keyring-eth-hd/src/index.ts @@ -349,16 +349,19 @@ class HdKeyring { * @param opts - The options for signing the message. * @returns The signature of the message. */ - async signTypedData( + async signTypedData< + Version extends SignTypedDataVersion, + Types extends MessageTypes, + Options extends { version: Version }, + >( withAccount: Hex, - typedData: TypedDataV1 | TypedMessage, - opts: HDKeyringAccountSelectionOptions & { - version: SignTypedDataVersion; - } = { version: SignTypedDataVersion.V1 }, + typedData: Version extends 'V1' ? TypedDataV1 : TypedMessage, + opts?: HDKeyringAccountSelectionOptions & Options, ): Promise { + 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); diff --git a/packages/keyring-eth-simple/src/simple-keyring.ts b/packages/keyring-eth-simple/src/simple-keyring.ts index dc46e9ad..87ea28a0 100644 --- a/packages/keyring-eth-simple/src/simple-keyring.ts +++ b/packages/keyring-eth-simple/src/simple-keyring.ts @@ -156,9 +156,7 @@ export default class SimpleKeyring implements Keyring { Options extends { version: Version } & KeyringOpt, >( address: Hex, - typedData: Version extends SignTypedDataVersion.V1 - ? TypedDataV1 - : TypedMessage, + typedData: Version extends 'V1' ? TypedDataV1 : TypedMessage, opts?: Options, ): Promise { const options = opts ?? { version: SignTypedDataVersion.V1 }; From a20af8580e731d10a9893e56e62cfd305d3c28ca Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Fri, 21 Feb 2025 15:28:32 +0100 Subject: [PATCH 3/8] test: fix ledger tests --- .../keyring-eth-ledger-bridge/src/ledger-keyring.test.ts | 4 ++-- packages/keyring-eth-ledger-bridge/src/ledger-keyring.ts | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/keyring-eth-ledger-bridge/src/ledger-keyring.test.ts b/packages/keyring-eth-ledger-bridge/src/ledger-keyring.test.ts index 0297a89b..81425428 100644 --- a/packages/keyring-eth-ledger-bridge/src/ledger-keyring.test.ts +++ b/packages/keyring-eth-ledger-bridge/src/ledger-keyring.test.ts @@ -968,7 +968,7 @@ describe('LedgerKeyring', function () { ], }, }; - const options = { version: 'V4' }; + const options = { version: sigUtil.SignTypedDataVersion.V4 }; beforeEach(async function () { jest @@ -1043,7 +1043,7 @@ describe('LedgerKeyring', function () { it('throws an error if the signTypedData version is not v4', async function () { await expect( keyring.signTypedData(fakeAccounts[0], fixtureData, { - version: 'V3', + version: sigUtil.SignTypedDataVersion.V3, }), ).rejects.toThrow( 'Ledger: Only version 4 of typed data signing is supported', diff --git a/packages/keyring-eth-ledger-bridge/src/ledger-keyring.ts b/packages/keyring-eth-ledger-bridge/src/ledger-keyring.ts index ed5d6208..7d030b75 100644 --- a/packages/keyring-eth-ledger-bridge/src/ledger-keyring.ts +++ b/packages/keyring-eth-ledger-bridge/src/ledger-keyring.ts @@ -485,15 +485,16 @@ export class LedgerKeyring implements Keyring { } async signTypedData< - Version extends SignTypedDataVersion.V4, + Version extends SignTypedDataVersion, Types extends MessageTypes, Options extends { version?: Version }, >( withAccount: Hex, data: TypedMessage, - options: Options, + options?: Options, ): Promise { - const isV4 = options?.version === 'V4'; + const { version } = options ?? {}; + const isV4 = version === 'V4'; if (!isV4) { throw new Error( 'Ledger: Only version 4 of typed data signing is supported', From fc6bec1bfc2ba9c167878eb073590025a1ef5fcb Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Fri, 21 Feb 2025 15:47:53 +0100 Subject: [PATCH 4/8] test: fix simple keyring tests --- .../keyring-eth-simple/src/simple-keyring.test.ts | 11 ++++++----- packages/keyring-eth-simple/src/simple-keyring.ts | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/keyring-eth-simple/src/simple-keyring.test.ts b/packages/keyring-eth-simple/src/simple-keyring.test.ts index 6a35b429..1fe6a51d 100644 --- a/packages/keyring-eth-simple/src/simple-keyring.test.ts +++ b/packages/keyring-eth-simple/src/simple-keyring.test.ts @@ -360,6 +360,7 @@ describe('simple-keyring', function () { it('returns the expected value if invalid version is given', async function () { await keyring.deserialize([privKeyHex]); const signature = await keyring.signTypedData(address, typedData, { + // @ts-expect-error: intentionally passing invalid version version: 'FOO', }); expect(signature).toBe(expectedSignature); @@ -390,7 +391,7 @@ describe('simple-keyring', function () { it('works via `V1` string', async function () { await keyring.deserialize([privKeyHex]); const signature = await keyring.signTypedData(address, typedData, { - version: 'V1', + version: SignTypedDataVersion.V1, }); expect(signature).toBe(expectedSignature); const restored = recoverTypedSignature({ @@ -446,7 +447,7 @@ describe('simple-keyring', function () { await keyring.deserialize([privKeyHex]); const signature = await keyring.signTypedData(address, typedData, { - version: 'V3', + version: SignTypedDataVersion.V3, }); const restored = recoverTypedSignature({ data: typedData, @@ -506,7 +507,7 @@ describe('simple-keyring', function () { const address = (await keyring.getAccounts())[0]; assert(address, 'address is undefined'); const signature = await keyring.signTypedData(address, typedData, { - version: 'V3', + version: SignTypedDataVersion.V3, }); expect(signature).toBe(expectedSignature); const restored = recoverTypedSignature({ @@ -535,7 +536,7 @@ describe('simple-keyring', function () { await keyring.deserialize([privKeyHex]); const signature = await keyring.signTypedData(address, typedData, { - version: 'V4', + version: SignTypedDataVersion.V4, }); const restored = recoverTypedSignature({ data: typedData, @@ -734,7 +735,7 @@ describe('simple-keyring', function () { const address = (await keyring.getAccounts())[0]; assert(address, 'address is undefined'); const signature = await keyring.signTypedData(address, typedData, { - version: 'V4', + version: SignTypedDataVersion.V4, }); expect(signature).toBe(expectedSignature); const restored = recoverTypedSignature({ diff --git a/packages/keyring-eth-simple/src/simple-keyring.ts b/packages/keyring-eth-simple/src/simple-keyring.ts index 87ea28a0..e34d5233 100644 --- a/packages/keyring-eth-simple/src/simple-keyring.ts +++ b/packages/keyring-eth-simple/src/simple-keyring.ts @@ -153,7 +153,7 @@ export default class SimpleKeyring implements Keyring { async signTypedData< Version extends SignTypedDataVersion, Types extends MessageTypes, - Options extends { version: Version } & KeyringOpt, + Options extends { version?: Version } & KeyringOpt, >( address: Hex, typedData: Version extends 'V1' ? TypedDataV1 : TypedMessage, From 5f859426a80bfe6871e5025beae02d3c33a4e3fa Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Fri, 21 Feb 2025 15:53:39 +0100 Subject: [PATCH 5/8] bump coverage threshold --- packages/keyring-eth-ledger-bridge/jest.config.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/keyring-eth-ledger-bridge/jest.config.js b/packages/keyring-eth-ledger-bridge/jest.config.js index 8e17553d..5fb1fcc0 100644 --- a/packages/keyring-eth-ledger-bridge/jest.config.js +++ b/packages/keyring-eth-ledger-bridge/jest.config.js @@ -23,10 +23,10 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 90.78, + branches: 90.97, functions: 96, - lines: 95.02, - statements: 95.07, + lines: 95.03, + statements: 95.09, }, }, }); From e4436a779198b85956baa8b74b1eca3ebda18003 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Fri, 21 Feb 2025 18:43:34 +0100 Subject: [PATCH 6/8] update changelogs --- packages/keyring-eth-hd/CHANGELOG.md | 3 +++ packages/keyring-eth-ledger-bridge/CHANGELOG.md | 1 + packages/keyring-eth-simple/CHANGELOG.md | 2 ++ packages/keyring-eth-trezor/CHANGELOG.md | 1 + 4 files changed, 7 insertions(+) diff --git a/packages/keyring-eth-hd/CHANGELOG.md b/packages/keyring-eth-hd/CHANGELOG.md index b73b7243..874d9494 100644 --- a/packages/keyring-eth-hd/CHANGELOG.md +++ b/packages/keyring-eth-hd/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +- **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` when other versions are requested + ## [10.0.1] ### Changed diff --git a/packages/keyring-eth-ledger-bridge/CHANGELOG.md b/packages/keyring-eth-ledger-bridge/CHANGELOG.md index 80aeaa4a..81442e24 100644 --- a/packages/keyring-eth-ledger-bridge/CHANGELOG.md +++ b/packages/keyring-eth-ledger-bridge/CHANGELOG.md @@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The `signPersonalMessage` method now accepts an `Hex` typed value as the `withAccount` parameter. - The `signTypedData` method now accepts an `Hex` typed value as the `withAccount` parameter. - The `unlockAccountByAddress` method now accepts an `Hex` typed value as the `address` parameter. +- **BREAKING:** The `signTypedData` method now requires `SignTypedDataVersion` as version for the `options` argument ([#224](https://github.com/MetaMask/accounts/pull/224)). ### Removed diff --git a/packages/keyring-eth-simple/CHANGELOG.md b/packages/keyring-eth-simple/CHANGELOG.md index c7647f65..5cad5bc2 100644 --- a/packages/keyring-eth-simple/CHANGELOG.md +++ b/packages/keyring-eth-simple/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **BREAKING:** The `SimpleKeyring` class now implements `Keyring` from `@metamask/keyring-utils` ([#217](https://github.com/MetaMask/accounts/pull/217)) - The `deserialize` method now requires a `string[]` argument. +- **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` when other versions are requested ## [8.1.1] diff --git a/packages/keyring-eth-trezor/CHANGELOG.md b/packages/keyring-eth-trezor/CHANGELOG.md index bb61c365..5748f25d 100644 --- a/packages/keyring-eth-trezor/CHANGELOG.md +++ b/packages/keyring-eth-trezor/CHANGELOG.md @@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The `signPersonalMessage` method now accepts an `Hex` typed value as the `withAccount` parameter. - The `signTypedData` method now accepts an `Hex` typed value as the `withAccount` parameter. - The `unlockAccountByAddress` method now accepts an `Hex` typed value as the `address` parameter. +- **BREAKING:** The `signTypedData` `options.version` argument type has been restricted to accept `SignTypedDataVersion.V3 | SignTypedDataVersion.V4` ([#224](https://github.com/MetaMask/accounts/pull/224)) ### Removed From a761af18f7741dfa0e944f168786828e5386d8d7 Mon Sep 17 00:00:00 2001 From: Michele Esposito <34438276+mikesposito@users.noreply.github.com> Date: Fri, 21 Feb 2025 18:49:22 +0100 Subject: [PATCH 7/8] Update packages/keyring-eth-hd/CHANGELOG.md --- packages/keyring-eth-hd/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/keyring-eth-hd/CHANGELOG.md b/packages/keyring-eth-hd/CHANGELOG.md index 874d9494..02b6228d 100644 --- a/packages/keyring-eth-hd/CHANGELOG.md +++ b/packages/keyring-eth-hd/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + - **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` when other versions are requested From c1038eef93b7dc5f0cdaf3181844d8a475b88c46 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Tue, 25 Feb 2025 17:05:12 +0100 Subject: [PATCH 8/8] refactor!: `HdKeyring` implements `Keyring` --- packages/keyring-eth-hd/CHANGELOG.md | 2 + packages/keyring-eth-hd/package.json | 1 + .../keyring-eth-hd/src/hd-keyring.test.ts | 83 ++++++++----------- packages/keyring-eth-hd/src/hd-keyring.ts | 36 +++++--- packages/keyring-eth-hd/src/index.ts | 3 +- packages/keyring-eth-hd/tsconfig.build.json | 5 ++ packages/keyring-eth-hd/tsconfig.json | 3 + yarn.lock | 1 + 8 files changed, 73 insertions(+), 61 deletions(-) diff --git a/packages/keyring-eth-hd/CHANGELOG.md b/packages/keyring-eth-hd/CHANGELOG.md index dc9fa5f8..7152e330 100644 --- a/packages/keyring-eth-hd/CHANGELOG.md +++ b/packages/keyring-eth-hd/CHANGELOG.md @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **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` 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] diff --git a/packages/keyring-eth-hd/package.json b/packages/keyring-eth-hd/package.json index 9975fc2e..934d82fc 100644 --- a/packages/keyring-eth-hd/package.json +++ b/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", diff --git a/packages/keyring-eth-hd/src/hd-keyring.test.ts b/packages/keyring-eth-hd/src/hd-keyring.test.ts index 65335780..a4c52c19 100644 --- a/packages/keyring-eth-hd/src/hd-keyring.test.ts +++ b/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 => { + 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, diff --git a/packages/keyring-eth-hd/src/hd-keyring.ts b/packages/keyring-eth-hd/src/hd-keyring.ts index 6e9647d9..f3973e38 100644 --- a/packages/keyring-eth-hd/src/hd-keyring.ts +++ b/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 { + async serialize(): Promise { 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 = {}): Promise { + async deserialize( + opts: Partial, + ): Promise { 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 { return this.#wallets.map((wallet) => { assert(wallet.publicKey, 'Expected public key to be set'); return this.#addressfromPublicKey(wallet.publicKey); @@ -587,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 { if (this.root) { throw new Error( diff --git a/packages/keyring-eth-hd/src/index.ts b/packages/keyring-eth-hd/src/index.ts index cee44311..05e9f727 100644 --- a/packages/keyring-eth-hd/src/index.ts +++ b/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'; diff --git a/packages/keyring-eth-hd/tsconfig.build.json b/packages/keyring-eth-hd/tsconfig.build.json index d0e4b01a..684ef9c8 100644 --- a/packages/keyring-eth-hd/tsconfig.build.json +++ b/packages/keyring-eth-hd/tsconfig.build.json @@ -7,6 +7,11 @@ "rootDir": "src", "exactOptionalPropertyTypes": false }, + "references": [ + { + "path": "../keyring-utils/tsconfig.build.json" + } + ], "include": ["./src/**/*.ts", "./src/**/*.js"], "exclude": ["./src/**/*.test.ts"] } diff --git a/packages/keyring-eth-hd/tsconfig.json b/packages/keyring-eth-hd/tsconfig.json index bb71cc9b..9269863f 100644 --- a/packages/keyring-eth-hd/tsconfig.json +++ b/packages/keyring-eth-hd/tsconfig.json @@ -6,6 +6,9 @@ "exactOptionalPropertyTypes": false, "target": "es2017" }, + "references": [ + { "path": "../keyring-utils" } + ], "include": ["./src", "./types"], "exclude": ["./dist/**/*"] } diff --git a/yarn.lock b/yarn.lock index f7aa80ea..c7cfc34f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1669,6 +1669,7 @@ __metadata: "@metamask/bip39": "npm:^4.0.0" "@metamask/eth-sig-util": "npm:^8.2.0" "@metamask/key-tree": "npm:^10.0.2" + "@metamask/keyring-utils": "workspace:^" "@metamask/scure-bip39": "npm:^2.1.1" "@metamask/utils": "npm:^11.1.0" "@ts-bridge/cli": "npm:^0.6.3"