From 706b084ddff8fc447f45ae8c968fe0715abd3712 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 13 Jun 2017 19:17:04 -0700 Subject: [PATCH 01/13] Coin selection function for Branch and Bound coin selection algo Created a function which uses the branch and bound coin selection algorithm designed by Murch. --- src/Makefile.am | 2 + src/Makefile.test.include | 3 +- src/bench/coin_selection.cpp | 3 +- src/wallet/coinselection.cpp | 253 +++++++++++++++ src/wallet/coinselection.h | 18 ++ src/wallet/test/coinselector_tests.cpp | 556 +++++++++++++++++++++++++++++++++ src/wallet/wallet.cpp | 77 +++-- src/wallet/wallet.h | 22 +- 8 files changed, 880 insertions(+), 54 deletions(-) create mode 100644 src/wallet/coinselection.cpp create mode 100644 src/wallet/coinselection.h create mode 100644 src/wallet/test/coinselector_tests.cpp diff --git a/src/Makefile.am b/src/Makefile.am index f7abab482e9..bfe49f4f9b4 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -165,6 +165,7 @@ BITCOIN_CORE_H = \ wallet/rpcwallet.h \ wallet/wallet.h \ wallet/walletdb.h \ + wallet/coinselection.h \ warnings.h \ zmq/zmqabstractnotifier.h \ zmq/zmqconfig.h\ @@ -243,6 +244,7 @@ libbitcoin_wallet_a_SOURCES = \ wallet/rpcwallet.cpp \ wallet/wallet.cpp \ wallet/walletdb.cpp \ + wallet/coinselection.cpp \ $(BITCOIN_CORE_H) # crypto primitives library diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 6415b3d2e33..2e0f8dce678 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -91,7 +91,8 @@ BITCOIN_TESTS += \ wallet/test/wallet_test_fixture.h \ wallet/test/accounting_tests.cpp \ wallet/test/wallet_tests.cpp \ - wallet/test/crypto_tests.cpp + wallet/test/crypto_tests.cpp \ + wallet/test/coinselector_tests.cpp endif test_test_bitcoin_SOURCES = $(BITCOIN_TESTS) $(JSON_TEST_FILES) $(RAW_TEST_FILES) diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index f8956508f68..ae0d2c202c7 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -49,7 +49,8 @@ static void CoinSelection(benchmark::State& state) std::set setCoinsRet; CAmount nValueRet; - bool success = wallet.SelectCoinsMinConf(1003 * COIN, 1, 6, 0, vCoins, setCoinsRet, nValueRet); + CAmount fee_ret; + bool success = wallet.SelectCoinsMinConf(1003 * COIN, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret); assert(success); assert(nValueRet == 1003 * COIN); assert(setCoinsRet.size() == 2); diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp new file mode 100644 index 00000000000..aabf8aba183 --- /dev/null +++ b/src/wallet/coinselection.cpp @@ -0,0 +1,253 @@ +// Copyright (c) 2012-2016 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "wallet/coinselection.h" + +// Descending order comparator +struct { + bool operator()(const CInputCoin& t1, + const CInputCoin& t2) const + { + return t1.txout.nValue < t2.txout.nValue; + } +}; + +bool SelectCoinsBnB(std::vector& utxo_pool, const CAmount& target_value, const CAmount& cost_of_change, std::set& out_set, CAmount& value_ret, std::vector& fee_vec, CAmount& fee_ret) +{ + out_set.clear(); + value_ret = 0; + + if (utxo_pool.size() <=0) { + return false; + } + + CAmount selected_value = 0; + int depth = 0; + int tries = 100000; + std::vector> selection; // First bool: select the utxo at this index; Second bool: traversing second branch of this utxo + selection.assign(utxo_pool.size(), std::pair(false, false)); + bool done = false; + bool backtrack = false; + + // Sort the utxo_pool + std::sort(utxo_pool.begin(), utxo_pool.end(), descending); + + // Calculate remaining + CAmount remaining = 0; + for (CInputCoin utxo : utxo_pool) { + remaining += utxo.txout.nValue; + } + + // Depth first search to find + while (!done) + { + if (tries <= 0) { // Too many tries, exit + return false; + } else if (value_ret > target_value + cost_of_change) { // Selected value is out of range, go back and try other branch + backtrack = true; + } else if (selected_value >= target_value) { // Selected value is within range + done = true; + } else if (depth >= (int)utxo_pool.size()) { // Reached a leaf node, no solution here + backtrack = true; + } else if (selected_value + remaining < target_value) { // Cannot possibly reach target with amount remaining + if (depth == 0) { // At the first utxo, no possible selections, so exit + return false; + } else { + backtrack = true; + } + } else { // Continue down this branch + // Assert that this utxo is not negative. It should never be negative, effective value calculation should have removed it + assert(utxo_pool.at(depth).txout.nValue >= 0); + + // Remove this utxo from the remaining utxo amount + remaining -= utxo_pool.at(depth).txout.nValue; + // Inclusion branch first (Largest First Exploration) + selection.at(depth).first = true; + value_ret += utxo_pool.at(depth).txout.nValue; + ++depth; + } + + // Step back to the previous utxo and try the other branch + if (backtrack) { + backtrack = false; // Reset + --depth; + + // Walk backwards to find the first utxo which has not has its second branch traversed + while (selection.at(depth).second) { + // Reset this utxo's selection + if (selection.at(depth).first) { + value_ret -= utxo_pool.at(depth).txout.nValue; + } + selection.at(depth).first = false; + selection.at(depth).second = false; + remaining += utxo_pool.at(depth).txout.nValue; + + // Step back one + --depth; + + if (depth < 0) { // We have walked back to the first utxo and no branch is untraversed. No solution, exit. + return false; + } + } + + if (!done) { + // Now traverse the second branch of the utxo we have arrived at. + selection.at(depth).second = true; + + // These were always included first, try excluding now + selection.at(depth).first = false; + value_ret -= utxo_pool.at(depth).txout.nValue; + ++depth; + } + } + --tries; + } + + // Set output set + out_set.clear(); + for (unsigned int i = 0; i < selection.size(); ++i) { + if (selection.at(i).first) { + out_set.insert(utxo_pool.at(i)); + fee_ret += fee_vec.at(i); + } + } + + return true; +} + +static void ApproximateBestSubset(const std::vector& vValue, const CAmount& nTotalLower, const CAmount& nTargetValue, + std::vector& vfBest, CAmount& nBest, int iterations = 1000) +{ + std::vector vfIncluded; + + vfBest.assign(vValue.size(), true); + nBest = nTotalLower; + + FastRandomContext insecure_rand; + + for (int nRep = 0; nRep < iterations && nBest != nTargetValue; nRep++) + { + vfIncluded.assign(vValue.size(), false); + CAmount nTotal = 0; + bool fReachedTarget = false; + for (int nPass = 0; nPass < 2 && !fReachedTarget; nPass++) + { + for (unsigned int i = 0; i < vValue.size(); i++) + { + //The solver here uses a randomized algorithm, + //the randomness serves no real security purpose but is just + //needed to prevent degenerate behavior and it is important + //that the rng is fast. We do not use a constant random sequence, + //because there may be some privacy improvement by making + //the selection random. + if (nPass == 0 ? insecure_rand.randbool() : !vfIncluded[i]) + { + nTotal += vValue[i].txout.nValue; + vfIncluded[i] = true; + if (nTotal >= nTargetValue) + { + fReachedTarget = true; + if (nTotal < nBest) + { + nBest = nTotal; + vfBest = vfIncluded; + } + nTotal -= vValue[i].txout.nValue; + vfIncluded[i] = false; + } + } + } + } + } +} + +bool KnapsackSolver(std::vector& utxo_pool, const CAmount& nTargetValue, std::set& out_set, CAmount& value_ret) +{ + out_set.clear(); + value_ret = 0; + + // List of values less than target + boost::optional coinLowestLarger; + std::vector vValue; + CAmount nTotalLower = 0; + + random_shuffle(utxo_pool.begin(), utxo_pool.end(), GetRandInt); + + for (const CInputCoin coin : utxo_pool) + { + if (coin.txout.nValue == nTargetValue) + { + out_set.insert(coin); + value_ret += coin.txout.nValue; + return true; + } + else if (coin.txout.nValue < nTargetValue + MIN_CHANGE) + { + vValue.push_back(coin); + nTotalLower += coin.txout.nValue; + } + else if (!coinLowestLarger || coin.txout.nValue < coinLowestLarger->txout.nValue) + { + coinLowestLarger = coin; + } + } + + if (nTotalLower == nTargetValue) + { + for (const auto& input : vValue) + { + out_set.insert(input); + value_ret += input.txout.nValue; + } + return true; + } + + if (nTotalLower < nTargetValue) + { + if (!coinLowestLarger) + return false; + out_set.insert(coinLowestLarger.get()); + value_ret += coinLowestLarger->txout.nValue; + return true; + } + + // Solve subset sum by stochastic approximation + std::sort(vValue.begin(), vValue.end(), CompareValueOnly()); + std::reverse(vValue.begin(), vValue.end()); + std::vector vfBest; + CAmount nBest; + + ApproximateBestSubset(vValue, nTotalLower, nTargetValue, vfBest, nBest); + if (nBest != nTargetValue && nTotalLower >= nTargetValue + MIN_CHANGE) + ApproximateBestSubset(vValue, nTotalLower, nTargetValue + MIN_CHANGE, vfBest, nBest); + + // If we have a bigger coin and (either the stochastic approximation didn't find a good solution, + // or the next bigger coin is closer), return the bigger coin + if (coinLowestLarger && + ((nBest != nTargetValue && nBest < nTargetValue + MIN_CHANGE) || coinLowestLarger->txout.nValue <= nBest)) + { + out_set.insert(coinLowestLarger.get()); + value_ret += coinLowestLarger->txout.nValue; + } + else { + for (unsigned int i = 0; i < vValue.size(); i++) + if (vfBest[i]) + { + out_set.insert(vValue[i]); + value_ret += vValue[i].txout.nValue; + } + + if (LogAcceptCategory(BCLog::SELECTCOINS)) { + LogPrint(BCLog::SELECTCOINS, "SelectCoins() best subset: "); + for (unsigned int i = 0; i < vValue.size(); i++) { + if (vfBest[i]) { + LogPrint(BCLog::SELECTCOINS, "%s ", FormatMoney(vValue[i].txout.nValue)); + } + } + LogPrint(BCLog::SELECTCOINS, "total %s\n", FormatMoney(nBest)); + } + } + + return true; +} diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h new file mode 100644 index 00000000000..abbfd81c3f8 --- /dev/null +++ b/src/wallet/coinselection.h @@ -0,0 +1,18 @@ +// Copyright (c) 2017 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_COINSELECTION_H +#define BITCOIN_COINSELECTION_H + +#include "amount.h" +#include "primitives/transaction.h" +#include "random.h" +#include "wallet.h" + +bool SelectCoinsBnB(std::vector& utxo_pool, const CAmount& target_value, const CAmount& cost_of_change, std::set& out_set, CAmount& value_ret, std::vector& fee_vec, CAmount& fee_ret); + +// Original coin selection algorithm as a fallback +bool KnapsackSolver(std::vector& utxo_pool, const CAmount& nTargetValue, std::set& out_set, CAmount& value_ret); + +#endif // BITCOIN_COINSELECTION_H diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp new file mode 100644 index 00000000000..cdfaf471cf0 --- /dev/null +++ b/src/wallet/test/coinselector_tests.cpp @@ -0,0 +1,556 @@ +// Copyright (c) 2017 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "wallet/wallet.h" +#include "wallet/coinselection.h" +#include "amount.h" +#include "primitives/transaction.h" +#include "random.h" +#include "test/test_bitcoin.h" +#include "wallet/test/wallet_test_fixture.h" + +#include + +BOOST_FIXTURE_TEST_SUITE(coin_selection_tests, WalletTestingSetup) + +// how many times to run all the tests to have a chance to catch errors that only show up with particular random shuffles +#define RUN_TESTS 100 + +// some tests fail 1% of the time due to bad luck. +// we repeat those tests this many times and only complain if all iterations of the test fail +#define RANDOM_REPEATS 5 + +std::vector> wtxn; + +typedef std::set CoinSet; + +static std::vector vCoins; +static std::vector fee_vec; +static const CWallet testWallet; +static void add_coin(const CAmount& nValue, int nInput, std::vector& set) +{ + CMutableTransaction tx; + tx.vout.resize(nInput+1); + tx.vout[nInput].nValue = nValue; + std::unique_ptr wtx(new CWalletTx(&testWallet, MakeTransactionRef(std::move(tx)))); + set.emplace_back(wtx.get(), nInput); + fee_vec.push_back(0); +} + +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 + tx.vout.resize(nInput+1); + tx.vout[nInput].nValue = nValue; + if (fIsFromMe) { + // IsFromMe() returns (GetDebit() > 0), and GetDebit() is 0 if vin.empty(), + // so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe() + tx.vin.resize(1); + } + + // Check each element + for (unsigned int i = 0; i < a.size(); ++i) { + if (a[i] != b[i]) { + return false; + } + } + COutput output(wtx.get(), nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */); + vCoins.push_back(output); + wtxn.emplace_back(std::move(wtx)); + fee_vec.push_back(0); +} + +static void empty_wallet(void) +{ + vCoins.clear(); + wtxn.clear(); + balance = 0; + fee_vec.clear(); +} + +static bool equal_sets(CoinSet a, CoinSet b) +{ + std::pair ret = mismatch(a.begin(), a.end(), b.begin()); + return ret.first == a.end() && ret.second == b.end(); +} + +static CAmount make_hard_case(int utxos, std::vector& utxo_pool) +{ + utxo_pool.clear(); + CAmount target = 0; + for (int i = 0; i < utxos; ++i) { + target += (CAmount)1 << (utxos+i); + add_coin((CAmount)1 << (utxos+i), 2*i, utxo_pool); + add_coin(((CAmount)1 << (utxos+i)) + ((CAmount)1 << (utxos-1-i)), 2*i + 1, utxo_pool); + } + return target; +} + +// Branch and bound coin selection tests +BOOST_AUTO_TEST_CASE(bnb_search_test) +{ + // Setup + std::vector utxo_pool; + CoinSet selection; + CoinSet actual_selection; + CAmount value_ret = 0; + CAmount fee_ret = 0; + + ///////////////////////// + // Known Outcome tests // + ///////////////////////// + BOOST_TEST_MESSAGE("Testing known outcomes"); + + // Empty utxo pool + BOOST_CHECK(!SelectCoinsBnB(utxo_pool, 1 * CENT, 0.5 * CENT, selection, value_ret, fee_vec, fee_ret)); + selection.clear(); + + // Add 1, 2, and 3, utxos + add_coin(1 * CENT, 1, utxo_pool); + add_coin(2 * CENT, 2, utxo_pool); + add_coin(3 * CENT, 3, utxo_pool); + add_coin(4 * CENT, 4, utxo_pool); + + // Select 1 Cent + add_coin(1 * CENT, 1, actual_selection); + BOOST_CHECK(SelectCoinsBnB(utxo_pool, 1 * CENT, 0.5 * CENT, selection, value_ret, fee_vec, fee_ret)); + BOOST_CHECK(equal_sets(selection, actual_selection)); + actual_selection.clear(); + selection.clear(); + + // Select 2 Cent + add_coin(2 * CENT, 2, actual_selection); + BOOST_CHECK(SelectCoinsBnB(utxo_pool, 2 * CENT, 0.5 * CENT, selection, value_ret, fee_vec, fee_ret)); + BOOST_CHECK(equal_sets(selection, actual_selection)); + actual_selection.clear(); + selection.clear(); + + // Select 5 Cent + add_coin(4 * CENT, 4, actual_selection); + add_coin(1 * CENT, 1, actual_selection); + BOOST_CHECK(SelectCoinsBnB(utxo_pool, 5 * CENT, 0.5 * CENT, selection, value_ret, fee_vec, fee_ret)); + BOOST_CHECK(equal_sets(selection, actual_selection)); + actual_selection.clear(); + selection.clear(); + + // Select 11 Cent, not possible + BOOST_CHECK(!SelectCoinsBnB(utxo_pool, 11 * CENT, 0.5 * CENT, selection, value_ret, fee_vec, fee_ret)); + actual_selection.clear(); + selection.clear(); + + // Select 10 Cent + add_coin(5 * CENT, 5, utxo_pool); + add_coin(5 * CENT, 5, actual_selection); + add_coin(4 * CENT, 4, actual_selection); + add_coin(1 * CENT, 1, actual_selection); + BOOST_CHECK(SelectCoinsBnB(utxo_pool, 10 * CENT, 0.5 * CENT, selection, value_ret, fee_vec, fee_ret)); + BOOST_CHECK(equal_sets(selection, actual_selection)); + actual_selection.clear(); + selection.clear(); + + // Negative effective value + // Select 10 Cent but have 1 Cent not be possible because too small + add_coin(5 * CENT, 5, actual_selection); + add_coin(3 * CENT, 3, actual_selection); + add_coin(2 * CENT, 2, actual_selection); + BOOST_CHECK(SelectCoinsBnB(utxo_pool, 10 * CENT, 5000, selection, value_ret, fee_vec, fee_ret)); + + // Select 0.25 Cent, not possible + BOOST_CHECK(!SelectCoinsBnB(utxo_pool, 0.25 * CENT, 0.5 * CENT, selection, value_ret, fee_vec, fee_ret)); + actual_selection.clear(); + selection.clear(); + + // Iteration exhaustion test + CAmount target = make_hard_case(17, utxo_pool); + BOOST_CHECK(!SelectCoinsBnB(utxo_pool, target, 0, selection, value_ret, fee_vec, fee_ret)); // Should exhaust + target = make_hard_case(14, utxo_pool); + BOOST_CHECK(SelectCoinsBnB(utxo_pool, target, 0, selection, value_ret, fee_vec, fee_ret)); // Should not exhaust + + //////////////////// + // Behavior tests // + //////////////////// + BOOST_TEST_MESSAGE("Testing behavior"); + + // Populate utxo pool with 50 inputs from 1 to 50 + utxo_pool.clear(); + for (int i = 1; i <= 50; ++i) { + add_coin(i * CENT, i, utxo_pool); + } + + // Select 100 Cent + // One possible exact solution, should appear at least once. + add_coin(50 * CENT, 50, actual_selection); + add_coin(49 * CENT, 49, actual_selection); + add_coin(3 * CENT, 3, actual_selection); + bool found_sample_sol = false; + // Run 100 times, make sure above solution appears and that solutions are valid + for (int i = 0; i < 100; ++i) { + // Reset + value_ret = 0; + selection.clear(); + + // Run selection + BOOST_CHECK(SelectCoinsBnB(utxo_pool, 100 * CENT, 2 * CENT, selection, value_ret, fee_vec, fee_ret)); + if (equal_sets(selection, actual_selection)) { + found_sample_sol = true; + } + // Check the solution is within the bounds set + CAmount selection_value = 0; + for (auto utxo : selection) { + selection_value += utxo.txout.nValue; + } + BOOST_CHECK(selection_value >= 100 * CENT); + BOOST_CHECK(selection_value <= 102 * CENT); + } + BOOST_CHECK(found_sample_sol); + + // Select 1 Cent with pool of only greater than 5 Cent + utxo_pool.clear(); + for (int i = 5; i <= 20; ++i) { + add_coin(i * CENT, i, utxo_pool); + } + // Run 100 times, to make sure it is never finding a solution + for (int i = 0; i < 100; ++i) { + BOOST_CHECK(!SelectCoinsBnB(utxo_pool, 1 * CENT, 2 * CENT, selection, value_ret, fee_vec, fee_ret)); + } +} + + +// Original coin selection algorithm tests +BOOST_AUTO_TEST_CASE(knapsack_tests) +{ + CoinSet setCoinsRet, setCoinsRet2; + CAmount nValueRet; + CAmount fee_ret = 0; + + LOCK(testWallet.cs_wallet); + + // test multiple times to allow for differences in the shuffle order + for (int i = 0; i < RUN_TESTS; i++) + { + empty_wallet(); + + // with an empty wallet we can't even pay one cent + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + + 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, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + + // but we can find a new 1 cent + BOOST_CHECK( testWallet.SelectCoinsMinConf( 1 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + 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, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + + // we can make 3 cents of new coins + BOOST_CHECK( testWallet.SelectCoinsMinConf( 3 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK_EQUAL(nValueRet, 3 * CENT); + + add_coin(5*CENT); // add a mature 5 cent coin, + add_coin(10*CENT, 3, true); // a new 10 cent coin sent from one of our own addresses + add_coin(20*CENT); // and a mature 20 cent coin + + // 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, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + // 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, 6, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + // but we can make 37 cents if we accept new coins from ourself + BOOST_CHECK( testWallet.SelectCoinsMinConf(37 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK_EQUAL(nValueRet, 37 * CENT); + // and we can make 38 cents if we accept all new coins + BOOST_CHECK( testWallet.SelectCoinsMinConf(38 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK_EQUAL(nValueRet, 10 * CENT); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + + // now clear out the wallet and start again to test choosing between subsets of smaller coins and the next biggest coin + empty_wallet(); + + add_coin( 6*CENT); + add_coin( 7*CENT); + add_coin( 8*CENT); + add_coin(20*CENT); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(72 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + + // 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK_EQUAL(nValueRet, 11 * CENT); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); + + // check that the smallest bigger coin is used + add_coin( 1*COIN); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK_EQUAL(nValueRet, 2 * COIN); // we should get 2 BTC in 1 coin + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + + // empty the wallet and start again, now with fractions of a cent, to test small change avoidance + + empty_wallet(); + add_coin(MIN_CHANGE * 1 / 10); + add_coin(MIN_CHANGE * 2 / 10); + add_coin(MIN_CHANGE * 3 / 10); + add_coin(MIN_CHANGE * 4 / 10); + add_coin(MIN_CHANGE * 5 / 10); + + // 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount + + // if we add more small coins: + add_coin(MIN_CHANGE * 6 / 10); + add_coin(MIN_CHANGE * 7 / 10); + + // and try again to make 1.0 * MIN_CHANGE + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount + + // run the 'mtgox' test (see http://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) + // they tried to consolidate 10 50k coins into one 500k coin, and ended up with 50k in change + empty_wallet(); + for (int j = 0; j < 20; j++) + add_coin(50000 * COIN); + + BOOST_CHECK( testWallet.SelectCoinsMinConf(500000 * COIN, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK_EQUAL(nValueRet, 500000 * COIN); // we should get the exact amount + BOOST_CHECK_EQUAL(setCoinsRet.size(), 10U); // in ten coins + + // if there's not enough in the smaller coins to make at least 1 * MIN_CHANGE change (0.5+0.6+0.7 < 1.0+1.0), + // we need to try finding an exact subset anyway + + // sometimes it will fail, and so we use the next biggest coin: + empty_wallet(); + add_coin(MIN_CHANGE * 5 / 10); + add_coin(MIN_CHANGE * 6 / 10); + add_coin(MIN_CHANGE * 7 / 10); + add_coin(1111 * MIN_CHANGE); + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK_EQUAL(nValueRet, 1111 * MIN_CHANGE); // we get the bigger coin + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + + // but sometimes it's possible, and we use an exact subset (0.4 + 0.6 = 1.0) + empty_wallet(); + add_coin(MIN_CHANGE * 4 / 10); + add_coin(MIN_CHANGE * 6 / 10); + add_coin(MIN_CHANGE * 8 / 10); + add_coin(1111 * MIN_CHANGE); + BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + 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 + + // test avoiding small change + empty_wallet(); + add_coin(MIN_CHANGE * 5 / 100); + add_coin(MIN_CHANGE * 1); + add_coin(MIN_CHANGE * 100); + + // trying to make 100.01 from these three coins + BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 10001 / 100, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK_EQUAL(nValueRet, 101 * MIN_CHANGE); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); + + // test with many inputs + for (CAmount amt=1500; amt < COIN; amt*=10) { + empty_wallet(); + // 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + if (amt - 2000 < MIN_CHANGE) { + // needs more than one input: + uint16_t returnSize = std::ceil((2000.0 + MIN_CHANGE)/amt); + CAmount returnValue = amt * returnSize; + BOOST_CHECK_EQUAL(nValueRet, returnValue); + BOOST_CHECK_EQUAL(setCoinsRet.size(), returnSize); + } else { + // one input is sufficient: + BOOST_CHECK_EQUAL(nValueRet, amt); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + } + } + + // test randomness + { + empty_wallet(); + for (int i2 = 0; i2 < 100; i2++) + add_coin(COIN); + + // 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, 1, 6, 0, vCoins, setCoinsRet , nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, 1, 6, 0, vCoins, setCoinsRet2, nValueRet, fee_ret, CFeeRate(0), true)); + 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, 1, 6, 0, vCoins, setCoinsRet , nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, 1, 6, 0, vCoins, setCoinsRet2, nValueRet, fee_ret, CFeeRate(0), true)); + if (equal_sets(setCoinsRet, setCoinsRet2)) + fails++; + } + BOOST_CHECK_NE(fails, RANDOM_REPEATS); + + // add 75 cents in small change. not enough to make 90 cents, + // then try making 90 cents. there are multiple competing "smallest bigger" coins, + // one of which should be picked at random + add_coin(5 * CENT); + add_coin(10 * CENT); + add_coin(15 * CENT); + add_coin(20 * CENT); + add_coin(25 * CENT); + + 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, 1, 6, 0, vCoins, setCoinsRet , nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, 1, 6, 0, vCoins, setCoinsRet2, nValueRet, fee_ret, CFeeRate(0), true)); + if (equal_sets(setCoinsRet, setCoinsRet2)) + fails++; + } + BOOST_CHECK_NE(fails, RANDOM_REPEATS); + } + } + empty_wallet(); +} + +BOOST_AUTO_TEST_CASE(ApproximateBestSubset) +{ + CoinSet setCoinsRet; + CAmount nValueRet; + CAmount fee_ret; + + LOCK(testWallet.cs_wallet); + + empty_wallet(); + + // Test vValue sort order + for (int i = 0; i < 1000; i++) + add_coin(1000 * COIN); + add_coin(3 * COIN); + + BOOST_CHECK(testWallet.SelectCoinsMinConf(1003 * COIN, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + 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; + CAmount fee_ret = 0; + + // 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 + BOOST_CHECK(testWallet.SelectCoinsMinConf(target, 1, 6, 0, vCoins, out_set, out_value, fee_ret, rate, false) || + testWallet.SelectCoinsMinConf(target, 1, 6, 0, vCoins, out_set, out_value, fee_ret, rate, true)); + BOOST_CHECK_GE(out_value, target); + } +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index f8c6f9e87be..b8dadf749a0 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2348,7 +2348,7 @@ static void ApproximateBestSubset(const std::vector& vValue, const C } bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMine, const int nConfTheirs, const uint64_t nMaxAncestors, std::vector vCoins, - std::set& setCoinsRet, CAmount& nValueRet) const + std::set& setCoinsRet, CAmount& nValueRet, CAmount& fee_ret, const CFeeRate effective_fee, bool only_knapsack) const { setCoinsRet.clear(); nValueRet = 0; @@ -2356,14 +2356,18 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMin // List of values less than target boost::optional coinLowestLarger; std::vector vValue; - CAmount nTotalLower = 0; - - random_shuffle(vCoins.begin(), vCoins.end(), GetRandInt); - - for (const COutput &output : vCoins) - { - if (!output.fSpendable) - continue; + if (!only_knapsack) { + // Calculate cost of change + // TODO: In the future, we should use the change output actually made for the transaction and calculate the cost + // requred to spend it. + CAmount cost_of_change = effective_fee.GetFee(148+34); // 148 bytes for the input, 34 bytes for making the output + + // Filter by the min conf specs and add to vValue and calculate effective value + std::vector fee_vec; // To keep track of the fees for each input + for (const COutput &output : vCoins) + { + if (!output.fSpendable) + continue; const CWalletTx *pcoin = output.tx; @@ -2373,30 +2377,19 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMin if (!mempool.TransactionWithinChainLimit(pcoin->GetHash(), nMaxAncestors)) continue; - int i = output.i; - - CInputCoin coin = CInputCoin(pcoin, i); - - if (coin.txout.nValue == nTargetValue) - { - setCoinsRet.insert(coin); - nValueRet += coin.txout.nValue; - return true; - } - else if (coin.txout.nValue < nTargetValue + MIN_CHANGE) - { - vValue.push_back(coin); - nTotalLower += coin.txout.nValue; - } - else if (!coinLowestLarger || coin.txout.nValue < coinLowestLarger->txout.nValue) - { - coinLowestLarger = coin; + int i = output.i; + CInputCoin coin(pcoin, i); + coin.txout.nValue -= (output.nInputBytes < 0 ? 0 : effective_fee.GetFee(output.nInputBytes)); + // Only include outputs that are not negative effective value (i.e. not dust) + if (coin.txout.nValue > 0) { + vValue.push_back(coin); + fee_vec.push_back(output.nInputBytes < 0 ? 0 : effective_fee.GetFee(output.nInputBytes)); + } } - } - - if (nTotalLower == nTargetValue) - { - for (const auto& input : vValue) + return SelectCoinsBnB(vValue, nTargetValue, cost_of_change, setCoinsRet, nValueRet, fee_vec, fee_ret); + } else { + // Filter by the min conf specs and add to vValue + for (const COutput &output : vCoins) { setCoinsRet.insert(input); nValueRet += input.txout.nValue; @@ -2453,7 +2446,7 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMin return true; } -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, CAmount& fee_ret, const CFeeRate effective_fee, const CCoinControl* coinControl, bool knapsack_only) const { std::vector vCoins(vAvailableCoins); @@ -2486,6 +2479,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, outpoint.n)); } else @@ -2505,13 +2499,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, 1, 6, 0, vCoins, setCoinsRet, nValueRet) || - SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 1, 1, 0, vCoins, setCoinsRet, nValueRet) || - (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, 2, vCoins, setCoinsRet, nValueRet)) || - (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, std::min((size_t)4, nMaxChainLength/3), vCoins, setCoinsRet, nValueRet)) || - (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, nMaxChainLength/2, vCoins, setCoinsRet, nValueRet)) || - (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, nMaxChainLength, vCoins, setCoinsRet, nValueRet)) || - (bSpendZeroConfChange && !fRejectLongChains && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, std::numeric_limits::max(), vCoins, setCoinsRet, nValueRet)); + SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, knapsack_only) || + SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, knapsack_only) || + (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, 2, vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, knapsack_only)) || + (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, std::min((size_t)4, nMaxChainLength/3), vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, knapsack_only)) || + (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, nMaxChainLength/2, vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, knapsack_only)) || + (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, nMaxChainLength, vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, knapsack_only)) || + (bSpendZeroConfChange && !fRejectLongChains && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, std::numeric_limits::max(), vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, knapsack_only)); // because SelectCoinsMinConf clears the setCoinsRet, we now add the possible inputs to the coinset setCoinsRet.insert(setPresetCoins.begin(), setPresetCoins.end()); @@ -2761,7 +2755,8 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT if (pick_new_inputs) { nValueIn = 0; setCoins.clear(); - if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, &coin_control)) + setCoins.clear(); + if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, nFeeRet, nFeeRateNeeded, coinControl, !first_pass)) { strFailReason = _("Insufficient funds"); return false; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index f97a99d82a1..f725b598855 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -262,7 +262,7 @@ class CMerkleTx bool IsCoinBase() const { return tx->IsCoinBase(); } }; -/** +/** * A transaction with a bunch of additional info that only the owner cares about. * It includes any unrecorded transactions needed to link it back to the block chain. */ @@ -647,7 +647,7 @@ class CAccountingEntry }; -/** +/** * A CWallet is an extension of a keystore, which also maintains a set of transactions and balances, * and provides the ability to create new transactions. */ @@ -663,7 +663,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface * 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, CAmount& fee_ret, const CFeeRate effective_fee = CFeeRate(0), const CCoinControl *coinControl = NULL, bool knapsack_only = false) const; CWalletDB *pwalletdbEncryption; @@ -837,7 +837,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface * completion the coin set and corresponding actual target value is * assembled */ - bool SelectCoinsMinConf(const CAmount& nTargetValue, int nConfMine, int nConfTheirs, uint64_t nMaxAncestors, std::vector vCoins, std::set& setCoinsRet, CAmount& nValueRet) const; + bool SelectCoinsMinConf(const CAmount& nTargetValue, int nConfMine, int nConfTheirs, uint64_t nMaxAncestors, std::vector vCoins, std::set& setCoinsRet, CAmount& nValueRet, CAmount& fee_ret, const CFeeRate effective_fee = CFeeRate(0), bool only_knapsack = false) const; bool IsSpent(const uint256& hash, unsigned int n) const; @@ -904,7 +904,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface void GetKeyBirthTimes(std::map &mapKeyBirth) const; unsigned int ComputeTimeSmart(const CWalletTx& wtx) const; - /** + /** * Increment the next transaction order id * @return next transaction order id */ @@ -1033,7 +1033,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface } void GetScriptForMining(std::shared_ptr &script); - + unsigned int GetKeyPoolSize() { AssertLockHeld(cs_wallet); // set{Ex,In}ternalKeyPool @@ -1064,8 +1064,8 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface // This function will perform salvage on the wallet if requested, as long as only one wallet is // being loaded (CWallet::ParameterInteraction forbids -salvagewallet, -zapwallettxes or -upgradewallet with multiwallet). static bool Verify(); - - /** + + /** * Address book entry changed. * @note called with lock cs_wallet held. */ @@ -1074,7 +1074,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface const std::string &purpose, ChangeType status)> NotifyAddressBookChanged; - /** + /** * Wallet transaction added, removed or updated. * @note called with lock cs_wallet held. */ @@ -1128,7 +1128,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface /* Generates a new HD master key (will not be activated) */ CPubKey GenerateNewHDMasterKey(); - + /* Set the current HD master key (will reset the chain child index counters) Sets the master key's version based on the current wallet version (so the caller must ensure the current wallet version is correct before calling @@ -1168,7 +1168,7 @@ class CReserveKey : public CReserveScript }; -/** +/** * Account information. * Stored in wallet with key "acc"+string account name. */ From d66d0e52301bd61fcb6feeea8ef25e7b8553e62f Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 15 Jun 2017 17:33:27 -0700 Subject: [PATCH 02/13] Move original coin selector and tests Moved the original coin selection algorithm to coinselection.cpp and its tests to coin_selection_test.cpp --- src/wallet/coinselection.cpp | 8 +- src/wallet/coinselection.h | 2 +- src/wallet/test/coinselector_tests.cpp | 308 ++++++++++++++++++++++++++++-- src/wallet/test/wallet_tests.cpp | 337 --------------------------------- 4 files changed, 299 insertions(+), 356 deletions(-) diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index aabf8aba183..d2065af121c 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -3,6 +3,8 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include "wallet/coinselection.h" +#include "util.h" +#include "utilmoneystr.h" // Descending order comparator struct { @@ -22,7 +24,6 @@ bool SelectCoinsBnB(std::vector& utxo_pool, const CAmount& target_va return false; } - CAmount selected_value = 0; int depth = 0; int tries = 100000; std::vector> selection; // First bool: select the utxo at this index; Second bool: traversing second branch of this utxo @@ -46,11 +47,11 @@ bool SelectCoinsBnB(std::vector& utxo_pool, const CAmount& target_va return false; } else if (value_ret > target_value + cost_of_change) { // Selected value is out of range, go back and try other branch backtrack = true; - } else if (selected_value >= target_value) { // Selected value is within range + } else if (value_ret >= target_value) { // Selected value is within range done = true; } else if (depth >= (int)utxo_pool.size()) { // Reached a leaf node, no solution here backtrack = true; - } else if (selected_value + remaining < target_value) { // Cannot possibly reach target with amount remaining + } else if (value_ret + remaining < target_value) { // Cannot possibly reach target with amount remaining if (depth == 0) { // At the first utxo, no possible selections, so exit return false; } else { @@ -105,7 +106,6 @@ bool SelectCoinsBnB(std::vector& utxo_pool, const CAmount& target_va } // Set output set - out_set.clear(); for (unsigned int i = 0; i < selection.size(); ++i) { if (selection.at(i).first) { out_set.insert(utxo_pool.at(i)); diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index abbfd81c3f8..1f7c8741bae 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -8,7 +8,7 @@ #include "amount.h" #include "primitives/transaction.h" #include "random.h" -#include "wallet.h" +#include "wallet/wallet.h" bool SelectCoinsBnB(std::vector& utxo_pool, const CAmount& target_value, const CAmount& cost_of_change, std::set& out_set, CAmount& value_ret, std::vector& fee_vec, CAmount& fee_ret); diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index cdfaf471cf0..d5a7870938a 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 std::vector fee_vec; static const CWallet testWallet; + static void add_coin(const CAmount& nValue, int nInput, std::vector& set) { CMutableTransaction tx; @@ -40,7 +41,6 @@ static void add_coin(const CAmount& nValue, int nInput, std::vector& 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 @@ -51,13 +51,6 @@ static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = fa // so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe() tx.vin.resize(1); } - - // Check each element - for (unsigned int i = 0; i < a.size(); ++i) { - if (a[i] != b[i]) { - return false; - } - } COutput output(wtx.get(), nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */); vCoins.push_back(output); wtxn.emplace_back(std::move(wtx)); @@ -198,13 +191,10 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) if (equal_sets(selection, actual_selection)) { found_sample_sol = true; } + // Check the solution is within the bounds set - CAmount selection_value = 0; - for (auto utxo : selection) { - selection_value += utxo.txout.nValue; - } - BOOST_CHECK(selection_value >= 100 * CENT); - BOOST_CHECK(selection_value <= 102 * CENT); + BOOST_CHECK(value_ret >= 100 * CENT); + BOOST_CHECK(value_ret <= 102 * CENT); } BOOST_CHECK(found_sample_sol); @@ -553,4 +543,294 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) } } + +// Original coin selection algorithm tests +BOOST_AUTO_TEST_CASE(knapsack_tests) +{ + CoinSet setCoinsRet, setCoinsRet2; + CAmount nValueRet; + + LOCK(testWallet.cs_wallet); + + // test multiple times to allow for differences in the shuffle order + for (int i = 0; i < RUN_TESTS; i++) + { + empty_wallet(); + + // with an empty wallet we can't even pay one cent + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet, true)); + + 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, 1, 6, 0, vCoins, setCoinsRet, nValueRet, true)); + + // but we can find a new 1 cent + BOOST_CHECK( testWallet.SelectCoinsMinConf( 1 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + 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, 1, 6, 0, vCoins, setCoinsRet, nValueRet, true)); + + // we can make 3 cents of new coins + BOOST_CHECK( testWallet.SelectCoinsMinConf( 3 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK_EQUAL(nValueRet, 3 * CENT); + + add_coin(5*CENT); // add a mature 5 cent coin, + add_coin(10*CENT, 3, true); // a new 10 cent coin sent from one of our own addresses + add_coin(20*CENT); // and a mature 20 cent coin + + // 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, 1, 6, 0, vCoins, setCoinsRet, nValueRet, true)); + // 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, 6, 6, 0, vCoins, setCoinsRet, nValueRet, true)); + // but we can make 37 cents if we accept new coins from ourself + BOOST_CHECK( testWallet.SelectCoinsMinConf(37 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK_EQUAL(nValueRet, 37 * CENT); + // and we can make 38 cents if we accept all new coins + BOOST_CHECK( testWallet.SelectCoinsMinConf(38 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK_EQUAL(nValueRet, 10 * CENT); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + + // now clear out the wallet and start again to test choosing between subsets of smaller coins and the next biggest coin + empty_wallet(); + + add_coin( 6*CENT); + add_coin( 7*CENT); + add_coin( 8*CENT); + add_coin(20*CENT); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(72 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + + // 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK_EQUAL(nValueRet, 11 * CENT); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); + + // check that the smallest bigger coin is used + add_coin( 1*COIN); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK_EQUAL(nValueRet, 2 * COIN); // we should get 2 BTC in 1 coin + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + + // empty the wallet and start again, now with fractions of a cent, to test small change avoidance + + empty_wallet(); + add_coin(MIN_CHANGE * 1 / 10); + add_coin(MIN_CHANGE * 2 / 10); + add_coin(MIN_CHANGE * 3 / 10); + add_coin(MIN_CHANGE * 4 / 10); + add_coin(MIN_CHANGE * 5 / 10); + + // 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount + + // if we add more small coins: + add_coin(MIN_CHANGE * 6 / 10); + add_coin(MIN_CHANGE * 7 / 10); + + // and try again to make 1.0 * MIN_CHANGE + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount + + // run the 'mtgox' test (see http://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) + // they tried to consolidate 10 50k coins into one 500k coin, and ended up with 50k in change + empty_wallet(); + for (int j = 0; j < 20; j++) + add_coin(50000 * COIN); + + BOOST_CHECK( testWallet.SelectCoinsMinConf(500000 * COIN, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK_EQUAL(nValueRet, 500000 * COIN); // we should get the exact amount + BOOST_CHECK_EQUAL(setCoinsRet.size(), 10U); // in ten coins + + // if there's not enough in the smaller coins to make at least 1 * MIN_CHANGE change (0.5+0.6+0.7 < 1.0+1.0), + // we need to try finding an exact subset anyway + + // sometimes it will fail, and so we use the next biggest coin: + empty_wallet(); + add_coin(MIN_CHANGE * 5 / 10); + add_coin(MIN_CHANGE * 6 / 10); + add_coin(MIN_CHANGE * 7 / 10); + add_coin(1111 * MIN_CHANGE); + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK_EQUAL(nValueRet, 1111 * MIN_CHANGE); // we get the bigger coin + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + + // but sometimes it's possible, and we use an exact subset (0.4 + 0.6 = 1.0) + empty_wallet(); + add_coin(MIN_CHANGE * 4 / 10); + add_coin(MIN_CHANGE * 6 / 10); + add_coin(MIN_CHANGE * 8 / 10); + add_coin(1111 * MIN_CHANGE); + BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + 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 + + // test avoiding small change + empty_wallet(); + add_coin(MIN_CHANGE * 5 / 100); + add_coin(MIN_CHANGE * 1); + add_coin(MIN_CHANGE * 100); + + // trying to make 100.01 from these three coins + BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 10001 / 100, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK_EQUAL(nValueRet, 101 * MIN_CHANGE); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); + + // test with many inputs + for (CAmount amt=1500; amt < COIN; amt*=10) { + empty_wallet(); + // 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + if (amt - 2000 < MIN_CHANGE) { + // needs more than one input: + uint16_t returnSize = std::ceil((2000.0 + MIN_CHANGE)/amt); + CAmount returnValue = amt * returnSize; + BOOST_CHECK_EQUAL(nValueRet, returnValue); + BOOST_CHECK_EQUAL(setCoinsRet.size(), returnSize); + } else { + // one input is sufficient: + BOOST_CHECK_EQUAL(nValueRet, amt); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + } + } + + // test randomness + { + empty_wallet(); + for (int i2 = 0; i2 < 100; i2++) + add_coin(COIN); + + // 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, 1, 6, 0, vCoins, setCoinsRet , nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, 1, 6, 0, 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, 1, 6, 0, vCoins, setCoinsRet , nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, 1, 6, 0, vCoins, setCoinsRet2, nValueRet)); + if (equal_sets(setCoinsRet, setCoinsRet2)) + fails++; + } + BOOST_CHECK_NE(fails, RANDOM_REPEATS); + + // add 75 cents in small change. not enough to make 90 cents, + // then try making 90 cents. there are multiple competing "smallest bigger" coins, + // one of which should be picked at random + add_coin(5 * CENT); + add_coin(10 * CENT); + add_coin(15 * CENT); + add_coin(20 * CENT); + add_coin(25 * CENT); + + 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, 1, 6, 0, vCoins, setCoinsRet , nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, 1, 6, 0, vCoins, setCoinsRet2, nValueRet)); + if (equal_sets(setCoinsRet, setCoinsRet2)) + fails++; + } + BOOST_CHECK_NE(fails, RANDOM_REPEATS); + } + } + empty_wallet(); +} + +BOOST_AUTO_TEST_CASE(ApproximateBestSubset) +{ + CoinSet setCoinsRet; + CAmount nValueRet; + + LOCK(testWallet.cs_wallet); + + empty_wallet(); + + // Test vValue sort order + for (int i = 0; i < 1000; i++) + add_coin(1000 * COIN); + add_coin(3 * COIN); + + BOOST_CHECK(testWallet.SelectCoinsMinConf(1003 * COIN, 1, 6, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK_EQUAL(nValueRet, 1003 * COIN); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); + + empty_wallet(); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 4a2cc9a1399..de058b6a233 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -25,345 +25,8 @@ extern UniValue importmulti(const JSONRPCRequest& request); extern UniValue dumpwallet(const JSONRPCRequest& request); extern UniValue importwallet(const JSONRPCRequest& request); -// how many times to run all the tests to have a chance to catch errors that only show up with particular random shuffles -#define RUN_TESTS 100 - -// some tests fail 1% of the time due to bad luck. -// we repeat those tests this many times and only complain if all iterations of the test fail -#define RANDOM_REPEATS 5 - -std::vector> wtxn; - -typedef std::set CoinSet; - BOOST_FIXTURE_TEST_SUITE(wallet_tests, WalletTestingSetup) -static const CWallet testWallet; -static std::vector vCoins; - -static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0) -{ - static int nextLockTime = 0; - CMutableTransaction tx; - tx.nLockTime = nextLockTime++; // so all transactions get different hashes - tx.vout.resize(nInput+1); - tx.vout[nInput].nValue = nValue; - if (fIsFromMe) { - // IsFromMe() returns (GetDebit() > 0), and GetDebit() is 0 if vin.empty(), - // so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe() - tx.vin.resize(1); - } - std::unique_ptr wtx(new CWalletTx(&testWallet, MakeTransactionRef(std::move(tx)))); - if (fIsFromMe) - { - wtx->fDebitCached = true; - wtx->nDebitCached = 1; - } - COutput output(wtx.get(), nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */); - vCoins.push_back(output); - wtxn.emplace_back(std::move(wtx)); -} - -static void empty_wallet(void) -{ - vCoins.clear(); - wtxn.clear(); -} - -static bool equal_sets(CoinSet a, CoinSet b) -{ - std::pair ret = mismatch(a.begin(), a.end(), b.begin()); - return ret.first == a.end() && ret.second == b.end(); -} - -BOOST_AUTO_TEST_CASE(coin_selection_tests) -{ - CoinSet setCoinsRet, setCoinsRet2; - CAmount nValueRet; - - LOCK(testWallet.cs_wallet); - - // test multiple times to allow for differences in the shuffle order - for (int i = 0; i < RUN_TESTS; i++) - { - empty_wallet(); - - // with an empty wallet we can't even pay one cent - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet)); - - 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, 1, 6, 0, vCoins, setCoinsRet, nValueRet)); - - // but we can find a new 1 cent - BOOST_CHECK( testWallet.SelectCoinsMinConf( 1 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - 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, 1, 6, 0, vCoins, setCoinsRet, nValueRet)); - - // we can make 3 cents of new coins - BOOST_CHECK( testWallet.SelectCoinsMinConf( 3 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 3 * CENT); - - add_coin(5*CENT); // add a mature 5 cent coin, - add_coin(10*CENT, 3, true); // a new 10 cent coin sent from one of our own addresses - add_coin(20*CENT); // and a mature 20 cent coin - - // 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, 1, 6, 0, vCoins, setCoinsRet, nValueRet)); - // 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, 6, 6, 0, vCoins, setCoinsRet, nValueRet)); - // but we can make 37 cents if we accept new coins from ourself - BOOST_CHECK( testWallet.SelectCoinsMinConf(37 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 37 * CENT); - // and we can make 38 cents if we accept all new coins - BOOST_CHECK( testWallet.SelectCoinsMinConf(38 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 10 * CENT); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - - // now clear out the wallet and start again to test choosing between subsets of smaller coins and the next biggest coin - empty_wallet(); - - add_coin( 6*CENT); - add_coin( 7*CENT); - add_coin( 8*CENT); - add_coin(20*CENT); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK(!testWallet.SelectCoinsMinConf(72 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - - // 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 11 * CENT); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); - - // check that the smallest bigger coin is used - add_coin( 1*COIN); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 2 * COIN); // we should get 2 BTC in 1 coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - - // empty the wallet and start again, now with fractions of a cent, to test small change avoidance - - empty_wallet(); - add_coin(MIN_CHANGE * 1 / 10); - add_coin(MIN_CHANGE * 2 / 10); - add_coin(MIN_CHANGE * 3 / 10); - add_coin(MIN_CHANGE * 4 / 10); - add_coin(MIN_CHANGE * 5 / 10); - - // 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount - - // if we add more small coins: - add_coin(MIN_CHANGE * 6 / 10); - add_coin(MIN_CHANGE * 7 / 10); - - // and try again to make 1.0 * MIN_CHANGE - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount - - // run the 'mtgox' test (see http://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) - // they tried to consolidate 10 50k coins into one 500k coin, and ended up with 50k in change - empty_wallet(); - for (int j = 0; j < 20; j++) - add_coin(50000 * COIN); - - BOOST_CHECK( testWallet.SelectCoinsMinConf(500000 * COIN, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 500000 * COIN); // we should get the exact amount - BOOST_CHECK_EQUAL(setCoinsRet.size(), 10U); // in ten coins - - // if there's not enough in the smaller coins to make at least 1 * MIN_CHANGE change (0.5+0.6+0.7 < 1.0+1.0), - // we need to try finding an exact subset anyway - - // sometimes it will fail, and so we use the next biggest coin: - empty_wallet(); - add_coin(MIN_CHANGE * 5 / 10); - add_coin(MIN_CHANGE * 6 / 10); - add_coin(MIN_CHANGE * 7 / 10); - add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1111 * MIN_CHANGE); // we get the bigger coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - - // but sometimes it's possible, and we use an exact subset (0.4 + 0.6 = 1.0) - empty_wallet(); - add_coin(MIN_CHANGE * 4 / 10); - add_coin(MIN_CHANGE * 6 / 10); - add_coin(MIN_CHANGE * 8 / 10); - add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - 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 - - // test avoiding small change - empty_wallet(); - add_coin(MIN_CHANGE * 5 / 100); - add_coin(MIN_CHANGE * 1); - add_coin(MIN_CHANGE * 100); - - // trying to make 100.01 from these three coins - BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 10001 / 100, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 101 * MIN_CHANGE); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); - - // test with many inputs - for (CAmount amt=1500; amt < COIN; amt*=10) { - empty_wallet(); - // 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - if (amt - 2000 < MIN_CHANGE) { - // needs more than one input: - uint16_t returnSize = std::ceil((2000.0 + MIN_CHANGE)/amt); - CAmount returnValue = amt * returnSize; - BOOST_CHECK_EQUAL(nValueRet, returnValue); - BOOST_CHECK_EQUAL(setCoinsRet.size(), returnSize); - } else { - // one input is sufficient: - BOOST_CHECK_EQUAL(nValueRet, amt); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - } - } - - // test randomness - { - empty_wallet(); - for (int i2 = 0; i2 < 100; i2++) - add_coin(COIN); - - // 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, 1, 6, 0, vCoins, setCoinsRet , nValueRet)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, 1, 6, 0, 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, 1, 6, 0, vCoins, setCoinsRet , nValueRet)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, 1, 6, 0, vCoins, setCoinsRet2, nValueRet)); - if (equal_sets(setCoinsRet, setCoinsRet2)) - fails++; - } - BOOST_CHECK_NE(fails, RANDOM_REPEATS); - - // add 75 cents in small change. not enough to make 90 cents, - // then try making 90 cents. there are multiple competing "smallest bigger" coins, - // one of which should be picked at random - add_coin(5 * CENT); - add_coin(10 * CENT); - add_coin(15 * CENT); - add_coin(20 * CENT); - add_coin(25 * CENT); - - 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, 1, 6, 0, vCoins, setCoinsRet , nValueRet)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, 1, 6, 0, vCoins, setCoinsRet2, nValueRet)); - if (equal_sets(setCoinsRet, setCoinsRet2)) - fails++; - } - BOOST_CHECK_NE(fails, RANDOM_REPEATS); - } - } - empty_wallet(); -} - -BOOST_AUTO_TEST_CASE(ApproximateBestSubset) -{ - CoinSet setCoinsRet; - CAmount nValueRet; - - LOCK(testWallet.cs_wallet); - - empty_wallet(); - - // Test vValue sort order - for (int i = 0; i < 1000; i++) - add_coin(1000 * COIN); - add_coin(3 * COIN); - - BOOST_CHECK(testWallet.SelectCoinsMinConf(1003 * COIN, 1, 6, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1003 * COIN); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); - - empty_wallet(); -} - BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) { LOCK(cs_main); From 5de15972543dba32f41d5fe20354b548186972bb Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 15 Jun 2017 17:34:15 -0700 Subject: [PATCH 03/13] Use BnB selector before using original selector Set SelectCoinsMinConf to use the new selector algorithm first before falling back to the original selector algorithm. --- src/wallet/wallet.cpp | 105 ++++++-------------------------------------------- 1 file changed, 11 insertions(+), 94 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b8dadf749a0..a5338f05cc6 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -9,6 +9,7 @@ #include "checkpoints.h" #include "chain.h" #include "wallet/coincontrol.h" +#include "wallet/coinselection.h" #include "consensus/consensus.h" #include "consensus/validation.h" #include "fs.h" @@ -22,6 +23,7 @@ #include "policy/rbf.h" #include "primitives/block.h" #include "primitives/transaction.h" +#include "random.h" #include "script/script.h" #include "script/sign.h" #include "scheduler.h" @@ -67,15 +69,6 @@ const uint256 CMerkleTx::ABANDON_HASH(uint256S("00000000000000000000000000000000 * @{ */ -struct CompareValueOnly -{ - bool operator()(const CInputCoin& t1, - const CInputCoin& t2) const - { - return t1.txout.nValue < t2.txout.nValue; - } -}; - std::string COutput::ToString() const { return strprintf("COutput(%s, %d, %d) [%s]", tx->GetHash().ToString(), i, nDepth, FormatMoney(tx->tx->vout[i].nValue)); @@ -2301,60 +2294,12 @@ const CTxOut& CWallet::FindNonChangeParentOutput(const CTransaction& tx, int out return ptx->vout[n]; } -static void ApproximateBestSubset(const std::vector& vValue, const CAmount& nTotalLower, const CAmount& nTargetValue, - std::vector& vfBest, CAmount& nBest, int iterations = 1000) -{ - std::vector vfIncluded; - - vfBest.assign(vValue.size(), true); - nBest = nTotalLower; - - FastRandomContext insecure_rand; - - for (int nRep = 0; nRep < iterations && nBest != nTargetValue; nRep++) - { - vfIncluded.assign(vValue.size(), false); - CAmount nTotal = 0; - bool fReachedTarget = false; - for (int nPass = 0; nPass < 2 && !fReachedTarget; nPass++) - { - for (unsigned int i = 0; i < vValue.size(); i++) - { - //The solver here uses a randomized algorithm, - //the randomness serves no real security purpose but is just - //needed to prevent degenerate behavior and it is important - //that the rng is fast. We do not use a constant random sequence, - //because there may be some privacy improvement by making - //the selection random. - if (nPass == 0 ? insecure_rand.randbool() : !vfIncluded[i]) - { - nTotal += vValue[i].txout.nValue; - vfIncluded[i] = true; - if (nTotal >= nTargetValue) - { - fReachedTarget = true; - if (nTotal < nBest) - { - nBest = nTotal; - vfBest = vfIncluded; - } - nTotal -= vValue[i].txout.nValue; - vfIncluded[i] = false; - } - } - } - } - } -} - bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMine, const int nConfTheirs, const uint64_t nMaxAncestors, std::vector vCoins, std::set& setCoinsRet, CAmount& nValueRet, CAmount& fee_ret, const CFeeRate effective_fee, bool only_knapsack) const { setCoinsRet.clear(); nValueRet = 0; - // List of values less than target - boost::optional coinLowestLarger; std::vector vValue; if (!only_knapsack) { // Calculate cost of change @@ -2405,45 +2350,17 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMin nValueRet += coinLowestLarger->txout.nValue; return true; } + if (!use_only_knapsack) { + // Calculate cost of change + // TODO: Actually calculate + CAmount cost_of_change = 10000; - // Solve subset sum by stochastic approximation - std::sort(vValue.begin(), vValue.end(), CompareValueOnly()); - std::reverse(vValue.begin(), vValue.end()); - std::vector vfBest; - CAmount nBest; - - ApproximateBestSubset(vValue, nTotalLower, nTargetValue, vfBest, nBest); - if (nBest != nTargetValue && nTotalLower >= nTargetValue + MIN_CHANGE) - ApproximateBestSubset(vValue, nTotalLower, nTargetValue + MIN_CHANGE, vfBest, nBest); - - // If we have a bigger coin and (either the stochastic approximation didn't find a good solution, - // or the next bigger coin is closer), return the bigger coin - if (coinLowestLarger && - ((nBest != nTargetValue && nBest < nTargetValue + MIN_CHANGE) || coinLowestLarger->txout.nValue <= nBest)) - { - setCoinsRet.insert(coinLowestLarger.get()); - nValueRet += coinLowestLarger->txout.nValue; - } - else { - for (unsigned int i = 0; i < vValue.size(); i++) - if (vfBest[i]) - { - setCoinsRet.insert(vValue[i]); - nValueRet += vValue[i].txout.nValue; - } - - if (LogAcceptCategory(BCLog::SELECTCOINS)) { - LogPrint(BCLog::SELECTCOINS, "SelectCoins() best subset: "); - for (unsigned int i = 0; i < vValue.size(); i++) { - if (vfBest[i]) { - LogPrint(BCLog::SELECTCOINS, "%s ", FormatMoney(vValue[i].txout.nValue)); - } - } - LogPrint(BCLog::SELECTCOINS, "total %s\n", FormatMoney(nBest)); - } + FastRandomContext rand; + return SelectCoinsBnB(vValue, nTargetValue, cost_of_change, setCoinsRet, nValueRet, &rand) || + KnapsackSolver(vValue, nTargetValue, setCoinsRet, nValueRet); + } else { + return KnapsackSolver(vValue, nTargetValue, setCoinsRet, nValueRet); } - - return true; } bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAmount& nTargetValue, std::set& setCoinsRet, CAmount& nValueRet, CAmount& fee_ret, const CFeeRate effective_fee, const CCoinControl* coinControl, bool knapsack_only) const From 1b6d13c1ed4d436d9ec15a8ff4483e634262b5ad Mon Sep 17 00:00:00 2001 From: Gregory Sanders Date: Fri, 16 Jun 2017 12:48:51 -0700 Subject: [PATCH 04/13] Have COutput calculate size of the input required to spend it. Note: This code was copied and pasted from a PR by @instagibbs --- src/wallet/feebumper.cpp | 27 --------------------------- src/wallet/wallet.cpp | 38 ++++++++++++++++++++++++++++++++++++++ src/wallet/wallet.h | 22 +++++++++++++++++++++- 3 files changed, 59 insertions(+), 28 deletions(-) diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index c1ea2b6290b..bb3620efa83 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -15,33 +15,6 @@ #include "util.h" #include "net.h" -// Calculate the size of the transaction assuming all signatures are max size -// Use DummySignatureCreator, which inserts 72 byte signatures everywhere. -// TODO: re-use this in CWallet::CreateTransaction (right now -// CreateTransaction uses the constructed dummy-signed tx to do a priority -// calculation, but we should be able to refactor after priority is removed). -// NOTE: this requires that all inputs must be in mapWallet (eg the tx should -// be IsAllFromMe). -int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *pWallet) -{ - CMutableTransaction txNew(tx); - std::vector vCoins; - // Look up the inputs. We should have already checked that this transaction - // IsAllFromMe(ISMINE_SPENDABLE), so every input should already be in our - // wallet, with a valid index into the vout array. - for (auto& input : tx.vin) { - const auto mi = pWallet->mapWallet.find(input.prevout.hash); - assert(mi != pWallet->mapWallet.end() && input.prevout.n < mi->second.tx->vout.size()); - vCoins.emplace_back(CInputCoin(&(mi->second), input.prevout.n)); - } - if (!pWallet->DummySignTx(txNew, vCoins)) { - // This should never happen, because IsAllFromMe(ISMINE_SPENDABLE) - // implies that we can sign for every input. - return -1; - } - return GetVirtualTransactionSize(txNew); -} - bool CFeeBumper::preconditionChecks(const CWallet *pWallet, const CWalletTx& wtx) { if (pWallet->HasWalletSpend(wtx.GetHash())) { vErrors.push_back("Transaction has descendants in the wallet"); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a5338f05cc6..2cb8ca49ba5 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1503,6 +1503,44 @@ int CWalletTx::GetRequestCount() const return nRequests; } +int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *pWallet) +{ + CMutableTransaction txNew(tx); + std::vector vCoins; + // Look up the inputs. We should have already checked that this transaction + // IsAllFromMe(ISMINE_SPENDABLE), so every input should already be in our + // wallet, with a valid index into the vout array, and the ability to sign. + for (auto& input : tx.vin) { + const auto mi = pWallet->mapWallet.find(input.prevout.hash); + if (mi == pWallet->mapWallet.end()) { + return -1; + } + assert(input.prevout.n < mi->second.tx->vout.size()); + vCoins.emplace_back(CInputCoin(&(mi->second), input.prevout.n)); + } + if (!pWallet->DummySignTx(txNew, vCoins)) { + // This should never happen, because IsAllFromMe(ISMINE_SPENDABLE) + // implies that we can sign for every input. + return -1; + } + return GetVirtualTransactionSize(txNew); +} + +int CWalletTx::GetSpendSize(unsigned int i) const +{ + CMutableTransaction txn; + txn.vin.push_back(CTxIn(COutPoint(GetHash(), i))); + int totalBytes = CalculateMaximumSignedTxSize(txn, pwallet); + if (totalBytes == -1) return -1; + int witnessversion = 0; + std::vector witnessprogram; + // We don't want to multi-count segwit empty vin and flag bytes + if (tx->vout[i].scriptPubKey.IsWitnessProgram(witnessversion, witnessprogram)) { + totalBytes -= 2; + } + return totalBytes - GetVirtualTransactionSize(CMutableTransaction()); +} + void CWalletTx::GetAmounts(std::list& listReceived, std::list& listSent, CAmount& nFee, std::string& strSentAccount, const isminefilter& filter) const { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index f725b598855..67edd88e5b9 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -8,6 +8,7 @@ #include "amount.h" #include "policy/feerate.h" +#include "policy/policy.h" #include "streams.h" #include "tinyformat.h" #include "ui_interface.h" @@ -453,6 +454,9 @@ class CWalletTx : public CMerkleTx CAmount GetAvailableWatchOnlyCredit(const bool& fUseCache=true) const; CAmount GetChange() const; + // Get the marginal bytes if spending the specified output from this transaction + int GetSpendSize(unsigned int i) const; + void GetAmounts(std::list& listReceived, std::list& listSent, CAmount& nFee, std::string& strSentAccount, const isminefilter& filter) const; @@ -513,6 +517,9 @@ class COutput int i; int nDepth; + /** Pre-computed estimated size of this output as a fully-signed input in a transaction */ + int nInputBytes; + /** Whether we have the private keys to spend this output */ bool fSpendable; @@ -528,7 +535,12 @@ class COutput COutput(const CWalletTx *txIn, int iIn, int nDepthIn, bool fSpendableIn, bool fSolvableIn, bool fSafeIn) { - tx = txIn; i = iIn; nDepth = nDepthIn; fSpendable = fSpendableIn; fSolvable = fSolvableIn; fSafe = fSafeIn; + tx = txIn; i = iIn; nDepth = nDepthIn; fSpendable = fSpendableIn; fSolvable = fSolvableIn; fSafe = fSafeIn; nInputBytes = -1; + // If known and signable by the given wallet, compute nInputBytes + // Failure will keep this value -1 + if (fSpendable && tx) { + nInputBytes = tx->GetSpendSize(i); + } } std::string ToString() const; @@ -1223,4 +1235,12 @@ bool CWallet::DummySignTx(CMutableTransaction &txNew, const ContainerType &coins return true; } +// Calculate the size of the transaction assuming all signatures are max size +// Use DummySignatureCreator, which inserts 72 byte signatures everywhere. +// TODO: re-use this in CWallet::CreateTransaction (right now +// CreateTransaction uses the constructed dummy-signed tx to do a priority +// calculation, but we should be able to refactor after priority is removed). +// NOTE: this requires that all inputs must be in mapWallet (eg the tx should +// be IsAllFromMe). +int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *pWallet); #endif // BITCOIN_WALLET_WALLET_H From bfa7790058e0f314348539dd0594c92b7da5999d Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 16 Jun 2017 11:16:26 -0700 Subject: [PATCH 05/13] Calculate and use effective values for coin selection --- src/wallet/test/coinselector_tests.cpp | 62 +++++++++++++++++----------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index d5a7870938a..5701cac4a81 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -558,24 +558,24 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) empty_wallet(); // with an empty wallet we can't even pay one cent - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); 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, 1, 6, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); // but we can find a new 1 cent - BOOST_CHECK( testWallet.SelectCoinsMinConf( 1 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 1 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); 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, 1, 6, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 3 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); // we can make 3 cents of new coins - BOOST_CHECK( testWallet.SelectCoinsMinConf( 3 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 3 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); BOOST_CHECK_EQUAL(nValueRet, 3 * CENT); add_coin(5*CENT); // add a mature 5 cent coin, @@ -585,33 +585,33 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) // 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, 1, 6, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); // 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, 6, 6, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, 6, 6, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); // but we can make 37 cents if we accept new coins from ourself - BOOST_CHECK( testWallet.SelectCoinsMinConf(37 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(37 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); BOOST_CHECK_EQUAL(nValueRet, 37 * CENT); // and we can make 38 cents if we accept all new coins - BOOST_CHECK( testWallet.SelectCoinsMinConf(38 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(38 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(34 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 7 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 8 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 9 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); BOOST_CHECK_EQUAL(nValueRet, 10 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -625,30 +625,30 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); - BOOST_CHECK(!testWallet.SelectCoinsMinConf(72 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(71 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(72 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); // 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(11 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); BOOST_CHECK_EQUAL(nValueRet, 11 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); @@ -657,11 +657,11 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(95 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(195 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); BOOST_CHECK_EQUAL(nValueRet, 2 * COIN); // we should get 2 BTC in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -676,14 +676,14 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) // 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount // if we add more small coins: @@ -691,7 +691,7 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) add_coin(MIN_CHANGE * 7 / 10); // and try again to make 1.0 * MIN_CHANGE - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount // run the 'mtgox' test (see http://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) @@ -700,7 +700,7 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) for (int j = 0; j < 20; j++) add_coin(50000 * COIN); - BOOST_CHECK( testWallet.SelectCoinsMinConf(500000 * COIN, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(500000 * COIN, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); BOOST_CHECK_EQUAL(nValueRet, 500000 * COIN); // we should get the exact amount BOOST_CHECK_EQUAL(setCoinsRet.size(), 10U); // in ten coins @@ -713,7 +713,7 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) add_coin(MIN_CHANGE * 6 / 10); add_coin(MIN_CHANGE * 7 / 10); add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); BOOST_CHECK_EQUAL(nValueRet, 1111 * MIN_CHANGE); // we get the bigger coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -723,7 +723,7 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) add_coin(MIN_CHANGE * 6 / 10); add_coin(MIN_CHANGE * 8 / 10); add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); 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 @@ -734,12 +734,12 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) add_coin(MIN_CHANGE * 100); // trying to make 100.01 from these three coins - BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 10001 / 100, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 10001 / 100, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 9990 / 100, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); BOOST_CHECK_EQUAL(nValueRet, 101 * MIN_CHANGE); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); @@ -749,7 +749,7 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) // 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(2000, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); if (amt - 2000 < MIN_CHANGE) { // needs more than one input: uint16_t returnSize = std::ceil((2000.0 + MIN_CHANGE)/amt); @@ -826,7 +826,7 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) add_coin(1000 * COIN); add_coin(3 * COIN); - BOOST_CHECK(testWallet.SelectCoinsMinConf(1003 * COIN, 1, 6, 0, vCoins, setCoinsRet, nValueRet, true)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(1003 * COIN, 1, 6, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); BOOST_CHECK_EQUAL(nValueRet, 1003 * COIN); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); From 2a754c0e3cc0ccb5f141e363de69fb4e498adac8 Mon Sep 17 00:00:00 2001 From: Gregory Sanders Date: Fri, 5 May 2017 14:13:07 -0400 Subject: [PATCH 06/13] add FeeRate companions to Get_Fee wallet helpers --- src/wallet/wallet.cpp | 22 +++++++++++++++++++++- src/wallet/wallet.h | 12 ++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 2cb8ca49ba5..59bc1689b07 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2986,6 +2986,11 @@ CAmount CWallet::GetRequiredFee(unsigned int nTxBytes) return std::max(minTxFee.GetFee(nTxBytes), ::minRelayTxFee.GetFee(nTxBytes)); } +CFeeRate CWallet::GetRequiredFeeRate() +{ + return std::max(minTxFee, ::minRelayTxFee); +} + CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc) { /* User control of how to calculate fee uses the following parameter precedence: @@ -3043,7 +3048,22 @@ CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_c return fee_needed; } - +CFeeRate CWallet::GetMinimumFeeRate(unsigned int nConfirmTarget, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, bool ignoreUserSetFee) +{ + CFeeRate nFeeRateNeeded = payTxFee; + // payTxFee is the user-set global for desired feerate + // User didn't set: use -txconfirmtarget to estimate... + if (nFeeRateNeeded == CFeeRate(0) || ignoreUserSetFee) { + int estimateFoundTarget = nConfirmTarget; + nFeeRateNeeded = estimator.estimateSmartFee(nConfirmTarget, &estimateFoundTarget, pool); + // ... unless we don't have enough mempool data for estimatefee, then use fallbackFee + if (nFeeRateNeeded == CFeeRate(0)) + nFeeRateNeeded = fallbackFee; + } + // prevent user from paying a fee below minRelayTxFee or minTxFee + nFeeRateNeeded = std::max(nFeeRateNeeded, GetRequiredFeeRate()); + return nFeeRateNeeded; +} DBErrors CWallet::LoadWallet(bool& fFirstRunRet) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 67edd88e5b9..d1cd762982c 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -978,11 +978,23 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface */ static CAmount GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc); /** + * Estimate the minimum fee rate considering user set parameters + * and the required fee + */ + static CFeeRate GetMinimumFeeRate(unsigned int nConfirmTarget, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, bool ignoreUserSetFee = false); + + /** * Return the minimum required fee taking into account the * floating relay fee and user set minimum transaction fee */ static CAmount GetRequiredFee(unsigned int nTxBytes); + /** + * Return the minimum required fee taking into account the + * floating relay fee and user set minimum transaction fee + */ + static CFeeRate GetRequiredFeeRate(); + bool NewKeyPool(); size_t KeypoolCountExternalKeys(); bool TopUpKeyPool(unsigned int kpSize = 0); From 11d2fd8120b25cd17eb2945f22502dff49d45afc Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 16 Jun 2017 16:25:46 -0700 Subject: [PATCH 07/13] Use the feerate to calculate the cost of change Use the feerate passed into SelectCoins to calculate the cost of creating a change output to be used in the branch and bound searcher. --- src/wallet/wallet.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 59bc1689b07..dc89ee18e23 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2390,8 +2390,9 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMin } if (!use_only_knapsack) { // Calculate cost of change - // TODO: Actually calculate - CAmount cost_of_change = 10000; + // TODO: In the future, we should use the change output actually made for the transaction and calculate the cost + // requred to spend it. + CAmount cost_of_change = effective_fee(148+34); // 148 bytes for the input, 34 bytes for making the output FastRandomContext rand; return SelectCoinsBnB(vValue, nTargetValue, cost_of_change, setCoinsRet, nValueRet, &rand) || From b9a0d20b4e1cda42bc8f51e74e4bd9092a54832d Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 16 Jun 2017 16:40:34 -0700 Subject: [PATCH 08/13] Use an actual fee rate for calculating effective values --- src/wallet/wallet.cpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index dc89ee18e23..a47926e5346 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2392,7 +2392,7 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMin // Calculate cost of change // TODO: In the future, we should use the change output actually made for the transaction and calculate the cost // requred to spend it. - CAmount cost_of_change = effective_fee(148+34); // 148 bytes for the input, 34 bytes for making the output + CAmount cost_of_change = effective_fee.GetFee(148+34); // 148 bytes for the input, 34 bytes for making the output FastRandomContext rand; return SelectCoinsBnB(vValue, nTargetValue, cost_of_change, setCoinsRet, nValueRet, &rand) || @@ -2707,6 +2707,19 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT txNew.vout.push_back(txout); } + // Get the fee rate to use effective values in coin selection + // Allow to override the default confirmation target over the CoinControl instance + int currentConfirmationTarget = nTxConfirmTarget; + if (coinControl && coinControl->nConfirmTarget > 0) { + currentConfirmationTarget = coinControl->nConfirmTarget; + } + + CFeeRate nFeeRateNeeded = GetMinimumFeeRate(currentConfirmationTarget, ::mempool, ::feeEstimator); + + if (coinControl && coinControl->fOverrideFeeRate) { + nFeeRateNeeded = coinControl->nFeeRate; + } + // Choose coins to use if (pick_new_inputs) { nValueIn = 0; From 93e460af61060713468f24f5a3406c940e68652a Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 16 Jun 2017 18:13:35 -0700 Subject: [PATCH 09/13] Test full coinselection algorithms Test the full coin selection algorithm which begins with the branch and bound and falls back to the original algo --- src/wallet/test/coinselector_tests.cpp | 43 ++++++++++++++++++++++++++++++++++ src/wallet/wallet.cpp | 7 +++--- src/wallet/wallet.h | 2 +- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 5701cac4a81..a0dae767aa0 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -11,6 +11,7 @@ #include "wallet/test/wallet_test_fixture.h" #include +#include BOOST_FIXTURE_TEST_SUITE(coin_selection_tests, WalletTestingSetup) @@ -28,6 +29,7 @@ typedef std::set CoinSet; static std::vector vCoins; static std::vector fee_vec; static const CWallet testWallet; +static CAmount balance = 0; static void add_coin(const CAmount& nValue, int nInput, std::vector& set) { @@ -41,6 +43,7 @@ static void add_coin(const CAmount& nValue, int nInput, std::vector& 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 @@ -833,4 +836,44 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) 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; + + // 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((unsigned long)(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 + BOOST_CHECK(testWallet.SelectCoinsMinConf(target, 1, 6, 0, vCoins, out_set, out_value, rate)); + BOOST_CHECK_GE(out_value, target); + } +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a47926e5346..d5eec826848 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2714,7 +2714,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT currentConfirmationTarget = coinControl->nConfirmTarget; } - CFeeRate nFeeRateNeeded = GetMinimumFeeRate(currentConfirmationTarget, ::mempool, ::feeEstimator); + CFeeRate nFeeRateNeeded = GetMinimumFeeRate(currentConfirmationTarget, ::mempool, ::feeEstimator, &feeCalc); if (coinControl && coinControl->fOverrideFeeRate) { nFeeRateNeeded = coinControl->nFeeRate; @@ -3062,14 +3062,13 @@ CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_c return fee_needed; } -CFeeRate CWallet::GetMinimumFeeRate(unsigned int nConfirmTarget, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, bool ignoreUserSetFee) +CFeeRate CWallet::GetMinimumFeeRate(unsigned int nConfirmTarget, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc, bool ignoreUserSetFee) { CFeeRate nFeeRateNeeded = payTxFee; // payTxFee is the user-set global for desired feerate // User didn't set: use -txconfirmtarget to estimate... if (nFeeRateNeeded == CFeeRate(0) || ignoreUserSetFee) { - int estimateFoundTarget = nConfirmTarget; - nFeeRateNeeded = estimator.estimateSmartFee(nConfirmTarget, &estimateFoundTarget, pool); + nFeeRateNeeded = estimator.estimateSmartFee(nConfirmTarget, feeCalc, pool); // ... unless we don't have enough mempool data for estimatefee, then use fallbackFee if (nFeeRateNeeded == CFeeRate(0)) nFeeRateNeeded = fallbackFee; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index d1cd762982c..f4a61572a42 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -981,7 +981,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface * Estimate the minimum fee rate considering user set parameters * and the required fee */ - static CFeeRate GetMinimumFeeRate(unsigned int nConfirmTarget, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, bool ignoreUserSetFee = false); + static CFeeRate GetMinimumFeeRate(unsigned int nConfirmTarget, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc, bool ignoreUserSetFee = false); /** * Return the minimum required fee taking into account the From 3c373cc815ea64d7ea9e40328b281d884cec523c Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 20 Jun 2017 16:41:47 -0700 Subject: [PATCH 10/13] Use BnB selector only on first pass, knapsack selector on rest Separate SelectCoinsMinConf to allow for either BnB selector or knapsack selector. This allows CreateTransaction to run the BnB selector on the first pass and the knapsack selector on the rest to keep the original selection algorithm as a fallback. Add a return parameter to selectcoins and selectoinsminconf to allow the caller to know whether bnb was used for the coinselection or not. This allows us to cleanly skip change output creation since BnB would only be successful if there is no change. Changed used_bnb and first_pass to be just first_pass renamed as use_bnb --- src/bench/coin_selection.cpp | 3 +- src/wallet/test/coinselector_tests.cpp | 93 ++++++++++++------------- src/wallet/wallet.cpp | 122 +++++++++++++++++++++++++-------- src/wallet/wallet.h | 6 +- 4 files changed, 145 insertions(+), 79 deletions(-) diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index ae0d2c202c7..9bd765913b9 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -50,7 +50,8 @@ static void CoinSelection(benchmark::State& state) std::set setCoinsRet; CAmount nValueRet; CAmount fee_ret; - bool success = wallet.SelectCoinsMinConf(1003 * COIN, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret); + bool success = wallet.SelectCoinsMinConf(1003 * COIN, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true) + || wallet.SelectCoinsMinConf(1003 * COIN, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false); 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 a0dae767aa0..64bdcbd2d85 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -228,24 +228,24 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) empty_wallet(); // with an empty wallet we can't even pay one cent - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); 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, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); // but we can find a new 1 cent - BOOST_CHECK( testWallet.SelectCoinsMinConf( 1 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 1 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); 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, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 3 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); // we can make 3 cents of new coins - BOOST_CHECK( testWallet.SelectCoinsMinConf( 3 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 3 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); BOOST_CHECK_EQUAL(nValueRet, 3 * CENT); add_coin(5*CENT); // add a mature 5 cent coin, @@ -255,33 +255,33 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) // 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, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); // 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, 6, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, 6, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); // but we can make 37 cents if we accept new coins from ourself - BOOST_CHECK( testWallet.SelectCoinsMinConf(37 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(37 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); BOOST_CHECK_EQUAL(nValueRet, 37 * CENT); // and we can make 38 cents if we accept all new coins - BOOST_CHECK( testWallet.SelectCoinsMinConf(38 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(38 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(34 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 7 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 8 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 9 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); BOOST_CHECK_EQUAL(nValueRet, 10 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -295,30 +295,30 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); - BOOST_CHECK(!testWallet.SelectCoinsMinConf(72 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(71 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(72 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); // 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(11 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); BOOST_CHECK_EQUAL(nValueRet, 11 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); @@ -327,11 +327,11 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(95 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(195 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); BOOST_CHECK_EQUAL(nValueRet, 2 * COIN); // we should get 2 BTC in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -346,14 +346,14 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) // 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount // if we add more small coins: @@ -361,7 +361,7 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) add_coin(MIN_CHANGE * 7 / 10); // and try again to make 1.0 * MIN_CHANGE - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount // run the 'mtgox' test (see http://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) @@ -370,7 +370,7 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) for (int j = 0; j < 20; j++) add_coin(50000 * COIN); - BOOST_CHECK( testWallet.SelectCoinsMinConf(500000 * COIN, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(500000 * COIN, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); BOOST_CHECK_EQUAL(nValueRet, 500000 * COIN); // we should get the exact amount BOOST_CHECK_EQUAL(setCoinsRet.size(), 10U); // in ten coins @@ -383,7 +383,7 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) add_coin(MIN_CHANGE * 6 / 10); add_coin(MIN_CHANGE * 7 / 10); add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); BOOST_CHECK_EQUAL(nValueRet, 1111 * MIN_CHANGE); // we get the bigger coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -393,7 +393,7 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) add_coin(MIN_CHANGE * 6 / 10); add_coin(MIN_CHANGE * 8 / 10); add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); 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 @@ -404,12 +404,12 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) add_coin(MIN_CHANGE * 100); // trying to make 100.01 from these three coins - BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 10001 / 100, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 10001 / 100, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 9990 / 100, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); BOOST_CHECK_EQUAL(nValueRet, 101 * MIN_CHANGE); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); @@ -419,7 +419,7 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) // 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(2000, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); if (amt - 2000 < MIN_CHANGE) { // needs more than one input: uint16_t returnSize = std::ceil((2000.0 + MIN_CHANGE)/amt); @@ -441,8 +441,8 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) // 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, 1, 6, 0, vCoins, setCoinsRet , nValueRet, fee_ret, CFeeRate(0), true)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, 1, 6, 0, vCoins, setCoinsRet2, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, 1, 6, 0, vCoins, setCoinsRet , nValueRet, fee_ret, CFeeRate(0), false)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, 1, 6, 0, vCoins, setCoinsRet2, nValueRet, fee_ret, CFeeRate(0), false)); BOOST_CHECK(!equal_sets(setCoinsRet, setCoinsRet2)); int fails = 0; @@ -450,8 +450,8 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) { // 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, 1, 6, 0, vCoins, setCoinsRet , nValueRet, fee_ret, CFeeRate(0), true)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, 1, 6, 0, vCoins, setCoinsRet2, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, 1, 6, 0, vCoins, setCoinsRet , nValueRet, fee_ret, CFeeRate(0), false)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, 1, 6, 0, vCoins, setCoinsRet2, nValueRet, fee_ret, CFeeRate(0), false)); if (equal_sets(setCoinsRet, setCoinsRet2)) fails++; } @@ -471,8 +471,8 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) { // 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, 1, 6, 0, vCoins, setCoinsRet , nValueRet, fee_ret, CFeeRate(0), true)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, 1, 6, 0, vCoins, setCoinsRet2, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, 1, 6, 0, vCoins, setCoinsRet , nValueRet, fee_ret, CFeeRate(0), false)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, 1, 6, 0, vCoins, setCoinsRet2, nValueRet, fee_ret, CFeeRate(0), false)); if (equal_sets(setCoinsRet, setCoinsRet2)) fails++; } @@ -497,7 +497,7 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) add_coin(1000 * COIN); add_coin(3 * COIN); - BOOST_CHECK(testWallet.SelectCoinsMinConf(1003 * COIN, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), true)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(1003 * COIN, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, CFeeRate(0), false)); BOOST_CHECK_EQUAL(nValueRet, 1003 * COIN); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); @@ -540,8 +540,8 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) target = rand.randrange(balance - 1000) + 1000; // Perform selection - BOOST_CHECK(testWallet.SelectCoinsMinConf(target, 1, 6, 0, vCoins, out_set, out_value, fee_ret, rate, false) || - testWallet.SelectCoinsMinConf(target, 1, 6, 0, vCoins, out_set, out_value, fee_ret, rate, true)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(target, 1, 6, 0, vCoins, out_set, out_value, fee_ret, rate, true) || + testWallet.SelectCoinsMinConf(target, 1, 6, 0, vCoins, out_set, out_value, fee_ret, rate, false)); BOOST_CHECK_GE(out_value, target); } } @@ -774,8 +774,8 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) // 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, 1, 6, 0, vCoins, setCoinsRet , nValueRet)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, 1, 6, 0, vCoins, setCoinsRet2, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, 1, 6, 0, vCoins, setCoinsRet , nValueRet, CFeeRate(0), true)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, 1, 6, 0, vCoins, setCoinsRet2, nValueRet, CFeeRate(0), true)); BOOST_CHECK(!equal_sets(setCoinsRet, setCoinsRet2)); int fails = 0; @@ -783,8 +783,8 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) { // 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, 1, 6, 0, vCoins, setCoinsRet , nValueRet)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, 1, 6, 0, vCoins, setCoinsRet2, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, 1, 6, 0, vCoins, setCoinsRet , nValueRet, CFeeRate(0), true)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, 1, 6, 0, vCoins, setCoinsRet2, nValueRet, CFeeRate(0), true)); if (equal_sets(setCoinsRet, setCoinsRet2)) fails++; } @@ -804,8 +804,8 @@ BOOST_AUTO_TEST_CASE(knapsack_tests) { // 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, 1, 6, 0, vCoins, setCoinsRet , nValueRet)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, 1, 6, 0, vCoins, setCoinsRet2, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, 1, 6, 0, vCoins, setCoinsRet , nValueRet, CFeeRate(0), true)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, 1, 6, 0, vCoins, setCoinsRet2, nValueRet, CFeeRate(0), true)); if (equal_sets(setCoinsRet, setCoinsRet2)) fails++; } @@ -871,7 +871,8 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) target = rand.randrange(balance - 1000) + 1000; // Perform selection - BOOST_CHECK(testWallet.SelectCoinsMinConf(target, 1, 6, 0, vCoins, out_set, out_value, rate)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(target, 1, 6, 0, vCoins, out_set, out_value, rate, false) || + testWallet.SelectCoinsMinConf(target, 1, 6, 0, vCoins, out_set, out_value, rate, true)); BOOST_CHECK_GE(out_value, target); } } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d5eec826848..6346bb57348 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2333,13 +2333,17 @@ const CTxOut& CWallet::FindNonChangeParentOutput(const CTransaction& tx, int out } bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMine, const int nConfTheirs, const uint64_t nMaxAncestors, std::vector vCoins, - std::set& setCoinsRet, CAmount& nValueRet, CAmount& fee_ret, const CFeeRate effective_fee, bool only_knapsack) const + std::set& setCoinsRet, CAmount& nValueRet, CAmount& fee_ret, const CFeeRate effective_fee, bool use_bnb, int change_size) const { setCoinsRet.clear(); nValueRet = 0; std::vector vValue; - if (!only_knapsack) { + if (use_bnb) { + // Get the fee rate to use for the change fee rate + FeeCalculation feeCalc; + CFeeRate change_feerate = GetMinimumFeeRate(1008, ::mempool, ::feeEstimator, &feeCalc); + // Calculate cost of change // TODO: In the future, we should use the change output actually made for the transaction and calculate the cost // requred to spend it. @@ -2394,15 +2398,52 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMin // requred to spend it. CAmount cost_of_change = effective_fee.GetFee(148+34); // 148 bytes for the input, 34 bytes for making the output + // Filter by the min conf specs and add to vValue and calculate effective value + for (const COutput &output : vCoins) + { + if (!output.fSpendable) + continue; + + const CWalletTx *pcoin = output.tx; + + if (output.nDepth < (pcoin->IsFromMe(ISMINE_ALL) ? nConfMine : nConfTheirs)) + continue; + + if (!mempool.TransactionWithinChainLimit(pcoin->GetHash(), nMaxAncestors)) + continue; + + int i = output.i; + CInputCoin coin(pcoin, i); + coin.txout.nValue -= (output.nInputBytes < 0 ? 0 : effective_fee.GetFee(output.nInputBytes)); + vValue.push_back(coin); + } + FastRandomContext rand; - return SelectCoinsBnB(vValue, nTargetValue, cost_of_change, setCoinsRet, nValueRet, &rand) || - KnapsackSolver(vValue, nTargetValue, setCoinsRet, nValueRet); + return SelectCoinsBnB(vValue, nTargetValue, cost_of_change, setCoinsRet, nValueRet, &rand); } else { + // Filter by the min conf specs and add to vValue + for (const COutput &output : vCoins) + { + if (!output.fSpendable) + continue; + + const CWalletTx *pcoin = output.tx; + + if (output.nDepth < (pcoin->IsFromMe(ISMINE_ALL) ? nConfMine : nConfTheirs)) + continue; + + if (!mempool.TransactionWithinChainLimit(pcoin->GetHash(), nMaxAncestors)) + continue; + + int i = output.i; + CInputCoin coin(pcoin, i); + vValue.push_back(coin); + } return KnapsackSolver(vValue, nTargetValue, setCoinsRet, nValueRet); } } -bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAmount& nTargetValue, std::set& setCoinsRet, CAmount& nValueRet, CAmount& fee_ret, const CFeeRate effective_fee, const CCoinControl* coinControl, bool knapsack_only) const +bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAmount& nTargetValue, std::set& setCoinsRet, CAmount& nValueRet, CAmount& fee_ret, const CFeeRate effective_fee, const CCoinControl* coinControl, bool use_bnb, int change_size) const { std::vector vCoins(vAvailableCoins); @@ -2455,13 +2496,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, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, knapsack_only) || - SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, knapsack_only) || - (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, 2, vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, knapsack_only)) || - (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, std::min((size_t)4, nMaxChainLength/3), vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, knapsack_only)) || - (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, nMaxChainLength/2, vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, knapsack_only)) || - (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, nMaxChainLength, vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, knapsack_only)) || - (bSpendZeroConfChange && !fRejectLongChains && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, std::numeric_limits::max(), vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, knapsack_only)); + SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 1, 6, 0, vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, use_bnb, change_size) || + SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 1, 1, 0, vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, use_bnb, change_size) || + (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, 2, vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, use_bnb, change_size)) || + (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, std::min((size_t)4, nMaxChainLength/3), vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, use_bnb, change_size)) || + (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, nMaxChainLength/2, vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, use_bnb, change_size)) || + (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, nMaxChainLength, vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, use_bnb, change_size)) || + (bSpendZeroConfChange && !fRejectLongChains && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, std::numeric_limits::max(), vCoins, setCoinsRet, nValueRet, fee_ret, effective_fee, use_bnb, change_size)); // because SelectCoinsMinConf clears the setCoinsRet, we now add the possible inputs to the coinset setCoinsRet.insert(setPresetCoins.begin(), setPresetCoins.end()); @@ -2660,9 +2701,27 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT size_t change_prototype_size = GetSerializeSize(change_prototype_txout, SER_DISK, 0); CFeeRate discard_rate = GetDiscardRate(::feeEstimator); + + // Get the fee rate to use effective values in coin selection + // Allow to override the default confirmation target over the CoinControl instance + int currentConfirmationTarget = nTxConfirmTarget; + if (coinControl && coinControl->nConfirmTarget > 0) { + currentConfirmationTarget = coinControl->nConfirmTarget; + } + + CFeeRate nFeeRateNeeded = GetMinimumFeeRate(currentConfirmationTarget, ::mempool, ::feeEstimator, &feeCalc); + + if (coinControl && coinControl->fOverrideFeeRate) { + nFeeRateNeeded = coinControl->nFeeRate; + } + 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. + bool use_bnb = true; // Start with no fee and loop until there is enough fee while (true) { @@ -2675,6 +2734,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT CAmount nValueToSelect = nValue; if (nSubtractFeeFromAmount == 0) nValueToSelect += nFeeRet; + // vouts to the payees for (const auto& recipient : vecSend) { @@ -2689,6 +2749,9 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT fFirst = false; txout.nValue -= nFeeRet % nSubtractFeeFromAmount; } + } else if (use_bnb){ + // On the first pass BnB selector, include the fee cost for outputs + output_fees += nFeeRateNeeded.GetFee(::GetSerializeSize(txout, SER_NETWORK, PROTOCOL_VERSION)); } if (IsDust(txout, ::dustRelayFee)) @@ -2706,35 +2769,33 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT } txNew.vout.push_back(txout); } - - // Get the fee rate to use effective values in coin selection - // Allow to override the default confirmation target over the CoinControl instance - int currentConfirmationTarget = nTxConfirmTarget; - if (coinControl && coinControl->nConfirmTarget > 0) { - currentConfirmationTarget = coinControl->nConfirmTarget; - } - - CFeeRate nFeeRateNeeded = GetMinimumFeeRate(currentConfirmationTarget, ::mempool, ::feeEstimator, &feeCalc); - - if (coinControl && coinControl->fOverrideFeeRate) { - nFeeRateNeeded = coinControl->nFeeRate; + if (use_bnb) { + nValueToSelect += output_fees; } // Choose coins to use if (pick_new_inputs) { nValueIn = 0; setCoins.clear(); - setCoins.clear(); - if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, nFeeRet, nFeeRateNeeded, coinControl, !first_pass)) + if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, nFeeRet, nFeeRateNeeded, coinControl, use_bnb)) { - 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 (use_bnb) { + use_bnb = false; + continue; + } + else { + strFailReason = _("Insufficient funds"); + return false; + } + } + if (use_bnb) { + nFeeRet += output_fees; } } const CAmount nChange = nValueIn - nValueToSelect; - - if (nChange > 0) + if (nChange > 0 && !use_bnb) // Don't make change if bnb selector was used { // Fill a vout to ourself CTxOut newTxOut(nChange, scriptChange); @@ -2863,6 +2924,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT // Include more fee and try again. nFeeRet = nFeeNeeded; + use_bnb = false; continue; } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index f4a61572a42..f45e13f511e 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -675,7 +675,8 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface * 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, CAmount& fee_ret, const CFeeRate effective_fee = CFeeRate(0), const CCoinControl *coinControl = NULL, bool knapsack_only = false) const; + // TODO: Change the hard coded change_size later when we aren't just using P2PKH change outputs + bool SelectCoins(const std::vector& vAvailableCoins, const CAmount& nTargetValue, std::set& setCoinsRet, CAmount& nValueRet, CAmount& fee_ret, const CFeeRate effective_fee, const CCoinControl *coinControl = NULL, bool use_bnb = true, int change_size = 148+34) const; CWalletDB *pwalletdbEncryption; @@ -849,7 +850,8 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface * completion the coin set and corresponding actual target value is * assembled */ - bool SelectCoinsMinConf(const CAmount& nTargetValue, int nConfMine, int nConfTheirs, uint64_t nMaxAncestors, std::vector vCoins, std::set& setCoinsRet, CAmount& nValueRet, CAmount& fee_ret, const CFeeRate effective_fee = CFeeRate(0), bool only_knapsack = false) const; + // TODO: Change the hard coded change_size when we aren't only using P2PKH change outputs + bool SelectCoinsMinConf(const CAmount& nTargetValue, int nConfMine, int nConfTheirs, uint64_t nMaxAncestors, std::vector vCoins, std::set& setCoinsRet, CAmount& nValueRet, CAmount& fee_ret, const CFeeRate effective_fee, bool use_bnb = true, int change_size = 148+34) const; bool IsSpent(const uint256& hash, unsigned int n) const; From ffcb5241a3bd9244d2b4cdd25be3025a3eaad8bc Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 22 Jun 2017 18:22:52 -0700 Subject: [PATCH 11/13] long target fee estimation for change cost and change size param Use the largest possible fee esitmation target for calculating the cost of change. Also pass in the size of the change in a parameter instead of a having it be a constant inside the function. --- src/wallet/wallet.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6346bb57348..73d42ccefbc 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2345,9 +2345,7 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMin CFeeRate change_feerate = GetMinimumFeeRate(1008, ::mempool, ::feeEstimator, &feeCalc); // Calculate cost of change - // TODO: In the future, we should use the change output actually made for the transaction and calculate the cost - // requred to spend it. - CAmount cost_of_change = effective_fee.GetFee(148+34); // 148 bytes for the input, 34 bytes for making the output + CAmount cost_of_change = change_feerate.GetFee(change_size); // Filter by the min conf specs and add to vValue and calculate effective value std::vector fee_vec; // To keep track of the fees for each input From 74a863b0b3a6dc2d82ee960b70823852d1d8698f Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Sun, 2 Jul 2017 14:41:31 -0700 Subject: [PATCH 12/13] Include output fee cost in the target value Make sure to account for the cost of outputs in the target value. --- src/wallet/wallet.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 73d42ccefbc..3f187a97850 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2734,6 +2734,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT nValueToSelect += nFeeRet; // vouts to the payees + CAmount output_fees = 0; for (const auto& recipient : vecSend) { CTxOut txout(recipient.nAmount, recipient.scriptPubKey); From 6fa50a6288f1ebd8b1ba5bb3a439b4ccc6e1afe6 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 17 Aug 2017 14:57:35 -0700 Subject: [PATCH 13/13] Fix GetMinimumFeeRate; rebase fixes --- src/wallet/coinselection.cpp | 8 + src/wallet/test/coinselector_tests.cpp | 351 ++------------------------------- src/wallet/wallet.cpp | 144 ++++---------- src/wallet/wallet.h | 4 +- 4 files changed, 70 insertions(+), 437 deletions(-) diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index d2065af121c..11174f9ea37 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -8,6 +8,14 @@ // Descending order comparator struct { + bool operator()(CInputCoin a, CInputCoin b) const + { + return a.txout.nValue > b.txout.nValue; + } +} descending; + +struct CompareValueOnly +{ bool operator()(const CInputCoin& t1, const CInputCoin& t2) const { diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 64bdcbd2d85..f0844c1ac73 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -41,6 +41,17 @@ static void add_coin(const CAmount& nValue, int nInput, std::vector& fee_vec.push_back(0); } +static void add_coin(const CAmount& nValue, int nInput, CoinSet& set) +{ + CMutableTransaction tx; + tx.vout.resize(nInput+1); + tx.vout[nInput].nValue = nValue; + std::unique_ptr wtx(new CWalletTx(&testWallet, MakeTransactionRef(std::move(tx)))); + set.emplace(wtx.get(), nInput); + fee_vec.push_back(0); +} + + static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0) { balance += nValue; @@ -54,6 +65,12 @@ static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = fa // so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe() tx.vin.resize(1); } + std::unique_ptr wtx(new CWalletTx(&testWallet, MakeTransactionRef(std::move(tx)))); + if (fIsFromMe) + { + wtx->fDebitCached = true; + wtx->nDebitCached = 1; + } COutput output(wtx.get(), nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */); vCoins.push_back(output); wtxn.emplace_back(std::move(wtx)); @@ -89,6 +106,9 @@ static CAmount make_hard_case(int utxos, std::vector& utxo_pool) // Branch and bound coin selection tests BOOST_AUTO_TEST_CASE(bnb_search_test) { + + LOCK(testWallet.cs_wallet); + // Setup std::vector utxo_pool; CoinSet selection; @@ -546,335 +566,4 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) } } - -// Original coin selection algorithm tests -BOOST_AUTO_TEST_CASE(knapsack_tests) -{ - CoinSet setCoinsRet, setCoinsRet2; - CAmount nValueRet; - - LOCK(testWallet.cs_wallet); - - // test multiple times to allow for differences in the shuffle order - for (int i = 0; i < RUN_TESTS; i++) - { - empty_wallet(); - - // with an empty wallet we can't even pay one cent - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - - 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, 1, 6, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - - // but we can find a new 1 cent - BOOST_CHECK( testWallet.SelectCoinsMinConf( 1 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - 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, 1, 6, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - - // we can make 3 cents of new coins - BOOST_CHECK( testWallet.SelectCoinsMinConf( 3 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - BOOST_CHECK_EQUAL(nValueRet, 3 * CENT); - - add_coin(5*CENT); // add a mature 5 cent coin, - add_coin(10*CENT, 3, true); // a new 10 cent coin sent from one of our own addresses - add_coin(20*CENT); // and a mature 20 cent coin - - // 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, 1, 6, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - // 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, 6, 6, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - // but we can make 37 cents if we accept new coins from ourself - BOOST_CHECK( testWallet.SelectCoinsMinConf(37 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - BOOST_CHECK_EQUAL(nValueRet, 37 * CENT); - // and we can make 38 cents if we accept all new coins - BOOST_CHECK( testWallet.SelectCoinsMinConf(38 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - BOOST_CHECK_EQUAL(nValueRet, 10 * CENT); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - - // now clear out the wallet and start again to test choosing between subsets of smaller coins and the next biggest coin - empty_wallet(); - - add_coin( 6*CENT); - add_coin( 7*CENT); - add_coin( 8*CENT); - add_coin(20*CENT); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - BOOST_CHECK(!testWallet.SelectCoinsMinConf(72 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - - // 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - BOOST_CHECK_EQUAL(nValueRet, 11 * CENT); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); - - // check that the smallest bigger coin is used - add_coin( 1*COIN); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - BOOST_CHECK_EQUAL(nValueRet, 2 * COIN); // we should get 2 BTC in 1 coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - - // empty the wallet and start again, now with fractions of a cent, to test small change avoidance - - empty_wallet(); - add_coin(MIN_CHANGE * 1 / 10); - add_coin(MIN_CHANGE * 2 / 10); - add_coin(MIN_CHANGE * 3 / 10); - add_coin(MIN_CHANGE * 4 / 10); - add_coin(MIN_CHANGE * 5 / 10); - - // 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount - - // if we add more small coins: - add_coin(MIN_CHANGE * 6 / 10); - add_coin(MIN_CHANGE * 7 / 10); - - // and try again to make 1.0 * MIN_CHANGE - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount - - // run the 'mtgox' test (see http://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) - // they tried to consolidate 10 50k coins into one 500k coin, and ended up with 50k in change - empty_wallet(); - for (int j = 0; j < 20; j++) - add_coin(50000 * COIN); - - BOOST_CHECK( testWallet.SelectCoinsMinConf(500000 * COIN, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - BOOST_CHECK_EQUAL(nValueRet, 500000 * COIN); // we should get the exact amount - BOOST_CHECK_EQUAL(setCoinsRet.size(), 10U); // in ten coins - - // if there's not enough in the smaller coins to make at least 1 * MIN_CHANGE change (0.5+0.6+0.7 < 1.0+1.0), - // we need to try finding an exact subset anyway - - // sometimes it will fail, and so we use the next biggest coin: - empty_wallet(); - add_coin(MIN_CHANGE * 5 / 10); - add_coin(MIN_CHANGE * 6 / 10); - add_coin(MIN_CHANGE * 7 / 10); - add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - BOOST_CHECK_EQUAL(nValueRet, 1111 * MIN_CHANGE); // we get the bigger coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - - // but sometimes it's possible, and we use an exact subset (0.4 + 0.6 = 1.0) - empty_wallet(); - add_coin(MIN_CHANGE * 4 / 10); - add_coin(MIN_CHANGE * 6 / 10); - add_coin(MIN_CHANGE * 8 / 10); - add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - 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 - - // test avoiding small change - empty_wallet(); - add_coin(MIN_CHANGE * 5 / 100); - add_coin(MIN_CHANGE * 1); - add_coin(MIN_CHANGE * 100); - - // trying to make 100.01 from these three coins - BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 10001 / 100, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - BOOST_CHECK_EQUAL(nValueRet, 101 * MIN_CHANGE); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); - - // test with many inputs - for (CAmount amt=1500; amt < COIN; amt*=10) { - empty_wallet(); - // 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, 1, 1, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - if (amt - 2000 < MIN_CHANGE) { - // needs more than one input: - uint16_t returnSize = std::ceil((2000.0 + MIN_CHANGE)/amt); - CAmount returnValue = amt * returnSize; - BOOST_CHECK_EQUAL(nValueRet, returnValue); - BOOST_CHECK_EQUAL(setCoinsRet.size(), returnSize); - } else { - // one input is sufficient: - BOOST_CHECK_EQUAL(nValueRet, amt); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - } - } - - // test randomness - { - empty_wallet(); - for (int i2 = 0; i2 < 100; i2++) - add_coin(COIN); - - // 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, 1, 6, 0, vCoins, setCoinsRet , nValueRet, CFeeRate(0), true)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, 1, 6, 0, vCoins, setCoinsRet2, nValueRet, CFeeRate(0), true)); - 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, 1, 6, 0, vCoins, setCoinsRet , nValueRet, CFeeRate(0), true)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, 1, 6, 0, vCoins, setCoinsRet2, nValueRet, CFeeRate(0), true)); - if (equal_sets(setCoinsRet, setCoinsRet2)) - fails++; - } - BOOST_CHECK_NE(fails, RANDOM_REPEATS); - - // add 75 cents in small change. not enough to make 90 cents, - // then try making 90 cents. there are multiple competing "smallest bigger" coins, - // one of which should be picked at random - add_coin(5 * CENT); - add_coin(10 * CENT); - add_coin(15 * CENT); - add_coin(20 * CENT); - add_coin(25 * CENT); - - 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, 1, 6, 0, vCoins, setCoinsRet , nValueRet, CFeeRate(0), true)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, 1, 6, 0, vCoins, setCoinsRet2, nValueRet, CFeeRate(0), true)); - if (equal_sets(setCoinsRet, setCoinsRet2)) - fails++; - } - BOOST_CHECK_NE(fails, RANDOM_REPEATS); - } - } - empty_wallet(); -} - -BOOST_AUTO_TEST_CASE(ApproximateBestSubset) -{ - CoinSet setCoinsRet; - CAmount nValueRet; - - LOCK(testWallet.cs_wallet); - - empty_wallet(); - - // Test vValue sort order - for (int i = 0; i < 1000; i++) - add_coin(1000 * COIN); - add_coin(3 * COIN); - - BOOST_CHECK(testWallet.SelectCoinsMinConf(1003 * COIN, 1, 6, 0, vCoins, setCoinsRet, nValueRet, CFeeRate(0), true)); - 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; - - // 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((unsigned long)(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 - BOOST_CHECK(testWallet.SelectCoinsMinConf(target, 1, 6, 0, vCoins, out_set, out_value, rate, false) || - testWallet.SelectCoinsMinConf(target, 1, 6, 0, vCoins, out_set, out_value, rate, true)); - BOOST_CHECK_GE(out_value, target); - } -} - BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3f187a97850..af1095baa70 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2342,7 +2342,9 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMin if (use_bnb) { // Get the fee rate to use for the change fee rate FeeCalculation feeCalc; - CFeeRate change_feerate = GetMinimumFeeRate(1008, ::mempool, ::feeEstimator, &feeCalc); + CCoinControl temp; + temp.m_confirm_target = 1008; + CFeeRate change_feerate = GetMinimumFeeRate(temp, ::mempool, ::feeEstimator, &feeCalc); // Calculate cost of change CAmount cost_of_change = change_feerate.GetFee(change_size); @@ -2354,13 +2356,13 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMin if (!output.fSpendable) continue; - const CWalletTx *pcoin = output.tx; + const CWalletTx *pcoin = output.tx; - if (output.nDepth < (pcoin->IsFromMe(ISMINE_ALL) ? nConfMine : nConfTheirs)) - continue; + if (output.nDepth < (pcoin->IsFromMe(ISMINE_ALL) ? nConfMine : nConfTheirs)) + continue; - if (!mempool.TransactionWithinChainLimit(pcoin->GetHash(), nMaxAncestors)) - continue; + if (!mempool.TransactionWithinChainLimit(pcoin->GetHash(), nMaxAncestors)) + continue; int i = output.i; CInputCoin coin(pcoin, i); @@ -2376,52 +2378,6 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMin // Filter by the min conf specs and add to vValue for (const COutput &output : vCoins) { - setCoinsRet.insert(input); - nValueRet += input.txout.nValue; - } - return true; - } - - if (nTotalLower < nTargetValue) - { - if (!coinLowestLarger) - return false; - setCoinsRet.insert(coinLowestLarger.get()); - nValueRet += coinLowestLarger->txout.nValue; - return true; - } - if (!use_only_knapsack) { - // Calculate cost of change - // TODO: In the future, we should use the change output actually made for the transaction and calculate the cost - // requred to spend it. - CAmount cost_of_change = effective_fee.GetFee(148+34); // 148 bytes for the input, 34 bytes for making the output - - // Filter by the min conf specs and add to vValue and calculate effective value - for (const COutput &output : vCoins) - { - if (!output.fSpendable) - continue; - - const CWalletTx *pcoin = output.tx; - - if (output.nDepth < (pcoin->IsFromMe(ISMINE_ALL) ? nConfMine : nConfTheirs)) - continue; - - if (!mempool.TransactionWithinChainLimit(pcoin->GetHash(), nMaxAncestors)) - continue; - - int i = output.i; - CInputCoin coin(pcoin, i); - coin.txout.nValue -= (output.nInputBytes < 0 ? 0 : effective_fee.GetFee(output.nInputBytes)); - vValue.push_back(coin); - } - - FastRandomContext rand; - return SelectCoinsBnB(vValue, nTargetValue, cost_of_change, setCoinsRet, nValueRet, &rand); - } else { - // Filter by the min conf specs and add to vValue - for (const COutput &output : vCoins) - { if (!output.fSpendable) continue; @@ -2441,12 +2397,12 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMin } } -bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAmount& nTargetValue, std::set& setCoinsRet, CAmount& nValueRet, CAmount& fee_ret, const CFeeRate effective_fee, const CCoinControl* coinControl, bool use_bnb, int change_size) const +bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAmount& nTargetValue, std::set& setCoinsRet, CAmount& nValueRet, CAmount& fee_ret, const CFeeRate effective_fee, const CCoinControl& coin_control, bool use_bnb, int change_size) 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) { for (const COutput& out : vCoins) { @@ -2463,8 +2419,7 @@ 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) { std::map::const_iterator it = mapWallet.find(outpoint.hash); @@ -2482,7 +2437,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, it->i))) it = vCoins.erase(it); @@ -2701,17 +2656,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT CFeeRate discard_rate = GetDiscardRate(::feeEstimator); // Get the fee rate to use effective values in coin selection - // Allow to override the default confirmation target over the CoinControl instance - int currentConfirmationTarget = nTxConfirmTarget; - if (coinControl && coinControl->nConfirmTarget > 0) { - currentConfirmationTarget = coinControl->nConfirmTarget; - } - - CFeeRate nFeeRateNeeded = GetMinimumFeeRate(currentConfirmationTarget, ::mempool, ::feeEstimator, &feeCalc); - - if (coinControl && coinControl->fOverrideFeeRate) { - nFeeRateNeeded = coinControl->nFeeRate; - } + CFeeRate nFeeRateNeeded = GetMinimumFeeRate(coin_control, ::mempool, ::feeEstimator, &feeCalc); nFeeRet = 0; bool pick_new_inputs = true; @@ -2776,7 +2721,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT if (pick_new_inputs) { nValueIn = 0; setCoins.clear(); - if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, nFeeRet, nFeeRateNeeded, coinControl, use_bnb)) + if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, nFeeRet, nFeeRateNeeded, coin_control, use_bnb)) { // If BnB was used, it was the first pass. No longer the first pass and continue loop with knapsack. if (use_bnb) { @@ -3068,6 +3013,18 @@ CFeeRate CWallet::GetRequiredFeeRate() CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc) { + // Get the minimum fee rate + CAmount fee_needed = GetMinimumFeeRate(coin_control, pool, estimator, feeCalc).GetFee(nTxBytes); + // But always obey the maximum + if (fee_needed > maxTxFee) { + fee_needed = maxTxFee; + if (feeCalc) feeCalc->reason = FeeReason::MAXTXFEE; + } + return fee_needed; +} + +CFeeRate CWallet::GetMinimumFeeRate(const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc) +{ /* User control of how to calculate fee uses the following parameter precedence: 1. coin_control.m_feerate 2. coin_control.m_confirm_target @@ -3075,15 +3032,15 @@ CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_c 4. nTxConfirmTarget (user-set global variable) The first parameter that is set is used. */ - CAmount fee_needed; + CFeeRate feerate_needed ; if (coin_control.m_feerate) { // 1. - fee_needed = coin_control.m_feerate->GetFee(nTxBytes); + feerate_needed = *(coin_control.m_feerate); if (feeCalc) feeCalc->reason = FeeReason::PAYTXFEE; // Allow to override automatic min/max check over coin control instance - if (coin_control.fOverrideFeeRate) return fee_needed; + if (coin_control.fOverrideFeeRate) return feerate_needed; } else if (!coin_control.m_confirm_target && ::payTxFee != CFeeRate(0)) { // 3. TODO: remove magic value of 0 for global payTxFee - fee_needed = ::payTxFee.GetFee(nTxBytes); + feerate_needed = ::payTxFee; if (feeCalc) feeCalc->reason = FeeReason::PAYTXFEE; } else { // 2. or 4. @@ -3095,48 +3052,27 @@ CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_c if (coin_control.m_fee_mode == FeeEstimateMode::CONSERVATIVE) conservative_estimate = true; else if (coin_control.m_fee_mode == FeeEstimateMode::ECONOMICAL) conservative_estimate = false; - fee_needed = estimator.estimateSmartFee(target, feeCalc, conservative_estimate).GetFee(nTxBytes); - if (fee_needed == 0) { + feerate_needed = estimator.estimateSmartFee(target, feeCalc, conservative_estimate); + if (feerate_needed == CFeeRate(0)) { // if we don't have enough data for estimateSmartFee, then use fallbackFee - fee_needed = fallbackFee.GetFee(nTxBytes); + feerate_needed = fallbackFee; if (feeCalc) feeCalc->reason = FeeReason::FALLBACK; } // Obey mempool min fee when using smart fee estimation - CAmount min_mempool_fee = pool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFee(nTxBytes); - if (fee_needed < min_mempool_fee) { - fee_needed = min_mempool_fee; + CFeeRate min_mempool_feerate = pool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000); + if (feerate_needed < min_mempool_feerate) { + feerate_needed = min_mempool_feerate; if (feeCalc) feeCalc->reason = FeeReason::MEMPOOL_MIN; } } // prevent user from paying a fee below minRelayTxFee or minTxFee - CAmount required_fee = GetRequiredFee(nTxBytes); - if (required_fee > fee_needed) { - fee_needed = required_fee; + CFeeRate required_feerate = GetRequiredFeeRate(); + if (required_feerate > feerate_needed) { + feerate_needed = required_feerate; if (feeCalc) feeCalc->reason = FeeReason::REQUIRED; } - // But always obey the maximum - if (fee_needed > maxTxFee) { - fee_needed = maxTxFee; - if (feeCalc) feeCalc->reason = FeeReason::MAXTXFEE; - } - return fee_needed; -} - -CFeeRate CWallet::GetMinimumFeeRate(unsigned int nConfirmTarget, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc, bool ignoreUserSetFee) -{ - CFeeRate nFeeRateNeeded = payTxFee; - // payTxFee is the user-set global for desired feerate - // User didn't set: use -txconfirmtarget to estimate... - if (nFeeRateNeeded == CFeeRate(0) || ignoreUserSetFee) { - nFeeRateNeeded = estimator.estimateSmartFee(nConfirmTarget, feeCalc, pool); - // ... unless we don't have enough mempool data for estimatefee, then use fallbackFee - if (nFeeRateNeeded == CFeeRate(0)) - nFeeRateNeeded = fallbackFee; - } - // prevent user from paying a fee below minRelayTxFee or minTxFee - nFeeRateNeeded = std::max(nFeeRateNeeded, GetRequiredFeeRate()); - return nFeeRateNeeded; + return feerate_needed; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index f45e13f511e..1705a2ada30 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -676,7 +676,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface * if they are not ours */ // TODO: Change the hard coded change_size later when we aren't just using P2PKH change outputs - bool SelectCoins(const std::vector& vAvailableCoins, const CAmount& nTargetValue, std::set& setCoinsRet, CAmount& nValueRet, CAmount& fee_ret, const CFeeRate effective_fee, const CCoinControl *coinControl = NULL, bool use_bnb = true, int change_size = 148+34) const; + bool SelectCoins(const std::vector& vAvailableCoins, const CAmount& nTargetValue, std::set& setCoinsRet, CAmount& nValueRet, CAmount& fee_ret, const CFeeRate effective_fee, const CCoinControl& coin_control, bool use_bnb = true, int change_size = 148+34) const; CWalletDB *pwalletdbEncryption; @@ -983,7 +983,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface * Estimate the minimum fee rate considering user set parameters * and the required fee */ - static CFeeRate GetMinimumFeeRate(unsigned int nConfirmTarget, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc, bool ignoreUserSetFee = false); + static CFeeRate GetMinimumFeeRate(const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc); /** * Return the minimum required fee taking into account the