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

Do both BnB and Knapsack coin selection in SelectCoinsMinConf

Browse source 
Instead of switching which algorithm to use based on use_bnb, just run
both in SelectCoinsMinConf. If BnB fails, do Knapsack.
This commit is contained in:
Andrew Chow 2019-10-30 16:08:55 -04:00
parent 01dc8ebda5
commit de26eb0e1f
2 changed files with 21 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -287,9 +287,9 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
empty_wallet();
add_coin(1 * CENT);
vCoins.at(0).nInputBytes = 40;
BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used));
coin_selection_params_bnb.m_subtract_fee_outputs = true;
BOOST_CHECK(testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used));
BOOST_CHECK(bnb_used);
BOOST_CHECK_EQUAL(nValueRet, 1 * CENT);
// Make sure that can use BnB when there are preset inputs
@ -549,17 +549,19 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
for (int i = 0; i < RUN_TESTS; i++) {
// picking 50 from 100 coins doesn't depend on the shuffle,
// but does depend on randomness in the stochastic approximation code
BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, filter_standard, vCoins, setCoinsRet , nValueRet, coin_selection_params, bnb_used));
BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, filter_standard, vCoins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used));
BOOST_CHECK(KnapsackSolver(50 * COIN, GroupCoins(vCoins), setCoinsRet, nValueRet));
BOOST_CHECK(KnapsackSolver(50 * COIN, GroupCoins(vCoins), setCoinsRet2, nValueRet));
BOOST_CHECK(!equal_sets(setCoinsRet, setCoinsRet2));
int fails = 0;
for (int j = 0; j < RANDOM_REPEATS; j++)
{
// selecting 1 from 100 identical coins depends on the shuffle; this test will fail 1% of the time
// run the test RANDOM_REPEATS times and only complain if all of them fail
BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, filter_standard, vCoins, setCoinsRet , nValueRet, coin_selection_params, bnb_used));
BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, filter_standard, vCoins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used));
// Test that the KnapsackSolver selects randomly from equivalent coins (same value and same input size).
// When choosing 1 from 100 identical coins, 1% of the time, this test will choose the same coin twice
// which will cause it to fail.
// To avoid that issue, run the test RANDOM_REPEATS times and only complain if all of them fail
BOOST_CHECK(KnapsackSolver(COIN, GroupCoins(vCoins), setCoinsRet, nValueRet));
BOOST_CHECK(KnapsackSolver(COIN, GroupCoins(vCoins), setCoinsRet2, nValueRet));
if (equal_sets(setCoinsRet, setCoinsRet2))
fails++;
}
@ -579,10 +581,8 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
int fails = 0;
for (int j = 0; j < RANDOM_REPEATS; j++)
{
// selecting 1 from 100 identical coins depends on the shuffle; this test will fail 1% of the time
// run the test RANDOM_REPEATS times and only complain if all of them fail
BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, filter_standard, vCoins, setCoinsRet , nValueRet, coin_selection_params, bnb_used));
BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, filter_standard, vCoins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used));
BOOST_CHECK(KnapsackSolver(90*CENT, GroupCoins(vCoins), setCoinsRet, nValueRet));
BOOST_CHECK(KnapsackSolver(90*CENT, GroupCoins(vCoins), setCoinsRet2, nValueRet));
if (equal_sets(setCoinsRet, setCoinsRet2))
fails++;
}

21
src/wallet/wallet.cpp
View file

@ -2406,19 +2406,18 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil
effective_feerate = coin_selection_params.m_effective_feerate;
}
if (coin_selection_params.use_bnb) {
std::vector<OutputGroup> positive_groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, effective_feerate, coin_selection_params.m_long_term_feerate, eligibility_filter, true /* positive_only */);
std::vector<OutputGroup> positive_groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, effective_feerate, coin_selection_params.m_long_term_feerate, eligibility_filter, true /* positive_only */);
// Note that unlike KnapsackSolver, we do not include the fee for creating a change output as BnB will not create a change output.
if (SelectCoinsBnB(positive_groups, nTargetValue, coin_selection_params.m_cost_of_change, setCoinsRet, nValueRet)) {
bnb_used = true;
// Note that unlike KnapsackSolver, we do not include the fee for creating a change output as BnB will not create a change output.
return SelectCoinsBnB(positive_groups, nTargetValue, coin_selection_params.m_cost_of_change, setCoinsRet, nValueRet);
} else {
// 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.
std::vector<OutputGroup> all_groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, effective_feerate, coin_selection_params.m_long_term_feerate, eligibility_filter, false /* positive_only */);
bnb_used = false;
// While nTargetValue includes the transaction fees for non-input things, it does not include the fee for creating a change output.
// So we need to include that for KnapsackSolver as well, as we are expecting to create a change output.
return KnapsackSolver(nTargetValue + coin_selection_params.m_change_fee, all_groups, setCoinsRet, nValueRet);
return true;
}
// 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.
std::vector<OutputGroup> all_groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, effective_feerate, coin_selection_params.m_long_term_feerate, eligibility_filter, false /* positive_only */);
bnb_used = false;
// While nTargetValue includes the transaction fees for non-input things, it does not include the fee for creating a change output.
// So we need to include that for KnapsackSolver as well, as we are expecting to create a change output.
return KnapsackSolver(nTargetValue + coin_selection_params.m_change_fee, all_groups, setCoinsRet, nValueRet);
}
bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAmount& nTargetValue, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CCoinControl& coin_control, CoinSelectionParams& coin_selection_params, bool& bnb_used) const