Merge #10637: Coin Selection with Murch's algorithm

73b5bf2 Add a test to make sure that negative effective values are filtered (Andrew Chow)
76d2f06 Benchmark BnB in the worst case where it exhausts (Andrew Chow)
6a34ff5 Have SelectCoinsMinConf and SelectCoins use BnB or Knapsack and use it (Andrew Chow)
fab0488 Add a GetMinimumFeeRate function which is wrapped by GetMinimumFee (Andrew Chow)
cd927ff Move original knapsack solver tests to coinselector_tests.cpp (Andrew Chow)
fb716f7 Move current coin selection algorithm to coinselection.{cpp,h} (Andrew Chow)
4566ab7 Add tests for the Branch and Bound algorithm (Andrew Chow)
4b2716d Remove coinselection.h -> wallet.h circular dependency (Andrew Chow)
7d77eb1 Use a struct for output eligibility (Andrew Chow)
ce7435c Move output eligibility to a separate function (Andrew Chow)
0185939 Implement Branch and Bound coin selection in a new file (Andrew Chow)
f84fed8 Store effective value, fee, and long term fee in CInputCoin (Andrew Chow)
12ec29d Calculate and store the number of bytes required to spend an input (Andrew Chow)

Pull request description:

  This is an implementation of the [Branch and Bound coin selection algorithm written by Murch](http://murch.one/wp-content/uploads/2016/11/erhardt2016coinselection.pdf) (@xekyo). I have it set so this algorithm will run first and if it fails, it will fall back to the current coin selection algorithm. The coin selection algorithms and tests have been refactored to separate files instead of having them all in wallet.cpp.

  I have added some tests for the new algorithm and a test for all of coin selection in general. However, more tests may be needed, but I will need help with coming up with more test cases.

  This PR uses some code borrowed from #10360 to use effective values when selecting coins.

Tree-SHA512: b0500f406bf671e74984fae78e2d0fbc5e321ddf4f06182c5855e9d1984c4ef2764c7586d03e16fa4b578c340b21710324926f9ca472d5447a0d1ed43eb4357e

Author: laanwj

src/Makefile.am

@@ -172,6 +172,7 @@ BITCOIN_CORE_H = \
   wallet/wallet.h \
   wallet/walletdb.h \
   wallet/walletutil.h \
+  wallet/coinselection.h \
   warnings.h \
   zmq/zmqabstractnotifier.h \
   zmq/zmqconfig.h\
@@ -253,6 +254,7 @@ libbitcoin_wallet_a_SOURCES = \
   wallet/wallet.cpp \
   wallet/walletdb.cpp \
   wallet/walletutil.cpp \
+  wallet/coinselection.cpp \
   $(BITCOIN_CORE_H)
 
 # crypto primitives library

src/Makefile.test.include

@@ -94,7 +94,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)

src/bench/coin_selection.cpp

@@ -4,6 +4,7 @@
 
 #include <bench/bench.h>
 #include <wallet/wallet.h>
+#include <wallet/coinselection.h>
 
 #include <set>
 
@@ -44,7 +45,11 @@ static void CoinSelection(benchmark::State& state)
 
         std::set<CInputCoin> setCoinsRet;
         CAmount nValueRet;
-        bool success = wallet.SelectCoinsMinConf(1003 * COIN, 1, 6, 0, vCoins, setCoinsRet, nValueRet);
+        bool bnb_used;
+        CoinEligibilityFilter filter_standard(1, 6, 0);
+        CoinSelectionParams coin_selection_params(false, 34, 148, CFeeRate(0), 0);
+        bool success = wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)
+                       || wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used);
         assert(success);
         assert(nValueRet == 1003 * COIN);
         assert(setCoinsRet.size() == 2);
@@ -57,4 +62,47 @@ static void CoinSelection(benchmark::State& state)
     }
 }
 
+typedef std::set<CInputCoin> CoinSet;
+
+// Copied from src/wallet/test/coinselector_tests.cpp
+static void add_coin(const CAmount& nValue, int nInput, std::vector<CInputCoin>& set)
+{
+    CMutableTransaction tx;
+    tx.vout.resize(nInput + 1);
+    tx.vout[nInput].nValue = nValue;
+    set.emplace_back(MakeTransactionRef(tx), nInput);
+}
+// Copied from src/wallet/test/coinselector_tests.cpp
+static CAmount make_hard_case(int utxos, std::vector<CInputCoin>& 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;
+}
+
+static void BnBExhaustion(benchmark::State& state)
+{
+    // Setup
+    std::vector<CInputCoin> utxo_pool;
+    CoinSet selection;
+    CAmount value_ret = 0;
+    CAmount not_input_fees = 0;
+
+    while (state.KeepRunning()) {
+        // Benchmark
+        CAmount target = make_hard_case(17, utxo_pool);
+        SelectCoinsBnB(utxo_pool, target, 0, selection, value_ret, not_input_fees); // Should exhaust
+
+        // Cleanup
+        utxo_pool.clear();
+        selection.clear();
+    }
+}
+
 BENCHMARK(CoinSelection, 650);
+BENCHMARK(BnBExhaustion, 650);

src/consensus/validation.h

@@ -101,5 +101,10 @@ static inline int64_t GetBlockWeight(const CBlock& block)
 {
     return ::GetSerializeSize(block, SER_NETWORK, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * (WITNESS_SCALE_FACTOR - 1) + ::GetSerializeSize(block, SER_NETWORK, PROTOCOL_VERSION);
 }
+static inline int64_t GetTransationInputWeight(const CTxIn& txin)
+{
+    // scriptWitness size is added here because witnesses and txins are split up in segwit serialization.
+    return ::GetSerializeSize(txin, SER_NETWORK, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * (WITNESS_SCALE_FACTOR - 1) + ::GetSerializeSize(txin, SER_NETWORK, PROTOCOL_VERSION) + ::GetSerializeSize(txin.scriptWitness.stack, SER_NETWORK, PROTOCOL_VERSION);
+}
 
 #endif // BITCOIN_CONSENSUS_VALIDATION_H

src/policy/policy.cpp

@@ -258,3 +258,8 @@ int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t nSigOpCost)
 {
     return GetVirtualTransactionSize(GetTransactionWeight(tx), nSigOpCost);
 }
+
+int64_t GetVirtualTransactionInputSize(const CTxIn& txin, int64_t nSigOpCost)
+{
+    return GetVirtualTransactionSize(GetTransationInputWeight(txin), nSigOpCost);
+}

src/policy/policy.h

@@ -102,5 +102,6 @@ extern unsigned int nBytesPerSigOp;
 /** Compute the virtual transaction size (weight reinterpreted as bytes). */
 int64_t GetVirtualTransactionSize(int64_t nWeight, int64_t nSigOpCost);
 int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t nSigOpCost = 0);
+int64_t GetVirtualTransactionInputSize(const CTxIn& tx, int64_t nSigOpCost = 0);
 
 #endif // BITCOIN_POLICY_POLICY_H

src/script/sign.cpp

@@ -194,11 +194,16 @@ SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nI
     return data;
 }
 
+void UpdateInput(CTxIn& input, const SignatureData& data)
+{
+    input.scriptSig = data.scriptSig;
+    input.scriptWitness = data.scriptWitness;
+}
+
 void UpdateTransaction(CMutableTransaction& tx, unsigned int nIn, const SignatureData& data)
 {
     assert(tx.vin.size() > nIn);
-    tx.vin[nIn].scriptSig = data.scriptSig;
-    tx.vin[nIn].scriptWitness = data.scriptWitness;
+    UpdateInput(tx.vin[nIn], data);
 }
 
 bool SignSignature(const CKeyStore &keystore, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType)

src/script/sign.h

@@ -80,6 +80,7 @@ SignatureData CombineSignatures(const CScript& scriptPubKey, const BaseSignature
 /** Extract signature data from a transaction, and insert it. */
 SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nIn);
 void UpdateTransaction(CMutableTransaction& tx, unsigned int nIn, const SignatureData& data);
+void UpdateInput(CTxIn& input, const SignatureData& data);
 
 /* Check whether we know how to sign for an output like this, assuming we
  * have all private keys. While this function does not need private keys, the passed