From 9d9689e5a657956db8a30829c994600ec7d3098b Mon Sep 17 00:00:00 2001 From: furszy Date: Sun, 18 Dec 2022 10:39:19 -0300 Subject: [PATCH] coin selection: heap-ify SRD, don't return selection if exceeds max tx weight Uses a min-effective-value heap, so we can remove the least valuable input/s while the selected weight exceeds the maximum allowed weight. Co-authored-by: Murch --- src/wallet/coinselection.cpp | 42 ++++++++++++++++++++++++-- src/wallet/coinselection.h | 7 +++-- src/wallet/spend.cpp | 2 +- src/wallet/test/fuzz/coinselection.cpp | 2 +- 4 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 75027520dd9..f97df22b0e8 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -13,6 +13,7 @@ #include #include +#include namespace wallet { // Common selection error across the algorithms @@ -172,9 +173,20 @@ util::Result SelectCoinsBnB(std::vector& utxo_pool return result; } -util::Result SelectCoinsSRD(const std::vector& utxo_pool, CAmount target_value, FastRandomContext& rng) +class MinOutputGroupComparator +{ +public: + int operator() (const OutputGroup& group1, const OutputGroup& group2) const + { + return group1.GetSelectionAmount() > group2.GetSelectionAmount(); + } +}; + +util::Result SelectCoinsSRD(const std::vector& utxo_pool, CAmount target_value, FastRandomContext& rng, + int max_weight) { SelectionResult result(target_value, SelectionAlgorithm::SRD); + std::priority_queue, MinOutputGroupComparator> heap; // Include change for SRD as we want to avoid making really small change if the selection just // barely meets the target. Just use the lower bound change target instead of the randomly @@ -188,16 +200,40 @@ util::Result SelectCoinsSRD(const std::vector& utx Shuffle(indexes.begin(), indexes.end(), rng); CAmount selected_eff_value = 0; + int weight = 0; + bool max_tx_weight_exceeded = false; for (const size_t i : indexes) { const OutputGroup& group = utxo_pool.at(i); Assume(group.GetSelectionAmount() > 0); + + // Add group to selection + heap.push(group); selected_eff_value += group.GetSelectionAmount(); - result.AddInput(group); + weight += group.m_weight; + + // If the selection weight exceeds the maximum allowed size, remove the least valuable inputs until we + // are below max weight. + if (weight > max_weight) { + max_tx_weight_exceeded = true; // mark it in case we don't find any useful result. + do { + const OutputGroup& to_remove_group = heap.top(); + selected_eff_value -= to_remove_group.GetSelectionAmount(); + weight -= to_remove_group.m_weight; + heap.pop(); + } while (!heap.empty() && weight > max_weight); + } + + // Now check if we are above the target if (selected_eff_value >= target_value) { + // Result found, add it. + while (!heap.empty()) { + result.AddInput(heap.top()); + heap.pop(); + } return result; } } - return util::Error(); + return max_tx_weight_exceeded ? ErrorMaxWeightExceeded() : util::Error(); } /** Find a subset of the OutputGroups that is at least as large as, but as close as possible to, the diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index 807e75ee30d..06506aec0d3 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -416,9 +416,12 @@ util::Result SelectCoinsBnB(std::vector& utxo_pool * * @param[in] utxo_pool The positive effective value OutputGroups eligible for selection * @param[in] target_value The target value to select for - * @returns If successful, a SelectionResult, otherwise, std::nullopt + * @param[in] rng The randomness source to shuffle coins + * @param[in] max_weight The maximum allowed weight for a selection result to be valid + * @returns If successful, a valid SelectionResult, otherwise, util::Error */ -util::Result SelectCoinsSRD(const std::vector& utxo_pool, CAmount target_value, FastRandomContext& rng); +util::Result SelectCoinsSRD(const std::vector& utxo_pool, CAmount target_value, FastRandomContext& rng, + int max_weight); // Original coin selection algorithm as a fallback util::Result KnapsackSolver(std::vector& groups, const CAmount& nTargetValue, diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index b28da6ec42d..f81e91bcba6 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -579,7 +579,7 @@ util::Result ChooseSelectionResult(const CAmount& nTargetValue, results.push_back(*knapsack_result); } else append_error(knapsack_result); - if (auto srd_result{SelectCoinsSRD(groups.positive_group, nTargetValue, coin_selection_params.rng_fast)}) { + if (auto srd_result{SelectCoinsSRD(groups.positive_group, nTargetValue, coin_selection_params.rng_fast, max_inputs_weight)}) { srd_result->ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee); results.push_back(*srd_result); } else append_error(srd_result); diff --git a/src/wallet/test/fuzz/coinselection.cpp b/src/wallet/test/fuzz/coinselection.cpp index 4878b24c301..a057eda393b 100644 --- a/src/wallet/test/fuzz/coinselection.cpp +++ b/src/wallet/test/fuzz/coinselection.cpp @@ -90,7 +90,7 @@ FUZZ_TARGET(coinselection) // Run coinselection algorithms const auto result_bnb = SelectCoinsBnB(group_pos, target, cost_of_change); - auto result_srd = SelectCoinsSRD(group_pos, target, fast_random_context); + auto result_srd = SelectCoinsSRD(group_pos, target, fast_random_context, MAX_STANDARD_TX_WEIGHT); if (result_srd) result_srd->ComputeAndSetWaste(cost_of_change, cost_of_change, 0); CAmount change_target{GenerateChangeTarget(target, coin_params.m_change_fee, fast_random_context)};