diff --git a/packages/snap/CHANGELOG.md b/packages/snap/CHANGELOG.md index bf02a33a..84b31247 100644 --- a/packages/snap/CHANGELOG.md +++ b/packages/snap/CHANGELOG.md @@ -7,6 +7,11 @@ 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 ### Changed - Show a confirmation dialog before signing a PSBT from KeyringHandler and sending a transfer ([#591](https://github.com/MetaMask/snap-bitcoin-wallet/pull/591)) diff --git a/packages/snap/src/handlers/KeyringHandler.test.ts b/packages/snap/src/handlers/KeyringHandler.test.ts index 8691a519..73480768 100644 --- a/packages/snap/src/handlers/KeyringHandler.test.ts +++ b/packages/snap/src/handlers/KeyringHandler.test.ts @@ -198,11 +198,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, @@ -221,11 +224,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', @@ -280,7 +282,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', ), ); }); @@ -303,6 +305,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); @@ -396,8 +434,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[] = []; @@ -443,6 +481,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', @@ -450,6 +489,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', @@ -457,6 +503,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', @@ -464,10 +517,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], @@ -475,11 +538,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 7fc5a764..7f703690 100644 --- a/packages/snap/src/handlers/KeyringHandler.ts +++ b/packages/snap/src/handlers/KeyringHandler.ts @@ -224,10 +224,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]; @@ -243,10 +246,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', ); } } @@ -294,8 +300,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, @@ -465,10 +471,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`, ); }