[wallet] Use destination groups instead of coins in coin select

doc/release-notes-pr12257.md

@@ -0,0 +1,9 @@
+Notable changes
+===============
+
+Coin selection
+--------------
+- A new `-avoidpartialspends` flag has been added (default=false). If enabled, the wallet will try to spend UTXO's that point at the same destination
+  together. This is a privacy increase, as there will no longer be cases where a wallet will inadvertently spend only parts of the coins sent to
+  the same address (note that if someone were to send coins to that address after it was used, those coins will still be included in future
+  coin selections).

src/Makefile.am

@@ -274,6 +274,7 @@ libbitcoin_wallet_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES)
 libbitcoin_wallet_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
 libbitcoin_wallet_a_SOURCES = \
   interfaces/wallet.cpp \
+  wallet/coincontrol.cpp \
   wallet/crypter.cpp \
   wallet/db.cpp \
   wallet/feebumper.cpp \

src/bench/coin_selection.cpp

@@ -8,7 +8,7 @@
 
 #include <set>
 
-static void addCoin(const CAmount& nValue, const CWallet& wallet, std::vector<COutput>& vCoins)
+static void addCoin(const CAmount& nValue, const CWallet& wallet, std::vector<OutputGroup>& groups)
 {
     int nInput = 0;
 
@@ -21,7 +21,7 @@ static void addCoin(const CAmount& nValue, const CWallet& wallet, std::vector<CO
 
     int nAge = 6 * 24;
     COutput output(wtx, nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */);
-    vCoins.push_back(output);
+    groups.emplace_back(output.GetInputCoin(), 0, false, 0, 0);
 }
 
 // Simple benchmark for wallet coin selection. Note that it maybe be necessary
@@ -37,37 +37,41 @@ static void CoinSelection(benchmark::State& state)
     LOCK(wallet.cs_wallet);
 
     // Add coins.
-    std::vector<COutput> vCoins;
+    std::vector<OutputGroup> groups;
     for (int i = 0; i < 1000; ++i) {
-        addCoin(1000 * COIN, wallet, vCoins);
+        addCoin(1000 * COIN, wallet, groups);
     }
-    addCoin(3 * COIN, wallet, vCoins);
+    addCoin(3 * COIN, wallet, groups);
 
     const CoinEligibilityFilter filter_standard(1, 6, 0);
     const CoinSelectionParams coin_selection_params(true, 34, 148, CFeeRate(0), 0);
     while (state.KeepRunning()) {
         std::set<CInputCoin> setCoinsRet;
         CAmount nValueRet;
         bool bnb_used;
-        bool success = wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used);
+        bool success = wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used);
         assert(success);
         assert(nValueRet == 1003 * COIN);
         assert(setCoinsRet.size() == 2);
     }
 }
 
 typedef std::set<CInputCoin> CoinSet;
+static const CWallet testWallet("dummy", WalletDatabase::CreateDummy());
+std::vector<std::unique_ptr<CWalletTx>> wtxn;
 
 // Copied from src/wallet/test/coinselector_tests.cpp
-static void add_coin(const CAmount& nValue, int nInput, std::vector<CInputCoin>& set)
+static void add_coin(const CAmount& nValue, int nInput, std::vector<OutputGroup>& set)
 {
     CMutableTransaction tx;
     tx.vout.resize(nInput + 1);
     tx.vout[nInput].nValue = nValue;
-    set.emplace_back(MakeTransactionRef(tx), nInput);
+    std::unique_ptr<CWalletTx> wtx(new CWalletTx(&testWallet, MakeTransactionRef(std::move(tx))));
+    set.emplace_back(COutput(wtx.get(), nInput, 0, true, true, true).GetInputCoin(), 0, true, 0, 0);
+    wtxn.emplace_back(std::move(wtx));
 }
 // Copied from src/wallet/test/coinselector_tests.cpp
-static CAmount make_hard_case(int utxos, std::vector<CInputCoin>& utxo_pool)
+static CAmount make_hard_case(int utxos, std::vector<OutputGroup>& utxo_pool)
 {
     utxo_pool.clear();
     CAmount target = 0;
@@ -82,7 +86,7 @@ static CAmount make_hard_case(int utxos, std::vector<CInputCoin>& utxo_pool)
 static void BnBExhaustion(benchmark::State& state)
 {
     // Setup
-    std::vector<CInputCoin> utxo_pool;
+    std::vector<OutputGroup> utxo_pool;
     CoinSet selection;
     CAmount value_ret = 0;
     CAmount not_input_fees = 0;

src/util.h

@@ -355,4 +355,18 @@ std::string CopyrightHolders(const std::string& strPrefix);
  */
 int ScheduleBatchPriority(void);
 
+namespace util {
+
+//! Simplification of std insertion
+template <typename Tdst, typename Tsrc>
+inline void insert(Tdst& dst, const Tsrc& src) {
+    dst.insert(dst.begin(), src.begin(), src.end());
+}
+template <typename TsetT, typename Tsrc>
+inline void insert(std::set<TsetT>& dst, const Tsrc& src) {
+    dst.insert(src.begin(), src.end());
+}
+
+} // namespace util
+
 #endif // BITCOIN_UTIL_H

src/wallet/coincontrol.cpp

@@ -0,0 +1,23 @@
+// Copyright (c) 2018 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/coincontrol.h>
+
+#include <util.h>
+
+void CCoinControl::SetNull()
+{
+    destChange = CNoDestination();
+    m_change_type.reset();
+    fAllowOtherInputs = false;
+    fAllowWatchOnly = false;
+    m_avoid_partial_spends = gArgs.GetBoolArg("-avoidpartialspends", DEFAULT_AVOIDPARTIALSPENDS);
+    setSelected.clear();
+    m_feerate.reset();
+    fOverrideFeeRate = false;
+    m_confirm_target.reset();
+    m_signal_bip125_rbf.reset();
+    m_fee_mode = FeeEstimateMode::UNSET;
+}
+

src/wallet/coincontrol.h

@@ -32,6 +32,8 @@ class CCoinControl
     boost::optional<unsigned int> m_confirm_target;
     //! Override the wallet's m_signal_rbf if set
     boost::optional<bool> m_signal_bip125_rbf;
+    //! Avoid partial use of funds sent to a given address
+    bool m_avoid_partial_spends;
     //! Fee estimation mode to control arguments to estimateSmartFee
     FeeEstimateMode m_fee_mode;
 
@@ -40,19 +42,7 @@ class CCoinControl
         SetNull();
     }
 
-    void SetNull()
-    {
-        destChange = CNoDestination();
-        m_change_type.reset();
-        fAllowOtherInputs = false;
-        fAllowWatchOnly = false;
-        setSelected.clear();
-        m_feerate.reset();
-        fOverrideFeeRate = false;
-        m_confirm_target.reset();
-        m_signal_bip125_rbf.reset();
-        m_fee_mode = FeeEstimateMode::UNSET;
-    }
+    void SetNull();
 
     bool HasSelected() const
     {