From fa5ab0220e02377c3c855042ecdf1f5f950d0965 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Fri, 20 Mar 2026 10:59:18 +0100 Subject: [PATCH 1/2] move-only: Extract ProcessPong() helper This commit can be reviewed with the git options: --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space --- src/net_processing.cpp | 120 +++++++++++++++++++++-------------------- 1 file changed, 63 insertions(+), 57 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6d35db931a5..6930478421b 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1075,6 +1075,8 @@ private: */ void ProcessGetCFCheckPt(CNode& node, Peer& peer, DataStream& vRecv); + void ProcessPong(CNode& pfrom, Peer& peer, NodeClock::time_point ping_end, DataStream& vRecv); + /** Checks if address relay is permitted with peer. If needed, initializes * the m_addr_known bloom filter and sets m_addr_relay_enabled to true. * @@ -4902,63 +4904,7 @@ void PeerManagerImpl::ProcessMessage(Peer& peer, CNode& pfrom, const std::string } if (msg_type == NetMsgType::PONG) { - const auto ping_end{time_received}; - uint64_t nonce = 0; - size_t nAvail = vRecv.in_avail(); - bool bPingFinished = false; - std::string sProblem; - - if (nAvail >= sizeof(nonce)) { - vRecv >> nonce; - - // Only process pong message if there is an outstanding ping (old ping without nonce should never pong) - if (peer.m_ping_nonce_sent != 0) { - if (nonce == peer.m_ping_nonce_sent) { - // Matching pong received, this ping is no longer outstanding - bPingFinished = true; - const auto ping_time = ping_end - peer.m_ping_start.load(); - if (ping_time.count() >= 0) { - // Let connman know about this successful ping-pong - pfrom.PongReceived(ping_time); - if (pfrom.IsPrivateBroadcastConn()) { - m_tx_for_private_broadcast.NodeConfirmedReception(pfrom.GetId()); - LogDebug(BCLog::PRIVBROADCAST, "Got a PONG (the transaction will probably reach the network), marking for disconnect, %s", - pfrom.LogPeer()); - pfrom.fDisconnect = true; - } - } else { - // This should never happen - sProblem = "Timing mishap"; - } - } else { - // Nonce mismatches are normal when pings are overlapping - sProblem = "Nonce mismatch"; - if (nonce == 0) { - // This is most likely a bug in another implementation somewhere; cancel this ping - bPingFinished = true; - sProblem = "Nonce zero"; - } - } - } else { - sProblem = "Unsolicited pong without ping"; - } - } else { - // This is most likely a bug in another implementation somewhere; cancel this ping - bPingFinished = true; - sProblem = "Short payload"; - } - - if (!(sProblem.empty())) { - LogDebug(BCLog::NET, "pong peer=%d: %s, %x expected, %x received, %u bytes\n", - pfrom.GetId(), - sProblem, - peer.m_ping_nonce_sent, - nonce, - nAvail); - } - if (bPingFinished) { - peer.m_ping_nonce_sent = 0; - } + ProcessPong(pfrom, peer, /*ping_end=*/time_received, vRecv); return; } @@ -5608,6 +5554,66 @@ bool PeerManagerImpl::RejectIncomingTxs(const CNode& peer) const return false; } +void PeerManagerImpl::ProcessPong(CNode& pfrom, Peer& peer, const NodeClock::time_point ping_end, DataStream& vRecv) +{ + uint64_t nonce = 0; + size_t nAvail = vRecv.in_avail(); + bool bPingFinished = false; + std::string sProblem; + + if (nAvail >= sizeof(nonce)) { + vRecv >> nonce; + + // Only process pong message if there is an outstanding ping (old ping without nonce should never pong) + if (peer.m_ping_nonce_sent != 0) { + if (nonce == peer.m_ping_nonce_sent) { + // Matching pong received, this ping is no longer outstanding + bPingFinished = true; + const auto ping_time = ping_end - peer.m_ping_start.load(); + if (ping_time.count() >= 0) { + // Let connman know about this successful ping-pong + pfrom.PongReceived(ping_time); + if (pfrom.IsPrivateBroadcastConn()) { + m_tx_for_private_broadcast.NodeConfirmedReception(pfrom.GetId()); + LogDebug(BCLog::PRIVBROADCAST, "Got a PONG (the transaction will probably reach the network), marking for disconnect, %s", + pfrom.LogPeer()); + pfrom.fDisconnect = true; + } + } else { + // This should never happen + sProblem = "Timing mishap"; + } + } else { + // Nonce mismatches are normal when pings are overlapping + sProblem = "Nonce mismatch"; + if (nonce == 0) { + // This is most likely a bug in another implementation somewhere; cancel this ping + bPingFinished = true; + sProblem = "Nonce zero"; + } + } + } else { + sProblem = "Unsolicited pong without ping"; + } + } else { + // This is most likely a bug in another implementation somewhere; cancel this ping + bPingFinished = true; + sProblem = "Short payload"; + } + + if (!(sProblem.empty())) { + LogDebug(BCLog::NET, "pong peer=%d: %s, %x expected, %x received, %u bytes\n", + pfrom.GetId(), + sProblem, + peer.m_ping_nonce_sent, + nonce, + nAvail); + } + if (bPingFinished) { + peer.m_ping_nonce_sent = 0; + } +} + bool PeerManagerImpl::SetupAddressRelay(const CNode& node, Peer& peer) { // We don't participate in addr relay with outbound block-relay-only From fa204100e1456b14978071064112fce2acc527a6 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Fri, 24 Apr 2026 12:36:02 +0200 Subject: [PATCH 2/2] streams: Remove confusing DataStream::in_avail() The alias of the size() method is confusing, because: * It claims to be part of the Bitcoin Core stream subset (streams interface), but this is not used by any other stream interface. Mostly the `write(std::span)` and `read(std::span)` define the stream interface. * It casts the size_t to i32, but the only place that calls the function casts that back to size_t. * Providing this alias for size() without a proper reason is confusing. Fix all issues by removing it and using the size() method. --- src/net_processing.cpp | 2 +- src/streams.h | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6930478421b..b870df66c1e 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -5557,7 +5557,7 @@ bool PeerManagerImpl::RejectIncomingTxs(const CNode& peer) const void PeerManagerImpl::ProcessPong(CNode& pfrom, Peer& peer, const NodeClock::time_point ping_end, DataStream& vRecv) { uint64_t nonce = 0; - size_t nAvail = vRecv.in_avail(); + const size_t nAvail{vRecv.size()}; bool bPingFinished = false; std::string sProblem; diff --git a/src/streams.h b/src/streams.h index 6ef9d164494..96cea55eb49 100644 --- a/src/streams.h +++ b/src/streams.h @@ -188,7 +188,6 @@ public: return std::string{UCharCast(data()), UCharCast(data() + size())}; } - // // Vector subset // @@ -209,8 +208,6 @@ public: // // Stream subset // - int in_avail() const { return size(); } - void read(std::span dst) { if (dst.size() == 0) return;