diff --git a/README.md b/README.md index 4e5bf1f8d..d5867fe1e 100644 --- a/README.md +++ b/README.md @@ -52,10 +52,15 @@ linkStyle default opacity:0.5 account_api --> keyring_api; account_api --> keyring_utils; keyring_api --> keyring_utils; + eth_hd_keyring --> keyring_api; eth_hd_keyring --> keyring_utils; + eth_ledger_bridge_keyring --> keyring_api; eth_ledger_bridge_keyring --> keyring_utils; + eth_qr_keyring --> keyring_api; eth_qr_keyring --> keyring_utils; + eth_simple_keyring --> keyring_api; eth_simple_keyring --> keyring_utils; + eth_trezor_keyring --> keyring_api; eth_trezor_keyring --> keyring_utils; keyring_internal_api --> keyring_api; keyring_internal_api --> keyring_utils; diff --git a/packages/keyring-api/CHANGELOG.md b/packages/keyring-api/CHANGELOG.md index 2f9e06261..4bad4583b 100644 --- a/packages/keyring-api/CHANGELOG.md +++ b/packages/keyring-api/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `KeyringWrapper` base class to adapt legacy keyrings to `KeyringV2` ([#398](https://github.com/MetaMask/accounts/pull/398)) +- Add unified `KeyringV2` interface ([#397](https://github.com/MetaMask/accounts/pull/397)) + ## [21.2.0] ### Added diff --git a/packages/keyring-api/README.md b/packages/keyring-api/README.md index 2b49484c2..42e4e1627 100644 --- a/packages/keyring-api/README.md +++ b/packages/keyring-api/README.md @@ -230,6 +230,10 @@ A unified keyring interface, designed to work for both native (EVM) keyrings and - Interface name: `KeyringV2` - Location: `@metamask/keyring-api/src/api/v2/keyring.ts` +### Keyring wrapper + +The `KeyringWrapper` helper adapts existing keyrings that implement the legacy `Keyring` interface to the new `KeyringV2` interface. It is intended to be subclassed in concrete keyrings, overriding the account management and request-handling methods to delegate to the underlying implementation. + ## Migrating from 0.1.x to 0.2.x The following changes were made to the API, which may require changes to your diff --git a/packages/keyring-api/package.json b/packages/keyring-api/package.json index 2a3a36770..9de7a6224 100644 --- a/packages/keyring-api/package.json +++ b/packages/keyring-api/package.json @@ -49,7 +49,9 @@ "@metamask/keyring-utils": "workspace:^", "@metamask/superstruct": "^3.1.0", "@metamask/utils": "^11.1.0", - "bitcoin-address-validation": "^2.2.3" + "@types/uuid": "^9.0.8", + "bitcoin-address-validation": "^2.2.3", + "uuid": "^9.0.1" }, "devDependencies": { "@lavamoat/allow-scripts": "^3.2.1", diff --git a/packages/keyring-api/src/api/v2/index.ts b/packages/keyring-api/src/api/v2/index.ts index f9ce241b6..cc137c238 100644 --- a/packages/keyring-api/src/api/v2/index.ts +++ b/packages/keyring-api/src/api/v2/index.ts @@ -4,3 +4,4 @@ export * from './keyring-type'; export * from './create-account'; export * from './export-account'; export * from './private-key'; +export * from './wrapper'; diff --git a/packages/keyring-api/src/api/v2/wrapper/index.ts b/packages/keyring-api/src/api/v2/wrapper/index.ts new file mode 100644 index 000000000..cc1fba566 --- /dev/null +++ b/packages/keyring-api/src/api/v2/wrapper/index.ts @@ -0,0 +1,2 @@ +export * from './keyring-wrapper'; +export * from './keyring-address-resolver'; diff --git a/packages/keyring-api/src/api/v2/wrapper/keyring-address-resolver.test.ts b/packages/keyring-api/src/api/v2/wrapper/keyring-address-resolver.test.ts new file mode 100644 index 000000000..6acde44fa --- /dev/null +++ b/packages/keyring-api/src/api/v2/wrapper/keyring-address-resolver.test.ts @@ -0,0 +1,28 @@ +import { InMemoryKeyringAddressResolver } from './keyring-address-resolver'; + +describe('InMemoryKeyringAddressResolver', () => { + it('registers and resolves account IDs and addresses', () => { + const resolver = new InMemoryKeyringAddressResolver(); + + const address = '0xaBc'; + const id = resolver.register(address); + + expect(typeof id).toBe('string'); + + const resolvedAddress = resolver.getAddress(id); + expect(resolvedAddress).toBe(address); + + const resolvedId = resolver.getAccountId(address); + expect(resolvedId).toBe(id); + }); + + it('reuses the same ID when registering the same address', () => { + const resolver = new InMemoryKeyringAddressResolver(); + + const address = '0xaBc'; + const firstId = resolver.register(address); + const secondId = resolver.register(address); + + expect(firstId).toBe(secondId); + }); +}); diff --git a/packages/keyring-api/src/api/v2/wrapper/keyring-address-resolver.ts b/packages/keyring-api/src/api/v2/wrapper/keyring-address-resolver.ts new file mode 100644 index 000000000..9fc04ac03 --- /dev/null +++ b/packages/keyring-api/src/api/v2/wrapper/keyring-address-resolver.ts @@ -0,0 +1,54 @@ +import type { AccountId } from '@metamask/keyring-utils'; +import { v4 as uuidv4 } from 'uuid'; + +/** + * Mapping between an internal AccountId and the underlying keyring address. + */ +export type KeyringAddressResolver = { + /** + * Resolve an AccountId into an underlying address. + */ + getAddress(accountId: AccountId): string | undefined; + + /** + * Resolve an underlying address into an AccountId. + */ + getAccountId(address: string): AccountId | undefined; + + /** + * Register a new mapping. Implementations are free to decide how the + * AccountId is generated (e.g. UUIDv4). + */ + register(address: string): AccountId; +}; + +/** + * Simple in-memory AccountId/address resolver used by default. This is mostly + * intended for controller-managed lifecycles where wrappers live in memory. + */ +export class InMemoryKeyringAddressResolver implements KeyringAddressResolver { + readonly #idByAddress = new Map(); + + readonly #addressById = new Map(); + + getAddress(accountId: AccountId): string | undefined { + return this.#addressById.get(accountId); + } + + getAccountId(address: string): AccountId | undefined { + return this.#idByAddress.get(address); + } + + register(address: string): AccountId { + const existing = this.#idByAddress.get(address); + if (existing) { + return existing; + } + const id = uuidv4(); + + this.#idByAddress.set(address, id); + this.#addressById.set(id, address); + + return id; + } +} diff --git a/packages/keyring-api/src/api/v2/wrapper/keyring-wrapper.test.ts b/packages/keyring-api/src/api/v2/wrapper/keyring-wrapper.test.ts new file mode 100644 index 000000000..a4ac8e3b7 --- /dev/null +++ b/packages/keyring-api/src/api/v2/wrapper/keyring-wrapper.test.ts @@ -0,0 +1,203 @@ +import type { Keyring, AccountId } from '@metamask/keyring-utils'; +import type { Hex, Json } from '@metamask/utils'; +import { v4 as uuidv4 } from 'uuid'; + +import { InMemoryKeyringAddressResolver } from './keyring-address-resolver'; +import { KeyringWrapper } from './keyring-wrapper'; +import type { KeyringAccount } from '../../account'; +import type { KeyringCapabilities } from '../keyring-capabilities'; +import { KeyringType } from '../keyring-type'; + +class TestKeyringWrapper extends KeyringWrapper { + async getAccounts(): Promise { + const addresses = await this.inner.getAccounts(); + const scopes = this.capabilities.scopes ?? ['eip155:1']; + + return addresses.map((address) => { + const id = this.resolver.register(address); + + const account: KeyringAccount = { + id, + type: 'eip155:eoa', + address, + scopes, + options: {}, + methods: [], + }; + + return account; + }); + } + + public deletedAccountIds: AccountId[] = []; + + async createAccounts(): Promise { + return this.getAccounts(); + } + + async deleteAccount(accountId: AccountId): Promise { + this.deletedAccountIds.push(accountId); + } + + async submitRequest(): Promise { + return {}; + } +} + +class TestKeyring implements Keyring { + static type = 'Test Keyring'; + + public type = TestKeyring.type; + + readonly #accounts: Hex[]; + + public lastDeserializedState: Json | undefined; + + constructor(addresses: Hex[] = []) { + this.#accounts = addresses; + } + + async serialize(): Promise { + return { accounts: this.#accounts }; + } + + async deserialize(state: Json): Promise { + this.lastDeserializedState = state; + } + + async getAccounts(): Promise { + return this.#accounts; + } + + async addAccounts(_numberOfAccounts = 1): Promise { + return this.#accounts; + } +} + +const capabilities: KeyringCapabilities = { + scopes: ['eip155:10'], +}; + +const entropySourceId = 'test-entropy-source'; + +describe('KeyringWrapper', () => { + it('serializes and deserializes via the inner keyring', async () => { + const inner = new TestKeyring(['0x1']); + const wrapper = new TestKeyringWrapper({ + inner, + type: KeyringType.Hd, + capabilities, + entropySourceId, + }); + + const state = await wrapper.serialize(); + expect(state).toStrictEqual({ accounts: ['0x1'] }); + + await wrapper.deserialize(state); + expect(inner.lastDeserializedState).toStrictEqual(state); + }); + + it('returns accounts with stable IDs and correct addresses', async () => { + const addresses = ['0x1' as const, '0x2' as const]; + const inner = new TestKeyring(addresses); + const wrapper = new TestKeyringWrapper({ + inner, + type: KeyringType.Hd, + capabilities, + entropySourceId, + }); + + const accounts = await wrapper.getAccounts(); + + expect(accounts).toHaveLength(addresses.length); + + const ids = new Set(); + accounts.forEach((account: KeyringAccount, index) => { + expect(account.address).toBe(addresses[index]); + expect(account.type).toBe('eip155:eoa'); + expect(account.scopes).toStrictEqual(capabilities.scopes); + expect(account.options).toStrictEqual({}); + expect(account.methods).toStrictEqual([]); + + ids.add(account.id); + }); + + // Ensure IDs are unique + expect(ids.size).toBe(addresses.length); + + // getAccount should resolve by ID + const [firstAccount] = accounts; + expect(firstAccount).toBeDefined(); + if (!firstAccount) { + throw new Error('Expected at least one account from the wrapper'); + } + const resolved = await wrapper.getAccount(firstAccount.id); + expect(resolved.address).toBe(firstAccount.address); + }); + + it('throws when requesting an unknown account', async () => { + const inner = new TestKeyring([]); + const wrapper = new TestKeyringWrapper({ + inner, + type: KeyringType.Hd, + capabilities, + entropySourceId, + }); + + await expect(wrapper.getAccount(uuidv4())).rejects.toThrow( + 'Account not found for id', + ); + }); + + it('throws when account mapping exists but account object cannot be found', async () => { + const addresses = ['0x1' as const]; + const inner = new TestKeyring(addresses); + const resolver = new InMemoryKeyringAddressResolver(); + const wrapper = new TestKeyringWrapper({ + inner, + type: KeyringType.Hd, + capabilities, + resolver, + entropySourceId, + }); + + // Prime the resolver by calling getAccounts once + await wrapper.getAccounts(); + + // Now, simulate a missing account object by clearing the underlying + // accounts of the inner keyring. + const emptyInner = new TestKeyring([] as Hex[]); + const inconsistentWrapper = new TestKeyringWrapper({ + inner: emptyInner, + type: KeyringType.Hd, + capabilities, + resolver, + entropySourceId, + }); + + const accountId = resolver.getAccountId( + addresses[0] as string, + ) as AccountId; + + await expect(inconsistentWrapper.getAccount(accountId)).rejects.toThrow( + 'Account not found for id', + ); + }); + + it('falls back to mainnet scope when capabilities.scopes is not set', async () => { + const addresses = ['0x1' as const]; + const inner = new TestKeyring(addresses); + const wrapper = new TestKeyringWrapper({ + inner, + type: KeyringType.Hd, + entropySourceId, + // Explicitly omit scopes to exercise the default fallback. + capabilities: {} as KeyringCapabilities, + }); + + const accounts = await wrapper.getAccounts(); + + expect(accounts).toHaveLength(1); + expect(accounts[0]?.scopes).toStrictEqual(['eip155:1']); + }); +}); diff --git a/packages/keyring-api/src/api/v2/wrapper/keyring-wrapper.ts b/packages/keyring-api/src/api/v2/wrapper/keyring-wrapper.ts new file mode 100644 index 000000000..ea8d2ffa2 --- /dev/null +++ b/packages/keyring-api/src/api/v2/wrapper/keyring-wrapper.ts @@ -0,0 +1,184 @@ +import type { Keyring, AccountId } from '@metamask/keyring-utils'; +import type { Json } from '@metamask/utils'; + +import { + InMemoryKeyringAddressResolver, + type KeyringAddressResolver, +} from './keyring-address-resolver'; +import type { + CreateAccountOptions, + ExportAccountOptions, + ExportedAccount, +} from '..'; +import type { KeyringAccount } from '../../account'; +import type { KeyringRequest } from '../../request'; +import type { KeyringV2 } from '../keyring'; +import type { KeyringCapabilities } from '../keyring-capabilities'; +import type { KeyringType } from '../keyring-type'; + +/** + * Basic options for constructing a {@link KeyringWrapper}. + */ +export type KeyringWrapperOptions = { + /** + * The underlying "old" keyring instance that this wrapper adapts. + */ + inner: TInnerKeyring; + + /** + * The concrete keyring type exposed through the V2 interface. + */ + type: KeyringType; + + /** + * Capabilities of the underlying keyring. + */ + capabilities: KeyringCapabilities; + + /** + * Identifier for the entropy source associated with this keyring. + */ + entropySourceId: string; + + /** + * Resolver used to map between AccountId and underlying addresses. If not + * provided, an in-memory resolver will be used. + */ + resolver?: KeyringAddressResolver; +}; + +/** + * Generic adapter that turns an existing {@link Keyring} implementation into a + * {@link KeyringV2} instance. + * + * Consumers are expected to provide concrete mappings between high-level V2 + * operations and the underlying keyring methods (for example BIP-44 account + * creation, private-key import, and request handling). This class focuses on + * the common mechanics required by all adapters: state serialization, + * account-ID/address mapping and basic account management. + */ +export abstract class KeyringWrapper + implements KeyringV2 +{ + readonly type: `${KeyringType}`; + + readonly capabilities: KeyringCapabilities; + + protected readonly inner: TInnerKeyring; + + protected readonly resolver: KeyringAddressResolver; + + protected readonly entropySourceId: string; + + constructor(options: KeyringWrapperOptions) { + this.inner = options.inner; + this.type = `${options.type}`; + this.capabilities = options.capabilities; + this.resolver = options.resolver ?? new InMemoryKeyringAddressResolver(); + this.entropySourceId = options.entropySourceId; + } + + /** + * Serialize the underlying keyring state to a JSON-serializable object. + * + * This simply delegates to the legacy keyring's {@link Keyring.serialize} + * implementation. + * + * @returns The serialized keyring state. + */ + async serialize(): Promise { + return this.inner.serialize(); + } + + /** + * Hydrate the underlying keyring from a previously serialized state. + * + * This simply delegates to the legacy keyring's {@link Keyring.deserialize} + * implementation. + * + * @param state - The serialized keyring state. + */ + async deserialize(state: Json): Promise { + await this.inner.deserialize(state); + } + + /** + * Return all accounts managed by this keyring. + * + * Concrete adapters are responsible for mapping the underlying keyring's + * notion of accounts (typically addresses returned by + * {@link Keyring.getAccounts}) into {@link KeyringAccount} objects. + * Implementations should use the configured {@link KeyringAddressResolver} + * to establish the account ID/address mapping so that + * {@link getAccount} works as expected. + * + * @returns The list of managed accounts. + */ + abstract getAccounts(): Promise; + + /** + * Look up a single account by its {@link AccountId}. + * + * This method calls {@link getAccounts} and returns the matching + * {@link KeyringAccount} by ID, or throws if the ID is unknown in the + * current account set. + * + * @param accountId - The AccountId to look up. + * @returns The matching KeyringAccount. + */ + async getAccount(accountId: AccountId): Promise { + const accounts = await this.getAccounts(); + const account = accounts.find((item) => item.id === accountId); + if (!account) { + throw new Error(`Account not found for id: ${accountId}`); + } + + return account; + } + + /** + * Create one or more new accounts managed by this keyring. + * + * Implementations are responsible for interpreting the + * {@link CreateAccountOptions} (for example BIP-44 derivation or + * private-key import) and returning the resulting {@link KeyringAccount} + * objects. Implementors should also ensure that the resolver is updated so + * that {@link getAccount} works for newly created accounts. + */ + abstract createAccounts( + options: CreateAccountOptions, + ): Promise; + + /** + * Remove the account associated with the given {@link AccountId} from this + * keyring. + * + * Implementations are expected to translate the ID to an underlying + * address (typically via the resolver) and then invoke the appropriate + * removal mechanism on the legacy keyring. + */ + abstract deleteAccount(accountId: AccountId): Promise; + + /** + * Export the secrets associated with the given account in a format + * described by {@link ExportAccountOptions}. + * + * This method is optional, and concrete adapters should only + * implement it if the underlying keyring supports exporting + * accounts. + */ + exportAccount?( + accountId: AccountId, + options?: ExportAccountOptions, + ): Promise; + + /** + * Handle a high-level {@link KeyringRequest} on behalf of this keyring. + * + * Concrete adapters are responsible for routing the request's method and + * parameters to the appropriate legacy keyring APIs (for example signing + * transactions or decrypting messages) and returning a JSON-serializable + * result. + */ + abstract submitRequest(request: KeyringRequest): Promise; +} diff --git a/packages/keyring-eth-hd/CHANGELOG.md b/packages/keyring-eth-hd/CHANGELOG.md index be0587a12..69a415216 100644 --- a/packages/keyring-eth-hd/CHANGELOG.md +++ b/packages/keyring-eth-hd/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `HdKeyringV2` class implementing KeyringV2 interface ([#398](https://github.com/MetaMask/accounts/pull/398)) + - Wraps legacy `HdKeyring` to expose accounts via the unified KeyringV2 API and the `KeyringAccount` type. + ## [13.0.0] ### Added diff --git a/packages/keyring-eth-hd/package.json b/packages/keyring-eth-hd/package.json index 1ce4dc955..966737939 100644 --- a/packages/keyring-eth-hd/package.json +++ b/packages/keyring-eth-hd/package.json @@ -46,6 +46,7 @@ "@ethereumjs/util": "^9.1.0", "@metamask/eth-sig-util": "^8.2.0", "@metamask/key-tree": "^10.0.2", + "@metamask/keyring-api": "workspace:^", "@metamask/keyring-utils": "workspace:^", "@metamask/scure-bip39": "^2.1.1", "@metamask/utils": "^11.1.0", diff --git a/packages/keyring-eth-hd/src/hd-keyring-v2.test.ts b/packages/keyring-eth-hd/src/hd-keyring-v2.test.ts new file mode 100644 index 000000000..dd7d52cdd --- /dev/null +++ b/packages/keyring-eth-hd/src/hd-keyring-v2.test.ts @@ -0,0 +1,1043 @@ +import { TransactionFactory, type TypedTxData } from '@ethereumjs/tx'; +import { + EthAccountType, + EthMethod, + EthScope, + type KeyringRequest, + KeyringType, + PrivateKeyEncoding, +} from '@metamask/keyring-api'; +import type { AccountId } from '@metamask/keyring-utils'; +import type { Json } from '@metamask/utils'; + +import { HdKeyring } from './hd-keyring'; +import { HdKeyringV2 } from './hd-keyring-v2'; + +const TEST_MNEMONIC = + 'test test test test test test test test test test test junk'; +const TEST_ENTROPY_SOURCE_ID = 'test-entropy-source-id'; + +/** + * Helper function to create a minimal KeyringRequest for testing. + * + * @param accountId - The account ID to use in the request. + * @param method - The method name for the request. + * @param params - Optional array of parameters for the request. + * @returns A KeyringRequest object for testing. + */ +function createMockRequest( + accountId: AccountId, + method: string, + params: Json[] = [], +): KeyringRequest { + return { + id: '00000000-0000-0000-0000-000000000000', + scope: EthScope.Eoa, + account: accountId, + origin: 'http://localhost', + request: { + method, + params, + }, + }; +} + +describe('HdKeyringV2', () => { + let inner: HdKeyring; + let wrapper: HdKeyringV2; + + beforeEach(async () => { + inner = new HdKeyring(); + await inner.deserialize({ + mnemonic: Array.from(Buffer.from(TEST_MNEMONIC, 'utf8')), + numberOfAccounts: 0, + hdPath: "m/44'/60'/0'/0", + }); + + wrapper = new HdKeyringV2({ + legacyKeyring: inner, + entropySourceId: TEST_ENTROPY_SOURCE_ID, + }); + }); + + describe('constructor', () => { + it('creates a wrapper with correct type and capabilities', () => { + expect(wrapper.type).toBe(KeyringType.Hd); + expect(wrapper.capabilities.scopes).toStrictEqual([EthScope.Eoa]); + expect(wrapper.capabilities.bip44?.deriveIndex).toBe(true); + expect(wrapper.capabilities.bip44?.derivePath).toBe(false); + expect(wrapper.capabilities.bip44?.discover).toBe(false); + }); + }); + + describe('getAccounts', () => { + beforeEach(async () => { + await inner.addAccounts(2); + }); + + it('returns all accounts from the legacy keyring', async () => { + const accounts = await wrapper.getAccounts(); + + expect(accounts).toHaveLength(2); + }); + + it('creates KeyringAccount objects with correct structure', async () => { + const accounts = await wrapper.getAccounts(); + + expect(accounts.length).toBeGreaterThan(0); + const account = accounts[0]; + expect(account).toBeDefined(); + expect(account?.id).toBeDefined(); + expect(account?.address).toMatch(/^0x[0-9a-fA-F]{40}$/u); + expect(account?.type).toBe(EthAccountType.Eoa); + expect(account?.scopes).toStrictEqual([EthScope.Eoa]); + expect(account?.methods).toContain(EthMethod.PersonalSign); + expect(account?.methods).toContain(EthMethod.SignTransaction); + expect(account?.methods).toContain('eth_decrypt'); + expect(account?.options?.entropy?.type).toBe('mnemonic'); + + // Verify mnemonic entropy properties + const entropy = account?.options?.entropy; + expect(entropy).toBeDefined(); + expect('id' in (entropy ?? {})).toBe(true); + expect('groupIndex' in (entropy ?? {})).toBe(true); + expect('derivationPath' in (entropy ?? {})).toBe(true); + + // Assert on specific values only after type narrowing + expect(entropy && 'id' in entropy ? entropy.id : undefined).toBe( + TEST_ENTROPY_SOURCE_ID, + ); + expect( + entropy && 'groupIndex' in entropy ? entropy.groupIndex : undefined, + ).toBe(0); + expect( + entropy && 'derivationPath' in entropy + ? entropy.derivationPath + : undefined, + ).toBe("m/44'/60'/0'/0/0"); + }); + + it('caches KeyringAccount objects', async () => { + const accounts1 = await wrapper.getAccounts(); + const accounts2 = await wrapper.getAccounts(); + + // Same instances should be returned from cache + expect(accounts1[0]).toBe(accounts2[0]); + expect(accounts1[1]).toBe(accounts2[1]); + }); + + it('returns correct groupIndex for multiple accounts', async () => { + await inner.addAccounts(3); + const accounts = await wrapper.getAccounts(); + + expect(accounts).toHaveLength(5); + + const entropy0 = accounts[0]?.options?.entropy; + expect( + entropy0 && 'groupIndex' in entropy0 ? entropy0.groupIndex : undefined, + ).toBe(0); + + const entropy1 = accounts[1]?.options?.entropy; + expect( + entropy1 && 'groupIndex' in entropy1 ? entropy1.groupIndex : undefined, + ).toBe(1); + + const entropy2 = accounts[2]?.options?.entropy; + expect( + entropy2 && 'groupIndex' in entropy2 ? entropy2.groupIndex : undefined, + ).toBe(2); + + const entropy3 = accounts[3]?.options?.entropy; + expect( + entropy3 && 'groupIndex' in entropy3 ? entropy3.groupIndex : undefined, + ).toBe(3); + + const entropy4 = accounts[4]?.options?.entropy; + expect( + entropy4 && 'groupIndex' in entropy4 ? entropy4.groupIndex : undefined, + ).toBe(4); + }); + }); + + describe('deserialize', () => { + it('deserializes the legacy keyring state', async () => { + const newInner = new HdKeyring(); + const newWrapper = new HdKeyringV2({ + legacyKeyring: newInner, + entropySourceId: TEST_ENTROPY_SOURCE_ID, + }); + + await newWrapper.deserialize({ + mnemonic: Array.from(Buffer.from(TEST_MNEMONIC, 'utf8')), + numberOfAccounts: 2, + hdPath: "m/44'/60'/0'/0", + }); + + const accounts = await newWrapper.getAccounts(); + expect(accounts).toHaveLength(2); + }); + + it('clears the cache and rebuilds it', async () => { + await inner.addAccounts(2); + const accounts1 = await wrapper.getAccounts(); + + // Create a new inner keyring for re-deserialization + const newInner = new HdKeyring(); + const newWrapper = new HdKeyringV2({ + legacyKeyring: newInner, + entropySourceId: TEST_ENTROPY_SOURCE_ID, + }); + + await newWrapper.deserialize({ + mnemonic: Array.from(Buffer.from(TEST_MNEMONIC, 'utf8')), + numberOfAccounts: 1, + hdPath: "m/44'/60'/0'/0", + }); + + const accounts2 = await newWrapper.getAccounts(); + expect(accounts2).toHaveLength(1); + // Should be a different instance after deserialize + expect(accounts2[0]).not.toBe(accounts1[0]); + }); + }); + + describe('createAccounts', () => { + it('creates the first account at index 0', async () => { + const created = await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 0, + }); + + expect(created).toHaveLength(1); + + const account = created[0]; + const entropy = account?.options?.entropy; + expect( + entropy && 'groupIndex' in entropy ? entropy.groupIndex : undefined, + ).toBe(0); + + const allAccounts = await wrapper.getAccounts(); + expect(allAccounts).toHaveLength(1); + }); + + it('creates an account at a specific index', async () => { + // Create accounts sequentially + await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 0, + }); + await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 1, + }); + await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 2, + }); + + const created = await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 3, + }); + + expect(created).toHaveLength(1); + + const account = created[0]; + const entropy = account?.options?.entropy; + expect( + entropy && 'groupIndex' in entropy ? entropy.groupIndex : undefined, + ).toBe(3); + + // Should have created accounts 0, 1, 2, 3 + const allAccounts = await wrapper.getAccounts(); + expect(allAccounts).toHaveLength(4); + }); + + it('caches intermediate accounts when creating at higher index', async () => { + // Create accounts sequentially + await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 0, + }); + await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 1, + }); + await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 2, + }); + + const allAccounts = await wrapper.getAccounts(); + expect(allAccounts).toHaveLength(3); + + const entropy0 = allAccounts[0]?.options?.entropy; + expect( + entropy0 && 'groupIndex' in entropy0 ? entropy0.groupIndex : undefined, + ).toBe(0); + + const entropy1 = allAccounts[1]?.options?.entropy; + expect( + entropy1 && 'groupIndex' in entropy1 ? entropy1.groupIndex : undefined, + ).toBe(1); + + const entropy2 = allAccounts[2]?.options?.entropy; + expect( + entropy2 && 'groupIndex' in entropy2 ? entropy2.groupIndex : undefined, + ).toBe(2); + }); + + it('returns existing account if groupIndex already exists', async () => { + const first = await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 0, + }); + + const second = await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 0, + }); + + expect(second[0]).toBe(first[0]); + expect(await wrapper.getAccounts()).toHaveLength(1); + }); + + it('throws error for unsupported account creation type', async () => { + await expect( + wrapper.createAccounts({ + type: 'bip44:derive-path', + entropySource: TEST_ENTROPY_SOURCE_ID, + derivationPath: "m/44'/60'/0'/0/0", + }), + ).rejects.toThrow('Unsupported account creation type for HdKeyring'); + }); + + it('throws error for entropy source mismatch', async () => { + await expect( + wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: 'wrong-entropy-source', + groupIndex: 0, + }), + ).rejects.toThrow('Entropy source mismatch'); + }); + + it('creates multiple accounts sequentially', async () => { + await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 0, + }); + + await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 1, + }); + + await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 2, + }); + + const accounts = await wrapper.getAccounts(); + expect(accounts).toHaveLength(3); + }); + + it('syncs cache when legacy keyring adds accounts directly', async () => { + // Create first account via wrapper + await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 0, + }); + + expect(await wrapper.getAccounts()).toHaveLength(1); + + // Add accounts directly via legacy keyring (simulating external modification) + await inner.addAccounts(2); + + // Now create account at index 3 via wrapper + // This should detect the external changes and sync properly + const created = await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 3, + }); + + expect(created).toHaveLength(1); + + const account = created[0]; + const entropy = account?.options?.entropy; + expect( + entropy && 'groupIndex' in entropy ? entropy.groupIndex : undefined, + ).toBe(3); + + // Should have 4 accounts total (0, 1, 2, 3) + const allAccounts = await wrapper.getAccounts(); + expect(allAccounts).toHaveLength(4); + }); + + it('caches intermediate accounts when wrapper creates after inner adds multiple', async () => { + // Add multiple accounts directly via inner keyring (e.g., 0, 1, 2) + await inner.addAccounts(3); + + // Now create account at index 3 via wrapper + // The wrapper should cache all the intermediate accounts (0, 1, 2) that were added by inner + await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 3, + }); + + const allAccounts = await wrapper.getAccounts(); + expect(allAccounts).toHaveLength(4); + + // Verify all accounts are cached with correct groupIndex + const groupIndices = allAccounts + .map((a) => { + const ent = a.options.entropy; + return ent && 'groupIndex' in ent ? ent.groupIndex : -1; + }) + .sort((a, b) => a - b); + + expect(groupIndices).toStrictEqual([0, 1, 2, 3]); + + // Verify the accounts at indices 0, 1, 2 are in the cache + for (let i = 0; i < 3; i++) { + const account = allAccounts[i]; + expect(account).toBeDefined(); + if (account) { + // Access the account again to verify it's cached + const cachedAccount = await wrapper.getAccount(account.id); + // eslint-disable-next-line jest/no-conditional-expect + expect(cachedAccount).toBe(account); + } + } + }); + }); + + describe('deleteAccount', () => { + beforeEach(async () => { + // Create accounts sequentially at indices 0, 1, 2 + await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 0, + }); + await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 1, + }); + await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 2, + }); + }); + + it('removes an account from the keyring', async () => { + const accounts = await wrapper.getAccounts(); + expect(accounts.length).toBeGreaterThan(0); + const toDelete = accounts[0]; + expect(toDelete).toBeDefined(); + expect(toDelete?.id).toBeDefined(); + + if (toDelete?.id) { + await wrapper.deleteAccount(toDelete.id); + } + + const remaining = await wrapper.getAccounts(); + expect(remaining).toHaveLength(2); + expect(remaining.find((a) => a.id === toDelete?.id)).toBeUndefined(); + }); + + it('removes the account from the cache', async () => { + const accounts = await wrapper.getAccounts(); + expect(accounts.length).toBeGreaterThanOrEqual(2); + const toDelete = accounts[1]; + expect(toDelete).toBeDefined(); + expect(toDelete?.id).toBeDefined(); + + if (toDelete?.id) { + await wrapper.deleteAccount(toDelete.id); + } + + const remaining = await wrapper.getAccounts(); + expect(remaining.find((a) => a.id === toDelete?.id)).toBeUndefined(); + }); + + it('handles deleting middle account', async () => { + const accounts = await wrapper.getAccounts(); + expect(accounts.length).toBeGreaterThanOrEqual(2); + const middleAccount = accounts[1]; + expect(middleAccount).toBeDefined(); + expect(middleAccount?.id).toBeDefined(); + + if (middleAccount?.id) { + await wrapper.deleteAccount(middleAccount.id); + } + + const remaining = await wrapper.getAccounts(); + expect(remaining).toHaveLength(2); + }); + + it('syncs cache when legacy keyring removes accounts directly', async () => { + const accounts = await wrapper.getAccounts(); + expect(accounts).toHaveLength(3); + + // Remove an account directly via legacy keyring (simulating external modification) + const innerAccounts = await inner.getAccounts(); + expect(innerAccounts.length).toBeGreaterThan(0); + const firstAddress = innerAccounts[0]; + expect(firstAddress).toBeDefined(); + + // Type assertion safe here because we verified it exists + if (firstAddress) { + inner.removeAccount(firstAddress); + } + + // Now delete another account via wrapper + // This should detect the external change and sync properly + const remainingAccounts = await wrapper.getAccounts(); + expect(remainingAccounts.length).toBeGreaterThan(0); + const toDelete = remainingAccounts[0]; + expect(toDelete).toBeDefined(); + expect(toDelete?.id).toBeDefined(); + + if (toDelete?.id) { + await wrapper.deleteAccount(toDelete.id); + } + + const final = await wrapper.getAccounts(); + // Should have 1 account left (started with 3, removed 1 via legacy, removed 1 via wrapper) + expect(final).toHaveLength(1); + }); + + it('returns correct account by groupIndex after deletion', async () => { + // Create accounts at indices 0, 1, 2 + const accounts = await wrapper.getAccounts(); + expect(accounts).toHaveLength(3); + + const account0 = accounts.find( + (a) => + a.options.entropy && + 'groupIndex' in a.options.entropy && + a.options.entropy.groupIndex === 0, + ); + const account1 = accounts.find( + (a) => + a.options.entropy && + 'groupIndex' in a.options.entropy && + a.options.entropy.groupIndex === 1, + ); + const account2 = accounts.find( + (a) => + a.options.entropy && + 'groupIndex' in a.options.entropy && + a.options.entropy.groupIndex === 2, + ); + + expect(account0).toBeDefined(); + expect(account1).toBeDefined(); + expect(account2).toBeDefined(); + + // Delete account at groupIndex 1 + if (account1?.id) { + await wrapper.deleteAccount(account1.id); + } + + // After deletion, we should have accounts with groupIndex 0 and 2 + // but they'll be at array positions 0 and 1 + const remaining = await wrapper.getAccounts(); + expect(remaining).toHaveLength(2); + + // Now try to create an account at groupIndex 1 again + // This should return the EXISTING account at groupIndex 2, NOT create a new one + // The bug was that it would use array position [1] which is the account with groupIndex 2 + const result = await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 2, + }); + + // Should return the existing account with groupIndex 2 + expect(result).toHaveLength(1); + expect(result[0]?.id).toBe(account2?.id); + expect(result[0]?.address).toBe(account2?.address); + + // Should still have 2 accounts total + const final = await wrapper.getAccounts(); + expect(final).toHaveLength(2); + }); + + it('rejects creating account at non-sequential groupIndex after deletion', async () => { + // Create accounts at indices 0, 1, 2 + const accounts = await wrapper.getAccounts(); + expect(accounts).toHaveLength(3); + + const account1 = accounts.find( + (a) => + a.options.entropy && + 'groupIndex' in a.options.entropy && + a.options.entropy.groupIndex === 1, + ); + + expect(account1).toBeDefined(); + + // Delete account at groupIndex 1 + if (account1?.id) { + await wrapper.deleteAccount(account1.id); + } + + // After deletion, we have accounts with groupIndex 0 and 2 (inner size is 2) + const remaining = await wrapper.getAccounts(); + expect(remaining).toHaveLength(2); + + // Now try to create an account at groupIndex 3 (higher than inner size of 2) + // This should fail because the inner keyring only supports sequential creation at index 2 + await expect( + wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 3, + }), + ).rejects.toThrow( + 'Cannot create account at group index 3: inner keyring only supports sequential creation at index 2', + ); + + // Verify we can still create at the next sequential index (2) + // But this already exists, so it should return the existing account + const result = await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 2, + }); + + expect(result).toHaveLength(1); + const newAccount = result[0]; + const entropy = newAccount?.options.entropy; + expect( + entropy && 'groupIndex' in entropy ? entropy.groupIndex : undefined, + ).toBe(2); + + // Should still have 2 accounts total (the account at index 2 already existed) + const final = await wrapper.getAccounts(); + expect(final).toHaveLength(2); + }); + }); + + describe('exportAccount', () => { + beforeEach(async () => { + await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 0, + }); + }); + + it('exports an account as private key in hexadecimal encoding', async () => { + const accounts = await wrapper.getAccounts(); + const account = accounts[0]; + + // Assert account exists using a type-safe approach + expect(account).toBeDefined(); + + // TypeScript knows account could be undefined, but we've asserted it exists + // Use a safe access pattern + const accountId = account?.id ?? ''; + expect(accountId).not.toBe(''); + + const exported = await wrapper.exportAccount(accountId); + + expect(exported.type).toBe('private-key'); + expect(exported.privateKey).toMatch(/^0x[0-9a-fA-F]{64}$/u); + expect(exported.encoding).toBe(PrivateKeyEncoding.Hexadecimal); + }); + + it('accepts explicit hexadecimal encoding option', async () => { + const accounts = await wrapper.getAccounts(); + const account = accounts[0]; + + expect(account).toBeDefined(); + + const accountId = account?.id ?? ''; + expect(accountId).not.toBe(''); + + const exported = await wrapper.exportAccount(accountId, { + type: 'private-key', + encoding: PrivateKeyEncoding.Hexadecimal, + }); + + expect(exported.type).toBe('private-key'); + expect(exported.privateKey).toMatch(/^0x[0-9a-fA-F]{64}$/u); + }); + + it('throws error for unsupported encoding', async () => { + const accounts = await wrapper.getAccounts(); + const account = accounts[0]; + + expect(account).toBeDefined(); + + const accountId = account?.id ?? ''; + expect(accountId).not.toBe(''); + + await expect( + wrapper.exportAccount(accountId, { + type: 'private-key', + encoding: 'base58', + }), + ).rejects.toThrow('Unsupported encoding for Ethereum HD keyring'); + }); + }); + + describe('submitRequest', () => { + let accountId: AccountId; + + beforeEach(async () => { + const created = await wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: TEST_ENTROPY_SOURCE_ID, + groupIndex: 0, + }); + + const account = created[0]; + // Use a safe fallback that will cause tests to fail if account doesn't exist + accountId = account?.id ?? ('' as AccountId); + }); + + describe('eth_signTransaction', () => { + it('signs a transaction', async () => { + const txParams: TypedTxData = { + nonce: '0x00', + gasPrice: '0x09184e72a000', + gasLimit: '0x2710', + to: '0x0000000000000000000000000000000000000001', + value: '0x1000', + }; + const tx = TransactionFactory.fromTxData(txParams); + + const request = createMockRequest( + accountId, + EthMethod.SignTransaction, + [tx as unknown as Json], + ); + + const result = await wrapper.submitRequest(request); + expect(result).toBeDefined(); + expect(typeof result).toBe('object'); + }); + + it('throws error for missing params', async () => { + const request = createMockRequest( + accountId, + EthMethod.SignTransaction, + [], + ); + + await expect(wrapper.submitRequest(request)).rejects.toThrow( + 'Invalid params for eth_signTransaction', + ); + }); + }); + + describe('eth_sign', () => { + it('signs a message', async () => { + const request = createMockRequest(accountId, EthMethod.Sign, [ + '0x0000000000000000000000000000000000000000', + '0x68656c6c6f', + ]); + + const result = await wrapper.submitRequest(request); + expect(typeof result).toBe('string'); + expect((result as string).startsWith('0x')).toBe(true); + }); + + it('throws error for missing params', async () => { + const request = createMockRequest(accountId, EthMethod.Sign, [ + '0x0000000000000000000000000000000000000000', + ]); + + await expect(wrapper.submitRequest(request)).rejects.toThrow( + 'Invalid params for eth_sign', + ); + }); + }); + + describe('personal_sign', () => { + it('signs a personal message', async () => { + const request = createMockRequest(accountId, EthMethod.PersonalSign, [ + '0x68656c6c6f', + ]); + + const result = await wrapper.submitRequest(request); + expect(typeof result).toBe('string'); + expect((result as string).startsWith('0x')).toBe(true); + }); + + it('throws error for missing params', async () => { + const request = createMockRequest( + accountId, + EthMethod.PersonalSign, + [], + ); + + await expect(wrapper.submitRequest(request)).rejects.toThrow( + 'Invalid params for personal_sign', + ); + }); + }); + + describe('eth_signTypedData_v1', () => { + it('signs typed data v1', async () => { + const typedData = [ + { + type: 'string', + name: 'Message', + value: 'Hi, Alice!', + }, + ]; + + const request = createMockRequest( + accountId, + EthMethod.SignTypedDataV1, + ['0x0000000000000000000000000000000000000000', typedData], + ); + + const result = await wrapper.submitRequest(request); + expect(typeof result).toBe('string'); + }); + + it('throws error for missing params', async () => { + const request = createMockRequest( + accountId, + EthMethod.SignTypedDataV1, + ['0x0000000000000000000000000000000000000000'], + ); + + await expect(wrapper.submitRequest(request)).rejects.toThrow( + 'Invalid params for eth_signTypedData_v1', + ); + }); + }); + + describe('eth_signTypedData_v3', () => { + it('signs typed data v3', async () => { + const typedData = { + types: { + EIP712Domain: [ + { name: 'name', type: 'string' }, + { name: 'version', type: 'string' }, + { name: 'chainId', type: 'uint256' }, + ], + Message: [{ name: 'content', type: 'string' }], + }, + domain: { + name: 'Test', + version: '1', + chainId: 1, + }, + primaryType: 'Message', + message: { + content: 'Hello!', + }, + }; + + const request = createMockRequest( + accountId, + EthMethod.SignTypedDataV3, + [ + '0x0000000000000000000000000000000000000000', + typedData, // Pass object, not stringified + ], + ); + + const result = await wrapper.submitRequest(request); + expect(typeof result).toBe('string'); + }); + }); + + describe('eth_signTypedData_v4', () => { + it('signs typed data v4', async () => { + const typedData = { + types: { + EIP712Domain: [ + { name: 'name', type: 'string' }, + { name: 'version', type: 'string' }, + { name: 'chainId', type: 'uint256' }, + ], + Message: [{ name: 'content', type: 'string' }], + }, + domain: { + name: 'Test', + version: '1', + chainId: 1, + }, + primaryType: 'Message', + message: { + content: 'Hello!', + }, + }; + + const request = createMockRequest( + accountId, + EthMethod.SignTypedDataV4, + [ + '0x0000000000000000000000000000000000000000', + typedData, // Pass object, not stringified + ], + ); + + const result = await wrapper.submitRequest(request); + expect(typeof result).toBe('string'); + }); + }); + + describe('eth_getEncryptionPublicKey', () => { + it('gets encryption public key', async () => { + const request = createMockRequest( + accountId, + 'eth_getEncryptionPublicKey', + [], + ); + + const result = await wrapper.submitRequest(request); + expect(typeof result).toBe('string'); + }); + }); + + describe('eth_decrypt', () => { + it('decrypts a message', async () => { + // First get the encryption public key + const pubKeyRequest = createMockRequest( + accountId, + 'eth_getEncryptionPublicKey', + [], + ); + + const pubKey = await wrapper.submitRequest(pubKeyRequest); + + // Encrypt a message (this would normally be done externally) + const encryptedData = { + version: 'x25519-xsalsa20-poly1305', + nonce: 'test-nonce', + ephemPublicKey: String(pubKey), + ciphertext: 'test-ciphertext', + }; + + const decryptRequest = createMockRequest(accountId, 'eth_decrypt', [ + encryptedData, + ]); + + // This will fail with actual decryption due to invalid encrypted data + await expect(wrapper.submitRequest(decryptRequest)).rejects.toThrow( + 'Invalid padding: string should have whole number of bytes', + ); + }); + + it('throws error for missing params', async () => { + const request = createMockRequest(accountId, 'eth_decrypt', []); + + await expect(wrapper.submitRequest(request)).rejects.toThrow( + 'Invalid params for eth_decrypt', + ); + }); + }); + + describe('eth_getAppKeyAddress', () => { + it('gets app key address', async () => { + const request = createMockRequest(accountId, 'eth_getAppKeyAddress', [ + 'https://example.com', + ]); + + const result = await wrapper.submitRequest(request); + expect(typeof result).toBe('string'); + }); + + it('throws error for missing params', async () => { + const request = createMockRequest( + accountId, + 'eth_getAppKeyAddress', + [], + ); + + await expect(wrapper.submitRequest(request)).rejects.toThrow( + 'Invalid params for eth_getAppKeyAddress', + ); + }); + }); + + describe('eth_signEip7702Authorization', () => { + it('signs EIP-7702 authorization', async () => { + // EIP-7702 authorization format (array of tuples) + const authorization = [ + 1, // chainId + '0x0000000000000000000000000000000000000001', // address + 0, // nonce + ]; + + const request = createMockRequest( + accountId, + 'eth_signEip7702Authorization', + [authorization], + ); + + const result = await wrapper.submitRequest(request); + expect(result).toBeDefined(); + }); + + it('throws error for missing params', async () => { + const request = createMockRequest( + accountId, + 'eth_signEip7702Authorization', + [], + ); + + await expect(wrapper.submitRequest(request)).rejects.toThrow( + 'Invalid params for eth_signEip7702Authorization', + ); + }); + }); + + describe('unsupported methods', () => { + it('throws error for unsupported method', async () => { + const request = createMockRequest(accountId, 'unsupported_method', []); + + await expect(wrapper.submitRequest(request)).rejects.toThrow( + 'Unsupported method for HdKeyringWrapper', + ); + }); + }); + + describe('params validation', () => { + it('throws error when params is not an array', async () => { + const request = { + id: '00000000-0000-0000-0000-000000000000', + scope: EthScope.Eoa, + account: accountId, + origin: 'http://localhost', + request: { + method: EthMethod.PersonalSign, + params: 'invalid' as unknown as Json[], + }, + }; + + await expect(wrapper.submitRequest(request)).rejects.toThrow( + 'Expected params to be an array', + ); + }); + }); + }); +}); diff --git a/packages/keyring-eth-hd/src/hd-keyring-v2.ts b/packages/keyring-eth-hd/src/hd-keyring-v2.ts new file mode 100644 index 000000000..e5ba50308 --- /dev/null +++ b/packages/keyring-eth-hd/src/hd-keyring-v2.ts @@ -0,0 +1,400 @@ +import type { TypedTransaction } from '@ethereumjs/tx'; +import type { + EIP7702Authorization, + EthEncryptedData, + MessageTypes, + TypedDataV1, + TypedMessage, +} from '@metamask/eth-sig-util'; +import { SignTypedDataVersion } from '@metamask/eth-sig-util'; +import { + type CreateAccountOptions, + EthAccountType, + EthMethod, + EthScope, + type ExportAccountOptions, + type ExportedAccount, + type KeyringAccount, + KeyringAccountEntropyTypeOption, + type KeyringCapabilities, + type KeyringRequest, + KeyringType, + type KeyringV2, + KeyringWrapper, + PrivateKeyEncoding, +} from '@metamask/keyring-api'; +import type { AccountId } from '@metamask/keyring-utils'; +import type { Hex, Json } from '@metamask/utils'; + +import type { DeserializableHDKeyringState, HdKeyring } from './hd-keyring'; + +/** + * Additional Ethereum methods supported by HD keyring that are not in the standard EthMethod enum. + * These are primarily encryption and utility methods. + */ +enum HdKeyringEthMethod { + Decrypt = 'eth_decrypt', + GetEncryptionPublicKey = 'eth_getEncryptionPublicKey', + GetAppKeyAddress = 'eth_getAppKeyAddress', + SignEip7702Authorization = 'eth_signEip7702Authorization', +} + +const hdKeyringV2Capabilities: KeyringCapabilities = { + scopes: [EthScope.Eoa], + bip44: { + deriveIndex: true, + derivePath: false, + discover: false, + }, +}; + +/** + * Methods supported by HD keyring EOA accounts. + * Combines standard Ethereum methods with HD keyring-specific methods. + */ +const HD_KEYRING_EOA_METHODS = [ + EthMethod.SignTransaction, + EthMethod.Sign, + EthMethod.PersonalSign, + EthMethod.SignTypedDataV1, + EthMethod.SignTypedDataV3, + EthMethod.SignTypedDataV4, + HdKeyringEthMethod.Decrypt, + HdKeyringEthMethod.GetEncryptionPublicKey, + HdKeyringEthMethod.GetAppKeyAddress, + HdKeyringEthMethod.SignEip7702Authorization, +]; + +/** + * Concrete {@link KeyringV2} adapter for {@link HdKeyring}. + * + * This wrapper exposes the accounts and signing capabilities of the legacy + * HD keyring via the unified V2 interface. + */ +export type HdKeyringV2Options = { + legacyKeyring: HdKeyring; + entropySourceId: string; +}; + +export class HdKeyringV2 + extends KeyringWrapper + implements KeyringV2 +{ + /** + * In-memory cache of KeyringAccount objects. + * Maps address (as Hex) to KeyringAccount. + */ + readonly #accountsCache = new Map(); + + constructor(options: HdKeyringV2Options) { + super({ + type: KeyringType.Hd, + inner: options.legacyKeyring, + capabilities: hdKeyringV2Capabilities, + entropySourceId: options.entropySourceId, + }); + } + + /** + * Helper method to safely cast a KeyringAccount address to Hex type. + * The KeyringAccount.address is typed as string, but for Ethereum accounts + * it should always be a valid Hex address. + * + * @param address - The address from a KeyringAccount. + * @returns The address as Hex type. + */ + #toHexAddress(address: string): Hex { + return address as Hex; + } + + /** + * Creates a KeyringAccount object for the given address and index. + * + * @param address - The account address. + * @param addressIndex - The account index in the HD path. + * @returns The created KeyringAccount. + */ + #createKeyringAccount(address: Hex, addressIndex: number): KeyringAccount { + const id = this.resolver.register(address); + + const account: KeyringAccount = { + id, + type: EthAccountType.Eoa, + address, + scopes: this.capabilities.scopes, + methods: HD_KEYRING_EOA_METHODS, + options: { + entropy: { + derivationPath: `${this.inner.hdPath}/${addressIndex}`, + groupIndex: addressIndex, + id: this.entropySourceId, + type: KeyringAccountEntropyTypeOption.Mnemonic, + }, + }, + }; + + // Cache the account + this.#accountsCache.set(address, account); + + return account; + } + + async getAccounts(): Promise { + const addresses = await this.inner.getAccounts(); + + return addresses.map((address, addressIndex) => { + // Check if we already have this account cached + const cached = this.#accountsCache.get(address); + if (cached) { + return cached; + } + + // Create and cache the account if not already cached + return this.#createKeyringAccount(address, addressIndex); + }); + } + + async deserialize(state: Json): Promise { + // Clear the cache when deserializing + this.#accountsCache.clear(); + + // Deserialize the legacy keyring + await this.inner.deserialize( + state as Partial, + ); + + // Rebuild the cache by populating it with all accounts + // We call getAccounts() which will repopulate the cache as a side effect + await this.getAccounts(); + } + + async createAccounts( + options: CreateAccountOptions, + ): Promise { + // For HD keyring, we only support BIP-44 derive index + if (options.type !== 'bip44:derive-index') { + throw new Error( + `Unsupported account creation type for HdKeyring: ${options.type}`, + ); + } + + // Validate that the entropy source matches this keyring's entropy source + if (options.entropySource !== this.entropySourceId) { + throw new Error( + `Entropy source mismatch: expected '${this.entropySourceId}', got '${options.entropySource}'`, + ); + } + + // Sync with the inner keyring state in case it was modified externally + // This ensures our cache is up-to-date before we make changes + const currentAccounts = await this.getAccounts(); + const currentCount = currentAccounts.length; + const targetIndex = options.groupIndex; + + // Check if an account with this groupIndex already exists + // We need to search by groupIndex value, not array position, because after + // account deletion the array indices don't match the groupIndex values + const existingAccount = currentAccounts.find( + (account) => + account.options.entropy && + 'groupIndex' in account.options.entropy && + account.options.entropy.groupIndex === targetIndex, + ); + + if (existingAccount) { + return [existingAccount]; + } + + // The inner HD keyring only supports sequential account creation starting from + // walletMap.size. It does NOT support creating accounts at arbitrary derivation + // indices. After account deletion, those derivation indices are permanently lost. + // + // We enforce this limitation: reject requests for groupIndex values that would + // require the inner keyring to skip indices (i.e., when there are gaps from deletions). + const innerSize = (await this.inner.getAccounts()).length; + if (targetIndex !== innerSize) { + throw new Error( + `Cannot create account at group index ${targetIndex}: ` + + `inner keyring only supports sequential creation at index ${innerSize}. ` + + `Account deletion creates permanent gaps in the derivation index sequence.`, + ); + } + + // Calculate how many accounts we need to add to reach the target index + const accountsToAdd = targetIndex - currentCount + 1; + + if (accountsToAdd > 0) { + await this.inner.addAccounts(accountsToAdd); + } + + // Get the updated addresses after adding new accounts + const updatedAddresses = await this.inner.getAccounts(); + const targetAddress = updatedAddresses[targetIndex]; + + if (!targetAddress) { + throw new Error(`Failed to create account at group index ${targetIndex}`); + } + + // Create and cache the KeyringAccount for the target address + const newAccount = this.#createKeyringAccount(targetAddress, targetIndex); + + return [newAccount]; + } + + /** + * Delete an account from the keyring. + * + * ⚠️ Warning: Deleting accounts may invalidate the groupIndex values stored + * in cached KeyringAccount objects, since the legacy keyring uses array + * indices. Consider clearing the cache if account order consistency is critical. + * + * @param accountId - The account ID to delete. + */ + async deleteAccount(accountId: AccountId): Promise { + // Sync with the inner keyring state in case it was modified externally + // This ensures our cache is up-to-date before we make changes + await this.getAccounts(); + + const account = await this.getAccount(accountId); + const hexAddress = this.#toHexAddress(account.address); + + // Remove from the legacy keyring + this.inner.removeAccount(hexAddress); + + // Remove from our cache + this.#accountsCache.delete(hexAddress); + } + + async exportAccount( + accountId: AccountId, + options?: ExportAccountOptions, + ): Promise { + const account = await this.getAccount(accountId); + + // Validate encoding - we only support hexadecimal for Ethereum keys + const requestedEncoding = + options?.encoding ?? PrivateKeyEncoding.Hexadecimal; + + if (requestedEncoding !== PrivateKeyEncoding.Hexadecimal) { + throw new Error( + `Unsupported encoding for Ethereum HD keyring: ${requestedEncoding}. Only '${PrivateKeyEncoding.Hexadecimal}' is supported.`, + ); + } + + // The legacy HdKeyring returns a hex string without 0x prefix. + const privateKeyHex = await this.inner.exportAccount( + this.#toHexAddress(account.address), + ); + + const exported: ExportedAccount = { + type: 'private-key', + privateKey: `0x${privateKeyHex}`, + encoding: PrivateKeyEncoding.Hexadecimal, + }; + + return exported; + } + + async submitRequest(request: KeyringRequest): Promise { + const { method, params = [] } = request.request; + + const { address } = await this.getAccount(request.account); + const hexAddress = this.#toHexAddress(address); + + // Validate params is an array + if (!Array.isArray(params)) { + throw new Error('Expected params to be an array'); + } + + switch (method) { + case `${EthMethod.SignTransaction}`: { + if (params.length < 1) { + throw new Error('Invalid params for eth_signTransaction'); + } + const [tx] = params; + return this.inner.signTransaction( + hexAddress, + tx as unknown as TypedTransaction, + ) as unknown as Json; + } + + case `${EthMethod.Sign}`: { + if (params.length < 2) { + throw new Error('Invalid params for eth_sign'); + } + const [, data] = params; + return this.inner.signMessage(hexAddress, data as string); + } + + case `${EthMethod.PersonalSign}`: { + if (params.length < 1) { + throw new Error('Invalid params for personal_sign'); + } + const [data] = params; + return this.inner.signPersonalMessage(hexAddress, data as string); + } + + case `${EthMethod.SignTypedDataV1}`: + case `${EthMethod.SignTypedDataV3}`: + case `${EthMethod.SignTypedDataV4}`: { + if (params.length < 2) { + throw new Error(`Invalid params for ${method}`); + } + const [, data] = params; + let version: SignTypedDataVersion; + if (method === EthMethod.SignTypedDataV4) { + version = SignTypedDataVersion.V4; + } else if (method === EthMethod.SignTypedDataV3) { + version = SignTypedDataVersion.V3; + } else { + version = SignTypedDataVersion.V1; + } + + return this.inner.signTypedData( + hexAddress, + data as unknown as TypedDataV1 | TypedMessage, + { + version, + }, + ); + } + + case `${HdKeyringEthMethod.Decrypt}`: { + if (params.length < 1) { + throw new Error('Invalid params for eth_decrypt'); + } + const [encryptedData] = params; + return this.inner.decryptMessage( + hexAddress, + encryptedData as unknown as EthEncryptedData, + ); + } + + case `${HdKeyringEthMethod.GetEncryptionPublicKey}`: { + return this.inner.getEncryptionPublicKey(hexAddress); + } + + case `${HdKeyringEthMethod.GetAppKeyAddress}`: { + if (params.length < 1) { + throw new Error('Invalid params for eth_getAppKeyAddress'); + } + const [origin] = params; + return this.inner.getAppKeyAddress(hexAddress, origin as string); + } + + case `${HdKeyringEthMethod.SignEip7702Authorization}`: { + if (params.length < 1) { + throw new Error('Invalid params for eth_signEip7702Authorization'); + } + const [authorization] = params; + return this.inner.signEip7702Authorization( + hexAddress, + authorization as unknown as EIP7702Authorization, + ); + } + + default: + throw new Error(`Unsupported method for HdKeyringWrapper: ${method}`); + } + } +} diff --git a/packages/keyring-eth-hd/src/index.ts b/packages/keyring-eth-hd/src/index.ts index 05e9f7276..5fcde30c2 100644 --- a/packages/keyring-eth-hd/src/index.ts +++ b/packages/keyring-eth-hd/src/index.ts @@ -5,3 +5,4 @@ export type { HDKeyringOptions, HDKeyringAccountSelectionOptions, } from './hd-keyring'; +export { HdKeyringV2, HdKeyringV2Options } from './hd-keyring-v2'; diff --git a/packages/keyring-eth-hd/tsconfig.build.json b/packages/keyring-eth-hd/tsconfig.build.json index 684ef9c88..63bfcde80 100644 --- a/packages/keyring-eth-hd/tsconfig.build.json +++ b/packages/keyring-eth-hd/tsconfig.build.json @@ -10,6 +10,9 @@ "references": [ { "path": "../keyring-utils/tsconfig.build.json" + }, + { + "path": "../keyring-api/tsconfig.build.json" } ], "include": ["./src/**/*.ts", "./src/**/*.js"], diff --git a/packages/keyring-eth-hd/tsconfig.json b/packages/keyring-eth-hd/tsconfig.json index ea50b31be..d69b59c1b 100644 --- a/packages/keyring-eth-hd/tsconfig.json +++ b/packages/keyring-eth-hd/tsconfig.json @@ -6,7 +6,7 @@ "exactOptionalPropertyTypes": false, "target": "es2017" }, - "references": [{ "path": "../keyring-utils" }], + "references": [{ "path": "../keyring-utils" }, { "path": "../keyring-api" }], "include": ["./src", "./types"], "exclude": ["./dist/**/*"] } diff --git a/packages/keyring-eth-ledger-bridge/CHANGELOG.md b/packages/keyring-eth-ledger-bridge/CHANGELOG.md index 20f2b81df..90156f774 100644 --- a/packages/keyring-eth-ledger-bridge/CHANGELOG.md +++ b/packages/keyring-eth-ledger-bridge/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `LedgerKeyringV2` class implementing KeyringV2 interface ([#398](https://github.com/MetaMask/accounts/pull/398)) + - Wraps legacy `LedgerKeyring` to expose accounts via the unified KeyringV2 API and the `KeyringAccount` type. + ## [11.1.2] ### Changed diff --git a/packages/keyring-eth-ledger-bridge/jest.config.js b/packages/keyring-eth-ledger-bridge/jest.config.js index f20625df1..6613160ff 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: 91.89, - functions: 97.93, - lines: 97.28, - statements: 97.31, + branches: 92.54, + functions: 98.11, + lines: 97.61, + statements: 97.63, }, }, }); diff --git a/packages/keyring-eth-ledger-bridge/package.json b/packages/keyring-eth-ledger-bridge/package.json index 495a21932..cad06bded 100644 --- a/packages/keyring-eth-ledger-bridge/package.json +++ b/packages/keyring-eth-ledger-bridge/package.json @@ -51,6 +51,7 @@ "@ethereumjs/util": "^9.1.0", "@ledgerhq/hw-app-eth": "^6.42.0", "@metamask/eth-sig-util": "^8.2.0", + "@metamask/keyring-api": "workspace:^", "hdkey": "^2.1.0" }, "devDependencies": { diff --git a/packages/keyring-eth-ledger-bridge/src/index.ts b/packages/keyring-eth-ledger-bridge/src/index.ts index c84c944fc..721f511cf 100644 --- a/packages/keyring-eth-ledger-bridge/src/index.ts +++ b/packages/keyring-eth-ledger-bridge/src/index.ts @@ -5,3 +5,7 @@ export type * from './ledger-bridge'; export * from './ledger-transport-middleware'; export type * from './type'; export * from './ledger-hw-app'; +export { + LedgerKeyringV2, + type LedgerKeyringV2Options, +} from './ledger-keyring-v2'; diff --git a/packages/keyring-eth-ledger-bridge/src/ledger-keyring-v2.test.ts b/packages/keyring-eth-ledger-bridge/src/ledger-keyring-v2.test.ts new file mode 100644 index 000000000..a74373af8 --- /dev/null +++ b/packages/keyring-eth-ledger-bridge/src/ledger-keyring-v2.test.ts @@ -0,0 +1,318 @@ +import type { TypedTransaction } from '@ethereumjs/tx'; +import { SignTypedDataVersion } from '@metamask/eth-sig-util'; +import { + EthMethod, + EthScope, + KeyringAccountEntropyTypeOption, + type KeyringRequest, +} from '@metamask/keyring-api'; +import type { AccountId } from '@metamask/keyring-utils'; +import type { Hex, Json } from '@metamask/utils'; +import HDKey from 'hdkey'; + +import type { LedgerBridge, LedgerBridgeOptions } from './ledger-bridge'; +import { LedgerKeyring } from './ledger-keyring'; +import { LedgerKeyringV2 } from './ledger-keyring-v2'; + +const TEST_ENTROPY_SOURCE_ID = 'test-entropy-source-id'; + +const fakeXPubKey = + 'xpub6FnCn6nSzZAw5Tw7cgR9bi15UV96gLZhjDstkXXxvCLsUXBGXPdSnLFbdpq8p9HmGsApME5hQTZ3emM2rnY5agb9rXpVGyy3bdW6EEgAtqt'; +const fakeHdKey = HDKey.fromExtendedKey(fakeXPubKey); + +/** + * Creates a mock KeyringRequest for testing. + * + * @param accountId - The account ID for the request. + * @param method - The method to invoke. + * @param params - Optional parameters for the request. + * @returns A mock KeyringRequest. + */ +function createMockRequest( + accountId: AccountId, + method: string, + params: Json[] = [], +): KeyringRequest { + return { + id: '00000000-0000-0000-0000-000000000000', + scope: EthScope.Eoa, + account: accountId, + origin: 'http://localhost', + request: { + method, + params, + }, + }; +} + +/** + * Get a mock bridge for the LedgerKeyring. + * + * @returns A mock bridge. + */ +function getMockBridge(): LedgerBridge { + return { + init: jest.fn().mockResolvedValue(undefined), + destroy: jest.fn().mockResolvedValue(undefined), + isDeviceConnected: true, + getPublicKey: jest.fn(), + deviceSignTransaction: jest.fn(), + deviceSignMessage: jest.fn(), + deviceSignTypedData: jest.fn(), + attemptMakeApp: jest.fn().mockResolvedValue(true), + updateTransportMethod: jest.fn().mockResolvedValue(true), + } as unknown as LedgerBridge; +} + +describe('LedgerKeyringV2', () => { + let inner: LedgerKeyring; + let wrapper: LedgerKeyringV2; + let bridge: LedgerBridge; + + beforeEach(async () => { + bridge = getMockBridge(); + inner = new LedgerKeyring({ bridge }); + await inner.init(); + // Set the HDKey directly to avoid needing a real unlock + // eslint-disable-next-line require-atomic-updates + inner.hdk = fakeHdKey; + wrapper = new LedgerKeyringV2({ + legacyKeyring: inner, + entropySourceId: TEST_ENTROPY_SOURCE_ID, + }); + }); + + it('constructs with expected type and capabilities', () => { + expect(wrapper.type).toBe('ledger'); + expect(wrapper.capabilities.scopes).toStrictEqual([EthScope.Eoa]); + }); + + it('deserializes via the inner keyring and rebuilds the cache', async () => { + await inner.addAccounts(2); + const serializedState = await inner.serialize(); + + await wrapper.deserialize(serializedState); + + const accounts = await wrapper.getAccounts(); + expect(accounts).toHaveLength(2); + }); + + describe('getAccounts and caching', () => { + beforeEach(async () => { + await inner.addAccounts(2); + }); + + it('returns accounts and caches them', async () => { + const a1 = await wrapper.getAccounts(); + const a2 = await wrapper.getAccounts(); + + expect(a1).toHaveLength(2); + expect(a1[0]).toBe(a2[0]); + expect(a1[1]).toBe(a2[1]); + }); + + it('sets entropy options as private-key type', async () => { + const accounts = await wrapper.getAccounts(); + expect(accounts).toHaveLength(2); + + for (const account of accounts) { + expect(account.options.entropy).toBeDefined(); + expect(account.options.entropy?.type).toBe( + KeyringAccountEntropyTypeOption.PrivateKey, + ); + } + }); + }); + + describe('createAccounts', () => { + it('creates an account via legacy keyring and returns it', async () => { + const created = await wrapper.createAccounts(); + expect(created).toHaveLength(1); + + const all = await wrapper.getAccounts(); + expect(all).toHaveLength(1); + }); + + it('throws when the legacy keyring does not return an address', async () => { + jest + .spyOn(inner, 'addAccounts') + .mockResolvedValueOnce([undefined] as unknown as Hex[]); + + await expect(wrapper.createAccounts()).rejects.toThrow( + 'Failed to create Ledger keyring account', + ); + }); + }); + + describe('deleteAccount', () => { + beforeEach(async () => { + await inner.addAccounts(2); + }); + + it('removes account from legacy and cache', async () => { + const accounts = await wrapper.getAccounts(); + expect(accounts.length).toBeGreaterThanOrEqual(2); + + const toDelete = accounts[0]; + expect(toDelete).toBeDefined(); + + if (toDelete?.id) { + await wrapper.deleteAccount(toDelete.id); + } + + const remaining = await wrapper.getAccounts(); + expect(remaining.find((a) => a.id === toDelete?.id)).toBeUndefined(); + }); + }); + + describe('submitRequest', () => { + let accountId: AccountId; + + beforeEach(async () => { + const createdAccounts = await wrapper.createAccounts(); + const [account] = createdAccounts; + if (!account) { + throw new Error('Expected at least one account'); + } + accountId = account.id; + }); + + it('signs a transaction', async () => { + const tx = {} as unknown as TypedTransaction; + + const mockTx = { + getSenderAddress: jest + .fn() + .mockReturnValue(Buffer.from('0'.repeat(40), 'hex')), + verifySignature: jest.fn().mockReturnValue(true), + } as unknown as TypedTransaction; + + const signTransactionSpy = jest + .spyOn(inner, 'signTransaction') + .mockResolvedValueOnce(mockTx); + + const req = createMockRequest(accountId, EthMethod.SignTransaction, [ + tx as unknown as Json, + ]); + + // eslint-disable-next-line jest/no-restricted-matchers + await expect(wrapper.submitRequest(req)).resolves.toStrictEqual(mockTx); + + expect(signTransactionSpy).toHaveBeenCalledWith(expect.any(String), tx); + }); + + it('throws when params are missing for eth_signTransaction', async () => { + const req = createMockRequest(accountId, EthMethod.SignTransaction, []); + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + 'Invalid params for eth_signTransaction', + ); + }); + + it('signs eth_sign', async () => { + const signMessageSpy = jest + .spyOn(inner, 'signMessage') + .mockResolvedValueOnce('0xsignature'); + + const req = createMockRequest(accountId, EthMethod.Sign, [ + '0x0000000000000000000000000000000000000000', + '0x68656c6c6f', + ]); + const res = await wrapper.submitRequest(req); + expect(res).toBe('0xsignature'); + expect(signMessageSpy).toHaveBeenCalledWith( + expect.any(String), + '0x68656c6c6f', + ); + }); + + it('throws when params are missing for eth_sign', async () => { + const req = createMockRequest(accountId, EthMethod.Sign, []); + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + 'Invalid params for eth_sign', + ); + }); + + it('signs personal_sign', async () => { + const signPersonalMessageSpy = jest + .spyOn(inner, 'signPersonalMessage') + .mockResolvedValueOnce('0xsignature'); + + const req = createMockRequest(accountId, EthMethod.PersonalSign, [ + '0x68656c6c6f', + ]); + const res = await wrapper.submitRequest(req); + expect(res).toBe('0xsignature'); + expect(signPersonalMessageSpy).toHaveBeenCalledWith( + expect.any(String), + '0x68656c6c6f', + ); + }); + + it('throws when params are missing for personal_sign', async () => { + const req = createMockRequest(accountId, EthMethod.PersonalSign, []); + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + 'Invalid params for personal_sign', + ); + }); + + it('signs typed data V4', async () => { + const typedData = { foo: 'bar4' }; + + const signTypedDataSpy = jest + .spyOn(inner, 'signTypedData') + .mockResolvedValueOnce('0xv4'); + + const req = createMockRequest(accountId, EthMethod.SignTypedDataV4, [ + '0x0000000000000000000000000000000000000000', + typedData as unknown as Json, + ]); + + const res = await wrapper.submitRequest(req); + + expect(res).toBe('0xv4'); + expect(signTypedDataSpy).toHaveBeenCalledWith( + expect.any(String), + typedData, + { version: SignTypedDataVersion.V4 }, + ); + }); + + it('throws when params are missing for typed data methods', async () => { + const req = createMockRequest(accountId, EthMethod.SignTypedDataV4, []); + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + `Invalid params for ${EthMethod.SignTypedDataV4}`, + ); + }); + + it('throws for unsupported method', async () => { + const method = 'unsupported_method'; + const req = createMockRequest(accountId, method, []); + await expect(wrapper.submitRequest(req)).rejects.toThrow( + `Unsupported method for LedgerKeyringV2: ${method}`, + ); + }); + + it('throws when params is not an array', async () => { + const req = createMockRequest(accountId, EthMethod.PersonalSign, []); + // @ts-expect-error Intentionally provide invalid params shape + req.request.params = null; + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + 'Expected params to be an array', + ); + }); + + it('defaults params to empty array when undefined', async () => { + const req = createMockRequest(accountId, EthMethod.PersonalSign, []); + delete req.request.params; + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + 'Invalid params for personal_sign', + ); + }); + }); +}); diff --git a/packages/keyring-eth-ledger-bridge/src/ledger-keyring-v2.ts b/packages/keyring-eth-ledger-bridge/src/ledger-keyring-v2.ts new file mode 100644 index 000000000..6eae117cd --- /dev/null +++ b/packages/keyring-eth-ledger-bridge/src/ledger-keyring-v2.ts @@ -0,0 +1,216 @@ +import type { TypedTransaction } from '@ethereumjs/tx'; +import type { MessageTypes, TypedMessage } from '@metamask/eth-sig-util'; +import { SignTypedDataVersion } from '@metamask/eth-sig-util'; +import { + EthAccountType, + EthMethod, + EthScope, + KeyringAccountEntropyTypeOption, + type KeyringAccount, + type KeyringCapabilities, + type KeyringRequest, + KeyringType, + type KeyringV2, + KeyringWrapper, +} from '@metamask/keyring-api'; +import type { AccountId } from '@metamask/keyring-utils'; +import type { Hex, Json } from '@metamask/utils'; + +import type { + LedgerKeyring, + LedgerKeyringSerializedState, +} from './ledger-keyring'; + +const ledgerKeyringV2Capabilities: KeyringCapabilities = { + scopes: [EthScope.Eoa], +}; + +/** + * Methods supported by Ledger keyring EOA accounts. + * Ledger keyrings support signing transactions, personal messages, and typed data v4. + */ +const LEDGER_KEYRING_EOA_METHODS = [ + EthMethod.SignTransaction, + EthMethod.Sign, + EthMethod.PersonalSign, + EthMethod.SignTypedDataV4, +]; + +export type LedgerKeyringV2Options = { + legacyKeyring: LedgerKeyring; + entropySourceId: string; +}; + +export class LedgerKeyringV2 + extends KeyringWrapper + implements KeyringV2 +{ + /** + * In-memory cache of KeyringAccount objects. + * Maps address (as Hex) to KeyringAccount. + */ + readonly #accountsCache = new Map(); + + constructor(options: LedgerKeyringV2Options) { + super({ + type: KeyringType.Ledger, + inner: options.legacyKeyring, + capabilities: ledgerKeyringV2Capabilities, + entropySourceId: options.entropySourceId, + }); + } + + /** + * Helper method to safely cast a KeyringAccount address to Hex type. + * The KeyringAccount.address is typed as string, but for Ethereum accounts + * it should always be a valid Hex address. + * + * @param address - The address from a KeyringAccount. + * @returns The address as Hex type. + */ + #toHexAddress(address: string): Hex { + return address as Hex; + } + + /** + * Creates a KeyringAccount object for the given address. + * + * @param address - The account address. + * @returns The created KeyringAccount. + */ + #createKeyringAccount(address: Hex): KeyringAccount { + const id = this.resolver.register(address); + + const account: KeyringAccount = { + id, + type: EthAccountType.Eoa, + address, + scopes: this.capabilities.scopes, + methods: LEDGER_KEYRING_EOA_METHODS, + options: { + entropy: { + type: KeyringAccountEntropyTypeOption.PrivateKey, + }, + }, + }; + + // Cache the account + this.#accountsCache.set(address, account); + + return account; + } + + async getAccounts(): Promise { + const addresses = await this.inner.getAccounts(); + + return addresses.map((address) => { + // Check if we already have this account cached + const cached = this.#accountsCache.get(address); + if (cached) { + return cached; + } + + // Create and cache the account if not already cached + return this.#createKeyringAccount(address); + }); + } + + async deserialize(state: Json): Promise { + // Clear the cache when deserializing + this.#accountsCache.clear(); + + // Deserialize the legacy keyring + await this.inner.deserialize(state as LedgerKeyringSerializedState); + + // Rebuild the cache by populating it with all accounts + // We call getAccounts() which will repopulate the cache as a side effect + await this.getAccounts(); + } + + async createAccounts(): Promise { + const [address] = await this.inner.addAccounts(1); + + if (!address) { + throw new Error('Failed to create Ledger keyring account'); + } + + const hexAddress = this.#toHexAddress(address); + const account = this.#createKeyringAccount(hexAddress); + + return [account]; + } + + async deleteAccount(accountId: AccountId): Promise { + // Sync with the inner keyring state in case it was modified externally + // This ensures our cache is up-to-date before we make changes + await this.getAccounts(); + + const account = await this.getAccount(accountId); + const hexAddress = this.#toHexAddress(account.address); + + // Remove from the legacy keyring + this.inner.removeAccount(hexAddress); + + // Remove from our cache + this.#accountsCache.delete(hexAddress); + } + + async submitRequest(request: KeyringRequest): Promise { + const { method, params = [] } = request.request; + + const { address } = await this.getAccount(request.account); + const hexAddress = this.#toHexAddress(address); + + // Validate params is an array + if (!Array.isArray(params)) { + throw new Error('Expected params to be an array'); + } + + switch (method) { + case `${EthMethod.SignTransaction}`: { + if (params.length < 1) { + throw new Error('Invalid params for eth_signTransaction'); + } + const [tx] = params; + return this.inner.signTransaction( + hexAddress, + tx as unknown as TypedTransaction, + ) as unknown as Json; + } + + case `${EthMethod.Sign}`: { + if (params.length < 2) { + throw new Error('Invalid params for eth_sign'); + } + const [, data] = params; + return this.inner.signMessage(hexAddress, data as string); + } + + case `${EthMethod.PersonalSign}`: { + if (params.length < 1) { + throw new Error('Invalid params for personal_sign'); + } + const [data] = params; + return this.inner.signPersonalMessage(hexAddress, data as string); + } + + case `${EthMethod.SignTypedDataV4}`: { + if (params.length < 2) { + throw new Error(`Invalid params for ${method}`); + } + const [, data] = params; + + return this.inner.signTypedData( + hexAddress, + data as unknown as TypedMessage, + { + version: SignTypedDataVersion.V4, + }, + ); + } + + default: + throw new Error(`Unsupported method for LedgerKeyringV2: ${method}`); + } + } +} diff --git a/packages/keyring-eth-ledger-bridge/tsconfig.build.json b/packages/keyring-eth-ledger-bridge/tsconfig.build.json index 4cb6c93da..f23f77fd0 100644 --- a/packages/keyring-eth-ledger-bridge/tsconfig.build.json +++ b/packages/keyring-eth-ledger-bridge/tsconfig.build.json @@ -6,7 +6,10 @@ "rootDir": "src", "exactOptionalPropertyTypes": false }, - "references": [{ "path": "../keyring-utils/tsconfig.build.json" }], + "references": [ + { "path": "../keyring-utils/tsconfig.build.json" }, + { "path": "../keyring-api/tsconfig.build.json" } + ], "include": ["./src/**/*.ts"], "exclude": ["./src/**/*.test.ts"] } diff --git a/packages/keyring-eth-ledger-bridge/tsconfig.json b/packages/keyring-eth-ledger-bridge/tsconfig.json index b46028b05..4b4cc95cf 100644 --- a/packages/keyring-eth-ledger-bridge/tsconfig.json +++ b/packages/keyring-eth-ledger-bridge/tsconfig.json @@ -5,7 +5,7 @@ "exactOptionalPropertyTypes": false, "target": "es2017" }, - "references": [{ "path": "../keyring-utils" }], + "references": [{ "path": "../keyring-utils" }, { "path": "../keyring-api" }], "include": ["./src"], "exclude": ["./dist/**/*"] } diff --git a/packages/keyring-eth-qr/CHANGELOG.md b/packages/keyring-eth-qr/CHANGELOG.md index a4ed08a05..6c1548848 100644 --- a/packages/keyring-eth-qr/CHANGELOG.md +++ b/packages/keyring-eth-qr/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `QrKeyringV2` class implementing KeyringV2 interface ([#398](https://github.com/MetaMask/accounts/pull/398)) + - Wraps legacy `QrKeyring` to expose accounts via the unified KeyringV2 API and the `KeyringAccount` type. + ## [1.1.0] ### Added diff --git a/packages/keyring-eth-qr/package.json b/packages/keyring-eth-qr/package.json index 080ecb5d8..3dede05d4 100644 --- a/packages/keyring-eth-qr/package.json +++ b/packages/keyring-eth-qr/package.json @@ -53,6 +53,7 @@ "@ethereumjs/util": "^9.1.0", "@keystonehq/bc-ur-registry-eth": "^0.19.1", "@metamask/eth-sig-util": "^8.2.0", + "@metamask/keyring-api": "workspace:^", "@metamask/keyring-utils": "workspace:^", "@metamask/utils": "^11.1.0", "async-mutex": "^0.5.0", diff --git a/packages/keyring-eth-qr/src/index.ts b/packages/keyring-eth-qr/src/index.ts index 36a60d855..3fdfed9e4 100644 --- a/packages/keyring-eth-qr/src/index.ts +++ b/packages/keyring-eth-qr/src/index.ts @@ -17,3 +17,4 @@ export { type SerializedQrKeyringState, type SerializedUR, } from './qr-keyring'; +export { QrKeyringV2, type QrKeyringV2Options } from './qr-keyring-v2'; diff --git a/packages/keyring-eth-qr/src/qr-keyring-v2.test.ts b/packages/keyring-eth-qr/src/qr-keyring-v2.test.ts new file mode 100644 index 000000000..bb559477e --- /dev/null +++ b/packages/keyring-eth-qr/src/qr-keyring-v2.test.ts @@ -0,0 +1,279 @@ +import type { TypedTransaction, TypedTxData } from '@ethereumjs/tx'; +import { + EthMethod, + EthScope, + KeyringAccountEntropyTypeOption, + type KeyringRequest, +} from '@metamask/keyring-api'; +import type { AccountId } from '@metamask/keyring-utils'; +import type { Hex, Json } from '@metamask/utils'; + +import { QrKeyring, type QrKeyringBridge } from './qr-keyring'; +import { QrKeyringV2 } from './qr-keyring-v2'; +import { KNOWN_HDKEY_UR } from '../test/fixtures'; + +const TEST_ENTROPY_SOURCE_ID = 'test-entropy-source-id'; + +/** + * Creates a mock KeyringRequest for testing. + * + * @param accountId - The account ID for the request. + * @param method - The method to invoke. + * @param params - Optional parameters for the request. + * @returns A mock KeyringRequest. + */ +function createMockRequest( + accountId: AccountId, + method: string, + params: Json[] = [], +): KeyringRequest { + return { + id: '00000000-0000-0000-0000-000000000000', + scope: EthScope.Eoa, + account: accountId, + origin: 'http://localhost', + request: { + method, + params, + }, + }; +} + +/** + * Get a mock bridge for the QrKeyring. + * + * @returns A mock bridge with a requestScan method. + */ +function getMockBridge(): QrKeyringBridge { + return { + requestScan: jest.fn(), + }; +} + +describe('QrKeyringV2', () => { + let inner: QrKeyring; + let wrapper: QrKeyringV2; + + beforeEach(async () => { + inner = new QrKeyring({ bridge: getMockBridge(), ur: KNOWN_HDKEY_UR }); + wrapper = new QrKeyringV2({ + legacyKeyring: inner, + entropySourceId: TEST_ENTROPY_SOURCE_ID, + }); + }); + + it('constructs with expected type and capabilities', () => { + expect(wrapper.type).toBe('qr'); + expect(wrapper.capabilities.scopes).toStrictEqual([EthScope.Eoa]); + }); + + it('deserializes via the inner keyring and rebuilds the cache', async () => { + await inner.addAccounts(2); + const serializedState = await inner.serialize(); + + await wrapper.deserialize(serializedState); + + const accounts = await wrapper.getAccounts(); + expect(accounts).toHaveLength(2); + }); + + describe('getAccounts and caching', () => { + beforeEach(async () => { + await inner.addAccounts(2); + }); + + it('returns accounts and caches them', async () => { + const a1 = await wrapper.getAccounts(); + const a2 = await wrapper.getAccounts(); + + expect(a1).toHaveLength(2); + expect(a1[0]).toBe(a2[0]); + expect(a1[1]).toBe(a2[1]); + }); + + it('sets entropy options as private-key type', async () => { + const accounts = await wrapper.getAccounts(); + expect(accounts).toHaveLength(2); + + for (const account of accounts) { + expect(account.options.entropy).toBeDefined(); + expect(account.options.entropy?.type).toBe( + KeyringAccountEntropyTypeOption.PrivateKey, + ); + } + }); + }); + + describe('createAccounts', () => { + it('creates an account via legacy keyring and returns it', async () => { + const created = await wrapper.createAccounts(); + expect(created).toHaveLength(1); + + const all = await wrapper.getAccounts(); + expect(all).toHaveLength(1); + }); + + it('throws when the legacy keyring does not return an address', async () => { + jest + .spyOn(inner, 'addAccounts') + .mockResolvedValueOnce([undefined] as unknown as Hex[]); + + await expect(wrapper.createAccounts()).rejects.toThrow( + 'Failed to create QR keyring account', + ); + }); + }); + + describe('deleteAccount', () => { + beforeEach(async () => { + await wrapper.createAccounts(); + await wrapper.createAccounts(); + }); + + it('removes account from legacy and cache', async () => { + const accounts = await wrapper.getAccounts(); + expect(accounts.length).toBeGreaterThanOrEqual(2); + + const toDelete = accounts[0]; + expect(toDelete).toBeDefined(); + + if (toDelete?.id) { + await wrapper.deleteAccount(toDelete.id); + } + + const remaining = await wrapper.getAccounts(); + expect(remaining.find((a) => a.id === toDelete?.id)).toBeUndefined(); + }); + }); + + describe('submitRequest', () => { + let accountId: AccountId; + + beforeEach(async () => { + const createdAccounts = await wrapper.createAccounts(); + const [account] = createdAccounts; + if (!account) { + throw new Error('Expected at least one account'); + } + accountId = account.id; + }); + + it('signs a transaction', async () => { + const tx = {} as unknown as TypedTransaction; + + const mockTxData: TypedTxData = { + nonce: BigInt(0), + gasLimit: BigInt(21000), + gasPrice: BigInt(1000000000), + to: '0x0000000000000000000000000000000000000000', + value: BigInt(0), + data: '0x', + v: BigInt(1), + r: BigInt(2), + s: BigInt(3), + }; + const signTransactionSpy = jest + .spyOn(inner, 'signTransaction') + .mockResolvedValueOnce(mockTxData); + + const req = createMockRequest(accountId, EthMethod.SignTransaction, [ + tx as unknown as Json, + ]); + + // eslint-disable-next-line jest/no-restricted-matchers + await expect(wrapper.submitRequest(req)).resolves.toStrictEqual( + mockTxData, + ); + + expect(signTransactionSpy).toHaveBeenCalledWith(expect.any(String), tx); + }); + + it('throws when params are missing for eth_signTransaction', async () => { + const req = createMockRequest(accountId, EthMethod.SignTransaction, []); + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + 'Invalid params for eth_signTransaction', + ); + }); + + it('signs personal_sign', async () => { + const signPersonalMessageSpy = jest + .spyOn(inner, 'signPersonalMessage') + .mockResolvedValueOnce('0xsignature'); + + const req = createMockRequest(accountId, EthMethod.PersonalSign, [ + '0x68656c6c6f', + ]); + const res = await wrapper.submitRequest(req); + expect(res).toBe('0xsignature'); + expect(signPersonalMessageSpy).toHaveBeenCalledWith( + expect.any(String), + '0x68656c6c6f', + ); + }); + + it('throws when params are missing for personal_sign', async () => { + const req = createMockRequest(accountId, EthMethod.PersonalSign, []); + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + 'Invalid params for personal_sign', + ); + }); + + it('signs typed data V4', async () => { + const typedData = { foo: 'bar4' }; + + const signTypedDataSpy = jest + .spyOn(inner, 'signTypedData') + .mockResolvedValueOnce('0xv4'); + + const req = createMockRequest(accountId, EthMethod.SignTypedDataV4, [ + '0x0000000000000000000000000000000000000000', + typedData as unknown as Json, + ]); + + const res = await wrapper.submitRequest(req); + + expect(res).toBe('0xv4'); + expect(signTypedDataSpy).toHaveBeenCalledWith( + expect.any(String), + typedData, + ); + }); + + it('throws when params are missing for typed data methods', async () => { + const req = createMockRequest(accountId, EthMethod.SignTypedDataV4, []); + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + `Invalid params for ${EthMethod.SignTypedDataV4}`, + ); + }); + + it('throws for unsupported method', async () => { + const method = 'unsupported_method'; + const req = createMockRequest(accountId, method, []); + await expect(wrapper.submitRequest(req)).rejects.toThrow( + `Unsupported method for QrKeyringV2: ${method}`, + ); + }); + + it('throws when params is not an array', async () => { + const req = createMockRequest(accountId, EthMethod.PersonalSign, []); + // @ts-expect-error Intentionally provide invalid params shape + req.request.params = null; + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + 'Expected params to be an array', + ); + }); + + it('defaults params to empty array when undefined', async () => { + const req = createMockRequest(accountId, EthMethod.PersonalSign, []); + delete req.request.params; + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + 'Invalid params for personal_sign', + ); + }); + }); +}); diff --git a/packages/keyring-eth-qr/src/qr-keyring-v2.ts b/packages/keyring-eth-qr/src/qr-keyring-v2.ts new file mode 100644 index 000000000..ff1944038 --- /dev/null +++ b/packages/keyring-eth-qr/src/qr-keyring-v2.ts @@ -0,0 +1,200 @@ +import type { TypedTransaction } from '@ethereumjs/tx'; +import type { MessageTypes, TypedMessage } from '@metamask/eth-sig-util'; +import { + EthAccountType, + EthMethod, + EthScope, + KeyringAccountEntropyTypeOption, + type KeyringAccount, + type KeyringCapabilities, + type KeyringRequest, + KeyringType, + type KeyringV2, + KeyringWrapper, +} from '@metamask/keyring-api'; +import type { AccountId } from '@metamask/keyring-utils'; +import type { Hex, Json } from '@metamask/utils'; + +import type { QrKeyring, SerializedQrKeyringState } from './qr-keyring'; + +const qrKeyringV2Capabilities: KeyringCapabilities = { + scopes: [EthScope.Eoa], +}; + +/** + * Methods supported by QR keyring EOA accounts. + * QR keyrings support signing transactions, typed data v4, and personal messages. + */ +const QR_KEYRING_EOA_METHODS = [ + EthMethod.SignTransaction, + EthMethod.PersonalSign, + EthMethod.SignTypedDataV4, +]; + +export type QrKeyringV2Options = { + legacyKeyring: QrKeyring; + entropySourceId: string; +}; + +export class QrKeyringV2 + extends KeyringWrapper + implements KeyringV2 +{ + /** + * In-memory cache of KeyringAccount objects. + * Maps address (as Hex) to KeyringAccount. + */ + readonly #accountsCache = new Map(); + + constructor(options: QrKeyringV2Options) { + super({ + type: KeyringType.Qr, + inner: options.legacyKeyring, + capabilities: qrKeyringV2Capabilities, + entropySourceId: options.entropySourceId, + }); + } + + /** + * Helper method to safely cast a KeyringAccount address to Hex type. + * The KeyringAccount.address is typed as string, but for Ethereum accounts + * it should always be a valid Hex address. + * + * @param address - The address from a KeyringAccount. + * @returns The address as Hex type. + */ + #toHexAddress(address: string): Hex { + return address as Hex; + } + + /** + * Creates a KeyringAccount object for the given address. + * + * @param address - The account address. + * @returns The created KeyringAccount. + */ + #createKeyringAccount(address: Hex): KeyringAccount { + const id = this.resolver.register(address); + + const account: KeyringAccount = { + id, + type: EthAccountType.Eoa, + address, + scopes: this.capabilities.scopes, + methods: QR_KEYRING_EOA_METHODS, + options: { + entropy: { + type: KeyringAccountEntropyTypeOption.PrivateKey, + }, + }, + }; + + // Cache the account + this.#accountsCache.set(address, account); + + return account; + } + + async getAccounts(): Promise { + const addresses = await this.inner.getAccounts(); + + return addresses.map((address) => { + // Check if we already have this account cached + const cached = this.#accountsCache.get(address); + if (cached) { + return cached; + } + + // Create and cache the account if not already cached + return this.#createKeyringAccount(address); + }); + } + + async deserialize(state: Json): Promise { + // Clear the cache when deserializing + this.#accountsCache.clear(); + + // Deserialize the legacy keyring + await this.inner.deserialize(state as SerializedQrKeyringState); + + // Rebuild the cache by populating it with all accounts + // We call getAccounts() which will repopulate the cache as a side effect + await this.getAccounts(); + } + + async createAccounts(): Promise { + const [address] = await this.inner.addAccounts(1); + + if (!address) { + throw new Error('Failed to create QR keyring account'); + } + + const hexAddress = this.#toHexAddress(address); + const account = this.#createKeyringAccount(hexAddress); + + return [account]; + } + + async deleteAccount(accountId: AccountId): Promise { + // Sync with the inner keyring state in case it was modified externally + // This ensures our cache is up-to-date before we make changes + await this.getAccounts(); + + const account = await this.getAccount(accountId); + const hexAddress = this.#toHexAddress(account.address); + + // Remove from the legacy keyring + this.inner.removeAccount(hexAddress); + + // Remove from our cache + this.#accountsCache.delete(hexAddress); + } + + async submitRequest(request: KeyringRequest): Promise { + const { method, params = [] } = request.request; + + const { address } = await this.getAccount(request.account); + const hexAddress = this.#toHexAddress(address); + + // Validate params is an array + if (!Array.isArray(params)) { + throw new Error('Expected params to be an array'); + } + + switch (method) { + case `${EthMethod.SignTransaction}`: { + if (params.length < 1) { + throw new Error('Invalid params for eth_signTransaction'); + } + const [tx] = params; + return this.inner.signTransaction( + hexAddress, + tx as unknown as TypedTransaction, + ) as unknown as Json; + } + + case `${EthMethod.PersonalSign}`: { + if (params.length < 1) { + throw new Error('Invalid params for personal_sign'); + } + const [data] = params; + return this.inner.signPersonalMessage(hexAddress, data as Hex); + } + + case `${EthMethod.SignTypedDataV4}`: { + if (params.length < 2) { + throw new Error(`Invalid params for ${method}`); + } + const [, data] = params; + + return this.inner.signTypedData( + hexAddress, + data as unknown as TypedMessage, + ); + } + + default: + throw new Error(`Unsupported method for QrKeyringV2: ${method}`); + } + } +} diff --git a/packages/keyring-eth-qr/tsconfig.build.json b/packages/keyring-eth-qr/tsconfig.build.json index 08ef33751..91ae66199 100644 --- a/packages/keyring-eth-qr/tsconfig.build.json +++ b/packages/keyring-eth-qr/tsconfig.build.json @@ -9,6 +9,9 @@ "references": [ { "path": "../keyring-utils/tsconfig.build.json" + }, + { + "path": "../keyring-api/tsconfig.build.json" } ], "include": ["./src/**/*.ts"], diff --git a/packages/keyring-eth-qr/tsconfig.json b/packages/keyring-eth-qr/tsconfig.json index f6a8db2fb..924281cde 100644 --- a/packages/keyring-eth-qr/tsconfig.json +++ b/packages/keyring-eth-qr/tsconfig.json @@ -3,7 +3,7 @@ "compilerOptions": { "baseUrl": "./" }, - "references": [{ "path": "../keyring-utils" }], + "references": [{ "path": "../keyring-utils" }, { "path": "../keyring-api" }], "include": ["./src", "./test"], "exclude": ["./dist/**/*"] } diff --git a/packages/keyring-eth-simple/CHANGELOG.md b/packages/keyring-eth-simple/CHANGELOG.md index 5238e9ea9..c157ca1d6 100644 --- a/packages/keyring-eth-simple/CHANGELOG.md +++ b/packages/keyring-eth-simple/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `SimpleKeyringV2` class implementing KeyringV2 interface ([#398](https://github.com/MetaMask/accounts/pull/398)) + - Wraps legacy `SimpleKeyring` to expose accounts via the unified KeyringV2 API and the `KeyringAccount` type. + ## [11.0.0] ### Changed diff --git a/packages/keyring-eth-simple/package.json b/packages/keyring-eth-simple/package.json index c09b87bce..353ebc4d2 100644 --- a/packages/keyring-eth-simple/package.json +++ b/packages/keyring-eth-simple/package.json @@ -46,6 +46,7 @@ "dependencies": { "@ethereumjs/util": "^9.1.0", "@metamask/eth-sig-util": "^8.2.0", + "@metamask/keyring-api": "workspace:^", "@metamask/utils": "^11.1.0", "ethereum-cryptography": "^2.1.2", "randombytes": "^2.1.0" diff --git a/packages/keyring-eth-simple/src/simple-keyring-v2.test.ts b/packages/keyring-eth-simple/src/simple-keyring-v2.test.ts new file mode 100644 index 000000000..6bcaaf5f8 --- /dev/null +++ b/packages/keyring-eth-simple/src/simple-keyring-v2.test.ts @@ -0,0 +1,431 @@ +import type { TypedTransaction } from '@ethereumjs/tx'; +import { SignTypedDataVersion } from '@metamask/eth-sig-util'; +import { + EthMethod, + EthScope, + KeyringAccountEntropyTypeOption, + KeyringType, + PrivateKeyEncoding, + type ExportAccountOptions, + type KeyringRequest, +} from '@metamask/keyring-api'; +import type { AccountId } from '@metamask/keyring-utils'; +import type { Json } from '@metamask/utils'; + +// eslint-disable-next-line @typescript-eslint/naming-convention +import SimpleKeyring from './simple-keyring'; +import { SimpleKeyringV2 } from './simple-keyring-v2'; + +/** + * Creates a mock KeyringRequest for testing. + * + * @param accountId - The account ID for the request. + * @param method - The method to invoke. + * @param params - Optional parameters for the request. + * @returns A mock KeyringRequest. + */ +function createMockRequest( + accountId: AccountId, + method: string, + params: Json[] = [], +): KeyringRequest { + return { + id: '00000000-0000-0000-0000-000000000000', + scope: EthScope.Eoa, + account: accountId, + origin: 'http://localhost', + request: { + method, + params, + }, + }; +} + +describe('SimpleKeyringV2', () => { + let inner: SimpleKeyring; + let wrapper: SimpleKeyringV2; + + beforeEach(async () => { + inner = new SimpleKeyring([]); + wrapper = new SimpleKeyringV2({ + legacyKeyring: inner, + entropySourceId: 'test-entropy', + }); + }); + + it('constructs with expected type and capabilities', () => { + expect(wrapper.type).toBe(KeyringType.PrivateKey); + expect(wrapper.capabilities.scopes).toStrictEqual([EthScope.Eoa]); + expect(wrapper.capabilities.privateKey).toBeDefined(); + expect(wrapper.capabilities.privateKey?.importFormats).toHaveLength(1); + expect(wrapper.capabilities.privateKey?.importFormats?.[0]).toStrictEqual({ + encoding: PrivateKeyEncoding.Hexadecimal, + }); + expect(wrapper.capabilities.privateKey?.exportFormats).toHaveLength(1); + expect(wrapper.capabilities.privateKey?.exportFormats?.[0]).toStrictEqual({ + encoding: PrivateKeyEncoding.Hexadecimal, + }); + }); + + it('deserializes via the inner keyring and rebuilds the cache', async () => { + await inner.addAccounts(2); + const serializedState = await inner.serialize(); + + await wrapper.deserialize(serializedState); + + const accounts = await wrapper.getAccounts(); + expect(accounts).toHaveLength(2); + }); + + describe('getAccounts and caching', () => { + beforeEach(async () => { + await inner.addAccounts(2); + }); + + it('returns accounts and caches them', async () => { + const a1 = await wrapper.getAccounts(); + const a2 = await wrapper.getAccounts(); + + expect(a1).toHaveLength(2); + expect(a1[0]).toBe(a2[0]); + expect(a1[1]).toBe(a2[1]); + }); + + it('sets entropy options as private-key type', async () => { + const accounts = await wrapper.getAccounts(); + expect(accounts).toHaveLength(2); + + for (const account of accounts) { + expect(account.options.entropy).toBeDefined(); + expect(account.options.entropy?.type).toBe( + KeyringAccountEntropyTypeOption.PrivateKey, + ); + } + }); + }); + + describe('createAccounts', () => { + it('creates an account via legacy keyring and returns it', async () => { + const created = await wrapper.createAccounts(); + expect(created).toHaveLength(1); + + const all = await wrapper.getAccounts(); + expect(all).toHaveLength(1); + }); + + it('throws when the legacy keyring does not return an address', async () => { + type Hex = `0x${string}`; + jest + .spyOn(inner, 'addAccounts') + .mockResolvedValueOnce([undefined] as unknown as Hex[]); + + await expect(wrapper.createAccounts()).rejects.toThrow( + 'Failed to create simple keyring account', + ); + }); + + it('imports a private key when options are provided', async () => { + const privateKey = + '0x4af1bceebf7f3634ec3cff8a2c38e51178d5d4ce585c52d6043e5e2cc3418bb0'; + + const created = await wrapper.createAccounts({ + type: 'private-key:import', + privateKey, + encoding: PrivateKeyEncoding.Hexadecimal, + }); + + expect(created).toHaveLength(1); + expect(created[0]?.address).toBeDefined(); + + const all = await wrapper.getAccounts(); + expect(all).toHaveLength(1); + expect(all[0]?.address).toBe(created[0]?.address); + }); + + it('throws when importing with unsupported encoding', async () => { + const privateKey = + '0x4af1bceebf7f3634ec3cff8a2c38e51178d5d4ce585c52d6043e5e2cc3418bb0'; + + await expect( + wrapper.createAccounts({ + type: 'private-key:import', + privateKey, + encoding: 'base58' as PrivateKeyEncoding, + }), + ).rejects.toThrow( + "Unsupported encoding for Simple keyring: base58. Only 'hexadecimal' is supported.", + ); + }); + + it('preserves existing accounts when importing a new private key', async () => { + // Create a random account first + const firstAccount = await wrapper.createAccounts(); + expect(firstAccount).toHaveLength(1); + + // Import a specific private key + const privateKey = + '0x4af1bceebf7f3634ec3cff8a2c38e51178d5d4ce585c52d6043e5e2cc3418bb0'; + + const importedAccount = await wrapper.createAccounts({ + type: 'private-key:import', + privateKey, + encoding: PrivateKeyEncoding.Hexadecimal, + }); + + expect(importedAccount).toHaveLength(1); + + // Both accounts should exist + const all = await wrapper.getAccounts(); + expect(all).toHaveLength(2); + expect(all.map((a) => a.address)).toContain(firstAccount[0]?.address); + expect(all.map((a) => a.address)).toContain(importedAccount[0]?.address); + }); + + it('throws when import fails to add an account', async () => { + const privateKey = + '0x4af1bceebf7f3634ec3cff8a2c38e51178d5d4ce585c52d6043e5e2cc3418bb0'; + + jest.spyOn(inner, 'getAccounts').mockResolvedValueOnce([]); + + await expect( + wrapper.createAccounts({ + type: 'private-key:import', + privateKey, + encoding: PrivateKeyEncoding.Hexadecimal, + }), + ).rejects.toThrow('Failed to import private key'); + }); + }); + + describe('deleteAccount', () => { + beforeEach(async () => { + await wrapper.createAccounts(); + await wrapper.createAccounts(); + }); + + it('removes account from legacy and cache', async () => { + const accounts = await wrapper.getAccounts(); + expect(accounts.length).toBeGreaterThanOrEqual(2); + + const toDelete = accounts[0]; + expect(toDelete).toBeDefined(); + + if (toDelete?.id) { + await wrapper.deleteAccount(toDelete.id); + } + + const remaining = await wrapper.getAccounts(); + expect(remaining.find((a) => a.id === toDelete?.id)).toBeUndefined(); + }); + }); + + describe('exportAccount', () => { + beforeEach(async () => { + await wrapper.createAccounts(); + }); + + it('exports a private key in hex', async () => { + const accounts = await wrapper.getAccounts(); + const [firstAccount] = accounts; + // eslint-disable-next-line jest/no-if + if (!firstAccount) { + throw new Error('Expected at least one account'); + } + const exported = await wrapper.exportAccount(firstAccount.id); + + expect(exported.type).toBe('private-key'); + expect(exported.privateKey).toMatch(/^0x[0-9a-fA-F]{64}$/u); + expect(exported.encoding).toBe(PrivateKeyEncoding.Hexadecimal); + }); + + it('throws when requesting an unsupported encoding', async () => { + const accounts = await wrapper.getAccounts(); + const [firstAccount] = accounts; + // eslint-disable-next-line jest/no-if + if (!firstAccount) { + throw new Error('Expected at least one account'); + } + + const exportPromise = wrapper.exportAccount(firstAccount.id, { + type: 'private-key', + encoding: 'unsupported-encoding' as PrivateKeyEncoding, + } as ExportAccountOptions); + + await expect(exportPromise).rejects.toThrow( + 'Unsupported encoding for Simple keyring', + ); + }); + }); + + describe('submitRequest', () => { + let accountId: AccountId; + + beforeEach(async () => { + const createdAccounts = await wrapper.createAccounts(); + const [account] = createdAccounts; + if (!account) { + throw new Error('Expected at least one account'); + } + accountId = account.id; + }); + + it('signs a transaction', async () => { + const tx = {} as unknown as TypedTransaction; + + const signTransactionSpy = jest + .spyOn(inner, 'signTransaction') + .mockResolvedValueOnce('0xdeadbeef' as unknown as TypedTransaction); + + const req = createMockRequest(accountId, EthMethod.SignTransaction, [ + tx as unknown as Json, + ]); + + // eslint-disable-next-line jest/no-restricted-matchers + await expect(wrapper.submitRequest(req)).resolves.toBe('0xdeadbeef'); + + expect(signTransactionSpy).toHaveBeenCalledWith(expect.any(String), tx); + }); + + it('throws when params are missing for eth_signTransaction', async () => { + const req = createMockRequest(accountId, EthMethod.SignTransaction, []); + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + 'Invalid params for eth_signTransaction', + ); + }); + + it('signs eth_sign', async () => { + const req = createMockRequest(accountId, EthMethod.Sign, [ + '0x0000000000000000000000000000000000000000', + '0x68656c6c6f', + ]); + const res = await wrapper.submitRequest(req); + expect(typeof res).toBe('string'); + }); + + it('signs personal_sign', async () => { + const req = createMockRequest(accountId, EthMethod.PersonalSign, [ + '0x68656c6c6f', + ]); + const res = await wrapper.submitRequest(req); + expect(typeof res).toBe('string'); + }); + + it('throws for unsupported method', async () => { + const method = 'unsupported_method'; + const req = createMockRequest(accountId, method, []); + await expect(wrapper.submitRequest(req)).rejects.toThrow( + `Unsupported method for SimpleKeyringV2: ${method}`, + ); + }); + + it('throws when params is not an array', async () => { + const req = createMockRequest(accountId, EthMethod.Sign, []); + // @ts-expect-error Intentionally provide invalid params shape + req.request.params = null; + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + 'Expected params to be an array', + ); + }); + + it('defaults params to empty array when undefined', async () => { + const req = createMockRequest(accountId, EthMethod.Sign, []); + delete req.request.params; + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + 'Invalid params for eth_sign', + ); + }); + + it('throws when params are missing for eth_sign', async () => { + const req = createMockRequest(accountId, EthMethod.Sign, []); + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + 'Invalid params for eth_sign', + ); + }); + + it('throws when params are missing for personal_sign', async () => { + const req = createMockRequest(accountId, EthMethod.PersonalSign, []); + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + 'Invalid params for personal_sign', + ); + }); + + it('throws when params are missing for typed data methods', async () => { + const req = createMockRequest(accountId, EthMethod.SignTypedDataV4, []); + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + `Invalid params for ${EthMethod.SignTypedDataV4}`, + ); + }); + + it('signs typed data V1', async () => { + const typedData = { foo: 'bar' }; + + const signTypedDataSpy = jest + .spyOn(inner, 'signTypedData') + .mockResolvedValueOnce('0xv1'); + + const req = createMockRequest(accountId, EthMethod.SignTypedDataV1, [ + '0x0000000000000000000000000000000000000000', + typedData as unknown as Json, + ]); + + const res = await wrapper.submitRequest(req); + + expect(res).toBe('0xv1'); + expect(signTypedDataSpy).toHaveBeenCalledWith( + expect.any(String), + typedData, + { version: SignTypedDataVersion.V1 }, + ); + }); + + it('signs typed data V3', async () => { + const typedData = { foo: 'bar3' }; + + const signTypedDataSpy = jest + .spyOn(inner, 'signTypedData') + .mockResolvedValueOnce('0xv3'); + + const req = createMockRequest(accountId, EthMethod.SignTypedDataV3, [ + '0x0000000000000000000000000000000000000000', + typedData as unknown as Json, + ]); + + const res = await wrapper.submitRequest(req); + + expect(res).toBe('0xv3'); + expect(signTypedDataSpy).toHaveBeenCalledWith( + expect.any(String), + typedData, + { version: SignTypedDataVersion.V3 }, + ); + }); + + it('signs typed data V4', async () => { + const typedData = { foo: 'bar4' }; + + const signTypedDataSpy = jest + .spyOn(inner, 'signTypedData') + .mockResolvedValueOnce('0xv4'); + + const req = createMockRequest(accountId, EthMethod.SignTypedDataV4, [ + '0x0000000000000000000000000000000000000000', + typedData as unknown as Json, + ]); + + const res = await wrapper.submitRequest(req); + + expect(res).toBe('0xv4'); + expect(signTypedDataSpy).toHaveBeenCalledWith( + expect.any(String), + typedData, + { version: SignTypedDataVersion.V4 }, + ); + }); + }); +}); diff --git a/packages/keyring-eth-simple/src/simple-keyring-v2.ts b/packages/keyring-eth-simple/src/simple-keyring-v2.ts new file mode 100644 index 000000000..d173af3a1 --- /dev/null +++ b/packages/keyring-eth-simple/src/simple-keyring-v2.ts @@ -0,0 +1,266 @@ +import type { TypedTransaction } from '@ethereumjs/tx'; +import type { + MessageTypes, + TypedDataV1, + TypedMessage, +} from '@metamask/eth-sig-util'; +import { SignTypedDataVersion } from '@metamask/eth-sig-util'; +import { + EthAccountType, + EthMethod, + EthScope, + type CreateAccountOptions, + type ExportAccountOptions, + type ExportedAccount, + type KeyringAccount, + type KeyringCapabilities, + type KeyringRequest, + KeyringType, + type KeyringV2, + KeyringWrapper, + PrivateKeyEncoding, + KeyringAccountEntropyTypeOption, +} from '@metamask/keyring-api'; +import type { AccountId } from '@metamask/keyring-utils'; +import type { Hex, Json } from '@metamask/utils'; + +// eslint-disable-next-line @typescript-eslint/naming-convention +import type SimpleKeyring from './simple-keyring'; + +const simpleKeyringV2Capabilities: KeyringCapabilities = { + scopes: [EthScope.Eoa], + privateKey: { + importFormats: [{ encoding: PrivateKeyEncoding.Hexadecimal }], + exportFormats: [{ encoding: PrivateKeyEncoding.Hexadecimal }], + }, +}; + +const SIMPLE_KEYRING_EOA_METHODS = [ + EthMethod.SignTransaction, + EthMethod.Sign, + EthMethod.PersonalSign, + EthMethod.SignTypedDataV1, + EthMethod.SignTypedDataV3, + EthMethod.SignTypedDataV4, +]; + +export type SimpleKeyringV2Options = { + legacyKeyring: SimpleKeyring; + entropySourceId: string; +}; + +export class SimpleKeyringV2 + extends KeyringWrapper + implements KeyringV2 +{ + readonly #accountsCache = new Map(); + + constructor(options: SimpleKeyringV2Options) { + super({ + type: KeyringType.PrivateKey, + inner: options.legacyKeyring, + capabilities: simpleKeyringV2Capabilities, + entropySourceId: options.entropySourceId, + }); + } + + #toHexAddress(address: string): Hex { + return address as Hex; + } + + #createKeyringAccount(address: Hex): KeyringAccount { + const id = this.resolver.register(address); + + const account: KeyringAccount = { + id, + type: EthAccountType.Eoa, + address, + scopes: this.capabilities.scopes, + methods: SIMPLE_KEYRING_EOA_METHODS, + options: { + entropy: { + type: KeyringAccountEntropyTypeOption.PrivateKey, + }, + }, + }; + + this.#accountsCache.set(address, account); + + return account; + } + + async getAccounts(): Promise { + const addresses = await this.inner.getAccounts(); + + return addresses.map((address) => { + const cached = this.#accountsCache.get(address); + if (cached) { + return cached; + } + + return this.#createKeyringAccount(address); + }); + } + + async deserialize(state: Json): Promise { + this.#accountsCache.clear(); + + await this.inner.deserialize(state as string[]); + + await this.getAccounts(); + } + + async createAccounts( + options?: CreateAccountOptions, + ): Promise { + // Handle private key import + if (options?.type === 'private-key:import') { + const { privateKey, encoding } = options; + + // Validate encoding + if (encoding !== PrivateKeyEncoding.Hexadecimal) { + throw new Error( + `Unsupported encoding for Simple keyring: ${encoding}. Only '${PrivateKeyEncoding.Hexadecimal}' is supported.`, + ); + } + + // Get current accounts to preserve them + const currentAccounts = await this.inner.serialize(); + + // Import the new private key by deserializing with all accounts + await this.inner.deserialize([...currentAccounts, privateKey]); + + // Get the newly added account (last one) + const addresses = await this.inner.getAccounts(); + const newAddress = addresses[addresses.length - 1]; + + if (!newAddress) { + throw new Error('Failed to import private key'); + } + + const hexAddress = this.#toHexAddress(newAddress); + const account = this.#createKeyringAccount(hexAddress); + + return [account]; + } + + // Handle random account creation (default behavior when no options provided) + const [address] = await this.inner.addAccounts(1); + + if (!address) { + throw new Error('Failed to create simple keyring account'); + } + + const hexAddress = this.#toHexAddress(address); + const account = this.#createKeyringAccount(hexAddress); + + return [account]; + } + + async deleteAccount(accountId: AccountId): Promise { + await this.getAccounts(); + + const account = await this.getAccount(accountId); + const hexAddress = this.#toHexAddress(account.address); + + this.inner.removeAccount(hexAddress); + + this.#accountsCache.delete(hexAddress); + } + + async exportAccount( + accountId: AccountId, + options?: ExportAccountOptions, + ): Promise { + const account = await this.getAccount(accountId); + + const requestedEncoding = + options?.encoding ?? PrivateKeyEncoding.Hexadecimal; + + if (requestedEncoding !== PrivateKeyEncoding.Hexadecimal) { + throw new Error( + `Unsupported encoding for Simple keyring: ${requestedEncoding}. Only '${PrivateKeyEncoding.Hexadecimal}' is supported.`, + ); + } + + const privateKeyHex = await this.inner.exportAccount( + this.#toHexAddress(account.address), + ); + + const exported: ExportedAccount = { + type: 'private-key', + privateKey: `0x${privateKeyHex}`, + encoding: PrivateKeyEncoding.Hexadecimal, + }; + + return exported; + } + + async submitRequest(request: KeyringRequest): Promise { + const { method, params = [] } = request.request; + + const { address } = await this.getAccount(request.account); + const hexAddress = this.#toHexAddress(address); + + if (!Array.isArray(params)) { + throw new Error('Expected params to be an array'); + } + + switch (method) { + case `${EthMethod.SignTransaction}`: { + if (params.length < 1) { + throw new Error('Invalid params for eth_signTransaction'); + } + const [tx] = params; + return this.inner.signTransaction( + hexAddress, + tx as unknown as TypedTransaction, + ) as unknown as Json; + } + + case `${EthMethod.Sign}`: { + if (params.length < 2) { + throw new Error('Invalid params for eth_sign'); + } + const [, data] = params; + return this.inner.signMessage(hexAddress, data as string); + } + + case `${EthMethod.PersonalSign}`: { + if (params.length < 1) { + throw new Error('Invalid params for personal_sign'); + } + const [data] = params; + return this.inner.signPersonalMessage(hexAddress, data as Hex); + } + + case `${EthMethod.SignTypedDataV1}`: + case `${EthMethod.SignTypedDataV3}`: + case `${EthMethod.SignTypedDataV4}`: { + if (params.length < 2) { + throw new Error(`Invalid params for ${method}`); + } + const [, data] = params; + let version: SignTypedDataVersion; + if (method === EthMethod.SignTypedDataV4) { + version = SignTypedDataVersion.V4; + } else if (method === EthMethod.SignTypedDataV3) { + version = SignTypedDataVersion.V3; + } else { + version = SignTypedDataVersion.V1; + } + + return this.inner.signTypedData( + hexAddress, + data as unknown as TypedDataV1 | TypedMessage, + { + version, + }, + ); + } + + default: + throw new Error(`Unsupported method for SimpleKeyringV2: ${method}`); + } + } +} diff --git a/packages/keyring-eth-simple/tsconfig.build.json b/packages/keyring-eth-simple/tsconfig.build.json index 08ef33751..91ae66199 100644 --- a/packages/keyring-eth-simple/tsconfig.build.json +++ b/packages/keyring-eth-simple/tsconfig.build.json @@ -9,6 +9,9 @@ "references": [ { "path": "../keyring-utils/tsconfig.build.json" + }, + { + "path": "../keyring-api/tsconfig.build.json" } ], "include": ["./src/**/*.ts"], diff --git a/packages/keyring-eth-simple/tsconfig.json b/packages/keyring-eth-simple/tsconfig.json index 3b734c355..ae9156fab 100644 --- a/packages/keyring-eth-simple/tsconfig.json +++ b/packages/keyring-eth-simple/tsconfig.json @@ -3,7 +3,7 @@ "compilerOptions": { "baseUrl": "./" }, - "references": [{ "path": "../keyring-utils" }], + "references": [{ "path": "../keyring-utils" }, { "path": "../keyring-api" }], "include": ["./src"], "exclude": ["./dist/**/*"] } diff --git a/packages/keyring-eth-trezor/CHANGELOG.md b/packages/keyring-eth-trezor/CHANGELOG.md index 813a11b33..abf71cd65 100644 --- a/packages/keyring-eth-trezor/CHANGELOG.md +++ b/packages/keyring-eth-trezor/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `TrezorKeyringV2` class implementing KeyringV2 interface ([#398](https://github.com/MetaMask/accounts/pull/398)) + - Wraps legacy `TrezorKeyring` to expose accounts via the unified KeyringV2 API and the `KeyringAccount` type. + ## [9.0.0] ### Changed diff --git a/packages/keyring-eth-trezor/jest.config.js b/packages/keyring-eth-trezor/jest.config.js index 0bb9dd487..039993803 100644 --- a/packages/keyring-eth-trezor/jest.config.js +++ b/packages/keyring-eth-trezor/jest.config.js @@ -23,10 +23,10 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 52.38, - functions: 91.22, - lines: 90.15, - statements: 90.35, + branches: 57.74, + functions: 92.42, + lines: 92.4, + statements: 92.51, }, }, }); diff --git a/packages/keyring-eth-trezor/package.json b/packages/keyring-eth-trezor/package.json index 91f1f0cee..bbae2e9c9 100644 --- a/packages/keyring-eth-trezor/package.json +++ b/packages/keyring-eth-trezor/package.json @@ -49,6 +49,7 @@ "@ethereumjs/tx": "^5.4.0", "@ethereumjs/util": "^9.1.0", "@metamask/eth-sig-util": "^8.2.0", + "@metamask/keyring-api": "workspace:^", "@metamask/utils": "^11.1.0", "@trezor/connect-plugin-ethereum": "^9.0.5", "@trezor/connect-web": "^9.6.0", diff --git a/packages/keyring-eth-trezor/src/index.ts b/packages/keyring-eth-trezor/src/index.ts index 6cb20b9e7..185812951 100644 --- a/packages/keyring-eth-trezor/src/index.ts +++ b/packages/keyring-eth-trezor/src/index.ts @@ -2,3 +2,7 @@ export * from './trezor-keyring'; export * from './onekey-keyring'; export type * from './trezor-bridge'; export * from './trezor-connect-bridge'; +export { + TrezorKeyringV2, + type TrezorKeyringV2Options, +} from './trezor-keyring-v2'; diff --git a/packages/keyring-eth-trezor/src/trezor-keyring-v2.test.ts b/packages/keyring-eth-trezor/src/trezor-keyring-v2.test.ts new file mode 100644 index 000000000..7ce2c7fdf --- /dev/null +++ b/packages/keyring-eth-trezor/src/trezor-keyring-v2.test.ts @@ -0,0 +1,336 @@ +import type { TypedTransaction } from '@ethereumjs/tx'; +import { SignTypedDataVersion } from '@metamask/eth-sig-util'; +import { + EthMethod, + EthScope, + KeyringAccountEntropyTypeOption, + type KeyringRequest, +} from '@metamask/keyring-api'; +import type { AccountId } from '@metamask/keyring-utils'; +import type { Hex, Json } from '@metamask/utils'; +import HDKey from 'hdkey'; + +import type { TrezorBridge } from './trezor-bridge'; +import { TrezorKeyring } from './trezor-keyring'; +import { TrezorKeyringV2 } from './trezor-keyring-v2'; + +const TEST_ENTROPY_SOURCE_ID = 'test-entropy-source-id'; + +const fakeXPubKey = + 'xpub6FnCn6nSzZAw5Tw7cgR9bi15UV96gLZhjDstkXXxvCLsUXBGXPdSnLFbdpq8p9HmGsApME5hQTZ3emM2rnY5agb9rXpVGyy3bdW6EEgAtqt'; +const fakeHdKey = HDKey.fromExtendedKey(fakeXPubKey); + +/** + * Creates a mock KeyringRequest for testing. + * + * @param accountId - The account ID for the request. + * @param method - The method to invoke. + * @param params - Optional parameters for the request. + * @returns A mock KeyringRequest. + */ +function createMockRequest( + accountId: AccountId, + method: string, + params: Json[] = [], +): KeyringRequest { + return { + id: '00000000-0000-0000-0000-000000000000', + scope: EthScope.Eoa, + account: accountId, + origin: 'http://localhost', + request: { + method, + params, + }, + }; +} + +/** + * Get a mock bridge for the TrezorKeyring. + * + * @returns A mock bridge. + */ +function getMockBridge(): TrezorBridge { + return { + init: jest.fn().mockResolvedValue(undefined), + dispose: jest.fn().mockResolvedValue(undefined), + getPublicKey: jest.fn(), + ethereumSignTransaction: jest.fn(), + ethereumSignMessage: jest.fn(), + ethereumSignTypedData: jest.fn(), + } as unknown as TrezorBridge; +} + +describe('TrezorKeyringV2', () => { + let inner: TrezorKeyring; + let wrapper: TrezorKeyringV2; + let bridge: TrezorBridge; + + beforeEach(async () => { + bridge = getMockBridge(); + inner = new TrezorKeyring({ bridge }); + await inner.init(); + // Set the HDKey directly to avoid needing a real unlock + // eslint-disable-next-line require-atomic-updates + inner.hdk = fakeHdKey; + wrapper = new TrezorKeyringV2({ + legacyKeyring: inner, + entropySourceId: TEST_ENTROPY_SOURCE_ID, + }); + }); + + it('constructs with expected type and capabilities', () => { + expect(wrapper.type).toBe('trezor'); + expect(wrapper.capabilities.scopes).toStrictEqual([EthScope.Eoa]); + }); + + it('deserializes via the inner keyring and rebuilds the cache', async () => { + await inner.addAccounts(2); + const serializedState = await inner.serialize(); + + await wrapper.deserialize(serializedState); + + const accounts = await wrapper.getAccounts(); + expect(accounts).toHaveLength(2); + }); + + describe('getAccounts and caching', () => { + beforeEach(async () => { + await inner.addAccounts(2); + }); + + it('returns accounts and caches them', async () => { + const a1 = await wrapper.getAccounts(); + const a2 = await wrapper.getAccounts(); + + expect(a1).toHaveLength(2); + expect(a1[0]).toBe(a2[0]); + expect(a1[1]).toBe(a2[1]); + }); + + it('sets entropy options as private-key type', async () => { + const accounts = await wrapper.getAccounts(); + expect(accounts).toHaveLength(2); + + for (const account of accounts) { + expect(account.options.entropy).toBeDefined(); + expect(account.options.entropy?.type).toBe( + KeyringAccountEntropyTypeOption.PrivateKey, + ); + } + }); + }); + + describe('createAccounts', () => { + it('creates an account via legacy keyring and returns it', async () => { + const created = await wrapper.createAccounts(); + expect(created).toHaveLength(1); + + const all = await wrapper.getAccounts(); + expect(all).toHaveLength(1); + }); + + it('throws when the legacy keyring does not return an address', async () => { + jest + .spyOn(inner, 'addAccounts') + .mockResolvedValueOnce([undefined] as unknown as Hex[]); + + await expect(wrapper.createAccounts()).rejects.toThrow( + 'Failed to create Trezor keyring account', + ); + }); + }); + + describe('deleteAccount', () => { + beforeEach(async () => { + await inner.addAccounts(2); + }); + + it('removes account from legacy and cache', async () => { + const accounts = await wrapper.getAccounts(); + expect(accounts.length).toBeGreaterThanOrEqual(2); + + const toDelete = accounts[0]; + expect(toDelete).toBeDefined(); + + if (toDelete?.id) { + await wrapper.deleteAccount(toDelete.id); + } + + const remaining = await wrapper.getAccounts(); + expect(remaining.find((a) => a.id === toDelete?.id)).toBeUndefined(); + }); + }); + + describe('submitRequest', () => { + let accountId: AccountId; + + beforeEach(async () => { + const createdAccounts = await wrapper.createAccounts(); + const [account] = createdAccounts; + if (!account) { + throw new Error('Expected at least one account'); + } + accountId = account.id; + }); + + it('signs a transaction', async () => { + const tx = {} as unknown as TypedTransaction; + + const mockTx = { + getSenderAddress: jest + .fn() + .mockReturnValue(Buffer.from('0'.repeat(40), 'hex')), + } as unknown as TypedTransaction; + + const signTransactionSpy = jest + .spyOn(inner, 'signTransaction') + .mockResolvedValueOnce(mockTx); + + const req = createMockRequest(accountId, EthMethod.SignTransaction, [ + tx as unknown as Json, + ]); + + // eslint-disable-next-line jest/no-restricted-matchers + await expect(wrapper.submitRequest(req)).resolves.toStrictEqual(mockTx); + + expect(signTransactionSpy).toHaveBeenCalledWith(expect.any(String), tx); + }); + + it('throws when params are missing for eth_signTransaction', async () => { + const req = createMockRequest(accountId, EthMethod.SignTransaction, []); + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + 'Invalid params for eth_signTransaction', + ); + }); + + it('signs eth_sign', async () => { + const signMessageSpy = jest + .spyOn(inner, 'signMessage') + .mockResolvedValueOnce('0xsignature'); + + const req = createMockRequest(accountId, EthMethod.Sign, [ + '0x0000000000000000000000000000000000000000', + '0x68656c6c6f', + ]); + const res = await wrapper.submitRequest(req); + expect(res).toBe('0xsignature'); + expect(signMessageSpy).toHaveBeenCalledWith( + expect.any(String), + '0x68656c6c6f', + ); + }); + + it('throws when params are missing for eth_sign', async () => { + const req = createMockRequest(accountId, EthMethod.Sign, []); + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + 'Invalid params for eth_sign', + ); + }); + + it('signs personal_sign', async () => { + const signPersonalMessageSpy = jest + .spyOn(inner, 'signPersonalMessage') + .mockResolvedValueOnce('0xsignature'); + + const req = createMockRequest(accountId, EthMethod.PersonalSign, [ + '0x68656c6c6f', + ]); + const res = await wrapper.submitRequest(req); + expect(res).toBe('0xsignature'); + expect(signPersonalMessageSpy).toHaveBeenCalledWith( + expect.any(String), + '0x68656c6c6f', + ); + }); + + it('throws when params are missing for personal_sign', async () => { + const req = createMockRequest(accountId, EthMethod.PersonalSign, []); + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + 'Invalid params for personal_sign', + ); + }); + + it('signs typed data V3', async () => { + const typedData = { foo: 'bar3' }; + + const signTypedDataSpy = jest + .spyOn(inner, 'signTypedData') + .mockResolvedValueOnce('0xv3'); + + const req = createMockRequest(accountId, EthMethod.SignTypedDataV3, [ + '0x0000000000000000000000000000000000000000', + typedData as unknown as Json, + ]); + + const res = await wrapper.submitRequest(req); + + expect(res).toBe('0xv3'); + expect(signTypedDataSpy).toHaveBeenCalledWith( + expect.any(String), + typedData, + { version: SignTypedDataVersion.V3 }, + ); + }); + + it('signs typed data V4', async () => { + const typedData = { foo: 'bar4' }; + + const signTypedDataSpy = jest + .spyOn(inner, 'signTypedData') + .mockResolvedValueOnce('0xv4'); + + const req = createMockRequest(accountId, EthMethod.SignTypedDataV4, [ + '0x0000000000000000000000000000000000000000', + typedData as unknown as Json, + ]); + + const res = await wrapper.submitRequest(req); + + expect(res).toBe('0xv4'); + expect(signTypedDataSpy).toHaveBeenCalledWith( + expect.any(String), + typedData, + { version: SignTypedDataVersion.V4 }, + ); + }); + + it('throws when params are missing for typed data methods', async () => { + const req = createMockRequest(accountId, EthMethod.SignTypedDataV4, []); + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + `Invalid params for ${EthMethod.SignTypedDataV4}`, + ); + }); + + it('throws for unsupported method', async () => { + const method = 'unsupported_method'; + const req = createMockRequest(accountId, method, []); + await expect(wrapper.submitRequest(req)).rejects.toThrow( + `Unsupported method for TrezorKeyringV2: ${method}`, + ); + }); + + it('throws when params is not an array', async () => { + const req = createMockRequest(accountId, EthMethod.PersonalSign, []); + // @ts-expect-error Intentionally provide invalid params shape + req.request.params = null; + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + 'Expected params to be an array', + ); + }); + + it('defaults params to empty array when undefined', async () => { + const req = createMockRequest(accountId, EthMethod.PersonalSign, []); + delete req.request.params; + + await expect(wrapper.submitRequest(req)).rejects.toThrow( + 'Invalid params for personal_sign', + ); + }); + }); +}); diff --git a/packages/keyring-eth-trezor/src/trezor-keyring-v2.ts b/packages/keyring-eth-trezor/src/trezor-keyring-v2.ts new file mode 100644 index 000000000..adc7e4fcb --- /dev/null +++ b/packages/keyring-eth-trezor/src/trezor-keyring-v2.ts @@ -0,0 +1,219 @@ +import type { TypedTransaction } from '@ethereumjs/tx'; +import type { MessageTypes, TypedMessage } from '@metamask/eth-sig-util'; +import { SignTypedDataVersion } from '@metamask/eth-sig-util'; +import { + EthAccountType, + EthMethod, + EthScope, + KeyringAccountEntropyTypeOption, + type KeyringAccount, + type KeyringCapabilities, + type KeyringRequest, + KeyringType, + type KeyringV2, + KeyringWrapper, +} from '@metamask/keyring-api'; +import type { AccountId } from '@metamask/keyring-utils'; +import type { Hex, Json } from '@metamask/utils'; + +import type { TrezorControllerState, TrezorKeyring } from './trezor-keyring'; + +const trezorKeyringV2Capabilities: KeyringCapabilities = { + scopes: [EthScope.Eoa], +}; + +/** + * Methods supported by Trezor keyring EOA accounts. + * Trezor keyrings support signing transactions, personal messages, and typed data (v3 and v4). + */ +const TREZOR_KEYRING_EOA_METHODS = [ + EthMethod.SignTransaction, + EthMethod.Sign, + EthMethod.PersonalSign, + EthMethod.SignTypedDataV3, + EthMethod.SignTypedDataV4, +]; + +export type TrezorKeyringV2Options = { + legacyKeyring: TrezorKeyring; + entropySourceId: string; +}; + +export class TrezorKeyringV2 + extends KeyringWrapper + implements KeyringV2 +{ + /** + * In-memory cache of KeyringAccount objects. + * Maps address (as Hex) to KeyringAccount. + */ + readonly #accountsCache = new Map(); + + constructor(options: TrezorKeyringV2Options) { + super({ + type: KeyringType.Trezor, + inner: options.legacyKeyring, + capabilities: trezorKeyringV2Capabilities, + entropySourceId: options.entropySourceId, + }); + } + + /** + * Helper method to safely cast a KeyringAccount address to Hex type. + * The KeyringAccount.address is typed as string, but for Ethereum accounts + * it should always be a valid Hex address. + * + * @param address - The address from a KeyringAccount. + * @returns The address as Hex type. + */ + #toHexAddress(address: string): Hex { + return address as Hex; + } + + /** + * Creates a KeyringAccount object for the given address. + * + * @param address - The account address. + * @returns The created KeyringAccount. + */ + #createKeyringAccount(address: Hex): KeyringAccount { + const id = this.resolver.register(address); + + const account: KeyringAccount = { + id, + type: EthAccountType.Eoa, + address, + scopes: this.capabilities.scopes, + methods: TREZOR_KEYRING_EOA_METHODS, + options: { + entropy: { + type: KeyringAccountEntropyTypeOption.PrivateKey, + }, + }, + }; + + // Cache the account + this.#accountsCache.set(address, account); + + return account; + } + + async getAccounts(): Promise { + const addresses = await this.inner.getAccounts(); + + return addresses.map((address) => { + // Check if we already have this account cached + const cached = this.#accountsCache.get(address); + if (cached) { + return cached; + } + + // Create and cache the account if not already cached + return this.#createKeyringAccount(address); + }); + } + + async deserialize(state: Json): Promise { + // Clear the cache when deserializing + this.#accountsCache.clear(); + + // Deserialize the legacy keyring + await this.inner.deserialize(state as TrezorControllerState); + + // Rebuild the cache by populating it with all accounts + // We call getAccounts() which will repopulate the cache as a side effect + await this.getAccounts(); + } + + async createAccounts(): Promise { + const [address] = await this.inner.addAccounts(1); + + if (!address) { + throw new Error('Failed to create Trezor keyring account'); + } + + const hexAddress = this.#toHexAddress(address); + const account = this.#createKeyringAccount(hexAddress); + + return [account]; + } + + async deleteAccount(accountId: AccountId): Promise { + // Sync with the inner keyring state in case it was modified externally + // This ensures our cache is up-to-date before we make changes + await this.getAccounts(); + + const account = await this.getAccount(accountId); + const hexAddress = this.#toHexAddress(account.address); + + // Remove from the legacy keyring + this.inner.removeAccount(hexAddress); + + // Remove from our cache + this.#accountsCache.delete(hexAddress); + } + + async submitRequest(request: KeyringRequest): Promise { + const { method, params = [] } = request.request; + + const { address } = await this.getAccount(request.account); + const hexAddress = this.#toHexAddress(address); + + // Validate params is an array + if (!Array.isArray(params)) { + throw new Error('Expected params to be an array'); + } + + switch (method) { + case `${EthMethod.SignTransaction}`: { + if (params.length < 1) { + throw new Error('Invalid params for eth_signTransaction'); + } + const [tx] = params; + return this.inner.signTransaction( + hexAddress, + tx as unknown as TypedTransaction, + ) as unknown as Json; + } + + case `${EthMethod.Sign}`: { + if (params.length < 2) { + throw new Error('Invalid params for eth_sign'); + } + const [, data] = params; + return this.inner.signMessage(hexAddress, data as string); + } + + case `${EthMethod.PersonalSign}`: { + if (params.length < 1) { + throw new Error('Invalid params for personal_sign'); + } + const [data] = params; + return this.inner.signPersonalMessage(hexAddress, data as string); + } + + case `${EthMethod.SignTypedDataV3}`: + case `${EthMethod.SignTypedDataV4}`: { + if (params.length < 2) { + throw new Error(`Invalid params for ${method}`); + } + const [, data] = params; + const version = + method === EthMethod.SignTypedDataV4 + ? SignTypedDataVersion.V4 + : SignTypedDataVersion.V3; + + return this.inner.signTypedData( + hexAddress, + data as unknown as TypedMessage, + { + version, + }, + ); + } + + default: + throw new Error(`Unsupported method for TrezorKeyringV2: ${method}`); + } + } +} diff --git a/packages/keyring-eth-trezor/tsconfig.build.json b/packages/keyring-eth-trezor/tsconfig.build.json index 926b19e74..d43758d2d 100644 --- a/packages/keyring-eth-trezor/tsconfig.build.json +++ b/packages/keyring-eth-trezor/tsconfig.build.json @@ -10,7 +10,10 @@ "lib": ["ES2020"], "target": "es2017" }, - "references": [{ "path": "../keyring-utils/tsconfig.build.json" }], + "references": [ + { "path": "../keyring-utils/tsconfig.build.json" }, + { "path": "../keyring-api/tsconfig.build.json" } + ], "include": ["./src/**/*.ts"], "exclude": ["./src/**/*.test.ts"] } diff --git a/packages/keyring-eth-trezor/tsconfig.json b/packages/keyring-eth-trezor/tsconfig.json index 16c034161..b329155c0 100644 --- a/packages/keyring-eth-trezor/tsconfig.json +++ b/packages/keyring-eth-trezor/tsconfig.json @@ -8,7 +8,7 @@ "lib": ["ES2020"], "target": "es2017" }, - "references": [{ "path": "../keyring-utils" }], + "references": [{ "path": "../keyring-utils" }, { "path": "../keyring-api" }], "include": ["./src"], "exclude": ["./dist/**/*"] } diff --git a/tsconfig.packages.json b/tsconfig.packages.json index 847976f51..9ce325f80 100644 --- a/tsconfig.packages.json +++ b/tsconfig.packages.json @@ -28,7 +28,7 @@ * `jest.config.packages.js`. */ "paths": { - "@metamask/*": ["../*/src"] + "@metamask/*": ["./packages/*/src", "../*/src"] } } } diff --git a/yarn.lock b/yarn.lock index cde1599d3..4f74e792f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1682,6 +1682,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-api": "workspace:^" "@metamask/keyring-utils": "workspace:^" "@metamask/scure-bip39": "npm:^2.1.1" "@metamask/utils": "npm:^11.1.0" @@ -1711,6 +1712,7 @@ __metadata: "@ledgerhq/types-live": "npm:^6.52.0" "@metamask/auto-changelog": "npm:^3.4.4" "@metamask/eth-sig-util": "npm:^8.2.0" + "@metamask/keyring-api": "workspace:^" "@metamask/keyring-utils": "workspace:^" "@metamask/utils": "npm:^11.1.0" "@ts-bridge/cli": "npm:^0.6.3" @@ -1745,6 +1747,7 @@ __metadata: "@lavamoat/allow-scripts": "npm:^3.2.1" "@metamask/auto-changelog": "npm:^3.4.4" "@metamask/eth-sig-util": "npm:^8.2.0" + "@metamask/keyring-api": "workspace:^" "@metamask/keyring-utils": "workspace:^" "@metamask/utils": "npm:^11.1.0" "@types/hdkey": "npm:^2.0.1" @@ -1811,6 +1814,7 @@ __metadata: "@lavamoat/preinstall-always-fail": "npm:^2.1.0" "@metamask/auto-changelog": "npm:^3.4.4" "@metamask/eth-sig-util": "npm:^8.2.0" + "@metamask/keyring-api": "workspace:^" "@metamask/keyring-utils": "workspace:^" "@metamask/utils": "npm:^11.1.0" "@ts-bridge/cli": "npm:^0.6.3" @@ -1881,6 +1885,7 @@ __metadata: "@lavamoat/preinstall-always-fail": "npm:^2.1.0" "@metamask/auto-changelog": "npm:^3.4.4" "@metamask/eth-sig-util": "npm:^8.2.0" + "@metamask/keyring-api": "workspace:^" "@metamask/keyring-utils": "workspace:^" "@metamask/utils": "npm:^11.1.0" "@trezor/connect-plugin-ethereum": "npm:^9.0.5" @@ -1969,6 +1974,7 @@ __metadata: "@ts-bridge/cli": "npm:^0.6.3" "@types/jest": "npm:^29.5.12" "@types/node": "npm:^20.12.12" + "@types/uuid": "npm:^9.0.8" "@types/webextension-polyfill": "npm:^0.12.1" bitcoin-address-validation: "npm:^2.2.3" deepmerge: "npm:^4.2.2" @@ -1980,6 +1986,7 @@ __metadata: tsd: "npm:^0.31.0" typedoc: "npm:^0.25.13" typescript: "npm:~5.6.3" + uuid: "npm:^9.0.1" languageName: unknown linkType: soft