From 7bd736bb7d42d7563dfc09299fca261c17948e6d Mon Sep 17 00:00:00 2001 From: Marco Peereboom Date: Wed, 22 Apr 2026 01:55:35 -0700 Subject: [PATCH] feat(keyring): enable P2TR taproot account creation Remove v1 P2WPKH-only guards from createAccount, extractAddressType, and discoverAccounts. The full P2TR pipeline (BDK descriptors, CAIP type mappings, BIP-86 derivation paths, extension UI) already exists; only the whitelist checks blocked taproot. Widen address type validation to accept P2TR alongside P2WPKH. Accept BIP-86 (purpose 86) derivation paths. Include P2TR in account discovery so existing taproot UTXOs are found during wallet setup. Test coverage: unskip existing BIP-86 vector, add P2TR derivation path extraction, P2TR-only creation, P2TR+path combined validation, and P2TR account discovery with history filtering. --- packages/snap/CHANGELOG.md | 6 ++ .../snap/src/handlers/KeyringHandler.test.ts | 84 ++++++++++++++++--- packages/snap/src/handlers/KeyringHandler.ts | 31 ++++--- 3 files changed, 100 insertions(+), 21 deletions(-) diff --git a/packages/snap/CHANGELOG.md b/packages/snap/CHANGELOG.md index c255e4028..5dc3d3126 100644 --- a/packages/snap/CHANGELOG.md +++ b/packages/snap/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Enable P2TR (taproot) account creation alongside P2WPKH +- Accept BIP-86 (purpose 86) derivation paths for taproot accounts +- Include P2TR in account discovery + ## [1.10.1] ### Fixed diff --git a/packages/snap/src/handlers/KeyringHandler.test.ts b/packages/snap/src/handlers/KeyringHandler.test.ts index b2f0047f5..52e9c2dc9 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 P2WPKH and P2TR address type from derivationPath: %s', async ({ purpose, addressType }) => { const options = { scope: BtcScope.Signet, @@ -220,11 +223,10 @@ describe('KeyringHandler', () => { }, ); - // skip non-P2WPKH address types as they are not supported on v1 + // skip non-P2WPKH/P2TR address types as they are not yet supported 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', @@ -279,7 +281,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', ), ); }); @@ -302,6 +304,42 @@ describe('KeyringHandler', () => { expect(mockAccounts.create).toHaveBeenCalledWith(expectedCreateParams); }); + it('succeeds when addressType and derivationPath both indicate P2TR', async () => { + const options = { + scope: BtcScope.Mainnet, + addressType: BtcAccountType.P2tr, + derivationPath: "m/86'/0'/0'", // BIP-86 taproot path + }; + 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 only P2TR addressType is provided', async () => { + const options = { + scope: BtcScope.Mainnet, + addressType: BtcAccountType.P2tr, + index: 3, + }; + const expectedCreateParams: CreateAccountParams = { + network: 'bitcoin', + index: 3, + addressType: 'p2tr', + entropySource: 'm', + synchronize: false, + }; + + await handler.createAccount(options); + expect(mockAccounts.create).toHaveBeenCalledWith(expectedCreateParams); + }); + it('propagates errors from createAccount', async () => { const error = new Error('createAccount error'); mockAccounts.create.mockRejectedValue(error); @@ -395,8 +433,8 @@ 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]; + // P2WPKH and P2TR are supported + const addressTypes = [BtcAccountType.P2wpkh, BtcAccountType.P2tr]; const totalCombinations = scopes.length * addressTypes.length; const expected: DiscoveredAccount[] = []; @@ -442,6 +480,7 @@ describe('KeyringHandler', () => { it('returns mix of accounts with and without history, filtering correctly', async () => { // create mock accounts - some with history, some without + // Each scope produces two discover calls (P2WPKH + P2TR) const accountWithHistory1 = mock({ addressType: 'p2wpkh', network: 'bitcoin', @@ -449,6 +488,13 @@ describe('KeyringHandler', () => { derivationPath: ['m', "84'", "0'", "0'"], }); + const p2trNoHistory1 = mock({ + addressType: 'p2tr', + network: 'bitcoin', + listTransactions: jest.fn().mockReturnValue([]), + derivationPath: ['m', "86'", "0'", "0'"], + }); + const accountWithoutHistory = mock({ addressType: 'p2wpkh', network: 'testnet', @@ -456,6 +502,13 @@ describe('KeyringHandler', () => { derivationPath: ['m', "84'", "1'", "0'"], }); + const p2trNoHistory2 = mock({ + addressType: 'p2tr', + network: 'testnet', + listTransactions: jest.fn().mockReturnValue([]), + derivationPath: ['m', "86'", "1'", "0'"], + }); + const accountWithHistory2 = mock({ addressType: 'p2wpkh', network: 'signet', @@ -463,10 +516,20 @@ describe('KeyringHandler', () => { derivationPath: ['m', "84'", "1'", "0'"], }); + const p2trWithHistory = mock({ + addressType: 'p2tr', + network: 'signet', + listTransactions: jest.fn().mockReturnValue([{}]), // has history + derivationPath: ['m', "86'", "1'", "0'"], + }); + mockAccounts.discover .mockResolvedValueOnce(accountWithHistory1) + .mockResolvedValueOnce(p2trNoHistory1) .mockResolvedValueOnce(accountWithoutHistory) - .mockResolvedValueOnce(accountWithHistory2); + .mockResolvedValueOnce(p2trNoHistory2) + .mockResolvedValueOnce(accountWithHistory2) + .mockResolvedValueOnce(p2trWithHistory); const discovered = await handler.discoverAccounts( [BtcScope.Mainnet, BtcScope.Testnet, BtcScope.Signet], @@ -474,11 +537,12 @@ describe('KeyringHandler', () => { groupIndex, ); - expect(mockAccounts.discover).toHaveBeenCalledTimes(3); - expect(discovered).toHaveLength(2); + expect(mockAccounts.discover).toHaveBeenCalledTimes(6); + expect(discovered).toHaveLength(3); expect(discovered).toStrictEqual([ mapToDiscoveredAccount(accountWithHistory1), mapToDiscoveredAccount(accountWithHistory2), + mapToDiscoveredAccount(p2trWithHistory), ]); }); diff --git a/packages/snap/src/handlers/KeyringHandler.ts b/packages/snap/src/handlers/KeyringHandler.ts index 14e2b2497..15a1a7cba 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 and P2TR address types + 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,13 @@ 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 a supported type + 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 +290,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 +410,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`, ); }