From af56d63eca8a246c02506c2aef7ea8a22e2d07bb Mon Sep 17 00:00:00 2001 From: Murch Date: Tue, 28 Jun 2022 17:27:06 -0400 Subject: [PATCH] Revert "bnb: exit selection when best_waste is 0" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 9b5950db8683f9b4be03f79ee0aae8a780b01a4b. Waste can be negative. At feerates lower than long_term_feerate this means that a waste of 0 may be a suboptimal solution and this causes the search to exit prematurely. Only when the feerate is equal to the long_term_feerate would achieving a waste of 0 indicate that we have achieved an optimal solution, because it would mean that the excess is 0. It seems unlikely that this would ever occur outside of test cases, and even then we should prefer solutions with more inputs over solutions with fewer according to previous decisions—but solutions with more inputs are found later in the branch exploration. The "optimization" described in #18257 and implemented in #18262 is therefore a premature exit on a suboptimal solution and should be reverted. --- src/wallet/coinselection.cpp | 3 --- src/wallet/test/coinselector_tests.cpp | 7 ++++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 07df8d9fc..49e6bac46 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -104,9 +104,6 @@ std::optional SelectCoinsBnB(std::vector& utxo_poo if (curr_waste <= best_waste) { best_selection = curr_selection; best_waste = curr_waste; - if (best_waste == 0) { - break; - } } curr_waste -= (curr_value - selection_target); // Remove the excess value as we will be selecting different coins now backtrack = true; diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index d6f47e995..27202cd7f 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -198,8 +198,8 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) expected_result.Clear(); // Select 5 Cent - add_coin(4 * CENT, 4, expected_result); - add_coin(1 * CENT, 1, expected_result); + add_coin(3 * CENT, 3, expected_result); + add_coin(2 * CENT, 2, expected_result); const auto result3 = SelectCoinsBnB(GroupCoins(utxo_pool), 5 * CENT, 0.5 * CENT); BOOST_CHECK(result3); BOOST_CHECK(EquivalentResult(expected_result, *result3)); @@ -224,8 +224,9 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) // Select 10 Cent add_coin(5 * CENT, 5, utxo_pool); - add_coin(5 * CENT, 5, expected_result); add_coin(4 * CENT, 4, expected_result); + add_coin(3 * CENT, 3, expected_result); + add_coin(2 * CENT, 2, expected_result); add_coin(1 * CENT, 1, expected_result); const auto result5 = SelectCoinsBnB(GroupCoins(utxo_pool), 10 * CENT, 0.5 * CENT); BOOST_CHECK(result5);