diff --git a/qa/pull-tester/rpc-tests.py b/qa/pull-tester/rpc-tests.py index 373477a10b2..273adac0b36 100755 --- a/qa/pull-tester/rpc-tests.py +++ b/qa/pull-tester/rpc-tests.py @@ -166,6 +166,7 @@ 'p2p-leaktests.py', 'replace-by-fee.py', 'p2p-policy.py', + 'wallet_create_tx.py', ] if ENABLE_ZMQ: testScripts.append('zmq_test.py') diff --git a/qa/rpc-tests/wallet_create_tx.py b/qa/rpc-tests/wallet_create_tx.py new file mode 100755 index 00000000000..66f43c821dd --- /dev/null +++ b/qa/rpc-tests/wallet_create_tx.py @@ -0,0 +1,32 @@ +#!/usr/bin/env python3 +# 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. + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, +) + + +class CreateTxWalletTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = False + self.num_nodes = 1 + + def run_test(self): + # Check that we have some (old) blocks and that anti-fee-sniping is disabled + assert_equal(self.nodes[0].getblockchaininfo()['blocks'], 120) + txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1) + tx = self.nodes[0].decoderawtransaction(self.nodes[0].gettransaction(txid)['hex']) + assert_equal(tx['locktime'], 0) + + # Check that anti-fee-sniping is enabled when we mine a recent block + self.nodes[0].generate(1) + txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1) + tx = self.nodes[0].decoderawtransaction(self.nodes[0].gettransaction(txid)['hex']) + assert 0 < tx['locktime'] <= 121 + + +if __name__ == '__main__': + CreateTxWalletTest().main() diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b2539c401a4..bda823415b0 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2417,34 +2417,25 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, bool ov return true; } -bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, - int& nChangePosInOut, std::string& strFailReason, const CCoinControl* coinControl, bool sign) +static bool IsCurrentForAntiFeeSniping() { - CAmount nValue = 0; - int nChangePosRequest = nChangePosInOut; - unsigned int nSubtractFeeFromAmount = 0; - for (const auto& recipient : vecSend) - { - if (nValue < 0 || recipient.nAmount < 0) - { - strFailReason = _("Transaction amounts must not be negative"); - return false; - } - nValue += recipient.nAmount; - - if (recipient.fSubtractFeeFromAmount) - nSubtractFeeFromAmount++; + if (IsInitialBlockDownload()) { + return false; } - if (vecSend.empty()) - { - strFailReason = _("Transaction must have at least one recipient"); + constexpr int64_t MAX_ANTI_FEE_SNIPING_TIP_AGE = 8 * 60 * 60; // in seconds + if (chainActive.Tip()->GetBlockTime() < (GetTime() - MAX_ANTI_FEE_SNIPING_TIP_AGE)) { return false; } + return true; +} - wtxNew.fTimeReceivedIsTxTime = true; - wtxNew.BindWallet(this); - CMutableTransaction txNew; - +/** + * Return a height-based locktime for new transactions (uses the height of the + * current chain tip unless we are not synced with the current chain + */ +static uint32_t GetLocktimeForNewTransaction() +{ + uint32_t locktime; // Discourage fee sniping. // // For a large miner the value of the transactions in the best block and @@ -2465,22 +2456,61 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt // enough, that fee sniping isn't a problem yet, but by implementing a fix // now we ensure code won't be written that makes assumptions about // nLockTime that preclude a fix later. - txNew.nLockTime = chainActive.Height(); + if (IsCurrentForAntiFeeSniping()) { + locktime = chainActive.Height(); + + // Secondly occasionally randomly pick a nLockTime even further back, so + // that transactions that are delayed after signing for whatever reason, + // e.g. high-latency mix networks and some CoinJoin implementations, have + // better privacy. + if (GetRandInt(10) == 0) { + locktime = std::max(0, (int)locktime - GetRandInt(100)); + } + } else { + // If our chain is lagging behind, we can't discourage fee sniping nor help + // the privacy of high-latency transactions. To avoid leaking a potentially + // unique "nLockTime fingerprint", set nLockTime to a constant. + locktime = 0; + } + assert(locktime <= (unsigned int)chainActive.Height()); + assert(locktime < LOCKTIME_THRESHOLD); + return locktime; +} + +bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, + int& nChangePosInOut, std::string& strFailReason, const CCoinControl* coinControl, bool sign) +{ + CAmount nValue = 0; + int nChangePosRequest = nChangePosInOut; + unsigned int nSubtractFeeFromAmount = 0; + for (const auto& recipient : vecSend) + { + if (nValue < 0 || recipient.nAmount < 0) + { + strFailReason = _("Transaction amounts must not be negative"); + return false; + } + nValue += recipient.nAmount; - // Secondly occasionally randomly pick a nLockTime even further back, so - // that transactions that are delayed after signing for whatever reason, - // e.g. high-latency mix networks and some CoinJoin implementations, have - // better privacy. - if (GetRandInt(10) == 0) - txNew.nLockTime = std::max(0, (int)txNew.nLockTime - GetRandInt(100)); + if (recipient.fSubtractFeeFromAmount) + nSubtractFeeFromAmount++; + } + if (vecSend.empty()) + { + strFailReason = _("Transaction must have at least one recipient"); + return false; + } - assert(txNew.nLockTime <= (unsigned int)chainActive.Height()); - assert(txNew.nLockTime < LOCKTIME_THRESHOLD); + wtxNew.fTimeReceivedIsTxTime = true; + wtxNew.BindWallet(this); + CMutableTransaction txNew; { set > setCoins; LOCK2(cs_main, cs_wallet); { + txNew.nLockTime = GetLocktimeForNewTransaction(); + std::vector vAvailableCoins; AvailableCoins(vAvailableCoins, true, coinControl);