Correct `msgs_sent_since_pong` tracking for gossip forwards
What the developer wrote
Authored by Matt Corallo
Correct `msgs_sent_since_pong` tracking for gossip forwards
When we forward gossip messages, we use the already-encoded copy
we have, pushing it onto the `gossip_broadcast_buffer`. When we
have free socket buffer space, and there are no non-gossip messages
pending, we'll remove it from the `gossip_broadcast_buffer` and
push it onto the normal `pending_outbound_buffer` socket queue.
Separately, we use `msgs_sent_since_pong` to ensure that our peer
is being responsive and our messages are getting through with
minimal latency. After we send 32 messages (if we've already
received the `pong` for the last `ping` wensent), we send an extra
`ping` and will only queue up an additional 32 forwarded gossip
messages before we start dropping forwarded gossip on the floor.
Previously, we (arguably) incorrectly incremented
`msgs_sent_since_pong` when we pushed a message onto the
`gossip_broadcast_buffer`, not when it was actually queued up in
our `pending_outbound_buffer` immediate socket queue. This is
fairly strange - the point of `msgs_sent_since_pong` is to keep
peer latency low, we already independently enforce memory limits to
ensure we drop forwarded gossip messages if a peer's message queues
have grown too large. This means we potentially disconnect a peer
for not draining the backlog forwarded-gossip socket queue fast
enough, rather than simply dropping gossip.
While this shouldn't be a big deal in practice, I do see (mostly
Tor) peers disconnect on my node occasionally, and while its likely
due to Tor circuits hanging and a disconnect is needed, the
incorrect tracking here makes state analysis difficult as there are
nearly always a flow of enough gossip messages to "send to the
peer". Its also possible that this allows incredibly low bandwidth
connections to stay afloat more durably.
What changed, and why it matters
This change fixes how a Lightning node counts messages it has sent to a peer. Previously, the node counted forwarded gossip messages as 'sent' when they were merely placed in a broadcast buffer, even though they had not yet been written to the actual network socket. This could cause the node to think the peer was slow or unresponsive and disconnect it prematurely, especially for peers on slow networks like Tor. The fix moves the count to the point where the message is actually queued for sending on the socket, so the node only disconnects peers for genuine backpressure, not for buffered gossip waiting to be sent.
Review and merge. This is a correctness fix for peer liveness/backpressure accounting. Operators running nodes with many Tor or low-bandwidth peers should benefit from fewer spurious disconnects. No immediate incident response is required; the issue is a reliability bug rather than an exploitable vulnerability.
Security signals we found
Incorrect backpressure accounting could lead to premature peer disconnection
Fixes mismatch between message counter and actual socket transmission
May improve stability of low-bandwidth and Tor peer connections
No cryptographic, memory-safety, or authentication issue present
Evidence from the diff
The patch moves the msgs_sent_since_pong increment from enqueue_encoded_gossip_broadcast (where gossip is appended to gossip_broadcast_buffer) to the point in do_write_buffer where buffered gossip is popped from gossip_broadcast_buffer and pushed onto pending_outbound_buffer for immediate socket transmission. The helper enqueue_encoded_gossip_broadcast is removed and its callers now push directly to gossip_broadcast_buffer. This aligns the ping/pong backpressure counter with actual socket queueing rather than with gossip buffering, preventing premature ping-driven disconnects when the broadcast buffer is large but the socket is healthy.
Changed components
lightning/src/ln/peer_handler.rsPeer::msgs_sent_since_pong countergossip_broadcast_buffer handlingping/pong liveness logicInspect captured patch +7 / −18
diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs
index 02390b8..8299626 100644
--- a/lightning/src/ln/peer_handler.rs
+++ b/lightning/src/ln/peer_handler.rs
@@ -1551,6 +1551,7 @@ where
}
if peer.should_buffer_gossip_broadcast() {
if let Some(msg) = peer.gossip_broadcast_buffer.pop_front() {
+ peer.msgs_sent_since_pong += 1;
peer.pending_outbound_buffer
.push_back(peer.channel_encryptor.encrypt_buffer(msg));
}
@@ -1714,12 +1715,6 @@ where
peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(message));
}
- /// Append a message to a peer's pending outbound/write gossip broadcast buffer
- fn enqueue_encoded_gossip_broadcast(&self, peer: &mut Peer, encoded_message: MessageBuf) {
- peer.msgs_sent_since_pong += 1;
- peer.gossip_broadcast_buffer.push_back(encoded_message);
- }
-
fn do_read_event(
&self, peer_descriptor: &mut Descriptor, data: &[u8],
) -> Result<bool, PeerHandleError> {
@@ -2689,10 +2684,8 @@ where
{
continue;
}
- self.enqueue_encoded_gossip_broadcast(
- &mut *peer,
- MessageBuf::from_encoded(&encoded_msg),
- );
+ let encoded_message = MessageBuf::from_encoded(&encoded_msg);
+ peer.gossip_broadcast_buffer.push_back(encoded_message);
}
},
wire::Message::NodeAnnouncement(ref msg) => {
@@ -2733,10 +2726,8 @@ where
{
continue;
}
- self.enqueue_encoded_gossip_broadcast(
- &mut *peer,
- MessageBuf::from_encoded(&encoded_msg),
- );
+ let encoded_message = MessageBuf::from_encoded(&encoded_msg);
+ peer.gossip_broadcast_buffer.push_back(encoded_message);
}
},
wire::Message::ChannelUpdate(ref msg) => {
@@ -2772,10 +2763,8 @@ where
{
continue;
}
- self.enqueue_encoded_gossip_broadcast(
- &mut *peer,
- MessageBuf::from_encoded(&encoded_msg),
- );
+ let encoded_message = MessageBuf::from_encoded(&encoded_msg);
+ peer.gossip_broadcast_buffer.push_back(encoded_message);
}
},
_ => {
Why this scored 35/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.