From 1284223691127e76135a46d251c52416104f0ff1 Mon Sep 17 00:00:00 2001 From: furszy Date: Wed, 4 Jan 2023 10:22:53 -0300 Subject: wallet: refactor coin selection algos to return util::Result so the selection processes can retrieve different errors and not uninformative std::nullopt --- src/wallet/coinselection.cpp | 14 +++++++------- src/wallet/coinselection.h | 7 ++++--- src/wallet/spend.cpp | 19 ++++++++++++++----- 3 files changed, 25 insertions(+), 15 deletions(-) (limited to 'src/wallet') diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 3fd0280b8b..5ea9b7fad3 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -63,7 +63,7 @@ struct { static const size_t TOTAL_TRIES = 100000; -std::optional SelectCoinsBnB(std::vector& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change) +util::Result SelectCoinsBnB(std::vector& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change) { SelectionResult result(selection_target, SelectionAlgorithm::BNB); CAmount curr_value = 0; @@ -78,7 +78,7 @@ std::optional SelectCoinsBnB(std::vector& utxo_poo curr_available_value += utxo.GetSelectionAmount(); } if (curr_available_value < selection_target) { - return std::nullopt; + return util::Error(); } // Sort the utxo_pool @@ -152,7 +152,7 @@ std::optional SelectCoinsBnB(std::vector& utxo_poo // Check for solution if (best_selection.empty()) { - return std::nullopt; + return util::Error(); } // Set output set @@ -165,7 +165,7 @@ std::optional SelectCoinsBnB(std::vector& utxo_poo return result; } -std::optional SelectCoinsSRD(const std::vector& utxo_pool, CAmount target_value, FastRandomContext& rng) +util::Result SelectCoinsSRD(const std::vector& utxo_pool, CAmount target_value, FastRandomContext& rng) { SelectionResult result(target_value, SelectionAlgorithm::SRD); @@ -190,7 +190,7 @@ std::optional SelectCoinsSRD(const std::vector& ut return result; } } - return std::nullopt; + return util::Error(); } /** Find a subset of the OutputGroups that is at least as large as, but as close as possible to, the @@ -252,7 +252,7 @@ static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::v } } -std::optional KnapsackSolver(std::vector& groups, const CAmount& nTargetValue, +util::Result KnapsackSolver(std::vector& groups, const CAmount& nTargetValue, CAmount change_target, FastRandomContext& rng) { SelectionResult result(nTargetValue, SelectionAlgorithm::KNAPSACK); @@ -286,7 +286,7 @@ std::optional KnapsackSolver(std::vector& groups, } if (nTotalLower < nTargetValue) { - if (!lowest_larger) return std::nullopt; + if (!lowest_larger) return util::Error(); result.AddInput(*lowest_larger); return result; } diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index 3abd22c207..95a3bb2bc8 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -13,6 +13,7 @@ #include #include #include +#include #include @@ -408,7 +409,7 @@ public: int GetWeight() const { return m_weight; } }; -std::optional SelectCoinsBnB(std::vector& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change); +util::Result SelectCoinsBnB(std::vector& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change); /** Select coins by Single Random Draw. OutputGroups are selected randomly from the eligible * outputs until the target is satisfied @@ -417,10 +418,10 @@ std::optional SelectCoinsBnB(std::vector& utxo_poo * @param[in] target_value The target value to select for * @returns If successful, a SelectionResult, otherwise, std::nullopt */ -std::optional SelectCoinsSRD(const std::vector& utxo_pool, CAmount target_value, FastRandomContext& rng); +util::Result SelectCoinsSRD(const std::vector& utxo_pool, CAmount target_value, FastRandomContext& rng); // Original coin selection algorithm as a fallback -std::optional KnapsackSolver(std::vector& groups, const CAmount& nTargetValue, +util::Result KnapsackSolver(std::vector& groups, const CAmount& nTargetValue, CAmount change_target, FastRandomContext& rng); } // namespace wallet diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index a8ecce119a..a55e4c45a8 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -554,25 +554,34 @@ util::Result ChooseSelectionResult(const CAmount& nTargetValue, { // Vector of results. We will choose the best one based on waste. std::vector results; + std::vector> errors; + auto append_error = [&] (const util::Result& result) { + // If any specific error message appears here, then something different from a simple "no selection found" happened. + // Let's save it, so it can be retrieved to the user if no other selection algorithm succeeded. + if (HasErrorMsg(result)) { + errors.emplace_back(result); + } + }; if (auto bnb_result{SelectCoinsBnB(groups.positive_group, nTargetValue, coin_selection_params.m_cost_of_change)}) { results.push_back(*bnb_result); - } + } else append_error(bnb_result); // The knapsack solver has some legacy behavior where it will spend dust outputs. We retain this behavior, so don't filter for positive only here. if (auto knapsack_result{KnapsackSolver(groups.mixed_group, nTargetValue, coin_selection_params.m_min_change_target, coin_selection_params.rng_fast)}) { knapsack_result->ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee); results.push_back(*knapsack_result); - } + } else append_error(knapsack_result); if (auto srd_result{SelectCoinsSRD(groups.positive_group, nTargetValue, coin_selection_params.rng_fast)}) { 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); if (results.empty()) { - // No solution found - return util::Error(); + // No solution found, retrieve the first explicit error (if any). + // future: add 'severity level' to errors so the worst one can be retrieved instead of the first one. + return errors.empty() ? util::Error() : errors.front(); } std::vector eligible_results; -- cgit v1.2.3 From 6107ec2229c5f5b4e944a6b10d38010c850094ac Mon Sep 17 00:00:00 2001 From: furszy Date: Sun, 18 Dec 2022 10:28:59 -0300 Subject: coin selection: knapsack, select closest UTXO above target if result exceeds max tx size The simplest scenario where this is useful is on the 'check_max_weight' unit test already: We create 1515 UTXOs with 0.033 BTC each, and 1 UTXO with 50 BTC. Then perform Coin Selection. As the selection of the 1515 small UTXOs exceeds the max allowed tx size, the expectation here is to receive a selection result that only contain the big UTXO (which is not happening for the reasons stated below). As knapsack returns a result that exceeds the max allowed transaction size, we fallback to SRD, which selects coins randomly up until the target is met. So we end up with a selection result with lot more coins than what is needed. --- src/wallet/coinselection.cpp | 19 ++++++++++++++++++- src/wallet/coinselection.h | 2 +- src/wallet/spend.cpp | 8 +++++++- src/wallet/test/coinselector_tests.cpp | 13 ++++++++++++- src/wallet/test/fuzz/coinselection.cpp | 10 +++++++--- 5 files changed, 45 insertions(+), 7 deletions(-) (limited to 'src/wallet') diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 5ea9b7fad3..75027520dd 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -15,6 +15,13 @@ #include namespace wallet { +// Common selection error across the algorithms +static util::Result ErrorMaxWeightExceeded() +{ + return util::Error{_("The inputs size exceeds the maximum weight. " + "Please try sending a smaller amount or manually consolidating your wallet's UTXOs")}; +} + // Descending order comparator struct { bool operator()(const OutputGroup& a, const OutputGroup& b) const @@ -253,7 +260,7 @@ static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::v } util::Result KnapsackSolver(std::vector& groups, const CAmount& nTargetValue, - CAmount change_target, FastRandomContext& rng) + CAmount change_target, FastRandomContext& rng, int max_weight) { SelectionResult result(nTargetValue, SelectionAlgorithm::KNAPSACK); @@ -313,6 +320,16 @@ util::Result KnapsackSolver(std::vector& groups, c } } + // If the result exceeds the maximum allowed size, return closest UTXO above the target + if (result.GetWeight() > max_weight) { + // No coin above target, nothing to do. + if (!lowest_larger) return ErrorMaxWeightExceeded(); + + // Return closest UTXO above target + result.Clear(); + result.AddInput(*lowest_larger); + } + if (LogAcceptCategory(BCLog::SELECTCOINS, BCLog::Level::Debug)) { std::string log_message{"Coin selection best subset: "}; for (unsigned int i = 0; i < applicable_groups.size(); i++) { diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index 95a3bb2bc8..807e75ee30 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -422,7 +422,7 @@ util::Result SelectCoinsSRD(const std::vector& utx // Original coin selection algorithm as a fallback util::Result KnapsackSolver(std::vector& groups, const CAmount& nTargetValue, - CAmount change_target, FastRandomContext& rng); + CAmount change_target, FastRandomContext& rng, int max_weight); } // namespace wallet #endif // BITCOIN_WALLET_COINSELECTION_H diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index a55e4c45a8..b28da6ec42 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -563,12 +563,18 @@ util::Result ChooseSelectionResult(const CAmount& nTargetValue, } }; + // Maximum allowed weight + int max_inputs_weight = MAX_STANDARD_TX_WEIGHT - (coin_selection_params.tx_noinputs_size * WITNESS_SCALE_FACTOR); + if (auto bnb_result{SelectCoinsBnB(groups.positive_group, nTargetValue, coin_selection_params.m_cost_of_change)}) { results.push_back(*bnb_result); } else append_error(bnb_result); + // As Knapsack and SRD can create change, also deduce change weight. + max_inputs_weight -= (coin_selection_params.change_output_size * WITNESS_SCALE_FACTOR); + // The knapsack solver has some legacy behavior where it will spend dust outputs. We retain this behavior, so don't filter for positive only here. - if (auto knapsack_result{KnapsackSolver(groups.mixed_group, nTargetValue, coin_selection_params.m_min_change_target, coin_selection_params.rng_fast)}) { + if (auto knapsack_result{KnapsackSolver(groups.mixed_group, nTargetValue, coin_selection_params.m_min_change_target, coin_selection_params.rng_fast, max_inputs_weight)}) { knapsack_result->ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee); results.push_back(*knapsack_result); } else append_error(knapsack_result); diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 8f626addde..db8322e3cb 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -87,6 +87,14 @@ static void add_coin(CoinsResult& available_coins, CWallet& wallet, const CAmoun available_coins.Add(OutputType::BECH32, {COutPoint(wtx.GetHash(), nInput), txout, nAge, CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, wtx.GetTxTime(), fIsFromMe, feerate}); } +// Helper +std::optional KnapsackSolver(std::vector& groups, const CAmount& nTargetValue, + CAmount change_target, FastRandomContext& rng) +{ + auto res{KnapsackSolver(groups, nTargetValue, change_target, rng, MAX_STANDARD_TX_WEIGHT)}; + return res ? std::optional(*res) : std::nullopt; +} + /** Check if SelectionResult a is equivalent to SelectionResult b. * Equivalent means same input values, but maybe different inputs (i.e. same value, different prevout) */ static bool EquivalentResult(const SelectionResult& a, const SelectionResult& b) @@ -987,7 +995,10 @@ BOOST_AUTO_TEST_CASE(check_max_weight) chain); BOOST_CHECK(result); - BOOST_CHECK(has_coin(result->GetInputSet(), CAmount(50 * COIN))); + // Verify that only the 50 BTC UTXO was selected + const auto& selection_res = result->GetInputSet(); + BOOST_CHECK(selection_res.size() == 1); + BOOST_CHECK((*selection_res.begin())->GetEffectiveValue() == 50 * COIN); } { diff --git a/src/wallet/test/fuzz/coinselection.cpp b/src/wallet/test/fuzz/coinselection.cpp index 304190eec1..4878b24c30 100644 --- a/src/wallet/test/fuzz/coinselection.cpp +++ b/src/wallet/test/fuzz/coinselection.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include @@ -44,6 +45,9 @@ static void GroupCoins(FuzzedDataProvider& fuzzed_data_provider, const std::vect if (valid_outputgroup) output_groups.push_back(output_group); } +// Returns true if the result contains an error and the message is not empty +static bool HasErrorMsg(const util::Result& res) { return !util::ErrorString(res).empty(); } + FUZZ_TARGET(coinselection) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; @@ -90,12 +94,12 @@ FUZZ_TARGET(coinselection) 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)}; - auto result_knapsack = KnapsackSolver(group_all, target, change_target, fast_random_context); + auto result_knapsack = KnapsackSolver(group_all, target, change_target, fast_random_context, MAX_STANDARD_TX_WEIGHT); if (result_knapsack) result_knapsack->ComputeAndSetWaste(cost_of_change, cost_of_change, 0); // If the total balance is sufficient for the target and we are not using - // effective values, Knapsack should always find a solution. - if (total_balance >= target && subtract_fee_outputs) { + // effective values, Knapsack should always find a solution (unless the selection exceeded the max tx weight). + if (total_balance >= target && subtract_fee_outputs && !HasErrorMsg(result_knapsack)) { assert(result_knapsack); } } -- cgit v1.2.3 From 9d9689e5a657956db8a30829c994600ec7d3098b Mon Sep 17 00:00:00 2001 From: furszy Date: Sun, 18 Dec 2022 10:39:19 -0300 Subject: 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(-) (limited to 'src/wallet') diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 75027520dd..f97df22b0e 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 807e75ee30..06506aec0d 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 b28da6ec42..f81e91bcba 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 4878b24c30..a057eda393 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)}; -- cgit v1.2.3 From d3a1c098e4b5df2ebbae20c6e390c3d783950e93 Mon Sep 17 00:00:00 2001 From: furszy Date: Fri, 13 Jan 2023 13:23:38 -0300 Subject: test: coin selection, add coverage for SRD --- src/wallet/test/coinselector_tests.cpp | 94 ++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) (limited to 'src/wallet') diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index db8322e3cb..01ce6d76f4 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -929,6 +929,100 @@ BOOST_AUTO_TEST_CASE(effective_value_test) BOOST_CHECK_EQUAL(output5.GetEffectiveValue(), nValue); // The effective value should be equal to the absolute value if input_bytes is -1 } + +static util::Result SelectCoinsSRD(const CAmount& target, + const CoinSelectionParams& cs_params, + const node::NodeContext& m_node, + int max_weight, + std::function coin_setup) +{ + std::unique_ptr wallet = std::make_unique(m_node.chain.get(), "", CreateMockWalletDatabase()); + wallet->LoadWallet(); + LOCK(wallet->cs_wallet); + wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + wallet->SetupDescriptorScriptPubKeyMans(); + + CoinEligibilityFilter filter(0, 0, 0); // accept all coins without ancestors + Groups group = GroupOutputs(*wallet, coin_setup(*wallet), cs_params, {{filter}})[filter].all_groups; + return SelectCoinsSRD(group.positive_group, target, cs_params.rng_fast, max_weight); +} + +BOOST_AUTO_TEST_CASE(srd_tests) +{ + // Test SRD: + // 1) Insufficient funds, select all provided coins and fail. + // 2) Exceeded max weight, coin selection always surpasses the max allowed weight. + // 3) Select coins without surpassing the max weight (some coins surpasses the max allowed weight, some others not) + + FastRandomContext rand; + CoinSelectionParams dummy_params{ // Only used to provide the 'avoid_partial' flag. + rand, + /*change_output_size=*/34, + /*change_spend_size=*/68, + /*min_change_target=*/CENT, + /*effective_feerate=*/CFeeRate(0), + /*long_term_feerate=*/CFeeRate(0), + /*discard_feerate=*/CFeeRate(0), + /*tx_noinputs_size=*/10 + 34, // static header size + output size + /*avoid_partial=*/false, + }; + + { + // ######################################################### + // 1) Insufficient funds, select all provided coins and fail + // ######################################################### + CAmount target = 49.5L * COIN; + int max_weight = 10000; // high enough to not fail for this reason. + const auto& res = SelectCoinsSRD(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) { + CoinsResult available_coins; + for (int j = 0; j < 10; ++j) { + add_coin(available_coins, wallet, CAmount(1 * COIN)); + add_coin(available_coins, wallet, CAmount(2 * COIN)); + } + return available_coins; + }); + BOOST_CHECK(!res); + BOOST_CHECK(util::ErrorString(res).empty()); // empty means "insufficient funds" + } + + { + // ########################### + // 2) Test max weight exceeded + // ########################### + CAmount target = 49.5L * COIN; + int max_weight = 3000; + const auto& res = SelectCoinsSRD(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) { + CoinsResult available_coins; + for (int j = 0; j < 10; ++j) { + add_coin(available_coins, wallet, CAmount(1 * COIN), CFeeRate(0), 144, false, 0, true); + add_coin(available_coins, wallet, CAmount(2 * COIN), CFeeRate(0), 144, false, 0, true); + } + return available_coins; + }); + BOOST_CHECK(!res); + BOOST_CHECK(util::ErrorString(res).original.find("The inputs size exceeds the maximum weight") != std::string::npos); + } + + { + // ################################################################################################################ + // 3) Test selection when some coins surpass the max allowed weight while others not. --> must find a good solution + // ################################################################################################################ + CAmount target = 25.33L * COIN; + int max_weight = 10000; // WU + const auto& res = SelectCoinsSRD(target, dummy_params, m_node, max_weight, [&](CWallet& wallet) { + CoinsResult available_coins; + for (int j = 0; j < 60; ++j) { // 60 UTXO --> 19,8 BTC total --> 60 × 272 WU = 16320 WU + add_coin(available_coins, wallet, CAmount(0.33 * COIN), CFeeRate(0), 144, false, 0, true); + } + for (int i = 0; i < 10; i++) { // 10 UTXO --> 20 BTC total --> 10 × 272 WU = 2720 WU + add_coin(available_coins, wallet, CAmount(2 * COIN), CFeeRate(0), 144, false, 0, true); + } + return available_coins; + }); + BOOST_CHECK(res); + } +} + static util::Result select_coins(const CAmount& target, const CoinSelectionParams& cs_params, const CCoinControl& cc, std::function coin_setup, interfaces::Chain* chain) { std::unique_ptr wallet = std::make_unique(chain, "", CreateMockWalletDatabase()); -- cgit v1.2.3 From 2d112584e384de10021c64e4700455d71326824e Mon Sep 17 00:00:00 2001 From: furszy Date: Sun, 18 Dec 2022 20:16:19 -0300 Subject: coin selection: BnB, don't return selection if exceeds max allowed tx weight --- src/wallet/coinselection.cpp | 12 ++++++++++-- src/wallet/coinselection.h | 3 ++- src/wallet/spend.cpp | 2 +- src/wallet/test/coinselector_tests.cpp | 8 +++++++- src/wallet/test/fuzz/coinselection.cpp | 2 +- 5 files changed, 21 insertions(+), 6 deletions(-) (limited to 'src/wallet') diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index f97df22b0e..aa977de6c3 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -71,11 +71,13 @@ struct { static const size_t TOTAL_TRIES = 100000; -util::Result SelectCoinsBnB(std::vector& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change) +util::Result SelectCoinsBnB(std::vector& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change, + int max_weight) { SelectionResult result(selection_target, SelectionAlgorithm::BNB); CAmount curr_value = 0; std::vector curr_selection; // selected utxo indexes + int curr_selection_weight = 0; // sum of selected utxo weight // Calculate curr_available_value CAmount curr_available_value = 0; @@ -97,6 +99,7 @@ util::Result SelectCoinsBnB(std::vector& utxo_pool CAmount best_waste = MAX_MONEY; bool is_feerate_high = utxo_pool.at(0).fee > utxo_pool.at(0).long_term_fee; + bool max_tx_weight_exceeded = false; // Depth First search loop for choosing the UTXOs for (size_t curr_try = 0, utxo_pool_index = 0; curr_try < TOTAL_TRIES; ++curr_try, ++utxo_pool_index) { @@ -106,6 +109,9 @@ util::Result SelectCoinsBnB(std::vector& utxo_pool curr_value > selection_target + cost_of_change || // Selected value is out of range, go back and try other branch (curr_waste > best_waste && is_feerate_high)) { // Don't select things which we know will be more wasteful if the waste is increasing backtrack = true; + } else if (curr_selection_weight > max_weight) { // Exceeding weight for standard tx, cannot find more solutions by adding more inputs + max_tx_weight_exceeded = true; // at least one selection attempt exceeded the max weight + backtrack = true; } else if (curr_value >= selection_target) { // Selected value is within range curr_waste += (curr_value - selection_target); // This is the excess value which is added to the waste for the below comparison // Adding another UTXO after this check could bring the waste down if the long term fee is higher than the current fee. @@ -135,6 +141,7 @@ util::Result SelectCoinsBnB(std::vector& utxo_pool OutputGroup& utxo = utxo_pool.at(utxo_pool_index); curr_value -= utxo.GetSelectionAmount(); curr_waste -= utxo.fee - utxo.long_term_fee; + curr_selection_weight -= utxo.m_weight; curr_selection.pop_back(); } else { // Moving forwards, continuing down this branch OutputGroup& utxo = utxo_pool.at(utxo_pool_index); @@ -154,13 +161,14 @@ util::Result SelectCoinsBnB(std::vector& utxo_pool curr_selection.push_back(utxo_pool_index); curr_value += utxo.GetSelectionAmount(); curr_waste += utxo.fee - utxo.long_term_fee; + curr_selection_weight += utxo.m_weight; } } } // Check for solution if (best_selection.empty()) { - return util::Error(); + return max_tx_weight_exceeded ? ErrorMaxWeightExceeded() : util::Error(); } // Set output set diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index 06506aec0d..4862efada8 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -409,7 +409,8 @@ public: int GetWeight() const { return m_weight; } }; -util::Result SelectCoinsBnB(std::vector& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change); +util::Result SelectCoinsBnB(std::vector& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change, + int max_weight); /** Select coins by Single Random Draw. OutputGroups are selected randomly from the eligible * outputs until the target is satisfied diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index f81e91bcba..57fb7202bb 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -566,7 +566,7 @@ util::Result ChooseSelectionResult(const CAmount& nTargetValue, // Maximum allowed weight int max_inputs_weight = MAX_STANDARD_TX_WEIGHT - (coin_selection_params.tx_noinputs_size * WITNESS_SCALE_FACTOR); - if (auto bnb_result{SelectCoinsBnB(groups.positive_group, nTargetValue, coin_selection_params.m_cost_of_change)}) { + if (auto bnb_result{SelectCoinsBnB(groups.positive_group, nTargetValue, coin_selection_params.m_cost_of_change, max_inputs_weight)}) { results.push_back(*bnb_result); } else append_error(bnb_result); diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 01ce6d76f4..f511a449ed 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -87,7 +87,7 @@ static void add_coin(CoinsResult& available_coins, CWallet& wallet, const CAmoun available_coins.Add(OutputType::BECH32, {COutPoint(wtx.GetHash(), nInput), txout, nAge, CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, wtx.GetTxTime(), fIsFromMe, feerate}); } -// Helper +// Helpers std::optional KnapsackSolver(std::vector& groups, const CAmount& nTargetValue, CAmount change_target, FastRandomContext& rng) { @@ -95,6 +95,12 @@ std::optional KnapsackSolver(std::vector& groups, return res ? std::optional(*res) : std::nullopt; } +std::optional SelectCoinsBnB(std::vector& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change) +{ + auto res{SelectCoinsBnB(utxo_pool, selection_target, cost_of_change, MAX_STANDARD_TX_WEIGHT)}; + return res ? std::optional(*res) : std::nullopt; +} + /** Check if SelectionResult a is equivalent to SelectionResult b. * Equivalent means same input values, but maybe different inputs (i.e. same value, different prevout) */ static bool EquivalentResult(const SelectionResult& a, const SelectionResult& b) diff --git a/src/wallet/test/fuzz/coinselection.cpp b/src/wallet/test/fuzz/coinselection.cpp index a057eda393..9be8efab62 100644 --- a/src/wallet/test/fuzz/coinselection.cpp +++ b/src/wallet/test/fuzz/coinselection.cpp @@ -88,7 +88,7 @@ FUZZ_TARGET(coinselection) GroupCoins(fuzzed_data_provider, utxo_pool, coin_params, /*positive_only=*/false, group_all); // Run coinselection algorithms - const auto result_bnb = SelectCoinsBnB(group_pos, target, cost_of_change); + const auto result_bnb = SelectCoinsBnB(group_pos, target, cost_of_change, MAX_STANDARD_TX_WEIGHT); 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); -- cgit v1.2.3 From 5a2bc45ee0b123e461c5191322ed0b43524c3d82 Mon Sep 17 00:00:00 2001 From: furszy Date: Sun, 18 Dec 2022 21:10:07 -0300 Subject: wallet: clean post coin selection max weight filter Now the coin selection algorithms contemplate the maximum allowed weight internally and return std::nullopt if their result exceeds it. --- src/wallet/spend.cpp | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) (limited to 'src/wallet') diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 57fb7202bb..140b7720c7 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -590,21 +590,9 @@ util::Result ChooseSelectionResult(const CAmount& nTargetValue, return errors.empty() ? util::Error() : errors.front(); } - std::vector 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(MAX_STANDARD_TX_WEIGHT); - }); - - if (eligible_results.empty()) { - return util::Error{_("The inputs size exceeds the maximum weight. " - "Please try sending a smaller amount or manually consolidating your wallet's UTXOs")}; - } - // 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(eligible_results.begin(), eligible_results.end()); - return best_result; + return *std::min_element(results.begin(), results.end()); } util::Result SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const PreSelectedInputs& pre_set_inputs, -- cgit v1.2.3 From ba9431c505e1590db6103b9632134985cd4704dc Mon Sep 17 00:00:00 2001 From: furszy Date: Thu, 19 Jan 2023 22:27:48 -0300 Subject: test: coverage for bnb max weight Basic positive and negative scenarios --- src/wallet/test/coinselector_tests.cpp | 45 +++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 4 deletions(-) (limited to 'src/wallet') diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index f511a449ed..f325fe43ca 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -68,7 +68,7 @@ static void add_coin(const CAmount& nValue, int nInput, CoinSet& set, CAmount fe set.insert(std::make_shared(coin)); } -static void add_coin(CoinsResult& available_coins, CWallet& wallet, const CAmount& nValue, CFeeRate feerate = CFeeRate(0), int nAge = 6*24, bool fIsFromMe = false, int nInput =0, bool spendable = false) +static void add_coin(CoinsResult& available_coins, CWallet& wallet, const CAmount& nValue, CFeeRate feerate = CFeeRate(0), int nAge = 6*24, bool fIsFromMe = false, int nInput =0, bool spendable = false, int custom_size = 0) { CMutableTransaction tx; tx.nLockTime = nextLockTime++; // so all transactions get different hashes @@ -84,7 +84,7 @@ static void add_coin(CoinsResult& available_coins, CWallet& wallet, const CAmoun assert(ret.second); CWalletTx& wtx = (*ret.first).second; const auto& txout = wtx.tx->vout.at(nInput); - available_coins.Add(OutputType::BECH32, {COutPoint(wtx.GetHash(), nInput), txout, nAge, CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, wtx.GetTxTime(), fIsFromMe, feerate}); + available_coins.Add(OutputType::BECH32, {COutPoint(wtx.GetHash(), nInput), txout, nAge, custom_size == 0 ? CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr) : custom_size, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, wtx.GetTxTime(), fIsFromMe, feerate}); } // Helpers @@ -142,13 +142,15 @@ static CAmount make_hard_case(int utxos, std::vector& utxo_pool) return target; } -inline std::vector& GroupCoins(const std::vector& available_coins) +inline std::vector& GroupCoins(const std::vector& available_coins, bool subtract_fee_outputs = false) { static std::vector static_groups; static_groups.clear(); for (auto& coin : available_coins) { static_groups.emplace_back(); - static_groups.back().Insert(std::make_shared(coin), /*ancestors=*/ 0, /*descendants=*/ 0); + OutputGroup& group = static_groups.back(); + group.Insert(std::make_shared(coin), /*ancestors=*/ 0, /*descendants=*/ 0); + group.m_subtract_fee_outputs = subtract_fee_outputs; } return static_groups; } @@ -411,6 +413,41 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) const auto result13 = SelectCoins(*wallet, available_coins, selected_input, 10 * CENT, coin_control, coin_selection_params_bnb); BOOST_CHECK(EquivalentResult(expected_result, *result13)); } + + { + // Test bnb max weight exceeded + // Inputs set [10, 9, 8, 5, 3, 1], Selection Target = 16 and coin 5 exceeding the max weight. + + std::unique_ptr wallet = std::make_unique(m_node.chain.get(), "", m_args, CreateMockWalletDatabase()); + wallet->LoadWallet(); + LOCK(wallet->cs_wallet); + wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + wallet->SetupDescriptorScriptPubKeyMans(); + + CoinsResult available_coins; + add_coin(available_coins, *wallet, 10 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true); + add_coin(available_coins, *wallet, 9 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true); + add_coin(available_coins, *wallet, 8 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true); + add_coin(available_coins, *wallet, 5 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true, /*custom_size=*/MAX_STANDARD_TX_WEIGHT); + add_coin(available_coins, *wallet, 3 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true); + add_coin(available_coins, *wallet, 1 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true); + + CAmount selection_target = 16 * CENT; + const auto& no_res = SelectCoinsBnB(GroupCoins(available_coins.All(), /*subtract_fee_outputs*/true), + selection_target, /*cost_of_change=*/0, MAX_STANDARD_TX_WEIGHT); + BOOST_ASSERT(!no_res); + BOOST_CHECK(util::ErrorString(no_res).original.find("The inputs size exceeds the maximum weight") != std::string::npos); + + // Now add same coin value with a good size and check that it gets selected + add_coin(available_coins, *wallet, 5 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true); + const auto& res = SelectCoinsBnB(GroupCoins(available_coins.All(), /*subtract_fee_outputs*/true), selection_target, /*cost_of_change=*/0); + + expected_result.Clear(); + add_coin(8 * CENT, 2, expected_result); + add_coin(5 * CENT, 2, expected_result); + add_coin(3 * CENT, 2, expected_result); + BOOST_CHECK(EquivalentResult(expected_result, *res)); + } } BOOST_AUTO_TEST_CASE(knapsack_solver_test) -- cgit v1.2.3 From 25ab14712b9d80276016f9fc9bff7fb9c1d09635 Mon Sep 17 00:00:00 2001 From: furszy Date: Thu, 19 Jan 2023 22:45:48 -0300 Subject: refactor: coinselector_tests, unify wallet creation code same lines of code repeated across the entire file over and over. --- src/wallet/test/coinselector_tests.cpp | 100 ++++++++++----------------------- 1 file changed, 31 insertions(+), 69 deletions(-) (limited to 'src/wallet') diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index f325fe43ca..7f66179517 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -174,6 +174,16 @@ inline std::vector& KnapsackGroupOutputs(const CoinsResult& availab return static_groups.all_groups.mixed_group; } +static std::unique_ptr NewWallet(const node::NodeContext& m_node, const std::string& wallet_name = "") +{ + std::unique_ptr wallet = std::make_unique(m_node.chain.get(), wallet_name, CreateMockWalletDatabase()); + BOOST_CHECK(wallet->LoadWallet() == DBErrors::LOAD_OK); + LOCK(wallet->cs_wallet); + wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + wallet->SetupDescriptorScriptPubKeyMans(); + return wallet; +} + // Branch and bound coin selection tests BOOST_AUTO_TEST_CASE(bnb_search_test) { @@ -310,11 +320,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) coin_selection_params_bnb.m_subtract_fee_outputs = true; { - std::unique_ptr wallet = std::make_unique(m_node.chain.get(), "", CreateMockWalletDatabase()); - wallet->LoadWallet(); - LOCK(wallet->cs_wallet); - wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); - wallet->SetupDescriptorScriptPubKeyMans(); + std::unique_ptr wallet = NewWallet(m_node); CoinsResult available_coins; @@ -332,11 +338,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) } { - std::unique_ptr wallet = std::make_unique(m_node.chain.get(), "", CreateMockWalletDatabase()); - wallet->LoadWallet(); - LOCK(wallet->cs_wallet); - wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); - wallet->SetupDescriptorScriptPubKeyMans(); + std::unique_ptr wallet = NewWallet(m_node); CoinsResult available_coins; @@ -351,15 +353,13 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) selected_input.Insert(select_coin, coin_selection_params_bnb.m_subtract_fee_outputs); available_coins.Erase({available_coins.coins[OutputType::BECH32].begin()->outpoint}); coin_selection_params_bnb.m_effective_feerate = CFeeRate(0); + LOCK(wallet->cs_wallet); const auto result10 = SelectCoins(*wallet, available_coins, selected_input, 10 * CENT, coin_control, coin_selection_params_bnb); BOOST_CHECK(result10); } { - std::unique_ptr wallet = std::make_unique(m_node.chain.get(), "", CreateMockWalletDatabase()); - wallet->LoadWallet(); - LOCK(wallet->cs_wallet); - wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); - wallet->SetupDescriptorScriptPubKeyMans(); + std::unique_ptr wallet = NewWallet(m_node); + LOCK(wallet->cs_wallet); // Every 'SelectCoins' call requires it CoinsResult available_coins; @@ -418,11 +418,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) // Test bnb max weight exceeded // Inputs set [10, 9, 8, 5, 3, 1], Selection Target = 16 and coin 5 exceeding the max weight. - std::unique_ptr wallet = std::make_unique(m_node.chain.get(), "", m_args, CreateMockWalletDatabase()); - wallet->LoadWallet(); - LOCK(wallet->cs_wallet); - wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); - wallet->SetupDescriptorScriptPubKeyMans(); + std::unique_ptr wallet = NewWallet(m_node); CoinsResult available_coins; add_coin(available_coins, *wallet, 10 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true); @@ -455,11 +451,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) FastRandomContext rand{}; const auto temp1{[&rand](std::vector& g, const CAmount& v, CAmount c) { return KnapsackSolver(g, v, c, rand); }}; const auto KnapsackSolver{temp1}; - std::unique_ptr wallet = std::make_unique(m_node.chain.get(), "", CreateMockWalletDatabase()); - wallet->LoadWallet(); - LOCK(wallet->cs_wallet); - wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); - wallet->SetupDescriptorScriptPubKeyMans(); + std::unique_ptr wallet = NewWallet(m_node); CoinsResult available_coins; @@ -765,11 +757,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) BOOST_AUTO_TEST_CASE(ApproximateBestSubset) { FastRandomContext rand{}; - std::unique_ptr wallet = std::make_unique(m_node.chain.get(), "", CreateMockWalletDatabase()); - wallet->LoadWallet(); - LOCK(wallet->cs_wallet); - wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); - wallet->SetupDescriptorScriptPubKeyMans(); + std::unique_ptr wallet = NewWallet(m_node); CoinsResult available_coins; @@ -787,11 +775,8 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) // Tests that with the ideal conditions, the coin selector will always be able to find a solution that can pay the target value BOOST_AUTO_TEST_CASE(SelectCoins_test) { - std::unique_ptr wallet = std::make_unique(m_node.chain.get(), "", CreateMockWalletDatabase()); - wallet->LoadWallet(); - LOCK(wallet->cs_wallet); - wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); - wallet->SetupDescriptorScriptPubKeyMans(); + std::unique_ptr wallet = NewWallet(m_node); + LOCK(wallet->cs_wallet); // Every 'SelectCoins' call requires it // Random generator stuff std::default_random_engine generator; @@ -979,12 +964,7 @@ static util::Result SelectCoinsSRD(const CAmount& target, int max_weight, std::function coin_setup) { - std::unique_ptr wallet = std::make_unique(m_node.chain.get(), "", CreateMockWalletDatabase()); - wallet->LoadWallet(); - LOCK(wallet->cs_wallet); - wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); - wallet->SetupDescriptorScriptPubKeyMans(); - + std::unique_ptr wallet = NewWallet(m_node); CoinEligibilityFilter filter(0, 0, 0); // accept all coins without ancestors Groups group = GroupOutputs(*wallet, coin_setup(*wallet), cs_params, {{filter}})[filter].all_groups; return SelectCoinsSRD(group.positive_group, target, cs_params.rng_fast, max_weight); @@ -1066,16 +1046,12 @@ BOOST_AUTO_TEST_CASE(srd_tests) } } -static util::Result select_coins(const CAmount& target, const CoinSelectionParams& cs_params, const CCoinControl& cc, std::function coin_setup, interfaces::Chain* chain) +static util::Result select_coins(const CAmount& target, const CoinSelectionParams& cs_params, const CCoinControl& cc, std::function coin_setup, const node::NodeContext& m_node) { - std::unique_ptr wallet = std::make_unique(chain, "", CreateMockWalletDatabase()); - wallet->LoadWallet(); - LOCK(wallet->cs_wallet); - wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); - wallet->SetupDescriptorScriptPubKeyMans(); - + std::unique_ptr wallet = NewWallet(m_node); auto available_coins = coin_setup(*wallet); + LOCK(wallet->cs_wallet); auto result = SelectCoins(*wallet, available_coins, /*pre_set_inputs=*/ {}, target, cc, cs_params); if (result) { const auto signedTxSize = 10 + 34 + 68 * result->GetInputSet().size(); // static header size + output size + inputs size (P2WPKH) @@ -1109,8 +1085,6 @@ BOOST_AUTO_TEST_CASE(check_max_weight) /*avoid_partial=*/false, }; - auto chain{m_node.chain.get()}; - { // Scenario 1: // The actor starts with 1x 50.0 BTC and 1515x 0.033 BTC (~100.0 BTC total) unspent outputs @@ -1129,7 +1103,7 @@ BOOST_AUTO_TEST_CASE(check_max_weight) add_coin(available_coins, wallet, CAmount(50 * COIN), CFeeRate(0), 144, false, 0, true); return available_coins; }, - chain); + m_node); BOOST_CHECK(result); // Verify that only the 50 BTC UTXO was selected @@ -1157,7 +1131,7 @@ BOOST_AUTO_TEST_CASE(check_max_weight) } return available_coins; }, - chain); + m_node); BOOST_CHECK(has_coin(result->GetInputSet(), CAmount(0.0625 * COIN))); BOOST_CHECK(has_coin(result->GetInputSet(), CAmount(0.025 * COIN))); @@ -1178,7 +1152,7 @@ BOOST_AUTO_TEST_CASE(check_max_weight) } return available_coins; }, - chain); + m_node); // No results // 1515 inputs * 68 bytes = 103,020 bytes @@ -1193,20 +1167,11 @@ BOOST_AUTO_TEST_CASE(SelectCoins_effective_value_test) // This test creates a coin whose value is higher than the target but whose effective value is lower than the target. // The coin is selected using coin control, with m_allow_other_inputs = false. SelectCoins should fail due to insufficient funds. - std::unique_ptr wallet = std::make_unique(m_node.chain.get(), "", CreateMockWalletDatabase()); - wallet->LoadWallet(); - LOCK(wallet->cs_wallet); - wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); - wallet->SetupDescriptorScriptPubKeyMans(); + std::unique_ptr wallet = NewWallet(m_node); CoinsResult available_coins; { - std::unique_ptr dummyWallet = std::make_unique(m_node.chain.get(), "dummy", CreateMockWalletDatabase()); - dummyWallet->LoadWallet(); - LOCK(dummyWallet->cs_wallet); - dummyWallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); - dummyWallet->SetupDescriptorScriptPubKeyMans(); - + std::unique_ptr dummyWallet = NewWallet(m_node, /*wallet_name=*/"dummy"); add_coin(available_coins, *dummyWallet, 100000); // 0.001 BTC } @@ -1230,6 +1195,7 @@ BOOST_AUTO_TEST_CASE(SelectCoins_effective_value_test) cc.SetInputWeight(output.outpoint, 148); cc.SelectExternal(output.outpoint, output.txout); + LOCK(wallet->cs_wallet); const auto preset_inputs = *Assert(FetchSelectedInputs(*wallet, cc, cs_params)); available_coins.Erase({available_coins.coins[OutputType::BECH32].begin()->outpoint}); @@ -1242,11 +1208,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_coinsresult_test, BasicTestingSetup) // Test case to verify CoinsResult object sanity. CoinsResult available_coins; { - std::unique_ptr dummyWallet = std::make_unique(m_node.chain.get(), "dummy", CreateMockWalletDatabase()); - BOOST_CHECK_EQUAL(dummyWallet->LoadWallet(), DBErrors::LOAD_OK); - LOCK(dummyWallet->cs_wallet); - dummyWallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); - dummyWallet->SetupDescriptorScriptPubKeyMans(); + std::unique_ptr dummyWallet = NewWallet(m_node, /*wallet_name=*/"dummy"); // Add some coins to 'available_coins' for (int i=0; i<10; i++) { -- cgit v1.2.3