Skip to content

Commit

Permalink
Revert "bnb: exit selection when best_waste is 0"
Browse files Browse the repository at this point in the history
This reverts commit 9b5950d.

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 bitcoin#18257 and implemented in bitcoin#18262 is
therefore a premature exit on a suboptimal solution and should be reverted.
  • Loading branch information
murchandamus committed Jun 28, 2022
1 parent 480d806 commit af56d63
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 6 deletions.
3 changes: 0 additions & 3 deletions src/wallet/coinselection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,6 @@ std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& 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;
Expand Down
7 changes: 4 additions & 3 deletions src/wallet/test/coinselector_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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);
Expand Down

0 comments on commit af56d63

Please sign in to comment.