Skip to content

Commit 56cdf30

Browse files
committed
Give peers one timer tick to finish handshake before disconnecting
This ensures we don't let a hung connection stick around forever if the peer never completes the initial handshake. This also resolves a race where, on receiving a second connection from a peer, we may reset their_node_id to None to prevent sending messages even though the `channel_encryptor` `is_ready_for_encryption()`. Sending pings only checks the `channel_encryptor` status, not `their_node_id` resulting in an `unwrap` on `None` in `enqueue_message`.
1 parent 69ba24a commit 56cdf30

File tree

1 file changed

+15
-9
lines changed

1 file changed

+15
-9
lines changed

lightning/src/ln/peer_handler.rs

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -857,6 +857,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref, CMH: Deref> P
857857
let features = InitFeatures::known();
858858
let resp = msgs::Init { features };
859859
self.enqueue_message(peer, &resp);
860+
peer.awaiting_pong_tick_intervals = 0;
860861
},
861862
NextNoiseStep::ActThree => {
862863
let their_node_id = try_potential_handleerror!(peer.channel_encryptor.process_act_three(&peer.pending_read_buffer[..]));
@@ -867,6 +868,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref, CMH: Deref> P
867868
let features = InitFeatures::known();
868869
let resp = msgs::Init { features };
869870
self.enqueue_message(peer, &resp);
871+
peer.awaiting_pong_tick_intervals = 0;
870872
},
871873
NextNoiseStep::NoiseComplete => {
872874
if peer.pending_read_is_header {
@@ -1530,12 +1532,20 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref, CMH: Deref> P
15301532
let peer_count = peers.len();
15311533

15321534
peers.retain(|descriptor, peer| {
1533-
if !peer.channel_encryptor.is_ready_for_encryption() {
1534-
// The peer needs to complete its handshake before we can exchange messages
1535-
return true;
1535+
let mut do_disconnect_peer = false;
1536+
if !peer.channel_encryptor.is_ready_for_encryption() || peer.their_node_id.is_none() {
1537+
// The peer needs to complete its handshake before we can exchange messages. We
1538+
// give peers one timer tick to complet handshake.
1539+
if peer.awaiting_pong_tick_intervals != 0 {
1540+
do_disconnect_peer = true;
1541+
} else {
1542+
peer.awaiting_pong_tick_intervals = 1;
1543+
return true;
1544+
}
15361545
}
15371546

1538-
if (peer.awaiting_pong_tick_intervals > 0 && !peer.received_message_since_timer_tick)
1547+
if do_disconnect_peer
1548+
|| (peer.awaiting_pong_tick_intervals > 0 && !peer.received_message_since_timer_tick)
15391549
|| peer.awaiting_pong_tick_intervals as u64 >
15401550
MAX_BUFFER_DRAIN_TICK_INTERVALS_PER_PEER as u64 * peer_count as u64
15411551
{
@@ -1546,11 +1556,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref, CMH: Deref> P
15461556
node_id_to_descriptor.remove(&node_id);
15471557
self.message_handler.chan_handler.peer_disconnected(&node_id, false);
15481558
}
1549-
None => {
1550-
// This can't actually happen as we should have hit
1551-
// is_ready_for_encryption() previously on this same peer.
1552-
unreachable!();
1553-
},
1559+
None => {},
15541560
}
15551561
return false;
15561562
}

0 commit comments

Comments
 (0)