net: notice a peer that has gone quiet, and stay audible to peers that check - #55
Merged
Conversation
…t check A connection whose path dies silently -- NAT drops an idle mapping, a relay restarts, a route changes -- never delivers FIN. Nothing becomes readable, no error is raised, and the node simply stops hearing from that peer while the socket looks perfectly healthy. #51 bounded this for the rendezvous registration; the connection that comes out of the rendezvous had the same hole, and so did every ordinary peer. Stamp nLastRecv and nLastSend on the socket path and act on them: - nothing at all within 60s of connecting, disconnect. In testing this fired twice per run against dead .btf descriptors handed out by the relay, which is where the ghosts have been coming from. - queued to send for 10 minutes with nothing going out, disconnect. - nothing received for 30 minutes, disconnect. A healthy peer relays a block inventory about every two minutes, so this is fifteen intervals of silence. Bitcoin used ninety minutes against ten-minute blocks. Disconnecting is the whole cure: reconnection already works, and a peer we cannot hear is worth what no peer is worth. A node with nothing to relay would now look dead to a peer applying the same rule, so send "ping" after ten minutes of having said nothing. It carries no payload and expects no answer -- arriving is the entire content, because the receive path has already stamped nLastRecv by the time it is parsed. Requiring an answer would have read every node predating this change as dead, since those ignore unknown commands rather than replying. Verified on the live network: synced 0 to 3790 with five peers and no spurious disconnects, then again with the interval cut to 20s, which put 54 pings on the wire with no peer dropping us and no exceptions.
Bitflash-sh
added a commit
that referenced
this pull request
Jul 30, 2026
…#56) A node running this code cannot tell "in sync with everyone" from "not hearing anyone". The version message carries no height -- 0.1.0 never had the field and this fork inherited the gap -- so the only evidence of a problem is a number on screen that stopped moving, which looks the same as a network with nothing to report. It is the structural reason a broken node here has always been indistinguishable from an idle one. Append nBestHeight to the version message and keep what a peer announced in CNode::nStartingHeight. The field goes last, so a node that predates it stops reading after addr and treats the rest as trailing bytes, which cost it one log line. Reading it is guarded by vRecv.empty() and never required: peers that do not send it keep nStartingHeight at -1 and are left out of the median. Every release so far sends the short form. GetPeerMedianHeight() takes the median rather than the maximum so a single peer claiming an absurd height cannot move it. Then use it. After five minutes more than one block behind the peer median, the node says so in the log -- not behind a debug category, because an operator who already enabled -debug is not the one who needs telling -- and the GUI carries the same sentence in its status area. "8 blocks behind for 27 minutes" is something a user can act on; a frozen counter is not. Verified against two nodes on the live network: the one starting from an empty datadir read its peer at 3797, reported 2636 then 1274 then 433 blocks behind as it caught up, and went quiet on its own once level. The other logged "peer height = -1" for the older nodes it met on the real network, which is the compatibility path doing its job. Known limit: nStartingHeight is a snapshot from handshake time and goes stale on a long-lived connection, so this catches a node that is receiving something but not blocks, not one that has gone completely deaf. The inactivity timeout from #55 covers that case and refreshes these heights when it forces the reconnect.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows #51. That one bounded the wait for a dial to arrive at the rendezvous; this one bounds the connection that comes out of it, and every ordinary peer besides, which had exactly the same hole.
The hole
A connection whose network path dies silently — NAT drops an idle mapping, a relay restarts, a route changes — never delivers FIN. Nothing becomes readable, no error is raised anywhere, and the node just stops hearing from that peer while the socket looks perfectly healthy from the inside. It is the same shape as every other bug this project has had: a broken node is indistinguishable from an idle one.
What this does
nLastRecvandnLastSendare stamped on the socket path, and three rules act on them:Thirty minutes is fifteen block intervals at the two-minute target. Bitcoin used ninety against ten-minute blocks.
Disconnecting is the entire cure — reconnection already works, and a peer we cannot hear is worth what no peer is worth.
The ping, and why it does not expect an answer
A node with nothing to relay would now look dead to a peer applying the same rule, so it sends
pingafter ten minutes of having said nothing.The message carries no payload and gets no reply. Arriving is the whole content: by the time it is parsed, the receive path has already stamped
nLastRecv, which is the only thing the sender wanted. Requiring a reply would have read every node predating this change as dead, because those ignore unknown commands rather than answering them.Nothing here strands a node that does not upgrade.
ProcessMessagealready ends in// Ignore unknown commands for extensibility, and trailing bytes only produce a log line.Testing
On the live network, from an empty datadir:
recv=0 send=1— dead.btfdescriptors the relay handed out. That is the ghost problem showing up as a measurement rather than a guessNot in this change
Satoshi did all of the above plus a watchdog that restarts
ThreadSocketHandlerif the thread itself hangs, in the same commit (70e79525c). That is a different mechanism — it monitors the thread, not the connections — and on Windows it runs into_endthread()not unwinding, which #52 already had to work around. Worth doing separately, on its own evidence.