From ae9768c8b796f3c4333c0a34c7a8736b84412d4d Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Mon, 26 Oct 2015 14:55:17 +0100 Subject: [PATCH 1/7] fix locking issue with new mempool limiting Current master crashes on OSX with an exception: "boost: mutex lock failed in pthread_mutex_lock: Invalid argument" (cherry picked from commit 0d699fc821048ab9316b0004e6552c8f1dc5e5f4) Zcash: Also adds the `clear` call that this was fixing. Upstream added it in https://github.com/bitcoin/bitcoin/pull/6722 which we never backported (instead implementing our own mempool limiting logic). --- src/txmempool.cpp | 11 +++++++++-- src/txmempool.h | 1 + 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 112a065eb..7208e7b11 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -59,6 +59,8 @@ CTxMemPoolEntry::GetPriority(unsigned int currentHeight) const CTxMemPool::CTxMemPool(const CFeeRate& _minRelayFee) : nTransactionsUpdated(0) { + _clear(); //lock free clear + // Sanity checks off by default for performance, because otherwise // accepting transactions becomes O(N^2) where N is the number // of transactions in the pool @@ -494,9 +496,8 @@ void CTxMemPool::removeWithoutBranchId(uint32_t nMemPoolBranchId) } } -void CTxMemPool::clear() +void CTxMemPool::_clear() { - LOCK(cs); mapTx.clear(); mapNextTx.clear(); totalTxSize = 0; @@ -504,6 +505,12 @@ void CTxMemPool::clear() ++nTransactionsUpdated; } +void CTxMemPool::clear() +{ + LOCK(cs); + _clear(); +} + void CTxMemPool::check(const CCoinsViewCache *pcoins) const { if (nCheckFrequency == 0) diff --git a/src/txmempool.h b/src/txmempool.h index 530ce44a7..b5e10ccd7 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -210,6 +210,7 @@ public: std::list& conflicts, bool fCurrentEstimate = true); void removeWithoutBranchId(uint32_t nMemPoolBranchId); void clear(); + void _clear(); //lock free void queryHashes(std::vector& vtxid); void pruneSpent(const uint256& hash, CCoins &coins); unsigned int GetTransactionsUpdated() const; From 13d8f294ace6df857339a3ee517624b2ba9e2e7c Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 8 Apr 2015 11:20:00 -0700 Subject: [PATCH 2/7] Replace trickle nodes with per-node/message Poisson delays We used to have a trickle node, a node which was chosen in each iteration of the send loop that was privileged and allowed to send out queued up non-time critical messages. Since the removal of the fixed sleeps in the network code, this resulted in fast and attackable treatment of such broadcasts. This pull request changes the 3 remaining trickle use cases by random delays: * Local address broadcast (while also removing the the wiping of the seen filter) * Address relay * Inv relay (for transactions; blocks are always relayed immediately) The code is based on older commits by Patrick Strateman. (cherry picked from commit 5400ef6bcb9d243b2b21697775aa6491115420f3) --- src/main.cpp | 34 ++++++++++++++-------------------- src/main.h | 11 +++++++++-- src/net.cpp | 16 ++++++++++------ src/net.h | 8 +++++++- src/test/DoS_tests.cpp | 14 +++++++------- 5 files changed, 47 insertions(+), 36 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 910089ed5..bfa028c85 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -7253,7 +7253,7 @@ bool ProcessMessages(const CChainParams& chainparams, CNode* pfrom) } -bool SendMessages(const Consensus::Params& params, CNode* pto, bool fSendTrickle) +bool SendMessages(const Consensus::Params& params, CNode* pto) { { // Don't send anything until we get its version message @@ -7294,28 +7294,17 @@ bool SendMessages(const Consensus::Params& params, CNode* pto, bool fSendTrickle return true; // Address refresh broadcast - static int64_t nLastRebroadcast; - if (!IsInitialBlockDownload(params) && (GetTime() - nLastRebroadcast > 24 * 60 * 60)) - { - LOCK(cs_vNodes); - for (CNode* pnode : vNodes) - { - // Periodically clear addrKnown to allow refresh broadcasts - if (nLastRebroadcast) - pnode->addrKnown.reset(); - - // Rebroadcast our address - AdvertizeLocal(pnode); - } - if (!vNodes.empty()) - nLastRebroadcast = GetTime(); + int64_t nNow = GetTimeMicros(); + if (!IsInitialBlockDownload(params) && pto->nNextLocalAddrSend < nNow) { + AdvertizeLocal(pto); + pto->nNextLocalAddrSend = PoissonNextSend(nNow, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL); } // // Message: addr // - if (fSendTrickle) - { + if (pto->nNextAddrSend < nNow) { + pto->nNextAddrSend = PoissonNextSend(nNow, AVG_ADDRESS_BROADCAST_INTERVAL); vector vAddr; vAddr.reserve(pto->vAddrToSend.size()); for (const CAddress& addr : pto->vAddrToSend) @@ -7395,8 +7384,13 @@ bool SendMessages(const Consensus::Params& params, CNode* pto, bool fSendTrickle vector vInv; vector vInvWait; { + bool fSendTrickle = pto->fWhitelisted; + if (pto->nNextInvSend < nNow) { + fSendTrickle = true; + pto->nNextInvSend = PoissonNextSend(nNow, AVG_INVENTORY_BROADCAST_INTERVAL); + } LOCK(pto->cs_inventory); - vInv.reserve(pto->vInventoryToSend.size()); + vInv.reserve(std::min(1000, pto->vInventoryToSend.size())); vInvWait.reserve(pto->vInventoryToSend.size()); for (const CInv& inv : pto->vInventoryToSend) { @@ -7436,7 +7430,7 @@ bool SendMessages(const Consensus::Params& params, CNode* pto, bool fSendTrickle pto->PushMessage("inv", vInv); // Detect whether we're stalling - int64_t nNow = GetTimeMicros(); + nNow = GetTimeMicros(); if (!pto->fDisconnect && state.nStallingSince && state.nStallingSince < nNow - 1000000 * BLOCK_STALLING_TIMEOUT) { // Stalling only triggers when the block download window cannot move. During normal steady state, // the download window should be much larger than the to-be-downloaded set of blocks, so disconnection diff --git a/src/main.h b/src/main.h index e45cf1ecd..c33513e2f 100644 --- a/src/main.h +++ b/src/main.h @@ -114,6 +114,14 @@ static const unsigned int WITNESS_WRITE_INTERVAL = 10 * 60; static const unsigned int WITNESS_WRITE_UPDATES = 10000; /** Maximum length of reject messages. */ static const unsigned int MAX_REJECT_MESSAGE_LENGTH = 111; +/** Average delay between local address broadcasts in seconds. */ +static const unsigned int AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL = 24 * 24 * 60; +/** Average delay between peer address broadcasts in seconds. */ +static const unsigned int AVG_ADDRESS_BROADCAST_INTERVAL = 30; +/** Average delay between trickled inventory broadcasts in seconds. + * Blocks, whitelisted receivers, and a random 25% of transactions bypass this. */ +static const unsigned int AVG_INVENTORY_BROADCAST_INTERVAL = 5; + static const unsigned int DEFAULT_LIMITFREERELAY = 15; static const bool DEFAULT_RELAYPRIORITY = false; static const int64_t DEFAULT_MAX_TIP_AGE = 24 * 60 * 60; @@ -256,9 +264,8 @@ bool ProcessMessages(const CChainParams& chainparams, CNode* pfrom); * * @param[in] params Active chain parameters. * @param[in] pto The node which we are sending messages to. - * @param[in] fSendTrickle When true send the trickled data, otherwise trickle the data until true. */ -bool SendMessages(const Consensus::Params& params, CNode* pto, bool fSendTrickle); +bool SendMessages(const Consensus::Params& params, CNode* pto); /** Run an instance of the script checking thread */ void ThreadScriptCheck(); /** Check whether we are doing an initial block download (synchronizing from disk or network) */ diff --git a/src/net.cpp b/src/net.cpp index 08fb6f9fe..358f68707 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -30,6 +30,8 @@ #include +#include + #include // Dump addresses to peers.dat and banlist.dat every 15 minutes (900s) @@ -1692,11 +1694,6 @@ void ThreadMessageHandler() } } - // Poll the connected nodes for messages - CNode* pnodeTrickle = NULL; - if (!vNodesCopy.empty()) - pnodeTrickle = vNodesCopy[GetRand(vNodesCopy.size())]; - bool fSleep = true; for (CNode* pnode : vNodesCopy) @@ -1729,7 +1726,7 @@ void ThreadMessageHandler() { TRY_LOCK(pnode->cs_vSend, lockSend); if (lockSend) - g_signals.SendMessages(chainparams.GetConsensus(), pnode, pnode == pnodeTrickle || pnode->fWhitelisted); + g_signals.SendMessages(chainparams.GetConsensus(), pnode); } boost::this_thread::interruption_point(); } @@ -2258,6 +2255,9 @@ CNode::CNode(SOCKET hSocketIn, const CAddress& addrIn, const std::string& addrNa nStartingHeight = -1; filterInventoryKnown.reset(); fGetAddr = false; + nNextLocalAddrSend = 0; + nNextAddrSend = 0; + nNextInvSend = 0; fRelayTxes = false; fSentAddr = false; pfilter = new CBloomFilter(); @@ -2418,3 +2418,7 @@ void CNode::EndMessage() UNLOCK_FUNCTION(cs_vSend) return CSipHasher(k0, k1).Write(&vchNetGroup[0], vchNetGroup.size()).Finalize(); } + +int64_t PoissonNextSend(int64_t nNow, int average_interval_seconds) { + return nNow + (int64_t)(log1p(GetRand(1ULL << 48) * -0.0000000000000035527136788 /* -1/2^48 */) * average_interval_seconds * -1000000.0 + 0.5); +} diff --git a/src/net.h b/src/net.h index 0dd4d9bc4..04195f38a 100644 --- a/src/net.h +++ b/src/net.h @@ -117,7 +117,7 @@ struct CNodeSignals { boost::signals2::signal GetHeight; boost::signals2::signal ProcessMessages; - boost::signals2::signal SendMessages; + boost::signals2::signal SendMessages; boost::signals2::signal InitializeNode; boost::signals2::signal FinalizeNode; }; @@ -336,6 +336,8 @@ public: CRollingBloomFilter addrKnown; bool fGetAddr; std::set setKnown; + int64_t nNextAddrSend; + int64_t nNextLocalAddrSend; // inventory based relay CRollingBloomFilter filterInventoryKnown; @@ -343,6 +345,7 @@ public: CCriticalSection cs_inventory; std::set setAskFor; std::multimap mapAskFor; + int64_t nNextInvSend; // Ping time measurement: // The pong reply we're expecting, or 0 if no pong expected. @@ -723,4 +726,7 @@ void RelayTransaction(const CTransaction& tx); void RelayTransaction(const CTransaction& tx, const CDataStream& ss); +/** Return a timestamp in the future (in microseconds) for exponentially distributed events. */ +int64_t PoissonNextSend(int64_t nNow, int average_interval_seconds); + #endif // BITCOIN_NET_H diff --git a/src/test/DoS_tests.cpp b/src/test/DoS_tests.cpp index 89b5220e6..b006c2160 100644 --- a/src/test/DoS_tests.cpp +++ b/src/test/DoS_tests.cpp @@ -54,7 +54,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning) CNode dummyNode1(INVALID_SOCKET, addr1, "", true); dummyNode1.nVersion = 1; Misbehaving(dummyNode1.GetId(), 100); // Should get banned - SendMessages(params, &dummyNode1, false); + SendMessages(params, &dummyNode1); BOOST_CHECK(CNode::IsBanned(addr1)); BOOST_CHECK(!CNode::IsBanned(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned @@ -62,11 +62,11 @@ BOOST_AUTO_TEST_CASE(DoS_banning) CNode dummyNode2(INVALID_SOCKET, addr2, "", true); dummyNode2.nVersion = 1; Misbehaving(dummyNode2.GetId(), 50); - SendMessages(params, &dummyNode2, false); + SendMessages(params, &dummyNode2); BOOST_CHECK(!CNode::IsBanned(addr2)); // 2 not banned yet... BOOST_CHECK(CNode::IsBanned(addr1)); // ... but 1 still should be Misbehaving(dummyNode2.GetId(), 50); - SendMessages(params, &dummyNode2, false); + SendMessages(params, &dummyNode2); BOOST_CHECK(CNode::IsBanned(addr2)); } @@ -79,13 +79,13 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) CNode dummyNode1(INVALID_SOCKET, addr1, "", true); dummyNode1.nVersion = 1; Misbehaving(dummyNode1.GetId(), 100); - SendMessages(params, &dummyNode1, false); + SendMessages(params, &dummyNode1); BOOST_CHECK(!CNode::IsBanned(addr1)); Misbehaving(dummyNode1.GetId(), 10); - SendMessages(params, &dummyNode1, false); + SendMessages(params, &dummyNode1); BOOST_CHECK(!CNode::IsBanned(addr1)); Misbehaving(dummyNode1.GetId(), 1); - SendMessages(params, &dummyNode1, false); + SendMessages(params, &dummyNode1); BOOST_CHECK(CNode::IsBanned(addr1)); mapArgs.erase("-banscore"); } @@ -102,7 +102,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) dummyNode.nVersion = 1; Misbehaving(dummyNode.GetId(), 100); - SendMessages(params, &dummyNode, false); + SendMessages(params, &dummyNode); BOOST_CHECK(CNode::IsBanned(addr)); SetMockTime(nStartTime+60*60); From 44787c0179825d75f43f88482211e6682032fffd Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Mon, 11 Apr 2016 12:52:29 -0400 Subject: [PATCH 3/7] Use txid as key in mapAlreadyAskedFor Previously we used the CInv that would be sent to the peer announcing the transaction as the key, but using the txid instead allows us to decouple the p2p layer from the application logic (which relies on this map to avoid duplicate tx requests). (cherry picked from commit 7e91f632c70ff1848a152f24ee67a06796803943) --- src/main.cpp | 2 +- src/net.cpp | 6 +++--- src/net.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index bfa028c85..51489d991 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -6629,7 +6629,7 @@ bool static ProcessMessage(const CChainParams& chainparams, CNode* pfrom, string CValidationState state; pfrom->setAskFor.erase(inv.hash); - mapAlreadyAskedFor.erase(inv); + mapAlreadyAskedFor.erase(inv.hash); if (!AlreadyHave(inv) && AcceptToMemoryPool(chainparams, mempool, state, tx, true, &fMissingInputs)) { diff --git a/src/net.cpp b/src/net.cpp index 358f68707..3e8f5167b 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -76,7 +76,7 @@ CCriticalSection cs_vNodes; map mapRelay; deque > vRelayExpiration; CCriticalSection cs_mapRelay; -limitedmap mapAlreadyAskedFor(MAX_INV_SZ); +limitedmap mapAlreadyAskedFor(MAX_INV_SZ); static deque vOneShots; static CCriticalSection cs_vOneShots; @@ -2318,7 +2318,7 @@ void CNode::AskFor(const CInv& inv) // We're using mapAskFor as a priority queue, // the key is the earliest time the request can be sent int64_t nRequestTime; - limitedmap::const_iterator it = mapAlreadyAskedFor.find(inv); + limitedmap::const_iterator it = mapAlreadyAskedFor.find(inv.hash); if (it != mapAlreadyAskedFor.end()) nRequestTime = it->second; else @@ -2337,7 +2337,7 @@ void CNode::AskFor(const CInv& inv) if (it != mapAlreadyAskedFor.end()) mapAlreadyAskedFor.update(it, nRequestTime); else - mapAlreadyAskedFor.insert(std::make_pair(inv, nRequestTime)); + mapAlreadyAskedFor.insert(std::make_pair(inv.hash, nRequestTime)); mapAskFor.insert(std::make_pair(nRequestTime, inv)); } diff --git a/src/net.h b/src/net.h index 04195f38a..26ee6f61b 100644 --- a/src/net.h +++ b/src/net.h @@ -167,7 +167,7 @@ extern CCriticalSection cs_vNodes; extern std::map mapRelay; extern std::deque > vRelayExpiration; extern CCriticalSection cs_mapRelay; -extern limitedmap mapAlreadyAskedFor; +extern limitedmap mapAlreadyAskedFor; extern std::vector vAddedNodes; extern CCriticalSection cs_vAddedNodes; From ad8abd68a104db0c5d593276a8d7c46994cd09b3 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 14 Apr 2016 16:04:50 +0200 Subject: [PATCH 4/7] Change mapRelay to store CTransactions (cherry picked from commit 38c310299cfef419d42744362b90c1700b598953) --- src/main.cpp | 7 ++----- src/net.cpp | 17 ++++------------- src/net.h | 5 ++--- 3 files changed, 8 insertions(+), 21 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 51489d991..012d23edf 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -6106,7 +6106,7 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam // Send stream from relay memory { LOCK(cs_mapRelay); - map::iterator mi = mapRelay.find(inv); + map::iterator mi = mapRelay.find(inv.hash); if (mi != mapRelay.end()) { pfrom->PushMessage(inv.GetCommand(), (*mi).second); pushed = true; @@ -6114,10 +6114,7 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam } if (!pushed && inv.type == MSG_TX) { if (isInMempool) { - CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); - ss.reserve(1000); - ss << tx; - pfrom->PushMessage("tx", ss); + pfrom->PushMessage("tx", tx); pushed = true; } } diff --git a/src/net.cpp b/src/net.cpp index 3e8f5167b..3e36ded54 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -73,8 +73,8 @@ std::string strSubVersion; vector vNodes; CCriticalSection cs_vNodes; -map mapRelay; -deque > vRelayExpiration; +map mapRelay; +deque > vRelayExpiration; CCriticalSection cs_mapRelay; limitedmap mapAlreadyAskedFor(MAX_INV_SZ); @@ -2033,14 +2033,6 @@ instance_of_cnetcleanup; void RelayTransaction(const CTransaction& tx) -{ - CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); - ss.reserve(10000); - ss << tx; - RelayTransaction(tx, ss); -} - -void RelayTransaction(const CTransaction& tx, const CDataStream& ss) { CInv inv(MSG_TX, tx.GetHash()); { @@ -2052,9 +2044,8 @@ void RelayTransaction(const CTransaction& tx, const CDataStream& ss) vRelayExpiration.pop_front(); } - // Save original serialized message so newer versions are preserved - mapRelay.insert(std::make_pair(inv, ss)); - vRelayExpiration.push_back(std::make_pair(GetTime() + 15 * 60, inv)); + mapRelay.insert(std::make_pair(inv.hash, tx)); + vRelayExpiration.push_back(std::make_pair(GetTime() + 15 * 60, inv.hash)); } LOCK(cs_vNodes); for (CNode* pnode : vNodes) diff --git a/src/net.h b/src/net.h index 26ee6f61b..499fd7a6d 100644 --- a/src/net.h +++ b/src/net.h @@ -164,8 +164,8 @@ extern int nMaxConnections; extern std::vector vNodes; extern CCriticalSection cs_vNodes; -extern std::map mapRelay; -extern std::deque > vRelayExpiration; +extern std::map mapRelay; +extern std::deque > vRelayExpiration; extern CCriticalSection cs_mapRelay; extern limitedmap mapAlreadyAskedFor; @@ -723,7 +723,6 @@ public: class CTransaction; void RelayTransaction(const CTransaction& tx); -void RelayTransaction(const CTransaction& tx, const CDataStream& ss); /** Return a timestamp in the future (in microseconds) for exponentially distributed events. */ From 465c2492e70567fe799dc160ed18faa4b1569457 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Thu, 5 Aug 2021 14:19:51 +0100 Subject: [PATCH 5/7] More precise terminology: "lock free" -> "unlocked" --- src/txmempool.cpp | 2 +- src/txmempool.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 7208e7b11..a90585911 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -59,7 +59,7 @@ CTxMemPoolEntry::GetPriority(unsigned int currentHeight) const CTxMemPool::CTxMemPool(const CFeeRate& _minRelayFee) : nTransactionsUpdated(0) { - _clear(); //lock free clear + _clear(); // unlocked clear // Sanity checks off by default for performance, because otherwise // accepting transactions becomes O(N^2) where N is the number diff --git a/src/txmempool.h b/src/txmempool.h index b5e10ccd7..8c3d396b1 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -210,7 +210,7 @@ public: std::list& conflicts, bool fCurrentEstimate = true); void removeWithoutBranchId(uint32_t nMemPoolBranchId); void clear(); - void _clear(); //lock free + void _clear(); // unlocked void queryHashes(std::vector& vtxid); void pruneSpent(const uint256& hash, CCoins &coins); unsigned int GetTransactionsUpdated() const; From 6be1a2a9dadeff706a78b0d25bcf970f74532ad1 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 5 Aug 2021 20:54:47 +0100 Subject: [PATCH 6/7] test: Wait for transaction propagation in shorter_block_times RPC test This was previously a transient test failure, that started reliably failing after the move to Poisson delays. --- qa/rpc-tests/shorter_block_times.py | 1 + 1 file changed, 1 insertion(+) diff --git a/qa/rpc-tests/shorter_block_times.py b/qa/rpc-tests/shorter_block_times.py index d3ff5e079..41c25a6df 100755 --- a/qa/rpc-tests/shorter_block_times.py +++ b/qa/rpc-tests/shorter_block_times.py @@ -63,6 +63,7 @@ class ShorterBlockTimes(BitcoinTestFramework): myopid = self.nodes[0].z_sendmany(node0_taddr, recipients, 1, 0) txid = wait_and_assert_operationid_status(self.nodes[0], myopid) assert_equal(147, self.nodes[0].getrawtransaction(txid, 1)['expiryheight']) # height + 1 + 40 + self.sync_all() # Ensure the transaction has propagated to node 1 self.nodes[1].generate(1) self.sync_all() assert_equal(20, Decimal(self.nodes[0].z_gettotalbalance()['private'])) From 75b9fc4f2703a76975340f292fa80e76802e4999 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Mon, 9 Aug 2021 17:43:48 +0100 Subject: [PATCH 7/7] test: Fix race condition in p2p_txexpiringsoon The recent changes to mempool inv logic mean that nodes are much less likely to immediately return an `inv` message in response to a `mempool` message. The `p2p_txexpiringsoon` RPC test was relying on the prior behaviour. `TestNode.sync_with_ping` now takes an optional `waiting_for` closure that allows the caller to require that a specific message kind is received prior to the timeout. --- qa/rpc-tests/p2p_txexpiringsoon.py | 2 +- qa/rpc-tests/tx_expiry_helper.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/qa/rpc-tests/p2p_txexpiringsoon.py b/qa/rpc-tests/p2p_txexpiringsoon.py index 6472b6fba..4cc4a38c7 100755 --- a/qa/rpc-tests/p2p_txexpiringsoon.py +++ b/qa/rpc-tests/p2p_txexpiringsoon.py @@ -51,7 +51,7 @@ class TxExpiringSoonTest(BitcoinTestFramework): testnode.send_message(msg_mempool()) # Sync up with node after p2p messages delivered - testnode.sync_with_ping() + testnode.sync_with_ping(waiting_for=lambda x: x.last_inv) with mininode_lock: msg = testnode.last_inv diff --git a/qa/rpc-tests/tx_expiry_helper.py b/qa/rpc-tests/tx_expiry_helper.py index 94a6cc2c5..5b6594add 100755 --- a/qa/rpc-tests/tx_expiry_helper.py +++ b/qa/rpc-tests/tx_expiry_helper.py @@ -66,12 +66,13 @@ class TestNode(NodeConnCB): # The following function is mostly copied from p2p-acceptblock.py # Sync up with the node after delivery of a message - def sync_with_ping(self, timeout=30): + def sync_with_ping(self, timeout=30, waiting_for=None): self.connection.send_message(msg_ping(nonce=self.ping_counter)) sleep_time = 0.05 while timeout > 0: with mininode_lock: - if self.last_pong.nonce == self.ping_counter: + ready = True if waiting_for is None else waiting_for(self) is not None + if ready and self.last_pong.nonce == self.ping_counter: self.ping_counter += 1 return time.sleep(sleep_time)