← Watch feed
move-only: Extract ProcessPong() helper
Public commit record
60/100 · Adequate
What the developer wrote
Authored by MarcoFalke
move-only: Extract ProcessPong() helper
This commit can be reviewed with the git options:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
✓ Descriptive subject✓ Names a concrete action or component✓ Provides an explanatory body
What changed, and why it matters
This commit simply moves the existing code that handles PONG network messages into a new helper function called ProcessPong(). There are no functional changes, no bug fixes, and no security changes. It is a code-cleanup refactor to make the large ProcessMessage() function easier to read and maintain.
Recommended action
No security action needed. Treat as ordinary refactoring code review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change is a move-only extraction. The exact same PONG handling logic (nonce parsing, ping timing, nonce mismatch handling, logging, and private-broadcast disconnect logic) is relocated from inside PeerManagerImpl::ProcessMessage() into a new private method PeerManagerImpl::ProcessPong(). The commit message explicitly states ‘move-only’ and recommends using git –color-moved to verify. The diff confirms the logic is copied verbatim with only indentation and parameter-passing adjustments.
Changed components
src/net_processing.cppP2P PONG message handlingInspect captured patch +63 / −57
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 6d35db93..69304784 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
Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.