[wallet] Optional '-avoidreuse' flag which defaults to not reusing addresses in sends

doc/man/bitcoin-qt.1

@@ -341,6 +341,12 @@ Delete all wallet transactions and only recover those parts of the
 blockchain through \fB\-rescan\fR on startup (1 = keep tx meta data e.g.
 account owner and payment request information, 2 = drop tx meta
 data)
+.HP
+\fB\-avoidreuse\fR
+.IP
+Mark addresses which have been used to fund transactions in the past,
+and avoid reusing these in future funding, except when explicitly
+requested (default: 0)
 .PP
 ZeroMQ notification options:
 .HP

doc/man/bitcoind.1

@@ -346,6 +346,12 @@ Delete all wallet transactions and only recover those parts of the
 blockchain through \fB\-rescan\fR on startup (1 = keep tx meta data e.g.
 account owner and payment request information, 2 = drop tx meta
 data)
+.HP
+\fB\-avoidreuse\fR
+.IP
+Mark addresses which have been used to fund transactions in the past,
+and avoid reusing these in future funding, except when explicitly
+requested (default: 0)
 .PP
 ZeroMQ notification options:
 .HP

src/qt/bitcoinstrings.cpp

@@ -51,6 +51,9 @@ QT_TRANSLATE_NOOP("bitcoin-core", ""
 "Delete all wallet transactions and only recover those parts of the "
 "blockchain through -rescan on startup"),
 QT_TRANSLATE_NOOP("bitcoin-core", ""
+"Mark addresses which have been used to fund transactions in the past, "
+"and avoid reusing these in future funding, except when explicitly requested"),
+QT_TRANSLATE_NOOP("bitcoin-core", ""
 "Discover own IP addresses (default: 1 when listening and no -externalip or -"
 "proxy)"),
 QT_TRANSLATE_NOOP("bitcoin-core", ""

src/rpc/client.cpp

@@ -39,6 +39,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "sendtoaddress", 4, "subtractfeefromamount" },
     { "sendtoaddress", 5 , "replaceable" },
     { "sendtoaddress", 6 , "conf_target" },
+    { "sendtoaddress", 7, "allowdirty" },
     { "settxfee", 0, "amount" },
     { "getreceivedbyaddress", 1, "minconf" },
     { "getreceivedbyaccount", 1, "minconf" },

src/wallet/coincontrol.h

@@ -21,6 +21,8 @@ class CCoinControl
     bool fAllowWatchOnly;
     //! Override estimated feerate
     bool fOverrideFeeRate;
+    //! Allows inclusion of dirty (previously used) addresses
+    bool fAllowDirtyAddresses;
     //! Feerate to use if overrideFeeRate is true
     CFeeRate nFeeRate;
     //! Override the default confirmation target, 0 = use default
@@ -40,6 +42,7 @@ class CCoinControl
         destChange = CNoDestination();
         fAllowOtherInputs = false;
         fAllowWatchOnly = false;
+        fAllowDirtyAddresses = false;
         setSelected.clear();
         nFeeRate = CFeeRate(0);
         fOverrideFeeRate = false;

src/wallet/rpcwallet.cpp

@@ -401,9 +401,10 @@ UniValue sendtoaddress(const JSONRPCRequest& request)
         return NullUniValue;
     }
 
-    if (request.fHelp || request.params.size() < 2 || request.params.size() > 8)
+    if (request.fHelp || request.params.size() < 2 || request.params.size() > 9)
         throw std::runtime_error(
-            "sendtoaddress \"address\" amount ( \"comment\" \"comment_to\" subtractfeefromamount replaceable conf_target \"estimate_mode\")\n"
+            std::string() +
+            "sendtoaddress \"address\" amount ( \"comment\" \"comment_to\" subtractfeefromamount replaceable conf_target \"estimate_mode\" allowdirty )\n"
             "\nSend an amount to a given address.\n"
             + HelpRequiringPassphrase(pwallet) +
             "\nArguments:\n"
@@ -422,12 +423,15 @@ UniValue sendtoaddress(const JSONRPCRequest& request)
             "       \"UNSET\"\n"
             "       \"ECONOMICAL\"\n"
             "       \"CONSERVATIVE\"\n"
+            "9. allowdirty             (boolean, optional, " + (GetBoolArg("-avoidreuse", DEFAULT_AVOIDREUSE) ? std::string("default=") + (DEFAULT_AVOIDREUSE ? "true" : "false") : "unavailable") + ") Allows spending from dirty addresses; addresses are considered\n"
+            "                             dirty if they have previously been used in a transaction.\n"
             "\nResult:\n"
             "\"txid\"                  (string) The transaction id.\n"
             "\nExamples:\n"
             + HelpExampleCli("sendtoaddress", "\"1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\" 0.1")
             + HelpExampleCli("sendtoaddress", "\"1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\" 0.1 \"donation\" \"seans outpost\"")
             + HelpExampleCli("sendtoaddress", "\"1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\" 0.1 \"\" \"\" true")
+            + HelpExampleCli("sendtoaddress", "\"1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\" 0.1 \"\" \"\" false true")
             + HelpExampleRpc("sendtoaddress", "\"1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\", 0.1, \"donation\", \"seans outpost\"")
         );
 
@@ -469,6 +473,7 @@ UniValue sendtoaddress(const JSONRPCRequest& request)
         }
     }
 
+    coin_control.fAllowDirtyAddresses = !request.params[8].isNull() && request.params[8].get_bool();
 
     EnsureWalletIsUnlocked(pwallet);
 
@@ -3094,7 +3099,7 @@ static const CRPCCommand commands[] =
     { "wallet",             "move",                     &movecmd,                  false,  {"fromaccount","toaccount","amount","minconf","comment"} },
     { "wallet",             "sendfrom",                 &sendfrom,                 false,  {"fromaccount","toaddress","amount","minconf","comment","comment_to"} },
     { "wallet",             "sendmany",                 &sendmany,                 false,  {"fromaccount","amounts","minconf","comment","subtractfeefrom","replaceable","conf_target","estimate_mode"} },
-    { "wallet",             "sendtoaddress",            &sendtoaddress,            false,  {"address","amount","comment","comment_to","subtractfeefromamount","replaceable","conf_target","estimate_mode"} },
+    { "wallet",             "sendtoaddress",            &sendtoaddress,            false,  {"address","amount","comment","comment_to","subtractfeefromamount","replaceable","conf_target","estimate_mode","allowdirty"} },
     { "wallet",             "setaccount",               &setaccount,               true,   {"address","account"} },
     { "wallet",             "settxfee",                 &settxfee,                 true,   {"amount"} },
     { "wallet",             "signmessage",              &signmessage,              true,   {"address","message"} },

src/wallet/wallet.cpp

@@ -851,6 +851,37 @@ bool CWallet::MarkReplaced(const uint256& originalHash, const uint256& newHash)
     return success;
 }
 
+void CWallet::SetDirtyState(const uint256& hash, unsigned int n, bool dirty)
+{
+    const CWalletTx* srctx = GetWalletTx(hash);
+    if (srctx) {
+        CTxDestination dst;
+        if (ExtractDestination(srctx->tx->vout[n].scriptPubKey, dst)) {
+            AssertLockHeld(cs_wallet);
+            if (::IsMine(*this, dst)) {
+                if (dirty && !GetDestData(dst, "dirty", NULL)) {
+                    AddDestData(dst, "dirty", "p"); // p for "present", opposite of absent (null)
+                } else if (!dirty && GetDestData(dst, "dirty", NULL)) {
+                    EraseDestData(dst, "dirty");
+                }
+            }
+        }
+    }
+}
+
+bool CWallet::IsDirty(const uint256& hash, unsigned int n) const
+{
+    const CWalletTx* srctx = GetWalletTx(hash);
+    if (srctx) {
+        CTxDestination dst;
+        if (ExtractDestination(srctx->tx->vout[n].scriptPubKey, dst)) {
+            AssertLockHeld(cs_wallet);
+            return ::IsMine(*this, dst) && GetDestData(dst, "dirty", NULL);
+        }
+    }
+    return false;
+}
+
 bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose)
 {
     LOCK(cs_wallet);
@@ -859,6 +890,14 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose)
 
     uint256 hash = wtxIn.GetHash();
 
+    if (GetBoolArg("-avoidreuse", DEFAULT_AVOIDREUSE)) {
+        // Mark used destinations as dirty
+        for (const CTxIn& txin : wtxIn.tx->vin) {
+            const COutPoint& op = txin.prevout;
+            SetDirtyState(op.hash, op.n, true);
+        }
+    }
+
     // Inserts only if not already there, returns tx inserted or tx found
     std::pair<std::map<uint256, CWalletTx>::iterator, bool> ret = mapWallet.insert(std::make_pair(hash, wtxIn));
     CWalletTx& wtx = (*ret.first).second;
@@ -1699,6 +1738,7 @@ CAmount CWalletTx::GetImmatureCredit(bool fUseCache) const
 
 CAmount CWalletTx::GetAvailableCredit(bool fUseCache) const
 {
+    bool fAllowDirtyAddresses = !GetBoolArg("-avoidreuse", DEFAULT_AVOIDREUSE);
     if (pwallet == 0)
         return 0;
 
@@ -1713,7 +1753,7 @@ CAmount CWalletTx::GetAvailableCredit(bool fUseCache) const
     uint256 hashTx = GetHash();
     for (unsigned int i = 0; i < tx->vout.size(); i++)
     {
-        if (!pwallet->IsSpent(hashTx, i))
+        if (!pwallet->IsSpent(hashTx, i) && (fAllowDirtyAddresses || !pwallet->IsDirty(hashTx, i)))
         {
             const CTxOut &txout = tx->vout[i];
             nCredit += pwallet->GetCredit(txout, ISMINE_SPENDABLE);
@@ -2034,6 +2074,7 @@ CAmount CWallet::GetAvailableBalance(const CCoinControl* coinControl) const
 void CWallet::AvailableCoins(std::vector<COutput> &vCoins, bool fOnlySafe, const CCoinControl *coinControl, const CAmount &nMinimumAmount, const CAmount &nMaximumAmount, const CAmount &nMinimumSumAmount, const uint64_t &nMaximumCount, const int &nMinDepth, const int &nMaxDepth) const
 {
     vCoins.clear();
+    bool fAllowDirtyAddresses = !GetBoolArg("-avoidreuse", DEFAULT_AVOIDREUSE) || (coinControl && coinControl->fAllowDirtyAddresses);
 
     {
         LOCK2(cs_main, cs_wallet);
@@ -2119,6 +2160,9 @@ void CWallet::AvailableCoins(std::vector<COutput> &vCoins, bool fOnlySafe, const
                     continue;
                 }
 
+                if (!fAllowDirtyAddresses && IsDirty(wtxid, i))
+                    continue;
+
                 bool fSpendableIn = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (coinControl && coinControl->fAllowWatchOnly && (mine & ISMINE_WATCH_SOLVABLE) != ISMINE_NO);
                 bool fSolvableIn = (mine & (ISMINE_SPENDABLE | ISMINE_WATCH_SOLVABLE)) != ISMINE_NO;
 
@@ -3785,6 +3829,7 @@ std::string CWallet::GetWalletHelpString(bool showDebug)
     strUsage += HelpMessageOpt("-walletnotify=<cmd>", _("Execute command when a wallet transaction changes (%s in cmd is replaced by TxID)"));
     strUsage += HelpMessageOpt("-zapwallettxes=<mode>", _("Delete all wallet transactions and only recover those parts of the blockchain through -rescan on startup") +
                                " " + _("(1 = keep tx meta data e.g. account owner and payment request information, 2 = drop tx meta data)"));
+    strUsage += HelpMessageOpt("-avoidreuse", _("Mark addresses which have been used to fund transactions in the past, and avoid reusing these in future funding, except when explicitly requested") + " " + strprintf(_("(default: %u)"), DEFAULT_AVOIDREUSE));
 
     if (showDebug)
     {

src/wallet/wallet.h

@@ -65,6 +65,8 @@ static const bool DEFAULT_WALLETBROADCAST = true;
 static const bool DEFAULT_DISABLE_WALLET = false;
 //! if set, all keys will be derived by using BIP32
 static const bool DEFAULT_USE_HD_WALLET = true;
+//! if set, addresses will be marked dirty once spent from, and will be excluded from future coin selects
+static const bool DEFAULT_AVOIDREUSE = false;
 
 extern const char * DEFAULT_WALLET_DAT;
 
@@ -843,6 +845,8 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
     bool SelectCoinsMinConf(const CAmount& nTargetValue, int nConfMine, int nConfTheirs, uint64_t nMaxAncestors, std::vector<COutput> vCoins, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet) const;
 
     bool IsSpent(const uint256& hash, unsigned int n) const;
+    bool IsDirty(const uint256& hash, unsigned int n) const;
+    void SetDirtyState(const uint256& hash, unsigned int n, bool dirty);
 
     bool IsLockedCoin(uint256 hash, unsigned int n) const;
     void LockCoin(const COutPoint& output);

test/functional/avoidreuse.py

@@ -0,0 +1,93 @@
+#!/usr/bin/env python3
+# 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.
+"""Test the -avoidreuse config option."""
+
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_raises_jsonrpc
+from decimal import Decimal
+
+class AvoidReuseTest(BitcoinTestFramework):
+
+    def __init__(self):
+        super().__init__()
+        self.setup_clean_chain = False
+        self.num_nodes = 2
+        self.extra_args = [[], ['-avoidreuse=1']]
+
+    def reset_balance(self, node, discardaddr):
+        '''
+        Throw away all owned coins by the node so it gets a balance of 0.
+        '''
+        balance = node.getbalance()
+        if balance > 0.5:
+            tosend = '%.5f' % (balance - Decimal(0.01))
+            node.sendtoaddress(address=discardaddr, amount=tosend, allowdirty=True)
+
+    def run_test(self):
+        '''
+        Set up initial chain and run tests defined below
+        '''
+
+        self.nodes[0].generate(110)
+        self.sync_all()
+        self.reset_balance(self.nodes[1], self.nodes[0].getnewaddress())
+        self.test_fund_send_fund_send()
+        self.reset_balance(self.nodes[1], self.nodes[0].getnewaddress())
+        self.test_fund_send_fund_senddirty()
+
+    def test_fund_send_fund_send(self):
+        '''
+        Test the simple case where [1] generates a new address A, then
+        [0] sends 10 BTC to A.
+        [1] spends 5 BTC from A. (leaving roughly 4 BTC useable)
+        [0] sends 10 BTC to A again.
+        [1] tries to spend 10 BTC (fails; dirty).
+        [1] tries to spend 4 BTC (succeeds; change address sufficient)
+        '''
+
+        fundaddr = self.nodes[1].getnewaddress()
+        retaddr = self.nodes[0].getnewaddress()
+
+        self.nodes[0].sendtoaddress(fundaddr, 10)
+        self.nodes[0].generate(1)
+        self.sync_all()
+
+        self.nodes[1].sendtoaddress(retaddr, 5)
+        self.sync_all()
+        self.nodes[0].generate(1)
+
+        self.nodes[0].sendtoaddress(fundaddr, 10)
+        self.sync_all()
+        self.nodes[0].generate(1)
+
+        assert_raises_jsonrpc(-6, "Insufficient funds", self.nodes[1].sendtoaddress, retaddr, 10)
+
+        self.nodes[1].sendtoaddress(retaddr, 4)
+
+    def test_fund_send_fund_senddirty(self):
+        '''
+        Test the same as above, except send the 10 BTC with the allowdirty flag
+        set. This means the 10 BTC send should succeed, where it fails above.
+        '''
+
+        fundaddr = self.nodes[1].getnewaddress()
+        retaddr = self.nodes[0].getnewaddress()
+
+        self.nodes[0].sendtoaddress(fundaddr, 10)
+        self.nodes[0].generate(1)
+        self.sync_all()
+
+        self.nodes[1].sendtoaddress(retaddr, 5)
+        self.sync_all()
+        self.nodes[0].generate(1)
+
+        self.nodes[0].sendtoaddress(fundaddr, 10)
+        self.sync_all()
+        self.nodes[0].generate(1)
+
+        self.nodes[1].sendtoaddress(address=retaddr, amount=10, allowdirty=True)
+
+if __name__ == '__main__':
+    AvoidReuseTest().main()

test/functional/test_runner.py

@@ -115,6 +115,7 @@
     'p2p-leaktests.py',
     'wallet-encryption.py',
     'uptime.py',
+    'avoidreuse.py',
 ]
 
 EXTENDED_SCRIPTS = [