diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 78dbe58a01ee8..7e812e30b0951 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -370,6 +370,157 @@ 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}; + int nBlocksInFlightValidHeaders{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 compressed headers (when possible) for block announcements. + bool fPreferHeadersCompressed{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. */ + bool m_provides_cmpctblocks{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 seconds 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}; + + /* + * State associated with objects download. + * + * Tx download algorithm: + * + * When inv comes in, queue up (process_time, inv) inside the peer's + * CNodeState (m_object_process_time) as long as m_object_announced for the peer + * isn't too big (MAX_PEER_OBJECT_ANNOUNCEMENTS). + * + * The process_time for a objects is set to nNow for outbound peers, + * nNow + 2 seconds for inbound peers. This is the time at which we'll + * consider trying to request the objects from the peer in + * SendMessages(). The delay for inbound peers is to allow outbound peers + * a chance to announce before we request from inbound peers, to prevent + * an adversary from using inbound connections to blind us to a + * objects (InvBlock). + * + * When we call SendMessages() for a given peer, + * we will loop over the objects in m_object_process_time, looking + * at the objects whose process_time <= nNow. We'll request each + * such objects that we don't have already and that hasn't been + * requested from another peer recently, up until we hit the + * MAX_PEER_OBJECT_IN_FLIGHT limit for the peer. Then we'll update + * g_already_asked_for for each requested inv, storing the time of the + * GETDATA request. We use g_already_asked_for to coordinate objects + * requests amongst our peers. + * + * For objects that we still need but we have already recently + * requested from some other peer, we'll reinsert (process_time, inv) + * back into the peer's m_object_process_time at the point in the future at + * which the most recent GETDATA request would time out (ie + * GetObjectInterval + the request time stored in g_already_asked_for). + * We add an additional delay for inbound peers, again to prefer + * attempting download from outbound peers first. + * We also add an extra small random delay up to 2 seconds + * to avoid biasing some peers over others. (e.g., due to fixed ordering + * of peer processing in ThreadMessageHandler). + * + * When we receive a objects from a peer, we remove the inv from the + * peer's m_object_in_flight set and from their recently announced set + * (m_object_announced). We also clear g_already_asked_for for that entry, so + * that if somehow the objects is not accepted but also not added to + * the reject filter, then we will eventually redownload from other + * peers. + */ + struct ObjectDownloadState { + /* Track when to attempt download of announced objects (process + * time in micros -> inv) + */ + std::multimap m_object_process_time; + + //! Store all the objects a peer has recently announced + std::set m_object_announced; + + //! Store objects which were requested by us, with timestamp + std::map m_object_in_flight; + + //! Periodically check for stuck getdata requests + std::chrono::microseconds m_check_expiry_timer{0}; + }; + + ObjectDownloadState m_object_download; + + //! 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: @@ -420,6 +571,7 @@ class PeerManagerImpl final : public PeerManager void ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, const std::chrono::microseconds time_received, const std::atomic& interruptMsgProc) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex); + void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) override; bool IsBanned(NodeId pnode) override EXCLUSIVE_LOCKS_REQUIRED(cs_main, !m_peer_mutex); void EraseObjectRequest(NodeId nodeid, const CInv& inv) override EXCLUSIVE_LOCKS_REQUIRED(::cs_main); void RequestObject(NodeId nodeid, const CInv& inv, std::chrono::microseconds current_time, @@ -569,6 +721,14 @@ class PeerManagerImpl final : public PeerManager */ std::map m_peer_map GUARDED_BY(m_peer_mutex); + /** Map maintaining per-node state. */ + 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); + /** Get a pointer to a mutable CNodeState. */ + CNodeState* State(NodeId pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + std::atomic m_next_inv_to_inbounds{0us}; /** Check whether the last unknown block a peer advertised is not yet known. */ @@ -666,6 +826,9 @@ class PeerManagerImpl final : public PeerManager /** 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 m_num_preferred_download_peers GUARDED_BY(cs_main){0}; + bool AlreadyHave(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main, !m_recent_confirmed_transactions_mutex); @@ -718,6 +881,16 @@ class PeerManagerImpl final : public PeerManager 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 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); + + /** Height of the highest block announced using BIP 152 high-bandwidth mode. */ + int m_highest_fast_announce{0}; + /* Returns a bool indicating whether we requested this block. * Also used if a block was /not/ received and timed out or started with another peer */ @@ -780,179 +953,24 @@ class PeerManagerImpl final : public PeerManager /** Offset into vExtraTxnForCompact to insert the next tx */ size_t vExtraTxnForCompactIt GUARDED_BY(g_cs_orphans) = 0; }; -} // namespace - -namespace { - /** Number of preferable block download peers. */ - int nPreferredDownload GUARDED_BY(cs_main) = 0; -} // 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}; - int nBlocksInFlightValidHeaders{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 compressed headers (when possible) for block announcements. - bool fPreferHeadersCompressed{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. */ - bool m_provides_cmpctblocks{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 seconds 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}; - - /* - * State associated with objects download. - * - * Tx download algorithm: - * - * When inv comes in, queue up (process_time, inv) inside the peer's - * CNodeState (m_object_process_time) as long as m_object_announced for the peer - * isn't too big (MAX_PEER_OBJECT_ANNOUNCEMENTS). - * - * The process_time for a objects is set to nNow for outbound peers, - * nNow + 2 seconds for inbound peers. This is the time at which we'll - * consider trying to request the objects from the peer in - * SendMessages(). The delay for inbound peers is to allow outbound peers - * a chance to announce before we request from inbound peers, to prevent - * an adversary from using inbound connections to blind us to a - * objects (InvBlock). - * - * When we call SendMessages() for a given peer, - * we will loop over the objects in m_object_process_time, looking - * at the objects whose process_time <= nNow. We'll request each - * such objects that we don't have already and that hasn't been - * requested from another peer recently, up until we hit the - * MAX_PEER_OBJECT_IN_FLIGHT limit for the peer. Then we'll update - * g_already_asked_for for each requested inv, storing the time of the - * GETDATA request. We use g_already_asked_for to coordinate objects - * requests amongst our peers. - * - * For objects that we still need but we have already recently - * requested from some other peer, we'll reinsert (process_time, inv) - * back into the peer's m_object_process_time at the point in the future at - * which the most recent GETDATA request would time out (ie - * GetObjectInterval + the request time stored in g_already_asked_for). - * We add an additional delay for inbound peers, again to prefer - * attempting download from outbound peers first. - * We also add an extra small random delay up to 2 seconds - * to avoid biasing some peers over others. (e.g., due to fixed ordering - * of peer processing in ThreadMessageHandler). - * - * When we receive a objects from a peer, we remove the inv from the - * peer's m_object_in_flight set and from their recently announced set - * (m_object_announced). We also clear g_already_asked_for for that entry, so - * that if somehow the objects is not accepted but also not added to - * the reject filter, then we will eventually redownload from other - * peers. - */ - struct ObjectDownloadState { - /* Track when to attempt download of announced objects (process - * time in micros -> inv) - */ - std::multimap m_object_process_time; - - //! Store all the objects a peer has recently announced - std::set m_object_announced; - - //! Store objects which were requested by us, with timestamp - std::map m_object_in_flight; - - //! Periodically check for stuck getdata requests - std::chrono::microseconds m_check_expiry_timer{0}; - }; - - ObjectDownloadState m_object_download; - - //! 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) {} -}; // Keeps track of the time (in microseconds) when transactions were requested last time unordered_limitedmap g_already_asked_for(MAX_INV_SZ, MAX_INV_SZ * 2); unordered_limitedmap g_erased_object_requests(MAX_INV_SZ, MAX_INV_SZ * 2); -/** 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); - if (it == mapNodeState.end()) +const CNodeState* PeerManagerImpl::State(NodeId pnode) const EXCLUSIVE_LOCKS_REQUIRED(cs_main) +{ + std::map::const_iterator it = m_node_states.find(pnode); + if (it == m_node_states.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 @@ -1037,16 +1055,6 @@ static void PushInv(Peer& peer, const CInv& inv) peer.m_tx_relay->vInventoryOtherToSend.push_back(inv); } -static void UpdatePreferredDownload(const CNode& node, const Peer& peer, 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() && CanServeBlocks(peer); - - nPreferredDownload += state->fPreferredDownload; -} - std::chrono::microseconds PeerManagerImpl::NextInvToInbounds(std::chrono::microseconds now, std::chrono::seconds average_interval) { @@ -1478,16 +1486,14 @@ size_t PeerManagerImpl::GetRequestedObjectCount(NodeId nodeid) const { AssertLockHeld(cs_main); - CNodeState* state = State(nodeid); + const CNodeState* state = State(nodeid); if (state == nullptr) return 0; return state->m_object_download.m_object_process_time.size(); } -// 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); @@ -1498,7 +1504,7 @@ void PeerManagerImpl::InitializeNode(CNode& node, ServiceFlags our_services) { NodeId nodeid = node.GetId(); { LOCK(cs_main); - mapNodeState.emplace_hint(mapNodeState.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(node.IsInboundConn())); + m_node_states.emplace_hint(m_node_states.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(node.IsInboundConn())); } PeerRef peer = std::make_shared(nodeid, our_services, /* block_relay_only = */ node.IsBlockOnlyConn()); { @@ -1555,18 +1561,18 @@ void PeerManagerImpl::FinalizeNode(const CNode& node) { mapBlocksInFlight.erase(entry.hash); } WITH_LOCK(g_cs_orphans, m_orphanage.EraseForPeer(nodeid)); - nPreferredDownload -= state->fPreferredDownload; + m_num_preferred_download_peers -= state->fPreferredDownload; nPeersWithValidatedDownloads -= (state->nBlocksInFlightValidHeaders != 0); assert(nPeersWithValidatedDownloads >= 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(nPeersWithValidatedDownloads == 0); assert(m_outbound_peers_with_protect_from_disconnect == 0); } @@ -1605,7 +1611,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; @@ -1910,12 +1916,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); - /** * Maintain state about the best-seen block and fast-announce a compact block * to compatible peers. @@ -1926,20 +1926,19 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha LOCK(cs_main); - static int nHighestFastAnnounce = 0; - if (pindex->nHeight <= nHighestFastAnnounce) + if (pindex->nHeight <= m_highest_fast_announce) return; - nHighestFastAnnounce = pindex->nHeight; + m_highest_fast_announce = pindex->nHeight; uint256 hashBlock(pblock->GetHash()); const std::shared_future lazy_ser{ 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; + LOCK(m_most_recent_block_mutex); + m_most_recent_block_hash = hashBlock; + m_most_recent_block = pblock; + m_most_recent_compact_block = pcmpctblock; } m_connman.ForEachNode([this, pindex, &lazy_ser, &hashBlock](CNode* pnode) { @@ -2280,9 +2279,9 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& std::shared_ptr a_recent_block; std::shared_ptr a_recent_compact_block; { - LOCK(cs_most_recent_block); - a_recent_block = most_recent_block; - a_recent_compact_block = most_recent_compact_block; + LOCK(m_most_recent_block_mutex); + a_recent_block = m_most_recent_block; + a_recent_compact_block = m_most_recent_compact_block; } bool need_activate_chain = false; @@ -3402,7 +3401,9 @@ void PeerManagerImpl::ProcessMessage( // Potentially mark this peer as a preferred download peer. { LOCK(cs_main); - UpdatePreferredDownload(pfrom, *peer, State(pfrom.GetId())); + CNodeState* state = State(pfrom.GetId()); + state->fPreferredDownload = (!pfrom.IsInboundConn() || pfrom.HasPermission(NetPermissionFlags::NoBan)) && !pfrom.IsAddrFetchConn() && CanServeBlocks(*peer); + m_num_preferred_download_peers += state->fPreferredDownload; } // Self advertisement & GETADDR logic @@ -3871,8 +3872,8 @@ void PeerManagerImpl::ProcessMessage( { 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)) { @@ -3925,10 +3926,10 @@ void PeerManagerImpl::ProcessMessage( 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); @@ -5438,7 +5439,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // the latest blocks is from an inbound peer, we have to be sure to // eventually download it (and not just wait indefinitely for an // outbound peer to have it). - if (nPreferredDownload == 0 || mapBlocksInFlight.empty()) { + if (m_num_preferred_download_peers == 0 || mapBlocksInFlight.empty()) { sync_blocks_and_headers_from_peer = true; } } @@ -5557,9 +5558,9 @@ bool PeerManagerImpl::SendMessages(CNode* pto) bool fGotBlockFromCache = false; { - LOCK(cs_most_recent_block); - if (most_recent_block_hash == pBestIndex->GetBlockHash()) { - m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::CMPCTBLOCK, *most_recent_compact_block)); + LOCK(m_most_recent_block_mutex); + if (m_most_recent_block_hash == pBestIndex->GetBlockHash()) { + m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::CMPCTBLOCK, *m_most_recent_compact_block)); fGotBlockFromCache = true; } } @@ -5832,7 +5833,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() - nMaxTipAge) { - 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 @@ -5961,4 +5962,4 @@ bool PeerManagerImpl::SendMessages(CNode* pto) } } // release cs_main return true; -} +} \ No newline at end of file diff --git a/src/net_processing.h b/src/net_processing.h index 2b281df0c9418..17f0e2e500a37 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -127,6 +127,9 @@ class PeerManager : public CValidationInterface, public NetEventsInterface 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; + virtual bool IsBanned(NodeId pnode) = 0; virtual void EraseObjectRequest(NodeId nodeid, const CInv& inv) = 0; diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index dbf154531d8b8..a1d992cc01afb 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 @@ -202,7 +200,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) {