Merge pull request #7771 from BitGo/BTC-2891.fix-bkr-feecalc

fix(abstract-utxo): improve fee calculation and refactor backup key recovery

Author: OttoAllmendinger

modules/abstract-utxo/src/recovery/backupKeyRecovery.ts

@@ -1,6 +1,5 @@
 import _ from 'lodash';
 import * as utxolib from '@bitgo/utxo-lib';
-import { Dimensions } from '@bitgo/unspents';
 import {
   BitGoBase,
   ErrorNoInputToRecover,
@@ -20,6 +19,7 @@ import { generateAddressWithChainAndIndex } from '../address';
 import { forCoin, RecoveryProvider } from './RecoveryProvider';
 import { MempoolApi } from './mempoolApi';
 import { CoingeckoApi } from './coingeckoApi';
+import { createBackupKeyRecoveryPsbt, getRecoveryAmount } from './psbt';
 
 type ScriptType2Of3 = utxolib.bitgo.outputScripts.ScriptType2Of3;
 type ChainCode = utxolib.bitgo.ChainCode;
@@ -31,6 +31,14 @@ type WalletUnspentJSON = utxolib.bitgo.WalletUnspent & {
 
 const { getInternalChainCode, scriptTypeForChain, outputScripts, getExternalChainCode } = utxolib.bitgo;
 
+// V1 only deals with BTC. 50 sat/vbyte is very arbitrary.
+export const DEFAULT_RECOVERY_FEERATE_SAT_VBYTE_V1 = 50;
+
+// FIXME(BTC-2691): it is unclear why sweeps have a different default than regular recovery. 100 sat/vbyte is extremely high.
+export const DEFAULT_RECOVERY_FEERATE_SAT_VBYTE_V1_SWEEP = 100;
+
+// FIXME(BTC-2691): it makes little sense to have a single default for every coin.
+export const DEFAULT_RECOVERY_FEERATE_SAT_VBYTE_V2 = 50;
 export interface FormattedOfflineVaultTxInfo {
   txInfo: {
     unspents?: WalletUnspentJSON[];
@@ -92,6 +100,7 @@ export interface RecoverParams {
   apiKey?: string;
   userKeyPath?: string;
   recoveryProvider?: RecoveryProvider;
+  /** Satoshi per byte */
   feeRate?: number;
 }
 
@@ -323,27 +332,17 @@ export async function backupKeyRecovery(
     throw new ErrorNoInputToRecover();
   }
 
-  // Build the psbt
-  const psbt = utxolib.bitgo.createPsbtForNetwork({ network: coin.network });
-  // xpubs can become handy for many things.
-  utxolib.bitgo.addXpubsToPsbt(psbt, walletKeys);
   const txInfo = {} as BackupKeyRecoveryTransansaction;
   const feePerByte: number =
-    params.feeRate !== undefined ? params.feeRate : await getRecoveryFeePerBytes(coin, { defaultValue: 50 });
-
-  // KRS recovery transactions have a 2nd output to pay the recovery fee, like paygo fees.
-  const dimensions = Dimensions.fromPsbt(psbt).plus(isKrsRecovery ? Dimensions.SingleOutput.p2wsh : Dimensions.ZERO);
-  const approximateFee = BigInt(dimensions.getVSize() * feePerByte);
+    params.feeRate !== undefined
+      ? params.feeRate
+      : await getRecoveryFeePerBytes(coin, { defaultValue: DEFAULT_RECOVERY_FEERATE_SAT_VBYTE_V2 });
 
   txInfo.inputs =
     responseTxFormat === 'legacy'
       ? unspents.map((u) => ({ ...u, value: Number(u.value), valueString: u.value.toString(), prevTx: undefined }))
       : undefined;
 
-  unspents.forEach((unspent) => {
-    utxolib.bitgo.addWalletUnspentToPsbt(psbt, unspent, walletKeys, 'user', 'backup');
-  });
-
   let krsFee = BigInt(0);
   if (isKrsRecovery && params.krsProvider) {
     try {
@@ -354,33 +353,26 @@ export async function backupKeyRecovery(
     }
   }
 
-  const recoveryAmount = totalInputAmount - approximateFee - krsFee;
-
-  if (recoveryAmount < BigInt(0)) {
-    throw new Error(`this wallet\'s balance is too low to pay the fees specified by the KRS provider.
-          Existing balance on wallet: ${totalInputAmount.toString()}. Estimated network fee for the recovery transaction
-          : ${approximateFee.toString()}, KRS fee to pay: ${krsFee.toString()}. After deducting fees, your total
-          recoverable balance is ${recoveryAmount.toString()}`);
-  }
-
-  const recoveryOutputScript = utxolib.address.toOutputScript(params.recoveryDestination, coin.network);
-  psbt.addOutput({ script: recoveryOutputScript, value: recoveryAmount });
-
+  let krsFeeAddress: string | undefined;
   if (krsProvider && krsFee > BigInt(0)) {
     if (!krsProvider.feeAddresses) {
       throw new Error(`keyProvider must define feeAddresses`);
     }
 
-    const krsFeeAddress = krsProvider.feeAddresses[coin.getChain()];
+    krsFeeAddress = krsProvider.feeAddresses[coin.getChain()];
 
     if (!krsFeeAddress) {
       throw new Error('this KRS provider has not configured their fee structure yet - recovery cannot be completed');
     }
-
-    const krsFeeOutputScript = utxolib.address.toOutputScript(krsFeeAddress, coin.network);
-    psbt.addOutput({ script: krsFeeOutputScript, value: krsFee });
   }
 
+  const psbt = createBackupKeyRecoveryPsbt(coin.network, walletKeys, unspents, {
+    feeRateSatVB: feePerByte,
+    recoveryDestination: params.recoveryDestination,
+    keyRecoveryServiceFee: krsFee,
+    keyRecoveryServiceFeeAddress: krsFeeAddress,
+  });
+
   if (isUnsignedSweep) {
     return {
       txHex: psbt.toHex(),
@@ -408,6 +400,7 @@ export async function backupKeyRecovery(
   if (isKrsRecovery) {
     txInfo.coin = coin.getChain();
     txInfo.backupKey = params.backupKey;
+    const recoveryAmount = getRecoveryAmount(psbt, params.recoveryDestination);
     txInfo.recoveryAmount = Number(recoveryAmount);
     txInfo.recoveryAmountString = recoveryAmount.toString();
   }
@@ -446,7 +439,9 @@ export async function v1BackupKeyRecovery(
     throw new Error('invalid recoveryDestination');
   }
 
-  const recoveryFeePerByte = await getRecoveryFeePerBytes(coin, { defaultValue: 50 });
+  const recoveryFeePerByte = await getRecoveryFeePerBytes(coin, {
+    defaultValue: DEFAULT_RECOVERY_FEERATE_SAT_VBYTE_V1,
+  });
   const v1wallet = await bitgo.wallets().get({ id: params.walletId });
   return await v1wallet.recover({
     ...params,
@@ -472,7 +467,9 @@ export async function v1Sweep(
 
   let recoveryFeePerByte = 100;
   if (bitgo.env === 'prod') {
-    recoveryFeePerByte = await getRecoveryFeePerBytes(coin, { defaultValue: 100 });
+    recoveryFeePerByte = await getRecoveryFeePerBytes(coin, {
+      defaultValue: DEFAULT_RECOVERY_FEERATE_SAT_VBYTE_V1_SWEEP,
+    });
   }
 
   const v1wallet = await bitgo.wallets().get({ id: params.walletId });

modules/abstract-utxo/src/recovery/psbt.ts

@@ -0,0 +1,92 @@
+import * as utxolib from '@bitgo/utxo-lib';
+import { Dimensions } from '@bitgo/unspents';
+
+type RootWalletKeys = utxolib.bitgo.RootWalletKeys;
+type WalletUnspent<TNumber extends number | bigint> = utxolib.bitgo.WalletUnspent<TNumber>;
+
+class InsufficientFundsError extends Error {
+  constructor(
+    public totalInputAmount: bigint,
+    public approximateFee: bigint,
+    public krsFee: bigint,
+    public recoveryAmount: bigint
+  ) {
+    super(
+      `This wallet's balance is too low to pay the fees specified by the KRS provider.` +
+        `Existing balance on wallet: ${totalInputAmount.toString()}. ` +
+        `Estimated network fee for the recovery transaction: ${approximateFee.toString()}` +
+        `KRS fee to pay: ${krsFee.toString()}. ` +
+        `After deducting fees, your total recoverable balance is ${recoveryAmount.toString()}`
+    );
+  }
+}
+
+export function createBackupKeyRecoveryPsbt(
+  network: utxolib.Network,
+  rootWalletKeys: RootWalletKeys,
+  unspents: WalletUnspent<bigint>[],
+  {
+    feeRateSatVB,
+    recoveryDestination,
+    keyRecoveryServiceFee,
+    keyRecoveryServiceFeeAddress,
+  }: {
+    feeRateSatVB: number;
+    recoveryDestination: string;
+    keyRecoveryServiceFee: bigint;
+    keyRecoveryServiceFeeAddress: string | undefined;
+  }
+): utxolib.bitgo.UtxoPsbt {
+  if (keyRecoveryServiceFee > 0 && !keyRecoveryServiceFeeAddress) {
+    throw new Error('keyRecoveryServiceFeeAddress is required when keyRecoveryServiceFee is provided');
+  }
+
+  const psbt = utxolib.bitgo.createPsbtForNetwork({ network });
+  utxolib.bitgo.addXpubsToPsbt(psbt, rootWalletKeys);
+  unspents.forEach((unspent) => {
+    utxolib.bitgo.addWalletUnspentToPsbt(psbt, unspent, rootWalletKeys, 'user', 'backup');
+  });
+
+  let dimensions = Dimensions.fromPsbt(psbt).plus(
+    Dimensions.fromOutput({ script: utxolib.address.toOutputScript(recoveryDestination, network) })
+  );
+
+  if (keyRecoveryServiceFeeAddress) {
+    dimensions = dimensions.plus(
+      Dimensions.fromOutput({
+        script: utxolib.address.toOutputScript(keyRecoveryServiceFeeAddress, network),
+      })
+    );
+  }
+
+  const approximateFee = BigInt(dimensions.getVSize() * feeRateSatVB);
+
+  const totalInputAmount = utxolib.bitgo.unspentSum(unspents, 'bigint');
+
+  const recoveryAmount = totalInputAmount - approximateFee - keyRecoveryServiceFee;
+
+  // FIXME(BTC-2650): we should check for dust limit here instead
+  if (recoveryAmount < BigInt(0)) {
+    throw new InsufficientFundsError(totalInputAmount, approximateFee, keyRecoveryServiceFee, recoveryAmount);
+  }
+
+  psbt.addOutput({ script: utxolib.address.toOutputScript(recoveryDestination, network), value: recoveryAmount });
+
+  if (keyRecoveryServiceFeeAddress) {
+    psbt.addOutput({
+      script: utxolib.address.toOutputScript(keyRecoveryServiceFeeAddress, network),
+      value: keyRecoveryServiceFee,
+    });
+  }
+
+  return psbt;
+}
+
+export function getRecoveryAmount(psbt: utxolib.bitgo.UtxoPsbt, address: string): bigint {
+  const recoveryOutputScript = utxolib.address.toOutputScript(address, psbt.network);
+  const output = psbt.txOutputs.find((o) => o.script.equals(recoveryOutputScript));
+  if (!output) {
+    throw new Error(`Recovery destination output not found in PSBT: ${address}`);
+  }
+  return output.value;
+}

modules/abstract-utxo/test/unit/fixtures/bch/recovery/backupKeyRecovery-fullSignedRecovery-customUserKeyPath-p2sh.json

@@ -25,6 +25,6 @@
       "valueString": "300000000"
     }
   ],
-  "transactionHex": "02000000038bb70cc33ba011c71d725c686a81b1f19d2955d204300f384838725f076edd9800000000fdfe000048304502210098eda9635891330990f775ae0dc39ea8ae25c4e0a5ce89c05a923f9f14b6ba1f02201982367892d23ef19f9265751c94b11b2c202ee56c900d104fa634147c15675941483045022100a1e554194187d658c043e76bafb71fc3742506beb4a48859f532b09d7e1c207c02205a1fbca7f6a2e3882b479895efab25243c57d1744d3f4b86d8c7c8ef13f2d7af414c69522102587d7749d1ed2a3d3d300b969e68cbedb042ea19e3ee90c4131e2092f5e1181e21030795af84ecc10252d8a894f54845beeb5624a1c24c3747cc654bd430539dee3521029b30ebe8eb23f8cec82f25a80e3b423979ec3ba1fe07d9d4ed9f6361258bc31d53aeffffffffeec68db14dcb1f1340f46c6f9d9a7303915ccd9022d59e7c9e13dae7c91ffe9b00000000fdfd000047304402200c990acb71e278854ec0e5e9575cf6c85c7ab7d498ea2404208d33551d463b5f022042c9117fc8811b1cdca6a210de5cf8a83f00dac16a0edb9e1cf1d7c25808e69341483045022100e89ededd4a954ef6da9ddb7f418efb6fba674e5f96f90f6c3952d846835a80a202207b7764ea85f1e6c428474e0ca5da24a6ad492b31c7a20d56bf1d1be86fd58bf9414c69522102587d7749d1ed2a3d3d300b969e68cbedb042ea19e3ee90c4131e2092f5e1181e21030795af84ecc10252d8a894f54845beeb5624a1c24c3747cc654bd430539dee3521029b30ebe8eb23f8cec82f25a80e3b423979ec3ba1fe07d9d4ed9f6361258bc31d53aeffffffffe3ad05836997543b105275534081e8eac637e0e4d2a5bc544d7b0d22f32d49de00000000fdfd0000483045022100a0f040a70353436f3c2743f1e1f53818cc85986b5d66b263104a40bf33db081e02202d4090f15a98d7bf60a455b2cb40009555537dfbe3d0b550054d400953cefada41473044022003c005fcdd983209d6e62a806f7688af4e27a1f7a80e7fad23f04c14a698cf5902207e5c244df1f55634214390018ad7b5bf43152e36ffe99f7d51f3c73f7fbe8cda414c69522102587d7749d1ed2a3d3d300b969e68cbedb042ea19e3ee90c4131e2092f5e1181e21030795af84ecc10252d8a894f54845beeb5624a1c24c3747cc654bd430539dee3521029b30ebe8eb23f8cec82f25a80e3b423979ec3ba1fe07d9d4ed9f6361258bc31d53aeffffffff010c44c3230000000017a91439c65a0d0072a140694d6b13ec5f5f2437de99ff8700000000",
-  "txid": "7d7051163f29c1590d886358a23efce56228096e3e047903d9b46aa42f6e545e"
+  "transactionHex": "02000000038bb70cc33ba011c71d725c686a81b1f19d2955d204300f384838725f076edd9800000000fdfe0000483045022100e68d7057fdad1fd9e7da1dd0d2745600cd7ebc6b3bdfdc8c977c27f117dec1ee022014a862be7e83b092cea8c4791d47d9ea87bc3a7e4d7851fad30e9da0a8933efc41483045022100d4295855382edd094687ade706ccf51375c716e3acd2156cb0d7403f857a795f0220409c5b8f8ed66f43e563c2c4e401b8ca0cfab3c89452645c92c4010ee07d74d5414c69522102587d7749d1ed2a3d3d300b969e68cbedb042ea19e3ee90c4131e2092f5e1181e21030795af84ecc10252d8a894f54845beeb5624a1c24c3747cc654bd430539dee3521029b30ebe8eb23f8cec82f25a80e3b423979ec3ba1fe07d9d4ed9f6361258bc31d53aeffffffffeec68db14dcb1f1340f46c6f9d9a7303915ccd9022d59e7c9e13dae7c91ffe9b00000000fc004730440220487d165adcc526d5bf659e5dfec94e07c8eaa6567308d29a7b4676456e71288802204172d68f63bcc29141095b81a9366056b6d11260d86c6f1dfa8a154953b0a7854147304402205d3c5b6105a2fa1819973ef6b83c1575468be0bce6757992b365583c11690fa902200134cc5b58d6590664f45e797990334b4fa989b21ef2ec5194a9d3ae262855ad414c69522102587d7749d1ed2a3d3d300b969e68cbedb042ea19e3ee90c4131e2092f5e1181e21030795af84ecc10252d8a894f54845beeb5624a1c24c3747cc654bd430539dee3521029b30ebe8eb23f8cec82f25a80e3b423979ec3ba1fe07d9d4ed9f6361258bc31d53aeffffffffe3ad05836997543b105275534081e8eac637e0e4d2a5bc544d7b0d22f32d49de00000000fc00473044022025d60881a0bf878533362094e8a531f1a066fa2f85ac92d5965f20d7227682c20220685efc33bb4e3a81963f4ebd0a18ec088db96f20432e1c943228e2c1fff2996141473044022065fb4062083c3cbf12638cf087b36512d22458cdf76e5f92582992885efab050022039885486cc1b912d0843cf8227a7473b5938c8927a9b7f41efd03af87752fffe414c69522102587d7749d1ed2a3d3d300b969e68cbedb042ea19e3ee90c4131e2092f5e1181e21030795af84ecc10252d8a894f54845beeb5624a1c24c3747cc654bd430539dee3521029b30ebe8eb23f8cec82f25a80e3b423979ec3ba1fe07d9d4ed9f6361258bc31d53aeffffffff0160d8c1230000000017a91439c65a0d0072a140694d6b13ec5f5f2437de99ff8700000000",
+  "txid": "b50d92e5be1c143941ad3ce4aa176c69c6299cd4c689d5caceeff5f943f8ddb3"
 }

modules/abstract-utxo/test/unit/fixtures/bch/recovery/backupKeyRecovery-fullSignedRecovery-fixedFeeRate-p2sh.json

@@ -25,6 +25,6 @@
       "valueString": "300000000"
     }
   ],
-  "transactionHex": "0200000003d7c3d565c03d67573680c96285939855cc656beade6793cbb121fb2ca51f339c00000000fdfe0000483045022100dabac902e044562a23ac7ddf33f7fb98243506bf220a5a92dc80bd7833f62481022070aae3aac378fcecbf5f14f2ff57c440b2d950b74c69d63adc0516490fd9a0f4414830450221009fa93dbab63f2daaa13efd7ae7b0afa4c2fd3f6eb0263cd910b6e206e9852b9d02202567839a051d9fb7f9d1cd2a45bcdf50a9ccc0b058535d5ffb755159984ef062414c69522102d06c08ccf0fddefff881e869b951d4b92e936118b3360182c5b8c55f4c40bc6121030795af84ecc10252d8a894f54845beeb5624a1c24c3747cc654bd430539dee3521029b30ebe8eb23f8cec82f25a80e3b423979ec3ba1fe07d9d4ed9f6361258bc31d53aeffffffff6adf8f092a117182d89effd08726812693d051e2c6e78efc3b967f88a619f21600000000fdfd0000483045022100b44c1912b028c0d6adbf03089ceaa8a4058b478ab49e0045162b2bade888d24702201f5985068ebea8eff58aa933ea0c9ea3a8ecbc10d0049532235fbe4c051d2959414730440220757236f8ee0a1155252079c3ab454769c2703aed46f4d8f0546facbd7493549002201ceb3f90c93bdd4f064b007f664adc13fe4bdc160cc3b96d39895650bbec3a9d414c69522102d06c08ccf0fddefff881e869b951d4b92e936118b3360182c5b8c55f4c40bc6121030795af84ecc10252d8a894f54845beeb5624a1c24c3747cc654bd430539dee3521029b30ebe8eb23f8cec82f25a80e3b423979ec3ba1fe07d9d4ed9f6361258bc31d53aeffffffff3f482a856356853fe1e8cefffc23e30363038ba4c8c89bc505e4c5f0f595158c00000000fc0047304402204abbd1263eb5fab332594197bf1cd240b36c656ae9c0d1eb197963c4ea798b7502206640b2715d944a00f88c811bb68b2c1296445a59c92e5c8bc6594847c676afe94147304402201f5582671aac80b2e247445000942f512975193820a50c28e21067edce326b9f022029d264daf2da9ff12a02f21d64981ab128b9d61061866da067e3b24bb8703811414c69522102d06c08ccf0fddefff881e869b951d4b92e936118b3360182c5b8c55f4c40bc6121030795af84ecc10252d8a894f54845beeb5624a1c24c3747cc654bd430539dee3521029b30ebe8eb23f8cec82f25a80e3b423979ec3ba1fe07d9d4ed9f6361258bc31d53aeffffffff010c44c3230000000017a91439c65a0d0072a140694d6b13ec5f5f2437de99ff8700000000",
-  "txid": "f9f8e91672fccac62d3ad4a7f53ec8e4d730051c38b2ec9e1a923486ff02c493"
+  "transactionHex": "0200000003d7c3d565c03d67573680c96285939855cc656beade6793cbb121fb2ca51f339c00000000fc0047304402205c2c110269e115e52d5afbbc33c4cb8407f391156e19003307b8e440d724092b02206054509b229ad6c03a59a3a7eb6a06e915873125082e677b23b42cb23ed4e3b84147304402201ecb68fe27d7ad76562bf0bda73d185f795df711a179a7ea4e26f9578da872720220044397e408c7d3788f02b688b5a7de1f43fbbefe1959d20926a52d189306de29414c69522102d06c08ccf0fddefff881e869b951d4b92e936118b3360182c5b8c55f4c40bc6121030795af84ecc10252d8a894f54845beeb5624a1c24c3747cc654bd430539dee3521029b30ebe8eb23f8cec82f25a80e3b423979ec3ba1fe07d9d4ed9f6361258bc31d53aeffffffff6adf8f092a117182d89effd08726812693d051e2c6e78efc3b967f88a619f21600000000fdfe00004830450221009490a4723a5f83f076ce847161a3a8f7fe1b88ee222aa203b8002b43366ddbd602204a88daa818f63ca9475690386d09e7b44c90a2382a9d5048ccdeaaf663ca069241483045022100e6acf24d06227e8348d2303029e0602194ae4c8085ce572fe9ad9c6aab251f9602203797527deb5a14663d87719344ac205251f52ef6435bb6bf4ff5b185e6921243414c69522102d06c08ccf0fddefff881e869b951d4b92e936118b3360182c5b8c55f4c40bc6121030795af84ecc10252d8a894f54845beeb5624a1c24c3747cc654bd430539dee3521029b30ebe8eb23f8cec82f25a80e3b423979ec3ba1fe07d9d4ed9f6361258bc31d53aeffffffff3f482a856356853fe1e8cefffc23e30363038ba4c8c89bc505e4c5f0f595158c00000000fdfe00004830450221009e0f3204f6c3829ac91eb3f721a45a169af96a17ba23f89d20e76ba44a828c530220563fbafd634e4672163cafb4c2982ba69e290db7c10d366557c77f8e2216131241483045022100efcfbb07e483a105e8020940ca6d3139249f354d309f0548ecadfec877e4c05a02204b197f099abd4211e7f17a21c7a303435d12eeba39f17ff5fd441fe60f09bb15414c69522102d06c08ccf0fddefff881e869b951d4b92e936118b3360182c5b8c55f4c40bc6121030795af84ecc10252d8a894f54845beeb5624a1c24c3747cc654bd430539dee3521029b30ebe8eb23f8cec82f25a80e3b423979ec3ba1fe07d9d4ed9f6361258bc31d53aeffffffff0160d8c1230000000017a91439c65a0d0072a140694d6b13ec5f5f2437de99ff8700000000",
+  "txid": "3802e1ad47c85bbf65ed794641a1f2f06755f1569c3d69cd5e54db9abe9d579d"
 }