Fix inaccurate fee estimations

src/app/common/transactions/bitcoin/coinselect/local-coin-selection.spec.ts

@@ -21,25 +21,74 @@ const demoUtxos = [
   { value: 909 },
 ];
 
+function generate10kSpendWithDummyUtxoSet(recipient: string) {
+  return determineUtxosForSpend({
+    utxos: demoUtxos as any,
+    amount: 10_000,
+    feeRate: 20,
+    recipient,
+  });
+}
+
 describe(determineUtxosForSpend.name, () => {
-  function generate10kSpendWithTestData(recipient: string) {
-    return determineUtxosForSpend({
-      utxos: demoUtxos as any,
-      amount: 10_000,
-      feeRate: 20,
-      recipient,
+  describe('Estimated size', () => {
+    test('that Native Segwit, 1 input 2 outputs weighs 140 vBytes', () => {
+      const estimation = determineUtxosForSpend({
+        utxos: [{ value: 50_000 }] as any[],
+        amount: 40_000,
+        recipient: 'tb1qt28eagxcl9gvhq2rpj5slg7dwgxae2dn2hk93m',
+        feeRate: 20,
+      });
+      console.log(estimation);
+      expect(estimation.txVBytes).toBeGreaterThan(140);
+      expect(estimation.txVBytes).toBeLessThan(142);
+    });
+
+    test('that Native Segwit, 2 input 2 outputs weighs 200vBytes', () => {
+      const estimation = determineUtxosForSpend({
+        utxos: [{ value: 50_000 }, { value: 50_000 }] as any[],
+        amount: 60_000,
+        recipient: 'tb1qt28eagxcl9gvhq2rpj5slg7dwgxae2dn2hk93m',
+        feeRate: 20,
+      });
+      console.log(estimation);
+      expect(estimation.txVBytes).toBeGreaterThan(208);
+      expect(estimation.txVBytes).toBeLessThan(209);
     });
-  }
 
-  describe('sorting algorithm (biggest first and no dust)', () => {
+    test('that Native Segwit, 10 input 2 outputs weighs 200vBytes', () => {
+      const estimation = determineUtxosForSpend({
+        utxos: [
+          { value: 20_000 },
+          { value: 20_000 },
+          { value: 10_000 },
+          { value: 10_000 },
+          { value: 10_000 },
+          { value: 10_000 },
+          { value: 10_000 },
+          { value: 10_000 },
+          { value: 10_000 },
+          { value: 10_000 },
+        ] as any[],
+        amount: 100_000,
+        recipient: 'tb1qt28eagxcl9gvhq2rpj5slg7dwgxae2dn2hk93m',
+        feeRate: 20,
+      });
+      expect(estimation.txVBytes).toBeGreaterThan(750);
+      expect(estimation.txVBytes).toBeLessThan(751);
+    });
+  });
+
+  describe('sorting algorithm', () => {
     test('that it filters out dust utxos', () => {
-      const result = generate10kSpendWithTestData('tb1qt28eagxcl9gvhq2rpj5slg7dwgxae2dn2hk93m');
+      const result = generate10kSpendWithDummyUtxoSet('tb1qt28eagxcl9gvhq2rpj5slg7dwgxae2dn2hk93m');
+      console.log(result);
       const hasDust = result.filteredUtxos.some(utxo => utxo.value <= BTC_P2WPKH_DUST_AMOUNT);
       expect(hasDust).toBeFalsy();
     });
 
     test('that it sorts utxos in decending order', () => {
-      const result = generate10kSpendWithTestData('tb1qt28eagxcl9gvhq2rpj5slg7dwgxae2dn2hk93m');
+      const result = generate10kSpendWithDummyUtxoSet('tb1qt28eagxcl9gvhq2rpj5slg7dwgxae2dn2hk93m');
       result.inputs.forEach((u, i) => {
         const nextUtxo = result.inputs[i + 1];
         if (!nextUtxo) return;
@@ -50,38 +99,38 @@ describe(determineUtxosForSpend.name, () => {
 
   test('that it accepts a wrapped segwit address', () =>
     expect(() =>
-      generate10kSpendWithTestData('33SVjoCHJovrXxjDKLFSXo1h3t5KgkPzfH')
+      generate10kSpendWithDummyUtxoSet('33SVjoCHJovrXxjDKLFSXo1h3t5KgkPzfH')
     ).not.toThrowError());
 
   test('that it accepts a legacy addresses', () =>
     expect(() =>
-      generate10kSpendWithTestData('15PyZveQd28E2SHZu2ugkWZBp6iER41vXj')
+      generate10kSpendWithDummyUtxoSet('15PyZveQd28E2SHZu2ugkWZBp6iER41vXj')
     ).not.toThrowError());
 
   test('that it throws an error with non-legit address', () => {
     expect(() =>
-      generate10kSpendWithTestData('whoop-de-da-boop-da-de-not-a-bitcoin-address')
+      generate10kSpendWithDummyUtxoSet('whoop-de-da-boop-da-de-not-a-bitcoin-address')
     ).toThrowError();
   });
 
   test('that given a set of utxos, legacy is more expensive', () => {
-    const legacy = generate10kSpendWithTestData('15PyZveQd28E2SHZu2ugkWZBp6iER41vXj');
-    const segwit = generate10kSpendWithTestData('33SVjoCHJovrXxjDKLFSXo1h3t5KgkPzfH');
+    const legacy = generate10kSpendWithDummyUtxoSet('15PyZveQd28E2SHZu2ugkWZBp6iER41vXj');
+    const segwit = generate10kSpendWithDummyUtxoSet('33SVjoCHJovrXxjDKLFSXo1h3t5KgkPzfH');
     expect(legacy.fee).toBeGreaterThan(segwit.fee);
   });
 
   test('that given a set of utxos, wrapped segwit is more expensive than native', () => {
-    const segwit = generate10kSpendWithTestData('33SVjoCHJovrXxjDKLFSXo1h3t5KgkPzfH');
-    const native = generate10kSpendWithTestData('tb1qt28eagxcl9gvhq2rpj5slg7dwgxae2dn2hk93m');
+    const segwit = generate10kSpendWithDummyUtxoSet('33SVjoCHJovrXxjDKLFSXo1h3t5KgkPzfH');
+    const native = generate10kSpendWithDummyUtxoSet('tb1qt28eagxcl9gvhq2rpj5slg7dwgxae2dn2hk93m');
     expect(segwit.fee).toBeGreaterThan(native.fee);
   });
 
   test('that given a set of utxos, taproot is more expensive than native segwit', () => {
     // Non-obvious behaviour.
     // P2TR outputs = 34 vBytes
     // P2WPKH outputs = 22 vBytes
-    const native = generate10kSpendWithTestData('tb1qt28eagxcl9gvhq2rpj5slg7dwgxae2dn2hk93m');
-    const taproot = generate10kSpendWithTestData(
+    const native = generate10kSpendWithDummyUtxoSet('tb1qt28eagxcl9gvhq2rpj5slg7dwgxae2dn2hk93m');
+    const taproot = generate10kSpendWithDummyUtxoSet(
       'tb1parwmj7533de3k2fw2kntyqacspvhm67qnjcmpqnnpfvzu05l69nsczdywd'
     );
     expect(taproot.fee).toBeGreaterThan(native.fee);

src/app/common/transactions/bitcoin/coinselect/local-coin-selection.ts

@@ -1,13 +1,15 @@
+import BigNumber from 'bignumber.js';
 import { validate } from 'bitcoin-address-validation';
 
 import type { RpcSendTransferRecipient } from '@shared/rpc/methods/send-transfer';
 
+import { sumNumbers } from '@app/common/math/helpers';
 import { UtxoResponseItem } from '@app/query/bitcoin/bitcoin-client';
 
 import {
   filterUneconomicalUtxos,
   filterUneconomicalUtxosMultipleRecipients,
-  getSizeInfo,
+  getBitcoinTxSizeEstimation,
   getSizeInfoMultipleRecipients,
 } from '../utils';
 
@@ -33,9 +35,9 @@ export function determineUtxosForSpendAll({
   if (!validate(recipient)) throw new Error('Cannot calculate spend of invalid address type');
   const filteredUtxos = filterUneconomicalUtxos({ utxos, feeRate, address: recipient });
 
-  const sizeInfo = getSizeInfo({
-    inputLength: filteredUtxos.length,
-    outputLength: 1,
+  const sizeInfo = getBitcoinTxSizeEstimation({
+    inputCount: filteredUtxos.length,
+    outputCount: 1,
     recipient,
   });
 
@@ -52,6 +54,10 @@ export function determineUtxosForSpendAll({
   };
 }
 
+function getUtxoTotal(utxos: UtxoResponseItem[]) {
+  return sumNumbers(utxos.map(utxo => utxo.value));
+}
+
 export function determineUtxosForSpend({
   amount,
   feeRate,
@@ -60,47 +66,59 @@ export function determineUtxosForSpend({
 }: DetermineUtxosForSpendArgs) {
   if (!validate(recipient)) throw new Error('Cannot calculate spend of invalid address type');
 
-  const orderedUtxos = utxos.sort((a, b) => b.value - a.value);
-
-  const filteredUtxos = filterUneconomicalUtxos({
-    utxos: orderedUtxos,
+  const filteredUtxos: UtxoResponseItem[] = filterUneconomicalUtxos({
+    utxos: utxos.sort((a, b) => b.value - a.value),
     feeRate,
     address: recipient,
   });
 
-  const neededUtxos = [];
-  let sum = 0n;
-  let sizeInfo = null;
+  if (!filteredUtxos.length) throw new InsufficientFundsError();
 
-  for (const utxo of filteredUtxos) {
-    sizeInfo = getSizeInfo({
-      inputLength: neededUtxos.length,
-      outputLength: 2,
+  // Prepopulate with first UTXO, at least one is needed
+  const neededUtxos: UtxoResponseItem[] = [filteredUtxos[0]];
+
+  function estimateTransactionSize() {
+    return getBitcoinTxSizeEstimation({
+      inputCount: neededUtxos.length,
+      outputCount: 2,
       recipient,
     });
-    if (sum >= BigInt(amount) + BigInt(Math.ceil(sizeInfo.txVBytes * feeRate))) break;
+  }
 
-    sum += BigInt(utxo.value);
-    neededUtxos.push(utxo);
+  function hasSufficientUtxosForTx() {
+    const txEstimation = estimateTransactionSize();
+    const neededAmount = new BigNumber(txEstimation.txVBytes * feeRate).plus(amount);
+    return getUtxoTotal(neededUtxos).isGreaterThanOrEqualTo(neededAmount);
   }
 
-  if (!sizeInfo) throw new InsufficientFundsError();
+  function getRemainingUnspentUtxos() {
+    return filteredUtxos.filter(utxo => !neededUtxos.includes(utxo));
+  }
 
-  const fee = Math.ceil(sizeInfo.txVBytes * feeRate);
+  while (!hasSufficientUtxosForTx()) {
+    const [nextUtxo] = getRemainingUnspentUtxos();
+    if (!nextUtxo) throw new InsufficientFundsError();
+    neededUtxos.push(nextUtxo);
+  }
+
+  const fee = Math.ceil(
+    new BigNumber(estimateTransactionSize().txVBytes).multipliedBy(feeRate).toNumber()
+  );
 
   const outputs = [
     // outputs[0] = the desired amount going to recipient
     { value: BigInt(amount), address: recipient },
     // outputs[1] = the remainder to be returned to a change address
-    { value: sum - BigInt(amount) - BigInt(fee) },
+    { value: BigInt(getUtxoTotal(neededUtxos).toString()) - BigInt(amount) - BigInt(fee) },
   ];
 
   return {
     filteredUtxos,
     inputs: neededUtxos,
     outputs,
-    size: sizeInfo.txVBytes,
+    size: estimateTransactionSize().txVBytes,
     fee,
+    ...estimateTransactionSize(),
   };
 }
 

src/app/common/transactions/bitcoin/use-generate-bitcoin-tx.ts

@@ -166,7 +166,7 @@ export function useGenerateUnsignedNativeSegwitMultipleRecipientsTx() {
           tx.addOutputAddress(output.address, BigInt(output.value), networkMode);
         });
 
-        return { hex: tx.hex, fee, psbt: tx.toPSBT(), inputs };
+        return { hex: tx.hex, fee: fee, psbt: tx.toPSBT(), inputs };
       } catch (e) {
         // eslint-disable-next-line no-console
         console.log('Error signing bitcoin transaction', e);

src/app/common/transactions/bitcoin/utils.ts

@@ -34,9 +34,9 @@ export function getSpendableAmount({
 }) {
   const balance = utxos.map(utxo => utxo.value).reduce((prevVal, curVal) => prevVal + curVal, 0);
 
-  const size = getSizeInfo({
-    inputLength: utxos.length,
-    outputLength: 1,
+  const size = getBitcoinTxSizeEstimation({
+    inputCount: utxos.length,
+    outputCount: 1,
     recipient: address,
   });
   const fee = Math.ceil(size.txVBytes * feeRate);
@@ -80,22 +80,22 @@ export function filterUneconomicalUtxos({
   return filteredUtxos;
 }
 
-export function getSizeInfo(payload: {
-  inputLength: number;
-  outputLength: number;
+export function getBitcoinTxSizeEstimation(payload: {
+  inputCount: number;
+  outputCount: number;
   recipient: string;
 }) {
-  const { inputLength, recipient, outputLength } = payload;
+  const { inputCount, recipient, outputCount } = payload;
   const addressInfo = validate(recipient) ? getAddressInfo(recipient) : null;
   const outputAddressTypeWithFallback = addressInfo ? addressInfo.type : 'p2wpkh';
 
   const txSizer = new BtcSizeFeeEstimator();
   const sizeInfo = txSizer.calcTxSize({
     // Only p2wpkh is supported by the wallet
     input_script: 'p2wpkh',
-    input_count: inputLength,
+    input_count: inputCount,
     // From the address of the recipient, we infer the output type
-    [outputAddressTypeWithFallback + '_output_count']: outputLength,
+    [outputAddressTypeWithFallback + '_output_count']: outputCount,
   });
 
   return sizeInfo;

src/app/features/dialogs/increase-fee-dialog/hooks/use-btc-increase-fee.ts

@@ -1,3 +1,4 @@
+import { useMemo } from 'react';
 import { useNavigate } from 'react-router-dom';
 
 import * as btc from '@scure/btc-signer';
@@ -14,9 +15,9 @@ import { useBtcAssetBalance } from '@app/common/hooks/balance/btc/use-btc-balanc
 import { btcToSat } from '@app/common/money/unit-conversion';
 import { queryClient } from '@app/common/persistence';
 import {
+  getBitcoinTxSizeEstimation,
   getBitcoinTxValue,
   getRecipientAddressFromOutput,
-  getSizeInfo,
 } from '@app/common/transactions/bitcoin/utils';
 import { MAX_FEE_RATE_MULTIPLIER } from '@app/components/bitcoin-custom-fee/hooks/use-bitcoin-custom-fee';
 import { useBitcoinFeesList } from '@app/components/bitcoin-fees-list/use-bitcoin-fees-list';
@@ -39,11 +40,16 @@ export function useBtcIncreaseFee(btcTx: BitcoinTx) {
   const signTransaction = useSignBitcoinTx();
   const { broadcastTx, isBroadcasting } = useBitcoinBroadcastTransaction();
   const recipient = getRecipientAddressFromOutput(btcTx.vout, currentBitcoinAddress) || '';
-  const sizeInfo = getSizeInfo({
-    inputLength: btcTx.vin.length,
-    recipient,
-    outputLength: btcTx.vout.length,
-  });
+
+  const sizeInfo = useMemo(
+    () =>
+      getBitcoinTxSizeEstimation({
+        inputCount: btcTx.vin.length,
+        recipient,
+        outputCount: btcTx.vout.length,
+      }),
+    [btcTx.vin.length, btcTx.vout.length, recipient]
+  );
 
   const { btcAvailableAssetBalance } = useBtcAssetBalance(currentBitcoinAddress);
   const sendingAmount = getBitcoinTxValue(currentBitcoinAddress, btcTx);

src/app/pages/rpc-sign-psbt/use-rpc-sign-psbt.tsx

@@ -73,9 +73,7 @@ export function useRpcSignPsbt() {
           txValue: formatMoney(transferTotalAsMoney),
         };
 
-        navigate(RouteUrls.RpcSignPsbtSummary, {
-          state: psbtTxSummaryState,
-        });
+        navigate(RouteUrls.RpcSignPsbtSummary, { state: psbtTxSummaryState });
       },
       onError(e) {
         navigate(RouteUrls.RequestError, {

src/app/pages/send/send-crypto-asset-form/form/btc/btc-choose-fee.tsx

@@ -17,7 +17,6 @@ export function useBtcChooseFeeState() {
   const isSendingMax = useLocationStateWithCache('isSendingMax') as boolean;
   const txValues = useLocationStateWithCache('values') as BitcoinSendFormValues;
   const utxos = useLocationStateWithCache('utxos') as UtxoResponseItem[];
-
   return { isSendingMax, txValues, utxos };
 }