From 6a34ff5335786615771ca423134a484b04831c4e Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 5 Mar 2018 16:42:49 -0500 Subject: [PATCH] Have SelectCoinsMinConf and SelectCoins use BnB or Knapsack and use it Allows SelectCoinsMinConf and SelectCoins be able to switch between using BnB or Knapsack for choosing coins. Has SelectCoinsMinConf do the preprocessing necessary to support either BnB or Knapsack. This includes calculating the filtering the effective values for each input. Uses BnB in CreateTransaction to find an exact match for the output. If BnB fails, it will fallback to the Knapsack solver. --- src/bench/coin_selection.cpp | 5 +- src/wallet/test/coinselector_tests.cpp | 124 +++++++++++++++++-------- src/wallet/wallet.cpp | 118 +++++++++++++++++------ src/wallet/wallet.h | 18 +++- 4 files changed, 198 insertions(+), 67 deletions(-) diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index 55ff7b61f..605d9e3e6 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -44,8 +44,11 @@ static void CoinSelection(benchmark::State& state) std::set setCoinsRet; CAmount nValueRet; + bool bnb_used; CoinEligibilityFilter filter_standard(1, 6, 0); - bool success = wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, vCoins, setCoinsRet, nValueRet); + CoinSelectionParams coin_selection_params(false, 34, 148, CFeeRate(0), 0); + bool success = wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used) + || wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used); assert(success); assert(nValueRet == 1003 * COIN); assert(setCoinsRet.size() == 2); diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index aa1579c20..d29539830 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -28,6 +28,7 @@ typedef std::set CoinSet; static std::vector vCoins; static const CWallet testWallet("dummy", CWalletDBWrapper::CreateDummy()); +static CAmount balance = 0; static void add_coin(const CAmount& nValue, int nInput, std::vector& set) { @@ -47,6 +48,7 @@ static void add_coin(const CAmount& nValue, int nInput, CoinSet& set) static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0) { + balance += nValue; static int nextLockTime = 0; CMutableTransaction tx; tx.nLockTime = nextLockTime++; // so all transactions get different hashes @@ -72,6 +74,7 @@ static void empty_wallet(void) { vCoins.clear(); wtxn.clear(); + balance = 0; } static bool equal_sets(CoinSet a, CoinSet b) @@ -209,11 +212,13 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) CoinEligibilityFilter filter_standard(1, 6, 0); CoinEligibilityFilter filter_confirmed(1, 1, 0); CoinEligibilityFilter filter_standard_extra(6, 6, 0); +CoinSelectionParams coin_selection_params(false, 0, 0, CFeeRate(0), 0); BOOST_AUTO_TEST_CASE(knapsack_solver_test) { CoinSet setCoinsRet, setCoinsRet2; CAmount nValueRet; + bool bnb_used; LOCK(testWallet.cs_wallet); @@ -223,24 +228,24 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) empty_wallet(); // with an empty wallet we can't even pay one cent - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); add_coin(1*CENT, 4); // add a new 1 cent coin // with a new 1 cent coin, we still can't find a mature 1 cent - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // but we can find a new 1 cent - BOOST_CHECK( testWallet.SelectCoinsMinConf( 1 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 1 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); add_coin(2*CENT); // add a mature 2 cent coin // we can't make 3 cents of mature coins - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 3 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 3 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // we can make 3 cents of new coins - BOOST_CHECK( testWallet.SelectCoinsMinConf( 3 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 3 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 3 * CENT); add_coin(5*CENT); // add a mature 5 cent coin, @@ -250,33 +255,33 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // now we have new: 1+10=11 (of which 10 was self-sent), and mature: 2+5+20=27. total = 38 // we can't make 38 cents only if we disallow new coins: - BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // we can't even make 37 cents if we don't allow new coins even if they're from us - BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, filter_standard_extra, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, filter_standard_extra, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // but we can make 37 cents if we accept new coins from ourself - BOOST_CHECK( testWallet.SelectCoinsMinConf(37 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(37 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 37 * CENT); // and we can make 38 cents if we accept all new coins - BOOST_CHECK( testWallet.SelectCoinsMinConf(38 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(38 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 38 * CENT); // try making 34 cents from 1,2,5,10,20 - we can't do it exactly - BOOST_CHECK( testWallet.SelectCoinsMinConf(34 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(34 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 35 * CENT); // but 35 cents is closest BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // the best should be 20+10+5. it's incredibly unlikely the 1 or 2 got included (but possible) // when we try making 7 cents, the smaller coins (1,2,5) are enough. We should see just 2+5 - BOOST_CHECK( testWallet.SelectCoinsMinConf( 7 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 7 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 7 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // when we try making 8 cents, the smaller coins (1,2,5) are exactly enough. - BOOST_CHECK( testWallet.SelectCoinsMinConf( 8 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 8 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK(nValueRet == 8 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // when we try making 9 cents, no subset of smaller coins is enough, and we get the next bigger coin (10) - BOOST_CHECK( testWallet.SelectCoinsMinConf( 9 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 9 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 10 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -290,30 +295,30 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(30*CENT); // now we have 6+7+8+20+30 = 71 cents total // check that we have 71 and not 72 - BOOST_CHECK( testWallet.SelectCoinsMinConf(71 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK(!testWallet.SelectCoinsMinConf(72 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(71 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(72 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // now try making 16 cents. the best smaller coins can do is 6+7+8 = 21; not as good at the next biggest coin, 20 - BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 20 * CENT); // we should get 20 in one coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); add_coin( 5*CENT); // now we have 5+6+7+8+20+30 = 75 cents total // now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, better than the next biggest coin, 20 - BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 3 coins BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); add_coin( 18*CENT); // now we have 5+6+7+8+18+20+30 // and now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, the same as the next biggest coin, 18 - BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); // because in the event of a tie, the biggest coin wins // now try making 11 cents. we should get 5+6 - BOOST_CHECK( testWallet.SelectCoinsMinConf(11 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(11 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 11 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); @@ -322,11 +327,11 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin( 2*COIN); add_coin( 3*COIN); add_coin( 4*COIN); // now we have 5+6+7+8+18+20+30+100+200+300+400 = 1094 cents - BOOST_CHECK( testWallet.SelectCoinsMinConf(95 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(95 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1 * COIN); // we should get 1 BTC in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - BOOST_CHECK( testWallet.SelectCoinsMinConf(195 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(195 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 2 * COIN); // we should get 2 BTC in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -341,14 +346,14 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // try making 1 * MIN_CHANGE from the 1.5 * MIN_CHANGE // we'll get change smaller than MIN_CHANGE whatever happens, so can expect MIN_CHANGE exactly - BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); // but if we add a bigger coin, small change is avoided add_coin(1111*MIN_CHANGE); // try making 1 from 0.1 + 0.2 + 0.3 + 0.4 + 0.5 + 1111 = 1112.5 - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount // if we add more small coins: @@ -356,7 +361,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(MIN_CHANGE * 7 / 10); // and try again to make 1.0 * MIN_CHANGE - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount // run the 'mtgox' test (see http://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) @@ -365,7 +370,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) for (int j = 0; j < 20; j++) add_coin(50000 * COIN); - BOOST_CHECK( testWallet.SelectCoinsMinConf(500000 * COIN, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(500000 * COIN, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 500000 * COIN); // we should get the exact amount BOOST_CHECK_EQUAL(setCoinsRet.size(), 10U); // in ten coins @@ -378,7 +383,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(MIN_CHANGE * 6 / 10); add_coin(MIN_CHANGE * 7 / 10); add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1111 * MIN_CHANGE); // we get the bigger coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -388,7 +393,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(MIN_CHANGE * 6 / 10); add_coin(MIN_CHANGE * 8 / 10); add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); // we should get the exact amount BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // in two coins 0.4+0.6 @@ -399,12 +404,12 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(MIN_CHANGE * 100); // trying to make 100.01 from these three coins - BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 10001 / 100, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 10001 / 100, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE * 10105 / 100); // we should get all coins BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // but if we try to make 99.9, we should take the bigger of the two small coins to avoid small change - BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 9990 / 100, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 9990 / 100, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 101 * MIN_CHANGE); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); @@ -414,7 +419,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // Create 676 inputs (= (old MAX_STANDARD_TX_SIZE == 100000) / 148 bytes per input) for (uint16_t j = 0; j < 676; j++) add_coin(amt); - BOOST_CHECK(testWallet.SelectCoinsMinConf(2000, filter_confirmed, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(2000, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); if (amt - 2000 < MIN_CHANGE) { // needs more than one input: uint16_t returnSize = std::ceil((2000.0 + MIN_CHANGE)/amt); @@ -436,8 +441,8 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // 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)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, filter_standard, vCoins, setCoinsRet2, nValueRet)); + 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(!equal_sets(setCoinsRet, setCoinsRet2)); int fails = 0; @@ -445,8 +450,8 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) { // 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)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, filter_standard, vCoins, setCoinsRet2, nValueRet)); + 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)); if (equal_sets(setCoinsRet, setCoinsRet2)) fails++; } @@ -466,8 +471,8 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) { // 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)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, filter_standard, vCoins, setCoinsRet2, nValueRet)); + 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)); if (equal_sets(setCoinsRet, setCoinsRet2)) fails++; } @@ -481,6 +486,7 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) { CoinSet setCoinsRet; CAmount nValueRet; + bool bnb_used; LOCK(testWallet.cs_wallet); @@ -491,11 +497,55 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) add_coin(1000 * COIN); add_coin(3 * COIN); - BOOST_CHECK(testWallet.SelectCoinsMinConf(1003 * COIN, filter_standard, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(1003 * COIN, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1003 * COIN); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); empty_wallet(); } +// 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) +{ + // Random generator stuff + std::default_random_engine generator; + std::exponential_distribution distribution (100); + FastRandomContext rand; + + // Output stuff + CAmount out_value = 0; + CoinSet out_set; + CAmount target = 0; + bool bnb_used; + + // Run this test 100 times + for (int i = 0; i < 100; ++i) + { + // Reset + out_value = 0; + target = 0; + out_set.clear(); + empty_wallet(); + + // Make a wallet with 1000 exponentially distributed random inputs + for (int j = 0; j < 1000; ++j) + { + add_coin((CAmount)(distribution(generator)*10000000)); + } + + // Generate a random fee rate in the range of 100 - 400 + CFeeRate rate(rand.randrange(300) + 100); + + // Generate a random target value between 1000 and wallet balance + target = rand.randrange(balance - 1000) + 1000; + + // Perform selection + CoinSelectionParams coin_selection_params_knapsack(false, 34, 148, CFeeRate(0), 0); + CoinSelectionParams coin_selection_params_bnb(true, 34, 148, CFeeRate(0), 0); + BOOST_CHECK(testWallet.SelectCoinsMinConf(target, filter_standard, vCoins, out_set, out_value, coin_selection_params_bnb, bnb_used) || + testWallet.SelectCoinsMinConf(target, filter_standard, vCoins, out_set, out_value, coin_selection_params_knapsack, bnb_used)); + BOOST_CHECK_GE(out_value, target); + } +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 37e071d99..0a469d7ae 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2444,30 +2444,67 @@ bool CWallet::OutputEligibleForSpending(const COutput& output, const CoinEligibi } bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibilty_filter, std::vector vCoins, - std::set& setCoinsRet, CAmount& nValueRet) const + std::set& setCoinsRet, CAmount& nValueRet, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const { setCoinsRet.clear(); nValueRet = 0; std::vector utxo_pool; - for (const COutput &output : vCoins) - { - if (!OutputEligibleForSpending(output, eligibilty_filter)) - continue; + if (coin_selection_params.use_bnb) { - CInputCoin coin = CInputCoin(output.tx->tx, output.i); - utxo_pool.push_back(coin); + // Get long term estimate + FeeCalculation feeCalc; + CCoinControl temp; + temp.m_confirm_target = 1008; + CFeeRate long_term_feerate = GetMinimumFeeRate(temp, ::mempool, ::feeEstimator, &feeCalc); + + // Calculate cost of change + CAmount cost_of_change = GetDiscardRate(::feeEstimator).GetFee(coin_selection_params.change_spend_size) + coin_selection_params.effective_fee.GetFee(coin_selection_params.change_output_size); + + // Filter by the min conf specs and add to utxo_pool and calculate effective value + for (const COutput &output : vCoins) + { + if (!OutputEligibleForSpending(output, eligibilty_filter)) + continue; + + CInputCoin coin(output.tx->tx, output.i); + coin.effective_value = coin.txout.nValue - (output.nInputBytes < 0 ? 0 : coin_selection_params.effective_fee.GetFee(output.nInputBytes)); + // Only include outputs that are positive effective value (i.e. not dust) + if (coin.effective_value > 0) { + coin.fee = output.nInputBytes < 0 ? 0 : coin_selection_params.effective_fee.GetFee(output.nInputBytes); + coin.long_term_fee = output.nInputBytes < 0 ? 0 : long_term_feerate.GetFee(output.nInputBytes); + utxo_pool.push_back(coin); + } + } + // Calculate the fees for things that aren't inputs + CAmount not_input_fees = coin_selection_params.effective_fee.GetFee(coin_selection_params.tx_noinputs_size); + bnb_used = true; + return SelectCoinsBnB(utxo_pool, nTargetValue, cost_of_change, setCoinsRet, nValueRet, not_input_fees); + } else { + // Filter by the min conf specs and add to utxo_pool + for (const COutput &output : vCoins) + { + if (!OutputEligibleForSpending(output, eligibilty_filter)) + continue; + + CInputCoin coin = CInputCoin(output.tx->tx, output.i); + utxo_pool.push_back(coin); + } + bnb_used = false; + return KnapsackSolver(nTargetValue, utxo_pool, setCoinsRet, nValueRet); } - return KnapsackSolver(nTargetValue, utxo_pool, setCoinsRet, nValueRet); } -bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAmount& nTargetValue, std::set& setCoinsRet, CAmount& nValueRet, const CCoinControl* coinControl) const +bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAmount& nTargetValue, std::set& setCoinsRet, CAmount& nValueRet, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const { std::vector vCoins(vAvailableCoins); // coin control -> return all selected outputs (we want all selected to go into the transaction for sure) - if (coinControl && coinControl->HasSelected() && !coinControl->fAllowOtherInputs) + if (coin_control.HasSelected() && !coin_control.fAllowOtherInputs) { + // We didn't use BnB here, so set it to false. + bnb_used = false; + for (const COutput& out : vCoins) { if (!out.fSpendable) @@ -2483,10 +2520,12 @@ bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAm CAmount nValueFromPresetInputs = 0; std::vector vPresetInputs; - if (coinControl) - coinControl->ListSelected(vPresetInputs); + coin_control.ListSelected(vPresetInputs); for (const COutPoint& outpoint : vPresetInputs) { + // For now, don't use BnB if preset inputs are selected. TODO: Enable this later + bnb_used = false; + std::map::const_iterator it = mapWallet.find(outpoint.hash); if (it != mapWallet.end()) { @@ -2494,6 +2533,7 @@ bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAm // Clearly invalid input, fail if (pcoin->tx->vout.size() <= outpoint.n) return false; + // Just to calculate the marginal byte size nValueFromPresetInputs += pcoin->tx->vout[outpoint.n].nValue; setPresetCoins.insert(CInputCoin(pcoin->tx, outpoint.n)); } else @@ -2501,7 +2541,7 @@ bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAm } // remove preset inputs from vCoins - for (std::vector::iterator it = vCoins.begin(); it != vCoins.end() && coinControl && coinControl->HasSelected();) + for (std::vector::iterator it = vCoins.begin(); it != vCoins.end() && coin_control.HasSelected();) { if (setPresetCoins.count(CInputCoin(it->tx->tx, it->i))) it = vCoins.erase(it); @@ -2513,13 +2553,13 @@ bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAm bool fRejectLongChains = gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS); bool res = nTargetValue <= nValueFromPresetInputs || - SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(1, 6, 0), vCoins, setCoinsRet, nValueRet) || - SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(1, 1, 0), vCoins, setCoinsRet, nValueRet) || - (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, 2), vCoins, setCoinsRet, nValueRet)) || - (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, std::min((size_t)4, nMaxChainLength/3)), vCoins, setCoinsRet, nValueRet)) || - (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, nMaxChainLength/2), vCoins, setCoinsRet, nValueRet)) || - (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, nMaxChainLength), vCoins, setCoinsRet, nValueRet)) || - (bSpendZeroConfChange && !fRejectLongChains && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, std::numeric_limits::max()), vCoins, setCoinsRet, nValueRet)); + SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(1, 6, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used) || + SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(1, 1, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used) || + (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, 2), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || + (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, std::min((size_t)4, nMaxChainLength/3)), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || + (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, nMaxChainLength/2), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || + (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, nMaxChainLength), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || + (bSpendZeroConfChange && !fRejectLongChains && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, std::numeric_limits::max()), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // because SelectCoinsMinConf clears the setCoinsRet, we now add the possible inputs to the coinset setCoinsRet.insert(setPresetCoins.begin(), setPresetCoins.end()); @@ -2704,6 +2744,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT { std::vector vAvailableCoins; AvailableCoins(vAvailableCoins, true, &coin_control); + CoinSelectionParams coin_selection_params; // Parameters for coin selection, init with dummy // Create change script that will be used if we need change // TODO: pass in scriptChange instead of reservekey so @@ -2737,12 +2778,20 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT scriptChange = GetScriptForDestination(GetDestinationForKey(vchPubKey, change_type)); } CTxOut change_prototype_txout(0, scriptChange); - size_t change_prototype_size = GetSerializeSize(change_prototype_txout, SER_DISK, 0); + coin_selection_params.change_output_size = GetSerializeSize(change_prototype_txout, SER_DISK, 0); CFeeRate discard_rate = GetDiscardRate(::feeEstimator); + + // Get the fee rate to use effective values in coin selection + CFeeRate nFeeRateNeeded = GetMinimumFeeRate(coin_control, ::mempool, ::feeEstimator, &feeCalc); + nFeeRet = 0; bool pick_new_inputs = true; CAmount nValueIn = 0; + + // BnB selector is the only selector used when this is true. + // That should only happen on the first pass through the loop. + coin_selection_params.use_bnb = nSubtractFeeFromAmount == 0; // If we are doing subtract fee from recipient, then don't use BnB // Start with no fee and loop until there is enough fee while (true) { @@ -2755,7 +2804,9 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT CAmount nValueToSelect = nValue; if (nSubtractFeeFromAmount == 0) nValueToSelect += nFeeRet; + // vouts to the payees + coin_selection_params.tx_noinputs_size = 11; // Static vsize overhead + outputs vsize. 4 nVersion, 4 nLocktime, 1 input count, 1 output count, 1 witness overhead (dummy, flag, stack size) for (const auto& recipient : vecSend) { CTxOut txout(recipient.nAmount, recipient.scriptPubKey); @@ -2771,6 +2822,8 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT txout.nValue -= nFeeRet % nSubtractFeeFromAmount; } } + // Include the fee cost for outputs. Note this is only used for BnB right now + coin_selection_params.tx_noinputs_size += ::GetSerializeSize(txout, SER_NETWORK, PROTOCOL_VERSION); if (IsDust(txout, ::dustRelayFee)) { @@ -2789,18 +2842,27 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT } // Choose coins to use + bool bnb_used; if (pick_new_inputs) { nValueIn = 0; setCoins.clear(); - if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, &coin_control)) + coin_selection_params.change_spend_size = CalculateMaximumSignedInputSize(change_prototype_txout, this); + coin_selection_params.effective_fee = nFeeRateNeeded; + if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, coin_control, coin_selection_params, bnb_used)) { - strFailReason = _("Insufficient funds"); - return false; + // If BnB was used, it was the first pass. No longer the first pass and continue loop with knapsack. + if (bnb_used) { + coin_selection_params.use_bnb = false; + continue; + } + else { + strFailReason = _("Insufficient funds"); + return false; + } } } const CAmount nChange = nValueIn - nValueToSelect; - if (nChange > 0) { // Fill a vout to ourself @@ -2808,7 +2870,8 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT // Never create dust outputs; if we would, just // add the dust to the fee. - if (IsDust(newTxOut, discard_rate)) + // The nChange when BnB is used is always going to go to fees. + if (IsDust(newTxOut, discard_rate) || bnb_used) { nChangePosInOut = -1; nFeeRet += nChange; @@ -2881,7 +2944,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT // (because of reduced tx size) and so we should add a // change output. Only try this once. if (nChangePosInOut == -1 && nSubtractFeeFromAmount == 0 && pick_new_inputs) { - unsigned int tx_size_with_change = nBytes + change_prototype_size + 2; // Add 2 as a buffer in case increasing # of outputs changes compact size + unsigned int tx_size_with_change = nBytes + coin_selection_params.change_output_size + 2; // Add 2 as a buffer in case increasing # of outputs changes compact size CAmount fee_needed_with_change = GetMinimumFee(tx_size_with_change, coin_control, ::mempool, ::feeEstimator, nullptr); CAmount minimum_value_for_change = GetDustThreshold(change_prototype_txout, discard_rate); if (nFeeRet >= fee_needed_with_change + minimum_value_for_change) { @@ -2929,6 +2992,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT // Include more fee and try again. nFeeRet = nFeeNeeded; + coin_selection_params.use_bnb = false; continue; } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 8f518bae6..f50cde60b 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -642,6 +642,18 @@ private: std::vector _ssExtra; }; +struct CoinSelectionParams +{ + bool use_bnb = true; + size_t change_output_size = 0; + size_t change_spend_size = 0; + CFeeRate effective_fee = CFeeRate(0); + size_t tx_noinputs_size = 0; + + CoinSelectionParams(bool use_bnb, size_t change_output_size, size_t change_spend_size, CFeeRate effective_fee, size_t tx_noinputs_size) : use_bnb(use_bnb), change_output_size(change_output_size), change_spend_size(change_spend_size), effective_fee(effective_fee), tx_noinputs_size(tx_noinputs_size) {} + CoinSelectionParams() {} +}; + struct CoinEligibilityFilter { const int conf_mine; @@ -671,7 +683,8 @@ private: * all coins from coinControl are selected; Never select unconfirmed coins * if they are not ours */ - bool SelectCoins(const std::vector& vAvailableCoins, const CAmount& nTargetValue, std::set& setCoinsRet, CAmount& nValueRet, const CCoinControl *coinControl = nullptr) const; + bool SelectCoins(const std::vector& vAvailableCoins, const CAmount& nTargetValue, std::set& setCoinsRet, CAmount& nValueRet, + const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const; CWalletDB *pwalletdbEncryption; @@ -852,7 +865,8 @@ public: * completion the coin set and corresponding actual target value is * assembled */ - bool SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibilty_filter, std::vector vCoins, std::set& setCoinsRet, CAmount& nValueRet) const; + bool SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibilty_filter, std::vector vCoins, + std::set& setCoinsRet, CAmount& nValueRet, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const; bool IsSpent(const uint256& hash, unsigned int n) const;