From 886300b335f625638bbc5ce4d2dd7c14e2185bf6 Mon Sep 17 00:00:00 2001 From: Frederic HENG Date: Tue, 20 Jan 2026 14:54:23 +0100 Subject: [PATCH 01/17] feat: support Taproot account in KeyringHandler.ts --- packages/snap/src/handlers/KeyringHandler.ts | 28 ++++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index 14e2b2497..04e7dc9ad 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -214,10 +214,13 @@ export class KeyringHandler implements Keyring { let resolvedAddressType: AddressType; if (addressType) { - // only support P2WPKH addresses for v1 - if (addressType !== BtcAccountType.P2wpkh) { + // Support P2WPKH (native segwit) and P2TR (taproot) addresses + if ( + addressType !== BtcAccountType.P2wpkh && + addressType !== BtcAccountType.P2tr + ) { throw new FormatError( - 'Only native segwit (P2WPKH) addresses are supported', + 'Only native segwit (P2WPKH) and taproot (P2TR) addresses are supported', ); } resolvedAddressType = caipToAddressType[addressType]; @@ -233,10 +236,10 @@ export class KeyringHandler implements Keyring { resolvedAddressType = this.#extractAddressType(derivationPath); } else { resolvedAddressType = this.#defaultAddressType; - // validate default address type is P2WPKH just to be sure - if (resolvedAddressType !== 'p2wpkh') { + // validate default address type is P2WPKH or P2TR + if (resolvedAddressType !== 'p2wpkh' && resolvedAddressType !== 'p2tr') { throw new FormatError( - 'Only native segwit (P2WPKH) addresses are supported', + 'Only native segwit (P2WPKH) and taproot (P2TR) addresses are supported', ); } } @@ -284,8 +287,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, @@ -404,10 +407,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`, ); } From 447f0e576f7af2f559a09d1745f5d7d34ab45687 Mon Sep 17 00:00:00 2001 From: Frederic HENG Date: Wed, 21 Jan 2026 10:19:27 +0100 Subject: [PATCH 02/17] test: fix Taproot compatibility current KeyringHandler.ts unit tests --- .../snap/src/handlers/KeyringHandler.test.ts | 74 ++++++++++++++----- packages/snap/src/handlers/KeyringHandler.ts | 5 +- 2 files changed, 60 insertions(+), 19 deletions(-) diff --git a/packages/snap/src/handlers/KeyringHandler.test.ts b/packages/snap/src/handlers/KeyringHandler.test.ts index b2f0047f5..0513eff2e 100644 --- a/packages/snap/src/handlers/KeyringHandler.test.ts +++ b/packages/snap/src/handlers/KeyringHandler.test.ts @@ -279,7 +279,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', ), ); }); @@ -395,18 +395,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 +427,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 +445,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,11 +509,14 @@ 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), ]); }); diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index 04e7dc9ad..40de23f38 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -237,7 +237,10 @@ export class KeyringHandler implements Keyring { } else { resolvedAddressType = this.#defaultAddressType; // validate default address type is P2WPKH or P2TR - if (resolvedAddressType !== 'p2wpkh' && resolvedAddressType !== 'p2tr') { + if ( + resolvedAddressType !== 'p2wpkh' && + resolvedAddressType !== 'p2tr' + ) { throw new FormatError( 'Only native segwit (P2WPKH) and taproot (P2TR) addresses are supported', ); From 3764391a610c4ab796daf1a80f85c120547ccd2e Mon Sep 17 00:00:00 2001 From: Frederic HENG Date: Wed, 21 Jan 2026 10:49:08 +0100 Subject: [PATCH 03/17] test: add Taproot KeyringHandler unit tests --- .../snap/src/handlers/KeyringHandler.test.ts | 153 +++++++++++++++++- 1 file changed, 151 insertions(+), 2 deletions(-) diff --git a/packages/snap/src/handlers/KeyringHandler.test.ts b/packages/snap/src/handlers/KeyringHandler.test.ts index 0513eff2e..13bc79caa 100644 --- a/packages/snap/src/handlers/KeyringHandler.test.ts +++ b/packages/snap/src/handlers/KeyringHandler.test.ts @@ -197,11 +197,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, @@ -302,6 +305,120 @@ describe('KeyringHandler', () => { 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); @@ -520,6 +637,38 @@ describe('KeyringHandler', () => { ]); }); + 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); From 0a69195a737f4dd18ab16af1aacb0a976d8efa5f Mon Sep 17 00:00:00 2001 From: Frederic HENG Date: Wed, 21 Jan 2026 11:00:27 +0100 Subject: [PATCH 04/17] test: remove obsolete skipped tests and unused imports --- .../snap/src/handlers/KeyringHandler.test.ts | 91 +------------------ 1 file changed, 1 insertion(+), 90 deletions(-) diff --git a/packages/snap/src/handlers/KeyringHandler.test.ts b/packages/snap/src/handlers/KeyringHandler.test.ts index 13bc79caa..e7c8663fc 100644 --- a/packages/snap/src/handlers/KeyringHandler.test.ts +++ b/packages/snap/src/handlers/KeyringHandler.test.ts @@ -17,7 +17,6 @@ import type { } from '@metamask/keyring-api'; import { BtcAccountType, BtcScope } from '@metamask/keyring-api'; import { mock } from 'jest-mock-extended'; -import { assert } from 'superstruct'; import type { BitcoinAccount, Logger, SnapClient } from '../entities'; import { @@ -27,7 +26,7 @@ import { FormatError, } from '../entities'; import { scopeToNetwork, caipToAddressType, Caip19Asset } from './caip'; -import { KeyringHandler, CreateAccountRequest } from './KeyringHandler'; +import { KeyringHandler } from './KeyringHandler'; import type { KeyringRequestHandler } from './KeyringRequestHandler'; import { mapToDiscoveredAccount } from './mappings'; import type { @@ -87,68 +86,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([ @@ -223,32 +160,6 @@ describe('KeyringHandler', () => { }, ); - // 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(mockAccounts.create).toHaveBeenCalledWith(expectedCreateParams); - }, - ); - it('fails if derivationPath is invalid', async () => { const options = { scope: BtcScope.Signet, From 4984a509a954a41d11456baa1d0dfbb5d14865bc Mon Sep 17 00:00:00 2001 From: Frederic HENG Date: Wed, 21 Jan 2026 16:39:17 +0100 Subject: [PATCH 05/17] test: fix current integration tests compatibility --- packages/snap/integration-test/constants.ts | 7 + .../snap/integration-test/keyring.test.ts | 157 +++++++++--------- 2 files changed, 88 insertions(+), 76 deletions(-) diff --git a/packages/snap/integration-test/constants.ts b/packages/snap/integration-test/constants.ts index e19d50f10..a7a5dfe5a 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), 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, ); From 38afe7ad22ecc23d481712b2a592fb2aa4088aa1 Mon Sep 17 00:00:00 2001 From: Frederic HENG Date: Wed, 21 Jan 2026 18:55:19 +0100 Subject: [PATCH 06/17] test: add Taproot integration tests --- .../snap/integration-test/taproot.test.ts | 573 ++++++++++++++++++ 1 file changed, 573 insertions(+) create mode 100644 packages/snap/integration-test/taproot.test.ts diff --git a/packages/snap/integration-test/taproot.test.ts b/packages/snap/integration-test/taproot.test.ts new file mode 100644 index 000000000..d8244230b --- /dev/null +++ b/packages/snap/integration-test/taproot.test.ts @@ -0,0 +1,573 @@ +import type { KeyringAccount, KeyringRequest } 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 } from './constants'; +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, + }, + } as KeyringRequest, + }); + + 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, + }, + } as KeyringRequest, + }); + + const utxos = ( + response.response as { result: { result: { outpoint: string }[] } } + ).result.result; + + // 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: utxos[0]?.outpoint, + }, + }, + } as KeyringRequest, + }); + + expect(response).toRespondWith({ + pending: false, + result: expect.objectContaining({ + outpoint: utxos[0]?.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, + }, + } as KeyringRequest, + }); + + 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 () => { + // Create a simple PSBT template that sends to a P2WPKH address + const templatePsbt = getTemplatePsbt(); + + 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: templatePsbt, + feeRate: 3, + }, + }, + } as KeyringRequest, + }); + + expect(response).toRespondWith({ + pending: false, + result: { + psbt: expect.any(String), + }, + }); + }); + + it('signs a PSBT with Schnorr signature (fill and sign)', async () => { + const templatePsbt = getTemplatePsbt(); + + 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: templatePsbt, + feeRate: 3, + options: { + fill: true, + broadcast: false, + }, + }, + }, + } as KeyringRequest, + }); + + expect(response).toRespondWith({ + pending: false, + result: { + psbt: expect.any(String), + txid: null, + }, + }); + }); + + it('signs and broadcasts a PSBT from P2TR account', async () => { + const templatePsbt = getTemplatePsbt(); + + 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: templatePsbt, + feeRate: 3, + options: { + fill: true, + broadcast: true, + }, + }, + }, + } as KeyringRequest, + }); + + 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 templatePsbt = getTemplatePsbt(); + + 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: templatePsbt, + feeRate: 3, + }, + }, + } as KeyringRequest, + }); + + 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, + }, + }, + } as KeyringRequest, + }); + + 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 () => { + 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: [ + { + // Another P2TR address (bcrt1p...) + address: + 'bcrt1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vqc8gma6', + amount: '1000', + }, + ], + feeRate: 3, + }, + }, + } as KeyringRequest, + }); + + 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, + }, + }, + } as KeyringRequest, + }); + + 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!', + }, + }, + } as KeyringRequest, + }); + + 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: '', + }, + }, + } as KeyringRequest, + }); + + 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 () => { + const templatePsbt = getTemplatePsbt(); + + // 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: templatePsbt, + feeRate: 3, + options: { + fill: true, + broadcast: false, + }, + }, + }, + } as KeyringRequest, + }); + + 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, + }, + }, + } as KeyringRequest, + }); + + expect(response).toRespondWith({ + pending: false, + result: { + txid: expect.any(String), + }, + }); + }); + }); + + /** + * Returns a valid PSBT template for testing. + * This template has outputs defined but inputs will be filled by the snap + * when using `fill: true`. The same template works for both P2WPKH and P2TR + * accounts since the fill operation adds appropriate inputs. + * + * @returns A base64-encoded PSBT template string. + */ + function getTemplatePsbt(): string { + // This is the same PSBT template used in keyring-request.test.ts + // It has outputs to P2WPKH addresses and placeholder inputs that get replaced + // when fill: true is used. Decoded structure: + // - 3 placeholder inputs (will be replaced by account UTXOs) + // - Output: 30250 sats to bcrt1qstku2y3pfh9av50lxj55arm8r5gj8tf2yv5nxz + return 'cHNidP8BAI4CAAAAAAM1gwEAAAAAACJRIORP1Ndiq325lSC/jMG0RlhATHYmuuULfXgEHUM3u5i4AAAAAAAAAAAxai8AAUSx+i9Igg4HWdcpyagCs8mzuRCklgA7nRMkm69rAAAAAAAAAAAAAQACAAAAACp2AAAAAAAAFgAUgpMvYEJ/dp36svRJyRtNnpSo7bQAAAAAAAAAAAA='; + } +}); From 773784a72e9c81f7edc930797fe9a699c9dc7b03 Mon Sep 17 00:00:00 2001 From: Frederic HENG Date: Fri, 6 Feb 2026 14:59:18 +0100 Subject: [PATCH 07/17] chore: update unit test and fix linter warning --- packages/snap/snap.manifest.json | 2 +- packages/snap/src/handlers/KeyringHandler.test.ts | 4 +++- packages/snap/src/use-cases/AccountUseCases.ts | 1 + 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 33f1320ec..52846d051 100644 --- a/packages/snap/snap.manifest.json +++ b/packages/snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snap-bitcoin-wallet.git" }, "source": { - "shasum": "vRba/rR8D/OA3+XJO4+chU8zDYgLkdA18wSnehkisn8=", + "shasum": "poyVWHcTnoDDBLAHvzBdOVAAU0JaPOeDXDGoUjJug0Y=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snap/src/handlers/KeyringHandler.test.ts b/packages/snap/src/handlers/KeyringHandler.test.ts index e7c8663fc..468878477 100644 --- a/packages/snap/src/handlers/KeyringHandler.test.ts +++ b/packages/snap/src/handlers/KeyringHandler.test.ts @@ -17,6 +17,7 @@ import type { } from '@metamask/keyring-api'; import { BtcAccountType, BtcScope } from '@metamask/keyring-api'; import { mock } from 'jest-mock-extended'; +import { assert } from 'superstruct'; import type { BitcoinAccount, Logger, SnapClient } from '../entities'; import { @@ -26,7 +27,7 @@ import { FormatError, } from '../entities'; import { scopeToNetwork, caipToAddressType, Caip19Asset } from './caip'; -import { KeyringHandler } from './KeyringHandler'; +import { CreateAccountRequest, KeyringHandler } from './KeyringHandler'; import type { KeyringRequestHandler } from './KeyringRequestHandler'; import { mapToDiscoveredAccount } from './mappings'; import type { @@ -156,6 +157,7 @@ describe('KeyringHandler', () => { }; await handler.createAccount(options); + expect(assert).toHaveBeenCalledWith(options, CreateAccountRequest); expect(mockAccounts.create).toHaveBeenCalledWith(expectedCreateParams); }, ); diff --git a/packages/snap/src/use-cases/AccountUseCases.ts b/packages/snap/src/use-cases/AccountUseCases.ts index 11d75b91a..4a42906b2 100644 --- a/packages/snap/src/use-cases/AccountUseCases.ts +++ b/packages/snap/src/use-cases/AccountUseCases.ts @@ -262,6 +262,7 @@ export class AccountUseCases { const currConfirmed = tx.chain_position.is_confirmed; const statusChanged = + // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing (prevConfirmed && !currConfirmed) || (!prevConfirmed && currConfirmed); if (statusChanged) { From 501da4f296aae2813b11cd80462cc7706c86688b Mon Sep 17 00:00:00 2001 From: Frederic HENG Date: Mon, 9 Feb 2026 21:43:37 +0100 Subject: [PATCH 08/17] chore: separate assert from act for better readability --- packages/snap/src/handlers/KeyringHandler.test.ts | 15 +++++++++++++++ packages/snap/src/use-cases/AccountUseCases.ts | 1 - 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/snap/src/handlers/KeyringHandler.test.ts b/packages/snap/src/handlers/KeyringHandler.test.ts index 468878477..308bbccb7 100644 --- a/packages/snap/src/handlers/KeyringHandler.test.ts +++ b/packages/snap/src/handlers/KeyringHandler.test.ts @@ -157,6 +157,7 @@ describe('KeyringHandler', () => { }; await handler.createAccount(options); + expect(assert).toHaveBeenCalledWith(options, CreateAccountRequest); expect(mockAccounts.create).toHaveBeenCalledWith(expectedCreateParams); }, @@ -215,6 +216,7 @@ describe('KeyringHandler', () => { }; await handler.createAccount(options); + expect(mockAccounts.create).toHaveBeenCalledWith(expectedCreateParams); }); @@ -233,6 +235,7 @@ describe('KeyringHandler', () => { }; await handler.createAccount(options); + expect(mockAccounts.create).toHaveBeenCalledWith(expectedCreateParams); }); @@ -251,6 +254,7 @@ describe('KeyringHandler', () => { }; await handler.createAccount(options); + expect(mockAccounts.create).toHaveBeenCalledWith(expectedCreateParams); }); @@ -589,6 +593,7 @@ describe('KeyringHandler', () => { await expect( handler.discoverAccounts(scopes, entropySource, groupIndex), ).rejects.toThrow(error); + expect(mockAccounts.discover).toHaveBeenCalled(); }); }); @@ -604,6 +609,7 @@ describe('KeyringHandler', () => { }; const result = await handler.getAccountBalances(mockAccount.id); + expect(mockAccounts.get).toHaveBeenCalledWith(mockAccount.id); expect(result).toStrictEqual(expectedResponse); }); @@ -641,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); @@ -678,6 +686,7 @@ describe('KeyringHandler', () => { ]; const result = await handler.listAccounts(); + expect(mockAccounts.list).toHaveBeenCalled(); expect(result).toStrictEqual(expectedKeyringAccounts); }); @@ -687,6 +696,7 @@ describe('KeyringHandler', () => { mockAccounts.list.mockRejectedValue(error); await expect(handler.listAccounts()).rejects.toThrow(error); + expect(mockAccounts.list).toHaveBeenCalled(); }); }); @@ -698,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]); }); @@ -708,6 +719,7 @@ describe('KeyringHandler', () => { const result = await handler.filterAccountChains('some-id', [ BtcScope.Testnet, ]); + expect(mockAccounts.get).toHaveBeenCalledWith('some-id'); expect(result).toStrictEqual([]); }); @@ -726,6 +738,7 @@ describe('KeyringHandler', () => { describe('deleteAccount', () => { it('deletes account', async () => { await handler.deleteAccount('some-id'); + expect(mockAccounts.delete).toHaveBeenCalledWith('some-id'); }); @@ -734,6 +747,7 @@ describe('KeyringHandler', () => { mockAccounts.delete.mockRejectedValue(error); await expect(handler.deleteAccount('some-id')).rejects.toThrow(error); + expect(mockAccounts.delete).toHaveBeenCalled(); }); }); @@ -764,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/use-cases/AccountUseCases.ts b/packages/snap/src/use-cases/AccountUseCases.ts index 4a42906b2..11d75b91a 100644 --- a/packages/snap/src/use-cases/AccountUseCases.ts +++ b/packages/snap/src/use-cases/AccountUseCases.ts @@ -262,7 +262,6 @@ export class AccountUseCases { const currConfirmed = tx.chain_position.is_confirmed; const statusChanged = - // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing (prevConfirmed && !currConfirmed) || (!prevConfirmed && currConfirmed); if (statusChanged) { From 81a37f37405872e150dcca8590c80c7a09f9fd20 Mon Sep 17 00:00:00 2001 From: Frederic HENG Date: Mon, 9 Feb 2026 22:05:13 +0100 Subject: [PATCH 09/17] chore: add TEMPLATE_PSBT as a shared exported constant for integration tests --- packages/snap/integration-test/constants.ts | 4 ++ .../integration-test/keyring-request.test.ts | 17 +------- .../snap/integration-test/taproot.test.ts | 40 +++---------------- 3 files changed, 11 insertions(+), 50 deletions(-) diff --git a/packages/snap/integration-test/constants.ts b/packages/snap/integration-test/constants.ts index a7a5dfe5a..7f8cd45f3 100644 --- a/packages/snap/integration-test/constants.ts +++ b/packages/snap/integration-test/constants.ts @@ -57,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..eec15a36f 100644 --- a/packages/snap/integration-test/keyring-request.test.ts +++ b/packages/snap/integration-test/keyring-request.test.ts @@ -4,7 +4,7 @@ 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 { AccountCapability } from '../src/entities'; import type { FillPsbtResponse } from '../src/handlers/KeyringRequestHandler'; @@ -203,9 +203,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=='; @@ -366,10 +363,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, @@ -428,10 +421,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, @@ -490,10 +479,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({ diff --git a/packages/snap/integration-test/taproot.test.ts b/packages/snap/integration-test/taproot.test.ts index d8244230b..30e09b3a5 100644 --- a/packages/snap/integration-test/taproot.test.ts +++ b/packages/snap/integration-test/taproot.test.ts @@ -4,7 +4,7 @@ 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 { AccountCapability } from '../src/entities'; import type { FillPsbtResponse } from '../src/handlers/KeyringRequestHandler'; @@ -185,9 +185,6 @@ describe('Taproot (P2TR) Integration Tests', () => { describe('PSBT Signing (Schnorr)', () => { it('fills a PSBT for P2TR account', async () => { - // Create a simple PSBT template that sends to a P2WPKH address - const templatePsbt = getTemplatePsbt(); - const response = await snap.onKeyringRequest({ origin: ORIGIN, method: submitRequestMethod, @@ -199,7 +196,7 @@ describe('Taproot (P2TR) Integration Tests', () => { request: { method: AccountCapability.FillPsbt, params: { - psbt: templatePsbt, + psbt: TEMPLATE_PSBT, feeRate: 3, }, }, @@ -215,8 +212,6 @@ describe('Taproot (P2TR) Integration Tests', () => { }); it('signs a PSBT with Schnorr signature (fill and sign)', async () => { - const templatePsbt = getTemplatePsbt(); - const response = await snap.onKeyringRequest({ origin: ORIGIN, method: submitRequestMethod, @@ -228,7 +223,7 @@ describe('Taproot (P2TR) Integration Tests', () => { request: { method: AccountCapability.SignPsbt, params: { - psbt: templatePsbt, + psbt: TEMPLATE_PSBT, feeRate: 3, options: { fill: true, @@ -249,8 +244,6 @@ describe('Taproot (P2TR) Integration Tests', () => { }); it('signs and broadcasts a PSBT from P2TR account', async () => { - const templatePsbt = getTemplatePsbt(); - const response = await snap.onKeyringRequest({ origin: ORIGIN, method: submitRequestMethod, @@ -262,7 +255,7 @@ describe('Taproot (P2TR) Integration Tests', () => { request: { method: AccountCapability.SignPsbt, params: { - psbt: templatePsbt, + psbt: TEMPLATE_PSBT, feeRate: 3, options: { fill: true, @@ -286,8 +279,6 @@ describe('Taproot (P2TR) Integration Tests', () => { }); it('computes fee for P2TR transaction', async () => { - const templatePsbt = getTemplatePsbt(); - const response = await snap.onKeyringRequest({ origin: ORIGIN, method: submitRequestMethod, @@ -299,7 +290,7 @@ describe('Taproot (P2TR) Integration Tests', () => { request: { method: AccountCapability.ComputeFee, params: { - psbt: templatePsbt, + psbt: TEMPLATE_PSBT, feeRate: 3, }, }, @@ -498,8 +489,6 @@ describe('Taproot (P2TR) Integration Tests', () => { describe('Broadcast', () => { it('broadcasts a signed P2TR transaction', async () => { - const templatePsbt = getTemplatePsbt(); - // First sign the PSBT without broadcasting let response = await snap.onKeyringRequest({ origin: ORIGIN, @@ -512,7 +501,7 @@ describe('Taproot (P2TR) Integration Tests', () => { request: { method: AccountCapability.SignPsbt, params: { - psbt: templatePsbt, + psbt: TEMPLATE_PSBT, feeRate: 3, options: { fill: true, @@ -553,21 +542,4 @@ describe('Taproot (P2TR) Integration Tests', () => { }); }); }); - - /** - * Returns a valid PSBT template for testing. - * This template has outputs defined but inputs will be filled by the snap - * when using `fill: true`. The same template works for both P2WPKH and P2TR - * accounts since the fill operation adds appropriate inputs. - * - * @returns A base64-encoded PSBT template string. - */ - function getTemplatePsbt(): string { - // This is the same PSBT template used in keyring-request.test.ts - // It has outputs to P2WPKH addresses and placeholder inputs that get replaced - // when fill: true is used. Decoded structure: - // - 3 placeholder inputs (will be replaced by account UTXOs) - // - Output: 30250 sats to bcrt1qstku2y3pfh9av50lxj55arm8r5gj8tf2yv5nxz - return 'cHNidP8BAI4CAAAAAAM1gwEAAAAAACJRIORP1Ndiq325lSC/jMG0RlhATHYmuuULfXgEHUM3u5i4AAAAAAAAAAAxai8AAUSx+i9Igg4HWdcpyagCs8mzuRCklgA7nRMkm69rAAAAAAAAAAAAAQACAAAAACp2AAAAAAAAFgAUgpMvYEJ/dp36svRJyRtNnpSo7bQAAAAAAAAAAAA='; - } }); From 40f62432cd65aeb3e08eb8c0a59c8d49f20e824c Mon Sep 17 00:00:00 2001 From: Frederic HENG Date: Mon, 9 Feb 2026 23:25:30 +0100 Subject: [PATCH 10/17] fix: remove KeyringRequest typecast from integration tests --- .../integration-test/keyring-request.test.ts | 46 ++++++++++--------- .../snap/integration-test/taproot.test.ts | 38 +++++++-------- .../snap/integration-test/test-helpers.ts | 7 +++ 3 files changed, 51 insertions(+), 40 deletions(-) create mode 100644 packages/snap/integration-test/test-helpers.ts diff --git a/packages/snap/integration-test/keyring-request.test.ts b/packages/snap/integration-test/keyring-request.test.ts index eec15a36f..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, 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({ @@ -226,7 +228,7 @@ describe('KeyringRequestHandler', () => { }, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -258,7 +260,7 @@ describe('KeyringRequestHandler', () => { }, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -290,7 +292,7 @@ describe('KeyringRequestHandler', () => { }, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -321,7 +323,7 @@ describe('KeyringRequestHandler', () => { }, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWithError({ @@ -350,7 +352,7 @@ describe('KeyringRequestHandler', () => { psbt: TEMPLATE_PSBT, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWithError({ @@ -379,7 +381,7 @@ describe('KeyringRequestHandler', () => { feeRate: 3, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -405,7 +407,7 @@ describe('KeyringRequestHandler', () => { psbt: 'notAPsbt', }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWithError({ @@ -437,7 +439,7 @@ describe('KeyringRequestHandler', () => { feeRate: 3, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -463,7 +465,7 @@ describe('KeyringRequestHandler', () => { psbt: 'notAPsbt', }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWithError({ @@ -500,7 +502,7 @@ describe('KeyringRequestHandler', () => { }, }, }, - } as KeyringRequest, + }, }); const { result } = ( @@ -521,7 +523,7 @@ describe('KeyringRequestHandler', () => { psbt: result.psbt, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -547,7 +549,7 @@ describe('KeyringRequestHandler', () => { psbt: 'notAPsbt', }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWithError({ @@ -588,7 +590,7 @@ describe('KeyringRequestHandler', () => { feeRate: 3, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -614,7 +616,7 @@ describe('KeyringRequestHandler', () => { recipients: [{ address: 'notAnAddress', amount: '1000' }], }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWithError({ @@ -642,7 +644,7 @@ describe('KeyringRequestHandler', () => { message: 'Hello, world!', }, }, - } as KeyringRequest, + }, }); const ui = await response.getInterface(); diff --git a/packages/snap/integration-test/taproot.test.ts b/packages/snap/integration-test/taproot.test.ts index 30e09b3a5..3686e2bb5 100644 --- a/packages/snap/integration-test/taproot.test.ts +++ b/packages/snap/integration-test/taproot.test.ts @@ -1,10 +1,11 @@ -import type { KeyringAccount, KeyringRequest } from '@metamask/keyring-api'; +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'; @@ -99,7 +100,7 @@ describe('Taproot (P2TR) Integration Tests', () => { request: { method: AccountCapability.ListUtxos, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -126,12 +127,13 @@ describe('Taproot (P2TR) Integration Tests', () => { request: { method: AccountCapability.ListUtxos, }, - } as KeyringRequest, + }, }); const utxos = ( response.response as { result: { result: { outpoint: string }[] } } ).result.result; + const outpoint = getRequiredOutpoint(utxos); // Then get a specific UTXO response = await snap.onKeyringRequest({ @@ -145,16 +147,16 @@ describe('Taproot (P2TR) Integration Tests', () => { request: { method: AccountCapability.GetUtxo, params: { - outpoint: utxos[0]?.outpoint, + outpoint, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ pending: false, result: expect.objectContaining({ - outpoint: utxos[0]?.outpoint, + outpoint, address: expect.stringMatching(/^bcrt1p/u), }), }); @@ -172,7 +174,7 @@ describe('Taproot (P2TR) Integration Tests', () => { request: { method: AccountCapability.PublicDescriptor, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -200,7 +202,7 @@ describe('Taproot (P2TR) Integration Tests', () => { feeRate: 3, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -231,7 +233,7 @@ describe('Taproot (P2TR) Integration Tests', () => { }, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -263,7 +265,7 @@ describe('Taproot (P2TR) Integration Tests', () => { }, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -294,7 +296,7 @@ describe('Taproot (P2TR) Integration Tests', () => { feeRate: 3, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -329,7 +331,7 @@ describe('Taproot (P2TR) Integration Tests', () => { feeRate: 3, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -366,7 +368,7 @@ describe('Taproot (P2TR) Integration Tests', () => { feeRate: 3, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -405,7 +407,7 @@ describe('Taproot (P2TR) Integration Tests', () => { feeRate: 3, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ @@ -436,7 +438,7 @@ describe('Taproot (P2TR) Integration Tests', () => { message: 'Hello, Taproot!', }, }, - } as KeyringRequest, + }, }); const ui = await response.getInterface(); @@ -469,7 +471,7 @@ describe('Taproot (P2TR) Integration Tests', () => { message: '', }, }, - } as KeyringRequest, + }, }); const ui = await response.getInterface(); @@ -509,7 +511,7 @@ describe('Taproot (P2TR) Integration Tests', () => { }, }, }, - } as KeyringRequest, + }, }); const { result } = ( @@ -531,7 +533,7 @@ describe('Taproot (P2TR) Integration Tests', () => { psbt: result.psbt, }, }, - } as KeyringRequest, + }, }); expect(response).toRespondWith({ 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; +}; From 03810620345447409469459775f246c442341396 Mon Sep 17 00:00:00 2001 From: Frederic HENG Date: Tue, 10 Feb 2026 11:43:25 +0100 Subject: [PATCH 11/17] refactor: extract trace helpers from createAccount --- packages/snap/src/handlers/KeyringHandler.ts | 38 ++++++++++++-------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index 40de23f38..072631bf2 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, @@ -274,11 +265,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); } } } @@ -392,6 +379,27 @@ 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', + ); + } + #extractAddressType(path: string): AddressType { const segments = path.split('/'); if (segments.length < 4) { From 8dd78951eca00d27ad0b3b4bcb3c89c1ac19ce0f Mon Sep 17 00:00:00 2001 From: Frederic HENG Date: Tue, 10 Feb 2026 11:56:45 +0100 Subject: [PATCH 12/17] refactor: extract to #assertSupportedAddressType to deduplicate validation --- packages/snap/src/handlers/KeyringHandler.ts | 28 +++++++------------- 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index 072631bf2..e222e5708 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -205,16 +205,8 @@ export class KeyringHandler implements Keyring { let resolvedAddressType: AddressType; if (addressType) { - // Support P2WPKH (native segwit) and P2TR (taproot) addresses - if ( - addressType !== BtcAccountType.P2wpkh && - addressType !== BtcAccountType.P2tr - ) { - throw new FormatError( - 'Only native segwit (P2WPKH) and taproot (P2TR) addresses are supported', - ); - } resolvedAddressType = caipToAddressType[addressType]; + this.#assertSupportedAddressType(resolvedAddressType); // if both addressType and derivationPath are provided, validate they match if (derivationPath) { @@ -227,15 +219,7 @@ export class KeyringHandler implements Keyring { resolvedAddressType = this.#extractAddressType(derivationPath); } else { resolvedAddressType = this.#defaultAddressType; - // validate default address type is P2WPKH or P2TR - if ( - resolvedAddressType !== 'p2wpkh' && - resolvedAddressType !== 'p2tr' - ) { - throw new FormatError( - 'Only native segwit (P2WPKH) and taproot (P2TR) addresses are supported', - ); - } + this.#assertSupportedAddressType(resolvedAddressType); } // FIXME: This if should be removed ASAP as the index should always be defined or be 0 @@ -379,6 +363,14 @@ export class KeyringHandler implements Keyring { }); } + #assertSupportedAddressType(addressType: AddressType): void { + if (addressType !== 'p2wpkh' && addressType !== 'p2tr') { + throw new FormatError( + 'Only native segwit (P2WPKH) and taproot (P2TR) addresses are supported', + ); + } + } + async #safeStartTrace(traceName: string): Promise { let started = false; await runSnapActionSafely( From 278273f56ee855647f9a08229a0f889551d90813 Mon Sep 17 00:00:00 2001 From: Frederic HENG Date: Tue, 10 Feb 2026 12:14:30 +0100 Subject: [PATCH 13/17] refactor: extract to #resolveAccountIndex method from createAccount --- packages/snap/src/handlers/KeyringHandler.ts | 60 +++++++++++++------- 1 file changed, 38 insertions(+), 22 deletions(-) diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index e222e5708..e0eea029f 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -199,7 +199,7 @@ export class KeyringHandler implements Keyring { accountNameSuggestion, } = options; - let resolvedIndex = derivationPath + const extractedIndex = derivationPath ? this.#extractAccountIndex(derivationPath) : index; @@ -222,19 +222,12 @@ export class KeyringHandler implements Keyring { this.#assertSupportedAddressType(resolvedAddressType); } - // 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 resolvedIndex = await this.#resolveAccountIndex( + extractedIndex, + entropySource, + scope, + resolvedAddressType, + ); const account = await this.#accountsUseCases.create({ network: scopeToNetwork[scope], @@ -363,14 +356,6 @@ export class KeyringHandler implements Keyring { }); } - #assertSupportedAddressType(addressType: AddressType): void { - if (addressType !== 'p2wpkh' && addressType !== 'p2tr') { - throw new FormatError( - 'Only native segwit (P2WPKH) and taproot (P2TR) addresses are supported', - ); - } - } - async #safeStartTrace(traceName: string): Promise { let started = false; await runSnapActionSafely( @@ -392,6 +377,37 @@ export class KeyringHandler implements Keyring { ); } + 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); + } + + #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) { From 9f86cf18099331a0a57c719c353b49cb7e2f81a6 Mon Sep 17 00:00:00 2001 From: Frederic HENG Date: Tue, 10 Feb 2026 12:43:30 +0100 Subject: [PATCH 14/17] refactor: extract to #resolveAddressType method to flatten createAccount with less nested conditions --- packages/snap/src/handlers/KeyringHandler.ts | 51 +++++++++++++------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index e0eea029f..23651499d 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -203,24 +203,10 @@ export class KeyringHandler implements Keyring { ? this.#extractAccountIndex(derivationPath) : index; - let resolvedAddressType: AddressType; - if (addressType) { - resolvedAddressType = caipToAddressType[addressType]; - this.#assertSupportedAddressType(resolvedAddressType); - - // 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; - this.#assertSupportedAddressType(resolvedAddressType); - } + const resolvedAddressType = this.#resolveAddressType( + addressType, + derivationPath, + ); const resolvedIndex = await this.#resolveAccountIndex( extractedIndex, @@ -400,6 +386,35 @@ export class KeyringHandler implements Keyring { 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( From 1336726c4bd04da7eb2c138fc0c0223a9ccf4101 Mon Sep 17 00:00:00 2001 From: Frederic HENG Date: Tue, 10 Feb 2026 13:13:45 +0100 Subject: [PATCH 15/17] fix: address the invalid Bech32m checksum in regtest P2TR recipient address issue --- packages/snap/integration-test/blockchain-utils.ts | 13 +++++++++++++ packages/snap/integration-test/taproot.test.ts | 8 +++++--- 2 files changed, 18 insertions(+), 3 deletions(-) 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/taproot.test.ts b/packages/snap/integration-test/taproot.test.ts index 3686e2bb5..c464265ef 100644 --- a/packages/snap/integration-test/taproot.test.ts +++ b/packages/snap/integration-test/taproot.test.ts @@ -346,6 +346,10 @@ describe('Taproot (P2TR) Integration Tests', () => { }); 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, @@ -359,9 +363,7 @@ describe('Taproot (P2TR) Integration Tests', () => { params: { recipients: [ { - // Another P2TR address (bcrt1p...) - address: - 'bcrt1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vqc8gma6', + address: p2trRecipient, amount: '1000', }, ], From 4066f32e67182f13233a0ef8f162a6a1a2937be1 Mon Sep 17 00:00:00 2001 From: Frederic HENG Date: Tue, 10 Feb 2026 14:35:55 +0100 Subject: [PATCH 16/17] fix: revert createAccount() refactoring, move it to another PR --- packages/snap/src/handlers/KeyringHandler.ts | 157 ++++++++----------- 1 file changed, 63 insertions(+), 94 deletions(-) diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index 23651499d..40de23f38 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -185,9 +185,18 @@ export class KeyringHandler implements Keyring { assert(options, CreateAccountRequest); const traceName = 'Create Bitcoin Account'; - const traceStarted = await this.#safeStartTrace(traceName); + let traceStarted = false; try { + await runSnapActionSafely( + async () => { + await this.#snapClient.startTrace(traceName); + traceStarted = true; + }, + this.#logger, + 'startTrace', + ); + const { metamask, scope, @@ -199,21 +208,58 @@ export class KeyringHandler implements Keyring { accountNameSuggestion, } = options; - const extractedIndex = derivationPath + let resolvedIndex = derivationPath ? this.#extractAccountIndex(derivationPath) : index; - const resolvedAddressType = this.#resolveAddressType( - addressType, - derivationPath, - ); + let resolvedAddressType: AddressType; + if (addressType) { + // Support P2WPKH (native segwit) and P2TR (taproot) addresses + if ( + addressType !== BtcAccountType.P2wpkh && + addressType !== BtcAccountType.P2tr + ) { + throw new FormatError( + 'Only native segwit (P2WPKH) and taproot (P2TR) 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 or P2TR + if ( + resolvedAddressType !== 'p2wpkh' && + resolvedAddressType !== 'p2tr' + ) { + throw new FormatError( + 'Only native segwit (P2WPKH) and taproot (P2TR) addresses are supported', + ); + } + } - const resolvedIndex = await this.#resolveAccountIndex( - extractedIndex, - entropySource, - scope, - resolvedAddressType, - ); + // 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], @@ -228,7 +274,11 @@ export class KeyringHandler implements Keyring { return mapToKeyringAccount(account); } finally { if (traceStarted) { - await this.#safeEndTrace(traceName); + await runSnapActionSafely( + async () => this.#snapClient.endTrace(traceName), + this.#logger, + 'endTrace', + ); } } } @@ -342,87 +392,6 @@ 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) { From 737224483d048f7be1a4736e622d547cf41dc2f0 Mon Sep 17 00:00:00 2001 From: Frederic HENG Date: Tue, 10 Feb 2026 20:20:27 +0100 Subject: [PATCH 17/17] refactor: reapply the previous refactoring of createAccount() --- packages/snap/src/handlers/KeyringHandler.ts | 157 +++++++++++-------- 1 file changed, 94 insertions(+), 63 deletions(-) diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index 40de23f38..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,58 +199,21 @@ export class KeyringHandler implements Keyring { accountNameSuggestion, } = options; - let resolvedIndex = derivationPath + const extractedIndex = derivationPath ? this.#extractAccountIndex(derivationPath) : index; - let resolvedAddressType: AddressType; - if (addressType) { - // Support P2WPKH (native segwit) and P2TR (taproot) addresses - if ( - addressType !== BtcAccountType.P2wpkh && - addressType !== BtcAccountType.P2tr - ) { - throw new FormatError( - 'Only native segwit (P2WPKH) and taproot (P2TR) 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 or P2TR - if ( - resolvedAddressType !== 'p2wpkh' && - resolvedAddressType !== 'p2tr' - ) { - throw new FormatError( - 'Only native segwit (P2WPKH) and taproot (P2TR) 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], @@ -274,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); } } } @@ -392,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) {