diff --git a/packages/bitcoin-wallet-snap/CHANGELOG.md b/packages/bitcoin-wallet-snap/CHANGELOG.md index 13994e80..a21d70a7 100644 --- a/packages/bitcoin-wallet-snap/CHANGELOG.md +++ b/packages/bitcoin-wallet-snap/CHANGELOG.md @@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add support for keyring API v2, including `bip44:derive-path`, `bip44:derive-index`, `bip44:derive-index-range`, and `bip44:discover` account creation types ([#43](https://github.com/MetaMask/internal-snaps/pull/43)) +- Add `exportAccount` method supporting WIF (base58) private key export ([#43](https://github.com/MetaMask/internal-snaps/pull/43)) +- Add `parseDerivationPath` to validate and parse BIP-44 derivation paths for native segwit (BIP-84) accounts ([#43](https://github.com/MetaMask/internal-snaps/pull/43)) + +### Changed + +- **BREAKING** Remove v1 keyring API support (`createAccount`, v1 `Keyring` interface) in favour of v2 ([#43](https://github.com/MetaMask/internal-snaps/pull/43)) +- Migrate `KeyringHandler` to implement `KeyringSnapRpc` from `@metamask/keyring-api/v2` ([#43](https://github.com/MetaMask/internal-snaps/pull/43)) +- Update `snap.manifest.json` to declare `derivePath` capability in the `bip44` keyring block ([#43](https://github.com/MetaMask/internal-snaps/pull/43)) +- Mark accounts as exportable in the keyring account mapping ([#43](https://github.com/MetaMask/internal-snaps/pull/43)) + ## [1.15.2] ### Fixed diff --git a/packages/bitcoin-wallet-snap/integration-test/client-request.test.ts b/packages/bitcoin-wallet-snap/integration-test/client-request.test.ts index 6a576dbc..f984332a 100644 --- a/packages/bitcoin-wallet-snap/integration-test/client-request.test.ts +++ b/packages/bitcoin-wallet-snap/integration-test/client-request.test.ts @@ -52,18 +52,18 @@ describe('OnClientRequestHandler', () => { const response = await snap.onKeyringRequest({ origin: ORIGIN, - method: 'keyring_createAccount', + method: 'keyring_createAccounts', params: { - options: { - scope: BtcScope.Regtest, - synchronize: false, - index: ACCOUNT_INDEX, - }, + type: 'bip44:derive-path', + entropySource: 'm', + derivationPath: `m/84'/1'/${ACCOUNT_INDEX}'`, }, }); if ('result' in response.response) { - account = response.response.result as KeyringAccount; + account = ( + response.response.result as KeyringAccount[] + )[0] as KeyringAccount; createdAccountId = account.id; } diff --git a/packages/bitcoin-wallet-snap/integration-test/cron-sync.test.ts b/packages/bitcoin-wallet-snap/integration-test/cron-sync.test.ts index 273e00ff..e489c5ef 100644 --- a/packages/bitcoin-wallet-snap/integration-test/cron-sync.test.ts +++ b/packages/bitcoin-wallet-snap/integration-test/cron-sync.test.ts @@ -60,22 +60,19 @@ describe('CronHandler', () => { // create account without initial sync const createResponse = await snap.onKeyringRequest({ origin: ORIGIN, - method: 'keyring_createAccount', + method: 'keyring_createAccounts', params: { - options: { - scope: BtcScope.Regtest, - addressType: BtcAccountType.P2wpkh, - synchronize: false, - index: ACCOUNT_INDEX, - }, + type: 'bip44:derive-path', + entropySource: 'm', + derivationPath: `m/84'/1'/${ACCOUNT_INDEX}'`, }, }); expect(createResponse.response).toBeDefined(); expect('result' in createResponse.response).toBe(true); - const account = (createResponse.response as { result: KeyringAccount }) - .result; + const account = (createResponse.response as { result: KeyringAccount[] }) + .result[0] as KeyringAccount; accountsToSync.push(account.id); diff --git a/packages/bitcoin-wallet-snap/integration-test/keyring-request.test.ts b/packages/bitcoin-wallet-snap/integration-test/keyring-request.test.ts index f777d740..b698dbe5 100644 --- a/packages/bitcoin-wallet-snap/integration-test/keyring-request.test.ts +++ b/packages/bitcoin-wallet-snap/integration-test/keyring-request.test.ts @@ -50,18 +50,18 @@ describe('KeyringRequestHandler', () => { const response = await snap.onKeyringRequest({ origin: ORIGIN, - method: 'keyring_createAccount', + method: 'keyring_createAccounts', params: { - options: { - scope: BtcScope.Regtest, - synchronize: false, - index: ACCOUNT_INDEX, - }, + type: 'bip44:derive-path', + entropySource: 'm', + derivationPath: `m/84'/1'/${ACCOUNT_INDEX}'`, }, }); if ('result' in response.response) { - account = response.response.result as KeyringAccount; + account = ( + response.response.result as KeyringAccount[] + )[0] as KeyringAccount; createdAccountId = account.id; } @@ -86,9 +86,9 @@ describe('KeyringRequestHandler', () => { }); expect(response).toRespondWithError({ - code: -32000, + code: -32603, message: - 'Invalid format: At path: params.account -- Expected a value of type `UuidV4`, but received: `"notAUUID"`', + 'At path: params.account -- Expected a value of type `UuidV4`, but received: `"notAUUID"`', stack: expect.anything(), }); }); @@ -109,14 +109,8 @@ describe('KeyringRequestHandler', () => { }); expect(response).toRespondWithError({ - code: -32601, - data: { - account: account.id, - cause: null, - method: 'invalidMethod', - }, - message: - 'Method not implemented or not supported: Unrecognized Bitcoin account capability', + code: -32603, + message: 'Unrecognized Bitcoin account capability', stack: expect.anything(), }); }); @@ -381,12 +375,8 @@ describe('KeyringRequestHandler', () => { }); expect(response).toRespondWithError({ - code: -32000, - message: 'Invalid format: Invalid PSBT', - data: { - cause: null, - transaction: 'notAPsbt', - }, + code: -32603, + message: 'Invalid PSBT', stack: expect.anything(), }); }); @@ -415,9 +405,9 @@ describe('KeyringRequestHandler', () => { }); expect(response).toRespondWithError({ - code: -32000, + code: -32603, message: - 'Invalid format: At path: account -- Expected an object, but received: undefined', + 'At path: account -- Expected an object, but received: undefined', stack: expect.anything(), }); }); @@ -442,9 +432,9 @@ describe('KeyringRequestHandler', () => { }); expect(response).toRespondWithError({ - code: -32000, + code: -32603, message: - 'Invalid format: At path: options -- Expected an object, but received: undefined', + 'At path: options -- Expected an object, but received: undefined', stack: expect.anything(), }); }); @@ -503,12 +493,8 @@ describe('KeyringRequestHandler', () => { }); expect(response).toRespondWithError({ - code: -32000, - message: 'Invalid format: Invalid PSBT', - data: { - cause: null, - transaction: 'notAPsbt', - }, + code: -32603, + message: 'Invalid PSBT', stack: expect.anything(), }); }); @@ -567,12 +553,8 @@ describe('KeyringRequestHandler', () => { }); expect(response).toRespondWithError({ - code: -32000, - message: 'Invalid format: Invalid PSBT', - data: { - cause: null, - transaction: 'notAPsbt', - }, + code: -32603, + message: 'Invalid PSBT', stack: expect.anything(), }); }); @@ -665,12 +647,8 @@ describe('KeyringRequestHandler', () => { }); expect(response).toRespondWithError({ - code: -32000, - message: 'Invalid format: Invalid PSBT', - data: { - cause: null, - transaction: 'notAPsbt', - }, + code: -32603, + message: 'Invalid PSBT', stack: expect.anything(), }); }); @@ -737,9 +715,8 @@ describe('KeyringRequestHandler', () => { }); expect(response).toRespondWithError({ - code: -32602, - data: { address: 'notAnAddress', amount: '1000', cause: null }, - message: 'Validation failed: Invalid recipient', + code: -32603, + message: 'Invalid recipient', stack: expect.anything(), }); }); diff --git a/packages/bitcoin-wallet-snap/integration-test/keyring.test.ts b/packages/bitcoin-wallet-snap/integration-test/keyring.test.ts index 0908ade2..ff7e3745 100644 --- a/packages/bitcoin-wallet-snap/integration-test/keyring.test.ts +++ b/packages/bitcoin-wallet-snap/integration-test/keyring.test.ts @@ -53,61 +53,41 @@ describe('Keyring', () => { }); }); - it('discover accounts successfully', async () => { + it('creates account at explicit derivation path', async () => { const response = await snap.onKeyringRequest({ origin: ORIGIN, - method: 'keyring_discoverAccounts', + method: 'keyring_createAccounts', params: { - scopes: [BtcScope.Regtest], // avoid using other networks than Regtest as real external calls will be performed - entropySource: 'm', // we don't know the real entropy source so "m" acts as the default - groupIndex: 0, + type: 'bip44:derive-path', + entropySource: 'm', + derivationPath: `m/84'/1'/${ACCOUNT_INDEX}'`, }, }); - // We should get 1 account, the p2wpkh one of Regtest expect(response).toRespondWith([ { - type: 'bip44', - scopes: [BtcScope.Regtest], - derivationPath: `m/84'/1'/${ACCOUNT_INDEX}'`, - }, - ]); - }); - - it('creates discovered account', async () => { - const response = await snap.onKeyringRequest({ - origin: ORIGIN, - method: 'keyring_createAccount', - params: { + type: BtcAccountType.P2wpkh, + id: expect.anything(), + address: TEST_ADDRESS_REGTEST, options: { - derivationPath: `m/84'/1'/${ACCOUNT_INDEX}'`, - scope: BtcScope.Regtest, - synchronize: true, - }, - }, - }); - - expect(response).toRespondWith({ - type: BtcAccountType.P2wpkh, - id: expect.anything(), - address: TEST_ADDRESS_REGTEST, - options: { - entropySource: 'm', - entropy: { - type: 'mnemonic', - id: 'm', - groupIndex: ACCOUNT_INDEX, - derivationPath: `m/84'/1'/${ACCOUNT_INDEX}'`, + entropySource: 'm', + entropy: { + type: 'mnemonic', + id: 'm', + groupIndex: ACCOUNT_INDEX, + derivationPath: `m/84'/1'/${ACCOUNT_INDEX}'`, + }, + exportable: true, }, - exportable: false, + scopes: [BtcScope.Regtest], + methods: Object.values(AccountCapability), }, - scopes: [BtcScope.Regtest], - methods: Object.values(AccountCapability), - }); + ]); if ('result' in response.response) { - accounts[TEST_ADDRESS_REGTEST] = response.response - .result as KeyringAccount; + accounts[TEST_ADDRESS_REGTEST] = ( + response.response.result as KeyringAccount[] + )[0]!; await snap.onCronjob({ method: 'fullScanAccount', @@ -133,32 +113,41 @@ describe('Keyring', () => { ])( 'creates a P2WPKH account: %s', async ({ expectedAddress, ...requestOpts }) => { + const derivationPath = `m/${accountTypeToPurpose[requestOpts.addressType]}/${scopeToCoinType[requestOpts.scope]}/${requestOpts.index}'`; const response = await snap.onKeyringRequest({ origin: ORIGIN, - method: 'keyring_createAccount', - params: { options: { ...requestOpts, synchronize: false } }, + method: 'keyring_createAccounts', + params: { + type: 'bip44:derive-path', + entropySource: 'm', + derivationPath, + }, }); - expect(response).toRespondWith({ - type: requestOpts.addressType, - id: expect.anything(), - address: expectedAddress, - options: { - entropySource: 'm', - entropy: { - type: 'mnemonic', - id: 'm', - groupIndex: requestOpts.index, - derivationPath: `m/${accountTypeToPurpose[requestOpts.addressType]}/${scopeToCoinType[requestOpts.scope]}/${requestOpts.index}'`, + expect(response).toRespondWith([ + { + type: requestOpts.addressType, + id: expect.anything(), + address: expectedAddress, + options: { + entropySource: 'm', + entropy: { + type: 'mnemonic', + id: 'm', + groupIndex: requestOpts.index, + derivationPath, + }, + exportable: true, }, - exportable: false, + scopes: [requestOpts.scope], + methods: Object.values(AccountCapability), }, - scopes: [requestOpts.scope], - methods: Object.values(AccountCapability), - }); + ]); if ('result' in response.response) { - accounts[expectedAddress] = response.response.result as KeyringAccount; + accounts[expectedAddress] = ( + response.response.result as KeyringAccount[] + )[0] as KeyringAccount; } }, ); @@ -222,7 +211,7 @@ describe('Keyring', () => { groupIndex: requestOpts.index, derivationPath: `m/${accountTypeToPurpose[requestOpts.addressType]}/${scopeToCoinType[requestOpts.scope]}/${requestOpts.index}'`, }, - exportable: false, + exportable: true, }, scopes: [requestOpts.scope], methods: Object.values(AccountCapability), @@ -237,76 +226,32 @@ describe('Keyring', () => { // Account already exists so we should get the same account const response = await snap.onKeyringRequest({ origin: ORIGIN, - method: 'keyring_createAccount', + method: 'keyring_createAccounts', params: { - options: { - scope: BtcScope.Regtest, - addressType: BtcAccountType.P2wpkh, - derivationPath: "m/84'/1'/0'", - }, + type: 'bip44:derive-path', + entropySource: 'm', + derivationPath: "m/84'/1'/0'", }, }); - expect(response).toRespondWith(accounts[TEST_ADDRESS_REGTEST]); + expect(response).toRespondWith([accounts[TEST_ADDRESS_REGTEST]]); }); - it('returns the same account if already exists', async () => { + it('returns the same account if already exists by index', async () => { + // Create at same path again — idempotent const response = await snap.onKeyringRequest({ origin: ORIGIN, - method: 'keyring_createAccount', + method: 'keyring_createAccounts', params: { - options: { - scope: BtcScope.Regtest, - addressType: BtcAccountType.P2wpkh, - index: ACCOUNT_INDEX, - }, + type: 'bip44:derive-path', + entropySource: 'm', + derivationPath: `m/84'/1'/${ACCOUNT_INDEX}'`, }, }); - expect(response).toRespondWith(accounts[TEST_ADDRESS_REGTEST]); + expect(response).toRespondWith([accounts[TEST_ADDRESS_REGTEST]]); }); - it.each([ - { - addressType: BtcAccountType.P2pkh, - scope: BtcScope.Mainnet, - expectedError: 'Only native segwit (P2WPKH) addresses are supported', - }, - { - addressType: BtcAccountType.P2sh, - scope: BtcScope.Testnet, - expectedError: 'Only native segwit (P2WPKH) addresses are supported', - }, - { - addressType: BtcAccountType.P2tr, - scope: BtcScope.Mainnet, - expectedError: 'Only native segwit (P2WPKH) addresses are supported', - }, - ])( - 'rejects creation of non-P2WPKH account: $addressType', - async ({ addressType, scope, expectedError }) => { - const response = await snap.onKeyringRequest({ - origin: ORIGIN, - method: 'keyring_createAccount', - params: { - options: { - scope, - addressType, - index: 0, - synchronize: false, - }, - }, - }); - - expect(response.response).toMatchObject({ - error: { - code: -32000, - message: `Invalid format: ${expectedError}`, - }, - }); - }, - ); - it.each([ { derivationPath: "m/44'/0'/0'", // (P2PKH) @@ -328,67 +273,39 @@ describe('Keyring', () => { async ({ derivationPath, expectedError }) => { const response = await snap.onKeyringRequest({ origin: ORIGIN, - method: 'keyring_createAccount', + method: 'keyring_createAccounts', params: { - options: { - scope: BtcScope.Regtest, - derivationPath, - synchronize: false, - }, + type: 'bip44:derive-path', + entropySource: 'm', + derivationPath, }, }); expect(response.response).toMatchObject({ error: { - code: -32000, - message: `Invalid format: ${expectedError}`, + code: -32603, + message: expectedError, }, }); }, ); - it('rejects creation when addressType and derivationPath mismatch', async () => { - const response = await snap.onKeyringRequest({ - origin: ORIGIN, - method: 'keyring_createAccount', - params: { - options: { - scope: BtcScope.Regtest, - addressType: BtcAccountType.P2wpkh, // Native segwit - derivationPath: "m/44'/0'/0'", // Legacy path (P2PKH) - synchronize: false, - }, - }, - }); - - expect(response.response).toMatchObject({ - error: { - code: -32000, - message: - 'Invalid format: Only native segwit (BIP-84) derivation paths are supported', - }, - }); - }); - - it('accepts creation when addressType and derivationPath both indicate P2WPKH', async () => { + it('accepts creation with BIP-84 derivation path', async () => { const response = await snap.onKeyringRequest({ origin: ORIGIN, - method: 'keyring_createAccount', + method: 'keyring_createAccounts', params: { - options: { - scope: BtcScope.Regtest, - addressType: BtcAccountType.P2wpkh, - derivationPath: "m/84'/1'/10'", // Native segwit path matching P2WPKH - synchronize: false, - }, + type: 'bip44:derive-path', + entropySource: 'm', + derivationPath: "m/84'/1'/10'", }, }); expect(response.response).toHaveProperty('result'); - const account: KeyringAccount = ( - response.response as { result: KeyringAccount } - ).result; + const { result } = response.response as { result: KeyringAccount[] }; + expect(result).toHaveLength(1); + const account = result[0] as KeyringAccount; expect(account.address).toMatch(/^bcrt1/u); // Native segwit address expect((account.options.entropy as { groupIndex: number }).groupIndex).toBe( 10, @@ -419,7 +336,7 @@ describe('Keyring', () => { it('lists all accounts', async () => { const response = await snap.onKeyringRequest({ origin: ORIGIN, - method: 'keyring_listAccounts', + method: 'keyring_getAccounts', }); expect(response).toRespondWith(Object.values(accounts)); @@ -503,9 +420,8 @@ describe('Keyring', () => { }); expect(response).toRespondWithError({ - code: -32001, - message: `Resource not found: Account not found`, - data: { id, cause: null }, + code: -32603, + message: 'Account not found', stack: expect.anything(), }); }); diff --git a/packages/bitcoin-wallet-snap/package.json b/packages/bitcoin-wallet-snap/package.json index fcab28c1..b1ea8933 100644 --- a/packages/bitcoin-wallet-snap/package.json +++ b/packages/bitcoin-wallet-snap/package.json @@ -52,12 +52,13 @@ "@metamask/auto-changelog": "^6.1.1", "@metamask/bitcoindevkit": "^0.1.13", "@metamask/key-tree": "^10.1.1", - "@metamask/keyring-api": "^23.2.0", - "@metamask/keyring-snap-sdk": "^8.0.0", + "@metamask/keyring-api": "^23.7.0", + "@metamask/keyring-snap-sdk": "^9.2.1", "@metamask/slip44": "^4.2.0", - "@metamask/snaps-cli": "^8.3.0", + "@metamask/snaps-cli": "^8.4.1", "@metamask/snaps-jest": "^10.2.0", - "@metamask/snaps-sdk": "11.1.1", + "@metamask/snaps-sdk": "^11.2.0", + "@metamask/superstruct": "^3.4.1", "@metamask/utils": "^11.9.0", "bip322-js": "^3.0.0", "concurrently": "^10.0.3", diff --git a/packages/bitcoin-wallet-snap/scripts/populate-en-locale.js b/packages/bitcoin-wallet-snap/scripts/populate-en-locale.js index e3f1ee6c..69eb821b 100644 --- a/packages/bitcoin-wallet-snap/scripts/populate-en-locale.js +++ b/packages/bitcoin-wallet-snap/scripts/populate-en-locale.js @@ -17,7 +17,7 @@ const englishLocale = { try { writeFileSync( join(__dirname, '../locales/en.json'), - JSON.stringify(englishLocale, null, 2), + `${JSON.stringify(englishLocale, null, 2)}\n`, ); console.log('[populate-en-locale] - en locale populated'); } catch (error) { diff --git a/packages/bitcoin-wallet-snap/snap.manifest.json b/packages/bitcoin-wallet-snap/snap.manifest.json index 3d81b372..25170faa 100644 --- a/packages/bitcoin-wallet-snap/snap.manifest.json +++ b/packages/bitcoin-wallet-snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/internal-snaps.git" }, "source": { - "shasum": "hqW3Vzx9j/RwIxE8Jru0RBzz4sR0D9CicTmWWOwKB1Y=", + "shasum": "o8/OvZvhYYpLZ1tndeGYlWOlUKCqO1V+T6e6bL8YAg8=", "location": { "npm": { "filePath": "dist/bundle.js", @@ -31,7 +31,25 @@ }, "initialPermissions": { "endowment:webassembly": {}, - "endowment:keyring": {}, + "endowment:keyring": { + "allowedOrigins": [], + "capabilities": { + "scopes": ["bip122:000000000019d6689c085ae165831e93"], + "privateKey": { + "exportFormats": [ + { + "encoding": "base58" + } + ] + }, + "bip44": { + "deriveIndex": true, + "deriveIndexRange": true, + "derivePath": true, + "discover": true + } + } + }, "snap_getBip32Entropy": [ { "path": ["m", "44'", "0'"], diff --git a/packages/bitcoin-wallet-snap/src/handlers/KeyringHandler.test.ts b/packages/bitcoin-wallet-snap/src/handlers/KeyringHandler.test.ts index c9d2658f..a94d33f5 100644 --- a/packages/bitcoin-wallet-snap/src/handlers/KeyringHandler.test.ts +++ b/packages/bitcoin-wallet-snap/src/handlers/KeyringHandler.test.ts @@ -10,7 +10,6 @@ import type { } from '@metamask/bitcoindevkit'; import { Address } from '@metamask/bitcoindevkit'; import type { - DiscoveredAccount, KeyringAccount, KeyringResponse, Transaction as KeyringTransaction, @@ -26,26 +25,21 @@ import { mock } from 'jest-mock-extended'; import { assert } from 'superstruct'; import type { BitcoinAccount, Logger, SnapClient } from '../entities'; -import { - AccountCapability, - CurrencyUnit, - Purpose, - FormatError, -} from '../entities'; -import type { - AccountUseCases, - CreateAccountParams, -} from '../use-cases/AccountUseCases'; -import { scopeToNetwork, caipToAddressType, Caip19Asset } from './caip'; -import { KeyringHandler, CreateAccountRequest } from './KeyringHandler'; +import { AccountCapability, CurrencyUnit } from '../entities'; +import type { AccountUseCases } from '../use-cases/AccountUseCases'; +import { Caip19Asset } from './caip'; +import { KeyringHandler } from './KeyringHandler'; import type { KeyringRequestHandler } from './KeyringRequestHandler'; -import { mapToDiscoveredAccount } from './mappings'; jest.mock('superstruct', () => ({ ...jest.requireActual('superstruct'), assert: jest.fn(), })); +jest.mock('wif', () => ({ + encode: jest.fn(() => 'K1WIFprivateKeyMockValue'), +})); + // TODO: enable when this is merged: https://github.com/rustwasm/wasm-bindgen/issues/1818 /* eslint-disable @typescript-eslint/naming-convention */ jest.mock('@metamask/bitcoindevkit', () => { @@ -109,325 +103,7 @@ describe('KeyringHandler', () => { ); beforeEach(() => { - mockAccounts.create.mockResolvedValue(mockAccount); - }); - - describe('createAccount', () => { - const entropySource = 'some-source'; - const index = 1; - const correlationId = 'correlation-id'; - - // non-P2WPKH address types as we are not supporting them for v1 - // eslint-disable-next-line jest/no-disabled-tests - it.skip('respects provided params', async () => { - const options = { - scope: BtcScope.Signet, - entropySource, - index, - addressType: BtcAccountType.P2pkh, - metamask: { - correlationId, - }, - accountNameSuggestion: 'My account', - synchronize: false, - }; - const expectedCreateParams: CreateAccountParams = { - network: scopeToNetwork[BtcScope.Signet], - entropySource, - index, - addressType: 'p2pkh', - synchronize: false, - correlationId, - accountName: 'My account', - }; - - await handler.createAccount(options); - - expect(assert).toHaveBeenCalledWith(options, CreateAccountRequest); - expect(mockAccounts.create).toHaveBeenCalledWith(expectedCreateParams); - expect(mockAccounts.fullScan).not.toHaveBeenCalled(); - }); - - // only P2WPKH (BIP-84) derivation paths are now supported for v1 - // eslint-disable-next-line jest/no-disabled-tests - it.skip('extracts index from derivationPath', async () => { - const options = { - scope: BtcScope.Signet, - derivationPath: "m/44'/0'/5'/*/*", // change and address indexes can be anything - }; - const expectedCreateParams: CreateAccountParams = { - network: 'signet', - index: 5, - addressType: 'p2pkh', - entropySource: 'm', - synchronize: true, - }; - - await handler.createAccount(options); - expect(mockAccounts.create).toHaveBeenCalledWith(expectedCreateParams); - - // Test with a valid derivationPath without change and address index - await handler.createAccount({ - ...options, - derivationPath: "m/44'/0'/3'", - }); - expect(mockAccounts.create).toHaveBeenCalledWith({ - ...expectedCreateParams, - index: 3, - }); - }); - - it('auto increment index', async () => { - // We should get index 1 - mockAccounts.list.mockResolvedValue([ - mock({ - entropySource: 'entropy1', - accountIndex: 1, - addressType: 'p2wpkh', - network: 'signet', - }), - mock({ - entropySource: 'entropy2', - accountIndex: 2, - addressType: 'p2wpkh', - network: 'signet', - }), - mock({ - entropySource: 'entropy2', - accountIndex: 0, - addressType: 'p2wpkh', - network: 'signet', - }), - mock({ - entropySource: 'entropy2', - accountIndex: 3, - addressType: 'p2tr', - network: 'bitcoin', - }), - ]); - - const options = { - scope: BtcScope.Signet, - index: null, - entropySource: 'entropy2', - }; - const expectedCreateParams: CreateAccountParams = { - network: 'signet', - index: 1, - addressType: 'p2wpkh', - entropySource: 'entropy2', - synchronize: false, - }; - - await handler.createAccount(options); - - expect(mockAccounts.list).toHaveBeenCalled(); - expect(mockAccounts.create).toHaveBeenCalledWith(expectedCreateParams); - }); - - it.each([{ purpose: Purpose.NativeSegwit, addressType: 'p2wpkh' }] as { - purpose: Purpose; - addressType: AddressType; - }[])( - 'extracts P2WPKH address type from derivationPath: %s', - async ({ purpose, addressType }) => { - const options = { - scope: BtcScope.Signet, - derivationPath: `m/${purpose}'/0'/0'`, - }; - const expectedCreateParams: CreateAccountParams = { - network: 'signet', - index: 0, - addressType, - entropySource: 'm', - synchronize: false, - }; - - await handler.createAccount(options); - expect(mockAccounts.create).toHaveBeenCalledWith(expectedCreateParams); - }, - ); - - // skip non-P2WPKH address types as they are not supported on v1 - // eslint-disable-next-line jest/no-disabled-tests - it.skip.each([ - { purpose: Purpose.Legacy, addressType: 'p2pkh' }, - { purpose: Purpose.Segwit, addressType: 'p2sh' }, - { purpose: Purpose.Taproot, addressType: 'p2tr' }, - { purpose: Purpose.Multisig, addressType: 'p2wsh' }, - ] as { purpose: Purpose; addressType: AddressType }[])( - 'extracts address type from derivationPath: %s', - async ({ purpose, addressType }) => { - const options = { - scope: BtcScope.Signet, - derivationPath: `m/${purpose}'/0'/0'`, - }; - const expectedCreateParams: CreateAccountParams = { - network: 'signet', - index: 0, - addressType, - entropySource: 'm', - synchronize: true, - }; - - await handler.createAccount(options); - expect(mockAccounts.create).toHaveBeenCalledWith(expectedCreateParams); - }, - ); - - it('fails if derivationPath is invalid', async () => { - const options = { - scope: BtcScope.Signet, - derivationPath: "m/44'/0'/NaN'", - }; - - await expect(handler.createAccount(options)).rejects.toThrow( - 'Invalid account index: NaN', - ); - - await expect( - handler.createAccount({ ...options, derivationPath: "m/60'/0'/0'" }), // unknown purpose - ).rejects.toThrow('Invalid BIP-purpose: 60'); - - await expect( - handler.createAccount({ ...options, derivationPath: "m/44'/0'/-1'" }), // negative index - ).rejects.toThrow("Invalid account index: -1'"); - - await expect( - handler.createAccount({ ...options, derivationPath: "m/44'" }), // missing segments - ).rejects.toThrow("Invalid derivation path: m/44'"); - }); - - it('fails when addressType and derivationPath mismatch', async () => { - const options = { - scope: BtcScope.Signet, - addressType: BtcAccountType.P2wpkh, - derivationPath: "m/44'/0'/0'", // Legacy path (P2PKH) - }; - - // The error comes from #extractAddressType which validates the derivation path first - await expect(handler.createAccount(options)).rejects.toThrow( - new FormatError( - 'Only native segwit (BIP-84) derivation paths are supported', - ), - ); - }); - - it('succeeds when addressType and derivationPath both indicate P2WPKH', async () => { - const options = { - scope: BtcScope.Signet, - addressType: BtcAccountType.P2wpkh, - derivationPath: "m/84'/0'/5'", // Native segwit path - }; - const expectedCreateParams: CreateAccountParams = { - network: 'signet', - index: 5, - addressType: 'p2wpkh', - entropySource: 'm', - synchronize: false, - }; - - await handler.createAccount(options); - expect(mockAccounts.create).toHaveBeenCalledWith(expectedCreateParams); - }); - - it('propagates errors from createAccount', async () => { - const error = new Error('createAccount error'); - mockAccounts.create.mockRejectedValue(error); - - await expect( - handler.createAccount({ scopes: [BtcScope.Mainnet], index: 0 }), - ).rejects.toThrow(error); - expect(mockAccounts.create).toHaveBeenCalled(); - }); - - describe('tracing', () => { - const options = { - scope: BtcScope.Mainnet, - index: 0, - }; - - beforeEach(() => { - mockSnapClient.startTrace.mockResolvedValue(true); - mockSnapClient.endTrace.mockResolvedValue(undefined); - }); - - it('calls startTrace and endTrace with correct trace name', async () => { - await handler.createAccount(options); - - expect(mockSnapClient.startTrace).toHaveBeenCalledWith( - 'Create Bitcoin Account', - ); - expect(mockSnapClient.endTrace).toHaveBeenCalledWith( - 'Create Bitcoin Account', - ); - }); - - it('calls startTrace before creating account', async () => { - const callOrder: string[] = []; - mockSnapClient.startTrace.mockImplementation(async () => { - callOrder.push('startTrace'); - return true; - }); - mockAccounts.create.mockImplementation(async () => { - callOrder.push('createAccount'); - return mockAccount; - }); - - await handler.createAccount(options); - - expect(callOrder).toStrictEqual(['startTrace', 'createAccount']); - }); - - it('calls endTrace after creating account', async () => { - const callOrder: string[] = []; - mockAccounts.create.mockImplementation(async () => { - callOrder.push('createAccount'); - return mockAccount; - }); - mockSnapClient.endTrace.mockImplementation(async () => { - callOrder.push('endTrace'); - }); - - await handler.createAccount(options); - - expect(callOrder).toStrictEqual(['createAccount', 'endTrace']); - }); - - it('creates account even if startTrace fails', async () => { - mockSnapClient.startTrace.mockResolvedValue(false); - - const result = await handler.createAccount(options); - - expect(result).toBeDefined(); - expect(mockAccounts.create).toHaveBeenCalled(); - expect(mockSnapClient.startTrace).toHaveBeenCalled(); - }); - - it('calls endTrace when startTrace returns true', async () => { - mockSnapClient.startTrace.mockResolvedValue(true); - - await handler.createAccount(options); - - expect(mockSnapClient.startTrace).toHaveBeenCalledWith( - 'Create Bitcoin Account', - ); - expect(mockSnapClient.endTrace).toHaveBeenCalledWith( - 'Create Bitcoin Account', - ); - }); - - it('does not call endTrace when startTrace returns false', async () => { - mockSnapClient.startTrace.mockResolvedValue(false); - - await handler.createAccount(options); - - expect(mockSnapClient.startTrace).toHaveBeenCalledWith( - 'Create Bitcoin Account', - ); - expect(mockSnapClient.endTrace).not.toHaveBeenCalled(); - }); - }); + jest.resetAllMocks(); }); describe('createAccounts', () => { @@ -651,16 +327,96 @@ describe('KeyringHandler', () => { }); it('rejects unsupported creation types', async () => { + await expect( + handler.createAccounts({ + type: 'bip44:unknown' as AccountCreationType, + entropySource, + } as Parameters[0]), + ).rejects.toThrow(/not supported|unsupported/iu); + expect(mockAccounts.createMany).not.toHaveBeenCalled(); + }); + + it('creates an account for Bip44DerivePath on mainnet', async () => { + const bitcoinAccount = buildMockAccount(0); + mockAccounts.createMany.mockResolvedValue([bitcoinAccount]); + + const result = await handler.createAccounts({ + type: AccountCreationType.Bip44DerivePath, + derivationPath: "m/84'/0'/0'", + entropySource, + }); + + expect(mockAccounts.createMany).toHaveBeenCalledWith([ + expect.objectContaining({ network: 'bitcoin', index: 0 }), + ]); + expect(result).toHaveLength(1); + }); + + it('creates an account for Bip44DerivePath on regtest', async () => { + const bitcoinAccount = buildMockAccount(3); + mockAccounts.createMany.mockResolvedValue([bitcoinAccount]); + + const result = await handler.createAccounts({ + type: AccountCreationType.Bip44DerivePath, + derivationPath: "m/84'/1'/3'", + entropySource, + }); + + expect(mockAccounts.createMany).toHaveBeenCalledWith([ + expect.objectContaining({ network: 'regtest', index: 3 }), + ]); + expect(result).toHaveLength(1); + }); + + it('rejects Bip44DerivePath with non-BIP84 purpose', async () => { await expect( handler.createAccounts({ type: AccountCreationType.Bip44DerivePath, - derivationPath: "m/84'/0'/0'", + derivationPath: "m/44'/0'/0'", entropySource, }), - ).rejects.toThrow(/not supported|unsupported/iu); + ).rejects.toThrow(/Only native segwit \(BIP-84\)/iu); + expect(mockAccounts.createMany).not.toHaveBeenCalled(); + }); + + it('rejects Bip44DerivePath with unsupported coin type', async () => { + await expect( + handler.createAccounts({ + type: AccountCreationType.Bip44DerivePath, + derivationPath: "m/84'/2'/0'", + entropySource, + }), + ).rejects.toThrow(/Unsupported coin type/iu); expect(mockAccounts.createMany).not.toHaveBeenCalled(); }); + it.each([ + { + label: 'missing account segment', + derivationPath: "m/84'/0'" as `m/${string}`, + }, + { + label: 'non-integer index', + derivationPath: "m/84'/0'/abc'" as `m/${string}`, + }, + { + label: 'negative index', + derivationPath: "m/84'/0'/-1'" as `m/${string}`, + }, + ])( + 'rejects Bip44DerivePath with invalid account index ($label)', + async ({ derivationPath }) => { + await expect( + handler.createAccounts({ + type: AccountCreationType.Bip44DerivePath, + derivationPath, + entropySource, + }), + ).rejects.toThrow(/Invalid derivation path/iu); + expect(mockAccounts.createMany).not.toHaveBeenCalled(); + }, + ); + it('propagates errors from createMany', async () => { const error = new Error('create error'); mockAccounts.createMany.mockRejectedValue(error); @@ -674,6 +430,71 @@ describe('KeyringHandler', () => { ).rejects.toThrow(error); }); + describe('bip44:discover', () => { + const buildDiscoveredAccount = (hasTxs: boolean): BitcoinAccount => + mock({ + id: 'discovered-id', + addressType: 'p2wpkh', + network: 'bitcoin', + derivationPath: ['myEntropy', "84'", "0'", "0'"], + entropySource: 'myEntropy', + accountIndex: 0, + publicAddress: mockAddress, + capabilities: [ + AccountCapability.SignPsbt, + AccountCapability.ComputeFee, + ], + listTransactions: jest.fn().mockReturnValue(hasTxs ? [{}] : []), + }); + + it('discovers then returns the account when it has on-chain history', async () => { + mockAccounts.discover.mockResolvedValue(buildDiscoveredAccount(true)); + + const result = await handler.createAccounts({ + type: AccountCreationType.Bip44Discover, + groupIndex: 0, + entropySource, + }); + + expect(mockAccounts.discover).toHaveBeenCalledWith({ + network: 'bitcoin', + entropySource, + index: 0, + addressType: 'p2wpkh', + }); + expect(mockAccounts.delete).not.toHaveBeenCalled(); + expect(result).toHaveLength(1); + expect(result[0]?.id).toBe('discovered-id'); + }); + + it('returns empty array when no on-chain history', async () => { + mockAccounts.discover.mockResolvedValue(buildDiscoveredAccount(false)); + + const result = await handler.createAccounts({ + type: AccountCreationType.Bip44Discover, + groupIndex: 0, + entropySource, + }); + + expect(mockAccounts.discover).toHaveBeenCalled(); + expect(mockAccounts.delete).not.toHaveBeenCalled(); + expect(result).toHaveLength(0); + }); + + it('propagates errors from discover as SnapError', async () => { + const error = new Error('discover error'); + mockAccounts.discover.mockRejectedValue(error); + + await expect( + handler.createAccounts({ + type: AccountCreationType.Bip44Discover, + groupIndex: 0, + entropySource, + }), + ).rejects.toThrow('discover error'); + }); + }); + describe('tracing', () => { const options = { type: AccountCreationType.Bip44DeriveIndex as const, @@ -720,107 +541,122 @@ describe('KeyringHandler', () => { }); }); - describe('discoverAccounts', () => { - const entropySource = 'some-source'; - const groupIndex = 0; - const scopes = Object.values(BtcScope); - - it('creates, scans and returns accounts for every scope/addressType combination', async () => { - // only P2WPKH is now supported for v1 - const addressTypes = [BtcAccountType.P2wpkh]; - const totalCombinations = scopes.length * addressTypes.length; - - const expected: DiscoveredAccount[] = []; - scopes.forEach((scope) => { - addressTypes.forEach((addrType) => { - const acc = mock({ - addressType: caipToAddressType[addrType], - network: scopeToNetwork[scope], - listTransactions: jest.fn().mockReturnValue([{}]), // has history - derivationPath: ['m', "84'", "0'", "0'"], - }); - - expected.push(mapToDiscoveredAccount(acc)); - mockAccounts.discover.mockResolvedValueOnce(acc); - }); - }); + describe('exportAccount', () => { + const accountId = 'some-id'; + const fakeWif = 'K1WIFprivateKeyMockValue'; + const fakePrivateKey = + '0xdeadbeefcafe0000000000000000000000000000000000000000000000000001'; - const discovered = await handler.discoverAccounts( - scopes, - entropySource, - groupIndex, - ); + beforeEach(() => { + mockAccounts.get.mockResolvedValue(mockAccount); + mockSnapClient.getPrivateEntropy.mockResolvedValue({ + privateKey: fakePrivateKey, + } as never); + const { encode } = jest.requireMock<{ encode: jest.Mock }>('wif'); + encode.mockReturnValue(fakeWif); + }); - expect(mockAccounts.discover).toHaveBeenCalledTimes(totalCombinations); + it('exports account as WIF (base58) private key by default', async () => { + const result = await handler.exportAccount(accountId); - // validate each individual create() call arguments - scopes.forEach((scope, sIdx) => { - addressTypes.forEach((addrType, aIdx) => { - const callIdx = sIdx * addressTypes.length + aIdx; - expect(mockAccounts.discover).toHaveBeenNthCalledWith(callIdx + 1, { - network: scopeToNetwork[scope], - entropySource, - index: groupIndex, - addressType: caipToAddressType[addrType], - }); - }); + expect(mockAccounts.get).toHaveBeenCalledWith(accountId); + expect(mockSnapClient.getPrivateEntropy).toHaveBeenCalledWith( + mockAccount.derivationPath.concat(['0', '0']), + ); + expect(result).toStrictEqual({ + type: 'private-key', + encoding: 'base58', + privateKey: fakeWif, }); - - // Order is not guaranteed, so compare as sets - expect(discovered).toHaveLength(expected.length); - expect(discovered).toStrictEqual(expect.arrayContaining(expected)); }); - it('returns mix of accounts with and without history, filtering correctly', async () => { - // create mock accounts - some with history, some without - const accountWithHistory1 = mock({ - addressType: 'p2wpkh', - network: 'bitcoin', - listTransactions: jest.fn().mockReturnValue([{}, {}]), // has 2 transactions - derivationPath: ['m', "84'", "0'", "0'"], + it('exports account with explicit base58 encoding option', async () => { + const result = await handler.exportAccount(accountId, { + type: 'private-key', + encoding: 'base58', }); - const accountWithoutHistory = mock({ - addressType: 'p2wpkh', - network: 'testnet', - listTransactions: jest.fn().mockReturnValue([]), // no history - derivationPath: ['m', "84'", "1'", "0'"], - }); + expect(result.encoding).toBe('base58'); + expect(result.privateKey).toBe(fakeWif); + }); - const accountWithHistory2 = mock({ - addressType: 'p2wpkh', - network: 'signet', - listTransactions: jest.fn().mockReturnValue([{}]), // has 1 transaction - derivationPath: ['m', "84'", "1'", "0'"], + it('throws for unsupported encoding', async () => { + await expect( + handler.exportAccount(accountId, { + type: 'private-key', + encoding: 'hexadecimal', + }), + ).rejects.toThrow('Only base58 (WIF) private key export is supported'); + }); + + it('throws when private entropy is not available', async () => { + mockSnapClient.getPrivateEntropy.mockResolvedValue({ + privateKey: undefined, + } as never); + + await expect(handler.exportAccount(accountId)).rejects.toThrow( + 'Error exporting account', + ); + }); + + it('wraps wif encoding errors in SnapError without leaking private key', async () => { + const { encode } = jest.requireMock<{ encode: jest.Mock }>('wif'); + encode.mockImplementation(() => { + throw new Error('encoding failed: privatekey=SENSITIVE'); }); - mockAccounts.discover - .mockResolvedValueOnce(accountWithHistory1) - .mockResolvedValueOnce(accountWithoutHistory) - .mockResolvedValueOnce(accountWithHistory2); + const error = await handler + .exportAccount(accountId) + .catch((caughtError) => caughtError); + // The SnapError message must not contain the sensitive encoding error + expect(error.message).not.toContain('SENSITIVE'); + expect(error.message).toContain('exporting account'); + }); - const discovered = await handler.discoverAccounts( - [BtcScope.Mainnet, BtcScope.Testnet, BtcScope.Signet], - entropySource, - groupIndex, + it('throws when account type is not p2wpkh', async () => { + mockAccounts.get.mockResolvedValue( + mock({ ...mockAccount, addressType: 'p2pkh' }), ); - expect(mockAccounts.discover).toHaveBeenCalledTimes(3); - expect(discovered).toHaveLength(2); - expect(discovered).toStrictEqual([ - mapToDiscoveredAccount(accountWithHistory1), - mapToDiscoveredAccount(accountWithHistory2), - ]); + await expect(handler.exportAccount(accountId)).rejects.toThrow( + 'Only p2wpkh accounts are supported for private key export', + ); }); - it('propagates errors from discover', async () => { - const error = new Error('discover error'); - mockAccounts.discover.mockRejectedValue(error); + it('produces a valid WIF-encoded private key using the real encoder', async () => { + const { encode: realEncode } = jest.requireActual<{ + encode: (wif: { + version: number; + privateKey: Uint8Array; + compressed: boolean; + }) => string; + }>('wif'); + jest + .requireMock<{ encode: jest.Mock }>('wif') + .encode.mockImplementation(realEncode); + + // Known 32-byte secp256k1 private key (from Bitcoin wiki WIF test vector) + const privateKeyHex = + '0c28fca386c7a227600b2fe50b7cae11ec86d3bf1fbe471be89827e19d72aa1d'; + mockSnapClient.getPrivateEntropy.mockResolvedValue({ + privateKey: `0x${privateKeyHex}`, + } as never); + + const expectedWif = realEncode({ + version: 0x80, // mainnet — matches mockAccount.network === 'bitcoin' + privateKey: Buffer.from(privateKeyHex, 'hex'), + compressed: true, + }); - await expect( - handler.discoverAccounts(scopes, entropySource, groupIndex), - ).rejects.toThrow(error); - expect(mockAccounts.discover).toHaveBeenCalled(); + const result = await handler.exportAccount(accountId); + + expect(result).toStrictEqual({ + type: 'private-key', + encoding: 'base58', + privateKey: expectedWif, + }); + // Mainnet compressed WIF always starts with 'K' or 'L' + expect(result.privateKey).toMatch(/^[KL]/u); }); }); @@ -866,7 +702,7 @@ describe('KeyringHandler', () => { id: 'myEntropy', type: 'mnemonic', }, - exportable: false, + exportable: true, }, methods: mockAccount.capabilities, }; @@ -885,7 +721,7 @@ describe('KeyringHandler', () => { }); }); - describe('listAccounts', () => { + describe('getAccounts', () => { it('lists accounts', async () => { mockAccounts.list.mockResolvedValue([mockAccount]); const expectedKeyringAccounts = [ @@ -902,13 +738,13 @@ describe('KeyringHandler', () => { id: 'myEntropy', type: 'mnemonic', }, - exportable: false, + exportable: true, }, methods: mockAccount.capabilities, }, ]; - const result = await handler.listAccounts(); + const result = await handler.getAccounts(); expect(mockAccounts.list).toHaveBeenCalled(); expect(result).toStrictEqual(expectedKeyringAccounts); }); @@ -917,43 +753,11 @@ describe('KeyringHandler', () => { const error = new Error(); mockAccounts.list.mockRejectedValue(error); - await expect(handler.listAccounts()).rejects.toThrow(error); + await expect(handler.getAccounts()).rejects.toThrow(error); expect(mockAccounts.list).toHaveBeenCalled(); }); }); - describe('filterAccountChains', () => { - it('includes chain if account network corresponds', async () => { - mockAccounts.get.mockResolvedValue(mockAccount); - - const result = await handler.filterAccountChains('some-id', [ - BtcScope.Mainnet, - ]); - expect(mockAccounts.get).toHaveBeenCalledWith('some-id'); - expect(result).toStrictEqual([BtcScope.Mainnet]); - }); - - it('does not include chain if account network does not correspond', async () => { - mockAccounts.get.mockResolvedValue(mockAccount); - - const result = await handler.filterAccountChains('some-id', [ - BtcScope.Testnet, - ]); - expect(mockAccounts.get).toHaveBeenCalledWith('some-id'); - expect(result).toStrictEqual([]); - }); - - it('propagates errors from get', async () => { - const error = new Error(); - mockAccounts.get.mockRejectedValue(error); - - await expect( - handler.filterAccountChains('some-id', [BtcScope.Mainnet]), - ).rejects.toThrow(error); - expect(mockAccounts.get).toHaveBeenCalled(); - }); - }); - describe('deleteAccount', () => { it('deletes account', async () => { await handler.deleteAccount('some-id'); @@ -969,7 +773,7 @@ describe('KeyringHandler', () => { }); }); - describe('listAccountAssets', () => { + describe('getAccountAssets', () => { it.each([ { tNetwork: 'bitcoin', caip19: Caip19Asset.Bitcoin }, { tNetwork: 'testnet', caip19: Caip19Asset.Testnet }, @@ -983,7 +787,7 @@ describe('KeyringHandler', () => { network: tNetwork, } as unknown as BitcoinAccount); - const result = await handler.listAccountAssets('some-id'); + const result = await handler.getAccountAssets('some-id'); expect(mockAccounts.get).toHaveBeenCalledWith('some-id'); expect(result).toStrictEqual([caip19]); @@ -994,12 +798,12 @@ describe('KeyringHandler', () => { const error = new Error(); mockAccounts.get.mockRejectedValue(error); - await expect(handler.listAccountAssets('some-id')).rejects.toThrow(error); + await expect(handler.getAccountAssets('some-id')).rejects.toThrow(error); expect(mockAccounts.get).toHaveBeenCalled(); }); }); - describe('listAccountTransactions', () => { + describe('getAccountTransactions', () => { const pagination = { limit: 10, next: null }; const mockAmount = mock({ @@ -1082,7 +886,7 @@ describe('KeyringHandler', () => { it('lists transactions successfully: send', async () => { const id = 'some-id'; - const result = await handler.listAccountTransactions(id, pagination); + const result = await handler.getAccountTransactions(id, pagination); expect(mockAccounts.get).toHaveBeenCalledWith(id); expect(result.data).toStrictEqual([expectedResult]); @@ -1092,7 +896,7 @@ describe('KeyringHandler', () => { const id = 'some-id'; mockAccount.isMine.mockReturnValueOnce(true); - const result = await handler.listAccountTransactions(id, pagination); + const result = await handler.getAccountTransactions(id, pagination); expect(mockAccounts.get).toHaveBeenCalledWith(id); expect(result.data).toStrictEqual([{ ...expectedResult, to: [] }]); @@ -1107,7 +911,7 @@ describe('KeyringHandler', () => { ]); mockAccount.isMine.mockReturnValueOnce(true); - const result = await handler.listAccountTransactions(id, pagination); + const result = await handler.getAccountTransactions(id, pagination); expect(mockAccounts.get).toHaveBeenCalledWith(id); expect(result.data).toStrictEqual([ @@ -1126,7 +930,7 @@ describe('KeyringHandler', () => { mockAccount.listTransactions.mockReturnValue(mockTransactions); - const result = await handler.listAccountTransactions(id, pagination); + const result = await handler.getAccountTransactions(id, pagination); expect(result.data).toHaveLength(pagination.limit); expect(result.next).toBe('txid-9'); @@ -1143,7 +947,7 @@ describe('KeyringHandler', () => { mockAccount.listTransactions.mockReturnValue(mockTransactions); - const result = await handler.listAccountTransactions(id, { + const result = await handler.getAccountTransactions(id, { ...pagination, next: 'txid-9', }); @@ -1155,14 +959,6 @@ describe('KeyringHandler', () => { }); }); - describe('updateAccount', () => { - it('throws not supported', async () => { - await expect(handler.updateAccount()).rejects.toThrow( - 'Method not supported.', - ); - }); - }); - describe('submitRequest', () => { it('calls KeyringRequestHandler', async () => { const mockRequest = mock(); @@ -1204,7 +1000,7 @@ describe('KeyringHandler', () => { }; jest - .spyOn(handler, 'listAccounts') + .spyOn(handler, 'getAccounts') .mockResolvedValueOnce([mockKeyringAccount1, mockKeyringAccount2]); const result = await handler.resolveAccountAddress( @@ -1212,7 +1008,7 @@ describe('KeyringHandler', () => { request, ); - expect(handler.listAccounts).toHaveBeenCalled(); + expect(handler.getAccounts).toHaveBeenCalled(); expect(result).toStrictEqual({ address: `${BtcScope.Regtest}:test123`, }); @@ -1230,7 +1026,7 @@ describe('KeyringHandler', () => { }; jest - .spyOn(handler, 'listAccounts') + .spyOn(handler, 'getAccounts') .mockResolvedValue([mockKeyringAccount1, mockKeyringAccount2]); const result = await handler.resolveAccountAddress( @@ -1259,7 +1055,7 @@ describe('KeyringHandler', () => { }); jest - .spyOn(handler, 'listAccounts') + .spyOn(handler, 'getAccounts') .mockResolvedValue([accountWithDifferentScope]); const result = await handler.resolveAccountAddress( @@ -1304,7 +1100,7 @@ describe('KeyringHandler', () => { }; jest - .spyOn(handler, 'listAccounts') + .spyOn(handler, 'getAccounts') .mockResolvedValue([mockKeyringAccount1, mockKeyringAccount2]); // First assert (scope) passes, second assert (request struct) throws diff --git a/packages/bitcoin-wallet-snap/src/handlers/KeyringHandler.ts b/packages/bitcoin-wallet-snap/src/handlers/KeyringHandler.ts index 1f64694a..903427ef 100644 --- a/packages/bitcoin-wallet-snap/src/handlers/KeyringHandler.ts +++ b/packages/bitcoin-wallet-snap/src/handlers/KeyringHandler.ts @@ -3,95 +3,61 @@ import { Amount } from '@metamask/bitcoindevkit'; import { AccountCreationType, assertCreateAccountOptionIsSupported, - BtcAccountType, - BtcScope, - CreateAccountRequestStruct, - CreateAccountsRequestStruct, - DeleteAccountRequestStruct, - DiscoverAccountsRequestStruct, - FilterAccountChainsStruct, - GetAccountBalancesRequestStruct, - GetAccountRequestStruct, - KeyringRpcMethod, - ListAccountAssetsRequestStruct, - ListAccountsRequestStruct, - ListAccountTransactionsRequestStruct, - MetaMaskOptionsStruct, - ResolveAccountAddressRequestStruct, - SetSelectedAccountsRequestStruct, - SubmitRequestRequestStruct, } from '@metamask/keyring-api'; import type { - CreateAccountOptions, - Keyring, - KeyringAccount, - KeyringResponse, Balance, + BtcScope, CaipAssetType, CaipAssetTypeOrId, + CreateAccountOptions, + KeyringAccount, + KeyringRequest, + KeyringResponse, Paginated, - Transaction, Pagination, - MetaMaskOptions, - DiscoveredAccount, - KeyringRequest, ResolvedAccountAddress, + Transaction, } from '@metamask/keyring-api'; -import type { CaipChainId, Json, JsonRpcRequest } from '@metamask/snaps-sdk'; -import { - assert, - boolean, - enums, - number, - object, - optional, - string, -} from 'superstruct'; - +import type { + ExportAccountOptions, + ExportedAccount, + KeyringSnapRpc, +} from '@metamask/keyring-api/v2'; +import { SnapError } from '@metamask/snaps-sdk'; +import type { CaipChainId, JsonRpcRequest } from '@metamask/snaps-sdk'; +import { sensitive } from '@metamask/superstruct'; +import { assert, is, string } from 'superstruct'; +import { encode } from 'wif'; + +import snapManifest from '../../snap.manifest.json'; +import type { Logger, SnapClient } from '../entities'; import { computeDisplayBalanceSats, FormatError, - InexistentMethodError, networkToCurrencyUnit, - Purpose, - purposeToAddressType, } from '../entities'; -import type { BitcoinAccount, Logger, SnapClient } from '../entities'; import type { AccountUseCases, CreateAccountParams, } from '../use-cases/AccountUseCases'; -import { - networkToCaip19, - caipToAddressType, - scopeToNetwork, - networkToScope, - NetworkStruct, -} from './caip'; +import { NetworkStruct, networkToCaip19, scopeToNetwork } from './caip'; import { CronMethod } from './CronHandler'; import type { KeyringRequestHandler } from './KeyringRequestHandler'; -import { - mapToDiscoveredAccount, - mapToKeyringAccount, - mapToTransaction, -} from './mappings'; +import { mapToKeyringAccount, mapToTransaction } from './mappings'; +import { parseDerivationPath } from './parsers'; import { BtcWalletRequestStruct, validateSelectedAccounts } from './validation'; -export const CreateAccountRequest = object({ - scope: enums(Object.values(BtcScope)), - addressType: optional(enums(Object.values(BtcAccountType))), - entropySource: optional(string()), - accountNameSuggestion: optional(string()), - synchronize: optional(boolean()), - index: optional(number()), - derivationPath: optional(string()), - ...MetaMaskOptionsStruct.schema, -}); - /** Maximum number of accounts to create in one internal createMany call. */ const MAX_CREATE_ACCOUNTS_PER_BATCH = 100; -export class KeyringHandler implements Keyring { +/** + * Scopes declared in the snap manifest's keyring capabilities block. + * Used to determine which networks are supported for account discovery. + */ +const SUPPORTED_SCOPES = snapManifest.initialPermissions['endowment:keyring'] + .capabilities.scopes as readonly BtcScope[]; + +export class KeyringHandler implements KeyringSnapRpc { readonly #accountsUseCases: AccountUseCases; readonly #keyringRequest: KeyringRequestHandler; @@ -116,85 +82,7 @@ export class KeyringHandler implements Keyring { this.#logger = logger; } - async route(request: JsonRpcRequest): Promise { - switch (request.method) { - case `${KeyringRpcMethod.ListAccounts}`: { - assert(request, ListAccountsRequestStruct); - return this.listAccounts(); - } - case `${KeyringRpcMethod.GetAccount}`: { - assert(request, GetAccountRequestStruct); - return this.getAccount(request.params.id); - } - case `${KeyringRpcMethod.CreateAccount}`: { - assert(request, CreateAccountRequestStruct); - return this.createAccount(request.params.options); - } - case `${KeyringRpcMethod.CreateAccounts}`: { - assert(request, CreateAccountsRequestStruct); - return this.createAccounts(request.params.options); - } - case `${KeyringRpcMethod.DiscoverAccounts}`: { - assert(request, DiscoverAccountsRequestStruct); - return this.discoverAccounts( - request.params.scopes as BtcScope[], - request.params.entropySource, - request.params.groupIndex, - ); - } - case `${KeyringRpcMethod.ListAccountTransactions}`: { - assert(request, ListAccountTransactionsRequestStruct); - return this.listAccountTransactions( - request.params.id, - request.params.pagination, - ); - } - case `${KeyringRpcMethod.ListAccountAssets}`: { - assert(request, ListAccountAssetsRequestStruct); - return this.listAccountAssets(request.params.id); - } - case `${KeyringRpcMethod.GetAccountBalances}`: { - assert(request, GetAccountBalancesRequestStruct); - return this.getAccountBalances(request.params.id); - } - case `${KeyringRpcMethod.FilterAccountChains}`: { - assert(request, FilterAccountChainsStruct); - return this.filterAccountChains( - request.params.id, - request.params.chains, - ); - } - case `${KeyringRpcMethod.DeleteAccount}`: { - assert(request, DeleteAccountRequestStruct); - await this.deleteAccount(request.params.id); - return null; - } - case `${KeyringRpcMethod.SubmitRequest}`: { - assert(request, SubmitRequestRequestStruct); - return this.submitRequest(request.params); - } - case `${KeyringRpcMethod.SetSelectedAccounts}`: { - assert(request, SetSelectedAccountsRequestStruct); - await this.setSelectedAccounts(request.params.accounts); - return null; - } - case `${KeyringRpcMethod.ResolveAccountAddress}`: { - assert(request, ResolveAccountAddressRequestStruct); - return this.resolveAccountAddress( - request.params.scope, - request.params.request, - ); - } - - default: { - throw new InexistentMethodError('Keyring method not supported', { - method: request.method, - }); - } - } - } - - async listAccounts(): Promise { + async getAccounts(): Promise { const accounts = await this.#accountsUseCases.list(); return accounts.map(mapToKeyringAccount); } @@ -204,126 +92,18 @@ export class KeyringHandler implements Keyring { return mapToKeyringAccount(account); } - async createAccount( - options: Record & MetaMaskOptions, - ): Promise { - assert(options, CreateAccountRequest); - - const traceName = 'Create Bitcoin Account'; - const traceStarted = await this.#snapClient.startTrace(traceName); - - try { - const { - metamask, - scope, - entropySource = 'm', - index, - derivationPath, - addressType, - synchronize = false, - accountNameSuggestion, - } = options; - - let resolvedIndex = derivationPath - ? this.#extractAccountIndex(derivationPath) - : index; - - let resolvedAddressType: AddressType; - if (addressType) { - // only support P2WPKH addresses for v1 - if (addressType !== BtcAccountType.P2wpkh) { - throw new FormatError( - 'Only native segwit (P2WPKH) addresses are supported', - ); - } - resolvedAddressType = caipToAddressType[addressType]; - - // if both addressType and derivationPath are provided, validate they match - if (derivationPath) { - const pathAddressType = this.#extractAddressType(derivationPath); - if (pathAddressType !== resolvedAddressType) { - throw new FormatError('Address type and derivation path mismatch'); - } - } - } else if (derivationPath) { - resolvedAddressType = this.#extractAddressType(derivationPath); - } else { - resolvedAddressType = this.#defaultAddressType; - // validate default address type is P2WPKH just to be sure - if (resolvedAddressType !== 'p2wpkh') { - throw new FormatError( - 'Only native segwit (P2WPKH) addresses are supported', - ); - } - } - - // FIXME: This if should be removed ASAP as the index should always be defined or be 0 - // The Snap automatically increasing the index per request creates significant issues - // such as: concurrency, lack of idempotency, dangling state (if MM crashes before saving the account), etc. - if (resolvedIndex === undefined || resolvedIndex === null) { - const accounts = (await this.#accountsUseCases.list()).filter( - (acc) => - acc.entropySource === entropySource && - acc.network === scopeToNetwork[scope] && - acc.addressType === resolvedAddressType, - ); - - resolvedIndex = this.#getLowestUnusedIndex(accounts); - } - - const account = await this.#accountsUseCases.create({ - network: scopeToNetwork[scope], - entropySource, - index: resolvedIndex, - addressType: resolvedAddressType, - correlationId: metamask?.correlationId, - synchronize, - accountName: accountNameSuggestion, - }); - - return mapToKeyringAccount(account); - } finally { - if (traceStarted) { - await this.#snapClient.endTrace(traceName); - } - } - } - async createAccounts( options: CreateAccountOptions, ): Promise { assertCreateAccountOptionIsSupported(options, [ + `${AccountCreationType.Bip44DerivePath}`, `${AccountCreationType.Bip44DeriveIndex}`, `${AccountCreationType.Bip44DeriveIndexRange}`, + `${AccountCreationType.Bip44Discover}`, ]); const { entropySource } = options; - const range = - options.type === AccountCreationType.Bip44DeriveIndex - ? { from: options.groupIndex, to: options.groupIndex } - : options.range; - - if ( - !Number.isSafeInteger(range.from) || - !Number.isSafeInteger(range.to) || - range.from < 0 || - range.to < 0 - ) { - throw new FormatError( - 'Account index range is invalid: from and to must be non-negative integers', - ); - } - - if (range.from > range.to) { - throw new FormatError( - 'Account index range is invalid: from must be less than or equal to to', - ); - } - - // Only P2WPKH (BIP-84) on bitcoin mainnet is supported for v1, mirroring - // the defaults used by `createAccount` when no scope is provided. - const network = scopeToNetwork[BtcScope.Mainnet]; const addressType = this.#defaultAddressType; if (addressType !== 'p2wpkh') { throw new FormatError( @@ -335,39 +115,107 @@ export class KeyringHandler implements Keyring { const traceStarted = await this.#snapClient.startTrace(traceName); try { - // `AccountUseCases.createMany` is idempotent: if an account already exists - // for the resolved derivation path, it will be returned as-is. - const accounts: BitcoinAccount[] = []; - let chunkFrom = range.from; - - while (chunkFrom <= range.to) { - const chunkTo = Math.min( - chunkFrom + MAX_CREATE_ACCOUNTS_PER_BATCH - 1, - range.to, - ); - const chunkRequests: CreateAccountParams[] = []; + if (options.type === AccountCreationType.Bip44DerivePath) { + const { index, network } = parseDerivationPath(options.derivationPath); - for (let index = chunkFrom; index <= chunkTo; index += 1) { - chunkRequests.push({ + const created = await this.#accountsUseCases.createMany([ + { network, entropySource, index, addressType, synchronize: false, + }, + ]); + + return created.map(mapToKeyringAccount); + } + + if (options.type === AccountCreationType.Bip44Discover) { + // For each supported scope, discover at this index. Accounts with + // on-chain activity are kept; those without are discarded in-memory. + // Returning [] signals end-of-discovery to the client. + const discovered: KeyringAccount[] = []; + + for (const scope of SUPPORTED_SCOPES) { + const network = scopeToNetwork[scope]; + const account = await this.#accountsUseCases.discover({ + network, + entropySource, + index: options.groupIndex, + addressType, }); + + if (account.listTransactions().length > 0) { + discovered.push(mapToKeyringAccount(account)); + } } - accounts.push( - ...(await this.#accountsUseCases.createMany(chunkRequests)), + return discovered; + } + + // Build the index range. For Bip44DeriveIndex the range is a single + // element; for Bip44DeriveIndexRange it comes from the options directly. + // At this point the discover branch has already returned, so this + // discriminant is exhaustive. + const range = + options.type === AccountCreationType.Bip44DeriveIndex + ? { from: options.groupIndex, to: options.groupIndex } + : options.range; + + if ( + !Number.isSafeInteger(range.from) || + !Number.isSafeInteger(range.to) || + range.from < 0 || + range.to < 0 + ) { + throw new FormatError( + 'Account index range is invalid: from and to must be non-negative integers', ); + } + + if (range.from > range.to) { + throw new FormatError( + 'Account index range is invalid: from must be less than or equal to to', + ); + } + + // `AccountUseCases.createMany` is idempotent: if an account already + // exists for the resolved derivation path, it will be returned as-is. + const created: KeyringAccount[] = []; + + for (const scope of SUPPORTED_SCOPES) { + const network = scopeToNetwork[scope]; + let chunkFrom = range.from; + + while (chunkFrom <= range.to) { + const chunkTo = Math.min( + chunkFrom + MAX_CREATE_ACCOUNTS_PER_BATCH - 1, + range.to, + ); + const chunkRequests: CreateAccountParams[] = []; + + for (let index = chunkFrom; index <= chunkTo; index += 1) { + chunkRequests.push({ + network, + entropySource, + index, + addressType, + synchronize: false, + }); + } + + const chunk = await this.#accountsUseCases.createMany(chunkRequests); + created.push(...chunk.map(mapToKeyringAccount)); - if (chunkTo === range.to) { - break; + chunkFrom = chunkTo + 1; } - chunkFrom = chunkTo + 1; } - return accounts.map(mapToKeyringAccount); + return created; + } catch (error: unknown) { + this.#logger.error({ error }, 'Error creating accounts batch'); + throw new SnapError(error as Error); } finally { if (traceStarted) { await this.#snapClient.endTrace(traceName); @@ -375,29 +223,65 @@ export class KeyringHandler implements Keyring { } } - async discoverAccounts( - scopes: BtcScope[], - entropySource: string, - groupIndex: number, - ): Promise { - const accounts = await Promise.all( - scopes.flatMap((scope) => - // only discover P2WPKH addresses - [BtcAccountType.P2wpkh].map(async (addressType) => - this.#accountsUseCases.discover({ - network: scopeToNetwork[scope], - entropySource, - index: groupIndex, - addressType: caipToAddressType[addressType], - }), - ), - ), - ); + async exportAccount( + accountId: string, + options?: ExportAccountOptions, + ): Promise { + // TODO: update to `"wif"` once the accounts repo adds WIF as a named + // encoding. WIF is Base58Check (not plain base58), so we treat the current + // `"base58"` wire value as a WIF request in the meantime. + const encoding = options?.encoding ?? 'base58'; + if (encoding !== 'base58') { + throw new Error( + `Only base58 (WIF) private key export is supported, got: ${encoding}`, + ); + } - // Return only accounts with history. - return accounts - .filter((account) => account.listTransactions().length > 0) - .map(mapToDiscoveredAccount); + const account = await this.#accountsUseCases.get(accountId); + + const addressType = account.addressType ?? this.#defaultAddressType; + if (addressType !== this.#defaultAddressType) { + throw new SnapError( + `Only ${this.#defaultAddressType} accounts are supported for private key export`, + ); + } + + try { + const entropy = await this.#snapClient.getPrivateEntropy( + // Export the private key for address index 0 (the primary address). + account.derivationPath.concat(['0', '0']), + ); + + if (!entropy.privateKey) { + throw new Error('Failed to get private entropy'); + } + const { privateKey } = entropy; + // Private key is returned in "0x..." format; transform to WIF (Base58Check). + const wifPrivateKey = encode({ + version: account.network === 'bitcoin' ? 0x80 : 0xef, // 0x80 mainnet, 0xef testnets + // eslint-disable-next-line no-restricted-globals + privateKey: Buffer.from(privateKey.slice(2), 'hex'), + compressed: true, + }); + + // SECURITY: use is() not assert() to avoid embedding the private key in a + // StructError message if encoding validation fails. + // TODO: replace string() with a WIF-specific struct once the accounts + // repo exports one. + if (!is(wifPrivateKey, sensitive(string()))) { + throw new Error('Derived private key failed encoding validation'); + } + + return { + type: 'private-key', + encoding, + privateKey: wifPrivateKey, + }; + } catch { + const errorMsg = 'Error exporting account'; + this.#logger.error(errorMsg); + throw new SnapError(errorMsg); + } } async getAccountBalances( @@ -416,33 +300,22 @@ export class KeyringHandler implements Keyring { }; } - async filterAccountChains(id: string, chains: string[]): Promise { - const account = await this.#accountsUseCases.get(id); - const accountChain = networkToScope[account.network]; - return chains.includes(accountChain) ? [accountChain] : []; - } - - async updateAccount(): Promise { - throw new InexistentMethodError('Method not supported.'); - } - async deleteAccount(id: string): Promise { await this.#accountsUseCases.delete(id); } - async listAccountAssets(id: string): Promise { + async getAccountAssets(id: string): Promise { const account = await this.#accountsUseCases.get(id); return [networkToCaip19[account.network]]; } - async listAccountTransactions( + async getAccountTransactions( id: string, { limit, next }: Pagination, ): Promise> { const account = await this.#accountsUseCases.get(id); const transactions = account.listTransactions(); - // Find starting index based on provided cursor let startIndex = 0; if (next) { const cursorIndex = transactions.findIndex( @@ -455,8 +328,7 @@ export class KeyringHandler implements Keyring { const hasMore = startIndex + limit < transactions.length; const nextCursor = hasMore && paginatedTxs.length > 0 - ? // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - paginatedTxs[paginatedTxs.length - 1]!.txid.toString() + ? (paginatedTxs[paginatedTxs.length - 1]?.txid.toString() ?? null) : null; return { @@ -478,7 +350,6 @@ export class KeyringHandler implements Keyring { allAccounts.map((acc) => acc.id), ); - // Schedule immediate background job to perform full scan await this.#snapClient.scheduleBackgroundEvent({ duration: 'PT1S', method: CronMethod.SyncSelectedAccounts, @@ -495,7 +366,7 @@ export class KeyringHandler implements Keyring { * @param scope - Request's scope (CAIP-2). * @param request - Signing request object. * @returns A Promise that resolves to the account address that must - * be used to process this signing request, or null if none candidates + * be used to process this signing request, or null if no candidates * could be found. */ async resolveAccountAddress( @@ -509,7 +380,7 @@ export class KeyringHandler implements Keyring { const requestWithoutCommonHeader = { method, params }; assert(requestWithoutCommonHeader, BtcWalletRequestStruct); - const allAccounts = await this.listAccounts(); + const allAccounts = await this.getAccounts(); const accountsWithThisScope = allAccounts.filter((account) => account.scopes.includes(scope), @@ -538,85 +409,4 @@ export class KeyringHandler implements Keyring { return null; } } - - #extractAddressType(path: string): AddressType { - const segments = path.split('/'); - if (segments.length < 4) { - throw new FormatError(`Invalid derivation path: ${path}`); - } - - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const purposePart = segments[1]!; - const match = purposePart.match(/^(\d+)/u); - if (!match) { - throw new FormatError(`Invalid purpose segment: ${purposePart}`); - } - - const purpose = Number(match[1]); - if (!Object.values(Purpose).includes(purpose)) { - throw new FormatError(`Invalid BIP-purpose: ${purpose}`); - } - - // only support native segwit (BIP-84) derivation paths for now - if ((purpose as Purpose) !== Purpose.NativeSegwit) { - throw new FormatError( - `Only native segwit (BIP-84) derivation paths are supported`, - ); - } - - const addressType = purposeToAddressType[purpose as Purpose]; - if (!addressType) { - throw new FormatError(`No address-type mapping for purpose: ${purpose}`); - } - - return addressType; - } - - #extractAccountIndex(path: string): number { - const segments = path.split('/'); - if (segments.length < 4) { - throw new FormatError(`Invalid derivation path: ${path}`); - } - - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const accountPart = segments[3]!; - const match = accountPart.match(/^(\d+)/u); - if (!match) { - throw new FormatError(`Invalid account index: ${accountPart}`); - } - - const index = Number(match[1]); - if (!Number.isInteger(index) || index < 0) { - throw new FormatError( - `Account index must be a non-negative integer, got: ${index}`, - ); - } - - return index; - } - - #getLowestUnusedIndex(accounts: BitcoinAccount[]): number { - if (accounts.length === 0) { - return 0; - } - - const usedIndices = accounts - .map((acc) => acc.accountIndex) - .sort((idxA, idxB) => idxA - idxB); - - let lowestUnusedIndex = 0; - - for (const usedIndex of usedIndices) { - /** - * From lower to higher, the moment we find a gap, we can use it - */ - if (usedIndex !== lowestUnusedIndex) { - break; - } - - lowestUnusedIndex += 1; - } - - return lowestUnusedIndex; - } } diff --git a/packages/bitcoin-wallet-snap/src/handlers/mappings.ts b/packages/bitcoin-wallet-snap/src/handlers/mappings.ts index 6c8131a8..e454750d 100644 --- a/packages/bitcoin-wallet-snap/src/handlers/mappings.ts +++ b/packages/bitcoin-wallet-snap/src/handlers/mappings.ts @@ -73,7 +73,7 @@ export function mapToKeyringAccount(account: BitcoinAccount): KeyringAccount { address: account.publicAddress.toString(), options: { entropySource: account.entropySource, // TODO: Legacy field. To be removed once multichain accounts are out. - exportable: false, + exportable: true, entropy: { type: 'mnemonic', id: account.entropySource, diff --git a/packages/bitcoin-wallet-snap/src/handlers/parsers.test.ts b/packages/bitcoin-wallet-snap/src/handlers/parsers.test.ts new file mode 100644 index 00000000..069684da --- /dev/null +++ b/packages/bitcoin-wallet-snap/src/handlers/parsers.test.ts @@ -0,0 +1,53 @@ +import { FormatError } from '../entities'; +import { parseDerivationPath } from './parsers'; + +jest.mock('@metamask/bitcoindevkit', () => ({ + Psbt: { from_string: jest.fn() }, +})); + +describe('parseDerivationPath', () => { + describe('valid paths', () => { + it.each([ + { + path: "m/84'/0'/0'", + expected: { index: 0, network: 'bitcoin' }, + }, + { + path: "m/84'/0'/5'", + expected: { index: 5, network: 'bitcoin' }, + }, + { + path: "m/84'/1'/0'", + expected: { index: 0, network: 'regtest' }, + }, + { + path: "m/84'/1'/99'", + expected: { index: 99, network: 'regtest' }, + }, + ])('parses $path correctly', ({ path, expected }) => { + expect(parseDerivationPath(path)).toStrictEqual(expected); + }); + }); + + describe('invalid paths', () => { + it('throws for a path with fewer than 4 segments', () => { + expect(() => parseDerivationPath("m/84'/0'")).toThrow(FormatError); + }); + + it('throws for a non-BIP-84 purpose', () => { + expect(() => parseDerivationPath("m/44'/0'/0'")).toThrow(FormatError); + }); + + it('throws for an unsupported coin type', () => { + expect(() => parseDerivationPath("m/84'/60'/0'")).toThrow(FormatError); + }); + + it('throws for a non-numeric account index', () => { + expect(() => parseDerivationPath("m/84'/0'/abc'")).toThrow(FormatError); + }); + + it('throws for a negative account index', () => { + expect(() => parseDerivationPath("m/84'/0'/-1'")).toThrow(FormatError); + }); + }); +}); diff --git a/packages/bitcoin-wallet-snap/src/handlers/parsers.ts b/packages/bitcoin-wallet-snap/src/handlers/parsers.ts index 3b96a918..b313f378 100644 --- a/packages/bitcoin-wallet-snap/src/handlers/parsers.ts +++ b/packages/bitcoin-wallet-snap/src/handlers/parsers.ts @@ -1,7 +1,57 @@ +import type { Network } from '@metamask/bitcoindevkit'; import { Psbt } from '@metamask/bitcoindevkit'; import { FormatError } from '../entities'; +type ParsedDerivationPath = { + index: number; + network: Network; +}; + +/** + * Parses and validates a BIP-44 derivation path string into its components. + * + * Only BIP-84 (native segwit) paths with coin type 0 (mainnet) or 1 (regtest) + * are accepted. + * + * @param path - A derivation path string, e.g. `m/84'/0'/2'` + * @returns The account index and network derived from the path. + * @throws {FormatError} If the path is malformed or uses an unsupported purpose or coin type. + */ +export function parseDerivationPath(path: string): ParsedDerivationPath { + const parts = path.split('/'); + if (parts.length < 4) { + throw new FormatError( + 'Invalid derivation path: expected at least 4 segments (m/purpose/coinType/accountIndex)', + ); + } + + const purpose = parts[1]?.replaceAll("'", ''); + const coinType = parts[2]?.replaceAll("'", ''); + const accountSegment = parts[3]?.replaceAll("'", ''); + + if (purpose !== '84') { + throw new FormatError( + 'Only native segwit (BIP-84) derivation paths are supported', + ); + } + + if (coinType !== '0' && coinType !== '1') { + throw new FormatError( + 'Unsupported coin type: only coin type 0 (mainnet) and 1 (regtest) are supported', + ); + } + + const index = parseInt(accountSegment ?? '', 10); + if (!Number.isInteger(index) || index < 0) { + throw new FormatError( + 'Invalid derivation path: account index must be a non-negative integer', + ); + } + + return { index, network: coinType === '0' ? 'bitcoin' : 'regtest' }; +} + /** * Parses a PSBT encoded as a base64 string into a PSBT object. * diff --git a/packages/bitcoin-wallet-snap/src/index.ts b/packages/bitcoin-wallet-snap/src/index.ts index 7751ca0f..3f2d1542 100644 --- a/packages/bitcoin-wallet-snap/src/index.ts +++ b/packages/bitcoin-wallet-snap/src/index.ts @@ -1,3 +1,4 @@ +import { handleKeyringRequest } from '@metamask/keyring-snap-sdk/v2'; import type { OnAssetsConversionHandler, OnAssetsLookupHandler, @@ -123,7 +124,9 @@ export const onClientRequest: OnClientRequestHandler = async ({ request }) => middleware.handle(async () => rpcHandler.route('metamask', request)); export const onKeyringRequest: OnKeyringRequestHandler = async ({ request }) => - middleware.handle(async () => keyringHandler.route(request)); + middleware.handle( + async () => (await handleKeyringRequest(keyringHandler, request)) ?? null, + ); export const onUserInput: OnUserInputHandler = async ({ id, event, context }) => middleware.handle(async () => userInputHandler.route(id, event, context)); diff --git a/packages/bitcoin-wallet-snap/tsconfig.json b/packages/bitcoin-wallet-snap/tsconfig.json index 6aeb6e95..6b93f13f 100644 --- a/packages/bitcoin-wallet-snap/tsconfig.json +++ b/packages/bitcoin-wallet-snap/tsconfig.json @@ -10,8 +10,8 @@ "noErrorTruncation": true, "noUncheckedIndexedAccess": true, "skipLibCheck": true, - "module": "CommonJS", - "moduleResolution": "node", + "module": "ESNext", + "moduleResolution": "bundler", "types": ["jest"] }, "include": ["**/*.ts", "**/*.tsx", "locales/*.json"] diff --git a/packages/sample-snap/package.json b/packages/sample-snap/package.json index be750b59..caea9590 100644 --- a/packages/sample-snap/package.json +++ b/packages/sample-snap/package.json @@ -42,9 +42,9 @@ "devDependencies": { "@jest/globals": "^29.5.0", "@metamask/auto-changelog": "^6.1.1", - "@metamask/snaps-cli": "^8.3.0", + "@metamask/snaps-cli": "^8.4.1", "@metamask/snaps-jest": "^10.2.0", - "@metamask/snaps-sdk": "11.1.1", + "@metamask/snaps-sdk": "^11.2.0", "@types/react": "18.2.4", "@types/react-dom": "18.2.4", "eslint": "^9.39.1", diff --git a/packages/tron-wallet-snap/CHANGELOG.md b/packages/tron-wallet-snap/CHANGELOG.md index 2d2df283..71db1a30 100644 --- a/packages/tron-wallet-snap/CHANGELOG.md +++ b/packages/tron-wallet-snap/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Bump `@metamask/keyring-api` from `^23.2.0` to `^23.7.0` ([#43](https://github.com/MetaMask/internal-snaps/pull/43)) +- Bump `@metamask/keyring-snap-sdk` from `^8.0.0` to `^9.2.1` ([#43](https://github.com/MetaMask/internal-snaps/pull/43)) +- Bump `@metamask/snaps-cli` from `^8.3.0` to `^8.4.1` ([#43](https://github.com/MetaMask/internal-snaps/pull/43)) +- Bump `@metamask/snaps-sdk` from `^11.1.1` to `^11.2.0` ([#43](https://github.com/MetaMask/internal-snaps/pull/43)) +- Bump `@metamask/superstruct` from `^3.2.1` to `^3.4.1` ([#43](https://github.com/MetaMask/internal-snaps/pull/43)) + ## [1.33.2] ### Changed diff --git a/packages/tron-wallet-snap/package.json b/packages/tron-wallet-snap/package.json index a8a4bde5..df2092c4 100644 --- a/packages/tron-wallet-snap/package.json +++ b/packages/tron-wallet-snap/package.json @@ -50,12 +50,12 @@ "devDependencies": { "@metamask/auto-changelog": "^6.1.1", "@metamask/key-tree": "^10.1.1", - "@metamask/keyring-api": "^23.2.0", - "@metamask/keyring-snap-sdk": "^8.0.0", - "@metamask/snaps-cli": "^8.3.0", + "@metamask/keyring-api": "^23.7.0", + "@metamask/keyring-snap-sdk": "^9.2.1", + "@metamask/snaps-cli": "^8.4.1", "@metamask/snaps-jest": "^10.2.0", - "@metamask/snaps-sdk": "11.1.1", - "@metamask/superstruct": "^3.2.1", + "@metamask/snaps-sdk": "^11.2.0", + "@metamask/superstruct": "^3.4.1", "async-mutex": "^0.5.0", "bignumber.js": "^9.3.1", "concurrently": "^10.0.3", diff --git a/packages/tron-wallet-snap/snap.manifest.json b/packages/tron-wallet-snap/snap.manifest.json index 7858e9cf..e13773c8 100644 --- a/packages/tron-wallet-snap/snap.manifest.json +++ b/packages/tron-wallet-snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/internal-snaps.git" }, "source": { - "shasum": "nOOgZJ+NdpbvQ8i1jOY1Vqh1ESeHgVPO93dklTU+YGs=", + "shasum": "tcnp/6xxntfergEzeQYc7k96SfveVo+5kBQHGEXqZkQ=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/yarn.lock b/yarn.lock index 33c62976..5fd773e5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1776,12 +1776,13 @@ __metadata: "@metamask/auto-changelog": "npm:^6.1.1" "@metamask/bitcoindevkit": "npm:^0.1.13" "@metamask/key-tree": "npm:^10.1.1" - "@metamask/keyring-api": "npm:^23.2.0" - "@metamask/keyring-snap-sdk": "npm:^8.0.0" + "@metamask/keyring-api": "npm:^23.7.0" + "@metamask/keyring-snap-sdk": "npm:^9.2.1" "@metamask/slip44": "npm:^4.2.0" - "@metamask/snaps-cli": "npm:^8.3.0" + "@metamask/snaps-cli": "npm:^8.4.1" "@metamask/snaps-jest": "npm:^10.2.0" - "@metamask/snaps-sdk": "npm:11.1.1" + "@metamask/snaps-sdk": "npm:^11.2.0" + "@metamask/superstruct": "npm:^3.4.1" "@metamask/utils": "npm:^11.9.0" bip322-js: "npm:^3.0.0" concurrently: "npm:^10.0.3" @@ -2068,7 +2069,7 @@ __metadata: languageName: node linkType: hard -"@metamask/keyring-api@npm:^23.2.0": +"@metamask/keyring-api@npm:^23.7.0": version: 23.7.0 resolution: "@metamask/keyring-api@npm:23.7.0" dependencies: @@ -2080,31 +2081,19 @@ __metadata: languageName: node linkType: hard -"@metamask/keyring-snap-sdk@npm:^8.0.0": - version: 8.0.0 - resolution: "@metamask/keyring-snap-sdk@npm:8.0.0" +"@metamask/keyring-snap-sdk@npm:^9.2.1": + version: 9.2.1 + resolution: "@metamask/keyring-snap-sdk@npm:9.2.1" dependencies: - "@metamask/keyring-utils": "npm:^3.2.0" + "@metamask/keyring-utils": "npm:^4.0.0" "@metamask/snaps-sdk": "npm:^11.0.0" - "@metamask/superstruct": "npm:^3.1.0" - "@metamask/utils": "npm:^11.1.0" + "@metamask/superstruct": "npm:^3.4.1" + "@metamask/utils": "npm:^11.11.0" webextension-polyfill: "npm:^0.12.0" peerDependencies: - "@metamask/keyring-api": ^22.0.0 + "@metamask/keyring-api": ^23.0.0 "@metamask/providers": ^19.0.0 - checksum: 10/271b4f84d214813c0312ffbbe3fb6201b66172210da4d06a155c3ddb359403ed1fa4c7afa424cd34fa9db04e3c876f30f69afb676bb7a905d392552d871ebd57 - languageName: node - linkType: hard - -"@metamask/keyring-utils@npm:^3.2.0": - version: 3.3.1 - resolution: "@metamask/keyring-utils@npm:3.3.1" - dependencies: - "@ethereumjs/tx": "npm:^5.4.0" - "@metamask/superstruct": "npm:^3.1.0" - "@metamask/utils": "npm:^11.11.0" - bitcoin-address-validation: "npm:^2.2.3" - checksum: 10/d0917b2f634d9eb2f563827739fca00c1675ee90674ec49b2b68afcb604aee843d6c5be5d79e9780fa22d29f71fc876f40b2d3d0ed4cab7f5948937ddd276691 + checksum: 10/90b01fca9cc1d055b3830e8a7d2503b40421c9ddc4e20b0db25fade73c430ac56bb482f340cd82a08a5013d76e0b8d8265c953b71f5e29be2c5e538d5cedd9e4 languageName: node linkType: hard @@ -2257,9 +2246,9 @@ __metadata: dependencies: "@jest/globals": "npm:^29.5.0" "@metamask/auto-changelog": "npm:^6.1.1" - "@metamask/snaps-cli": "npm:^8.3.0" + "@metamask/snaps-cli": "npm:^8.4.1" "@metamask/snaps-jest": "npm:^10.2.0" - "@metamask/snaps-sdk": "npm:11.1.1" + "@metamask/snaps-sdk": "npm:^11.2.0" "@types/react": "npm:18.2.4" "@types/react-dom": "npm:18.2.4" eslint: "npm:^9.39.1" @@ -2286,7 +2275,7 @@ __metadata: languageName: node linkType: hard -"@metamask/snaps-cli@npm:^8.3.0": +"@metamask/snaps-cli@npm:^8.4.1": version: 8.4.1 resolution: "@metamask/snaps-cli@npm:8.4.1" dependencies: @@ -2587,12 +2576,12 @@ __metadata: dependencies: "@metamask/auto-changelog": "npm:^6.1.1" "@metamask/key-tree": "npm:^10.1.1" - "@metamask/keyring-api": "npm:^23.2.0" - "@metamask/keyring-snap-sdk": "npm:^8.0.0" - "@metamask/snaps-cli": "npm:^8.3.0" + "@metamask/keyring-api": "npm:^23.7.0" + "@metamask/keyring-snap-sdk": "npm:^9.2.1" + "@metamask/snaps-cli": "npm:^8.4.1" "@metamask/snaps-jest": "npm:^10.2.0" - "@metamask/snaps-sdk": "npm:11.1.1" - "@metamask/superstruct": "npm:^3.2.1" + "@metamask/snaps-sdk": "npm:^11.2.0" + "@metamask/superstruct": "npm:^3.4.1" async-mutex: "npm:^0.5.0" bignumber.js: "npm:^9.3.1" concurrently: "npm:^10.0.3" @@ -2607,7 +2596,7 @@ __metadata: languageName: unknown linkType: soft -"@metamask/utils@npm:^11.0.1, @metamask/utils@npm:^11.1.0, @metamask/utils@npm:^11.10.0, @metamask/utils@npm:^11.11.0, @metamask/utils@npm:^11.4.0, @metamask/utils@npm:^11.4.2, @metamask/utils@npm:^11.8.1, @metamask/utils@npm:^11.9.0": +"@metamask/utils@npm:^11.0.1, @metamask/utils@npm:^11.10.0, @metamask/utils@npm:^11.11.0, @metamask/utils@npm:^11.4.0, @metamask/utils@npm:^11.4.2, @metamask/utils@npm:^11.8.1, @metamask/utils@npm:^11.9.0": version: 11.11.0 resolution: "@metamask/utils@npm:11.11.0" dependencies: