From 6ef99826b9ab2c9d95ed4b55403d22225bcf086a Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 15 Mar 2018 02:42:18 -0400 Subject: [PATCH 1/2] Actually disable BnB when there are preset inputs We don't want to use BnB when there are preset inputs because there is some weirdness with making that work with using the KnapsackSolver as the fallback. Currently we say that we haven't used bnb when there are preset inputs, but we don't actually disable BnB. This fixes that. --- src/wallet/wallet.cpp | 3 ++- src/wallet/wallet.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index bd5094085..db63a5a85 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2493,7 +2493,7 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil } } -bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAmount& nTargetValue, std::set& setCoinsRet, CAmount& nValueRet, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const +bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAmount& nTargetValue, std::set& setCoinsRet, CAmount& nValueRet, const CCoinControl& coin_control, CoinSelectionParams& coin_selection_params, bool& bnb_used) const { std::vector vCoins(vAvailableCoins); @@ -2523,6 +2523,7 @@ bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAm { // For now, don't use BnB if preset inputs are selected. TODO: Enable this later bnb_used = false; + coin_selection_params.use_bnb = false; std::map::const_iterator it = mapWallet.find(outpoint.hash); if (it != mapWallet.end()) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index dc0e8d07d..a88da71f9 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -674,7 +674,7 @@ private: * if they are not ours */ bool SelectCoins(const std::vector& vAvailableCoins, const CAmount& nTargetValue, std::set& setCoinsRet, CAmount& nValueRet, - const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const; + const CCoinControl& coin_control, CoinSelectionParams& coin_selection_params, bool& bnb_used) const; CWalletDB *pwalletdbEncryption; From 081bf54ee4a282eed72e7de409ad6b9ab97f2987 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 15 Mar 2018 11:25:50 -0400 Subject: [PATCH 2/2] Test that BnB is not used when there are preset inputs --- src/wallet/test/coinselector_tests.cpp | 16 +++++++++++++++- src/wallet/wallet.h | 17 ++++++++--------- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index f05c81cd4..6c36e2e96 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -4,6 +4,7 @@ #include "wallet/wallet.h" #include "wallet/coinselection.h" +#include "wallet/coincontrol.h" #include "amount.h" #include "primitives/transaction.h" #include "random.h" @@ -27,7 +28,7 @@ std::vector> wtxn; typedef std::set CoinSet; static std::vector vCoins; -static const CWallet testWallet("dummy", CWalletDBWrapper::CreateDummy()); +static CWallet testWallet("dummy", CWalletDBWrapper::CreateDummy()); static CAmount balance = 0; CoinEligibilityFilter filter_standard(1, 6, 0); @@ -72,6 +73,7 @@ static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = fa } COutput output(wtx.get(), nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */); vCoins.push_back(output); + testWallet.AddToWallet(*wtx.get()); wtxn.emplace_back(std::move(wtx)); } @@ -222,6 +224,18 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) add_coin(1); vCoins.at(0).nInputBytes = 40; // Make sure that it has a negative effective value. The next check should assert if this somehow got through. Otherwise it will fail BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used)); + + // Make sure that we aren't using BnB when there are preset inputs + empty_wallet(); + add_coin(5 * CENT); + add_coin(3 * CENT); + add_coin(2 * CENT); + CCoinControl coin_control; + coin_control.fAllowOtherInputs = true; + coin_control.Select(COutPoint(vCoins.at(0).tx->GetHash(), vCoins.at(0).i)); + BOOST_CHECK(testWallet.SelectCoins(vCoins, 10 * CENT, setCoinsRet, nValueRet, coin_control, coin_selection_params_bnb, bnb_used)); + BOOST_CHECK(!bnb_used); + BOOST_CHECK(!coin_selection_params_bnb.use_bnb); } BOOST_AUTO_TEST_CASE(knapsack_solver_test) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index a88da71f9..bb72eb168 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -667,15 +667,6 @@ private: std::mutex mutexScanning; friend class WalletRescanReserver; - - /** - * Select a set of coins such that nValueRet >= nTargetValue and at least - * 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& coin_control, CoinSelectionParams& coin_selection_params, bool& bnb_used) const; - CWalletDB *pwalletdbEncryption; //! the current wallet version: clients below this version are not able to load the wallet @@ -768,6 +759,14 @@ public: return *dbw; } + /** + * Select a set of coins such that nValueRet >= nTargetValue and at least + * 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& coin_control, CoinSelectionParams& coin_selection_params, bool& bnb_used) const; + /** Get a name for this wallet for logging/debugging purposes. */ const std::string& GetName() const { return m_name; }