Clear disconnect timer when exiting quiescence
What changed, and why it matters
This commit fixes a bug in the Lightning Dev Kit where a timer meant to disconnect unresponsive peers could fire by mistake after a splice operation completed, was aborted, or after reconnecting. The fix makes sure the timer is cleared whenever the protocol leaves its 'quiet' (quiescent) state, preventing unnecessary peer disconnections.
Review related quiescence exit paths to ensure no other direct `clear_quiescent()` calls leave the disconnect timer armed. Consider adding a lint or helper enforcement so future state exits must use `exit_quiescence()`.
Security signals we found
Spurious peer disconnection due to stale disconnect timer
State cleanup inconsistency between quiescent flag and response timer
Regression tests added for completed splice, aborted splice, and reconnect scenarios
Evidence from the diff
The patch replaces direct calls to clear_quiescent() with exit_quiescence() in three code paths: on_tx_signatures_exchange, reset_pending_splice_state, and peer_connected_get_handshake. exit_quiescence() clears both the quiescent bit and the disconnect timer (mark_response_received()), whereas clear_quiescent() only cleared the state bit. Leaving the timer armed caused DISCONNECT_PEER_AWAITING_RESPONSE_TICKS to expire after the splice finished or was aborted, producing a spurious DisconnectPeerWithWarning. The commit also adds regression tests covering successful splice completion, splice abort via tx_abort, and reconnect from quiescence.
Changed components
lightning/src/ln/channel.rslightning/src/ln/splicing_tests.rsInspect captured patch +200 / −4
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index b2c6b60..f6aa986 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -1719,7 +1719,10 @@ where
// We shouldn't be quiescent anymore upon reconnecting if:
// - We were in quiescence but a splice/RBF was never negotiated or
// - We were in quiescence but the splice negotiation failed due to disconnecting
- chan.context.channel_state.clear_quiescent();
+ //
+ // NOTE: While `exit_quiescence` clears the disconnect timer, it should already
+ // have been cleared by `remove_uncommitted_htlcs_and_mark_paused`.
+ chan.exit_quiescence();
None
} else {
None
@@ -7284,7 +7287,7 @@ where
self.pending_splice.take();
}
- self.context.channel_state.clear_quiescent();
+ self.exit_quiescence();
if current_is_awaiting_signatures {
self.context.interactive_tx_signing_session.take();
}
@@ -9341,7 +9344,6 @@ where
debug_assert!(!self.context.channel_state.is_awaiting_remote_revoke());
if let Some(pending_splice) = self.pending_splice.as_mut() {
- self.context.channel_state.clear_quiescent();
if let Some(FundingNegotiation::AwaitingSignatures {
mut funding,
funding_feerate_sat_per_1000_weight,
@@ -9390,6 +9392,8 @@ where
} else {
debug_assert!(false);
}
+
+ self.exit_quiescence();
} else {
self.funding.funding_transaction = Some(funding_tx.clone());
self.context.channel_state =
diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs
index 98be113..bfb6ee9 100644
--- a/lightning/src/ln/splicing_tests.rs
+++ b/lightning/src/ln/splicing_tests.rs
@@ -16,7 +16,8 @@ use crate::chain::ChannelMonitorUpdateStatus;
use crate::events::{ClosureReason, Event, FundingInfo, HTLCHandlingFailureType};
use crate::ln::chan_utils;
use crate::ln::channel::{
- CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE,
+ CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY, DISCONNECT_PEER_AWAITING_RESPONSE_TICKS,
+ FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE,
};
use crate::ln::channelmanager::{provided_init_features, PaymentId, BREAKDOWN_TIMEOUT};
use crate::ln::functional_test_utils::*;
@@ -7082,3 +7083,194 @@ fn test_splice_rbf_rejects_own_low_feerate_after_several_attempts() {
other => panic!("Expected SpliceFailed, got {:?}", other),
}
}
+
+#[test]
+fn test_no_disconnect_after_splice_completes() {
+ // Test that the disconnect timer is cleared when exiting quiescence after a successful splice
+ // negotiation. Previously, `on_tx_signatures_exchange` cleared the quiescent state but not the
+ // disconnect timer, causing a spurious disconnect after the splice completed.
+ let chanmon_cfgs = create_chanmon_cfgs(2);
+ let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+ let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+ let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+ let initial_channel_value_sat = 100_000;
+ let (_, _, channel_id, _) =
+ create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0);
+
+ let added_value = Amount::from_sat(50_000);
+ provide_utxo_reserves(&nodes, 2, added_value * 2);
+
+ let funding_contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value);
+ let new_funding_script = complete_splice_handshake(&nodes[0], &nodes[1]);
+
+ // Fire a tick while quiescent to arm the disconnect timer.
+ nodes[0].node.timer_tick_occurred();
+ nodes[1].node.timer_tick_occurred();
+
+ // Complete the splice negotiation, which should clear the timer when exiting quiescence.
+ complete_interactive_funding_negotiation(
+ &nodes[0],
+ &nodes[1],
+ channel_id,
+ funding_contribution,
+ new_funding_script,
+ );
+ let (_, splice_locked) = sign_interactive_funding_tx(&nodes[0], &nodes[1], false);
+ assert!(splice_locked.is_none());
+
+ let node_id_0 = nodes[0].node.get_our_node_id();
+ let node_id_1 = nodes[1].node.get_our_node_id();
+ expect_splice_pending_event(&nodes[0], &node_id_1);
+ expect_splice_pending_event(&nodes[1], &node_id_0);
+
+ // Fire enough ticks to trigger a disconnect if the timer wasn't properly cleared.
+ for _ in 0..DISCONNECT_PEER_AWAITING_RESPONSE_TICKS {
+ nodes[0].node.timer_tick_occurred();
+ nodes[1].node.timer_tick_occurred();
+ }
+
+ let has_disconnect = |events: &[MessageSendEvent]| {
+ events.iter().any(|event| {
+ matches!(
+ event,
+ MessageSendEvent::HandleError {
+ action: msgs::ErrorAction::DisconnectPeerWithWarning { .. },
+ ..
+ }
+ )
+ })
+ };
+ assert!(!has_disconnect(&nodes[0].node.get_and_clear_pending_msg_events()));
+ assert!(!has_disconnect(&nodes[1].node.get_and_clear_pending_msg_events()));
+}
+
+#[test]
+fn test_no_disconnect_after_splice_aborted() {
+ // Test that the disconnect timer is cleared when exiting quiescence after a splice negotiation
+ // is aborted via tx_abort. Previously, `reset_pending_splice_state` cleared the quiescent
+ // state but not the disconnect timer, causing a spurious disconnect after the abort.
+ let chanmon_cfgs = create_chanmon_cfgs(2);
+ let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+ let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+ let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+ let node_id_0 = nodes[0].node.get_our_node_id();
+ let node_id_1 = nodes[1].node.get_our_node_id();
+
+ let initial_channel_value_sat = 100_000;
+ let (_, _, channel_id, _) =
+ create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0);
+
+ let added_value = Amount::from_sat(50_000);
+ provide_utxo_reserves(&nodes, 2, added_value * 2);
+
+ let funding_contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value);
+ complete_splice_handshake(&nodes[0], &nodes[1]);
+
+ // Fire a tick while quiescent to arm the disconnect timer.
+ nodes[0].node.timer_tick_occurred();
+ nodes[1].node.timer_tick_occurred();
+
+ // Abort the splice, which should clear the timer when exiting quiescence.
+ nodes[0].node.abandon_splice(&channel_id, &node_id_1).unwrap();
+
+ expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution);
+
+ let msg_events = nodes[0].node.get_and_clear_pending_msg_events();
+ let tx_abort = msg_events
+ .iter()
+ .find_map(|event| {
+ if let MessageSendEvent::SendTxAbort { msg, .. } = event {
+ Some(msg.clone())
+ } else {
+ None
+ }
+ })
+ .expect("Expected SendTxAbort");
+
+ nodes[1].node.handle_tx_abort(node_id_0, &tx_abort);
+ let tx_abort_echo = get_event_msg!(nodes[1], MessageSendEvent::SendTxAbort, node_id_0);
+ nodes[1].node.get_and_clear_pending_events();
+
+ nodes[0].node.handle_tx_abort(node_id_1, &tx_abort_echo);
+
+ // Fire enough ticks to trigger a disconnect if the timer wasn't properly cleared.
+ for _ in 0..DISCONNECT_PEER_AWAITING_RESPONSE_TICKS {
+ nodes[0].node.timer_tick_occurred();
+ nodes[1].node.timer_tick_occurred();
+ }
+
+ let has_disconnect = |events: &[MessageSendEvent]| {
+ events.iter().any(|event| {
+ matches!(
+ event,
+ MessageSendEvent::HandleError {
+ action: msgs::ErrorAction::DisconnectPeerWithWarning { .. },
+ ..
+ }
+ )
+ })
+ };
+ assert!(!has_disconnect(&nodes[0].node.get_and_clear_pending_msg_events()));
+ assert!(!has_disconnect(&nodes[1].node.get_and_clear_pending_msg_events()));
+}
+
+#[test]
+fn test_no_disconnect_after_quiescence_on_reconnect() {
+ // Test that there is no spurious disconnect after reconnecting from a quiescent state. The
+ // disconnect timer is cleared by `remove_uncommitted_htlcs_and_mark_paused` during
+ // disconnection and by `exit_quiescence` during reconnection.
+ let chanmon_cfgs = create_chanmon_cfgs(2);
+ let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+ let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+ let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+ let node_id_0 = nodes[0].node.get_our_node_id();
+ let node_id_1 = nodes[1].node.get_our_node_id();
+
+ let initial_channel_value_sat = 100_000;
+ let (_, _, channel_id, _) =
+ create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0);
+
+ let added_value = Amount::from_sat(50_000);
+ provide_utxo_reserves(&nodes, 2, added_value * 2);
+
+ let funding_contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value);
+ complete_splice_handshake(&nodes[0], &nodes[1]);
+
+ // Fire a tick while quiescent to arm the disconnect timer.
+ nodes[0].node.timer_tick_occurred();
+ nodes[1].node.timer_tick_occurred();
+
+ // Disconnect and reconnect.
+ nodes[0].node.peer_disconnected(node_id_1);
+ nodes[1].node.peer_disconnected(node_id_0);
+
+ expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution);
+
+ let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);
+ reconnect_args.send_channel_ready = (true, true);
+ reconnect_args.send_announcement_sigs = (true, true);
+ reconnect_nodes(reconnect_args);
+
+ // Fire enough ticks to trigger a disconnect if the timer wasn't properly cleared.
+ for _ in 0..DISCONNECT_PEER_AWAITING_RESPONSE_TICKS {
+ nodes[0].node.timer_tick_occurred();
+ nodes[1].node.timer_tick_occurred();
+ }
+
+ let has_disconnect = |events: &[MessageSendEvent]| {
+ events.iter().any(|event| {
+ matches!(
+ event,
+ MessageSendEvent::HandleError {
+ action: msgs::ErrorAction::DisconnectPeerWithWarning { .. },
+ ..
+ }
+ )
+ })
+ };
+ assert!(!has_disconnect(&nodes[0].node.get_and_clear_pending_msg_events()));
+ assert!(!has_disconnect(&nodes[1].node.get_and_clear_pending_msg_events()));
+}
Why this scored 44/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.