From 1d87137227ee125018006935d55164a72265c935 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Mon, 14 Nov 2022 11:01:48 +0000 Subject: [PATCH 01/10] [validation] Annotate ChainstateManager::m_best_header as guarded by cs_main --- src/validation.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/validation.h b/src/validation.h index aba863db09..0044004b79 100644 --- a/src/validation.h +++ b/src/validation.h @@ -997,7 +997,7 @@ public: std::set m_failed_blocks; /** Best header we've seen so far (used for getheaders queries' starting points). */ - CBlockIndex* m_best_header = nullptr; + CBlockIndex* m_best_header GUARDED_BY(::cs_main){nullptr}; //! The total number of bytes available for us to use across all in-memory //! coins caches. This will be split somehow across chainstates. From 5f80d8d1eec14253407ceed1a813619d3e5a0fe4 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Tue, 20 Sep 2022 12:35:16 +0100 Subject: [PATCH 02/10] [net processing] Annotate nUnconnectingHeaders as guarded by g_msgproc_mutex --- src/net_processing.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index d224cae58a..14eac13aa4 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -415,7 +415,7 @@ struct CNodeState { //! The best header we have sent our peer. const CBlockIndex* pindexBestHeaderSent{nullptr}; //! Length of current-streak of unconnecting headers announcements - int nUnconnectingHeaders{0}; + int nUnconnectingHeaders GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0}; //! Whether we've started headers synchronization with this peer. bool fSyncStarted{false}; //! When to potentially disconnect peer for stalling headers download @@ -666,7 +666,8 @@ private: /** Potentially fetch blocks from this peer upon receipt of a new headers tip */ void HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, const CBlockIndex& last_header); /** Update peer state based on received headers message */ - void UpdatePeerStateForReceivedHeaders(CNode& pfrom, const CBlockIndex& last_header, bool received_new_header, bool may_have_more_headers); + void UpdatePeerStateForReceivedHeaders(CNode& pfrom, const CBlockIndex& last_header, bool received_new_header, bool may_have_more_headers) + EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); void SendBlockTransactions(CNode& pfrom, Peer& peer, const CBlock& block, const BlockTransactionsRequest& req); From d8c0d1c345546c6066274edbf3bf4fb5d6f75419 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Tue, 20 Sep 2022 14:30:29 +0100 Subject: [PATCH 03/10] [net processing] Move nUnconnectingHeaders from CNodeState to Peer --- src/net_processing.cpp | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 14eac13aa4..ddb735f00a 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -385,6 +385,9 @@ struct Peer { /** Whether we've sent our peer a sendheaders message. **/ std::atomic m_sent_sendheaders{false}; + /** Length of current-streak of unconnecting headers announcements */ + int nUnconnectingHeaders GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0}; + explicit Peer(NodeId id, ServiceFlags our_services) : m_id{id} , m_our_services{our_services} @@ -414,8 +417,6 @@ struct CNodeState { const CBlockIndex* pindexLastCommonBlock{nullptr}; //! The best header we have sent our peer. const CBlockIndex* pindexBestHeaderSent{nullptr}; - //! Length of current-streak of unconnecting headers announcements - int nUnconnectingHeaders GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0}; //! Whether we've started headers synchronization with this peer. bool fSyncStarted{false}; //! When to potentially disconnect peer for stalling headers download @@ -666,7 +667,7 @@ private: /** Potentially fetch blocks from this peer upon receipt of a new headers tip */ void HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, const CBlockIndex& last_header); /** Update peer state based on received headers message */ - void UpdatePeerStateForReceivedHeaders(CNode& pfrom, const CBlockIndex& last_header, bool received_new_header, bool may_have_more_headers) + void UpdatePeerStateForReceivedHeaders(CNode& pfrom, Peer& peer, const CBlockIndex& last_header, bool received_new_header, bool may_have_more_headers) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); void SendBlockTransactions(CNode& pfrom, Peer& peer, const CBlock& block, const BlockTransactionsRequest& req); @@ -2440,27 +2441,26 @@ arith_uint256 PeerManagerImpl::GetAntiDoSWorkThreshold() void PeerManagerImpl::HandleFewUnconnectingHeaders(CNode& pfrom, Peer& peer, const std::vector& headers) { - LOCK(cs_main); - CNodeState *nodestate = State(pfrom.GetId()); - - nodestate->nUnconnectingHeaders++; + peer.nUnconnectingHeaders++; // Try to fill in the missing headers. - if (MaybeSendGetHeaders(pfrom, GetLocator(m_chainman.m_best_header), peer)) { + const CBlockIndex* best_header{WITH_LOCK(cs_main, return m_chainman.m_best_header)}; + if (MaybeSendGetHeaders(pfrom, GetLocator(best_header), peer)) { LogPrint(BCLog::NET, "received header %s: missing prev block %s, sending getheaders (%d) to end (peer=%d, nUnconnectingHeaders=%d)\n", headers[0].GetHash().ToString(), headers[0].hashPrevBlock.ToString(), - m_chainman.m_best_header->nHeight, - pfrom.GetId(), nodestate->nUnconnectingHeaders); + best_header->nHeight, + pfrom.GetId(), peer.nUnconnectingHeaders); } + // Set hashLastUnknownBlock for this peer, so that if we // eventually get the headers - even from a different peer - // we can use this peer to download. - UpdateBlockAvailability(pfrom.GetId(), headers.back().GetHash()); + WITH_LOCK(cs_main, UpdateBlockAvailability(pfrom.GetId(), headers.back().GetHash())); // The peer may just be broken, so periodically assign DoS points if this // condition persists. - if (nodestate->nUnconnectingHeaders % MAX_UNCONNECTING_HEADERS == 0) { - Misbehaving(peer, 20, strprintf("%d non-connecting headers", nodestate->nUnconnectingHeaders)); + if (peer.nUnconnectingHeaders % MAX_UNCONNECTING_HEADERS == 0) { + Misbehaving(peer, 20, strprintf("%d non-connecting headers", peer.nUnconnectingHeaders)); } } @@ -2708,15 +2708,16 @@ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, c * whether that header was new and whether the headers message was full, * update the state we keep for the peer. */ -void PeerManagerImpl::UpdatePeerStateForReceivedHeaders(CNode& pfrom, +void PeerManagerImpl::UpdatePeerStateForReceivedHeaders(CNode& pfrom, Peer& peer, const CBlockIndex& last_header, bool received_new_header, bool may_have_more_headers) { + if (peer.nUnconnectingHeaders > 0) { + LogPrint(BCLog::NET, "peer=%d: resetting nUnconnectingHeaders (%d -> 0)\n", pfrom.GetId(), peer.nUnconnectingHeaders); + } + peer.nUnconnectingHeaders = 0; + LOCK(cs_main); CNodeState *nodestate = State(pfrom.GetId()); - if (nodestate->nUnconnectingHeaders > 0) { - LogPrint(BCLog::NET, "peer=%d: resetting nUnconnectingHeaders (%d -> 0)\n", pfrom.GetId(), nodestate->nUnconnectingHeaders); - } - nodestate->nUnconnectingHeaders = 0; UpdateBlockAvailability(pfrom.GetId(), last_header.GetBlockHash()); @@ -2901,7 +2902,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, } } - UpdatePeerStateForReceivedHeaders(pfrom, *pindexLast, received_new_header, nCount == MAX_HEADERS_RESULTS); + UpdatePeerStateForReceivedHeaders(pfrom, peer, *pindexLast, received_new_header, nCount == MAX_HEADERS_RESULTS); // Consider immediately downloading blocks. HeadersDirectFetchBlocks(pfrom, peer, *pindexLast); From 689b747fc34a5670a24bb5c4b1986dd8670b55ef Mon Sep 17 00:00:00 2001 From: dergoegge Date: Tue, 20 Sep 2022 12:37:48 +0100 Subject: [PATCH 04/10] [net processing] Annotate m_headers_sync_timeout as guarded by g_msgproc_mutex --- src/net_processing.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index ddb735f00a..946c9e62d5 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -420,7 +420,7 @@ struct CNodeState { //! Whether we've started headers synchronization with this peer. bool fSyncStarted{false}; //! When to potentially disconnect peer for stalling headers download - std::chrono::microseconds m_headers_sync_timeout{0us}; + std::chrono::microseconds m_headers_sync_timeout GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0us}; //! Since when we're stalling block download progress (in microseconds), or 0. std::chrono::microseconds m_stalling_since{0us}; std::list vBlocksInFlight; From 4b84e502f5ba69d650191ce8f2e20b86c1d427ed Mon Sep 17 00:00:00 2001 From: dergoegge Date: Tue, 20 Sep 2022 14:32:48 +0100 Subject: [PATCH 05/10] [net processing] Move m_headers_sync_timeout from CNodeState to Peer --- src/net_processing.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 946c9e62d5..7c2802ec63 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -388,6 +388,9 @@ struct Peer { /** Length of current-streak of unconnecting headers announcements */ int nUnconnectingHeaders GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0}; + /** When to potentially disconnect peer for stalling headers download */ + std::chrono::microseconds m_headers_sync_timeout GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0us}; + explicit Peer(NodeId id, ServiceFlags our_services) : m_id{id} , m_our_services{our_services} @@ -419,8 +422,6 @@ struct CNodeState { const CBlockIndex* pindexBestHeaderSent{nullptr}; //! Whether we've started headers synchronization with this peer. bool fSyncStarted{false}; - //! When to potentially disconnect peer for stalling headers download - std::chrono::microseconds m_headers_sync_timeout GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0us}; //! Since when we're stalling block download progress (in microseconds), or 0. std::chrono::microseconds m_stalling_since{0us}; std::list vBlocksInFlight; @@ -5428,7 +5429,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) LogPrint(BCLog::NET, "initial getheaders (%d) to peer=%d (startheight:%d)\n", pindexStart->nHeight, pto->GetId(), peer->m_starting_height); state.fSyncStarted = true; - state.m_headers_sync_timeout = current_time + HEADERS_DOWNLOAD_TIMEOUT_BASE + + peer->m_headers_sync_timeout = current_time + HEADERS_DOWNLOAD_TIMEOUT_BASE + ( // Convert HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER to microseconds before scaling // to maintain precision @@ -5755,10 +5756,10 @@ bool PeerManagerImpl::SendMessages(CNode* pto) } } // Check for headers sync timeouts - if (state.fSyncStarted && state.m_headers_sync_timeout < std::chrono::microseconds::max()) { + if (state.fSyncStarted && peer->m_headers_sync_timeout < std::chrono::microseconds::max()) { // Detect whether this is a stalling initial-headers-sync peer if (m_chainman.m_best_header->Time() <= GetAdjustedTime() - 24h) { - if (current_time > state.m_headers_sync_timeout && nSyncStarted == 1 && (m_num_preferred_download_peers - state.fPreferredDownload >= 1)) { + if (current_time > peer->m_headers_sync_timeout && nSyncStarted == 1 && (m_num_preferred_download_peers - state.fPreferredDownload >= 1)) { // Disconnect a peer (without NetPermissionFlags::NoBan permission) if it is our only sync peer, // and we have others we could be using instead. // Note: If all our peers are inbound, then we won't @@ -5777,13 +5778,13 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // this peer (eventually). state.fSyncStarted = false; nSyncStarted--; - state.m_headers_sync_timeout = 0us; + peer->m_headers_sync_timeout = 0us; } } } else { // After we've caught up once, reset the timeout so we can't trigger // disconnect later. - state.m_headers_sync_timeout = std::chrono::microseconds::max(); + peer->m_headers_sync_timeout = std::chrono::microseconds::max(); } } From 3605011e79d0d43ff532a2c3cc9cad4cc48eb06c Mon Sep 17 00:00:00 2001 From: dergoegge Date: Tue, 20 Sep 2022 12:46:39 +0100 Subject: [PATCH 06/10] [net processing] Annotate fPreferHeaders as guarded by g_msgproc_mutex --- src/net_processing.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 7c2802ec63..d9d4d3f5ea 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -431,7 +431,7 @@ struct CNodeState { //! Whether we consider this a preferred download peer. bool fPreferredDownload{false}; //! Whether this peer wants invs or headers (when possible) for block announcements. - bool fPreferHeaders{false}; + bool fPreferHeaders GUARDED_BY(NetEventsInterface::g_msgproc_mutex){false}; /** Whether this peer wants invs or cmpctblocks (when possible) for block announcements. */ bool m_requested_hb_cmpctblocks{false}; /** Whether this peer will send us cmpctblocks if we request them. */ From 8a2cb1f749ade31bb0655567cd0062db8a7164e5 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Tue, 20 Sep 2022 14:37:27 +0100 Subject: [PATCH 07/10] [net processing] Move fPreferHeaders from CNodeState to Peer --- src/net_processing.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index d9d4d3f5ea..f6fa9e25c3 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -391,6 +391,9 @@ struct Peer { /** When to potentially disconnect peer for stalling headers download */ std::chrono::microseconds m_headers_sync_timeout GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0us}; + /** Whether this peer wants invs or headers (when possible) for block announcements */ + bool fPreferHeaders GUARDED_BY(NetEventsInterface::g_msgproc_mutex){false}; + explicit Peer(NodeId id, ServiceFlags our_services) : m_id{id} , m_our_services{our_services} @@ -430,8 +433,6 @@ struct CNodeState { int nBlocksInFlight{0}; //! Whether we consider this a preferred download peer. bool fPreferredDownload{false}; - //! Whether this peer wants invs or headers (when possible) for block announcements. - bool fPreferHeaders GUARDED_BY(NetEventsInterface::g_msgproc_mutex){false}; /** Whether this peer wants invs or cmpctblocks (when possible) for block announcements. */ bool m_requested_hb_cmpctblocks{false}; /** Whether this peer will send us cmpctblocks if we request them. */ @@ -3447,8 +3448,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } if (msg_type == NetMsgType::SENDHEADERS) { - LOCK(cs_main); - State(pfrom.GetId())->fPreferHeaders = true; + peer->fPreferHeaders = true; return; } @@ -5454,7 +5454,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // add all to the inv queue. LOCK(peer->m_block_inv_mutex); std::vector vHeaders; - bool fRevertToInv = ((!state.fPreferHeaders && + bool fRevertToInv = ((!peer->fPreferHeaders && (!state.m_requested_hb_cmpctblocks || peer->m_blocks_for_headers_relay.size() > 1)) || peer->m_blocks_for_headers_relay.size() > MAX_BLOCKS_TO_ANNOUNCE); const CBlockIndex *pBestIndex = nullptr; // last header queued for delivery @@ -5531,7 +5531,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock)); } state.pindexBestHeaderSent = pBestIndex; - } else if (state.fPreferHeaders) { + } else if (peer->fPreferHeaders) { if (vHeaders.size() > 1) { LogPrint(BCLog::NET, "%s: %u headers, range (%s, %s), to peer=%d\n", __func__, vHeaders.size(), From 938a8e2566bf2f819adeb75a08c0c8ba9a85fe1c Mon Sep 17 00:00:00 2001 From: dergoegge Date: Tue, 20 Sep 2022 12:51:30 +0100 Subject: [PATCH 08/10] [net processing] Annotate m_recently_announced_invs as guarded by g_msgproc_mutex --- src/net_processing.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index f6fa9e25c3..9353f95086 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -482,7 +482,7 @@ struct CNodeState { const bool m_is_inbound; //! A rolling bloom filter of all announced tx CInvs to this peer. - CRollingBloomFilter m_recently_announced_invs = CRollingBloomFilter{INVENTORY_MAX_RECENT_RELAY, 0.000001}; + CRollingBloomFilter m_recently_announced_invs GUARDED_BY(NetEventsInterface::g_msgproc_mutex){INVENTORY_MAX_RECENT_RELAY, 0.000001}; CNodeState(bool is_inbound) : m_is_inbound(is_inbound) {} }; @@ -904,10 +904,12 @@ private: std::atomic m_last_tip_update{0s}; /** Determine whether or not a peer can request a transaction, and return it (or nullptr if not found or not allowed). */ - CTransactionRef FindTxForGetData(const CNode& peer, const GenTxid& gtxid, const std::chrono::seconds mempool_req, const std::chrono::seconds now) LOCKS_EXCLUDED(cs_main); + CTransactionRef FindTxForGetData(const CNode& peer, const GenTxid& gtxid, const std::chrono::seconds mempool_req, const std::chrono::seconds now) + LOCKS_EXCLUDED(cs_main) EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex); void ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic& interruptMsgProc) - EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex, peer.m_getdata_requests_mutex) LOCKS_EXCLUDED(::cs_main); + EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex, peer.m_getdata_requests_mutex, NetEventsInterface::g_msgproc_mutex) + LOCKS_EXCLUDED(::cs_main); /** Process a new block. Perform any post-processing housekeeping */ void ProcessBlock(CNode& node, const std::shared_ptr& block, bool force_processing, bool min_pow_checked); From 279c53d7e4e920d3d09da74689761a2961b1a636 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Tue, 20 Sep 2022 14:45:29 +0100 Subject: [PATCH 09/10] [net processing] Move m_recently_announced_invs from CNodeState to Peer --- src/net_processing.cpp | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 9353f95086..42f69ec3ae 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -278,6 +278,9 @@ struct Peer { /** A bloom filter for which transactions to announce to the peer. See BIP37. */ std::unique_ptr m_bloom_filter PT_GUARDED_BY(m_bloom_filter_mutex) GUARDED_BY(m_bloom_filter_mutex){nullptr}; + /** A rolling bloom filter of all announced tx CInvs to this peer */ + CRollingBloomFilter m_recently_announced_invs GUARDED_BY(NetEventsInterface::g_msgproc_mutex){INVENTORY_MAX_RECENT_RELAY, 0.000001}; + mutable RecursiveMutex m_tx_inventory_mutex; /** A filter of all the txids and wtxids that the peer has announced to * us or we have announced to the peer. We use this to avoid announcing @@ -314,6 +317,10 @@ struct Peer { { return WITH_LOCK(m_tx_relay_mutex, return m_tx_relay.get()); }; + const TxRelay* GetTxRelay() const EXCLUSIVE_LOCKS_REQUIRED(!m_tx_relay_mutex) + { + return WITH_LOCK(m_tx_relay_mutex, return m_tx_relay.get()); + }; /** A vector of addresses to send to the peer, limited to MAX_ADDR_TO_SEND. */ std::vector m_addrs_to_send GUARDED_BY(NetEventsInterface::g_msgproc_mutex); @@ -400,7 +407,7 @@ struct Peer { {} private: - Mutex m_tx_relay_mutex; + mutable Mutex m_tx_relay_mutex; /** Transaction relay data. May be a nullptr. */ std::unique_ptr m_tx_relay GUARDED_BY(m_tx_relay_mutex); @@ -481,9 +488,6 @@ struct CNodeState { //! Whether this peer is an inbound connection const bool m_is_inbound; - //! A rolling bloom filter of all announced tx CInvs to this peer. - CRollingBloomFilter m_recently_announced_invs GUARDED_BY(NetEventsInterface::g_msgproc_mutex){INVENTORY_MAX_RECENT_RELAY, 0.000001}; - CNodeState(bool is_inbound) : m_is_inbound(is_inbound) {} }; @@ -904,7 +908,7 @@ private: std::atomic m_last_tip_update{0s}; /** Determine whether or not a peer can request a transaction, and return it (or nullptr if not found or not allowed). */ - CTransactionRef FindTxForGetData(const CNode& peer, const GenTxid& gtxid, const std::chrono::seconds mempool_req, const std::chrono::seconds now) + CTransactionRef FindTxForGetData(const Peer& peer, const GenTxid& gtxid, const std::chrono::seconds mempool_req, const std::chrono::seconds now) LOCKS_EXCLUDED(cs_main) EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex); void ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic& interruptMsgProc) @@ -2254,7 +2258,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& } } -CTransactionRef PeerManagerImpl::FindTxForGetData(const CNode& peer, const GenTxid& gtxid, const std::chrono::seconds mempool_req, const std::chrono::seconds now) +CTransactionRef PeerManagerImpl::FindTxForGetData(const Peer& peer, const GenTxid& gtxid, const std::chrono::seconds mempool_req, const std::chrono::seconds now) { auto txinfo = m_mempool.info(gtxid); if (txinfo.tx) { @@ -2269,7 +2273,7 @@ CTransactionRef PeerManagerImpl::FindTxForGetData(const CNode& peer, const GenTx { LOCK(cs_main); // Otherwise, the transaction must have been announced recently. - if (State(peer.GetId())->m_recently_announced_invs.contains(gtxid.GetHash())) { + if (Assume(peer.GetTxRelay())->m_recently_announced_invs.contains(gtxid.GetHash())) { // If it was, it can be relayed from either the mempool... if (txinfo.tx) return std::move(txinfo.tx); // ... or the relay pool. @@ -2312,7 +2316,7 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic continue; } - CTransactionRef tx = FindTxForGetData(pfrom, ToGenTxid(inv), mempool_req, now); + CTransactionRef tx = FindTxForGetData(peer, ToGenTxid(inv), mempool_req, now); if (tx) { // WTX and WITNESS_TX imply we serialize with witness int nSendFlags = (inv.IsMsgTx() ? SERIALIZE_TRANSACTION_NO_WITNESS : 0); @@ -2336,8 +2340,7 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic for (const uint256& parent_txid : parent_ids_to_add) { // Relaying a transaction with a recent but unconfirmed parent. if (WITH_LOCK(tx_relay->m_tx_inventory_mutex, return !tx_relay->m_tx_inventory_known_filter.contains(parent_txid))) { - LOCK(cs_main); - State(pfrom.GetId())->m_recently_announced_invs.insert(parent_txid); + tx_relay->m_recently_announced_invs.insert(parent_txid); } } } else { @@ -5687,7 +5690,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) } if (tx_relay->m_bloom_filter && !tx_relay->m_bloom_filter->IsRelevantAndUpdate(*txinfo.tx)) continue; // Send - State(pto->GetId())->m_recently_announced_invs.insert(hash); + tx_relay->m_recently_announced_invs.insert(hash); vInv.push_back(inv); nRelayedTransactions++; { From 3a060ae7b67cc28fc60cf28cbc518fa1df24f545 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Tue, 20 Sep 2022 19:52:18 +0100 Subject: [PATCH 10/10] scripted-diff: Rename nUnconnectingHeaders and fPreferHeaders -BEGIN VERIFY SCRIPT- ren() { sed -i "s:\<$1\>:$2:g" $(git grep -l "\<$1\>" ./src ./test); } ren nUnconnectingHeaders m_num_unconnecting_headers_msgs ren fPreferHeaders m_prefers_headers ren MAX_UNCONNECTING_HEADERS MAX_NUM_UNCONNECTING_HEADERS_MSGS -END VERIFY SCRIPT- --- src/net_processing.cpp | 32 +++++++++++++++--------------- test/functional/p2p_sendheaders.py | 10 +++++----- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 42f69ec3ae..2348f6c059 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -135,7 +135,7 @@ static constexpr double BLOCK_DOWNLOAD_TIMEOUT_PER_PEER = 0.5; /** Maximum number of headers to announce when relaying blocks with headers message.*/ static const unsigned int MAX_BLOCKS_TO_ANNOUNCE = 8; /** Maximum number of unconnecting headers announcements before DoS score */ -static const int MAX_UNCONNECTING_HEADERS = 10; +static const int MAX_NUM_UNCONNECTING_HEADERS_MSGS = 10; /** Minimum blocks required to signal NODE_NETWORK_LIMITED */ static const unsigned int NODE_NETWORK_LIMITED_MIN_BLOCKS = 288; /** Average delay between local address broadcasts */ @@ -393,13 +393,13 @@ struct Peer { std::atomic m_sent_sendheaders{false}; /** Length of current-streak of unconnecting headers announcements */ - int nUnconnectingHeaders GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0}; + int m_num_unconnecting_headers_msgs GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0}; /** When to potentially disconnect peer for stalling headers download */ std::chrono::microseconds m_headers_sync_timeout GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0us}; /** Whether this peer wants invs or headers (when possible) for block announcements */ - bool fPreferHeaders GUARDED_BY(NetEventsInterface::g_msgproc_mutex){false}; + bool m_prefers_headers GUARDED_BY(NetEventsInterface::g_msgproc_mutex){false}; explicit Peer(NodeId id, ServiceFlags our_services) : m_id{id} @@ -2439,24 +2439,24 @@ arith_uint256 PeerManagerImpl::GetAntiDoSWorkThreshold() * * We'll send a getheaders message in response to try to connect the chain. * - * The peer can send up to MAX_UNCONNECTING_HEADERS in a row that + * The peer can send up to MAX_NUM_UNCONNECTING_HEADERS_MSGS in a row that * don't connect before given DoS points. * * Once a headers message is received that is valid and does connect, - * nUnconnectingHeaders gets reset back to 0. + * m_num_unconnecting_headers_msgs gets reset back to 0. */ void PeerManagerImpl::HandleFewUnconnectingHeaders(CNode& pfrom, Peer& peer, const std::vector& headers) { - peer.nUnconnectingHeaders++; + peer.m_num_unconnecting_headers_msgs++; // Try to fill in the missing headers. const CBlockIndex* best_header{WITH_LOCK(cs_main, return m_chainman.m_best_header)}; if (MaybeSendGetHeaders(pfrom, GetLocator(best_header), peer)) { - LogPrint(BCLog::NET, "received header %s: missing prev block %s, sending getheaders (%d) to end (peer=%d, nUnconnectingHeaders=%d)\n", + LogPrint(BCLog::NET, "received header %s: missing prev block %s, sending getheaders (%d) to end (peer=%d, m_num_unconnecting_headers_msgs=%d)\n", headers[0].GetHash().ToString(), headers[0].hashPrevBlock.ToString(), best_header->nHeight, - pfrom.GetId(), peer.nUnconnectingHeaders); + pfrom.GetId(), peer.m_num_unconnecting_headers_msgs); } // Set hashLastUnknownBlock for this peer, so that if we @@ -2466,8 +2466,8 @@ void PeerManagerImpl::HandleFewUnconnectingHeaders(CNode& pfrom, Peer& peer, // The peer may just be broken, so periodically assign DoS points if this // condition persists. - if (peer.nUnconnectingHeaders % MAX_UNCONNECTING_HEADERS == 0) { - Misbehaving(peer, 20, strprintf("%d non-connecting headers", peer.nUnconnectingHeaders)); + if (peer.m_num_unconnecting_headers_msgs % MAX_NUM_UNCONNECTING_HEADERS_MSGS == 0) { + Misbehaving(peer, 20, strprintf("%d non-connecting headers", peer.m_num_unconnecting_headers_msgs)); } } @@ -2718,10 +2718,10 @@ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, c void PeerManagerImpl::UpdatePeerStateForReceivedHeaders(CNode& pfrom, Peer& peer, const CBlockIndex& last_header, bool received_new_header, bool may_have_more_headers) { - if (peer.nUnconnectingHeaders > 0) { - LogPrint(BCLog::NET, "peer=%d: resetting nUnconnectingHeaders (%d -> 0)\n", pfrom.GetId(), peer.nUnconnectingHeaders); + if (peer.m_num_unconnecting_headers_msgs > 0) { + LogPrint(BCLog::NET, "peer=%d: resetting m_num_unconnecting_headers_msgs (%d -> 0)\n", pfrom.GetId(), peer.m_num_unconnecting_headers_msgs); } - peer.nUnconnectingHeaders = 0; + peer.m_num_unconnecting_headers_msgs = 0; LOCK(cs_main); CNodeState *nodestate = State(pfrom.GetId()); @@ -3453,7 +3453,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } if (msg_type == NetMsgType::SENDHEADERS) { - peer->fPreferHeaders = true; + peer->m_prefers_headers = true; return; } @@ -5459,7 +5459,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // add all to the inv queue. LOCK(peer->m_block_inv_mutex); std::vector vHeaders; - bool fRevertToInv = ((!peer->fPreferHeaders && + bool fRevertToInv = ((!peer->m_prefers_headers && (!state.m_requested_hb_cmpctblocks || peer->m_blocks_for_headers_relay.size() > 1)) || peer->m_blocks_for_headers_relay.size() > MAX_BLOCKS_TO_ANNOUNCE); const CBlockIndex *pBestIndex = nullptr; // last header queued for delivery @@ -5536,7 +5536,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock)); } state.pindexBestHeaderSent = pBestIndex; - } else if (peer->fPreferHeaders) { + } else if (peer->m_prefers_headers) { if (vHeaders.size() > 1) { LogPrint(BCLog::NET, "%s: %u headers, range (%s, %s), to peer=%d\n", __func__, vHeaders.size(), diff --git a/test/functional/p2p_sendheaders.py b/test/functional/p2p_sendheaders.py index 1ccc447b89..508d6fe403 100755 --- a/test/functional/p2p_sendheaders.py +++ b/test/functional/p2p_sendheaders.py @@ -546,15 +546,15 @@ class SendHeadersTest(BitcoinTestFramework): blocks = [] # Now we test that if we repeatedly don't send connecting headers, we # don't go into an infinite loop trying to get them to connect. - MAX_UNCONNECTING_HEADERS = 10 - for _ in range(MAX_UNCONNECTING_HEADERS + 1): + MAX_NUM_UNCONNECTING_HEADERS_MSGS = 10 + for _ in range(MAX_NUM_UNCONNECTING_HEADERS_MSGS + 1): blocks.append(create_block(tip, create_coinbase(height), block_time)) blocks[-1].solve() tip = blocks[-1].sha256 block_time += 1 height += 1 - for i in range(1, MAX_UNCONNECTING_HEADERS): + for i in range(1, MAX_NUM_UNCONNECTING_HEADERS_MSGS): # Send a header that doesn't connect, check that we get a getheaders. with p2p_lock: test_node.last_message.pop("getheaders", None) @@ -568,8 +568,8 @@ class SendHeadersTest(BitcoinTestFramework): blocks = blocks[2:] # Now try to see how many unconnecting headers we can send - # before we get disconnected. Should be 5*MAX_UNCONNECTING_HEADERS - for i in range(5 * MAX_UNCONNECTING_HEADERS - 1): + # before we get disconnected. Should be 5*MAX_NUM_UNCONNECTING_HEADERS_MSGS + for i in range(5 * MAX_NUM_UNCONNECTING_HEADERS_MSGS - 1): # Send a header that doesn't connect, check that we get a getheaders. with p2p_lock: test_node.last_message.pop("getheaders", None)