From a6e116c981e21e27dc895d713d1d6aa213142d1a Mon Sep 17 00:00:00 2001 From: Edgar Khanzadian Date: Wed, 24 Jan 2024 14:18:15 +0400 Subject: [PATCH] fix: filter out uneconomical utxos, closes #4505 --- .../coinselect/local-coin-selection.spec.ts | 2 +- .../coinselect/local-coin-selection.ts | 47 +++--- .../bitcoin/fees/bitcoin-fees.spec.ts | 143 ++++++++++++++++++ .../bitcoin/fees/btc-size-fee-estimator.ts | 10 +- .../fees/calculate-max-bitcoin-spend.ts | 50 ++++++ src/app/common/transactions/bitcoin/utils.ts | 68 ++++++++- .../bitcoin/hooks/use-calculate-max-spend.ts | 45 ++---- 7 files changed, 291 insertions(+), 74 deletions(-) create mode 100644 src/app/common/transactions/bitcoin/fees/bitcoin-fees.spec.ts create mode 100644 src/app/common/transactions/bitcoin/fees/calculate-max-bitcoin-spend.ts diff --git a/src/app/common/transactions/bitcoin/coinselect/local-coin-selection.spec.ts b/src/app/common/transactions/bitcoin/coinselect/local-coin-selection.spec.ts index e35f615f865..efe3f638dcb 100644 --- a/src/app/common/transactions/bitcoin/coinselect/local-coin-selection.spec.ts +++ b/src/app/common/transactions/bitcoin/coinselect/local-coin-selection.spec.ts @@ -34,7 +34,7 @@ describe(determineUtxosForSpend.name, () => { describe('sorting algorithm (biggest first and no dust)', () => { test('that it filters out dust utxos', () => { const result = generate10kSpendWithTestData('tb1qt28eagxcl9gvhq2rpj5slg7dwgxae2dn2hk93m'); - const hasDust = result.orderedUtxos.some(utxo => utxo.value <= BTC_P2WPKH_DUST_AMOUNT); + const hasDust = result.filteredUtxos.some(utxo => utxo.value <= BTC_P2WPKH_DUST_AMOUNT); expect(hasDust).toBeFalsy(); }); diff --git a/src/app/common/transactions/bitcoin/coinselect/local-coin-selection.ts b/src/app/common/transactions/bitcoin/coinselect/local-coin-selection.ts index cdbe309d93f..ade9b348719 100644 --- a/src/app/common/transactions/bitcoin/coinselect/local-coin-selection.ts +++ b/src/app/common/transactions/bitcoin/coinselect/local-coin-selection.ts @@ -1,10 +1,8 @@ -import { getAddressInfo, validate } from 'bitcoin-address-validation'; - -import { BTC_P2WPKH_DUST_AMOUNT } from '@shared/constants'; +import { validate } from 'bitcoin-address-validation'; import { UtxoResponseItem } from '@app/query/bitcoin/bitcoin-client'; -import { BtcSizeFeeEstimator } from '../fees/btc-size-fee-estimator'; +import { filterUneconomicalUtxos, getSizeInfo } from '../utils'; export interface DetermineUtxosForSpendArgs { amount: number; @@ -20,17 +18,12 @@ export function determineUtxosForSpendAll({ utxos, }: DetermineUtxosForSpendArgs) { if (!validate(recipient)) throw new Error('Cannot calculate spend of invalid address type'); + const filteredUtxos = filterUneconomicalUtxos({ utxos, feeRate, address: recipient }); - const addressInfo = getAddressInfo(recipient); - - const txSizer = new BtcSizeFeeEstimator(); - - const filteredUtxos = utxos.filter(utxo => utxo.value >= BTC_P2WPKH_DUST_AMOUNT); - - const sizeInfo = txSizer.calcTxSize({ - input_script: 'p2wpkh', - input_count: filteredUtxos.length, - [addressInfo.type + '_output_count']: 1, + const sizeInfo = getSizeInfo({ + inputLength: filteredUtxos.length, + outputLength: 1, + recipient, }); // Fee has already been deducted from the amount with send all @@ -54,25 +47,23 @@ export function determineUtxosForSpend({ }: DetermineUtxosForSpendArgs) { if (!validate(recipient)) throw new Error('Cannot calculate spend of invalid address type'); - const addressInfo = getAddressInfo(recipient); - - const orderedUtxos = utxos - .filter(utxo => utxo.value >= BTC_P2WPKH_DUST_AMOUNT) - .sort((a, b) => b.value - a.value); + const orderedUtxos = utxos.sort((a, b) => b.value - a.value); - const txSizer = new BtcSizeFeeEstimator(); + const filteredUtxos = filterUneconomicalUtxos({ + utxos: orderedUtxos, + feeRate, + address: recipient, + }); const neededUtxos = []; let sum = 0n; let sizeInfo = null; - for (const utxo of orderedUtxos) { - sizeInfo = txSizer.calcTxSize({ - // Only p2wpkh is supported by the wallet - input_script: 'p2wpkh', - input_count: neededUtxos.length, - // From the address of the recipient, we infer the output type - [addressInfo.type + '_output_count']: 2, + for (const utxo of filteredUtxos) { + sizeInfo = getSizeInfo({ + inputLength: neededUtxos.length, + outputLength: 2, + recipient, }); if (sum >= BigInt(amount) + BigInt(Math.ceil(sizeInfo.txVBytes * feeRate))) break; @@ -92,7 +83,7 @@ export function determineUtxosForSpend({ ]; return { - orderedUtxos, + filteredUtxos, inputs: neededUtxos, outputs, size: sizeInfo.txVBytes, diff --git a/src/app/common/transactions/bitcoin/fees/bitcoin-fees.spec.ts b/src/app/common/transactions/bitcoin/fees/bitcoin-fees.spec.ts new file mode 100644 index 00000000000..4e28ccdbde9 --- /dev/null +++ b/src/app/common/transactions/bitcoin/fees/bitcoin-fees.spec.ts @@ -0,0 +1,143 @@ +import BigNumber from 'bignumber.js'; +import { sha256 } from 'bitcoinjs-lib/src/crypto'; + +import { UtxoResponseItem } from '@app/query/bitcoin/bitcoin-client'; + +import { filterUneconomicalUtxos } from '../utils'; +import { calculateMaxBitcoinSpend } from './calculate-max-bitcoin-spend'; + +function generateTxId(value: number): UtxoResponseItem { + const buffer = Buffer.from(Math.random().toString()); + return { + txid: sha256(sha256(buffer)).toString(), + vout: 0, + status: { + confirmed: true, + block_height: 2568495, + block_hash: '000000000000008622fafce4a5388861b252d534f819d0f7cb5d4f2c5f9c1638', + block_time: 1703787327, + }, + value, + }; +} + +function generateTransactions(values: number[]) { + return values.map(val => generateTxId(val)); +} + +function generateAverageFee(value: number) { + return { + hourFee: BigNumber(value / 2), + halfHourFee: BigNumber(value), + fastestFee: BigNumber(value * 2), + }; +} + +describe(calculateMaxBitcoinSpend.name, () => { + const utxos = generateTransactions([600, 600, 1200, 1200, 10000, 10000, 25000, 40000, 50000000]); + + test('with 1 sat/vb fee', () => { + const fee = 1; + const maxBitcoinSpend = calculateMaxBitcoinSpend({ + address: '', + utxos, + fetchedFeeRates: generateAverageFee(fee), + }); + expect(maxBitcoinSpend.amount.amount.toNumber()).toEqual(50087948); + }); + + test('with 5 sat/vb fee', () => { + const fee = 5; + const maxBitcoinSpend = calculateMaxBitcoinSpend({ + address: '', + utxos, + fetchedFeeRates: generateAverageFee(fee), + }); + expect(maxBitcoinSpend.amount.amount.toNumber()).toEqual(50085342); + }); + + test('with 30 sat/vb fee', () => { + const fee = 30; + const maxBitcoinSpend = calculateMaxBitcoinSpend({ + address: '', + utxos, + fetchedFeeRates: generateAverageFee(fee), + }); + expect(maxBitcoinSpend.amount.amount.toNumber()).toEqual(50073585); + }); + + test('with 100 sat/vb fee', () => { + const fee = 100; + const maxBitcoinSpend = calculateMaxBitcoinSpend({ + address: '', + utxos, + fetchedFeeRates: generateAverageFee(fee), + }); + expect(maxBitcoinSpend.amount.amount.toNumber()).toEqual(50046950); + }); + + test('with 400 sat/vb fee', () => { + const fee = 400; + const maxBitcoinSpend = calculateMaxBitcoinSpend({ + address: '', + utxos, + fetchedFeeRates: generateAverageFee(fee), + }); + expect(maxBitcoinSpend.amount.amount.toNumber()).toEqual(49969100); + }); +}); + +describe(filterUneconomicalUtxos.name, () => { + const utxos = generateTransactions([600, 600, 1200, 1200, 10000, 10000, 25000, 40000, 50000000]); + + test('with 1 sat/vb fee', () => { + const fee = 1; + const filteredUtxos = filterUneconomicalUtxos({ + address: '', + utxos, + feeRate: fee, + }); + + expect(filteredUtxos.length).toEqual(9); + }); + + test('with 10 sat/vb fee', () => { + const fee = 10; + const filteredUtxos = filterUneconomicalUtxos({ + address: '', + utxos, + feeRate: fee, + }); + expect(filteredUtxos.length).toEqual(7); + }); + + test('with 30 sat/vb fee', () => { + const fee = 30; + const filteredUtxos = filterUneconomicalUtxos({ + address: '', + utxos, + feeRate: fee, + }); + expect(filteredUtxos.length).toEqual(5); + }); + + test('with 200 sat/vb fee', () => { + const fee = 200; + const filteredUtxos = filterUneconomicalUtxos({ + address: '', + utxos, + feeRate: fee, + }); + expect(filteredUtxos.length).toEqual(3); + }); + + test('with 400 sat/vb fee', () => { + const fee = 400; + const filteredUtxos = filterUneconomicalUtxos({ + address: '', + utxos, + feeRate: fee, + }); + expect(filteredUtxos.length).toEqual(2); + }); +}); diff --git a/src/app/common/transactions/bitcoin/fees/btc-size-fee-estimator.ts b/src/app/common/transactions/bitcoin/fees/btc-size-fee-estimator.ts index 2b4d4cf06fc..ecbe796f1d3 100644 --- a/src/app/common/transactions/bitcoin/fees/btc-size-fee-estimator.ts +++ b/src/app/common/transactions/bitcoin/fees/btc-size-fee-estimator.ts @@ -10,7 +10,7 @@ type InputScriptTypes = | 'p2wsh' | 'p2tr'; -interface Params { +interface TxSizerParams { input_count: number; input_script: InputScriptTypes; input_m: number; @@ -48,7 +48,7 @@ export class BtcSizeFeeEstimator { 'p2tr', ]; - defaultParams: Params = { + defaultParams: TxSizerParams = { input_count: 0, input_script: 'p2wpkh', input_m: 0, @@ -62,7 +62,7 @@ export class BtcSizeFeeEstimator { p2tr_output_count: 0, }; - params: Params = { ...this.defaultParams }; + params: TxSizerParams = { ...this.defaultParams }; getSizeOfScriptLengthElement(length: number) { if (length < 75) { @@ -128,7 +128,7 @@ export class BtcSizeFeeEstimator { return witness_vbytes * 3; } - prepareParams(opts: Partial) { + prepareParams(opts: Partial) { // Verify opts and set them to this.params opts = opts || Object.assign(this.defaultParams); @@ -279,7 +279,7 @@ export class BtcSizeFeeEstimator { }; } - calcTxSize(opts: Partial) { + calcTxSize(opts: Partial) { this.prepareParams(opts); const output_count = this.getOutputCount(); const { inputSize, inputWitnessSize } = this.getSizeBasedOnInputType(); diff --git a/src/app/common/transactions/bitcoin/fees/calculate-max-bitcoin-spend.ts b/src/app/common/transactions/bitcoin/fees/calculate-max-bitcoin-spend.ts new file mode 100644 index 00000000000..c5e38813c09 --- /dev/null +++ b/src/app/common/transactions/bitcoin/fees/calculate-max-bitcoin-spend.ts @@ -0,0 +1,50 @@ +import BigNumber from 'bignumber.js'; + +import { AverageBitcoinFeeRates } from '@shared/models/fees/bitcoin-fees.model'; +import { createMoney } from '@shared/models/money.model'; + +import { satToBtc } from '@app/common/money/unit-conversion'; +import { UtxoResponseItem } from '@app/query/bitcoin/bitcoin-client'; + +import { filterUneconomicalUtxos, getSpendableAmount } from '../utils'; + +interface CalculateMaxBitcoinSpend { + address: string; + utxos: UtxoResponseItem[]; + fetchedFeeRates?: AverageBitcoinFeeRates; + feeRate?: number; +} + +export function calculateMaxBitcoinSpend({ + address, + utxos, + feeRate, + fetchedFeeRates, +}: CalculateMaxBitcoinSpend) { + if (!utxos.length || !fetchedFeeRates) + return { + spendAllFee: 0, + amount: createMoney(0, 'BTC'), + spendableBitcoin: new BigNumber(0), + }; + + const currentFeeRate = feeRate ?? fetchedFeeRates.halfHourFee.toNumber(); + + const filteredUtxos = filterUneconomicalUtxos({ + utxos, + feeRate: currentFeeRate, + address, + }); + + const { spendableAmount, fee } = getSpendableAmount({ + utxos: filteredUtxos, + feeRate: currentFeeRate, + address, + }); + + return { + spendAllFee: fee, + amount: createMoney(spendableAmount, 'BTC'), + spendableBitcoin: satToBtc(spendableAmount), + }; +} diff --git a/src/app/common/transactions/bitcoin/utils.ts b/src/app/common/transactions/bitcoin/utils.ts index 6730c435990..54f57cf4a77 100644 --- a/src/app/common/transactions/bitcoin/utils.ts +++ b/src/app/common/transactions/bitcoin/utils.ts @@ -1,10 +1,13 @@ -import { getAddressInfo } from 'bitcoin-address-validation'; +import BigNumber from 'bignumber.js'; +import { getAddressInfo, validate } from 'bitcoin-address-validation'; +import { BTC_P2WPKH_DUST_AMOUNT } from '@shared/constants'; import { BitcoinTransactionVectorOutput } from '@shared/models/transactions/bitcoin-transaction.model'; import { BitcoinTx } from '@shared/models/transactions/bitcoin-transaction.model'; import { sumNumbers } from '@app/common/math/helpers'; import { satToBtc } from '@app/common/money/unit-conversion'; +import { UtxoResponseItem } from '@app/query/bitcoin/bitcoin-client'; import { truncateMiddle } from '@app/ui/utils/truncate-middle'; import { BtcSizeFeeEstimator } from './fees/btc-size-fee-estimator'; @@ -12,14 +15,71 @@ import { BtcSizeFeeEstimator } from './fees/btc-size-fee-estimator'; export function containsTaprootInput(tx: BitcoinTx) { return tx.vin.some(input => input.prevout.scriptpubkey_type === 'v1_p2tr'); } +export function getSpendableAmount({ + utxos, + feeRate, + address, +}: { + utxos: UtxoResponseItem[]; + feeRate: number; + address: string; +}) { + const balance = utxos.map(utxo => utxo.value).reduce((prevVal, curVal) => prevVal + curVal, 0); + + const size = getSizeInfo({ + inputLength: utxos.length, + outputLength: 1, + recipient: address, + }); + const fee = Math.ceil(size.txVBytes * feeRate); + const bigNumberBalance = BigNumber(balance); + return { + spendableAmount: BigNumber.max(0, bigNumberBalance.minus(fee)), + fee, + }; +} + +// Check if the spendable amount drops when adding a utxo. If it drops, don't use that utxo. +// Method might be not particularly efficient as it would +// go through the utxo array multiple times, but it's reliable. +export function filterUneconomicalUtxos({ + utxos, + feeRate, + address, +}: { + utxos: UtxoResponseItem[]; + feeRate: number; + address: string; +}) { + const { spendableAmount: fullSpendableAmount } = getSpendableAmount({ + utxos, + feeRate, + address, + }); + + const filteredUtxos = utxos + .filter(utxo => utxo.value >= BTC_P2WPKH_DUST_AMOUNT) + .filter(utxo => { + // calculate spendableAmount without that utxo. + const { spendableAmount } = getSpendableAmount({ + utxos: utxos.filter(u => u.txid !== utxo.txid), + feeRate, + address, + }); + // if spendable amount becomes bigger, do not use that utxo + return spendableAmount.toNumber() < fullSpendableAmount.toNumber(); + }); + return filteredUtxos; +} export function getSizeInfo(payload: { inputLength: number; - recipient: string; outputLength: number; + recipient: string; }) { const { inputLength, recipient, outputLength } = payload; - const addressInfo = getAddressInfo(recipient); + const addressInfo = validate(recipient) ? getAddressInfo(recipient) : null; + const outputAddressTypeWithFallback = addressInfo ? addressInfo.type : 'p2wpkh'; const txSizer = new BtcSizeFeeEstimator(); const sizeInfo = txSizer.calcTxSize({ @@ -27,7 +87,7 @@ export function getSizeInfo(payload: { input_script: 'p2wpkh', input_count: inputLength, // From the address of the recipient, we infer the output type - [addressInfo.type + '_output_count']: outputLength, + [outputAddressTypeWithFallback + '_output_count']: outputLength, }); return sizeInfo; diff --git a/src/app/pages/send/send-crypto-asset-form/family/bitcoin/hooks/use-calculate-max-spend.ts b/src/app/pages/send/send-crypto-asset-form/family/bitcoin/hooks/use-calculate-max-spend.ts index c63aecbe149..d83eef530f7 100644 --- a/src/app/pages/send/send-crypto-asset-form/family/bitcoin/hooks/use-calculate-max-spend.ts +++ b/src/app/pages/send/send-crypto-asset-form/family/bitcoin/hooks/use-calculate-max-spend.ts @@ -1,47 +1,20 @@ import { useCallback } from 'react'; -import BigNumber from 'bignumber.js'; -import { getAddressInfo, validate } from 'bitcoin-address-validation'; - -import { createMoney } from '@shared/models/money.model'; - -import { satToBtc } from '@app/common/money/unit-conversion'; -import { BtcSizeFeeEstimator } from '@app/common/transactions/bitcoin/fees/btc-size-fee-estimator'; -import { useCurrentNativeSegwitAddressBalance } from '@app/query/bitcoin/balance/btc-native-segwit-balance.hooks'; +import { calculateMaxBitcoinSpend } from '@app/common/transactions/bitcoin/fees/calculate-max-bitcoin-spend'; import { UtxoResponseItem } from '@app/query/bitcoin/bitcoin-client'; import { useAverageBitcoinFeeRates } from '@app/query/bitcoin/fees/fee-estimates.hooks'; export function useCalculateMaxBitcoinSpend() { - const balance = useCurrentNativeSegwitAddressBalance(); const { data: feeRates } = useAverageBitcoinFeeRates(); return useCallback( - (address = '', utxos: UtxoResponseItem[], feeRate?: number) => { - if (!utxos.length || !feeRates) - return { - spendAllFee: 0, - amount: createMoney(0, 'BTC'), - spendableBitcoin: new BigNumber(0), - }; - - const txSizer = new BtcSizeFeeEstimator(); - const addressInfo = validate(address) ? getAddressInfo(address) : null; - const addressTypeWithFallback = addressInfo ? addressInfo.type : 'p2wpkh'; - const size = txSizer.calcTxSize({ - input_script: 'p2wpkh', - input_count: utxos.length, - [`${addressTypeWithFallback}_output_count`]: 1, - }); - const fee = Math.ceil(size.txVBytes * (feeRate ?? feeRates.halfHourFee.toNumber())); - - const spendableAmount = BigNumber.max(0, balance.amount.minus(fee)); - - return { - spendAllFee: fee, - amount: createMoney(spendableAmount, 'BTC'), - spendableBitcoin: satToBtc(spendableAmount), - }; - }, - [balance.amount, feeRates] + (address = '', utxos: UtxoResponseItem[], feeRate?: number) => + calculateMaxBitcoinSpend({ + address, + utxos, + feeRate, + fetchedFeeRates: feeRates, + }), + [feeRates] ); }