Ensure we call `send_data` when we need to pause/unpause reads
What the developer wrote
Authored by Matt Corallo
Ensure we call `send_data` when we need to pause/unpause reads
In the previous commit, we moved the `send_data` `resume_read` flag
to also indicate that we should pause if its unset. This should
work as we mostly only set the flag when we're sending but may
cause us to fail to pause if we are blocked on gossip validation
but `awaiting_write_event` wasn't set as we had previously failed
to fully flush a buffer (which no longer implies read-pause).
Here we make this logic much more robust by ensuring we always make
at least one `send_data` call in `do_attempt_write_data` if we
need to pause read (or unpause read).
What changed, and why it matters
This commit fixes a logic bug in how Lightning Dev Kit tells the network layer to pause or resume reading data from a peer. Previously, after a partial buffer flush, the code could fail to tell the socket driver to pause reads even though it was busy validating gossip messages. The fix ensures at least one 'send_data' call is made whenever the desired read-pause state does not match the last state we told the driver about. This is a robustness fix in backpressure handling, not a clear-cut remote exploit.
Treat as a moderate-priority bugfix. Reviewers should verify that `sent_pause_read` is correctly updated on every `send_data` path and that the new `force_one_write` condition cannot cause spurious zero-byte writes in high-throughput scenarios. No immediate security advisory appears warranted based solely on this diff, but downstream users should include it in their next update.
Security signals we found
Backpressure signaling mismatch between protocol layer and socket driver
Potential failure to pause reads during gossip validation backlog
State tracking added to ensure pause/unpause commands are delivered
Fix references a regression introduced in the immediately preceding commit
Evidence from the diff
The change adds a sent_pause_read field to the Peer struct to remember whether the last SocketDescriptor::send_data call requested a read pause. In do_attempt_write_data, it now forces at least one send_data call when peer_should_read(peer) == peer.sent_pause_read, meaning the actual desired read state differs from the last signaled state. This corrects a case where a previous incomplete flush left awaiting_write_event false but reads should still have been paused because gossip validation was backlogged. The fix is defensive and improves state consistency between LDK’s internal backpressure logic and the socket driver’s pause/read flag.
Changed components
lightning/src/ln/peer_handler.rsPeer structdo_attempt_write_dataSocketDescriptor::send_data backpressure signalingInspect captured patch +18 / −9
diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs
index a3e7107..19b4b9e 100644
--- a/lightning/src/ln/peer_handler.rs
+++ b/lightning/src/ln/peer_handler.rs
@@ -781,6 +781,9 @@ struct Peer {
/// Note that these messages are *not* encrypted/MAC'd, and are only serialized.
gossip_broadcast_buffer: VecDeque<MessageBuf>,
awaiting_write_event: bool,
+ /// Set to true if the last call to [`SocketDescriptor::send_data`] for this peer had the
+ /// `should_read` flag unset, indicating we've told the driver to stop reading from this peer.
+ sent_pause_read: bool,
pending_read_buffer: Vec<u8>,
pending_read_buffer_pos: usize,
@@ -1440,6 +1443,7 @@ where
pending_outbound_buffer_first_msg_offset: 0,
gossip_broadcast_buffer: VecDeque::new(),
awaiting_write_event: false,
+ sent_pause_read: false,
pending_read_buffer,
pending_read_buffer_pos: 0,
@@ -1500,6 +1504,7 @@ where
pending_outbound_buffer_first_msg_offset: 0,
gossip_broadcast_buffer: VecDeque::new(),
awaiting_write_event: false,
+ sent_pause_read: false,
pending_read_buffer,
pending_read_buffer_pos: 0,
@@ -1535,10 +1540,14 @@ where
}
fn do_attempt_write_data(
- &self, descriptor: &mut Descriptor, peer: &mut Peer, force_one_write: bool,
+ &self, descriptor: &mut Descriptor, peer: &mut Peer, mut force_one_write: bool,
) {
- let mut have_written = false;
- while !peer.awaiting_write_event {
+ // If we detect that we should be reading from the peer but reads are currently paused, or
+ // vice versa, then we need to tell the socket driver to update their internal flag
+ // indicating whether or not reads are paused. Do this by forcing a write with the desired
+ // `continue_read` flag set, even if no outbound messages are currently queued.
+ force_one_write |= self.peer_should_read(peer) == peer.sent_pause_read;
+ while force_one_write || !peer.awaiting_write_event {
if peer.should_buffer_onion_message() {
if let Some((peer_node_id, _)) = peer.their_node_id {
let handler = &self.message_handler.onion_message_handler;
@@ -1606,20 +1615,20 @@ where
let should_read = self.peer_should_read(peer);
let next_buff = match peer.pending_outbound_buffer.front() {
None => {
- if force_one_write && !have_written {
- if should_read {
- let data_sent = descriptor.send_data(&[], should_read);
- debug_assert_eq!(data_sent, 0, "Can't write more than no data");
- }
+ if force_one_write {
+ let data_sent = descriptor.send_data(&[], should_read);
+ debug_assert_eq!(data_sent, 0, "Can't write more than no data");
+ peer.sent_pause_read = !should_read;
}
return;
},
Some(buff) => buff,
};
+ force_one_write = false;
let pending = &next_buff[peer.pending_outbound_buffer_first_msg_offset..];
let data_sent = descriptor.send_data(pending, should_read);
- have_written = true;
+ peer.sent_pause_read = !should_read;
peer.pending_outbound_buffer_first_msg_offset += data_sent;
if peer.pending_outbound_buffer_first_msg_offset == next_buff.len() {
peer.pending_outbound_buffer_first_msg_offset = 0;
Why this scored 49/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.