Clear stale monitor pending resends on reestablish
What changed, and why it matters
This patch fixes a bug in the Lightning Dev Kit where a node could accidentally resend old cryptographic messages to a peer after restarting. When a stale saved channel state was reloaded, the node might think it still owed the peer a 'revoke_and_ack' or 'commitment_signed' message that had already been sent. During channel reestablishment, the peer's commitment numbers now authoritatively clear these stale flags, preventing duplicate messages that could confuse or disrupt the channel.
Apply the patch. It is a targeted correctness fix for Lightning channel state resynchronization after restarts involving splicing. No additional immediate action is indicated beyond normal patch deployment and monitoring for related state-machine issues.
Security signals we found
Duplicate cryptographic message resend (revoke_and_ack / commitment_signed) due to stale in-flight state
State inconsistency between serialized ChannelManager and latest ChannelMonitor after restart
Race/interleaving between startup background monitor completion and splice-locked monitor update
Potential channel desynchronization or forced closure from duplicate commitment messages
Fix uses peer-provided commitment numbers as authoritative ground truth during reestablish
Evidence from the diff
The commit clears stale monitor_pending_revoke_and_ack and monitor_pending_commitment_signed flags during channel_reestablish handling when the peer’s commitment numbers indicate the corresponding messages were already delivered. This addresses a state inconsistency where a serialized ChannelManager lags behind a ChannelMonitor that already completed and released post-update messages. The bug is triggered when a startup background monitor-completion event is interleaved with splice promotion, causing the old completion to be blocked behind a new in-flight splice update and then later replayed via monitor_updating_restored. The fix uses next_remote_commitment_number and next_local_commitment_number from channel_reestablish as authoritative proofs to reset the stale resend flags. A regression test in splicing_tests.rs reproduces the scenario.
Changed components
lightning/src/ln/channel.rslightning/src/ln/splicing_tests.rslightning/src/ln/functional_test_utils.rsInspect captured patch +154 / −1
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 0679143..d831810 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -10714,6 +10714,9 @@ where
let required_revoke = if msg.next_remote_commitment_number == our_commitment_transaction {
// Remote isn't waiting on any RevokeAndACK from us!
// Note that if we need to repeat our ChannelReady we'll do that in the next if block.
+ // If a stale ChannelManager replayed a completed update, the monitor-pending state may
+ // still think we owe one; the reestablish proof is authoritative here.
+ self.context.monitor_pending_revoke_and_ack = false;
None
} else if msg.next_remote_commitment_number + 1 == our_commitment_transaction {
if self.context.channel_state.is_monitor_update_in_progress() {
@@ -10800,6 +10803,9 @@ where
});
if msg.next_local_commitment_number == next_counterparty_commitment_number {
+ // If a stale ChannelManager replayed a completed update, the monitor-pending state may
+ // still think we owe one.
+ self.context.monitor_pending_commitment_signed = false;
if required_revoke.is_some() || self.context.signer_pending_revoke_and_ack {
log_debug!(logger, "Reconnected with only lost outbound RAA");
} else {
diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index ac6f137..20fcbef 100644
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -1023,7 +1023,7 @@ pub fn get_updates_and_revoke<CM: AChannelManager, H: NodeHolder<CM = CM>>(
macro_rules! get_event_msg {
($node: expr, $event_type: path, $node_id: expr) => {{
let events = $node.node.get_and_clear_pending_msg_events();
- assert_eq!(events.len(), 1);
+ assert_eq!(events.len(), 1, "{events:?}");
match events[0] {
$event_type { ref node_id, ref msg } => {
assert_eq!(*node_id, $node_id);
diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs
index eefd74e..d6e78a7 100644
--- a/lightning/src/ln/splicing_tests.rs
+++ b/lightning/src/ln/splicing_tests.rs
@@ -3191,6 +3191,153 @@ fn test_holding_cell_claim_freed_after_inferred_splice_locked() {
.remove_watched_txn_and_outputs(prev_funding_outpoint, prev_funding_script);
}
+#[test]
+fn test_stale_monitor_pending_resends_cleared_by_reestablish() {
+ // A stale ChannelManager may be reloaded after a monitor update completed and released its
+ // messages to the peer. If a later splice-locked monitor update is in-flight while the channel
+ // reestablishes, completing it must not release the stale messages again.
+ let chanmon_cfgs = create_chanmon_cfgs(2);
+ let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+ let persister;
+ let chain_monitor;
+ let node_1_reload;
+ let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+ let mut 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 prev_funding_outpoint = get_monitor!(nodes[1], channel_id).get_funding_txo();
+ let prev_funding_script = get_monitor!(nodes[1], channel_id).get_funding_script();
+
+ let outputs = vec![
+ TxOut {
+ value: Amount::from_sat(initial_channel_value_sat / 4),
+ script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(),
+ },
+ TxOut {
+ value: Amount::from_sat(initial_channel_value_sat / 4),
+ script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(),
+ },
+ ];
+ let funding_contribution =
+ initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs).unwrap();
+ let (splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution);
+
+ // Only let node 0 see the splice lock for now.
+ confirm_transaction(&nodes[0], &splice_tx);
+ let splice_locked_0 = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceLocked, node_id_1);
+ nodes[1].node.handle_splice_locked(node_id_0, &splice_locked_0);
+ assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+
+ // Send an HTLC from node 0 to 1 that will get fully committed to.
+ let payment_amount = 1_000_000;
+ let (route, payment_hash, _payment_preimage, payment_secret) =
+ get_route_and_payment_hash!(nodes[0], nodes[1], payment_amount);
+ let onion = RecipientOnionFields::secret_only(payment_secret, payment_amount);
+ let payment_id = PaymentId(payment_hash.0);
+ nodes[0].node.send_payment_with_route(route, payment_hash, onion, payment_id).unwrap();
+ let htlc_update = get_htlc_update_msgs(&nodes[0], &node_id_1);
+ check_added_monitors(&nodes[0], 1);
+
+ chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
+ nodes[1].node.handle_update_add_htlc(node_id_0, &htlc_update.update_add_htlcs[0]);
+ nodes[1].node.handle_commitment_signed_batch_test(node_id_0, &htlc_update.commitment_signed);
+ check_added_monitors(&nodes[1], 1);
+ assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+
+ // Persist the `ChannelManager` while node 1 is still pending to send their RAA+CS to node 0.
+ let stale_manager_1 = nodes[1].node.encode();
+
+ // Let node 1 release its RAA+CS to node 0 and process them.
+ nodes[1].chain_monitor.complete_sole_pending_chan_update(&channel_id);
+ chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
+ let (raa, commitment_signed) = get_revoke_commit_msgs(&nodes[1], &node_id_0);
+ nodes[0].node.handle_revoke_and_ack(node_id_1, &raa);
+ check_added_monitors(&nodes[0], 1);
+ nodes[0].node.handle_commitment_signed_batch_test(node_id_1, &commitment_signed);
+ check_added_monitors(&nodes[0], 1);
+ let _dropped_raa = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, node_id_1);
+ assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+
+ // Reload node 1 with the stale `ChannelManager` and confirm the splice.
+ nodes[0].node.peer_disconnected(node_id_1);
+ nodes[1].node.peer_disconnected(node_id_0);
+ let latest_monitor_1 = get_monitor!(nodes[1], channel_id).encode();
+ reload_node!(
+ nodes[1],
+ &stale_manager_1,
+ &[&latest_monitor_1],
+ persister,
+ chain_monitor,
+ node_1_reload
+ );
+
+ mine_transaction_without_consistency_checks(&nodes[1], &splice_tx);
+ connect_blocks(&nodes[1], 5);
+ persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
+
+ // Reestablish the channel. While the `ChannelManager` should think it still owes node 0 its
+ // RAA+CS, it should determine from node 0's `channel_reestablish` that they were already
+ // delivered.
+ connect_nodes(&nodes[0], &nodes[1]);
+ check_added_monitors(&nodes[1], 1);
+ let reestablish_0 = get_chan_reestablish_msgs!(nodes[0], nodes[1]);
+ let reestablish_1 = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
+ nodes[1].node.handle_channel_reestablish(node_id_0, reestablish_0.first().unwrap());
+ let msg_events = nodes[1].node.get_and_clear_pending_msg_events();
+ assert!(
+ msg_events.iter().all(|event| !matches!(
+ event,
+ MessageSendEvent::SendRevokeAndACK { .. } | MessageSendEvent::UpdateHTLCs { .. }
+ )),
+ "stale monitor-pending resend leaked during reestablish: {msg_events:?}"
+ );
+
+ nodes[1].chain_monitor.complete_sole_pending_chan_update(&channel_id);
+ persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
+
+ let msg_events = nodes[1].node.get_and_clear_pending_msg_events();
+ assert!(
+ msg_events.iter().all(|event| !matches!(
+ event,
+ MessageSendEvent::SendRevokeAndACK { .. } | MessageSendEvent::UpdateHTLCs { .. }
+ )),
+ "stale monitor-pending resend leaked after reestablish: {msg_events:?}"
+ );
+ expect_channel_ready_event(&nodes[1], &node_id_0);
+
+ nodes[0].node.handle_channel_reestablish(node_id_1, reestablish_1.first().unwrap());
+ check_added_monitors(&nodes[0], 1);
+ expect_channel_ready_event(&nodes[0], &node_id_1);
+
+ // Finish fully committing the HTLC and make sure we can still send more payments.
+ let msg_events = nodes[0].node.get_and_clear_pending_msg_events();
+ assert_eq!(msg_events.len(), 3, "{msg_events:?}");
+ if let MessageSendEvent::SendRevokeAndACK { msg, .. } = &msg_events[0] {
+ nodes[1].node.handle_revoke_and_ack(node_id_0, msg);
+ check_added_monitors(&nodes[1], 1);
+ nodes[1].chain_monitor.complete_sole_pending_chan_update(&channel_id);
+ persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
+
+ assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+ nodes[1].node.process_pending_htlc_forwards();
+ expect_payment_claimable!(&nodes[1], payment_hash, payment_secret, payment_amount);
+ } else {
+ panic!("Unexpected event {:?}", &msg_events[0]);
+ }
+
+ send_payment(&nodes[0], &[&nodes[1]], payment_amount);
+
+ for node in &[&nodes[0], &nodes[1]] {
+ node.chain_source
+ .remove_watched_txn_and_outputs(prev_funding_outpoint, prev_funding_script.clone());
+ }
+}
+
#[test]
fn test_stale_announcement_signatures_ignored_after_splice_lock() {
// Regression test: a peer may transmit `announcement_signatures` signed over a pre-splice
Why this scored 66/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.