diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -109,12 +109,15 @@ } // 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/net_processing.h b/src/net_processing.h --- a/src/net_processing.h +++ b/src/net_processing.h @@ -42,6 +42,52 @@ * to the discouragement filter. */ static const int DISCOURAGEMENT_THRESHOLD{100}; +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: @@ -144,7 +190,22 @@ */ void UpdateAvalancheStatistics() const; + /** Get statistics from node state */ + bool GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats); + 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); + // overloaded variant of above to operate on CNode*s void Misbehaving(const CNode &node, int howmuch, const std::string &message) { @@ -233,17 +294,18 @@ //! Next time to check for stale tip int64_t m_stale_tip_check_time; -}; -struct CNodeStateStats { - 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); - /** Relay transaction to every node */ void RelayTransaction(const TxId &txid, const CConnman &connman); diff --git a/src/net_processing.cpp b/src/net_processing.cpp --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -581,64 +581,6 @@ 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); - - 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 bool isPreferredDownloadPeer(const CNode &pfrom) { LOCK(cs_main); const CNodeState *state = State(pfrom.GetId()); @@ -1116,8 +1058,8 @@ } { 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->IsInboundConn()) { PushNodeVersion(config, *pnode, m_connman, GetTime()); @@ -1177,12 +1119,12 @@ LOCK(cs_main); int misbehavior{0}; { - 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); + LOCK(m_peer_mutex); + m_peer_map.erase(nodeid); } CNodeState *state = State(nodeid); assert(state != nullptr); @@ -1227,7 +1169,24 @@ 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); diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -209,7 +209,7 @@ [&](const RPCHelpMan &self, const Config &config, const JSONRPCRequest &request) -> UniValue { 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"); @@ -223,7 +223,8 @@ 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()) {