diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index c407be20db..08bef971a0 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -1389,7 +1389,7 @@ void CSigSharesManager::RemoveBannedNodeStates() LOCK2(cs_main, cs); for (auto it = nodeStates.begin(); it != nodeStates.end();) { - if (IsBanned(it->first)) { + if (m_peerman->IsBanned(it->first)) { // re-request sigshares from other nodes it->second.requestedSigShares.ForEach([this](const SigShareKey& k, int64_t) { AssertLockHeld(cs); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 550e66de47..f336494a18 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -442,58 +442,6 @@ static CNodeState *State(NodeId pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return &it->second; } -/** - * Data structure for an individual peer. This struct is not protected by - * cs_main since it does not contain validation-critical data. - * - * Memory is owned by shared pointers and this object is destructed when - * the refcount drops to zero. - * - * TODO: move most members from CNodeState to this structure. - * TODO: move remaining application-layer data members from CNode to this structure. - */ -struct Peer { - /** Same id as the CNode object for this peer */ - const NodeId m_id{0}; - - /** Protects misbehavior data members */ - Mutex m_misbehavior_mutex; - /** Accumulated misbehavior score for this peer */ - int m_misbehavior_score GUARDED_BY(m_misbehavior_mutex){0}; - /** Whether this peer should be disconnected and marked as discouraged (unless it has the noban permission). */ - bool m_should_discourage GUARDED_BY(m_misbehavior_mutex){false}; - - /** Set of txids to reconsider once their parent transactions have been accepted **/ - std::set m_orphan_work_set GUARDED_BY(g_cs_orphans); - - /** Protects m_getdata_requests **/ - Mutex m_getdata_requests_mutex; - /** Work queue of items requested by this peer **/ - std::deque m_getdata_requests GUARDED_BY(m_getdata_requests_mutex); - - explicit Peer(NodeId id) : m_id(id) {} -}; - -using PeerRef = std::shared_ptr; - -/** - * Map of all Peer objects, keyed by peer id. This map is protected - * by the global g_peer_mutex. Once a shared pointer reference is - * taken, the lock may be released. Individual fields are protected by - * their own locks. - */ -Mutex g_peer_mutex; -static std::map g_peer_map GUARDED_BY(g_peer_mutex); - -/** Get a shared pointer to the Peer object. - * May return nullptr if the Peer object can't be found. */ -static PeerRef GetPeerRef(NodeId id) -{ - LOCK(g_peer_mutex); - auto it = g_peer_map.find(id); - return it != g_peer_map.end() ? it->second : nullptr; -} - static void UpdatePreferredDownload(const CNode& node, CNodeState* state) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { nPreferredDownload -= state->fPreferredDownload; @@ -954,8 +902,8 @@ void PeerManager::InitializeNode(CNode *pnode) { } { PeerRef peer = std::make_shared(nodeid); - LOCK(g_peer_mutex); - g_peer_map.emplace_hint(g_peer_map.end(), nodeid, std::move(peer)); + LOCK(m_peer_mutex); + m_peer_map.emplace_hint(m_peer_map.end(), nodeid, std::move(peer)); } if (!pnode->fInbound) PushNodeVersion(*pnode, m_connman, GetTime()); @@ -986,11 +934,9 @@ void PeerManager::FinalizeNode(const CNode& node) { LOCK(cs_main); { { - PeerRef peer = GetPeerRef(nodeid); + PeerRef peer = RemovePeer(nodeid); assert(peer != nullptr); misbehavior = WITH_LOCK(peer->m_misbehavior_mutex, return peer->m_misbehavior_score); - LOCK(g_peer_mutex); - g_peer_map.erase(nodeid); } CNodeState *state = State(nodeid); assert(state != nullptr); @@ -1029,7 +975,26 @@ void PeerManager::FinalizeNode(const CNode& node) { LogPrint(BCLog::NET, "Cleared nodestate for peer=%d\n", nodeid); } -bool GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats) { +PeerRef PeerManager::GetPeerRef(NodeId id) const +{ + LOCK(m_peer_mutex); + auto it = m_peer_map.find(id); + return it != m_peer_map.end() ? it->second : nullptr; +} + +PeerRef PeerManager::RemovePeer(NodeId id) +{ + PeerRef ret; + LOCK(m_peer_mutex); + auto it = m_peer_map.find(id); + if (it != m_peer_map.end()) { + ret = std::move(it->second); + m_peer_map.erase(it); + } + return ret; +} + +bool PeerManager::GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats) { { LOCK(cs_main); CNodeState* state = State(nodeid); @@ -1213,7 +1178,7 @@ void PeerManager::Misbehaving(const NodeId pnode, const int howmuch, const std:: } } -bool IsBanned(NodeId pnode) +bool PeerManager::IsBanned(NodeId pnode) { PeerRef peer = GetPeerRef(pnode); if (peer == nullptr) diff --git a/src/net_processing.h b/src/net_processing.h index 53d259c601..247fbf071a 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -29,6 +29,47 @@ static const unsigned int DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN = 100; static const bool DEFAULT_PEERBLOOMFILTERS = true; static const bool DEFAULT_PEERBLOCKFILTERS = false; +struct CNodeStateStats { + int m_misbehavior_score = 0; + int nSyncHeight = -1; + int nCommonHeight = -1; + std::vector vHeightInFlight; +}; + +/** + * Data structure for an individual peer. This struct is not protected by + * cs_main since it does not contain validation-critical data. + * + * Memory is owned by shared pointers and this object is destructed when + * the refcount drops to zero. + * + * TODO: move most members from CNodeState to this structure. + * TODO: move remaining application-layer data members from CNode to this structure. + */ +struct Peer { + /** Same id as the CNode object for this peer */ + const NodeId m_id{0}; + + /** Protects misbehavior data members */ + Mutex m_misbehavior_mutex; + /** Accumulated misbehavior score for this peer */ + int m_misbehavior_score GUARDED_BY(m_misbehavior_mutex){0}; + /** Whether this peer should be disconnected and marked as discouraged (unless it has the noban permission). */ + bool m_should_discourage GUARDED_BY(m_misbehavior_mutex){false}; + + /** Set of txids to reconsider once their parent transactions have been accepted **/ + std::set m_orphan_work_set GUARDED_BY(g_cs_orphans); + + /** Protects m_getdata_requests **/ + Mutex m_getdata_requests_mutex; + /** Work queue of items requested by this peer **/ + std::deque m_getdata_requests GUARDED_BY(m_getdata_requests_mutex); + + explicit Peer(NodeId id) : m_id(id) {} +}; + +using PeerRef = std::shared_ptr; + class PeerManager final : public CValidationInterface, public NetEventsInterface { public: PeerManager(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman, BanMan* banman, CScheduler &scheduler, @@ -87,7 +128,20 @@ public: */ void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message = ""); + /** Get statistics from node state */ + bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats); + + bool IsBanned(NodeId pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + private: + /** Get a shared pointer to the Peer object. + * May return an empty shared_ptr if the Peer object can't be found. */ + PeerRef GetPeerRef(NodeId id) const; + + /** Get a shared pointer to the Peer object and remove it from m_peer_map. + * May return an empty shared_ptr if the Peer object can't be found. */ + PeerRef RemovePeer(NodeId id); + /** * Potentially mark a node discouraged based on the contents of a BlockValidationState object * @@ -134,19 +188,18 @@ private: std::unique_ptr& m_llmq_ctx; int64_t m_stale_tip_check_time; //!< Next time to check for stale tip -}; -struct CNodeStateStats { - int m_misbehavior_score = 0; - int nSyncHeight = -1; - int nCommonHeight = -1; - std::vector vHeightInFlight; + /** Protects m_peer_map */ + mutable Mutex m_peer_mutex; + /** + * Map of all Peer objects, keyed by peer id. This map is protected + * by the m_peer_mutex. Once a shared pointer reference is + * taken, the lock may be released. Individual fields are protected by + * their own locks. + */ + std::map m_peer_map GUARDED_BY(m_peer_mutex); }; -/** Get statistics from node state */ -bool GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats); -bool IsBanned(NodeId nodeid) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - void EraseObjectRequest(NodeId nodeId, const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main); void RequestObject(NodeId nodeId, const CInv& inv, std::chrono::microseconds current_time, bool fForce=false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); size_t GetRequestedObjectCount(NodeId nodeId) EXCLUSIVE_LOCKS_REQUIRED(cs_main); diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 27e86c6d68..963d08a50f 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -253,11 +253,13 @@ public: } // Try to retrieve the CNodeStateStats for each node. - TRY_LOCK(::cs_main, lockMain); - if (lockMain) { - for (auto& node_stats : stats) { - std::get<1>(node_stats) = - GetNodeStateStats(std::get<0>(node_stats).nodeid, std::get<2>(node_stats)); + if (m_context->peerman) { + TRY_LOCK(::cs_main, lockMain); + if (lockMain) { + for (auto& node_stats : stats) { + std::get<1>(node_stats) = + m_context->peerman->GetNodeStateStats(std::get<0>(node_stats).nodeid, std::get<2>(node_stats)); + } } } return true; diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 55665cce79..058fe265e5 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -148,8 +148,9 @@ static UniValue getpeerinfo(const JSONRPCRequest& request) }.Check(request); NodeContext& node = EnsureNodeContext(request.context); - if(!node.connman) + if(!node.connman || !node.peerman) { throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); + } std::vector vstats; node.connman->GetNodeStats(vstats); @@ -159,7 +160,7 @@ static UniValue getpeerinfo(const JSONRPCRequest& request) for (const CNodeStats& stats : vstats) { UniValue obj(UniValue::VOBJ); CNodeStateStats statestats; - bool fStateStats = GetNodeStateStats(stats.nodeid, statestats); + bool fStateStats = node.peerman->GetNodeStateStats(stats.nodeid, statestats); obj.pushKV("id", stats.nodeid); obj.pushKV("addr", stats.addrName); if (stats.addrBind.IsValid()) {