From 37ecaf3e7a028486a0a1c9b717e8eb4214215805 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Thu, 17 Mar 2022 18:16:04 +0100 Subject: [PATCH 1/7] [net processing] Move CNodeState declaration above PeerManagerImpl --- src/net_processing.cpp | 195 +++++++++++++++++++++-------------------- 1 file changed, 98 insertions(+), 97 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 02eb3762b1..e0173a73dc 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -332,6 +332,104 @@ struct Peer { using PeerRef = std::shared_ptr; +/** + * Maintain validation-specific state about nodes, protected by cs_main, instead + * by CNode's own locks. This simplifies asynchronous operation, where + * processing of incoming data is done after the ProcessMessage call returns, + * and we're no longer holding the node's locks. + */ +struct CNodeState { + //! The best known block we know this peer has announced. + const CBlockIndex* pindexBestKnownBlock{nullptr}; + //! The hash of the last unknown block this peer has announced. + uint256 hashLastUnknownBlock{}; + //! The last full block we both have. + 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{0}; + //! 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}; + //! Since when we're stalling block download progress (in microseconds), or 0. + std::chrono::microseconds m_stalling_since{0us}; + std::list vBlocksInFlight; + //! When the first entry in vBlocksInFlight started downloading. Don't care when vBlocksInFlight is empty. + std::chrono::microseconds m_downloading_since{0us}; + 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{false}; + //! Whether this peer wants invs or cmpctblocks (when possible) for block announcements. + bool fPreferHeaderAndIDs{false}; + /** + * Whether this peer will send us cmpctblocks if we request them. + * This is not used to gate request logic, as we really only care about fSupportsDesiredCmpctVersion, + * but is used as a flag to "lock in" the version of compact blocks (fWantsCmpctWitness) we send. + */ + bool fProvidesHeaderAndIDs{false}; + //! Whether this peer can give us witnesses + bool fHaveWitness{false}; + //! Whether this peer wants witnesses in cmpctblocks/blocktxns + bool fWantsCmpctWitness{false}; + /** + * If we've announced NODE_WITNESS to this peer: whether the peer sends witnesses in cmpctblocks/blocktxns, + * otherwise: whether this peer sends non-witnesses in cmpctblocks/blocktxns. + */ + bool fSupportsDesiredCmpctVersion{false}; + + /** State used to enforce CHAIN_SYNC_TIMEOUT and EXTRA_PEER_CHECK_INTERVAL logic. + * + * Both are only in effect for outbound, non-manual, non-protected connections. + * Any peer protected (m_protect = true) is not chosen for eviction. A peer is + * marked as protected if all of these are true: + * - its connection type is IsBlockOnlyConn() == false + * - it gave us a valid connecting header + * - we haven't reached MAX_OUTBOUND_PEERS_TO_PROTECT_FROM_DISCONNECT yet + * - its chain tip has at least as much work as ours + * + * CHAIN_SYNC_TIMEOUT: if a peer's best known block has less work than our tip, + * set a timeout CHAIN_SYNC_TIMEOUT in the future: + * - If at timeout their best known block now has more work than our tip + * when the timeout was set, then either reset the timeout or clear it + * (after comparing against our current tip's work) + * - If at timeout their best known block still has less work than our + * tip did when the timeout was set, then send a getheaders message, + * and set a shorter timeout, HEADERS_RESPONSE_TIME seconds in future. + * If their best known block is still behind when that new timeout is + * reached, disconnect. + * + * EXTRA_PEER_CHECK_INTERVAL: after each interval, if we have too many outbound peers, + * drop the outbound one that least recently announced us a new block. + */ + struct ChainSyncTimeoutState { + //! A timeout used for checking whether our peer has sufficiently synced + std::chrono::seconds m_timeout{0s}; + //! A header with the work we require on our peer's chain + const CBlockIndex* m_work_header{nullptr}; + //! After timeout is reached, set to true after sending getheaders + bool m_sent_getheaders{false}; + //! Whether this peer is protected from disconnection due to a bad/slow chain + bool m_protect{false}; + }; + + ChainSyncTimeoutState m_chain_sync; + + //! Time of last new block announcement + int64_t m_last_block_announcement{0}; + + //! 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 = CRollingBloomFilter{INVENTORY_MAX_RECENT_RELAY, 0.000001}; + + CNodeState(bool is_inbound) : m_is_inbound(is_inbound) {} +}; + class PeerManagerImpl final : public PeerManager { public: @@ -716,103 +814,6 @@ namespace { } // namespace namespace { -/** - * Maintain validation-specific state about nodes, protected by cs_main, instead - * by CNode's own locks. This simplifies asynchronous operation, where - * processing of incoming data is done after the ProcessMessage call returns, - * and we're no longer holding the node's locks. - */ -struct CNodeState { - //! The best known block we know this peer has announced. - const CBlockIndex* pindexBestKnownBlock{nullptr}; - //! The hash of the last unknown block this peer has announced. - uint256 hashLastUnknownBlock{}; - //! The last full block we both have. - 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{0}; - //! 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}; - //! Since when we're stalling block download progress (in microseconds), or 0. - std::chrono::microseconds m_stalling_since{0us}; - std::list vBlocksInFlight; - //! When the first entry in vBlocksInFlight started downloading. Don't care when vBlocksInFlight is empty. - std::chrono::microseconds m_downloading_since{0us}; - 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{false}; - //! Whether this peer wants invs or cmpctblocks (when possible) for block announcements. - bool fPreferHeaderAndIDs{false}; - /** - * Whether this peer will send us cmpctblocks if we request them. - * This is not used to gate request logic, as we really only care about fSupportsDesiredCmpctVersion, - * but is used as a flag to "lock in" the version of compact blocks (fWantsCmpctWitness) we send. - */ - bool fProvidesHeaderAndIDs{false}; - //! Whether this peer can give us witnesses - bool fHaveWitness{false}; - //! Whether this peer wants witnesses in cmpctblocks/blocktxns - bool fWantsCmpctWitness{false}; - /** - * If we've announced NODE_WITNESS to this peer: whether the peer sends witnesses in cmpctblocks/blocktxns, - * otherwise: whether this peer sends non-witnesses in cmpctblocks/blocktxns. - */ - bool fSupportsDesiredCmpctVersion{false}; - - /** State used to enforce CHAIN_SYNC_TIMEOUT and EXTRA_PEER_CHECK_INTERVAL logic. - * - * Both are only in effect for outbound, non-manual, non-protected connections. - * Any peer protected (m_protect = true) is not chosen for eviction. A peer is - * marked as protected if all of these are true: - * - its connection type is IsBlockOnlyConn() == false - * - it gave us a valid connecting header - * - we haven't reached MAX_OUTBOUND_PEERS_TO_PROTECT_FROM_DISCONNECT yet - * - its chain tip has at least as much work as ours - * - * CHAIN_SYNC_TIMEOUT: if a peer's best known block has less work than our tip, - * set a timeout CHAIN_SYNC_TIMEOUT in the future: - * - If at timeout their best known block now has more work than our tip - * when the timeout was set, then either reset the timeout or clear it - * (after comparing against our current tip's work) - * - If at timeout their best known block still has less work than our - * tip did when the timeout was set, then send a getheaders message, - * and set a shorter timeout, HEADERS_RESPONSE_TIME seconds in future. - * If their best known block is still behind when that new timeout is - * reached, disconnect. - * - * EXTRA_PEER_CHECK_INTERVAL: after each interval, if we have too many outbound peers, - * drop the outbound one that least recently announced us a new block. - */ - struct ChainSyncTimeoutState { - //! A timeout used for checking whether our peer has sufficiently synced - std::chrono::seconds m_timeout{0s}; - //! A header with the work we require on our peer's chain - const CBlockIndex* m_work_header{nullptr}; - //! After timeout is reached, set to true after sending getheaders - bool m_sent_getheaders{false}; - //! Whether this peer is protected from disconnection due to a bad/slow chain - bool m_protect{false}; - }; - - ChainSyncTimeoutState m_chain_sync; - - //! Time of last new block announcement - int64_t m_last_block_announcement{0}; - - //! 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 = CRollingBloomFilter{INVENTORY_MAX_RECENT_RELAY, 0.000001}; - - CNodeState(bool is_inbound) : m_is_inbound(is_inbound) {} -}; /** Map maintaining per-node state. */ static std::map mapNodeState GUARDED_BY(cs_main); From a292df283a596efe7e1d40c33a6d614d70ed564d Mon Sep 17 00:00:00 2001 From: dergoegge Date: Sun, 20 Feb 2022 15:20:15 +0100 Subject: [PATCH 2/7] [net processing] Move mapNodeState into PeerManagerImpl --- src/net_processing.cpp | 33 +++++++++++++++++++++--------- src/net_processing.h | 3 +++ src/test/denialofservice_tests.cpp | 4 +--- 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index e0173a73dc..57fa3b1fad 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -462,6 +462,7 @@ public: void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message) override; void ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, const std::chrono::microseconds time_received, const std::atomic& interruptMsgProc) override; + void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) override; private: /** Consider evicting an outbound peer based on the amount of time they've been behind our tip */ @@ -580,6 +581,16 @@ private: */ std::map m_peer_map GUARDED_BY(m_peer_mutex); + /** Map maintaining per-node state. */ + std::map mapNodeState GUARDED_BY(cs_main); + + /** Get a pointer to a const CNodeState, used when not mutating the CNodeState object. */ + const CNodeState* State(NodeId pnode) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); + /** Get a pointer to a mutable CNodeState. */ + CNodeState* State(NodeId pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + + uint32_t GetFetchFlags(const CNode& pfrom) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); + std::atomic m_next_inv_to_inbounds{0us}; /** Number of nodes with fSyncStarted. */ @@ -815,16 +826,19 @@ namespace { namespace { -/** Map maintaining per-node state. */ -static std::map mapNodeState GUARDED_BY(cs_main); - -static CNodeState *State(NodeId pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { - std::map::iterator it = mapNodeState.find(pnode); +const CNodeState* PeerManagerImpl::State(NodeId pnode) const EXCLUSIVE_LOCKS_REQUIRED(cs_main) +{ + std::map::const_iterator it = mapNodeState.find(pnode); if (it == mapNodeState.end()) return nullptr; return &it->second; } +CNodeState* PeerManagerImpl::State(NodeId pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +{ + return const_cast(std::as_const(*this).State(pnode)); +} + /** * Whether the peer supports the address. For example, a peer that does not * implement BIP155 cannot receive Tor v3 addresses because it requires @@ -1214,9 +1228,7 @@ void PeerManagerImpl::AddTxAnnouncement(const CNode& node, const GenTxid& gtxid, m_txrequest.ReceivedInv(nodeid, gtxid, preferred, current_time + delay); } -// This function is used for testing the stale tip eviction logic, see -// denialofservice_tests.cpp -void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) +void PeerManagerImpl::UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) { LOCK(cs_main); CNodeState *state = State(node); @@ -1342,7 +1354,7 @@ bool PeerManagerImpl::GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) c { { LOCK(cs_main); - CNodeState* state = State(nodeid); + const CNodeState* state = State(nodeid); if (state == nullptr) return false; stats.nSyncHeight = state->pindexBestKnownBlock ? state->pindexBestKnownBlock->nHeight : -1; @@ -2122,7 +2134,8 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic } } -static uint32_t GetFetchFlags(const CNode& pfrom) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { +uint32_t PeerManagerImpl::GetFetchFlags(const CNode& pfrom) const EXCLUSIVE_LOCKS_REQUIRED(cs_main) +{ uint32_t nFetchFlags = 0; if (State(pfrom.GetId())->fHaveWitness) { nFetchFlags |= MSG_WITNESS_FLAG; diff --git a/src/net_processing.h b/src/net_processing.h index 7dacaee5c1..c982b919a6 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -87,6 +87,9 @@ public: /** Process a single message from a peer. Public for fuzz testing */ virtual void ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, const std::chrono::microseconds time_received, const std::atomic& interruptMsgProc) = 0; + + /** This function is used for testing the stale tip eviction logic, see denialofservice_tests.cpp */ + virtual void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) = 0; }; #endif // BITCOIN_NET_PROCESSING_H diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index f03ff5ba3a..39f4c031d3 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -34,8 +34,6 @@ static CService ip(uint32_t i) return CService(CNetAddr(s), Params().GetDefaultPort()); } -void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds); - BOOST_FIXTURE_TEST_SUITE(denialofservice_tests, TestingSetup) // Test eviction of an outbound peer whose chain never advances @@ -197,7 +195,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) // Update the last announced block time for the last // peer, and check that the next newest node gets evicted. - UpdateLastBlockAnnounceTime(vNodes.back()->GetId(), GetTime()); + peerLogic->UpdateLastBlockAnnounceTime(vNodes.back()->GetId(), GetTime()); peerLogic->CheckForStaleTipAndEvictPeers(); for (int i = 0; i < max_outbound_full_relay - 1; ++i) { From 490c08f96a34ed436c3d2cf7b9a3ed72694b6147 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Fri, 18 Mar 2022 23:38:37 +0100 Subject: [PATCH 3/7] [net processing] Move nPreferredDownload into PeerManagerImpl --- src/net_processing.cpp | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 57fa3b1fad..b81d40e5c5 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -610,6 +610,11 @@ private: /** Number of outbound peers with m_chain_sync.m_protect. */ int m_outbound_peers_with_protect_from_disconnect GUARDED_BY(cs_main) = 0; + /** Number of preferable block download peers. */ + int nPreferredDownload GUARDED_BY(cs_main){0}; + + void UpdatePreferredDownload(const CNode& node, CNodeState* state) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool AlreadyHaveTx(const GenTxid& gtxid) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** @@ -817,14 +822,6 @@ private: */ bool SetupAddressRelay(const CNode& node, Peer& peer); }; -} // namespace - -namespace { - /** Number of preferable block download peers. */ - int nPreferredDownload GUARDED_BY(cs_main) = 0; -} // namespace - -namespace { const CNodeState* PeerManagerImpl::State(NodeId pnode) const EXCLUSIVE_LOCKS_REQUIRED(cs_main) { @@ -878,7 +875,7 @@ static void AddKnownTx(Peer& peer, const uint256& hash) } } -static void UpdatePreferredDownload(const CNode& node, CNodeState* state) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +void PeerManagerImpl::UpdatePreferredDownload(const CNode& node, CNodeState* state) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { nPreferredDownload -= state->fPreferredDownload; From a4c55a93ef9277e1043c286120e2417652ee8bbb Mon Sep 17 00:00:00 2001 From: dergoegge Date: Fri, 18 Mar 2022 23:39:26 +0100 Subject: [PATCH 4/7] [net processing] Inline and simplify UpdatePreferredDownload We inline `UpdatePreferredDownload` because it is only used in one location during the version handshake. We simplify it by removing the initial subtraction of `state->fPreferredDownload` from `nPreferredDownload`. This is ok since the version handshake is only called once per peer and `state->fPreferredDownload` will always be false before the newly inlined code is called, making the subtraction a noop. --- src/net_processing.cpp | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b81d40e5c5..014b722ea3 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -613,8 +613,6 @@ private: /** Number of preferable block download peers. */ int nPreferredDownload GUARDED_BY(cs_main){0}; - void UpdatePreferredDownload(const CNode& node, CNodeState* state) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - bool AlreadyHaveTx(const GenTxid& gtxid) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** @@ -875,16 +873,6 @@ static void AddKnownTx(Peer& peer, const uint256& hash) } } -void PeerManagerImpl::UpdatePreferredDownload(const CNode& node, CNodeState* state) EXCLUSIVE_LOCKS_REQUIRED(cs_main) -{ - nPreferredDownload -= state->fPreferredDownload; - - // Whether this node should be marked as a preferred download node. - state->fPreferredDownload = (!node.IsInboundConn() || node.HasPermission(NetPermissionFlags::NoBan)) && !node.IsAddrFetchConn() && !node.fClient; - - nPreferredDownload += state->fPreferredDownload; -} - std::chrono::microseconds PeerManagerImpl::NextInvToInbounds(std::chrono::microseconds now, std::chrono::seconds average_interval) { @@ -2741,8 +2729,10 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // Potentially mark this peer as a preferred download peer. { - LOCK(cs_main); - UpdatePreferredDownload(pfrom, State(pfrom.GetId())); + LOCK(cs_main); + CNodeState* state = State(pfrom.GetId()); + state->fPreferredDownload = (!pfrom.IsInboundConn() || pfrom.HasPermission(NetPermissionFlags::NoBan)) && !pfrom.IsAddrFetchConn() && !pfrom.fClient; + nPreferredDownload += state->fPreferredDownload; } // Self advertisement & GETADDR logic From 10b83e2aa3393ef2c942fde7ac86e8cf3ea224c1 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Sun, 20 Feb 2022 15:40:36 +0100 Subject: [PATCH 5/7] [net processing] Move block cache state into PeerManagerImpl --- 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 014b722ea3..9b9b44cb48 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -679,6 +679,14 @@ private: std::chrono::microseconds NextInvToInbounds(std::chrono::microseconds now, std::chrono::seconds average_interval); + + // All of the following cache a recent block, and are protected by cs_most_recent_block + RecursiveMutex cs_most_recent_block; + std::shared_ptr most_recent_block GUARDED_BY(cs_most_recent_block); + std::shared_ptr most_recent_compact_block GUARDED_BY(cs_most_recent_block); + uint256 most_recent_block_hash GUARDED_BY(cs_most_recent_block); + bool fWitnessesPresentInMostRecentCompactBlock GUARDED_BY(cs_most_recent_block){false}; + /** Have we requested this block from a peer */ bool IsBlockRequested(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -1613,13 +1621,6 @@ void PeerManagerImpl::BlockDisconnected(const std::shared_ptr &blo m_recent_confirmed_transactions.reset(); } -// All of the following cache a recent block, and are protected by cs_most_recent_block -static RecursiveMutex cs_most_recent_block; -static std::shared_ptr most_recent_block GUARDED_BY(cs_most_recent_block); -static std::shared_ptr most_recent_compact_block GUARDED_BY(cs_most_recent_block); -static uint256 most_recent_block_hash GUARDED_BY(cs_most_recent_block); -static bool fWitnessesPresentInMostRecentCompactBlock GUARDED_BY(cs_most_recent_block); - /** * Maintain state about the best-seen block and fast-announce a compact block * to compatible peers. From 91c339243e11ec42eeeaca8fe015fc1c3e6338e1 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Wed, 20 Apr 2022 13:52:49 +0200 Subject: [PATCH 6/7] [net processing] Move nHighestFastAnnounce into PeerManagerImpl --- src/net_processing.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 9b9b44cb48..f00cfb727d 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -687,6 +687,9 @@ private: uint256 most_recent_block_hash GUARDED_BY(cs_most_recent_block); bool fWitnessesPresentInMostRecentCompactBlock GUARDED_BY(cs_most_recent_block){false}; + /** Height of the highest block announced using BIP 152 high-bandwidth mode. */ + int nHighestFastAnnounce{0}; + /** Have we requested this block from a peer */ bool IsBlockRequested(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -1632,7 +1635,6 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha LOCK(cs_main); - static int nHighestFastAnnounce = 0; if (pindex->nHeight <= nHighestFastAnnounce) return; nHighestFastAnnounce = pindex->nHeight; From 778343a379026ef233dffea67f5226565f6d5720 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Sat, 12 Mar 2022 16:24:31 +0100 Subject: [PATCH 7/7] scripted-diff: Rename PeerManagerImpl members -BEGIN VERIFY SCRIPT- ren() { sed -i "s:\<$1\>:$2:g" $(git grep -l "\<$1\>" ./src ./test); } ren mapNodeState m_node_states ren cs_most_recent_block m_most_recent_block_mutex ren most_recent_block m_most_recent_block ren most_recent_compact_block m_most_recent_compact_block ren most_recent_block_hash m_most_recent_block_hash ren fWitnessesPresentInMostRecentCompactBlock m_most_recent_compact_block_has_witnesses ren nPreferredDownload m_num_preferred_download_peers ren nHighestFastAnnounce m_highest_fast_announce -END VERIFY SCRIPT- --- src/net_processing.cpp | 82 +++++++++++++++++++++--------------------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index f00cfb727d..7f88d534b3 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -582,7 +582,7 @@ private: std::map m_peer_map GUARDED_BY(m_peer_mutex); /** Map maintaining per-node state. */ - std::map mapNodeState GUARDED_BY(cs_main); + std::map m_node_states GUARDED_BY(cs_main); /** Get a pointer to a const CNodeState, used when not mutating the CNodeState object. */ const CNodeState* State(NodeId pnode) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -611,7 +611,7 @@ private: int m_outbound_peers_with_protect_from_disconnect GUARDED_BY(cs_main) = 0; /** Number of preferable block download peers. */ - int nPreferredDownload GUARDED_BY(cs_main){0}; + int m_num_preferred_download_peers GUARDED_BY(cs_main){0}; bool AlreadyHaveTx(const GenTxid& gtxid) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -680,15 +680,15 @@ private: std::chrono::seconds average_interval); - // All of the following cache a recent block, and are protected by cs_most_recent_block - RecursiveMutex cs_most_recent_block; - std::shared_ptr most_recent_block GUARDED_BY(cs_most_recent_block); - std::shared_ptr most_recent_compact_block GUARDED_BY(cs_most_recent_block); - uint256 most_recent_block_hash GUARDED_BY(cs_most_recent_block); - bool fWitnessesPresentInMostRecentCompactBlock GUARDED_BY(cs_most_recent_block){false}; + // All of the following cache a recent block, and are protected by m_most_recent_block_mutex + RecursiveMutex m_most_recent_block_mutex; + std::shared_ptr m_most_recent_block GUARDED_BY(m_most_recent_block_mutex); + std::shared_ptr m_most_recent_compact_block GUARDED_BY(m_most_recent_block_mutex); + uint256 m_most_recent_block_hash GUARDED_BY(m_most_recent_block_mutex); + bool m_most_recent_compact_block_has_witnesses GUARDED_BY(m_most_recent_block_mutex){false}; /** Height of the highest block announced using BIP 152 high-bandwidth mode. */ - int nHighestFastAnnounce{0}; + int m_highest_fast_announce{0}; /** Have we requested this block from a peer */ bool IsBlockRequested(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -834,8 +834,8 @@ private: const CNodeState* PeerManagerImpl::State(NodeId pnode) const EXCLUSIVE_LOCKS_REQUIRED(cs_main) { - std::map::const_iterator it = mapNodeState.find(pnode); - if (it == mapNodeState.end()) + std::map::const_iterator it = m_node_states.find(pnode); + if (it == m_node_states.end()) return nullptr; return &it->second; } @@ -1236,7 +1236,7 @@ void PeerManagerImpl::InitializeNode(CNode *pnode) NodeId nodeid = pnode->GetId(); { LOCK(cs_main); - mapNodeState.emplace_hint(mapNodeState.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(pnode->IsInboundConn())); + m_node_states.emplace_hint(m_node_states.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(pnode->IsInboundConn())); assert(m_txrequest.Count(nodeid) == 0); } PeerRef peer = std::make_shared(nodeid, /*tx_relay=*/ !pnode->IsBlockOnlyConn()); @@ -1298,18 +1298,18 @@ void PeerManagerImpl::FinalizeNode(const CNode& node) } WITH_LOCK(g_cs_orphans, m_orphanage.EraseForPeer(nodeid)); m_txrequest.DisconnectedPeer(nodeid); - nPreferredDownload -= state->fPreferredDownload; + m_num_preferred_download_peers -= state->fPreferredDownload; m_peers_downloading_from -= (state->nBlocksInFlight != 0); assert(m_peers_downloading_from >= 0); m_outbound_peers_with_protect_from_disconnect -= state->m_chain_sync.m_protect; assert(m_outbound_peers_with_protect_from_disconnect >= 0); - mapNodeState.erase(nodeid); + m_node_states.erase(nodeid); - if (mapNodeState.empty()) { + if (m_node_states.empty()) { // Do a consistency check after the last peer is removed. assert(mapBlocksInFlight.empty()); - assert(nPreferredDownload == 0); + assert(m_num_preferred_download_peers == 0); assert(m_peers_downloading_from == 0); assert(m_outbound_peers_with_protect_from_disconnect == 0); assert(m_wtxid_relay_peers == 0); @@ -1635,9 +1635,9 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha LOCK(cs_main); - if (pindex->nHeight <= nHighestFastAnnounce) + if (pindex->nHeight <= m_highest_fast_announce) return; - nHighestFastAnnounce = pindex->nHeight; + m_highest_fast_announce = pindex->nHeight; bool fWitnessEnabled = DeploymentActiveAt(*pindex, m_chainparams.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT); uint256 hashBlock(pblock->GetHash()); @@ -1645,11 +1645,11 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha std::async(std::launch::deferred, [&] { return msgMaker.Make(NetMsgType::CMPCTBLOCK, *pcmpctblock); })}; { - LOCK(cs_most_recent_block); - most_recent_block_hash = hashBlock; - most_recent_block = pblock; - most_recent_compact_block = pcmpctblock; - fWitnessesPresentInMostRecentCompactBlock = fWitnessEnabled; + LOCK(m_most_recent_block_mutex); + m_most_recent_block_hash = hashBlock; + m_most_recent_block = pblock; + m_most_recent_compact_block = pcmpctblock; + m_most_recent_compact_block_has_witnesses = fWitnessEnabled; } m_connman.ForEachNode([this, pindex, fWitnessEnabled, &lazy_ser, &hashBlock](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { @@ -1856,10 +1856,10 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& std::shared_ptr a_recent_compact_block; bool fWitnessesPresentInARecentCompactBlock; { - LOCK(cs_most_recent_block); - a_recent_block = most_recent_block; - a_recent_compact_block = most_recent_compact_block; - fWitnessesPresentInARecentCompactBlock = fWitnessesPresentInMostRecentCompactBlock; + LOCK(m_most_recent_block_mutex); + a_recent_block = m_most_recent_block; + a_recent_compact_block = m_most_recent_compact_block; + fWitnessesPresentInARecentCompactBlock = m_most_recent_compact_block_has_witnesses; } bool need_activate_chain = false; @@ -2735,7 +2735,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, LOCK(cs_main); CNodeState* state = State(pfrom.GetId()); state->fPreferredDownload = (!pfrom.IsInboundConn() || pfrom.HasPermission(NetPermissionFlags::NoBan)) && !pfrom.IsAddrFetchConn() && !pfrom.fClient; - nPreferredDownload += state->fPreferredDownload; + m_num_preferred_download_peers += state->fPreferredDownload; } // Self advertisement & GETADDR logic @@ -3158,8 +3158,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, { std::shared_ptr a_recent_block; { - LOCK(cs_most_recent_block); - a_recent_block = most_recent_block; + LOCK(m_most_recent_block_mutex); + a_recent_block = m_most_recent_block; } BlockValidationState state; if (!m_chainman.ActiveChainstate().ActivateBestChain(state, a_recent_block)) { @@ -3210,10 +3210,10 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, std::shared_ptr recent_block; { - LOCK(cs_most_recent_block); - if (most_recent_block_hash == req.blockhash) - recent_block = most_recent_block; - // Unlock cs_most_recent_block to avoid cs_main lock inversion + LOCK(m_most_recent_block_mutex); + if (m_most_recent_block_hash == req.blockhash) + recent_block = m_most_recent_block; + // Unlock m_most_recent_block_mutex to avoid cs_main lock inversion } if (recent_block) { SendBlockTransactions(pfrom, *recent_block, req); @@ -4677,7 +4677,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) if (m_chainman.m_best_header == nullptr) { m_chainman.m_best_header = m_chainman.ActiveChain().Tip(); } - bool fFetch = state.fPreferredDownload || (nPreferredDownload == 0 && !pto->fClient && !pto->IsAddrFetchConn()); // Download if this is a nice peer, or we have no nice peers and this one might do. + bool fFetch = state.fPreferredDownload || (m_num_preferred_download_peers == 0 && !pto->fClient && !pto->IsAddrFetchConn()); // Download if this is a nice peer, or we have no nice peers and this one might do. if (!state.fSyncStarted && !pto->fClient && !fImporting && !fReindex) { // Only actively request headers from a single peer, unless we're close to today. if ((nSyncStarted == 0 && fFetch) || m_chainman.m_best_header->GetBlockTime() > GetAdjustedTime() - 24 * 60 * 60) { @@ -4782,12 +4782,12 @@ bool PeerManagerImpl::SendMessages(CNode* pto) bool fGotBlockFromCache = false; { - LOCK(cs_most_recent_block); - if (most_recent_block_hash == pBestIndex->GetBlockHash()) { - if (state.fWantsCmpctWitness || !fWitnessesPresentInMostRecentCompactBlock) - m_connman.PushMessage(pto, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, *most_recent_compact_block)); + LOCK(m_most_recent_block_mutex); + if (m_most_recent_block_hash == pBestIndex->GetBlockHash()) { + if (state.fWantsCmpctWitness || !m_most_recent_compact_block_has_witnesses) + m_connman.PushMessage(pto, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, *m_most_recent_compact_block)); else { - CBlockHeaderAndShortTxIDs cmpctblock(*most_recent_block, state.fWantsCmpctWitness); + CBlockHeaderAndShortTxIDs cmpctblock(*m_most_recent_block, state.fWantsCmpctWitness); m_connman.PushMessage(pto, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, cmpctblock)); } fGotBlockFromCache = true; @@ -5022,7 +5022,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) if (state.fSyncStarted && state.m_headers_sync_timeout < std::chrono::microseconds::max()) { // Detect whether this is a stalling initial-headers-sync peer if (m_chainman.m_best_header->GetBlockTime() <= GetAdjustedTime() - 24 * 60 * 60) { - if (current_time > state.m_headers_sync_timeout && nSyncStarted == 1 && (nPreferredDownload - state.fPreferredDownload >= 1)) { + if (current_time > state.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