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:^"