diff --git a/doc/man/bitcoin-qt.1 b/doc/man/bitcoin-qt.1 index ce252612e57..69288cb1777 100644 --- a/doc/man/bitcoin-qt.1 +++ b/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 diff --git a/doc/man/bitcoind.1 b/doc/man/bitcoind.1 index fb066e0c6f3..b0bb9e02d2d 100644 --- a/doc/man/bitcoind.1 +++ b/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 diff --git a/src/qt/bitcoinstrings.cpp b/src/qt/bitcoinstrings.cpp index b3d2cf1d55c..a5aba445116 100644 --- a/src/qt/bitcoinstrings.cpp +++ b/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", "" diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 775ad4b6c9a..92c3ebd14bf 100644 --- a/src/rpc/client.cpp +++ b/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" }, diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index bdd01bec12a..cabc276f1fa 100644 --- a/src/wallet/coincontrol.h +++ b/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; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 5f72e3b6f59..56147f0d249 100644 --- a/src/wallet/rpcwallet.cpp +++ b/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"} }, diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 5e9701c71cd..77b35e599bf 100644 --- a/src/wallet/wallet.cpp +++ b/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::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 &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 &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=", _("Execute command when a wallet transaction changes (%s in cmd is replaced by TxID)")); strUsage += HelpMessageOpt("-zapwallettxes=", _("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) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index e3715cdf376..6d21b0a35d8 100644 --- a/src/wallet/wallet.h +++ b/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 vCoins, std::set& 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); diff --git a/test/functional/avoidreuse.py b/test/functional/avoidreuse.py new file mode 100755 index 00000000000..cab7c9c71d0 --- /dev/null +++ b/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() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index b7bc6e841b0..d490d71fa83 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -115,6 +115,7 @@ 'p2p-leaktests.py', 'wallet-encryption.py', 'uptime.py', + 'avoidreuse.py', ] EXTENDED_SCRIPTS = [