Send missing splice_locked when confirmation precedes reestablishment

In most cases, we end up sending our `splice_locked` either implicitly
during reestablishment via
`ChannelReestablish::my_current_funding_locked`, or explicitly after
reestablishment. However, we did not consider that it's possible for the
node to be notified of the splice confirmation after connecting to their
peer but prior to reestablishing their channel. In such cases, we need
to explicitly send the `splice_locked` since it wasn't included in
`my_current_funding_locked`, but only after the channel has been
reestablished.

Found by the chanmon_consistency fuzz target.

Author: wpaulino

lightning/src/ln/channel.rs

@@ -1265,6 +1265,7 @@ pub(super) struct ReestablishResponses {
 	pub shutdown_msg: Option<msgs::Shutdown>,
 	pub tx_signatures: Option<msgs::TxSignatures>,
 	pub tx_abort: Option<msgs::TxAbort>,
+	pub splice_locked: Option<msgs::SpliceLocked>,
 	pub inferred_splice_locked: Option<msgs::SpliceLocked>,
 }
 
@@ -3503,6 +3504,12 @@ pub(super) struct ChannelContext<SP: SignerProvider> {
 	/// See-also <https://github.com/lightningnetwork/lnd/issues/4006>
 	pub workaround_lnd_bug_4006: Option<msgs::ChannelReady>,
 
+	/// The `my_current_funding_locked` txid included in our `channel_reestablish` for the current
+	/// reconnect, if any. We track this as we cannot tell what was included after we've already
+	/// sent it, as it's possible it was unconfirmed at the time we sent it, but confirmed shortly
+	/// after.
+	funding_locked_txid_sent_in_reestablish: Option<Txid>,
+
 	/// An option set when we wish to track how many ticks have elapsed while waiting for a response
 	/// from our counterparty after entering specific states. If the peer has yet to respond after
 	/// reaching `DISCONNECT_PEER_AWAITING_RESPONSE_TICKS`, a reconnection should be attempted to
@@ -4225,6 +4232,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
 			announcement_sigs: None,
 
 			workaround_lnd_bug_4006: None,
+			funding_locked_txid_sent_in_reestablish: None,
 			sent_message_awaiting_response: None,
 
 			latest_inbound_scid_alias: None,
@@ -4536,6 +4544,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
 			announcement_sigs: None,
 
 			workaround_lnd_bug_4006: None,
+			funding_locked_txid_sent_in_reestablish: None,
 			sent_message_awaiting_response: None,
 
 			latest_inbound_scid_alias: None,
@@ -10512,6 +10521,8 @@ where
 		// remaining cases either succeed or ErrorMessage-fail).
 		self.context.channel_state.clear_peer_disconnected();
 		self.mark_response_received();
+		let funding_locked_txid_sent_in_reestablish =
+			self.context.funding_locked_txid_sent_in_reestablish.take();
 
 		let shutdown_msg = self.get_outbound_shutdown();
 
@@ -10663,6 +10674,7 @@ where
 					shutdown_msg, announcement_sigs,
 					tx_signatures,
 					tx_abort: None,
+					splice_locked: None,
 					inferred_splice_locked: None,
 				});
 			}
@@ -10676,6 +10688,7 @@ where
 				shutdown_msg, announcement_sigs,
 				tx_signatures,
 				tx_abort,
+				splice_locked: None,
 				inferred_splice_locked: None,
 			});
 		}
@@ -10745,6 +10758,15 @@ where
 					splice_txid,
 				})
 		});
+		let splice_locked = self.pending_splice.as_ref().and_then(|pending_splice| {
+			pending_splice
+				.sent_funding_txid
+				.filter(|splice_txid| Some(*splice_txid) != funding_locked_txid_sent_in_reestablish)
+				.map(|splice_txid| msgs::SpliceLocked {
+					channel_id: self.context.channel_id,
+					splice_txid,
+				})
+		});
 
 		if msg.next_local_commitment_number == next_counterparty_commitment_number {
 			if required_revoke.is_some() || self.context.signer_pending_revoke_and_ack {
@@ -10763,6 +10785,7 @@ where
 				commitment_order: self.context.resend_order.clone(),
 				tx_signatures,
 				tx_abort,
+				splice_locked,
 				inferred_splice_locked,
 			})
 		} else if msg.next_local_commitment_number == next_counterparty_commitment_number - 1 {
@@ -10788,6 +10811,7 @@ where
 					commitment_order: self.context.resend_order.clone(),
 					tx_signatures: None,
 					tx_abort,
+					splice_locked,
 					inferred_splice_locked,
 				})
 			} else {
@@ -10815,6 +10839,7 @@ where
 					commitment_order: self.context.resend_order.clone(),
 					tx_signatures: None,
 					tx_abort,
+					splice_locked,
 					inferred_splice_locked,
 				})
 			}
@@ -12492,6 +12517,9 @@ where
 			log_info!(logger, "Sending a data_loss_protect with no previous remote per_commitment_secret for channel {}", &self.context.channel_id());
 			[0;32]
 		};
+		let my_current_funding_locked = self.maybe_get_my_current_funding_locked();
+		self.context.funding_locked_txid_sent_in_reestablish =
+			my_current_funding_locked.as_ref().map(|funding_locked| funding_locked.txid);
 		msgs::ChannelReestablish {
 			channel_id: self.context.channel_id(),
 			// The protocol has two different commitment number concepts - the "commitment
@@ -12515,7 +12543,7 @@ where
 			your_last_per_commitment_secret: remote_last_secret,
 			my_current_per_commitment_point: dummy_pubkey,
 			next_funding: self.maybe_get_next_funding(),
-			my_current_funding_locked: self.maybe_get_my_current_funding_locked(),
+			my_current_funding_locked,
 		}
 	}
 
@@ -17196,6 +17224,7 @@ impl<'a, 'b, 'c, ES: EntropySource, SP: SignerProvider>
 				announcement_sigs,
 
 				workaround_lnd_bug_4006: None,
+				funding_locked_txid_sent_in_reestablish: None,
 				sent_message_awaiting_response: None,
 
 				latest_inbound_scid_alias,

lightning/src/ln/channelmanager.rs

@@ -13285,10 +13285,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 							}
 						}
 						let need_lnd_workaround = chan.context.workaround_lnd_bug_4006.take();
-						let funding_tx_signed = responses.tx_signatures.map(|tx_signatures| FundingTxSigned {
-							tx_signatures: Some(tx_signatures),
-							..Default::default()
-						});
+						let funding_tx_signed = if responses.tx_signatures.is_some() || responses.splice_locked.is_some() {
+							Some(FundingTxSigned {
+								tx_signatures: responses.tx_signatures,
+								splice_locked: responses.splice_locked,
+								..Default::default()
+							})
+						} else {
+							None
+						};
 						let (htlc_forwards, decode_update_add_htlcs) = self.handle_channel_resumption(
 							&mut peer_state.pending_msg_events, chan, responses.raa, responses.commitment_update, responses.commitment_order,
 							Vec::new(), Vec::new(), None, responses.channel_ready, responses.announcement_sigs,

lightning/src/ln/splicing_tests.rs

@@ -751,7 +751,21 @@ pub fn lock_splice<'a, 'b, 'c, 'd>(
 		.get_monitor(splice_locked_for_node_b.channel_id)
 		.map(|monitor| monitor.get_funding_txo().txid)
 		.unwrap();
+	complete_splice_locked_exchange(
+		node_a,
+		node_b,
+		splice_locked_for_node_b,
+		is_0conf,
+		expected_discard_txids,
+		prev_funding_txid,
+	)
+}
 
+fn complete_splice_locked_exchange<'a, 'b, 'c, 'd>(
+	node_a: &'a Node<'b, 'c, 'd>, node_b: &'a Node<'b, 'c, 'd>,
+	splice_locked_for_node_b: &msgs::SpliceLocked, is_0conf: bool, expected_discard_txids: &[Txid],
+	prev_funding_txid: Txid,
+) -> SpliceLockedResult {
 	let node_id_a = node_a.node.get_our_node_id();
 	let node_id_b = node_b.node.get_our_node_id();
 
@@ -2585,6 +2599,83 @@ fn do_test_splice_reestablish(reload: bool, async_monitor_update: bool) {
 		.remove_watched_txn_and_outputs(prev_funding_outpoint, prev_funding_script);
 }
 
+#[test]
+fn test_splice_locked_waits_for_channel_reestablish() {
+	// If a splice confirms after `peer_connected` but before `channel_reestablish` is handled, the
+	// peer state is connected while the channel still has its disconnected bit set. We must not send
+	// `splice_locked` until the channel is reestablished, but should send it immediately after.
+	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 prev_funding_txid = get_monitor!(nodes[0], channel_id).get_funding_txo().txid;
+
+	send_payment(&nodes[0], &[&nodes[1]], 1_000_000);
+
+	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);
+
+	nodes[0].node.peer_disconnected(node_id_1);
+	nodes[1].node.peer_disconnected(node_id_0);
+
+	connect_nodes(&nodes[0], &nodes[1]);
+	let reestablish_0 =
+		get_event_msg!(nodes[0], MessageSendEvent::SendChannelReestablish, node_id_1);
+	let reestablish_1 =
+		get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, node_id_0);
+
+	confirm_transaction(&nodes[0], &splice_tx);
+	assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
+
+	nodes[1].node.handle_channel_reestablish(node_id_0, &reestablish_0);
+	let _ = get_event_msg!(nodes[1], MessageSendEvent::SendChannelUpdate, node_id_0);
+	nodes[0].node.handle_channel_reestablish(node_id_1, &reestablish_1);
+	let mut msg_events = nodes[0].node.get_and_clear_pending_msg_events();
+	assert_eq!(msg_events.len(), 2, "{msg_events:?}");
+	let splice_locked_0 =
+		if let MessageSendEvent::SendSpliceLocked { node_id, msg } = msg_events.remove(0) {
+			assert_eq!(node_id, node_id_1);
+			msg
+		} else {
+			panic!();
+		};
+	if let MessageSendEvent::SendChannelUpdate { node_id, .. } = msg_events.remove(0) {
+		assert_eq!(node_id, node_id_1);
+	} else {
+		panic!();
+	}
+
+	confirm_transaction(&nodes[1], &splice_tx);
+	complete_splice_locked_exchange(
+		&nodes[0],
+		&nodes[1],
+		&splice_locked_0,
+		false,
+		&[],
+		prev_funding_txid,
+	);
+
+	send_payment(&nodes[0], &[&nodes[1]], 1_000_000);
+}
+
 #[test]
 fn test_splice_confirms_on_both_sides_while_disconnected() {
 	// Regression test: when a splice transaction confirms on both sides while peers are