From d9079fe18dc5d81ce290876353555b51125127d1 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Mon, 20 Jul 2020 18:46:13 +0100 Subject: [net processing] Remove CNode::nServices Use Peer::m_their_services instead --- src/net.cpp | 3 +-- src/net.h | 5 +++-- src/net_processing.cpp | 7 +++---- src/net_processing.h | 1 + src/qt/rpcconsole.cpp | 2 +- src/rpc/net.cpp | 5 +++-- src/test/util/net.cpp | 2 +- 7 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 5082d6eb68..3244772ef6 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -603,7 +603,6 @@ Network CNode::ConnectedThroughNetwork() const void CNode::CopyStats(CNodeStats& stats) { stats.nodeid = this->GetId(); - X(nServices); X(addr); X(addrBind); stats.m_network = ConnectedThroughNetwork(); @@ -880,7 +879,7 @@ bool CConnman::AttemptToEvictConnection() .m_min_ping_time = node->m_min_ping_time, .m_last_block_time = node->m_last_block_time, .m_last_tx_time = node->m_last_tx_time, - .fRelevantServices = HasAllDesirableServiceFlags(node->nServices), + .fRelevantServices = node->m_has_all_wanted_services, .m_relay_txs = node->m_relays_txs.load(), .fBloomFilter = node->m_bloom_filter_loaded.load(), .nKeyedNetGroup = node->nKeyedNetGroup, diff --git a/src/net.h b/src/net.h index a986226d86..b9f6990275 100644 --- a/src/net.h +++ b/src/net.h @@ -187,7 +187,6 @@ class CNodeStats { public: NodeId nodeid; - ServiceFlags nServices; std::chrono::seconds m_last_send; std::chrono::seconds m_last_recv; std::chrono::seconds m_last_tx_time; @@ -346,7 +345,6 @@ public: std::unique_ptr m_serializer; NetPermissionFlags m_permissionFlags{NetPermissionFlags::None}; - std::atomic nServices{NODE_NONE}; /** * Socket used for communication with the node. @@ -482,6 +480,9 @@ public: // Peer selected us as (compact blocks) high-bandwidth peer (BIP152) std::atomic m_bip152_highbandwidth_from{false}; + /** Whether this peer provides all services that we want. Used for eviction decisions */ + std::atomic_bool m_has_all_wanted_services{false}; + /** Whether we should relay transactions to this peer (their version * message did not include fRelay=false and this is not a block-relay-only * connection). This only changes from false to true. It will never change diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 8889266a02..64ce34a7b8 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -223,9 +223,7 @@ struct Peer { * * TODO: remove redundant CNode::nLocalServices*/ const ServiceFlags m_our_services; - /** Services this peer offered to us. - * - * TODO: remove redundant CNode::nServices */ + /** Services this peer offered to us. */ std::atomic m_their_services{NODE_NONE}; /** Protects misbehavior data members */ @@ -1472,6 +1470,7 @@ bool PeerManagerImpl::GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) c PeerRef peer = GetPeerRef(nodeid); if (peer == nullptr) return false; + stats.their_services = peer->m_their_services; stats.m_starting_height = peer->m_starting_height; // It is common for nodes with good ping times to suddenly become lagged, // due to a new block arriving or other large transfer. @@ -2885,7 +2884,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::VERACK)); - pfrom.nServices = nServices; + pfrom.m_has_all_wanted_services = HasAllDesirableServiceFlags(nServices); peer->m_their_services = nServices; pfrom.SetAddrLocal(addrMe); { diff --git a/src/net_processing.h b/src/net_processing.h index 5fbae98c27..bcda9614d4 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -34,6 +34,7 @@ struct CNodeStateStats { uint64_t m_addr_processed = 0; uint64_t m_addr_rate_limited = 0; bool m_addr_relay_enabled{false}; + ServiceFlags their_services; }; class PeerManager : public CValidationInterface, public NetEventsInterface diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index b791fd30c4..70fccdef1c 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -1162,7 +1162,6 @@ void RPCConsole::updateDetailWidget() if (!stats->nodeStats.addrLocal.empty()) peerAddrDetails += "
" + tr("via %1").arg(QString::fromStdString(stats->nodeStats.addrLocal)); ui->peerHeading->setText(peerAddrDetails); - ui->peerServices->setText(GUIUtil::formatServicesStr(stats->nodeStats.nServices)); QString bip152_hb_settings; if (stats->nodeStats.m_bip152_highbandwidth_to) bip152_hb_settings = ts.to; if (stats->nodeStats.m_bip152_highbandwidth_from) bip152_hb_settings += (bip152_hb_settings.isEmpty() ? ts.from : QLatin1Char('/') + ts.from); @@ -1197,6 +1196,7 @@ void RPCConsole::updateDetailWidget() // This check fails for example if the lock was busy and // nodeStateStats couldn't be fetched. if (stats->fNodeStateStatsAvailable) { + ui->peerServices->setText(GUIUtil::formatServicesStr(stats->nodeStateStats.their_services)); // Sync height is init to -1 if (stats->nodeStateStats.nSyncHeight > -1) { ui->peerSyncHeight->setText(QString("%1").arg(stats->nodeStateStats.nSyncHeight)); diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index fad92629c5..27eea824bc 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -195,8 +195,9 @@ static RPCHelpMan getpeerinfo() if (stats.m_mapped_as != 0) { obj.pushKV("mapped_as", uint64_t(stats.m_mapped_as)); } - obj.pushKV("services", strprintf("%016x", stats.nServices)); - obj.pushKV("servicesnames", GetServicesNames(stats.nServices)); + ServiceFlags services{fStateStats ? statestats.their_services : ServiceFlags::NODE_NONE}; + obj.pushKV("services", strprintf("%016x", services)); + obj.pushKV("servicesnames", GetServicesNames(services)); obj.pushKV("lastsend", count_seconds(stats.m_last_send)); obj.pushKV("lastrecv", count_seconds(stats.m_last_recv)); obj.pushKV("last_transaction", count_seconds(stats.m_last_tx_time)); diff --git a/src/test/util/net.cpp b/src/test/util/net.cpp index 90a49d52d7..ab4dcb93f7 100644 --- a/src/test/util/net.cpp +++ b/src/test/util/net.cpp @@ -51,10 +51,10 @@ void ConnmanTestMsg::Handshake(CNode& node, if (node.fDisconnect) return; assert(node.nVersion == version); assert(node.GetCommonVersion() == std::min(version, PROTOCOL_VERSION)); - assert(node.nServices == remote_services); CNodeStateStats statestats; assert(peerman.GetNodeStateStats(node.GetId(), statestats)); assert(statestats.m_relay_txs == (relay_txs && !node.IsBlockOnlyConn())); + assert(statestats.their_services == remote_services); node.m_permissionFlags = permission_flags; if (successfully_connected) { CSerializedNetMsg msg_verack{mm.Make(NetMsgType::VERACK)}; -- cgit v1.2.3