diff options
author | Aurèle Oulès <aurele@oules.com> | 2022-10-04 14:17:11 +0200 |
---|---|---|
committer | Aurèle Oulès <aurele@oules.com> | 2022-12-05 19:32:11 +0100 |
commit | 6b563cae92957dc30dc35103a7c321fdb0115ef3 (patch) | |
tree | c5c1b2cffe292a60c9be010733758c1a93851b1e | |
parent | 7734a0160debfeea039b77cca1366cd9ee83feb6 (diff) | |
download | bitcoin-6b563cae92957dc30dc35103a7c321fdb0115ef3.tar.xz |
wallet: Check max tx weight in coin selector
Co-authored-by: Andrew Chow <github@achow101.com>
-rw-r--r-- | src/wallet/coinselection.cpp | 14 | ||||
-rw-r--r-- | src/wallet/coinselection.h | 7 | ||||
-rw-r--r-- | src/wallet/spend.cpp | 29 | ||||
-rwxr-xr-x | test/functional/rpc_fundrawtransaction.py | 2 |
4 files changed, 41 insertions, 11 deletions
diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index b889d31928..ba1d60fe44 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -5,6 +5,7 @@ #include <wallet/coinselection.h> #include <consensus/amount.h> +#include <consensus/consensus.h> #include <policy/feerate.h> #include <util/check.h> #include <util/system.h> @@ -354,6 +355,10 @@ void OutputGroup::Insert(const COutput& output, size_t ancestors, size_t descend // descendants is the count as seen from the top ancestor, not the descendants as seen from the // coin itself; thus, this value is counted as the max, not the sum m_descendants = std::max(m_descendants, descendants); + + if (output.input_bytes > 0) { + m_weight += output.input_bytes * WITNESS_SCALE_FACTOR; + } } bool OutputGroup::EligibleForSpending(const CoinEligibilityFilter& eligibility_filter) const @@ -436,18 +441,25 @@ void SelectionResult::Clear() { m_selected_inputs.clear(); m_waste.reset(); + m_weight = 0; } void SelectionResult::AddInput(const OutputGroup& group) { util::insert(m_selected_inputs, group.m_outputs); m_use_effective = !group.m_subtract_fee_outputs; + + m_weight += group.m_weight; } void SelectionResult::AddInputs(const std::set<COutput>& inputs, bool subtract_fee_outputs) { util::insert(m_selected_inputs, inputs); m_use_effective = !subtract_fee_outputs; + + m_weight += std::accumulate(inputs.cbegin(), inputs.cend(), 0, [](int sum, const auto& coin) { + return sum + std::max(coin.input_bytes, 0) * WITNESS_SCALE_FACTOR; + }); } void SelectionResult::Merge(const SelectionResult& other) @@ -462,6 +474,8 @@ void SelectionResult::Merge(const SelectionResult& other) } util::insert(m_selected_inputs, other.m_selected_inputs); assert(m_selected_inputs.size() == expected_count); + + m_weight += other.m_weight; } const std::set<COutput>& SelectionResult::GetInputSet() const diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index 2ab4061a9b..ecfc5bfd6b 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -6,6 +6,7 @@ #define BITCOIN_WALLET_COINSELECTION_H #include <consensus/amount.h> +#include <consensus/consensus.h> #include <policy/feerate.h> #include <primitives/transaction.h> #include <random.h> @@ -223,6 +224,8 @@ struct OutputGroup /** Indicate that we are subtracting the fee from outputs. * When true, the value that is used for coin selection is the UTXO's real value rather than effective value */ bool m_subtract_fee_outputs{false}; + /** Total weight of the UTXOs in this group. */ + int m_weight{0}; OutputGroup() {} OutputGroup(const CoinSelectionParams& params) : @@ -295,6 +298,8 @@ private: bool m_use_effective{false}; /** The computed waste */ std::optional<CAmount> m_waste; + /** Total weight of the selected inputs */ + int m_weight{0}; public: explicit SelectionResult(const CAmount target, SelectionAlgorithm algo) @@ -353,6 +358,8 @@ public: CAmount GetTarget() const { return m_target; } SelectionAlgorithm GetAlgo() const { return m_algo; } + + int GetWeight() const { return m_weight; } }; std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change); diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 52d10b74b5..1724b11268 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -2,9 +2,11 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <algorithm> #include <consensus/amount.h> #include <consensus/validation.h> #include <interfaces/chain.h> +#include <numeric> #include <policy/policy.h> #include <script/signingprovider.h> #include <util/check.h> @@ -555,14 +557,24 @@ std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, cons results.push_back(*srd_result); } - if (results.size() == 0) { + if (results.empty()) { // No solution found return std::nullopt; } + std::vector<SelectionResult> eligible_results; + std::copy_if(results.begin(), results.end(), std::back_inserter(eligible_results), [coin_selection_params](const SelectionResult& result) { + const auto initWeight{coin_selection_params.tx_noinputs_size * WITNESS_SCALE_FACTOR}; + return initWeight + result.GetWeight() <= static_cast<int>(MAX_STANDARD_TX_WEIGHT); + }); + + if (eligible_results.empty()) { + return std::nullopt; + } + // Choose the result with the least waste // If the waste is the same, choose the one which spends more inputs. - auto& best_result = *std::min_element(results.begin(), results.end()); + auto& best_result = *std::min_element(eligible_results.begin(), eligible_results.end()); return best_result; } @@ -867,19 +879,16 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( const auto change_spend_fee = coin_selection_params.m_discard_feerate.GetFee(coin_selection_params.change_spend_size); coin_selection_params.min_viable_change = std::max(change_spend_fee + 1, dust); + // Static vsize overhead + outputs vsize. 4 nVersion, 4 nLocktime, 1 input count, 1 witness overhead (dummy, flag, stack size) + coin_selection_params.tx_noinputs_size = 10 + GetSizeOfCompactSize(vecSend.size()); // bytes for output count + // vouts to the payees - if (!coin_selection_params.m_subtract_fee_outputs) { - coin_selection_params.tx_noinputs_size = 10; // Static vsize overhead + outputs vsize. 4 nVersion, 4 nLocktime, 1 input count, 1 witness overhead (dummy, flag, stack size) - coin_selection_params.tx_noinputs_size += GetSizeOfCompactSize(vecSend.size()); // bytes for output count - } for (const auto& recipient : vecSend) { CTxOut txout(recipient.nAmount, recipient.scriptPubKey); // Include the fee cost for outputs. - if (!coin_selection_params.m_subtract_fee_outputs) { - coin_selection_params.tx_noinputs_size += ::GetSerializeSize(txout, PROTOCOL_VERSION); - } + coin_selection_params.tx_noinputs_size += ::GetSerializeSize(txout, PROTOCOL_VERSION); if (IsDust(txout, wallet.chain().relayDustFee())) { return util::Error{_("Transaction amount too small")}; @@ -888,7 +897,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( } // Include the fees for things that aren't inputs, excluding the change output - const CAmount not_input_fees = coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.tx_noinputs_size); + const CAmount not_input_fees = coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.m_subtract_fee_outputs ? 0 : coin_selection_params.tx_noinputs_size); CAmount selection_target = recipients_sum + not_input_fees; // Fetch manually selected coins diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index 6874154f35..bf218bfee9 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -987,7 +987,7 @@ class RawTransactionsTest(BitcoinTestFramework): outputs[recipient.getnewaddress()] = 0.1 wallet.sendmany("", outputs) self.generate(self.nodes[0], 10) - assert_raises_rpc_error(-4, "Transaction too large", recipient.fundrawtransaction, rawtx) + assert_raises_rpc_error(-4, "Insufficient funds", recipient.fundrawtransaction, rawtx) self.nodes[0].unloadwallet("large") def test_external_inputs(self): |