diff --git a/src/miner.cpp b/src/miner.cpp index 015645c9c668..c3036fafc8c7 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -45,6 +45,7 @@ int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParam BlockAssembler::Options::Options() { blockMinFeeRate = CFeeRate(DEFAULT_BLOCK_MIN_TX_FEE); nBlockMaxWeight = DEFAULT_BLOCK_MAX_WEIGHT; + nMinTxAge = 0; } BlockAssembler::BlockAssembler(const CChainParams& params, const Options& options) : chainparams(params) @@ -52,6 +53,7 @@ BlockAssembler::BlockAssembler(const CChainParams& params, const Options& option blockMinFeeRate = options.blockMinFeeRate; // Limit weight to between 4K and MAX_BLOCK_WEIGHT-4K for sanity: nBlockMaxWeight = std::max(4000, std::min(MAX_BLOCK_WEIGHT - 4000, options.nBlockMaxWeight)); + nMinTxAge = options.nMinTxAge; } static BlockAssembler::Options DefaultOptions() @@ -66,6 +68,8 @@ static BlockAssembler::Options DefaultOptions() } else { options.blockMinFeeRate = CFeeRate(DEFAULT_BLOCK_MIN_TX_FEE); } + options.nMinTxAge = 0; + return options; } @@ -271,6 +275,7 @@ int BlockAssembler::UpdatePackagesForAdded(const CTxMemPool::setEntries& already bool BlockAssembler::SkipMapTxEntry(CTxMemPool::txiter it, indexed_modified_transaction_set &mapModifiedTx, CTxMemPool::setEntries &failedTx) { assert (it != mempool.mapTx.end()); + if (it->GetTime() > GetTime() - nMinTxAge) return true; // txn too recent return mapModifiedTx.count(it) || inBlock.count(it) || failedTx.count(it); } diff --git a/src/miner.h b/src/miner.h index 7c4c455072ed..2f32313d8de0 100644 --- a/src/miner.h +++ b/src/miner.h @@ -135,6 +135,7 @@ class BlockAssembler bool fIncludeWitness; unsigned int nBlockMaxWeight; CFeeRate blockMinFeeRate; + int64_t nMinTxAge; // Information on the current status of the block uint64_t nBlockWeight; @@ -153,6 +154,7 @@ class BlockAssembler Options(); size_t nBlockMaxWeight; CFeeRate blockMinFeeRate; + int64_t nMinTxAge; }; explicit BlockAssembler(const CChainParams& params); diff --git a/src/net.h b/src/net.h index 75c05c9cb5dd..d53a9075c9c3 100644 --- a/src/net.h +++ b/src/net.h @@ -723,6 +723,9 @@ class CNode // Used for BIP35 mempool sending bool fSendMempool GUARDED_BY(cs_inventory){false}; + // Used for scheduling rebroadcasts + std::chrono::seconds m_next_rebroadcast{0}; + // Last time a "MEMPOOL" request was serviced. std::atomic timeLastMempoolReq{0}; @@ -880,11 +883,14 @@ class CNode void MaybeSetAddrName(const std::string& addrNameIn); }; - - - - /** Return a timestamp in the future (in microseconds) for exponentially distributed events. */ int64_t PoissonNextSend(int64_t now, int average_interval_seconds); +/** Wrapper to return mockable type */ +inline std::chrono::seconds PoissonNextSend(std::chrono::seconds now, int average_interval_seconds) +{ + int64_t now_micros = (std::chrono::duration_cast(now)).count(); + return std::chrono::duration_cast(std::chrono::microseconds{PoissonNextSend(now_micros, average_interval_seconds)}); +} + #endif // BITCOIN_NET_H diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 520dfcbb66ae..f8c67595bb9f 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -110,6 +110,9 @@ static constexpr unsigned int INVENTORY_BROADCAST_MAX = 7 * INVENTORY_BROADCAST_ static constexpr unsigned int AVG_FEEFILTER_BROADCAST_INTERVAL = 10 * 60; /** Maximum feefilter broadcast delay after significant change. */ static constexpr unsigned int MAX_FEEFILTER_CHANGE_DELAY = 5 * 60; +/** Average delay between rebroadcasts in seconds. */ +static const unsigned int TX_REBROADCAST_INTERVAL = 60 * 60; + // Internal stuff namespace { @@ -1533,6 +1536,12 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm if (mi != mapRelay.end()) { connman->PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *mi->second)); push = true; + + auto num = mempool.setUnbroadcastTxIDs.erase(inv.hash); + if (num) { + LogPrint(BCLog::NET, "Removed %i from setUnbroadcastTxIDs \n", inv.hash.GetHex()); + } + } else if (pfrom->timeLastMempoolReq) { auto txinfo = mempool.info(inv.hash); // To protect privacy, do not answer getdata using the mempool when @@ -3804,6 +3813,30 @@ bool PeerLogicValidation::SendMessages(CNode* pto) } } + // Check for rebroadcasts + const auto current_time = GetTime(); + + if (pto->m_next_rebroadcast < current_time) { + bool fFirst = (pto->m_next_rebroadcast.count() == 0); + pto->m_next_rebroadcast = PoissonNextSend(current_time, TX_REBROADCAST_INTERVAL); + + if (!fFirst) { + std::set setRebroadcastTxs; + mempool.GetRebroadcastTransactions(setRebroadcastTxs); + + for (const auto& hash : setRebroadcastTxs) { + LogPrint(BCLog::NET, "Rebroadcast tx=%s peer=%d\n", hash.GetHex(), pto->GetId()); + } + + pto->setInventoryTxToSend.insert(setRebroadcastTxs.begin(), setRebroadcastTxs.end()); + + // also ensure inclusion of wallet txns that haven't been succesfully broadcast yet + // since set elements are unique, this will be a no-op if the txns are already in setInventoryTxToSend + pto->setInventoryTxToSend.insert(mempool.setUnbroadcastTxIDs.begin(), mempool.setUnbroadcastTxIDs.end()); + } + } + + // Time to send but the peer has requested we not relay transactions. if (fSendTrickle) { LOCK(pto->cs_filter); diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index 7e8291ddc8c6..5ccadec9663b 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -55,6 +55,9 @@ TransactionError BroadcastTransaction(const CTransactionRef tx, std::string& err // Transaction was accepted to the mempool. + // the mempool explicitly keeps track of wallet txns to ensure succesful initial broadcast + mempool.setUnbroadcastTxIDs.insert(hashTx); + if (wait_callback) { // For transactions broadcast from outside the wallet, make sure // that the wallet has been notified of the transaction before diff --git a/src/policy/policy.h b/src/policy/policy.h index ebe040f0ea7c..62b9709f44ae 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -20,6 +20,8 @@ class CTxOut; static const unsigned int DEFAULT_BLOCK_MAX_WEIGHT = MAX_BLOCK_WEIGHT - 4000; /** Default for -blockmintxfee, which sets the minimum feerate for a transaction in blocks created by mining code **/ static const unsigned int DEFAULT_BLOCK_MIN_TX_FEE = 1000; +/** Default rebroadcast in seconds - 30 min **/ +static const int64_t REBROADCAST_MIN_TX_AGE = 30 * 60; /** The maximum weight for transactions we're willing to relay/mine */ static const unsigned int MAX_STANDARD_TX_WEIGHT = 400000; /** The minimum non-witness size for transactions we're willing to relay/mine (1 segwit input + 1 P2WPKH output = 82 bytes) */ diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 92a0e3376962..313f8d5065c0 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -218,7 +218,9 @@ static UniValue getmininginfo(const JSONRPCRequest& request) static UniValue prioritisetransaction(const JSONRPCRequest& request) { RPCHelpMan{"prioritisetransaction", - "Accepts the transaction into mined blocks at a higher (or lower) priority\n", + "Accepts the transaction into mined blocks at a higher (or lower) priority.\n" + "\nNote that prioritizing a transaction could leak privacy, through both\n" + "block mining and likelihood of rebroadcast.", { {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id."}, {"dummy", RPCArg::Type::NUM, RPCArg::Optional::OMITTED_NAMED_ARG, "API-Compatibility for previous API. Must be zero or null.\n" diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 9257cff71874..10c4be6b4e2b 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -5,17 +5,19 @@ #include +#include #include #include #include -#include -#include +#include #include +#include #include #include -#include #include +#include #include +#include CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFee, int64_t _nTime, unsigned int _entryHeight, @@ -97,6 +99,29 @@ void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap &cachedDescendan mapTx.modify(updateIt, update_descendant_state(modifySize, modifyFee, modifyCount)); } +void CTxMemPool::GetRebroadcastTransactions(std::set& setRebroadcastTxs) +{ + // Don't rebroadcast txns during importing, reindex, or IBD to ensure we don't + // accidentally spam our peers with old transactions. + if (::ChainstateActive().IsInitialBlockDownload() || ::fImporting || ::fReindex) return; + + BlockAssembler::Options options; + options.nBlockMaxWeight = MAX_REBROADCAST_WEIGHT; + options.nMinTxAge = REBROADCAST_MIN_TX_AGE; + CScript scriptDummy = CScript() << OP_TRUE; + + // use CreateNewBlock to get set of transaction candidates + std::unique_ptr pblocktemplate = BlockAssembler(Params(), options).CreateNewBlock(scriptDummy); + + LOCK(cs); + for (const auto& tx : pblocktemplate->block.vtx) { + if (mapTx.find(tx->GetHash()) == mapTx.end()) continue; + + // add to rebroadcast set + setRebroadcastTxs.insert(tx->GetHash()); + } +} + // vHashesToUpdate is the set of transaction hashes from a disconnected block // which has been re-added to the mempool. // for each entry, look for descendants that are outside vHashesToUpdate, and diff --git a/src/txmempool.h b/src/txmempool.h index 6e5ba445d3da..24883f8b44de 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -35,6 +35,10 @@ extern CCriticalSection cs_main; /** Fake height value used in Coin to signify they are only in the memory pool (since 0.8) */ static const uint32_t MEMPOOL_HEIGHT = 0x7FFFFFFF; +// we rebroadcast 3/4 of max block weight (defined in consensus.h) +// to reduce noise due to circumstances such as miners mining priority txns +static const unsigned int MAX_REBROADCAST_WEIGHT = 3000000; + struct LockPoints { // Will be set to the blockchain height and median time past @@ -530,6 +534,10 @@ class CTxMemPool const setEntries & GetMemPoolParents(txiter entry) const EXCLUSIVE_LOCKS_REQUIRED(cs); const setEntries & GetMemPoolChildren(txiter entry) const EXCLUSIVE_LOCKS_REQUIRED(cs); uint64_t CalculateDescendantMaximum(txiter entry) const EXCLUSIVE_LOCKS_REQUIRED(cs); + + // track wallet transactions to ensur ethey are succesfully broadcast + std::set setUnbroadcastTxIDs; + private: typedef std::map cacheMap; @@ -614,6 +622,11 @@ class CTxMemPool */ void RemoveStaged(setEntries& stage, bool updateDescendants, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs); + /** Use CreateNewBlock with specific rebroadcast parameters to identify a set + * of transaction candidates & populate them in setRebroadcastTxs. + */ + void GetRebroadcastTransactions(std::set& setRebroadcastTxs); + /** When adding transactions from a disconnected block back to the mempool, * new mempool entries may have children in the mempool (which is generally * not the case when otherwise adding transactions). diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 03acf23508bc..d1d6dfc701da 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -46,6 +46,8 @@ const std::map WALLET_FLAG_CAVEATS{ }; static const size_t OUTPUT_GROUP_MAX_ENTRIES = 10; +// frequency of resubmitting txns to mempool- 24 hours in ms +static const int RESEND_TXS_FREQUENCY = 1000 * 60 * 60 * 24; static CCriticalSection cs_wallets; static std::vector> vpwallets GUARDED_BY(cs_wallets); @@ -2344,31 +2346,17 @@ bool CWalletTx::IsEquivalentTo(const CWalletTx& _tx) const return CTransaction(tx1) == CTransaction(tx2); } -// Rebroadcast transactions from the wallet. We do this on a random timer -// to slightly obfuscate which transactions come from our wallet. -// -// Ideally, we'd only resend transactions that we think should have been -// mined in the most recent block. Any transaction that wasn't in the top -// blockweight of transactions in the mempool shouldn't have been mined, -// and so is probably just sitting in the mempool waiting to be confirmed. -// Rebroadcasting does nothing to speed up confirmation and only damages -// privacy. +// Once a day, resbumit all wallet transactions to the node, +// incase it has been dropped from your mempool. void CWallet::ResendWalletTransactions() { // During reindex, importing and IBD, old wallet transactions become - // unconfirmed. Don't resend them as that would spam other nodes. + // unconfirmed. Don't need to resubmit to our node. if (!chain().isReadyToBroadcast()) return; - // Do this infrequently and randomly to avoid giving away - // that these are our transactions. - if (GetTime() < nNextResend || !fBroadcastTransactions) return; - bool fFirst = (nNextResend == 0); - nNextResend = GetTime() + GetRand(30 * 60); - if (fFirst) return; - - // Only do it if there's been a new block since last time - if (m_best_block_time < nLastResend) return; - nLastResend = GetTime(); + // Do this once per day. + if (GetTime() < nNextResend) return; + nNextResend = GetTime() + RESEND_TXS_FREQUENCY; int submitted_tx_count = 0; @@ -2376,15 +2364,11 @@ void CWallet::ResendWalletTransactions() auto locked_chain = chain().lock(); LOCK(cs_wallet); - // Relay transactions + // Resubmit transactions for (std::pair& item : mapWallet) { CWalletTx& wtx = item.second; - // Attempt to rebroadcast all txes more than 5 minutes older than - // the last block. SubmitMemoryPoolAndRelay() will not rebroadcast - // any confirmed or conflicting txs. - if (wtx.nTimeReceived > m_best_block_time - 5 * 60) continue; std::string unused_err_string; - if (wtx.SubmitMemoryPoolAndRelay(unused_err_string, true, *locked_chain)) ++submitted_tx_count; + if (wtx.SubmitMemoryPoolAndRelay(unused_err_string, false, *locked_chain)) ++submitted_tx_count; } } // locked_chain and cs_wallet diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index cf388ad827d8..7e2874d79be6 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -722,7 +722,6 @@ class CWallet final : public FillableSigningProvider, private interfaces::Chain: int nWalletMaxVersion GUARDED_BY(cs_wallet) = FEATURE_BASE; int64_t nNextResend = 0; - int64_t nLastResend = 0; bool fBroadcastTransactions = false; // Local time that the tip block was received. Used to schedule wallet rebroadcasts. std::atomic m_best_block_time {0}; diff --git a/test/functional/mempool_rebroadcast.py b/test/functional/mempool_rebroadcast.py new file mode 100755 index 000000000000..81e0a6dd4f84 --- /dev/null +++ b/test/functional/mempool_rebroadcast.py @@ -0,0 +1,271 @@ +#!/usr/bin/env python3 +# Copyright (c) 2009-2019 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 mempool rebroadcast logic. + +""" + +from collections import defaultdict +from test_framework.mininode import P2PInterface, mininode_lock +from test_framework.test_framework import BitcoinTestFramework +from test_framework.messages import ( + msg_mempool, + msg_getdata, + CInv +) +from test_framework.util import ( + assert_equal, + assert_greater_than, + assert_greater_than_or_equal, + wait_until, + disconnect_nodes, + connect_nodes, + random_transaction, + gen_return_txouts, + create_confirmed_utxos, + create_lots_of_big_transactions, +) +import time + +# Constant from net_processing +MAX_REBROADCAST_WEIGHT = 3000000 + +class P2PStoreTxInvs(P2PInterface): + def __init__(self): + super().__init__() + self.tx_invs_received = defaultdict(int) + + def on_inv(self, message): + # Store how many times invs have been received for each tx. + for i in message.inv: + if i.type == 1: + # save txid + self.tx_invs_received[i.hash] += 1 + + def get_invs(self): + with mininode_lock: + return list(self.tx_invs_received.keys()) + +class MempoolRebroadcastTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 2 + self.extra_args = [[ + "-acceptnonstdtxn=1", + "-blockmaxweight=3000000", + "-whitelist=127.0.0.1" + ]] * self.num_nodes + + def run_test(self): + self.test_simple_rebroadcast() + self.test_correct_invs() + self.test_rebroadcast_top_txns() + self.test_recency_filter() + + # helper method that uses getblocktemplate with node arg + # set to MAX_REBROADCAST_WEIGHT to find txns expected to + # be rebroadcast + def find_top_txns(self, node): + tmpl = node.getblocktemplate({'rules': ['segwit']}) + + tx_hshs = [] + for tx in tmpl['transactions']: + tx_hshs.append(tx['hash']) + + return tx_hshs + + def compare_txns_to_invs(self, txn_hshs, invs): + tx_ids = [int(txhsh, 16) for txhsh in txn_hshs] + + assert_equal(len(tx_ids), len(invs)) + assert_equal(tx_ids.sort(), invs.sort()) + + def test_simple_rebroadcast(self): + self.log.info("Test simplest rebroadcast case") + + node1 = self.nodes[0] + node2 = self.nodes[1] + + # generate mempool transactions that both nodes know about + for i in range(3): + node1.sendtoaddress(node2.getnewaddress(), 4) + + self.sync_all() + disconnect_nodes(node1, 1) + + # generate mempool transactions that only node1 knows about + for i in range(3): + node1.sendtoaddress(node2.getnewaddress(), 5) + + # check that mempools are different + assert_equal(len(node1.getrawmempool()), 6) + assert_equal(len(node2.getrawmempool()), 3) + + # bump mocktime 30 minutes to make sure the txns + # are not excluded from rebroadcast due to recency + mocktime = int(time.time()) + 31 * 60 + node1.setmocktime(mocktime) + node2.setmocktime(mocktime) + + # reconnect the bitcoin nodes + connect_nodes(node1, 1) + time.sleep(1) + mocktime += 300 * 60 # hit rebroadcast interval + node1.setmocktime(mocktime) + node2.setmocktime(mocktime) + # this sleep is needed to ensure the invs get sent + # before we bump the mocktime because of nNextInvSend + time.sleep(0.5) + + # bump by GETDATA interval + mocktime += 60 + node1.setmocktime(mocktime) + node2.setmocktime(mocktime) + + # check that node2 got txns bc rebroadcasting + wait_until(lambda: len(node2.getrawmempool()) == 6, timeout=30) + + def test_correct_invs(self): + self.log.info("Test that expected invs are rebroadcast") + + node = self.nodes[0] + node.setmocktime(0) + + # mine a block to clear out the mempool + node.generate(1) + assert_equal(len(node.getrawmempool()), 0) + + # add p2p connection + conn = node.add_p2p_connection(P2PStoreTxInvs()) + + # create txns + for i in range(3): + node.sendtoaddress(node.getnewaddress(), 2) + assert_equal(len(node.getrawmempool()), 3) + + # bump mocktime to ensure the txns won't be excluded due to recency filter + mocktime = int(time.time()) + 31 * 60 + node.setmocktime(mocktime) + + # add another p2p connection since txns aren't rebroadcast to the same peer (see filterInventoryKnown) + conn2 = node.add_p2p_connection(P2PStoreTxInvs()) + + # bump mocktime of node1 so rebroadcast is triggered + mocktime += 300 * 60 # hit rebroadcast interval + node.setmocktime(mocktime) + + # `nNextInvSend` delay on `setInventoryTxToSend + wait_until(lambda: conn2.get_invs(), timeout=30) + + # verify correct invs were sent + self.compare_txns_to_invs(node.getrawmempool(), conn2.get_invs()) + + def test_rebroadcast_top_txns(self): + self.log.info("Testing that only txns with top fee rate get rebroadcast") + + node = self.nodes[0] + node.setmocktime(0) + + # mine a block to clear out the mempool + node.generate(1) + assert_equal(len(node.getrawmempool()), 0) + + conn1 = node.add_p2p_connection(P2PStoreTxInvs()) + + # create txns + min_relay_fee = node.getnetworkinfo()["relayfee"] + txouts = gen_return_txouts() + utxo_count = 90 + utxos = create_confirmed_utxos(min_relay_fee, node, utxo_count) + base_fee = min_relay_fee*100 # our transactions are smaller than 100kb + txids = [] + + # Create 3 batches of transactions at 3 different fee rate levels + range_size = utxo_count // 3 + + for i in range(3): + txids.append([]) + start_range = i * range_size + end_range = start_range + range_size + txids[i] = create_lots_of_big_transactions(node, txouts, utxos[start_range:end_range], end_range - start_range, (i+1)*base_fee) + + # 90 transactions should be created + # confirm the invs were sent (initial broadcast) + assert_equal(len(node.getrawmempool()), 90) + wait_until(lambda: len(conn1.tx_invs_received) == 90) + + # confirm txns are more than max rebroadcast amount + assert_greater_than(node.getmempoolinfo()['bytes'], MAX_REBROADCAST_WEIGHT) + + self.sync_all() + + # age txns to ensure they won't be excluded due to recency filter + mocktime = int(time.time()) + 31 * 60 + node.setmocktime(mocktime) + + # add another p2p connection since txns aren't rebroadcast to the same peer (see filterInventoryKnown) + conn2 = node.add_p2p_connection(P2PStoreTxInvs()) + + # trigger rebroadcast to occur + mocktime += 300 * 60 # seconds + node.setmocktime(mocktime) + time.sleep(1) # ensure send message thread runs so invs get sent + + inv_count = len(conn2.get_invs()) + assert_greater_than(inv_count, 0) + + # confirm that the correct txns were rebroadcast + self.compare_txns_to_invs(self.find_top_txns(node), conn2.get_invs()) + + def test_recency_filter(self): + self.log.info("Test recent txns don't get rebroadcast") + + node = self.nodes[0] + node2 = self.nodes[1] + + node.setmocktime(0) + + # mine blocks to clear out the mempool + node.generate(10) + assert_equal(len(node.getrawmempool()), 0) + + # add p2p connection + conn = node.add_p2p_connection(P2PStoreTxInvs()) + + # create old txn + old_txn = node.sendtoaddress(node.getnewaddress(), 2) + assert_equal(len(node.getrawmempool()), 1) + wait_until(lambda: conn.get_invs(), timeout=30) + + # bump mocktime to ensure the txn is old + mocktime = int(time.time()) + 31 * 60 # seconds + node.setmocktime(mocktime) + + delta_time = 28 * 60 # seconds + while True: + # create a recent transaction + new_tx = node2.sendtoaddress(node2.getnewaddress(), 2) + new_tx_id = int(new_tx, 16) + + # ensure node1 has the transaction + wait_until(lambda: new_tx in node.getrawmempool()) + + # add another p2p connection since txns aren't rebroadcast + # to the same peer (see filterInventoryKnown) + new_conn = node.add_p2p_connection(P2PStoreTxInvs()) + + # bump mocktime to try to get rebroadcast, + # but not so much that the txn would be old + mocktime += delta_time + node.setmocktime(mocktime) + + time.sleep(1.1) + + # once we get any rebroadcasts, ensure the most recent txn is not included + if new_conn.get_invs(): + assert(new_tx_id not in new_conn.get_invs()) + break + +if __name__ == '__main__': + MempoolRebroadcastTest().main() + diff --git a/test/functional/mempool_wallet_transactions.py b/test/functional/mempool_wallet_transactions.py new file mode 100755 index 000000000000..fe6326df25ca --- /dev/null +++ b/test/functional/mempool_wallet_transactions.py @@ -0,0 +1,106 @@ +#!/usr/bin/env python3 +# Copyright (c) 2009-2019 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +""" +Ensure that wallet transactions get succesfully broadcast to at least one peer. +""" + +from collections import defaultdict +from test_framework.mininode import P2PInterface, mininode_lock +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + assert_greater_than, + wait_until, + create_lots_of_big_transactions, + create_confirmed_utxos, + gen_return_txouts, +) +import time + +class P2PStoreTxInvs(P2PInterface): + def __init__(self): + super().__init__() + self.tx_invs_received = defaultdict(int) + + def on_inv(self, message): + # Store how many times invs have been received for each tx. + for i in message.inv: + if i.type == 1: + # save txid + self.tx_invs_received[i.hash] += 1 + + def get_invs(self): + with mininode_lock: + return list(self.tx_invs_received.keys()) + +# Constant from net_processing +MAX_REBROADCAST_WEIGHT = 3000000 + +class MempoolWalletTransactionsTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 2 + self.extra_args = [["-whitelist=127.0.0.1", "-acceptnonstdtxn=1"]] * self.num_nodes + + def compare_txns_to_invs(self, txn_hshs, invs): + tx_ids = [int(txhsh, 16) for txhsh in txn_hshs] + + assert_equal(len(tx_ids), len(invs)) + assert_equal(tx_ids.sort(), invs.sort()) + + def run_test(self): + self.log.info("test that mempool will ensure initial broadcast of wallet txns") + + node = self.nodes[0] + + # generate top of mempool txns + min_relay_fee = node.getnetworkinfo()["relayfee"] + txouts = gen_return_txouts() + utxo_count = 90 + utxos = create_confirmed_utxos(min_relay_fee, node, utxo_count) + base_fee = min_relay_fee*100 # our transactions are smaller than 100kb + + txids = create_lots_of_big_transactions(node, txouts, utxos, 90, 3*base_fee) + + # check fee rate of these txns for comparison + txid = txids[0] + entry = node.getmempoolentry(txid) + high_fee_rate = entry['fee'] / entry['vsize'] + + # confirm txns are more than max rebroadcast amount + assert_greater_than(node.getmempoolinfo()['bytes'], MAX_REBROADCAST_WEIGHT) + + # generate a wallet txn that will be broadcast to nobody + us0 = create_confirmed_utxos(min_relay_fee, node, 1).pop() + inputs = [{ "txid" : us0["txid"], "vout" : us0["vout"]}] + outputs = {node.getnewaddress() : 0.0001} + tx = node.createrawtransaction(inputs, outputs) + node.settxfee(min_relay_fee) # specifically fund this tx with low fee + txF = node.fundrawtransaction(tx) + txFS = node.signrawtransactionwithwallet(txF['hex']) + wallettxid = node.sendrawtransaction(txFS['hex']) # txhsh in hex + + # ensure the wallet txn has a low fee rate & thus wont be + # rebroadcast due to top-of-mempool rule + walletentry = node.getmempoolentry(wallettxid) + low_fee_rate = walletentry['fee'] / walletentry['vsize'] + assert_greater_than(high_fee_rate, low_fee_rate) + + # add p2p connection + conn = node.add_p2p_connection(P2PStoreTxInvs()) + + # bump mocktime of node1 so rebroadcast is triggered + mocktime = int(time.time()) + 300 * 60 # hit rebroadcast interval + node.setmocktime(mocktime) + + # `nNextInvSend` delay on `setInventoryTxToSend + wait_until(lambda: conn.get_invs(), timeout=30) + + # verify the wallet txn inv was sent due to mempool tracking + wallettxinv = int(wallettxid, 16) + assert_equal(wallettxinv in conn.get_invs(), True) + +if __name__ == '__main__': + MempoolWalletTransactionsTest().main() + diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index ea8dbbaf9540..0c255c2dc932 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -108,6 +108,7 @@ 'p2p_feefilter.py', 'feature_reindex.py', 'feature_abortnode.py', + 'mempool_rebroadcast.py', # vv Tests less than 30s vv 'wallet_keypool_topup.py', 'feature_fee_estimation.py', @@ -133,6 +134,7 @@ 'interface_rpc.py', 'rpc_psbt.py', 'rpc_users.py', + 'mempool_wallet_transactions.py', 'feature_proxy.py', 'rpc_signrawtransaction.py', 'wallet_groups.py', diff --git a/test/functional/wallet_resendwallettransactions.py b/test/functional/wallet_resendwallettransactions.py index 91d26e9cb397..a85597c323e2 100755 --- a/test/functional/wallet_resendwallettransactions.py +++ b/test/functional/wallet_resendwallettransactions.py @@ -32,45 +32,45 @@ def skip_test_if_missing_module(self): self.skip_if_no_wallet() def run_test(self): - node = self.nodes[0] # alias - node.add_p2p_connection(P2PStoreTxInvs()) + node = self.nodes[0] - self.log.info("Create a new transaction and wait until it's broadcast") - txid = int(node.sendtoaddress(node.getnewaddress(), 1), 16) + self.log.info("Create a new wallet transaction") + + relayfee = node.getnetworkinfo()['relayfee'] + node.settxfee(relayfee) + txhsh = node.sendtoaddress(node.getnewaddress(), 1) + + assert txhsh in node.getrawmempool() + + # bump mocktime so the transaction should expire + # add an extra hour for good measure + two_weeks_in_seconds = 60 * 60 * 24 * 14 + mocktime = int(time.time()) + two_weeks_in_seconds + 60 * 60 + node.setmocktime(mocktime) + + # making a new transaction invokes ATMP which expires old txns + node.sendtoaddress(node.getnewaddress(), 1) + + # confirm txn is no longer in mempool + self.log.info("Confirm transaction is no longer in mempool") + assert txhsh not in node.getrawmempool() + + # bumptime so ResendWalletTransactions triggers + # we resend once / day, so bump 25 hours just to be sure + # we don't resubmit the first time, so we bump mocktime + # twice so the resend occurs the second time around + one_day_in_seconds = 60 * 60 * 25 + node.setmocktime(mocktime + one_day_in_seconds) - # Wallet rebroadcast is first scheduled 1 sec after startup (see - # nNextResend in ResendWalletTransactions()). Sleep for just over a - # second to be certain that it has been called before the first - # setmocktime call below. time.sleep(1.1) - # Can take a few seconds due to transaction trickling - wait_until(lambda: node.p2p.tx_invs_received[txid] >= 1, lock=mininode_lock) - - # Add a second peer since txs aren't rebroadcast to the same peer (see filterInventoryKnown) - node.add_p2p_connection(P2PStoreTxInvs()) - - self.log.info("Create a block") - # Create and submit a block without the transaction. - # Transactions are only rebroadcast if there has been a block at least five minutes - # after the last time we tried to broadcast. Use mocktime and give an extra minute to be sure. - block_time = int(time.time()) + 6 * 60 - node.setmocktime(block_time) - block = create_block(int(node.getbestblockhash(), 16), create_coinbase(node.getblockcount() + 1), block_time) - block.rehash() - block.solve() - node.submitblock(ToHex(block)) - - # Transaction should not be rebroadcast - node.p2ps[1].sync_with_ping() - assert_equal(node.p2ps[1].tx_invs_received[txid], 0) - - self.log.info("Transaction should be rebroadcast after 30 minutes") - # Use mocktime and give an extra 5 minutes to be sure. - rebroadcast_time = int(time.time()) + 41 * 60 - node.setmocktime(rebroadcast_time) - wait_until(lambda: node.p2ps[1].tx_invs_received[txid] >= 1, lock=mininode_lock) + node.setmocktime(mocktime + 2 * one_day_in_seconds) + + # confirm that its back in the mempool + self.log.info("Transaction should be resubmitted to mempool") + wait_until(lambda: txhsh in node.getrawmempool(), timeout=30) + if __name__ == '__main__': ResendWalletTransactionsTest().main()