net_processing: modernize PushNodeVersion()
What changed, and why it matters
This is a straightforward code cleanup in Bitcoin Core's network handshake routine. It renames local variables, splits declarations from assignments, and merges two nearly identical log lines into one. The commit message explicitly calls it a non-functional change, and the diff shows no altered behavior, no new network data, and no changed security checks.
No security action needed; treat as ordinary refactoring review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
PeerManagerImpl::PushNodeVersion() is refactored only: variables are declared then assigned, names are made more consistent (my_services, my_time, your_services, your_addr, etc.), the VERSION message construction uses the same values and same conditional logic as before, and the two LogDebug branches are collapsed into a single call with an inline conditional string. No protocol values, serialization, or access-control decisions changed.
Changed components
src/net_processing.cppPeerManagerImpl::PushNodeVersion()Inspect captured patch +37 / −21
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 20640096..9195b494 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1528,27 +1528,43 @@ void PeerManagerImpl::FindNextBlocks(std::vector<const CBlockIndex*>& vBlocks, c
void PeerManagerImpl::PushNodeVersion(CNode& pnode, const Peer& peer)
{
- uint64_t my_services{peer.m_our_services};
- const int64_t nTime{count_seconds(GetTime<std::chrono::seconds>())};
- uint64_t nonce = pnode.GetLocalNonce();
- const int nNodeStartingHeight{m_best_height};
- NodeId nodeid = pnode.GetId();
- CAddress addr = pnode.addr;
-
- CService addr_you = addr.IsRoutable() && !IsProxy(addr) && addr.IsAddrV1Compatible() ? addr : CService();
- uint64_t your_services{addr.nServices};
-
- const bool tx_relay{!RejectIncomingTxs(pnode)};
- MakeAndPushMessage(pnode, NetMsgType::VERSION, PROTOCOL_VERSION, my_services, nTime,
- your_services, CNetAddr::V1(addr_you), // Together the pre-version-31402 serialization of CAddress "addrYou" (without nTime)
- my_services, CNetAddr::V1(CService{}), // Together the pre-version-31402 serialization of CAddress "addrMe" (without nTime)
- nonce, strSubVersion, nNodeStartingHeight, tx_relay);
-
- if (fLogIPs) {
- LogDebug(BCLog::NET, "send version message: version %d, blocks=%d, them=%s, txrelay=%d, peer=%d\n", PROTOCOL_VERSION, nNodeStartingHeight, addr_you.ToStringAddrPort(), tx_relay, nodeid);
- } else {
- LogDebug(BCLog::NET, "send version message: version %d, blocks=%d, txrelay=%d, peer=%d\n", PROTOCOL_VERSION, nNodeStartingHeight, tx_relay, nodeid);
- }
+ uint64_t my_services;
+ int64_t my_time;
+ uint64_t your_services;
+ CService your_addr;
+ std::string my_user_agent;
+ int my_height;
+ bool my_tx_relay;
+
+ const CAddress& addr{pnode.addr};
+ my_services = peer.m_our_services;
+ my_time = count_seconds(GetTime<std::chrono::seconds>());
+ your_services = addr.nServices;
+ your_addr = addr.IsRoutable() && !IsProxy(addr) && addr.IsAddrV1Compatible() ? CService{addr} : CService{};
+ my_user_agent = strSubVersion;
+ my_height = m_best_height;
+ my_tx_relay = !RejectIncomingTxs(pnode);
+
+ MakeAndPushMessage(
+ pnode,
+ NetMsgType::VERSION,
+ PROTOCOL_VERSION,
+ my_services,
+ my_time,
+ // your_services + CNetAddr::V1(your_addr) is the pre-version-31402 serialization of your_addr (without nTime)
+ your_services, CNetAddr::V1(your_addr),
+ // same, for a dummy address
+ my_services, CNetAddr::V1(CService{}),
+ pnode.GetLocalNonce(),
+ my_user_agent,
+ my_height,
+ my_tx_relay);
+
+ LogDebug(
+ BCLog::NET, "send version message: version=%d, blocks=%d%s, txrelay=%d, peer=%d\n",
+ PROTOCOL_VERSION, my_height,
+ fLogIPs ? strprintf(", them=%s", your_addr.ToStringAddrPort()) : "",
+ my_tx_relay, pnode.GetId());
}
void PeerManagerImpl::UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds)
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.