Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/snap/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
84 changes: 74 additions & 10 deletions packages/snap/src/handlers/KeyringHandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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',
Expand Down Expand Up @@ -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',
),
);
});
Expand All @@ -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);
Expand Down Expand Up @@ -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[] = [];
Expand Down Expand Up @@ -443,43 +481,69 @@ 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<BitcoinAccount>({
addressType: 'p2wpkh',
network: 'bitcoin',
listTransactions: jest.fn().mockReturnValue([{}, {}]), // has 2 transactions
derivationPath: ['m', "84'", "0'", "0'"],
});

const p2trNoHistory1 = mock<BitcoinAccount>({
addressType: 'p2tr',
network: 'bitcoin',
listTransactions: jest.fn().mockReturnValue([]),
derivationPath: ['m', "86'", "0'", "0'"],
});

const accountWithoutHistory = mock<BitcoinAccount>({
addressType: 'p2wpkh',
network: 'testnet',
listTransactions: jest.fn().mockReturnValue([]), // no history
derivationPath: ['m', "84'", "1'", "0'"],
});

const p2trNoHistory2 = mock<BitcoinAccount>({
addressType: 'p2tr',
network: 'testnet',
listTransactions: jest.fn().mockReturnValue([]),
derivationPath: ['m', "86'", "1'", "0'"],
});

const accountWithHistory2 = mock<BitcoinAccount>({
addressType: 'p2wpkh',
network: 'signet',
listTransactions: jest.fn().mockReturnValue([{}]), // has 1 transaction
derivationPath: ['m', "84'", "1'", "0'"],
});

const p2trWithHistory = mock<BitcoinAccount>({
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],
entropySource,
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),
]);
});

Expand Down
31 changes: 20 additions & 11 deletions packages/snap/src/handlers/KeyringHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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',
);
}
}
Expand Down Expand Up @@ -294,8 +300,8 @@ export class KeyringHandler implements Keyring {
): Promise<DiscoveredAccount[]> {
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,
Expand Down Expand Up @@ -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`,
);
}

Expand Down