From 68714e97ebcc544ebf1807f624fbedaf0abb6cfa Mon Sep 17 00:00:00 2001 From: KTibow Date: Fri, 24 Jul 2026 15:39:31 -0700 Subject: [PATCH] fix(bitcoin): support sending to taproot (bc1p) addresses bitcoinjs-lib requires initEccLib() before it can build a P2TR output script, and nothing in the extension ever called it. address.toOutputScript() therefore threw "No ECC Library provided" for every bc1p... address, isAddress() swallowed that in its catch and returned false, and taproot recipients were rejected as invalid with no setting able to change it. Register @bitcoinerlab/secp256k1 once from a shared module and import it from the address validation and PSBT paths. That package is already in the lockfile at the same version by way of ledger-bitcoin, so this adds no new package to the bundle. Also size the recipient output from its own address type. Fee rates are quoted per vByte and floored at 1 sat/vB, so charging for a 43 vB taproot output as though it were a 31 vB P2WPKH one leaves the effective rate under the minimum relay fee and the broadcast fails. Co-Authored-By: Claude Opus 5 (1M context) --- packages/extension/package.json | 1 + .../src/providers/bitcoin/libs/init-ecc.ts | 13 +++ .../src/providers/bitcoin/libs/utils.ts | 1 + .../tests/bitcoin.address.validation.test.ts | 89 +++++++++++++++++++ .../src/providers/bitcoin/ui/libs/signer.ts | 1 + .../src/providers/bitcoin/ui/libs/tx-size.ts | 52 +++++++++-- .../bitcoin/ui/send-transaction/index.vue | 30 +++++-- .../action/views/swap/libs/bitcoin-gasvals.ts | 1 + yarn.lock | 1 + 9 files changed, 177 insertions(+), 12 deletions(-) create mode 100644 packages/extension/src/providers/bitcoin/libs/init-ecc.ts create mode 100644 packages/extension/src/providers/bitcoin/tests/bitcoin.address.validation.test.ts diff --git a/packages/extension/package.json b/packages/extension/package.json index 6a0999c11..0097877db 100644 --- a/packages/extension/package.json +++ b/packages/extension/package.json @@ -24,6 +24,7 @@ }, "dependencies": { "@amplitude/analytics-browser": "^2.42.4", + "@bitcoinerlab/secp256k1": "^1.2.0", "@enkryptcom/extension-bridge": "workspace:^", "@enkryptcom/hw-wallets": "workspace:^", "@enkryptcom/keyring": "workspace:^", diff --git a/packages/extension/src/providers/bitcoin/libs/init-ecc.ts b/packages/extension/src/providers/bitcoin/libs/init-ecc.ts new file mode 100644 index 000000000..29f25d110 --- /dev/null +++ b/packages/extension/src/providers/bitcoin/libs/init-ecc.ts @@ -0,0 +1,13 @@ +import * as ecc from '@bitcoinerlab/secp256k1'; +import { initEccLib } from 'bitcoinjs-lib'; + +/** + * bitcoinjs-lib does not bundle an elliptic curve implementation. Anything + * taproot related throws `No ECC Library provided` until one is registered, + * which makes `address.toOutputScript` reject every `bc1p...` address and, in + * turn, makes taproot recipients look invalid and unspendable to. + * + * Importing this module for its side effect registers the curve once. Import it + * from any module that validates addresses or builds output scripts. + */ +initEccLib(ecc); diff --git a/packages/extension/src/providers/bitcoin/libs/utils.ts b/packages/extension/src/providers/bitcoin/libs/utils.ts index 6da7331ca..bca9d4792 100644 --- a/packages/extension/src/providers/bitcoin/libs/utils.ts +++ b/packages/extension/src/providers/bitcoin/libs/utils.ts @@ -1,3 +1,4 @@ +import './init-ecc'; import { BitcoinNetworkInfo, HaskoinUnspentType } from '../types'; import { address as BTCAddress } from 'bitcoinjs-lib'; import { GasPriceTypes } from '@/providers/common/types'; diff --git a/packages/extension/src/providers/bitcoin/tests/bitcoin.address.validation.test.ts b/packages/extension/src/providers/bitcoin/tests/bitcoin.address.validation.test.ts new file mode 100644 index 000000000..16e47073b --- /dev/null +++ b/packages/extension/src/providers/bitcoin/tests/bitcoin.address.validation.test.ts @@ -0,0 +1,89 @@ +// @vitest-environment node +// +// bitcoinjs-lib verifies an ECC library by feeding it `Buffer.from(...)` test +// vectors, and @noble/curves rejects anything that is not a `Uint8Array`. Under +// the default jsdom environment those two come from different realms, so the +// `instanceof` check fails and `initEccLib` throws even though the library is +// fine. In a browser there is a single realm and the bundled Buffer polyfill +// extends that realm's Uint8Array, so this only affects the test harness. +import { describe, it, expect } from 'vitest'; +import bitcoinNetworks from '../networks'; +import { isAddress } from '../libs/utils'; +import { + calculateSizeBasedOnType, + getOutputCounterForAddress, +} from '../ui/libs/tx-size'; +import { PaymentType } from '../types/bitcoin-network'; + +const bitcoin = bitcoinNetworks.bitcoin.networkInfo; +const litecoin = bitcoinNetworks.litecoin.networkInfo; + +// BIP86 test vector +const TAPROOT = + 'bc1p5cyxnuxmeuwuvkwfem96lqzszd02n6xdcjrs20cac6yqjjwudpxqkedrcr'; +const NATIVE_SEGWIT = 'bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4'; +const P2SH = '3J98t1WpEZ73CNmQviecrnyiWrnqRhWNLy'; +const P2PKH = '1BvBMSEYstWetqTFn5Au4m4GFg7xJaNVN2'; +const P2WSH = 'bc1qrp33g0q5c5txsp9arysrx4k6zdkfs4nce4xj0gdcccefvpysxf3qccfmv3'; + +describe('Should validate bitcoin recipient addresses', () => { + it('should accept taproot addresses', () => { + expect(isAddress(TAPROOT, bitcoin)).to.be.eq(true); + }); + + it('should accept every other standard mainnet address type', () => { + expect(isAddress(NATIVE_SEGWIT, bitcoin)).to.be.eq(true); + expect(isAddress(P2WSH, bitcoin)).to.be.eq(true); + expect(isAddress(P2SH, bitcoin)).to.be.eq(true); + expect(isAddress(P2PKH, bitcoin)).to.be.eq(true); + }); + + it('should reject malformed addresses and addresses from another network', () => { + expect(isAddress('not an address', bitcoin)).to.be.eq(false); + expect(isAddress('', bitcoin)).to.be.eq(false); + // valid taproot address, wrong network + expect(isAddress(TAPROOT, litecoin)).to.be.eq(false); + // checksum broken on the last character + expect(isAddress(`${TAPROOT.slice(0, -1)}p`, bitcoin)).to.be.eq(false); + }); +}); + +describe('Should size outputs by their address type', () => { + it('should classify bech32 and bech32m outputs', () => { + expect(getOutputCounterForAddress(TAPROOT)).to.be.eq('p2tr_output_count'); + expect(getOutputCounterForAddress(NATIVE_SEGWIT)).to.be.eq( + 'p2wpkh_output_count', + ); + expect(getOutputCounterForAddress(P2WSH)).to.be.eq('p2wsh_output_count'); + }); + + it('should fall back to the account type for anything else', () => { + expect(getOutputCounterForAddress(P2SH)).to.be.eq(undefined); + expect(getOutputCounterForAddress(P2PKH)).to.be.eq(undefined); + expect(getOutputCounterForAddress('')).to.be.eq(undefined); + }); + + it('should charge for the larger taproot output', () => { + const toSegwit = calculateSizeBasedOnType(1, 2, PaymentType.P2WPKH, [ + NATIVE_SEGWIT, + ]); + const toTaproot = calculateSizeBasedOnType(1, 2, PaymentType.P2WPKH, [ + TAPROOT, + ]); + // P2TR_OUT_SIZE (43) - P2WPKH_OUT_SIZE (31) + expect(toTaproot - toSegwit).to.be.eq(12); + }); + + it('should size unknown outputs as the account type, as before', () => { + const withoutAddresses = calculateSizeBasedOnType(1, 2, PaymentType.P2WPKH); + expect( + calculateSizeBasedOnType(1, 2, PaymentType.P2WPKH, [NATIVE_SEGWIT]), + ).to.be.eq(withoutAddresses); + expect(calculateSizeBasedOnType(1, 2, PaymentType.P2WPKH, [''])).to.be.eq( + withoutAddresses, + ); + expect(calculateSizeBasedOnType(1, 2, PaymentType.P2WPKH, [P2SH])).to.be.eq( + withoutAddresses, + ); + }); +}); diff --git a/packages/extension/src/providers/bitcoin/ui/libs/signer.ts b/packages/extension/src/providers/bitcoin/ui/libs/signer.ts index 27f85e6fa..c226688d8 100644 --- a/packages/extension/src/providers/bitcoin/ui/libs/signer.ts +++ b/packages/extension/src/providers/bitcoin/ui/libs/signer.ts @@ -1,3 +1,4 @@ +import '../../libs/init-ecc'; import { InternalMethods, InternalOnMessageResponse } from '@/types/messenger'; import { SignerTransactionOptions, SignerMessageOptions } from '../types'; import sendUsingInternalMessengers from '@/libs/messenger/internal-messenger'; diff --git a/packages/extension/src/providers/bitcoin/ui/libs/tx-size.ts b/packages/extension/src/providers/bitcoin/ui/libs/tx-size.ts index 1cfa48830..40184a692 100644 --- a/packages/extension/src/providers/bitcoin/ui/libs/tx-size.ts +++ b/packages/extension/src/providers/bitcoin/ui/libs/tx-size.ts @@ -1,6 +1,7 @@ // https://github.com/jlopp/bitcoin-transaction-size-calculator/blob/master/index.html import { toBN } from 'web3-utils'; +import { address as BTCAddress } from 'bitcoinjs-lib'; import { PaymentType } from '../../types/bitcoin-network'; enum InputScriptType { @@ -234,16 +235,52 @@ const calculateSize = ( txWeight, }; }; +/** + * Bucket an output address into the counter `calculateSize` uses for it. + * + * Only bech32/bech32m outputs are classified. Those are the ones whose size + * differs enough from the account's own output type to matter: a P2TR or P2WSH + * output is 43 vB against 31 vB for P2WPKH, which is the difference between a + * transaction relaying and being dropped for paying under the minimum fee rate. + * Anything else falls back to the account's own type, which is at most 3 vB out. + */ +const getOutputCounterForAddress = ( + address: string, +): keyof calcOutputType | undefined => { + let decoded: { version: number; data: Buffer }; + try { + decoded = BTCAddress.fromBech32(address); + } catch { + return undefined; + } + if (decoded.version === 0) { + if (decoded.data.length === 20) return 'p2wpkh_output_count'; + if (decoded.data.length === 32) return 'p2wsh_output_count'; + } else if (decoded.version === 1 && decoded.data.length === 32) { + return 'p2tr_output_count'; + } + return undefined; +}; + +/** + * `outputAddresses` describes the outputs that are already known, in order. Any + * output without a matching entry — the change output, or a recipient address + * that is still being typed — is sized as the account's own payment type. + */ const calculateSizeBasedOnType = ( numInputs: number, numOutputs: number, type: PaymentType, + outputAddresses: string[] = [], ): number => { + const defaultCounter: keyof calcOutputType = + type === PaymentType.P2PKH ? 'p2pkh_output_count' : 'p2wpkh_output_count'; const output: calcOutputType = {}; - if (type === PaymentType.P2PKH) { - output.p2pkh_output_count = numOutputs; - } else { - output.p2wpkh_output_count = numOutputs; + for (let i = 0; i < numOutputs; i++) { + const counter = + (outputAddresses[i] && getOutputCounterForAddress(outputAddresses[i])) || + defaultCounter; + output[counter] = (output[counter] ?? 0) + 1; } const size = calculateSize( { @@ -257,4 +294,9 @@ const calculateSizeBasedOnType = ( ); return type === PaymentType.P2PKH ? size.txBytes : size.txVBytes; }; -export { InputScriptType, calculateSize, calculateSizeBasedOnType }; +export { + InputScriptType, + calculateSize, + calculateSizeBasedOnType, + getOutputCounterForAddress, +}; diff --git a/packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue b/packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue index 8937fe6fc..e164d932d 100644 --- a/packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue +++ b/packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue @@ -279,7 +279,7 @@ const nativeBalanceAfterTransaction = computed(() => { const setTransactionFees = (byteSize: number) => { const nativeVal = selectedAsset.value.price || '0'; - getGasCostValues( + return getGasCostValues( props.network as BitcoinNetwork, byteSize, nativeVal, @@ -294,16 +294,26 @@ const setBaseCosts = () => { }); }; +/** + * The recipient output is sized from `addressTo` so that sending to an address + * type larger than our own — a taproot one, say — is not underpaid. The change + * output has no entry and is sized as our own payment type. + */ +const updateTransactionFees = () => { + const txSize = calculateSizeBasedOnType( + accountUTXOs.value.length + (isSendToken.value ? 0 : 1), + 2, + (props.network as BitcoinNetwork).networkInfo.paymentType, + [addressTo.value], + ); + return setTransactionFees(Math.ceil(txSize)); +}; + const updateUTXOs = async () => { const api = (await props.network.api()) as BitcoinAPI; return api.getUTXOs(addressFrom.value).then(utxos => { accountUTXOs.value = utxos; - const txSize = calculateSizeBasedOnType( - accountUTXOs.value.length + (isSendToken.value ? 0 : 1), - 2, - (props.network as BitcoinNetwork).networkInfo.paymentType, - ); - setTransactionFees(Math.ceil(txSize)); + return updateTransactionFees(); }); }; @@ -369,6 +379,12 @@ watch([addressFrom], () => { } }); +watch([addressTo], () => { + updateTransactionFees().then(() => { + if (isMaxSelected.value) setMaxValue(); + }); +}); + const isOpenSelectContactFrom = ref(false); const isOpenSelectContactTo = ref(false); diff --git a/packages/extension/src/ui/action/views/swap/libs/bitcoin-gasvals.ts b/packages/extension/src/ui/action/views/swap/libs/bitcoin-gasvals.ts index d94afcd0e..1de50a829 100644 --- a/packages/extension/src/ui/action/views/swap/libs/bitcoin-gasvals.ts +++ b/packages/extension/src/ui/action/views/swap/libs/bitcoin-gasvals.ts @@ -15,6 +15,7 @@ export const getBitcoinGasVals = async ( tx.inputs.length, 2, (network as BitcoinNetwork).networkInfo.paymentType, + tx.outputs.map(output => output.address), ); return getGasCostValues( network as BitcoinNetwork, diff --git a/yarn.lock b/yarn.lock index 5d47e4bb5..65d0c311d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1726,6 +1726,7 @@ __metadata: resolution: "@enkryptcom/extension@workspace:packages/extension" dependencies: "@amplitude/analytics-browser": "npm:^2.42.4" + "@bitcoinerlab/secp256k1": "npm:^1.2.0" "@crxjs/vite-plugin": "npm:^2.4.0" "@enkryptcom/extension-bridge": "workspace:^" "@enkryptcom/hw-wallets": "workspace:^"