diff --git a/packages/snap/integration-test/blockchain-utils.ts b/packages/snap/integration-test/blockchain-utils.ts index 87f2e0807..df00e9a1d 100644 --- a/packages/snap/integration-test/blockchain-utils.ts +++ b/packages/snap/integration-test/blockchain-utils.ts @@ -174,4 +174,17 @@ export class BlockchainTestUtils { const balanceSats = await this.getBalance(address); return Number(balanceSats) / 100_000_000; } + + /** + * Generate a new Bitcoin address from the regtest node wallet. + * + * @param addressType - The address type to generate. Defaults to 'bech32'. + * Valid values: 'legacy', 'p2sh-segwit', 'bech32', 'bech32m'. + * @returns A new Bitcoin address. + */ + async getNewAddress( + addressType: 'legacy' | 'p2sh-segwit' | 'bech32' | 'bech32m' = 'bech32', + ): Promise { + return this.#execCli(`-rpcwallet=default getnewaddress "" ${addressType}`); + } } diff --git a/packages/snap/integration-test/constants.ts b/packages/snap/integration-test/constants.ts index e19d50f10..7f8cd45f3 100644 --- a/packages/snap/integration-test/constants.ts +++ b/packages/snap/integration-test/constants.ts @@ -9,6 +9,13 @@ export const TEST_ADDRESS_REGTEST = 'bcrt1qjtgffm20l9vu6a7gacxvpu2ej4kdcsgcgnly6t'; export const TEST_ADDRESS_MAINNET = 'bc1q832zlt4tgnqy88vd20mazw77dlt0j0wf2naw8q'; + +// P2TR (Taproot) addresses +export const TEST_ADDRESS_P2TR_MAINNET = + 'bc1p4rue37y0v9snd4z3fvw43d29u97qxf9j3fva72xy2t7hekg24dzsaz40mz'; +export const TEST_ADDRESS_P2TR_TESTNET = + 'tb1pwwjax3vpq6h69965hcr22vkpm4qdvyu2pz67wyj8eagp9vxkcz0q0ya20h'; + export const ORIGIN = 'metamask'; export const FUNDING_TX = { account: expect.any(Number), @@ -50,3 +57,7 @@ export const scopeToCoinType: Record = { [BtcScope.Signet]: "1'", [BtcScope.Regtest]: "1'", }; + +// PSBTs can be decoded here: https://bitcoincore.tech/apps/bitcoinjs-ui/index.html +export const TEMPLATE_PSBT = + 'cHNidP8BAI4CAAAAAAM1gwEAAAAAACJRIORP1Ndiq325lSC/jMG0RlhATHYmuuULfXgEHUM3u5i4AAAAAAAAAAAxai8AAUSx+i9Igg4HWdcpyagCs8mzuRCklgA7nRMkm69rAAAAAAAAAAAAAQACAAAAACp2AAAAAAAAFgAUgpMvYEJ/dp36svRJyRtNnpSo7bQAAAAAAAAAAAA='; diff --git a/packages/snap/integration-test/keyring-request.test.ts b/packages/snap/integration-test/keyring-request.test.ts index fde6972ea..f9f3614eb 100644 --- a/packages/snap/integration-test/keyring-request.test.ts +++ b/packages/snap/integration-test/keyring-request.test.ts @@ -1,10 +1,11 @@ -import type { KeyringAccount, KeyringRequest } from '@metamask/keyring-api'; +import type { KeyringAccount } from '@metamask/keyring-api'; import { BtcScope } from '@metamask/keyring-api'; import type { Snap } from '@metamask/snaps-jest'; import { assertIsConfirmationDialog, installSnap } from '@metamask/snaps-jest'; import { BlockchainTestUtils } from './blockchain-utils'; -import { MNEMONIC, ORIGIN } from './constants'; +import { MNEMONIC, ORIGIN, TEMPLATE_PSBT } from './constants'; +import { getRequiredOutpoint } from './test-helpers'; import { AccountCapability } from '../src/entities'; import type { FillPsbtResponse } from '../src/handlers/KeyringRequestHandler'; @@ -81,7 +82,7 @@ describe('KeyringRequestHandler', () => { request: { method: AccountCapability.SignPsbt, }, - } as KeyringRequest, + }, }); expect(response).toRespondWithError({ @@ -104,7 +105,7 @@ describe('KeyringRequestHandler', () => { request: { method: 'invalidMethod', }, - } as KeyringRequest, + }, }); expect(response).toRespondWithError({ @@ -134,7 +135,7 @@ describe('KeyringRequestHandler', () => { request: { method: AccountCapability.ListUtxos, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -155,6 +156,7 @@ describe('KeyringRequestHandler', () => { const utxos = ( response.response as { result: { result: { outpoint: string }[] } } ).result.result; + const outpoint = getRequiredOutpoint(utxos); response = await snap.onKeyringRequest({ origin: ORIGIN, @@ -167,10 +169,10 @@ describe('KeyringRequestHandler', () => { request: { method: AccountCapability.GetUtxo, params: { - outpoint: utxos[0]?.outpoint, + outpoint, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -191,7 +193,7 @@ describe('KeyringRequestHandler', () => { request: { method: AccountCapability.PublicDescriptor, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -203,9 +205,6 @@ describe('KeyringRequestHandler', () => { }); describe('signPsbt', () => { - // PSBTs can be decoded here: https://bitcoincore.tech/apps/bitcoinjs-ui/index.html - const TEMPLATE_PSBT = - 'cHNidP8BAI4CAAAAAAM1gwEAAAAAACJRIORP1Ndiq325lSC/jMG0RlhATHYmuuULfXgEHUM3u5i4AAAAAAAAAAAxai8AAUSx+i9Igg4HWdcpyagCs8mzuRCklgA7nRMkm69rAAAAAAAAAAAAAQACAAAAACp2AAAAAAAAFgAUgpMvYEJ/dp36svRJyRtNnpSo7bQAAAAAAAAAAAA='; const SIGNED_PSBT = 'cHNidP8BAI4CAAAAAAM1gwEAAAAAACJRIORP1Ndiq325lSC/jMG0RlhATHYmuuULfXgEHUM3u5i4AAAAAAAAAAAxai8AAUSx+i9Igg4HWdcpyagCs8mzuRCklgA7nRMkm69rAAAAAAAAAAAAAQACAAAAACp2AAAAAAAAFgAUgpMvYEJ/dp36svRJyRtNnpSo7bQAAAAAAAAAAA=='; @@ -229,7 +228,7 @@ describe('KeyringRequestHandler', () => { }, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -261,7 +260,7 @@ describe('KeyringRequestHandler', () => { }, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -293,7 +292,7 @@ describe('KeyringRequestHandler', () => { }, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -324,7 +323,7 @@ describe('KeyringRequestHandler', () => { }, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWithError({ @@ -353,7 +352,7 @@ describe('KeyringRequestHandler', () => { psbt: TEMPLATE_PSBT, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWithError({ @@ -366,10 +365,6 @@ describe('KeyringRequestHandler', () => { }); describe('fillPsbt', () => { - // PSBTs can be decoded here: https://bitcoincore.tech/apps/bitcoinjs-ui/index.html - const TEMPLATE_PSBT = - 'cHNidP8BAI4CAAAAAAM1gwEAAAAAACJRIORP1Ndiq325lSC/jMG0RlhATHYmuuULfXgEHUM3u5i4AAAAAAAAAAAxai8AAUSx+i9Igg4HWdcpyagCs8mzuRCklgA7nRMkm69rAAAAAAAAAAAAAQACAAAAACp2AAAAAAAAFgAUgpMvYEJ/dp36svRJyRtNnpSo7bQAAAAAAAAAAAA='; - it('fills a PSBT successfully', async () => { const response = await snap.onKeyringRequest({ origin: ORIGIN, @@ -386,7 +381,7 @@ describe('KeyringRequestHandler', () => { feeRate: 3, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -412,7 +407,7 @@ describe('KeyringRequestHandler', () => { psbt: 'notAPsbt', }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWithError({ @@ -428,10 +423,6 @@ describe('KeyringRequestHandler', () => { }); describe('computeFee', () => { - // PSBTs can be decoded here: https://bitcoincore.tech/apps/bitcoinjs-ui/index.html - const TEMPLATE_PSBT = - 'cHNidP8BAI4CAAAAAAM1gwEAAAAAACJRIORP1Ndiq325lSC/jMG0RlhATHYmuuULfXgEHUM3u5i4AAAAAAAAAAAxai8AAUSx+i9Igg4HWdcpyagCs8mzuRCklgA7nRMkm69rAAAAAAAAAAAAAQACAAAAACp2AAAAAAAAFgAUgpMvYEJ/dp36svRJyRtNnpSo7bQAAAAAAAAAAAA='; - it('computes the fee for a PSBT successfully', async () => { const response = await snap.onKeyringRequest({ origin: ORIGIN, @@ -448,7 +439,7 @@ describe('KeyringRequestHandler', () => { feeRate: 3, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -474,7 +465,7 @@ describe('KeyringRequestHandler', () => { psbt: 'notAPsbt', }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWithError({ @@ -490,10 +481,6 @@ describe('KeyringRequestHandler', () => { }); describe('broadcastPsbt', () => { - // PSBTs can be decoded here: https://bitcoincore.tech/apps/bitcoinjs-ui/index.html - const TEMPLATE_PSBT = - 'cHNidP8BAI4CAAAAAAM1gwEAAAAAACJRIORP1Ndiq325lSC/jMG0RlhATHYmuuULfXgEHUM3u5i4AAAAAAAAAAAxai8AAUSx+i9Igg4HWdcpyagCs8mzuRCklgA7nRMkm69rAAAAAAAAAAAAAQACAAAAACp2AAAAAAAAFgAUgpMvYEJ/dp36svRJyRtNnpSo7bQAAAAAAAAAAAA='; - it('broadcasts a PSBT successfully', async () => { // Prepare the PSBT to broadcast so we have a valid PSBT to broadcast let response = await snap.onKeyringRequest({ @@ -515,7 +502,7 @@ describe('KeyringRequestHandler', () => { }, }, }, - } as KeyringRequest, + }, }); const { result } = ( @@ -536,7 +523,7 @@ describe('KeyringRequestHandler', () => { psbt: result.psbt, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -562,7 +549,7 @@ describe('KeyringRequestHandler', () => { psbt: 'notAPsbt', }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWithError({ @@ -603,7 +590,7 @@ describe('KeyringRequestHandler', () => { feeRate: 3, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -629,7 +616,7 @@ describe('KeyringRequestHandler', () => { recipients: [{ address: 'notAnAddress', amount: '1000' }], }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWithError({ @@ -657,7 +644,7 @@ describe('KeyringRequestHandler', () => { message: 'Hello, world!', }, }, - } as KeyringRequest, + }, }); const ui = await response.getInterface(); diff --git a/packages/snap/integration-test/keyring.test.ts b/packages/snap/integration-test/keyring.test.ts index b092ae3cb..91b886424 100644 --- a/packages/snap/integration-test/keyring.test.ts +++ b/packages/snap/integration-test/keyring.test.ts @@ -9,6 +9,8 @@ import { ORIGIN, TEST_ADDRESS_REGTEST, TEST_ADDRESS_MAINNET, + TEST_ADDRESS_P2TR_MAINNET, + TEST_ADDRESS_P2TR_TESTNET, scopeToCoinType, accountTypeToPurpose, } from './constants'; @@ -64,7 +66,8 @@ describe('Keyring', () => { }, }); - // We should get 1 account, the p2wpkh one of Regtest + // Discovery now checks both P2WPKH and P2TR, but only returns accounts with history. + // We should get 1 account (P2WPKH) since only that one has funded history on Regtest. expect(response).toRespondWith([ { type: 'bip44', @@ -165,76 +168,52 @@ describe('Keyring', () => { }, ); - // skip non-P2WPKH address types as we are not supporting them for v1 - it.skip.each([ - { - addressType: BtcAccountType.P2pkh, - scope: BtcScope.Mainnet, - index: 0, - expectedAddress: '15feVv7kK3z7jxA4RZZzY7Fwdu3yqFwzcT', - }, - { - addressType: BtcAccountType.P2pkh, - scope: BtcScope.Testnet, - index: 0, - expectedAddress: 'mjPQaLkhZN3MxsYN8Nebzwevuz8vdTaRCq', - }, - { - addressType: BtcAccountType.P2sh, - scope: BtcScope.Mainnet, - index: 0, - expectedAddress: '3QVSaDYjxEh4L3K24eorrQjfVxPAKJMys2', - }, - { - addressType: BtcAccountType.P2sh, - scope: BtcScope.Testnet, - index: 0, - expectedAddress: '2NBG623WvXp1zxKB6gK2mnMe2mSDCur5qRU', - }, + it.each([ { addressType: BtcAccountType.P2tr, scope: BtcScope.Mainnet, index: 0, - expectedAddress: - 'bc1p4rue37y0v9snd4z3fvw43d29u97qxf9j3fva72xy2t7hekg24dzsaz40mz', + expectedAddress: TEST_ADDRESS_P2TR_MAINNET, }, { addressType: BtcAccountType.P2tr, scope: BtcScope.Testnet, index: 0, - expectedAddress: - 'tb1pwwjax3vpq6h69965hcr22vkpm4qdvyu2pz67wyj8eagp9vxkcz0q0ya20h', + expectedAddress: TEST_ADDRESS_P2TR_TESTNET, }, - ])('creates an account: %s', async ({ expectedAddress, ...requestOpts }) => { - const response = await snap.onKeyringRequest({ - origin: ORIGIN, - method: 'keyring_createAccount', - params: { options: { ...requestOpts, synchronize: false } }, - }); + ])( + 'creates a P2TR account: %s', + async ({ expectedAddress, ...requestOpts }) => { + const response = await snap.onKeyringRequest({ + origin: ORIGIN, + method: 'keyring_createAccount', + params: { options: { ...requestOpts, synchronize: false } }, + }); - 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: `m/${accountTypeToPurpose[requestOpts.addressType]}/${scopeToCoinType[requestOpts.scope]}/${requestOpts.index}'`, + }, + exportable: false, }, - exportable: false, - }, - scopes: [requestOpts.scope], - methods: Object.values(AccountCapability), - }); + scopes: [requestOpts.scope], + methods: Object.values(AccountCapability), + }); - // eslint-disable-next-line jest/no-conditional-in-test - if ('result' in response.response) { - accounts[expectedAddress] = response.response.result as KeyringAccount; - } - }); + // eslint-disable-next-line jest/no-conditional-in-test + if ('result' in response.response) { + accounts[expectedAddress] = response.response.result as KeyringAccount; + } + }, + ); it('returns the same account if already exists by derivationPath', async () => { // Account already exists so we should get the same account @@ -273,20 +252,17 @@ describe('Keyring', () => { { addressType: BtcAccountType.P2pkh, scope: BtcScope.Mainnet, - expectedError: 'Only native segwit (P2WPKH) addresses are supported', + expectedError: + 'Only native segwit (P2WPKH) and taproot (P2TR) 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', + expectedError: + 'Only native segwit (P2WPKH) and taproot (P2TR) addresses are supported', }, ])( - 'rejects creation of non-P2WPKH account: $addressType', + 'rejects creation of non-P2WPKH/P2TR account: $addressType', async ({ addressType, scope, expectedError }) => { const response = await snap.onKeyringRequest({ origin: ORIGIN, @@ -314,20 +290,15 @@ describe('Keyring', () => { { derivationPath: "m/44'/0'/0'", // (P2PKH) expectedError: - 'Only native segwit (BIP-84) derivation paths are supported', + 'Only native segwit (BIP-84) and taproot (BIP-86) derivation paths are supported', }, { derivationPath: "m/49'/0'/0'", // (P2SH) expectedError: - 'Only native segwit (BIP-84) derivation paths are supported', - }, - { - derivationPath: "m/86'/0'/0'", // (P2TR) - expectedError: - 'Only native segwit (BIP-84) derivation paths are supported', + 'Only native segwit (BIP-84) and taproot (BIP-86) derivation paths are supported', }, ])( - 'rejects creation with non-BIP84 derivation path: $derivationPath', + 'rejects creation with non-BIP84/BIP86 derivation path: $derivationPath', async ({ derivationPath, expectedError }) => { const response = await snap.onKeyringRequest({ origin: ORIGIN, @@ -368,7 +339,7 @@ describe('Keyring', () => { error: { code: -32000, message: - 'Invalid format: Only native segwit (BIP-84) derivation paths are supported', + 'Invalid format: Only native segwit (BIP-84) and taproot (BIP-86) derivation paths are supported', }, }); }); @@ -392,7 +363,41 @@ describe('Keyring', () => { const account: KeyringAccount = ( response.response as { result: KeyringAccount } ).result; - expect(account.address).toMatch(/^bcrt1/u); // Native segwit address + expect(account.address).toMatch(/^bcrt1q/u); // Native segwit address + expect((account.options.entropy as { groupIndex: number }).groupIndex).toBe( + 10, + ); + + // remove to avoid interfering with other tests + await snap.onKeyringRequest({ + origin: ORIGIN, + method: 'keyring_deleteAccount', + params: { + id: account.id, + }, + }); + }); + + it('accepts creation when addressType and derivationPath both indicate P2TR', async () => { + const response = await snap.onKeyringRequest({ + origin: ORIGIN, + method: 'keyring_createAccount', + params: { + options: { + scope: BtcScope.Regtest, + addressType: BtcAccountType.P2tr, + derivationPath: "m/86'/1'/10'", // Taproot path matching P2TR + synchronize: false, + }, + }, + }); + + expect(response.response).toHaveProperty('result'); + + const account: KeyringAccount = ( + response.response as { result: KeyringAccount } + ).result; + expect(account.address).toMatch(/^bcrt1p/u); // Taproot address expect((account.options.entropy as { groupIndex: number }).groupIndex).toBe( 10, ); diff --git a/packages/snap/integration-test/taproot.test.ts b/packages/snap/integration-test/taproot.test.ts new file mode 100644 index 000000000..c464265ef --- /dev/null +++ b/packages/snap/integration-test/taproot.test.ts @@ -0,0 +1,549 @@ +import type { KeyringAccount } from '@metamask/keyring-api'; +import { BtcAccountType, BtcScope } from '@metamask/keyring-api'; +import type { Snap } from '@metamask/snaps-jest'; +import { assertIsConfirmationDialog, installSnap } from '@metamask/snaps-jest'; + +import { BlockchainTestUtils } from './blockchain-utils'; +import { MNEMONIC, ORIGIN, TEMPLATE_PSBT } from './constants'; +import { getRequiredOutpoint } from './test-helpers'; +import { AccountCapability } from '../src/entities'; +import type { FillPsbtResponse } from '../src/handlers/KeyringRequestHandler'; + +const ACCOUNT_INDEX = 5; // Use different index to avoid conflicts with other tests +const submitRequestMethod = 'keyring_submitRequest'; + +describe('Taproot (P2TR) Integration Tests', () => { + let account: KeyringAccount; + let snap: Snap; + let blockchain: BlockchainTestUtils; + const origin = 'http://my-dapp.com'; + let createdAccountId: string | undefined; + + beforeAll(async () => { + blockchain = new BlockchainTestUtils(); + snap = await installSnap({ + options: { + secretRecoveryPhrase: MNEMONIC, + }, + }); + + snap.mockJsonRpc((request) => { + if (request.method === 'snap_manageAccounts') { + const params = request.params as Record | undefined; + if (params && params.method === 'getSelectedAccounts') { + return createdAccountId ? [createdAccountId] : []; + } + return null; + } + + if (request.method === 'snap_trackError') { + return {}; + } + + if (request.method === 'snap_scheduleBackgroundEvent') { + return 'mock-event-id'; + } + + return undefined; + }); + + // Create a P2TR (Taproot) account + const response = await snap.onKeyringRequest({ + origin: ORIGIN, + method: 'keyring_createAccount', + params: { + options: { + scope: BtcScope.Regtest, + addressType: BtcAccountType.P2tr, + synchronize: false, + index: ACCOUNT_INDEX, + }, + }, + }); + + if ('result' in response.response) { + account = response.response.result as KeyringAccount; + createdAccountId = account.id; + } + + // Fund the P2TR account + await blockchain.sendToAddress(account.address, 10); + await blockchain.mineBlocks(6); + await snap.onCronjob({ method: 'synchronizeAccounts' }); + }); + + describe('Account Creation', () => { + it('creates a P2TR account with correct properties', () => { + expect(account).toBeDefined(); + expect(account.type).toBe(BtcAccountType.P2tr); + // Regtest Taproot addresses start with bcrt1p + expect(account.address).toMatch(/^bcrt1p/u); + }); + + it('has correct derivation path for Taproot (BIP-86)', () => { + const entropy = account.options.entropy as { derivationPath: string }; + // BIP-86 uses purpose 86' for Taproot + expect(entropy.derivationPath).toMatch(/^m\/86'/u); + }); + }); + + describe('UTXO Management', () => { + it('lists UTXOs for P2TR account', async () => { + const response = await snap.onKeyringRequest({ + origin: ORIGIN, + method: submitRequestMethod, + params: { + id: account.id, + origin, + scope: BtcScope.Regtest, + account: account.id, + request: { + method: AccountCapability.ListUtxos, + }, + }, + }); + + expect(response).toRespondWith({ + pending: false, + result: expect.arrayContaining([ + expect.objectContaining({ + address: expect.stringMatching(/^bcrt1p/u), // Taproot address + value: '1000000000', // 10 BTC in sats + }), + ]), + }); + }); + + it('gets a specific UTXO for P2TR account', async () => { + // First get the list of UTXOs + let response = await snap.onKeyringRequest({ + origin: ORIGIN, + method: submitRequestMethod, + params: { + id: account.id, + origin, + scope: BtcScope.Regtest, + account: account.id, + request: { + method: AccountCapability.ListUtxos, + }, + }, + }); + + const utxos = ( + response.response as { result: { result: { outpoint: string }[] } } + ).result.result; + const outpoint = getRequiredOutpoint(utxos); + + // Then get a specific UTXO + response = await snap.onKeyringRequest({ + origin: ORIGIN, + method: submitRequestMethod, + params: { + id: account.id, + origin, + scope: BtcScope.Regtest, + account: account.id, + request: { + method: AccountCapability.GetUtxo, + params: { + outpoint, + }, + }, + }, + }); + + expect(response).toRespondWith({ + pending: false, + result: expect.objectContaining({ + outpoint, + address: expect.stringMatching(/^bcrt1p/u), + }), + }); + }); + + it('returns correct Taproot descriptor', async () => { + const response = await snap.onKeyringRequest({ + origin: ORIGIN, + method: submitRequestMethod, + params: { + id: account.id, + origin, + scope: BtcScope.Regtest, + account: account.id, + request: { + method: AccountCapability.PublicDescriptor, + }, + }, + }); + + expect(response).toRespondWith({ + pending: false, + // Taproot descriptor starts with 'tr(' + result: expect.stringMatching(/^tr\(/u), + }); + }); + }); + + describe('PSBT Signing (Schnorr)', () => { + it('fills a PSBT for P2TR account', async () => { + const response = await snap.onKeyringRequest({ + origin: ORIGIN, + method: submitRequestMethod, + params: { + id: account.id, + origin, + scope: BtcScope.Regtest, + account: account.id, + request: { + method: AccountCapability.FillPsbt, + params: { + psbt: TEMPLATE_PSBT, + feeRate: 3, + }, + }, + }, + }); + + expect(response).toRespondWith({ + pending: false, + result: { + psbt: expect.any(String), + }, + }); + }); + + it('signs a PSBT with Schnorr signature (fill and sign)', async () => { + const response = await snap.onKeyringRequest({ + origin: ORIGIN, + method: submitRequestMethod, + params: { + id: account.id, + origin, + scope: BtcScope.Regtest, + account: account.id, + request: { + method: AccountCapability.SignPsbt, + params: { + psbt: TEMPLATE_PSBT, + feeRate: 3, + options: { + fill: true, + broadcast: false, + }, + }, + }, + }, + }); + + expect(response).toRespondWith({ + pending: false, + result: { + psbt: expect.any(String), + txid: null, + }, + }); + }); + + it('signs and broadcasts a PSBT from P2TR account', async () => { + const response = await snap.onKeyringRequest({ + origin: ORIGIN, + method: submitRequestMethod, + params: { + id: account.id, + origin, + scope: BtcScope.Regtest, + account: account.id, + request: { + method: AccountCapability.SignPsbt, + params: { + psbt: TEMPLATE_PSBT, + feeRate: 3, + options: { + fill: true, + broadcast: true, + }, + }, + }, + }, + }); + + expect(response).toRespondWith({ + pending: false, + result: { + psbt: expect.any(String), + txid: expect.any(String), + }, + }); + + // Mine blocks to confirm the transaction + await blockchain.mineBlocks(1); + }); + + it('computes fee for P2TR transaction', async () => { + const response = await snap.onKeyringRequest({ + origin: ORIGIN, + method: submitRequestMethod, + params: { + id: account.id, + origin, + scope: BtcScope.Regtest, + account: account.id, + request: { + method: AccountCapability.ComputeFee, + params: { + psbt: TEMPLATE_PSBT, + feeRate: 3, + }, + }, + }, + }); + + expect(response).toRespondWith({ + pending: false, + result: { + fee: expect.any(String), + }, + }); + }); + }); + + describe('Send Transfer', () => { + it('sends funds from P2TR account to P2WPKH address', async () => { + const response = await snap.onKeyringRequest({ + origin: ORIGIN, + method: submitRequestMethod, + params: { + id: account.id, + origin, + scope: BtcScope.Regtest, + account: account.id, + request: { + method: AccountCapability.SendTransfer, + params: { + recipients: [ + { + // P2WPKH address (bcrt1q...) + address: 'bcrt1qstku2y3pfh9av50lxj55arm8r5gj8tf2yv5nxz', + amount: '1000', + }, + ], + feeRate: 3, + }, + }, + }, + }); + + expect(response).toRespondWith({ + pending: false, + result: { + txid: expect.any(String), + }, + }); + + // Mine blocks to confirm + await blockchain.mineBlocks(1); + }); + + it('sends funds from P2TR account to another P2TR address', async () => { + // Generate a valid P2TR regtest address from the node + const p2trRecipient = await blockchain.getNewAddress('bech32m'); + expect(p2trRecipient).toMatch(/^bcrt1p/u); + + const response = await snap.onKeyringRequest({ + origin: ORIGIN, + method: submitRequestMethod, + params: { + id: account.id, + origin, + scope: BtcScope.Regtest, + account: account.id, + request: { + method: AccountCapability.SendTransfer, + params: { + recipients: [ + { + address: p2trRecipient, + amount: '1000', + }, + ], + feeRate: 3, + }, + }, + }, + }); + + expect(response).toRespondWith({ + pending: false, + result: { + txid: expect.any(String), + }, + }); + + // Mine blocks to confirm + await blockchain.mineBlocks(1); + }); + + it('sends to multiple recipients from P2TR account', async () => { + const response = await snap.onKeyringRequest({ + origin: ORIGIN, + method: submitRequestMethod, + params: { + id: account.id, + origin, + scope: BtcScope.Regtest, + account: account.id, + request: { + method: AccountCapability.SendTransfer, + params: { + recipients: [ + { + address: 'bcrt1qstku2y3pfh9av50lxj55arm8r5gj8tf2yv5nxz', + amount: '500', + }, + { + address: 'bcrt1q4gfcga7jfjmm02zpvrh4ttc5k7lmnq2re52z2y', + amount: '500', + }, + ], + feeRate: 3, + }, + }, + }, + }); + + expect(response).toRespondWith({ + pending: false, + result: { + txid: expect.any(String), + }, + }); + + // Mine blocks to confirm + await blockchain.mineBlocks(1); + }); + }); + + describe('Message Signing (BIP-322)', () => { + it('signs a message with P2TR account using BIP-322', async () => { + const response = snap.onKeyringRequest({ + origin: ORIGIN, + method: submitRequestMethod, + params: { + id: account.id, + origin, + scope: BtcScope.Regtest, + account: account.id, + request: { + method: AccountCapability.SignMessage, + params: { + message: 'Hello, Taproot!', + }, + }, + }, + }); + + const ui = await response.getInterface(); + assertIsConfirmationDialog(ui); + await ui.ok(); + + const result = await response; + + expect(result).toRespondWith({ + pending: false, + result: { + // BIP-322 signature for Taproot + signature: expect.any(String), + }, + }); + }); + + it('signs an empty message with P2TR account', async () => { + const response = snap.onKeyringRequest({ + origin: ORIGIN, + method: submitRequestMethod, + params: { + id: account.id, + origin, + scope: BtcScope.Regtest, + account: account.id, + request: { + method: AccountCapability.SignMessage, + params: { + message: '', + }, + }, + }, + }); + + const ui = await response.getInterface(); + assertIsConfirmationDialog(ui); + await ui.ok(); + + const result = await response; + + expect(result).toRespondWith({ + pending: false, + result: { + signature: expect.any(String), + }, + }); + }); + }); + + describe('Broadcast', () => { + it('broadcasts a signed P2TR transaction', async () => { + // First sign the PSBT without broadcasting + let response = await snap.onKeyringRequest({ + origin: ORIGIN, + method: submitRequestMethod, + params: { + id: account.id, + origin, + scope: BtcScope.Regtest, + account: account.id, + request: { + method: AccountCapability.SignPsbt, + params: { + psbt: TEMPLATE_PSBT, + feeRate: 3, + options: { + fill: true, + broadcast: false, + }, + }, + }, + }, + }); + + const { result } = ( + response.response as { result: { result: FillPsbtResponse } } + ).result; + + // Then broadcast separately + response = await snap.onKeyringRequest({ + origin: ORIGIN, + method: submitRequestMethod, + params: { + id: account.id, + origin, + scope: BtcScope.Regtest, + account: account.id, + request: { + method: AccountCapability.BroadcastPsbt, + params: { + psbt: result.psbt, + }, + }, + }, + }); + + expect(response).toRespondWith({ + pending: false, + result: { + txid: expect.any(String), + }, + }); + }); + }); +}); diff --git a/packages/snap/integration-test/test-helpers.ts b/packages/snap/integration-test/test-helpers.ts new file mode 100644 index 000000000..b6eca4b00 --- /dev/null +++ b/packages/snap/integration-test/test-helpers.ts @@ -0,0 +1,7 @@ +export const getRequiredOutpoint = (utxos: { outpoint?: string }[]): string => { + const outpoint = utxos[0]?.outpoint; + if (!outpoint) { + throw new Error('Expected at least one UTXO with outpoint'); + } + return outpoint; +}; diff --git a/packages/snap/src/handlers/KeyringHandler.test.ts b/packages/snap/src/handlers/KeyringHandler.test.ts index b2f0047f5..308bbccb7 100644 --- a/packages/snap/src/handlers/KeyringHandler.test.ts +++ b/packages/snap/src/handlers/KeyringHandler.test.ts @@ -27,7 +27,7 @@ import { FormatError, } from '../entities'; import { scopeToNetwork, caipToAddressType, Caip19Asset } from './caip'; -import { KeyringHandler, CreateAccountRequest } from './KeyringHandler'; +import { CreateAccountRequest, KeyringHandler } from './KeyringHandler'; import type { KeyringRequestHandler } from './KeyringRequestHandler'; import { mapToDiscoveredAccount } from './mappings'; import type { @@ -87,68 +87,6 @@ describe('KeyringHandler', () => { }); 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 - 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 - 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([ @@ -197,11 +135,14 @@ describe('KeyringHandler', () => { expect(mockAccounts.create).toHaveBeenCalledWith(expectedCreateParams); }); - it.each([{ purpose: Purpose.NativeSegwit, addressType: 'p2wpkh' }] as { + it.each([ + { purpose: Purpose.NativeSegwit, addressType: 'p2wpkh' }, + { purpose: Purpose.Taproot, addressType: 'p2tr' }, + ] as { purpose: Purpose; addressType: AddressType; }[])( - 'extracts P2WPKH address type from derivationPath: %s', + 'extracts address type from derivationPath: %s', async ({ purpose, addressType }) => { const options = { scope: BtcScope.Signet, @@ -216,32 +157,8 @@ describe('KeyringHandler', () => { }; await handler.createAccount(options); - expect(mockAccounts.create).toHaveBeenCalledWith(expectedCreateParams); - }, - ); - // skip non-P2WPKH address types as they are not supported on v1 - 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(assert).toHaveBeenCalledWith(options, CreateAccountRequest); expect(mockAccounts.create).toHaveBeenCalledWith(expectedCreateParams); }, ); @@ -279,7 +196,7 @@ describe('KeyringHandler', () => { // 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', + 'Only native segwit (BIP-84) and taproot (BIP-86) derivation paths are supported', ), ); }); @@ -299,9 +216,126 @@ describe('KeyringHandler', () => { }; await handler.createAccount(options); + + expect(mockAccounts.create).toHaveBeenCalledWith(expectedCreateParams); + }); + + it('accepts explicit P2TR addressType', async () => { + const options = { + scope: BtcScope.Mainnet, + addressType: BtcAccountType.P2tr, + index: 0, + }; + const expectedCreateParams: CreateAccountParams = { + network: 'bitcoin', + index: 0, + addressType: 'p2tr', + entropySource: 'm', + synchronize: false, + }; + + await handler.createAccount(options); + + expect(mockAccounts.create).toHaveBeenCalledWith(expectedCreateParams); + }); + + it('succeeds when addressType and derivationPath both indicate P2TR', async () => { + const options = { + scope: BtcScope.Signet, + addressType: BtcAccountType.P2tr, + derivationPath: "m/86'/0'/5'", // Taproot path + }; + const expectedCreateParams: CreateAccountParams = { + network: 'signet', + index: 5, + addressType: 'p2tr', + entropySource: 'm', + synchronize: false, + }; + + await handler.createAccount(options); + expect(mockAccounts.create).toHaveBeenCalledWith(expectedCreateParams); }); + it('fails when P2TR addressType and P2WPKH derivationPath mismatch', async () => { + const options = { + scope: BtcScope.Signet, + addressType: BtcAccountType.P2tr, + derivationPath: "m/84'/0'/0'", // SegWit path, not Taproot + }; + + await expect(handler.createAccount(options)).rejects.toThrow( + new FormatError('Address type and derivation path mismatch'), + ); + }); + + it('fails when P2WPKH addressType and P2TR derivationPath mismatch', async () => { + const options = { + scope: BtcScope.Signet, + addressType: BtcAccountType.P2wpkh, + derivationPath: "m/86'/0'/0'", // Taproot path, not SegWit + }; + + await expect(handler.createAccount(options)).rejects.toThrow( + new FormatError('Address type and derivation path mismatch'), + ); + }); + + it('rejects unsupported P2PKH addressType', async () => { + const options = { + scope: BtcScope.Mainnet, + addressType: BtcAccountType.P2pkh, + index: 0, + }; + + await expect(handler.createAccount(options)).rejects.toThrow( + new FormatError( + 'Only native segwit (P2WPKH) and taproot (P2TR) addresses are supported', + ), + ); + }); + + it('auto increments index for P2TR accounts separately from P2WPKH', async () => { + mockAccounts.list.mockResolvedValue([ + mock({ + entropySource: 'entropy1', + accountIndex: 0, + addressType: 'p2wpkh', + network: 'signet', + }), + mock({ + entropySource: 'entropy1', + accountIndex: 0, + addressType: 'p2tr', + network: 'signet', + }), + mock({ + entropySource: 'entropy1', + accountIndex: 1, + addressType: 'p2tr', + network: 'signet', + }), + ]); + + const options = { + scope: BtcScope.Signet, + addressType: BtcAccountType.P2tr, + entropySource: 'entropy1', + index: null, + }; + + await handler.createAccount(options); + + // Should get index 2 for P2TR (0 and 1 are used) + expect(mockAccounts.create).toHaveBeenCalledWith( + expect.objectContaining({ + index: 2, + addressType: 'p2tr', + }), + ); + }); + it('propagates errors from createAccount', async () => { const error = new Error('createAccount error'); mockAccounts.create.mockRejectedValue(error); @@ -395,18 +429,21 @@ describe('KeyringHandler', () => { 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; + // P2WPKH and P2TR are supported + const addressTypeConfigs = [ + { type: BtcAccountType.P2wpkh, purpose: "84'" }, + { type: BtcAccountType.P2tr, purpose: "86'" }, + ]; + const totalCombinations = scopes.length * addressTypeConfigs.length; const expected: DiscoveredAccount[] = []; scopes.forEach((scope) => { - addressTypes.forEach((addrType) => { + addressTypeConfigs.forEach(({ type: addrType, purpose }) => { const acc = mock({ addressType: caipToAddressType[addrType], network: scopeToNetwork[scope], listTransactions: jest.fn().mockReturnValue([{}]), // has history - derivationPath: ['m', "84'", "0'", "0'"], + derivationPath: ['m', purpose, "0'", "0'"], }); expected.push(mapToDiscoveredAccount(acc)); @@ -424,8 +461,8 @@ describe('KeyringHandler', () => { // validate each individual create() call arguments scopes.forEach((scope, sIdx) => { - addressTypes.forEach((addrType, aIdx) => { - const callIdx = sIdx * addressTypes.length + aIdx; + addressTypeConfigs.forEach(({ type: addrType }, aIdx) => { + const callIdx = sIdx * addressTypeConfigs.length + aIdx; expect(mockAccounts.discover).toHaveBeenNthCalledWith(callIdx + 1, { network: scopeToNetwork[scope], entropySource, @@ -442,31 +479,63 @@ describe('KeyringHandler', () => { it('returns mix of accounts with and without history, filtering correctly', async () => { // create mock accounts - some with history, some without - const accountWithHistory1 = mock({ + // For each scope, we discover both P2WPKH and P2TR accounts + + // Mainnet P2WPKH - has history + const mainnetP2wpkh = mock({ addressType: 'p2wpkh', network: 'bitcoin', listTransactions: jest.fn().mockReturnValue([{}, {}]), // has 2 transactions derivationPath: ['m', "84'", "0'", "0'"], }); - const accountWithoutHistory = mock({ + // Mainnet P2TR - no history + const mainnetP2tr = mock({ + addressType: 'p2tr', + network: 'bitcoin', + listTransactions: jest.fn().mockReturnValue([]), // no history + derivationPath: ['m', "86'", "0'", "0'"], + }); + + // Testnet P2WPKH - no history + const testnetP2wpkh = mock({ addressType: 'p2wpkh', network: 'testnet', listTransactions: jest.fn().mockReturnValue([]), // no history derivationPath: ['m', "84'", "1'", "0'"], }); - const accountWithHistory2 = mock({ + // Testnet P2TR - no history + const testnetP2tr = mock({ + addressType: 'p2tr', + network: 'testnet', + listTransactions: jest.fn().mockReturnValue([]), // no history + derivationPath: ['m', "86'", "1'", "0'"], + }); + + // Signet P2WPKH - has history + const signetP2wpkh = mock({ addressType: 'p2wpkh', network: 'signet', listTransactions: jest.fn().mockReturnValue([{}]), // has 1 transaction derivationPath: ['m', "84'", "1'", "0'"], }); + // Signet P2TR - has history + const signetP2tr = mock({ + addressType: 'p2tr', + network: 'signet', + listTransactions: jest.fn().mockReturnValue([{}]), // has 1 transaction + derivationPath: ['m', "86'", "1'", "0'"], + }); + mockAccounts.discover - .mockResolvedValueOnce(accountWithHistory1) - .mockResolvedValueOnce(accountWithoutHistory) - .mockResolvedValueOnce(accountWithHistory2); + .mockResolvedValueOnce(mainnetP2wpkh) + .mockResolvedValueOnce(mainnetP2tr) + .mockResolvedValueOnce(testnetP2wpkh) + .mockResolvedValueOnce(testnetP2tr) + .mockResolvedValueOnce(signetP2wpkh) + .mockResolvedValueOnce(signetP2tr); const discovered = await handler.discoverAccounts( [BtcScope.Mainnet, BtcScope.Testnet, BtcScope.Signet], @@ -474,14 +543,49 @@ describe('KeyringHandler', () => { groupIndex, ); - expect(mockAccounts.discover).toHaveBeenCalledTimes(3); - expect(discovered).toHaveLength(2); + // 3 scopes × 2 address types = 6 discover calls + expect(mockAccounts.discover).toHaveBeenCalledTimes(6); + // 3 accounts have history: mainnetP2wpkh, signetP2wpkh, signetP2tr + expect(discovered).toHaveLength(3); expect(discovered).toStrictEqual([ - mapToDiscoveredAccount(accountWithHistory1), - mapToDiscoveredAccount(accountWithHistory2), + mapToDiscoveredAccount(mainnetP2wpkh), + mapToDiscoveredAccount(signetP2wpkh), + mapToDiscoveredAccount(signetP2tr), ]); }); + it('discovers P2TR accounts independently of P2WPKH history', async () => { + const p2wpkhNoHistory = mock({ + addressType: 'p2wpkh', + network: 'bitcoin', + listTransactions: jest.fn().mockReturnValue([]), + derivationPath: ['m', "84'", "0'", "0'"], + }); + + const p2trWithHistory = mock({ + addressType: 'p2tr', + network: 'bitcoin', + listTransactions: jest.fn().mockReturnValue([{}]), + derivationPath: ['m', "86'", "0'", "0'"], + }); + + mockAccounts.discover + .mockResolvedValueOnce(p2wpkhNoHistory) + .mockResolvedValueOnce(p2trWithHistory); + + const discovered = await handler.discoverAccounts( + [BtcScope.Mainnet], + entropySource, + groupIndex, + ); + + expect(mockAccounts.discover).toHaveBeenCalledTimes(2); + expect(discovered).toHaveLength(1); + expect(discovered[0]).toStrictEqual( + mapToDiscoveredAccount(p2trWithHistory), + ); + }); + it('propagates errors from discover', async () => { const error = new Error('discover error'); mockAccounts.discover.mockRejectedValue(error); @@ -489,6 +593,7 @@ describe('KeyringHandler', () => { await expect( handler.discoverAccounts(scopes, entropySource, groupIndex), ).rejects.toThrow(error); + expect(mockAccounts.discover).toHaveBeenCalled(); }); }); @@ -504,6 +609,7 @@ describe('KeyringHandler', () => { }; const result = await handler.getAccountBalances(mockAccount.id); + expect(mockAccounts.get).toHaveBeenCalledWith(mockAccount.id); expect(result).toStrictEqual(expectedResponse); }); @@ -541,12 +647,14 @@ describe('KeyringHandler', () => { }; const result = await handler.getAccount('some-id'); + expect(mockAccounts.get).toHaveBeenCalledWith('some-id'); expect(result).toStrictEqual(expectedKeyringAccount); }); it('propagates errors from get', async () => { const error = new Error(); + mockAccounts.get.mockRejectedValue(error); await expect(handler.getAccount('some-id')).rejects.toThrow(error); @@ -578,6 +686,7 @@ describe('KeyringHandler', () => { ]; const result = await handler.listAccounts(); + expect(mockAccounts.list).toHaveBeenCalled(); expect(result).toStrictEqual(expectedKeyringAccounts); }); @@ -587,6 +696,7 @@ describe('KeyringHandler', () => { mockAccounts.list.mockRejectedValue(error); await expect(handler.listAccounts()).rejects.toThrow(error); + expect(mockAccounts.list).toHaveBeenCalled(); }); }); @@ -598,6 +708,7 @@ describe('KeyringHandler', () => { const result = await handler.filterAccountChains('some-id', [ BtcScope.Mainnet, ]); + expect(mockAccounts.get).toHaveBeenCalledWith('some-id'); expect(result).toStrictEqual([BtcScope.Mainnet]); }); @@ -608,6 +719,7 @@ describe('KeyringHandler', () => { const result = await handler.filterAccountChains('some-id', [ BtcScope.Testnet, ]); + expect(mockAccounts.get).toHaveBeenCalledWith('some-id'); expect(result).toStrictEqual([]); }); @@ -626,6 +738,7 @@ describe('KeyringHandler', () => { describe('deleteAccount', () => { it('deletes account', async () => { await handler.deleteAccount('some-id'); + expect(mockAccounts.delete).toHaveBeenCalledWith('some-id'); }); @@ -634,6 +747,7 @@ describe('KeyringHandler', () => { mockAccounts.delete.mockRejectedValue(error); await expect(handler.deleteAccount('some-id')).rejects.toThrow(error); + expect(mockAccounts.delete).toHaveBeenCalled(); }); }); @@ -664,6 +778,7 @@ describe('KeyringHandler', () => { mockAccounts.get.mockRejectedValue(error); await expect(handler.listAccountAssets('some-id')).rejects.toThrow(error); + expect(mockAccounts.get).toHaveBeenCalled(); }); }); diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index 14e2b2497..23651499d 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -185,18 +185,9 @@ export class KeyringHandler implements Keyring { assert(options, CreateAccountRequest); const traceName = 'Create Bitcoin Account'; - let traceStarted = false; + const traceStarted = await this.#safeStartTrace(traceName); try { - await runSnapActionSafely( - async () => { - await this.#snapClient.startTrace(traceName); - traceStarted = true; - }, - this.#logger, - 'startTrace', - ); - const { metamask, scope, @@ -208,52 +199,21 @@ export class KeyringHandler implements Keyring { accountNameSuggestion, } = options; - let resolvedIndex = derivationPath + const extractedIndex = 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, - ); + const resolvedAddressType = this.#resolveAddressType( + addressType, + derivationPath, + ); - resolvedIndex = this.#getLowestUnusedIndex(accounts); - } + const resolvedIndex = await this.#resolveAccountIndex( + extractedIndex, + entropySource, + scope, + resolvedAddressType, + ); const account = await this.#accountsUseCases.create({ network: scopeToNetwork[scope], @@ -268,11 +228,7 @@ export class KeyringHandler implements Keyring { return mapToKeyringAccount(account); } finally { if (traceStarted) { - await runSnapActionSafely( - async () => this.#snapClient.endTrace(traceName), - this.#logger, - 'endTrace', - ); + await this.#safeEndTrace(traceName); } } } @@ -284,8 +240,8 @@ export class KeyringHandler implements Keyring { ): Promise { const accounts = await Promise.all( scopes.flatMap((scope) => - // only discover P2WPKH addresses - [BtcAccountType.P2wpkh].map(async (addressType) => + // discover P2WPKH and P2TR addresses + [BtcAccountType.P2wpkh, BtcAccountType.P2tr].map(async (addressType) => this.#accountsUseCases.discover({ network: scopeToNetwork[scope], entropySource, @@ -386,6 +342,87 @@ export class KeyringHandler implements Keyring { }); } + async #safeStartTrace(traceName: string): Promise { + let started = false; + await runSnapActionSafely( + async () => { + await this.#snapClient.startTrace(traceName); + started = true; + }, + this.#logger, + 'startTrace', + ); + return started; + } + + async #safeEndTrace(traceName: string): Promise { + await runSnapActionSafely( + async () => this.#snapClient.endTrace(traceName), + this.#logger, + 'endTrace', + ); + } + + async #resolveAccountIndex( + index: number | undefined | null, + entropySource: string, + scope: BtcScope, + addressType: AddressType, + ): Promise { + // FIXME: This fallback should be removed ASAP as the index should always + // be defined or be 0. Auto-incrementing creates concurrency, idempotency, + // and dangling state issues. + if (index !== undefined && index !== null) { + return index; + } + + const accounts = (await this.#accountsUseCases.list()).filter( + (acc) => + acc.entropySource === entropySource && + acc.network === scopeToNetwork[scope] && + acc.addressType === addressType, + ); + + return this.#getLowestUnusedIndex(accounts); + } + + #resolveAddressType( + addressType: BtcAccountType | undefined, + derivationPath: string | undefined, + ): AddressType { + // Case 1: explicit addressType provided + if (addressType) { + const resolved = caipToAddressType[addressType]; + this.#assertSupportedAddressType(resolved); + + if (derivationPath) { + const pathAddressType = this.#extractAddressType(derivationPath); + if (pathAddressType !== resolved) { + throw new FormatError('Address type and derivation path mismatch'); + } + } + + return resolved; + } + + // Case 2: infer from derivationPath + if (derivationPath) { + return this.#extractAddressType(derivationPath); + } + + // Case 3: fall back to default + this.#assertSupportedAddressType(this.#defaultAddressType); + return this.#defaultAddressType; + } + + #assertSupportedAddressType(addressType: AddressType): void { + if (addressType !== 'p2wpkh' && addressType !== 'p2tr') { + throw new FormatError( + 'Only native segwit (P2WPKH) and taproot (P2TR) addresses are supported', + ); + } + } + #extractAddressType(path: string): AddressType { const segments = path.split('/'); if (segments.length < 4) { @@ -404,10 +441,13 @@ export class KeyringHandler implements Keyring { throw new FormatError(`Invalid BIP-purpose: ${purpose}`); } - // only support native segwit (BIP-84) derivation paths for now - if ((purpose as Purpose) !== Purpose.NativeSegwit) { + // support native segwit (BIP-84) and taproot (BIP-86) derivation paths + if ( + (purpose as Purpose) !== Purpose.NativeSegwit && + (purpose as Purpose) !== Purpose.Taproot + ) { throw new FormatError( - `Only native segwit (BIP-84) derivation paths are supported`, + `Only native segwit (BIP-84) and taproot (BIP-86) derivation paths are supported`, ); }