diff options
Diffstat (limited to 'src')
-rw-r--r-- | src/Makefile.test.include | 1 | ||||
-rw-r--r-- | src/script/interpreter.h | 4 | ||||
-rw-r--r-- | src/script/sign.cpp | 5 | ||||
-rw-r--r-- | src/script/sign.h | 2 | ||||
-rw-r--r-- | src/wallet/feebumper.cpp | 94 | ||||
-rw-r--r-- | src/wallet/feebumper.h | 66 | ||||
-rw-r--r-- | src/wallet/interfaces.cpp | 2 | ||||
-rw-r--r-- | src/wallet/rpc/spend.cpp | 2 | ||||
-rw-r--r-- | src/wallet/test/feebumper_tests.cpp | 54 |
9 files changed, 204 insertions, 26 deletions
diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 75a404c50a..22a95f9682 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -162,6 +162,7 @@ BITCOIN_TESTS =\ if ENABLE_WALLET BITCOIN_TESTS += \ + wallet/test/feebumper_tests.cpp \ wallet/test/psbt_wallet_tests.cpp \ wallet/test/spend_tests.cpp \ wallet/test/wallet_tests.cpp \ diff --git a/src/script/interpreter.h b/src/script/interpreter.h index f91578d684..ba910cc945 100644 --- a/src/script/interpreter.h +++ b/src/script/interpreter.h @@ -307,10 +307,10 @@ using MutableTransactionSignatureChecker = GenericTransactionSignatureChecker<CM class DeferringSignatureChecker : public BaseSignatureChecker { protected: - BaseSignatureChecker& m_checker; + const BaseSignatureChecker& m_checker; public: - DeferringSignatureChecker(BaseSignatureChecker& checker) : m_checker(checker) {} + DeferringSignatureChecker(const BaseSignatureChecker& checker) : m_checker(checker) {} bool CheckECDSASignature(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override { diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 4014ebadbc..5da0d076d8 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -596,8 +596,11 @@ public: bool CheckECDSASignature(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override { return true; } bool CheckSchnorrSignature(Span<const unsigned char> sig, Span<const unsigned char> pubkey, SigVersion sigversion, ScriptExecutionData& execdata, ScriptError* serror) const override { return true; } }; -const DummySignatureChecker DUMMY_CHECKER; +} + +const BaseSignatureChecker& DUMMY_CHECKER = DummySignatureChecker(); +namespace { class DummySignatureCreator final : public BaseSignatureCreator { private: char m_r_len = 32; diff --git a/src/script/sign.h b/src/script/sign.h index 958d673b9f..813dfe04e3 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -52,6 +52,8 @@ public: bool CreateSchnorrSig(const SigningProvider& provider, std::vector<unsigned char>& sig, const XOnlyPubKey& pubkey, const uint256* leaf_hash, const uint256* merkle_root, SigVersion sigversion) const override; }; +/** A signature checker that accepts all signatures */ +extern const BaseSignatureChecker& DUMMY_CHECKER; /** A signature creator that just produces 71-byte empty signatures. */ extern const BaseSignatureCreator& DUMMY_SIGNATURE_CREATOR; /** A signature creator that just produces 72-byte empty signatures. */ diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index c2b8082eae..1d741cda6e 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 <consensus/validation.h> #include <interfaces/chain.h> #include <policy/fees.h> #include <policy/policy.h> @@ -19,7 +20,7 @@ namespace wallet { //! Check whether transaction has descendant in wallet or mempool, or has been //! mined, or conflicts with a mined transaction. Return a feebumper::Result. -static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWalletTx& wtx, std::vector<bilingual_str>& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) +static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWalletTx& wtx, bool require_mine, std::vector<bilingual_str>& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) { if (wallet.HasWalletSpend(wtx.tx)) { errors.push_back(Untranslated("Transaction has descendants in the wallet")); @@ -48,20 +49,21 @@ static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWallet return feebumper::Result::WALLET_ERROR; } - // check that original tx consists entirely of our inputs - // if not, we can't bump the fee, because the wallet has no way of knowing the value of the other inputs (thus the fee) - isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE; - if (!AllInputsMine(wallet, *wtx.tx, filter)) { - errors.push_back(Untranslated("Transaction contains inputs that don't belong to this wallet")); - return feebumper::Result::WALLET_ERROR; + if (require_mine) { + // check that original tx consists entirely of our inputs + // if not, we can't bump the fee, because the wallet has no way of knowing the value of the other inputs (thus the fee) + isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE; + if (!AllInputsMine(wallet, *wtx.tx, filter)) { + errors.push_back(Untranslated("Transaction contains inputs that don't belong to this wallet")); + return feebumper::Result::WALLET_ERROR; + } } - return feebumper::Result::OK; } //! 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<bilingual_str>& errors) +static feebumper::Result CheckFeeRate(const CWallet& wallet, const CWalletTx& wtx, const CFeeRate& newFeerate, const int64_t maxTxSize, CAmount old_fee, std::vector<bilingual_str>& 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 +85,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 @@ -150,12 +150,12 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid) if (wtx == nullptr) return false; std::vector<bilingual_str> errors_dummy; - feebumper::Result res = PreconditionChecks(wallet, *wtx, errors_dummy); + feebumper::Result res = PreconditionChecks(wallet, *wtx, /* require_mine=*/ true, errors_dummy); return res == feebumper::Result::OK; } Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCoinControl& coin_control, std::vector<bilingual_str>& errors, - CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx) + CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx, bool require_mine) { // We are going to modify coin control later, copy to re-use CCoinControl new_coin_control(coin_control); @@ -169,13 +169,63 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo } const CWalletTx& wtx = it->second; - Result result = PreconditionChecks(wallet, wtx, errors); + // Retrieve all of the UTXOs and add them to coin control + // While we're here, calculate the input amount + std::map<COutPoint, Coin> coins; + CAmount input_value = 0; + std::vector<CTxOut> spent_outputs; + 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; + 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, require_mine, 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<CRecipient> 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 +235,22 @@ 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); + // We need to make a temporary transaction with no input witnesses as the dummy signer expects them to be empty for external inputs + CMutableTransaction mtx{*wtx.tx}; + for (auto& txin : mtx.vin) { + txin.scriptSig.clear(); + txin.scriptWitness.SetNull(); + } + const int64_t maxTxSize{CalculateMaximumSignedTxSize(CTransaction(mtx), &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; } @@ -254,7 +310,7 @@ Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransacti const CWalletTx& oldWtx = it->second; // make sure the transaction still has no descendants and hasn't been mined in the meantime - Result result = PreconditionChecks(wallet, oldWtx, errors); + Result result = PreconditionChecks(wallet, oldWtx, /* require_mine=*/ false, errors); if (result != Result::OK) { return result; } diff --git a/src/wallet/feebumper.h b/src/wallet/feebumper.h index 191878a137..760ab58e5c 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 <consensus/consensus.h> +#include <script/interpreter.h> #include <primitives/transaction.h> class uint256; @@ -31,14 +33,25 @@ enum class Result //! Return whether transaction can be bumped. bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid); -//! Create bumpfee transaction based on feerate estimates. +/** Create bumpfee transaction based on feerate estimates. + * + * @param[in] wallet The wallet to use for this bumping + * @param[in] txid The txid of the transaction to bump + * @param[in] coin_control A CCoinControl object which provides feerates and other information used for coin selection + * @param[out] errors Errors + * @param[out] old_fee The fee the original transaction pays + * @param[out] new_fee the fee that the bump transaction pays + * @param[out] mtx The bump transaction itself + * @param[in] require_mine Whether the original transaction must consist of inputs that can be spent by the wallet + */ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCoinControl& coin_control, std::vector<bilingual_str>& errors, CAmount& old_fee, CAmount& new_fee, - CMutableTransaction& mtx); + CMutableTransaction& mtx, + bool require_mine); //! Sign the new transaction, //! @return false if the tx couldn't be found or if it was @@ -55,6 +68,55 @@ Result CommitTransaction(CWallet& wallet, std::vector<bilingual_str>& errors, uint256& bumped_txid); +struct SignatureWeights +{ +private: + int m_sigs_count{0}; + int64_t m_sigs_weight{0}; + +public: + void AddSigWeight(const size_t weight, const SigVersion sigversion) + { + switch (sigversion) { + case SigVersion::BASE: + m_sigs_weight += weight * WITNESS_SCALE_FACTOR; + m_sigs_count += 1 * WITNESS_SCALE_FACTOR; + break; + case SigVersion::WITNESS_V0: + m_sigs_weight += weight; + m_sigs_count++; + break; + case SigVersion::TAPROOT: + case SigVersion::TAPSCRIPT: + assert(false); + } + } + + int64_t GetWeightDiffToMax() const + { + // Note: the witness scaling factor is already accounted for because the count is multiplied by it. + return (/* max signature size=*/ 72 * m_sigs_count) - m_sigs_weight; + } +}; + +class SignatureWeightChecker : public DeferringSignatureChecker +{ +private: + SignatureWeights& m_weights; + +public: + SignatureWeightChecker(SignatureWeights& weights, const BaseSignatureChecker& checker) : DeferringSignatureChecker(checker), m_weights(weights) {} + + bool CheckECDSASignature(const std::vector<unsigned char>& sig, const std::vector<unsigned char>& pubkey, const CScript& script, SigVersion sigversion) const override + { + if (m_checker.CheckECDSASignature(sig, pubkey, script, sigversion)) { + m_weights.AddSigWeight(sig.size(), sigversion); + return true; + } + return false; + } +}; + } // namespace feebumper } // namespace wallet diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 4fbc519e39..98925e6b10 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -291,7 +291,7 @@ public: CAmount& new_fee, CMutableTransaction& mtx) override { - return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx) == feebumper::Result::OK; + return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx, /* require_mine= */ true) == feebumper::Result::OK; } bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(*m_wallet.get(), mtx); } bool commitBumpTransaction(const uint256& txid, diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index c6e7588fb4..66adc1d327 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -1045,7 +1045,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) CMutableTransaction mtx; feebumper::Result res; // Targeting feerate bump. - res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx); + res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx, /*require_mine=*/ !want_psbt); if (res != feebumper::Result::OK) { switch(res) { case feebumper::Result::INVALID_ADDRESS_OR_KEY: diff --git a/src/wallet/test/feebumper_tests.cpp b/src/wallet/test/feebumper_tests.cpp new file mode 100644 index 0000000000..6add86dc3d --- /dev/null +++ b/src/wallet/test/feebumper_tests.cpp @@ -0,0 +1,54 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php. + +#include <primitives/transaction.h> +#include <script/script.h> +#include <util/strencodings.h> +#include <wallet/feebumper.h> +#include <wallet/test/util.h> +#include <wallet/test/wallet_test_fixture.h> + +#include <boost/test/unit_test.hpp> + +namespace wallet { +namespace feebumper { +BOOST_FIXTURE_TEST_SUITE(feebumper_tests, WalletTestingSetup) + +static void CheckMaxWeightComputation(const std::string& script_str, const std::vector<std::string>& witness_str_stack, const std::string& prevout_script_str, int64_t expected_max_weight) +{ + std::vector script_data(ParseHex(script_str)); + CScript script(script_data.begin(), script_data.end()); + CTxIn input(uint256(), 0, script); + + for (const auto& s : witness_str_stack) { + input.scriptWitness.stack.push_back(ParseHex(s)); + } + + std::vector prevout_script_data(ParseHex(prevout_script_str)); + CScript prevout_script(prevout_script_data.begin(), prevout_script_data.end()); + + int64_t weight = GetTransactionInputWeight(input); + SignatureWeights weights; + SignatureWeightChecker size_checker(weights, DUMMY_CHECKER); + bool script_ok = VerifyScript(input.scriptSig, prevout_script, &input.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, size_checker); + BOOST_CHECK(script_ok); + weight += weights.GetWeightDiffToMax(); + BOOST_CHECK_EQUAL(weight, expected_max_weight); +} + +BOOST_AUTO_TEST_CASE(external_max_weight_test) +{ + // P2PKH + CheckMaxWeightComputation("453042021f03c8957c5ce12940ee6e3333ecc3f633d9a1ac53a55b3ce0351c617fa96abe021f0dccdcce3ef45a63998be9ec748b561baf077b8e862941d0cd5ec08f5afe68012102fccfeb395f0ecd3a77e7bc31c3bc61dc987418b18e395d441057b42ca043f22c", {}, "76a914f60dcfd3392b28adc7662669603641f578eed72d88ac", 593); + // P2SH-P2WPKH + CheckMaxWeightComputation("160014001dca1b22c599b5a56a87c78417ad2ff39552f1", {"3042021f5443c58eaf45f3e5ef46f8516f966b334a7d497cedda4edb2b9fad57c90c3b021f63a77cb56cde848e2e2dd20b487eec2f53101f634193786083f60b4d23a82301", "026cfe86116f161057deb240201d6b82ebd4f161e0200d63dc9aca65a1d6b38bb7"}, "a9147c8ab5ad7708b97ccb6b483d57aba48ee85214df87", 364); + // P2WPKH + CheckMaxWeightComputation("", {"3042021f0f8906f0394979d5b737134773e5b88bf036c7d63542301d600ab677ba5a59021f0e9fe07e62c113045fa1c1532e2914720e8854d189c4f5b8c88f57956b704401", "0359edba11ed1a0568094a6296a16c4d5ee4c8cfe2f5e2e6826871b5ecf8188f79"}, "00149961a78658030cc824af4c54fbf5294bec0cabdd", 272); + // P2WSH HTLC + CheckMaxWeightComputation("", {"3042021f5c4c29e6b686aae5b6d0751e90208592ea96d26bc81d78b0d3871a94a21fa8021f74dc2f971e438ccece8699c8fd15704c41df219ab37b63264f2147d15c34d801", "01", "6321024cf55e52ec8af7866617dc4e7ff8433758e98799906d80e066c6f32033f685f967029000b275210214827893e2dcbe4ad6c20bd743288edad21100404eb7f52ccd6062fd0e7808f268ac"}, "002089e84892873c679b1129edea246e484fd914c2601f776d4f2f4a001eb8059703", 318); +} + +BOOST_AUTO_TEST_SUITE_END() +} // namespace feebumper +} // namespace wallet |