diff --git a/README.md b/README.md index 9e469e389..4b6a53827 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,9 @@ linkStyle default opacity:0.5 eth_ledger_bridge_keyring --> keyring_api; eth_ledger_bridge_keyring --> keyring_utils; eth_ledger_bridge_keyring --> account_api; + eth_qr_keyring --> keyring_api; eth_qr_keyring --> keyring_utils; + eth_qr_keyring --> account_api; eth_simple_keyring --> keyring_api; eth_simple_keyring --> keyring_utils; eth_trezor_keyring --> keyring_utils; diff --git a/packages/keyring-eth-qr/CHANGELOG.md b/packages/keyring-eth-qr/CHANGELOG.md index a4ed08a05..e0b41a186 100644 --- a/packages/keyring-eth-qr/CHANGELOG.md +++ b/packages/keyring-eth-qr/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `QrKeyringV2` class implementing `KeyringV2` interface ([#411](https://github.com/MetaMask/accounts/pull/411)) + - Wraps legacy `QrKeyring` to expose accounts via the unified `KeyringV2` API and the `KeyringAccount` type. + - Extends `EthKeyringWrapper` for common Ethereum logic. + ## [1.1.0] ### Added diff --git a/packages/keyring-eth-qr/package.json b/packages/keyring-eth-qr/package.json index 080ecb5d8..b22df5a00 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", @@ -63,6 +64,7 @@ "@ethereumjs/common": "^4.4.0", "@keystonehq/metamask-airgapped-keyring": "^0.15.2", "@lavamoat/allow-scripts": "^3.2.1", + "@metamask/account-api": "workspace:^", "@metamask/auto-changelog": "^3.4.4", "@types/hdkey": "^2.0.1", "@types/jest": "^29.5.12", diff --git a/packages/keyring-eth-qr/src/index.ts b/packages/keyring-eth-qr/src/index.ts index 36a60d855..09d3ee843 100644 --- a/packages/keyring-eth-qr/src/index.ts +++ b/packages/keyring-eth-qr/src/index.ts @@ -17,3 +17,8 @@ export { type SerializedQrKeyringState, type SerializedUR, } from './qr-keyring'; +export { + QrKeyringV2, + type QrKeyringV2Options, + type QrAccountModeCreateOptions, +} 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..fcc450aa7 --- /dev/null +++ b/packages/keyring-eth-qr/src/qr-keyring-v2.test.ts @@ -0,0 +1,759 @@ +import type { Bip44Account } from '@metamask/account-api'; +import { + EthAccountType, + EthMethod, + EthScope, + type KeyringAccount, + KeyringAccountEntropyTypeOption, + KeyringType, +} from '@metamask/keyring-api'; + +import type { QrKeyringBridge } from '.'; +import { QrKeyring } from '.'; +import { + QrKeyringV2, + type QrKeyringCreateAccountOptions, +} from './qr-keyring-v2'; +import { + ACCOUNT_SERIALIZED_KEYRING_WITH_ACCOUNTS, + EXPECTED_ACCOUNTS, + HDKEY_SERIALIZED_KEYRING_WITH_ACCOUNTS, + HDKEY_SERIALIZED_KEYRING_WITH_NO_ACCOUNTS, + KNOWN_CRYPTO_ACCOUNT_UR, + KNOWN_HDKEY_UR, +} from '../test/fixtures'; + +/** + * Type alias for QR keyring HD mode accounts (BIP-44 derived). + */ +type QrHdAccount = Bip44Account; + +/** + * Get the first account from an array, throwing if empty. + * + * @param accounts - The accounts array. + * @returns The first account. + */ +function getFirstAccount(accounts: KeyringAccount[]): KeyringAccount { + if (accounts.length === 0) { + throw new Error('Expected at least one account'); + } + return accounts[0] as KeyringAccount; +} + +/** + * Get the first HD mode account from an array, throwing if empty. + * + * @param accounts - The accounts array. + * @returns The first account cast as QrHdAccount. + */ +function getFirstHdAccount(accounts: KeyringAccount[]): QrHdAccount { + if (accounts.length === 0) { + throw new Error('Expected at least one account'); + } + return accounts[0] as QrHdAccount; +} + +/** + * Get an account at a specific index, throwing if not present. + * + * @param accounts - The accounts array. + * @param index - The index to retrieve. + * @returns The account at the index. + */ +function getAccountAt( + accounts: KeyringAccount[], + index: number, +): KeyringAccount { + if (accounts.length <= index) { + throw new Error(`Expected account at index ${index}`); + } + return accounts[index] as KeyringAccount; +} + +const entropySource = HDKEY_SERIALIZED_KEYRING_WITH_NO_ACCOUNTS.xfp; + +/** + * Expected methods supported by QR keyring accounts. + */ +const EXPECTED_METHODS = [ + EthMethod.SignTransaction, + EthMethod.PersonalSign, + EthMethod.SignTypedDataV4, +]; + +/** + * Get a mock bridge for the QrKeyring. + * + * @returns A mock bridge with a requestScan method. + */ +function getMockBridge(): QrKeyringBridge { + return { + requestScan: jest.fn(), + }; +} + +/** + * Create a fresh QrKeyring with HD mode device. + * + * @returns The inner keyring. + */ +function createInnerKeyring(): QrKeyring { + return new QrKeyring({ + bridge: getMockBridge(), + ur: KNOWN_HDKEY_UR, + }); +} + +/** + * Create a QrKeyringV2 wrapper with a paired HD mode device and accounts. + * + * @param accountCount - Number of accounts to add. + * @returns The wrapper and inner keyring. + */ +async function createWrapperWithAccounts(accountCount = 3): Promise<{ + wrapper: QrKeyringV2; + inner: QrKeyring; +}> { + const inner = createInnerKeyring(); + await inner.addAccounts(accountCount); + + const wrapper = new QrKeyringV2({ legacyKeyring: inner, entropySource }); + return { wrapper, inner }; +} + +/** + * Create a QrKeyringV2 wrapper without any accounts. + * + * @returns The wrapper and inner keyring. + */ +function createEmptyWrapper(): { wrapper: QrKeyringV2; inner: QrKeyring } { + const inner = createInnerKeyring(); + const wrapper = new QrKeyringV2({ legacyKeyring: inner, entropySource }); + return { wrapper, inner }; +} + +/** + * Create a QrKeyringV2 wrapper with a paired Account mode device. + * + * @returns The wrapper and inner keyring. + */ +async function createAccountModeWrapper(): Promise<{ + wrapper: QrKeyringV2; + inner: QrKeyring; +}> { + const inner = new QrKeyring({ + bridge: getMockBridge(), + ur: KNOWN_CRYPTO_ACCOUNT_UR, + }); + await inner.addAccounts(1); + + const wrapper = new QrKeyringV2({ + legacyKeyring: inner, + entropySource: ACCOUNT_SERIALIZED_KEYRING_WITH_ACCOUNTS.xfp, + }); + return { wrapper, inner }; +} + +/** + * Helper to create account options for bip44:derive-index. + * + * @param groupIndex - The group index to derive. + * @param source - Optional entropy source override. + * @returns The create account options. + */ +function deriveIndexOptions( + groupIndex: number, + source: string = entropySource, +): { + type: 'bip44:derive-index'; + entropySource: string; + groupIndex: number; +} { + return { + type: 'bip44:derive-index' as const, + entropySource: source, + groupIndex, + }; +} + +/** + * Helper to create custom account options for Account mode. + * + * @param addressIndex - The index of the pre-defined address. + * @param source - Optional entropy source override. + * @returns The create account options. + */ +function customAccountOptions( + addressIndex: number, + source: string = ACCOUNT_SERIALIZED_KEYRING_WITH_ACCOUNTS.xfp, +): { + type: 'custom'; + entropySource: string; + addressIndex: number; +} { + return { + type: 'custom' as const, + entropySource: source, + addressIndex, + }; +} + +describe('QrKeyringV2', () => { + describe('constructor', () => { + it('creates a wrapper with correct type and capabilities', () => { + const { wrapper } = createEmptyWrapper(); + + expect(wrapper.type).toBe(KeyringType.Qr); + // Default capabilities when no device is paired + expect(wrapper.capabilities).toStrictEqual({ + scopes: [EthScope.Eoa], + bip44: { + deriveIndex: true, + derivePath: false, + discover: false, + }, + }); + }); + }); + + describe('getAccounts', () => { + it('returns correct capabilities immediately for pre-paired Account mode device', async () => { + const inner = new QrKeyring({ + bridge: getMockBridge(), + ur: KNOWN_CRYPTO_ACCOUNT_UR, + }); + await inner.addAccounts(1); + + const wrapper = new QrKeyringV2({ + legacyKeyring: inner, + entropySource: ACCOUNT_SERIALIZED_KEYRING_WITH_ACCOUNTS.xfp, + }); + + // Capabilities are correct immediately - no need to call getAccounts first + expect(wrapper.capabilities).toStrictEqual({ + scopes: [EthScope.Eoa], + custom: { + createAccounts: true, + }, + }); + }); + it('returns empty array when no device is paired', async () => { + const inner = new QrKeyring({ bridge: getMockBridge() }); + const wrapper = new QrKeyringV2({ legacyKeyring: inner, entropySource }); + + const accounts = await wrapper.getAccounts(); + + expect(accounts).toStrictEqual([]); + }); + + it('returns all accounts from the legacy keyring', async () => { + const { wrapper } = await createWrapperWithAccounts(3); + + const accounts = await wrapper.getAccounts(); + + expect(accounts).toHaveLength(3); + expect(accounts.map((a) => a.address)).toStrictEqual( + EXPECTED_ACCOUNTS.slice(0, 3), + ); + }); + + it('creates KeyringAccount objects with correct structure', async () => { + const { wrapper } = await createWrapperWithAccounts(1); + + const accounts = await wrapper.getAccounts(); + const account = getFirstAccount(accounts); + + expect(account).toMatchObject({ + type: EthAccountType.Eoa, + address: EXPECTED_ACCOUNTS[0], + scopes: [EthScope.Eoa], + methods: EXPECTED_METHODS, + options: { + entropy: { + type: KeyringAccountEntropyTypeOption.Mnemonic, + id: entropySource, + groupIndex: 0, + derivationPath: `m/44'/60'/0'/0/0`, + }, + }, + }); + expect(account.id).toBeDefined(); + }); + + it('returns correct groupIndex for multiple accounts', async () => { + const { wrapper } = await createWrapperWithAccounts(3); + + const accounts = await wrapper.getAccounts(); + + accounts.forEach((account, index) => { + expect((account as QrHdAccount).options.entropy.groupIndex).toBe(index); + }); + }); + + it('throws error in HD mode when address is not in indexes map', async () => { + const { wrapper, inner } = await createWrapperWithAccounts(1); + + // Mock serialize to return an inconsistent state + jest.spyOn(inner, 'serialize').mockResolvedValue({ + ...HDKEY_SERIALIZED_KEYRING_WITH_ACCOUNTS, + indexes: {}, // Empty indexes - inconsistent with accounts + }); + + await expect(wrapper.getAccounts()).rejects.toThrow( + /not found in device indexes/u, + ); + }); + + it('throws error when getPathFromAddress returns undefined', async () => { + const { wrapper, inner } = await createWrapperWithAccounts(1); + + jest.spyOn(inner, 'getPathFromAddress').mockReturnValue(undefined); + + await expect(wrapper.getAccounts()).rejects.toThrow( + /derivation path not found in keyring/u, + ); + }); + }); + + describe('deserialize', () => { + it('deserializes the legacy keyring state', async () => { + const inner = new QrKeyring({ bridge: getMockBridge() }); + const wrapper = new QrKeyringV2({ legacyKeyring: inner, entropySource }); + + await wrapper.deserialize(HDKEY_SERIALIZED_KEYRING_WITH_ACCOUNTS); + + const accounts = await wrapper.getAccounts(); + expect(accounts).toHaveLength(3); + expect(accounts.map((a) => a.address)).toStrictEqual( + EXPECTED_ACCOUNTS.slice(0, 3), + ); + }); + + it('clears the cache and rebuilds it', async () => { + const { wrapper } = await createWrapperWithAccounts(2); + + const accountsBefore = await wrapper.getAccounts(); + expect(accountsBefore).toHaveLength(2); + + await wrapper.deserialize(HDKEY_SERIALIZED_KEYRING_WITH_ACCOUNTS); + + const accountsAfter = await wrapper.getAccounts(); + expect(accountsAfter).toHaveLength(3); + // The accounts should be new objects (cache was cleared) + expect(accountsAfter[0]).not.toBe(accountsBefore[0]); + }); + + it('updates capabilities to HD mode after deserializing HD device state', async () => { + const inner = new QrKeyring({ bridge: getMockBridge() }); + const wrapper = new QrKeyringV2({ legacyKeyring: inner, entropySource }); + + await wrapper.deserialize(HDKEY_SERIALIZED_KEYRING_WITH_ACCOUNTS); + + expect(wrapper.capabilities).toStrictEqual({ + scopes: [EthScope.Eoa], + bip44: { + deriveIndex: true, + derivePath: false, + discover: false, + }, + }); + }); + + it('updates capabilities to Account mode after deserializing Account device state', async () => { + const inner = new QrKeyring({ bridge: getMockBridge() }); + const wrapper = new QrKeyringV2({ + legacyKeyring: inner, + entropySource: ACCOUNT_SERIALIZED_KEYRING_WITH_ACCOUNTS.xfp, + }); + + // Initially, capabilities are HD mode defaults (derivePath is false for QR) + + await wrapper.deserialize(ACCOUNT_SERIALIZED_KEYRING_WITH_ACCOUNTS); + + // After deserializing Account mode state, capabilities should update + expect(wrapper.capabilities).toStrictEqual({ + scopes: [EthScope.Eoa], + custom: { + createAccounts: true, + }, + }); + }); + + it('clears custom capability when transitioning from Account mode to HD mode', async () => { + const inner = new QrKeyring({ bridge: getMockBridge() }); + const wrapper = new QrKeyringV2({ + legacyKeyring: inner, + entropySource: ACCOUNT_SERIALIZED_KEYRING_WITH_ACCOUNTS.xfp, + }); + + // First deserialize Account mode to get custom capability + await wrapper.deserialize(ACCOUNT_SERIALIZED_KEYRING_WITH_ACCOUNTS); + expect(wrapper.capabilities.custom).toStrictEqual({ + createAccounts: true, + }); + + // Now deserialize HD mode state - custom capability should be cleared + await wrapper.deserialize(HDKEY_SERIALIZED_KEYRING_WITH_ACCOUNTS); + + expect(wrapper.capabilities.custom).toBeUndefined(); + expect(wrapper.capabilities).toStrictEqual({ + scopes: [EthScope.Eoa], + bip44: { + deriveIndex: true, + derivePath: false, + discover: false, + }, + }); + }); + }); + + describe('createAccounts', () => { + it('creates the first account at index 0', async () => { + const { wrapper } = createEmptyWrapper(); + + const newAccounts = await wrapper.createAccounts(deriveIndexOptions(0)); + const account = getFirstAccount(newAccounts); + + expect(account.address).toBe(EXPECTED_ACCOUNTS[0]); + }); + + it('creates an account at a specific index', async () => { + const { wrapper } = await createWrapperWithAccounts(2); + + const newAccounts = await wrapper.createAccounts(deriveIndexOptions(2)); + const account = getFirstAccount(newAccounts); + + expect(account.address).toBe(EXPECTED_ACCOUNTS[2]); + }); + + it('returns existing account if groupIndex already exists', async () => { + const { wrapper } = await createWrapperWithAccounts(2); + + const existingAccounts = await wrapper.getAccounts(); + const existingAccount = getFirstAccount(existingAccounts); + const newAccounts = await wrapper.createAccounts(deriveIndexOptions(0)); + const returnedAccount = getFirstAccount(newAccounts); + + expect(returnedAccount).toBe(existingAccount); + }); + + it('throws error for unsupported account creation type', async () => { + const { wrapper } = await createWrapperWithAccounts(0); + + await expect( + wrapper.createAccounts({ + type: 'private-key:import', + entropySource, + accountType: EthAccountType.Eoa, + encoding: 'hexadecimal', + privateKey: '0xabc', + } as unknown as QrKeyringCreateAccountOptions), + ).rejects.toThrow( + /Unsupported account creation type for HD mode QrKeyring/u, + ); + }); + + it('throws error for entropy source mismatch', async () => { + const { wrapper } = await createWrapperWithAccounts(0); + + await expect( + wrapper.createAccounts(deriveIndexOptions(0, 'wrong-entropy')), + ).rejects.toThrow(/Entropy source mismatch/u); + }); + + it('throws error when no device is paired', async () => { + const inner = new QrKeyring({ bridge: getMockBridge() }); + const wrapper = new QrKeyringV2({ legacyKeyring: inner, entropySource }); + + await expect( + wrapper.createAccounts(deriveIndexOptions(0, 'some-entropy')), + ).rejects.toThrow('No device paired. Cannot create accounts.'); + }); + + it('allows deriving accounts at any index (non-sequential)', async () => { + const { wrapper } = createEmptyWrapper(); + + const newAccounts = await wrapper.createAccounts(deriveIndexOptions(5)); + const account = getFirstHdAccount(newAccounts); + + expect(account.address).toBe(EXPECTED_ACCOUNTS[5]); + expect(account.options.entropy.groupIndex).toBe(5); + }); + + it('creates multiple accounts sequentially', async () => { + const { wrapper } = createEmptyWrapper(); + + const results = await Promise.all([ + wrapper.createAccounts(deriveIndexOptions(0)), + wrapper.createAccounts(deriveIndexOptions(1)), + wrapper.createAccounts(deriveIndexOptions(2)), + ]); + + results.forEach((accounts, index) => { + const account = getFirstAccount(accounts); + expect(account.address).toBe(EXPECTED_ACCOUNTS[index]); + }); + }); + + it('throws error when inner keyring fails to create account', async () => { + const { wrapper, inner } = createEmptyWrapper(); + + jest.spyOn(inner, 'addAccounts').mockResolvedValueOnce([]); + + await expect( + wrapper.createAccounts(deriveIndexOptions(0)), + ).rejects.toThrow('Failed to create new account'); + }); + + it('throws error for negative groupIndex', async () => { + const { wrapper } = createEmptyWrapper(); + + await expect( + wrapper.createAccounts(deriveIndexOptions(-1)), + ).rejects.toThrow(/Invalid groupIndex: -1/u); + }); + + it('throws error for unsupported derive-path type', async () => { + const { wrapper } = createEmptyWrapper(); + + await expect( + wrapper.createAccounts({ + type: 'bip44:derive-path', + entropySource, + derivationPath: `m/44'/60'/0'/0/0`, + } as unknown as QrKeyringCreateAccountOptions), + ).rejects.toThrow( + /Unsupported account creation type for HD mode QrKeyring: bip44:derive-path/u, + ); + }); + }); + + describe('deleteAccount', () => { + it('removes an account from the keyring', async () => { + const { wrapper } = await createWrapperWithAccounts(3); + + const accountsBefore = await wrapper.getAccounts(); + const accountToDelete = getAccountAt(accountsBefore, 1); + + await wrapper.deleteAccount(accountToDelete.id); + + const accountsAfter = await wrapper.getAccounts(); + expect(accountsAfter).toHaveLength(2); + expect(accountsAfter.map((a) => a.address)).not.toContain( + accountToDelete.address, + ); + }); + + it('removes the account from the cache', async () => { + const { wrapper } = await createWrapperWithAccounts(2); + + const accounts = await wrapper.getAccounts(); + const accountToDelete = getFirstAccount(accounts); + + await wrapper.deleteAccount(accountToDelete.id); + + await expect(wrapper.getAccount(accountToDelete.id)).rejects.toThrow( + /Account not found/u, + ); + }); + + it('throws error for non-existent account', async () => { + const { wrapper } = await createWrapperWithAccounts(1); + + await expect(wrapper.deleteAccount('non-existent-id')).rejects.toThrow( + /Account not found/u, + ); + }); + }); + + describe('getAccount', () => { + it('returns the account by ID', async () => { + const { wrapper } = await createWrapperWithAccounts(2); + + const accounts = await wrapper.getAccounts(); + const expectedAccount = getFirstAccount(accounts); + const account = await wrapper.getAccount(expectedAccount.id); + + expect(account).toBe(expectedAccount); + }); + + it('throws error for non-existent account', async () => { + const { wrapper } = await createWrapperWithAccounts(1); + + await expect(wrapper.getAccount('non-existent-id')).rejects.toThrow( + /Account not found/u, + ); + }); + }); + + describe('serialize', () => { + it('serializes the legacy keyring state', async () => { + const { wrapper, inner } = await createWrapperWithAccounts(2); + + const wrapperSerialized = await wrapper.serialize(); + const innerSerialized = await inner.serialize(); + + expect(wrapperSerialized).toStrictEqual(innerSerialized); + }); + }); + + describe('Account Mode (CryptoAccount)', () => { + describe('getAccounts', () => { + it('returns accounts with PrivateKey entropy type (pre-defined addresses)', async () => { + const { wrapper } = await createAccountModeWrapper(); + + const accounts = await wrapper.getAccounts(); + const account = getFirstAccount(accounts); + + expect(account.address).toBe( + ACCOUNT_SERIALIZED_KEYRING_WITH_ACCOUNTS.accounts[0], + ); + expect(account.options.entropy).toStrictEqual({ + type: KeyringAccountEntropyTypeOption.Custom, + }); + }); + + it('throws error in Account mode when address is not in paths map', async () => { + const { wrapper, inner } = await createAccountModeWrapper(); + + // Mock serialize to return an inconsistent state + jest.spyOn(inner, 'serialize').mockResolvedValue({ + ...ACCOUNT_SERIALIZED_KEYRING_WITH_ACCOUNTS, + paths: {}, // Empty paths - inconsistent with accounts + }); + + await expect(wrapper.getAccounts()).rejects.toThrow( + /not found in device paths/u, + ); + }); + }); + + describe('createAccounts', () => { + it('throws error when trying to derive by index', async () => { + const { wrapper } = await createAccountModeWrapper(); + + await expect( + wrapper.createAccounts({ + type: 'bip44:derive-index', + entropySource: ACCOUNT_SERIALIZED_KEYRING_WITH_ACCOUNTS.xfp, + groupIndex: 0, + }), + ).rejects.toThrow( + /Account mode devices only support 'custom' account creation type/u, + ); + }); + + it('throws error when trying to derive by path', async () => { + const { wrapper } = await createAccountModeWrapper(); + + await expect( + wrapper.createAccounts({ + type: 'bip44:derive-path', + entropySource: ACCOUNT_SERIALIZED_KEYRING_WITH_ACCOUNTS.xfp, + derivationPath: `m/44'/60'/0'/0/0`, + } as unknown as QrKeyringCreateAccountOptions), + ).rejects.toThrow( + /Account mode devices only support 'custom' account creation type/u, + ); + }); + + it('creates account using custom type with valid addressIndex', async () => { + const inner = new QrKeyring({ + bridge: getMockBridge(), + ur: KNOWN_CRYPTO_ACCOUNT_UR, + }); + // Do not add any accounts initially + const wrapper = new QrKeyringV2({ + legacyKeyring: inner, + entropySource: ACCOUNT_SERIALIZED_KEYRING_WITH_ACCOUNTS.xfp, + }); + + const newAccounts = await wrapper.createAccounts( + customAccountOptions(0), + ); + const account = getFirstAccount(newAccounts); + + expect(account.address).toBe( + ACCOUNT_SERIALIZED_KEYRING_WITH_ACCOUNTS.accounts[0], + ); + expect(account.options.entropy).toStrictEqual({ + type: KeyringAccountEntropyTypeOption.Custom, + }); + }); + + it('returns existing account if addressIndex already exists', async () => { + const { wrapper } = await createAccountModeWrapper(); + + const existingAccounts = await wrapper.getAccounts(); + const existingAccount = getFirstAccount(existingAccounts); + + const newAccounts = await wrapper.createAccounts( + customAccountOptions(0), + ); + const returnedAccount = getFirstAccount(newAccounts); + + expect(returnedAccount).toBe(existingAccount); + }); + + it('throws error for invalid addressIndex type', async () => { + const { wrapper } = await createAccountModeWrapper(); + + await expect( + wrapper.createAccounts({ + type: 'custom', + entropySource: ACCOUNT_SERIALIZED_KEYRING_WITH_ACCOUNTS.xfp, + addressIndex: 'invalid', + } as unknown as { type: 'custom'; entropySource: string; addressIndex: number }), + ).rejects.toThrow(/Invalid addressIndex/u); + }); + + it('throws error for negative addressIndex', async () => { + const { wrapper } = await createAccountModeWrapper(); + + await expect( + wrapper.createAccounts(customAccountOptions(-1)), + ).rejects.toThrow(/Invalid addressIndex/u); + }); + + it('throws error for addressIndex out of bounds', async () => { + const { wrapper } = await createAccountModeWrapper(); + + await expect( + wrapper.createAccounts(customAccountOptions(999)), + ).rejects.toThrow(/Address index 999 is out of bounds/u); + }); + + it('throws error when inner keyring fails to create account', async () => { + const inner = new QrKeyring({ + bridge: getMockBridge(), + ur: KNOWN_CRYPTO_ACCOUNT_UR, + }); + const wrapper = new QrKeyringV2({ + legacyKeyring: inner, + entropySource: ACCOUNT_SERIALIZED_KEYRING_WITH_ACCOUNTS.xfp, + }); + + jest.spyOn(inner, 'addAccounts').mockResolvedValueOnce([]); + + await expect( + wrapper.createAccounts(customAccountOptions(0)), + ).rejects.toThrow('Failed to create new account'); + }); + }); + + describe('deleteAccount', () => { + it('removes an account from the keyring', async () => { + const { wrapper, inner } = await createAccountModeWrapper(); + + const accounts = await wrapper.getAccounts(); + const account = getFirstAccount(accounts); + + await wrapper.deleteAccount(account.id); + + const remainingAddresses = await inner.getAccounts(); + expect(remainingAddresses).toHaveLength(0); + }); + }); + }); +}); 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..a5b5933ae --- /dev/null +++ b/packages/keyring-eth-qr/src/qr-keyring-v2.ts @@ -0,0 +1,437 @@ +import type { Bip44Account } from '@metamask/account-api'; +import { + type CreateAccountOptions, + EthAccountType, + EthKeyringWrapper, + EthMethod, + EthScope, + type KeyringAccount, + KeyringAccountEntropyTypeOption, + type KeyringCapabilities, + type KeyringV2, + KeyringType, + type EntropySourceId, + type CreateAccountBip44DeriveIndexOptions, +} from '@metamask/keyring-api'; +import type { AccountId } from '@metamask/keyring-utils'; +import type { Hex } from '@metamask/utils'; + +import { DeviceMode } from './device'; +import type { QrKeyring } from './qr-keyring'; + +/** + * Methods supported by QR keyring EOA accounts. + * QR keyrings support a subset of signing methods (no encryption, app keys, or EIP-7702). + */ +const QR_KEYRING_METHODS = [ + EthMethod.SignTransaction, + EthMethod.PersonalSign, + EthMethod.SignTypedDataV4, +]; + +/** + * Capabilities for HD mode devices (index-based derivation supported). + */ +const HD_MODE_CAPABILITIES: KeyringCapabilities = { + scopes: [EthScope.Eoa], + bip44: { + deriveIndex: true, + derivePath: false, + discover: false, + }, +}; + +/** + * Capabilities for Account mode devices (custom account selection). + */ +const ACCOUNT_MODE_CAPABILITIES: KeyringCapabilities = { + scopes: [EthScope.Eoa], + custom: { + createAccounts: true, + }, +}; + +/** + * Custom options for creating accounts on Account mode QR devices. + * Account mode devices provide pre-defined addresses that must be selected by index. + */ +export type QrAccountModeCreateOptions = { + type: 'custom'; + /** + * The entropy source ID (device fingerprint) to verify we're targeting the correct device. + */ + entropySource: EntropySourceId; + /** + * The index of the pre-defined address to add from the device. + * This refers to the position in the device's address list, not a BIP-44 derivation index. + */ + addressIndex: number; +}; + +/** + * Account creation options for QR keyring. + * Excludes unsupported types (custom, private-key:import) and adds our specific QrAccountModeCreateOptions. + */ +export type QrKeyringCreateAccountOptions = + | CreateAccountBip44DeriveIndexOptions + | QrAccountModeCreateOptions; + +/** + * Concrete {@link KeyringV2} adapter for {@link QrKeyring}. + * + * This wrapper exposes the accounts and signing capabilities of the legacy + * QR keyring via the unified V2 interface. + * + * Account handling differs by device mode: + * - **HD mode**: Accounts are derived by index with `groupIndex` values. + * Supports `bip44:derive-index` for index-based derivation. + * - **Account mode**: Accounts are treated as private key imports since the + * device provides pre-defined addresses with arbitrary paths. Uses `custom` + * account creation type to select addresses by their position in the device. + */ +export type QrKeyringV2Options = { + legacyKeyring: QrKeyring; + entropySource: EntropySourceId; +}; + +export class QrKeyringV2 + extends EthKeyringWrapper + implements KeyringV2 +{ + readonly entropySource: EntropySourceId; + + constructor(options: QrKeyringV2Options) { + super({ + type: KeyringType.Qr, + inner: options.legacyKeyring, + // Placeholder - will be overridden by getter below + capabilities: HD_MODE_CAPABILITIES, + }); + this.entropySource = options.entropySource; + + // Override the readonly capabilities property with a dynamic getter + // that returns capabilities based on the current device mode. + Object.defineProperty(this, 'capabilities', { + get: (): KeyringCapabilities => { + return this.inner.getMode() === DeviceMode.ACCOUNT + ? ACCOUNT_MODE_CAPABILITIES + : HD_MODE_CAPABILITIES; + }, + enumerable: true, + configurable: false, + }); + } + + /** + * Get the device state from the inner keyring. + * + * @returns The device state, or undefined if no device is paired. + */ + async #getDeviceState(): Promise< + | { + mode: DeviceMode.HD; + indexes: Record; + } + | { + mode: DeviceMode.ACCOUNT; + indexes: Record; + paths: Record; + } + | undefined + > { + const state = await this.inner.serialize(); + + if (!state?.initialized) { + return undefined; + } + + if (state.keyringMode === DeviceMode.ACCOUNT) { + return { + mode: DeviceMode.ACCOUNT, + indexes: state.indexes, + paths: state.paths, + }; + } + + return { + mode: DeviceMode.HD, + indexes: state.indexes, + }; + } + + /** + * Creates a Bip44Account for HD mode devices. + * + * @param address - The account address. + * @param addressIndex - The account index in the derivation path. + * @returns The created Bip44Account. + */ + #createHdModeAccount( + address: Hex, + addressIndex: number, + ): Bip44Account { + const id = this.registry.register(address); + + const derivationPath = this.inner.getPathFromAddress(address); + + if (!derivationPath) { + throw new Error( + `Cannot create account for address ${address}: derivation path not found in keyring.`, + ); + } + + const account: Bip44Account = { + id, + type: EthAccountType.Eoa, + address, + scopes: [...this.capabilities.scopes], + methods: [...QR_KEYRING_METHODS], + options: { + entropy: { + type: KeyringAccountEntropyTypeOption.Mnemonic, + id: this.entropySource, + groupIndex: addressIndex, + derivationPath, + }, + }, + }; + + this.registry.set(account); + return account; + } + + /** + * Creates a KeyringAccount for Account mode devices. + * + * Account mode devices provide pre-defined addresses with arbitrary derivation + * paths, so we treat them as private key imports rather than BIP-44 accounts. + * + * @param address - The account address. + * @returns The created KeyringAccount. + */ + #createAccountModeAccount(address: Hex): KeyringAccount { + const id = this.registry.register(address); + + const account: KeyringAccount = { + id, + type: EthAccountType.Eoa, + address, + scopes: [...this.capabilities.scopes], + methods: [...QR_KEYRING_METHODS], + options: { + entropy: { + type: KeyringAccountEntropyTypeOption.Custom, + }, + }, + }; + + this.registry.set(account); + return account; + } + + async getAccounts(): Promise { + const addresses = await this.inner.getAccounts(); + const deviceState = await this.#getDeviceState(); + + if (!deviceState) { + // No device paired yet, return empty + return []; + } + + const { mode, indexes } = deviceState; + + return addresses.map((address) => { + // Check if we already have this account in the registry + const existingId = this.registry.getAccountId(address); + if (existingId) { + const cached = this.registry.get(existingId); + if (cached) { + return cached; + } + } + + if (mode === DeviceMode.HD) { + // HD mode: index must be in the map + const addressIndex = indexes[address]; + if (addressIndex === undefined) { + throw new Error( + `Address ${address} not found in device indexes. This indicates an inconsistent keyring state.`, + ); + } + return this.#createHdModeAccount(address, addressIndex); + } + + // Account mode: validate address exists in paths map + const { paths } = deviceState; + if (paths[address] === undefined) { + throw new Error( + `Address ${address} not found in device paths. This indicates an inconsistent keyring state.`, + ); + } + return this.#createAccountModeAccount(address); + }); + } + + async createAccounts( + options: QrKeyringCreateAccountOptions, + ): Promise { + return this.withLock(async () => { + const deviceState = await this.#getDeviceState(); + + if (!deviceState) { + throw new Error('No device paired. Cannot create accounts.'); + } + + // Validate entropy source for all account creation types + if (options.entropySource !== this.entropySource) { + throw new Error( + `Entropy source mismatch: expected '${this.entropySource}', got '${options.entropySource}'`, + ); + } + + // Handle Account mode with custom account creation + if (deviceState.mode === DeviceMode.ACCOUNT) { + if (options.type !== 'custom') { + throw new Error( + `Account mode devices only support 'custom' account creation type, got '${options.type}'. ` + + `Use { type: 'custom', entropySource, addressIndex } to select a pre-defined address.`, + ); + } + return this.#createAccountModeAccounts(options, deviceState); + } + + // HD mode: support derive-index + return this.#createHdModeAccounts(options, deviceState); + }); + } + + /** + * Creates accounts for Account mode devices using custom options. + * + * @param options - The account creation options. + * @param deviceState - The current device state. + * @param deviceState.mode - The device mode (ACCOUNT). + * @param deviceState.paths - Map of addresses to derivation paths. + * @param deviceState.indexes - Map of addresses to their indexes. + * @returns The created accounts. + */ + async #createAccountModeAccounts( + options: QrAccountModeCreateOptions, + deviceState: { + mode: DeviceMode.ACCOUNT; + paths: Record; + indexes: Record; + }, + ): Promise { + const { addressIndex } = options; + + if (!Number.isInteger(addressIndex) || addressIndex < 0) { + throw new Error( + `Invalid addressIndex: ${String( + addressIndex, + )}. Must be a non-negative integer.`, + ); + } + + // Get available addresses from paths + const availableAddresses = Object.keys(deviceState.paths) as Hex[]; + if (addressIndex >= availableAddresses.length) { + throw new Error( + `Address index ${addressIndex} is out of bounds. Device has ${availableAddresses.length} pre-defined address(es).`, + ); + } + + const address = availableAddresses[addressIndex]; + + // Check if already exists + const currentAccounts = await this.getAccounts(); + const existingAccount = currentAccounts.find( + (account) => account.address.toLowerCase() === address?.toLowerCase(), + ); + if (existingAccount) { + return [existingAccount]; + } + + // Add the account via the inner keyring + this.inner.setAccountToUnlock(addressIndex); + const [newAddress] = await this.inner.addAccounts(1); + + if (!newAddress) { + throw new Error('Failed to create new account'); + } + + const newAccount = this.#createAccountModeAccount(newAddress); + return [newAccount]; + } + + /** + * Creates accounts for HD mode devices using index-based derivation. + * + * @param options - The account creation options. + * @param _deviceState - The current device state (reserved for future use). + * @param _deviceState.indexes - Map of addresses to their indexes. + * @returns The created accounts. + */ + async #createHdModeAccounts( + options: CreateAccountOptions, + _deviceState: { indexes: Record }, + ): Promise[]> { + if (options.type !== 'bip44:derive-index') { + throw new Error( + `Unsupported account creation type for HD mode QrKeyring: ${String( + options.type, + )}. Supported type: 'bip44:derive-index'.`, + ); + } + + if (!Number.isInteger(options.groupIndex) || options.groupIndex < 0) { + throw new Error( + `Invalid groupIndex: ${options.groupIndex}. Must be a non-negative integer.`, + ); + } + + const targetIndex = options.groupIndex; + + // Check if an account at this index already exists + const currentAccounts = await this.getAccounts(); + const existingAccount = currentAccounts.find( + (account) => + account.options.entropy?.type === + KeyringAccountEntropyTypeOption.Mnemonic && + account.options.entropy.groupIndex === targetIndex, + ); + + if (existingAccount) { + return [existingAccount as Bip44Account]; + } + + // Derive the account at the specified index + this.inner.setAccountToUnlock(targetIndex); + const [newAddress] = await this.inner.addAccounts(1); + + if (!newAddress) { + throw new Error('Failed to create new account'); + } + + const newAccount = this.#createHdModeAccount(newAddress, targetIndex); + return [newAccount]; + } + + /** + * Delete an account from the keyring. + * + * @param accountId - The account ID to delete. + */ + async deleteAccount(accountId: AccountId): Promise { + await this.withLock(async () => { + const { address } = await this.getAccount(accountId); + const hexAddress = this.toHexAddress(address); + + // Remove from the legacy keyring + this.inner.removeAccount(hexAddress); + + // Remove from the registry + this.registry.delete(accountId); + }); + } +} diff --git a/packages/keyring-eth-qr/src/qr-keyring.ts b/packages/keyring-eth-qr/src/qr-keyring.ts index f821ba975..7720906e0 100644 --- a/packages/keyring-eth-qr/src/qr-keyring.ts +++ b/packages/keyring-eth-qr/src/qr-keyring.ts @@ -157,6 +157,17 @@ export class QrKeyring implements Keyring { this.#accounts = (state.accounts ?? []).map(normalizeAddress); } + /** + * Get the derivation path for a given address. + * + * @param address - The address to get the derivation path for. + * @returns The derivation path for the address, or undefined if the device + * is not paired or the address is not found. + */ + getPathFromAddress(address: Hex): string | undefined { + return this.#device?.pathFromAddress(address); + } + /** * Adds accounts to the QrKeyring * @@ -242,6 +253,15 @@ export class QrKeyring implements Keyring { return source.name; } + /** + * Get the mode of the paired device. + * + * @returns The device mode, or undefined if no device is paired. + */ + getMode(): DeviceMode | undefined { + return this.#device?.getDeviceDetails().keyringMode; + } + /** * Fetch the first page of accounts. If the keyring is not currently initialized, * it will trigger a scan request to initialize it. diff --git a/packages/keyring-eth-qr/test/fixtures.ts b/packages/keyring-eth-qr/test/fixtures.ts index 799e10ead..c7ac5ef20 100644 --- a/packages/keyring-eth-qr/test/fixtures.ts +++ b/packages/keyring-eth-qr/test/fixtures.ts @@ -99,12 +99,12 @@ export const HDKEY_SERIALIZED_KEYRING_WITH_ACCOUNTS = { [EXPECTED_ACCOUNTS[1]]: 1, [EXPECTED_ACCOUNTS[2]]: 2, }, -}; +} as const; export const ACCOUNT_SERIALIZED_KEYRING_WITH_NO_ACCOUNTS = { - initialized: true, + initialized: true as const, name: 'imToken-Account 01', - keyringMode: DeviceMode.ACCOUNT, + keyringMode: DeviceMode.ACCOUNT as const, keyringAccount: KNOWN_HDKEY.getNote(), xfp: '37b5eed4', paths: { diff --git a/packages/keyring-eth-qr/tsconfig.build.json b/packages/keyring-eth-qr/tsconfig.build.json index 08ef33751..7c5eabdc3 100644 --- a/packages/keyring-eth-qr/tsconfig.build.json +++ b/packages/keyring-eth-qr/tsconfig.build.json @@ -9,6 +9,12 @@ "references": [ { "path": "../keyring-utils/tsconfig.build.json" + }, + { + "path": "../keyring-api/tsconfig.build.json" + }, + { + "path": "../account-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..50f5c182d 100644 --- a/packages/keyring-eth-qr/tsconfig.json +++ b/packages/keyring-eth-qr/tsconfig.json @@ -3,7 +3,11 @@ "compilerOptions": { "baseUrl": "./" }, - "references": [{ "path": "../keyring-utils" }], + "references": [ + { "path": "../keyring-utils" }, + { "path": "../keyring-api" }, + { "path": "../account-api" } + ], "include": ["./src", "./test"], "exclude": ["./dist/**/*"] } diff --git a/yarn.lock b/yarn.lock index c1eafc528..d85f62b3a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1756,8 +1756,10 @@ __metadata: "@keystonehq/bc-ur-registry-eth": "npm:^0.19.1" "@keystonehq/metamask-airgapped-keyring": "npm:^0.15.2" "@lavamoat/allow-scripts": "npm:^3.2.1" + "@metamask/account-api": "workspace:^" "@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"