foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-02 09:46:52 -05:00
Code Issues Activity

Merge bitcoin/bitcoin#25495: Revert "bnb: exit selection when best_waste is 0"

Browse source 
af56d63eca Revert "bnb: exit selection when best_waste is 0" (Murch)

Pull request description:

  This reverts commit 9b5950db86.

  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.

ACKs for top commit:
  sipa:
    utACK af56d63eca
  S3RK:
    utACK af56d63eca
  achow101:
    ACK af56d63eca
  glozow:
    utACK af56d63eca, agree it is incorrect to stop here unless we could rule out the possibility of a better solution with negative waste. `SelectCoinsBnB` doesn't know what long term feerate and effective feerate are (and probably shouldn't) so it's better to have no exit early condition at all.

Tree-SHA512: 470f1a49041a0042cb69d239fccac7512ace79871d43508b6e7f7a2f3aca3523930b16e00c5513b816d5fe078d9ab53e42b0a80fd3c3d48e6434f24c2b009077
This commit is contained in:
fanquake 2022-06-29 15:55:31 +01:00
commit cc22bd7f70
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
2 changed files with 4 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
src/wallet/coinselection.cpp
View file

@ -104,9 +104,6 @@ std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_poo
if (curr_waste <= best_waste) { if (curr_waste <= best_waste) {
best_selection = curr_selection; best_selection = curr_selection;
best_waste = curr_waste; 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 curr_waste -= (curr_value - selection_target); // Remove the excess value as we will be selecting different coins now
backtrack = true; backtrack = true;

7
src/wallet/test/coinselector_tests.cpp
View file

@ -198,8 +198,8 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
expected_result.Clear(); expected_result.Clear();
// Select 5 Cent // Select 5 Cent
add_coin(4 * CENT, 4, expected_result); add_coin(3 * CENT, 3, expected_result);
add_coin(1 * CENT, 1, expected_result); add_coin(2 * CENT, 2, expected_result);
const auto result3 = SelectCoinsBnB(GroupCoins(utxo_pool), 5 * CENT, 0.5 * CENT); const auto result3 = SelectCoinsBnB(GroupCoins(utxo_pool), 5 * CENT, 0.5 * CENT);
BOOST_CHECK(result3); BOOST_CHECK(result3);
BOOST_CHECK(EquivalentResult(expected_result, *result3)); BOOST_CHECK(EquivalentResult(expected_result, *result3));
@ -224,8 +224,9 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
// Select 10 Cent // Select 10 Cent
add_coin(5 * CENT, 5, utxo_pool); add_coin(5 * CENT, 5, utxo_pool);
add_coin(5 * CENT, 5, expected_result);
add_coin(4 * CENT, 4, 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); add_coin(1 * CENT, 1, expected_result);
const auto result5 = SelectCoinsBnB(GroupCoins(utxo_pool), 10 * CENT, 0.5 * CENT); const auto result5 = SelectCoinsBnB(GroupCoins(utxo_pool), 10 * CENT, 0.5 * CENT);
BOOST_CHECK(result5); BOOST_CHECK(result5);