From 612f1e44fe7ead319ae87653607614dd1bc14d60 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 5 Oct 2021 20:07:52 -0400 Subject: [PATCH 1/7] bumpfee: Calculate fee by looking up UTXOs Instead of calculating the fee by using what is stored in the wallet, calculate it by looking up the UTXOs. --- src/wallet/feebumper.cpp | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index c2b8082eae3..637153ec66e 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -61,7 +61,7 @@ static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWallet } //! Check if the user provided a valid feeRate -static feebumper::Result CheckFeeRate(const CWallet& wallet, const CWalletTx& wtx, const CFeeRate& newFeerate, const int64_t maxTxSize, std::vector& errors) +static feebumper::Result CheckFeeRate(const CWallet& wallet, const CWalletTx& wtx, const CFeeRate& newFeerate, const int64_t maxTxSize, CAmount old_fee, std::vector& errors) { // check that fee rate is higher than mempool's minimum fee // (no point in bumping fee if we know that the new tx won't be accepted to the mempool) @@ -83,8 +83,6 @@ static feebumper::Result CheckFeeRate(const CWallet& wallet, const CWalletTx& wt CFeeRate incrementalRelayFee = std::max(wallet.chain().relayIncrementalFee(), CFeeRate(WALLET_INCREMENTAL_RELAY_FEE)); // Given old total fee and transaction size, calculate the old feeRate - isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE; - CAmount old_fee = CachedTxGetDebit(wallet, wtx, filter) - wtx.tx->GetValueOut(); const int64_t txSize = GetVirtualTransactionSize(*(wtx.tx)); CFeeRate nOldFeeRate(old_fee, txSize); // Min total fee is old fee + relay fee @@ -169,13 +167,37 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo } const CWalletTx& wtx = it->second; + // Retrieve all of the UTXOs and add them to coin control + // While we're here, calculate the input amount + std::map coins; + CAmount input_value = 0; + for (const CTxIn& txin : wtx.tx->vin) { + coins[txin.prevout]; // Create empty map entry keyed by prevout. + } + wallet.chain().findCoins(coins); + for (const CTxIn& txin : wtx.tx->vin) { + const Coin& coin = coins.at(txin.prevout); + if (coin.out.IsNull()) { + errors.push_back(Untranslated(strprintf("%s:%u is already spent", txin.prevout.hash.GetHex(), txin.prevout.n))); + return Result::MISC_ERROR; + } + if (wallet.IsMine(txin.prevout)) { + new_coin_control.Select(txin.prevout); + } else { + new_coin_control.SelectExternal(txin.prevout, coin.out); + } + input_value += coin.out.nValue; + } + Result result = PreconditionChecks(wallet, wtx, errors); if (result != Result::OK) { return result; } // Fill in recipients(and preserve a single change key if there is one) + // While we're here, calculate the output amount std::vector recipients; + CAmount output_value = 0; for (const auto& output : wtx.tx->vout) { if (!OutputIsChange(wallet, output)) { CRecipient recipient = {output.scriptPubKey, output.nValue, false}; @@ -185,16 +207,16 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo ExtractDestination(output.scriptPubKey, change_dest); new_coin_control.destChange = change_dest; } + output_value += output.nValue; } - isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE; - old_fee = CachedTxGetDebit(wallet, wtx, filter) - wtx.tx->GetValueOut(); + old_fee = input_value - output_value; if (coin_control.m_feerate) { // The user provided a feeRate argument. // We calculate this here to avoid compiler warning on the cs_wallet lock - const int64_t maxTxSize{CalculateMaximumSignedTxSize(*wtx.tx, &wallet).vsize}; - Result res = CheckFeeRate(wallet, wtx, *new_coin_control.m_feerate, maxTxSize, errors); + const int64_t maxTxSize{CalculateMaximumSignedTxSize(*wtx.tx, &wallet, &new_coin_control).vsize}; + Result res = CheckFeeRate(wallet, wtx, *new_coin_control.m_feerate, maxTxSize, old_fee, errors); if (res != Result::OK) { return res; } From a0c3afb898016c2e0a76dc48f68eaa5c3ae6282c Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 5 Oct 2021 21:41:54 -0400 Subject: [PATCH 2/7] bumpfee: extract weights of external inputs when bumping fee When bumping the fee of a transaction containing external inputs, determine the weights of those inputs. Because signatures can have a variable size, the script is executed with a special SignatureChecker which will compute the total weight of the signatures in the transaction and the weight if they were all maximum size signatures. This allows us to compute the maximum weight of the input for use during coin selection. --- src/wallet/feebumper.cpp | 27 +++++++++++++++++++++ src/wallet/feebumper.h | 51 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 637153ec66e..d2e3f073016 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include #include #include @@ -171,6 +172,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo // While we're here, calculate the input amount std::map coins; CAmount input_value = 0; + std::vector spent_outputs; for (const CTxIn& txin : wtx.tx->vin) { coins[txin.prevout]; // Create empty map entry keyed by prevout. } @@ -187,6 +189,31 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo new_coin_control.SelectExternal(txin.prevout, coin.out); } input_value += coin.out.nValue; + spent_outputs.push_back(coin.out); + } + + // Figure out if we need to compute the input weight, and do so if necessary + PrecomputedTransactionData txdata; + txdata.Init(*wtx.tx, std::move(spent_outputs), /* force=*/ true); + for (unsigned int i = 0; i < wtx.tx->vin.size(); ++i) { + const CTxIn& txin = wtx.tx->vin.at(i); + const Coin& coin = coins.at(txin.prevout); + + if (new_coin_control.IsExternalSelected(txin.prevout)) { + // For external inputs, we estimate the size using the size of this input + int64_t input_weight = GetTransactionInputWeight(txin); + // Because signatures can have different sizes, we need to figure out all of the + // signature sizes and replace them with the max sized signature. + // In order to do this, we verify the script with a special SignatureChecker which + // will observe the signatures verified and record their sizes. + SignatureWeights weights; + TransactionSignatureChecker tx_checker(wtx.tx.get(), i, coin.out.nValue, txdata, MissingDataBehavior::FAIL); + SignatureWeightChecker size_checker(weights, tx_checker); + VerifyScript(txin.scriptSig, coin.out.scriptPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, size_checker); + // Add the difference between max and current to input_weight so that it represents the largest the input could be + input_weight += weights.GetWeightDiffToMax(); + new_coin_control.SetInputWeight(txin.prevout, input_weight); + } } Result result = PreconditionChecks(wallet, wtx, errors); diff --git a/src/wallet/feebumper.h b/src/wallet/feebumper.h index 191878a137b..908ef1117af 100644 --- a/src/wallet/feebumper.h +++ b/src/wallet/feebumper.h @@ -5,6 +5,8 @@ #ifndef BITCOIN_WALLET_FEEBUMPER_H #define BITCOIN_WALLET_FEEBUMPER_H +#include +#include