Fix stale pre-splice announcement_signatures during reestablish

lightning/src/ln/channel.rs

@@ -10325,7 +10325,23 @@ where
 			}
 		}
 
-		let announcement_sigs = self.get_announcement_sigs(node_signer, chain_hash, user_config, best_block.height, logger);
+		// If the counterparty's `my_current_funding_locked` matches the splice we've already
+		// confirmed and are about to promote, any `announcement_signatures` we'd generate here
+		// would be for the soon-to-be-superseded pre-splice funding. Skip them;
+		// `maybe_promote_splice_funding` will emit correct post-splice sigs once
+		// `inferred_splice_locked` is processed.
+		let our_splice_txid =
+			self.pending_splice.as_ref().and_then(|ps| ps.sent_funding_txid);
+		let splice_promotion_pending = msg
+			.my_current_funding_locked
+			.as_ref()
+			.map(|funding_locked| Some(funding_locked.txid) == our_splice_txid)
+			.unwrap_or(false);
+		let announcement_sigs = if splice_promotion_pending {
+			None
+		} else {
+			self.get_announcement_sigs(node_signer, chain_hash, user_config, best_block.height, logger)
+		};
 
 		let mut commitment_update = None;
 		let mut tx_signatures = None;
@@ -12139,6 +12155,17 @@ where
 		&mut self, node_signer: &NS, chain_hash: ChainHash, best_block_height: u32,
 		msg: &msgs::AnnouncementSignatures, user_config: &UserConfig
 	) -> Result<msgs::ChannelAnnouncement, ChannelError> {
+		// Ignore sigs signed over a `short_channel_id` other than our current one (e.g. stale
+		// pre-splice sigs arriving after our side has promoted). Verifying them against the
+		// current `UnsignedChannelAnnouncement` would always fail the hash check, but per BOLT #7
+		// that's not a protocol violation warranting a force-close.
+		if Some(msg.short_channel_id) != self.funding.get_short_channel_id() {
+			return Err(ChannelError::Ignore(format!(
+				"Ignoring announcement_signatures for short_channel_id {} which does not match our current short_channel_id {:?}",
+				msg.short_channel_id, self.funding.get_short_channel_id(),
+			)));
+		}
+
 		let announcement = self.get_channel_announcement(node_signer, chain_hash, user_config)?;
 
 		let msghash = hash_to_message!(&Sha256d::hash(&announcement.encode()[..])[..]);

lightning/src/ln/splicing_tests.rs

@@ -2183,6 +2183,209 @@ 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_confirms_on_both_sides_while_disconnected() {
+	// Regression test: when a splice transaction confirms on both sides while peers are
+	// disconnected, each peer's `channel_reestablish` carries `my_current_funding_locked` with the
+	// splice txid. The receiving side must not emit `announcement_signatures` for the pre-splice
+	// funding in that handler — those would be verified against the post-splice channel
+	// announcement on the peer and force-close the channel. Instead, sigs are generated after the
+	// inferred `splice_locked` promotes the splice funding.
+	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_outpoint = get_monitor!(nodes[0], channel_id).get_funding_txo();
+	let prev_funding_script = get_monitor!(nodes[0], channel_id).get_funding_script();
+
+	// Capture the pre-splice scid so we can later assert the announcement_sigs each side emits
+	// on reconnect carry the post-splice scid, not the pre-splice one the bug would emit.
+	let pre_splice_scid = nodes[0].node.list_channels()[0].short_channel_id.unwrap();
+
+	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);
+
+	// Disconnect before either side confirms the splice.
+	nodes[0].node.peer_disconnected(node_id_1);
+	nodes[1].node.peer_disconnected(node_id_0);
+
+	// Confirm the splice on both sides while disconnected. Each side's `transactions_confirmed`
+	// runs `check_get_splice_locked`, which sets `pending_splice.sent_funding_txid` so that
+	// `my_current_funding_locked` will carry the splice txid on reconnect. No `splice_locked`
+	// messages are queued while disconnected.
+	confirm_transaction(&nodes[0], &splice_tx);
+	confirm_transaction(&nodes[1], &splice_tx);
+	assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
+	assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+
+	// Reconnect manually so we can inspect each side's emitted `SendAnnouncementSignatures`.
+	// Each side's `channel_reestablish` carries `my_current_funding_locked` with the splice
+	// txid, triggering inferred `splice_locked` on the peer. With the fix in place,
+	// `announcement_signatures` are generated from the post-splice funding (via the promotion
+	// path) rather than the pre-splice funding (via the reestablish handler).
+	connect_nodes(&nodes[0], &nodes[1]);
+	let reestablish_0 = get_chan_reestablish_msgs!(nodes[0], nodes[1]);
+	let reestablish_1 = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
+	for msg in &reestablish_0 {
+		nodes[1].node.handle_channel_reestablish(node_id_0, msg);
+	}
+	for msg in &reestablish_1 {
+		nodes[0].node.handle_channel_reestablish(node_id_1, msg);
+	}
+	check_added_monitors(&nodes[0], 1);
+	check_added_monitors(&nodes[1], 1);
+	expect_channel_ready_event(&nodes[0], &node_id_1);
+	expect_channel_ready_event(&nodes[1], &node_id_0);
+
+	// Each side should emit exactly one `SendAnnouncementSignatures` (post-promotion). The
+	// pre-fix behavior would emit a second, stale pre-splice one — our assertion is that the
+	// only sigs we send carry the post-splice scid.
+	let take_announcement_sigs = |events: Vec<MessageSendEvent>| -> msgs::AnnouncementSignatures {
+		let mut sigs = events.into_iter().filter_map(|e| match e {
+			MessageSendEvent::SendAnnouncementSignatures { msg, .. } => Some(msg),
+			_ => None,
+		});
+		let only = sigs.next().expect("expected one SendAnnouncementSignatures");
+		assert!(sigs.next().is_none(), "expected only one SendAnnouncementSignatures");
+		only
+	};
+	let node_0_events = nodes[0].node.get_and_clear_pending_msg_events();
+	let node_1_events = nodes[1].node.get_and_clear_pending_msg_events();
+	let node_0_sigs = take_announcement_sigs(node_0_events);
+	let node_1_sigs = take_announcement_sigs(node_1_events);
+	assert_ne!(node_0_sigs.short_channel_id, pre_splice_scid);
+	assert_ne!(node_1_sigs.short_channel_id, pre_splice_scid);
+
+	// Cross-deliver to complete the post-splice announcement exchange, then drain the
+	// resulting `BroadcastChannelAnnouncement` events on each side.
+	nodes[1].node.handle_announcement_signatures(node_id_0, &node_0_sigs);
+	nodes[0].node.handle_announcement_signatures(node_id_1, &node_1_sigs);
+	let _ = nodes[0].node.get_and_clear_pending_msg_events();
+	let _ = nodes[1].node.get_and_clear_pending_msg_events();
+
+	// Channel must still be operational after reconnect — no force-close from mismatched
+	// announcement signatures.
+	send_payment(&nodes[0], &[&nodes[1]], 1_000_000);
+
+	// No stray events or messages left over.
+	assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
+	assert!(nodes[1].node.get_and_clear_pending_events().is_empty());
+	assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
+	assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+
+	// Clean up chain-source state for the retired pre-splice funding so end-of-test checks pass.
+	nodes[0]
+		.chain_source
+		.remove_watched_txn_and_outputs(prev_funding_outpoint, prev_funding_script.clone());
+	nodes[1]
+		.chain_source
+		.remove_watched_txn_and_outputs(prev_funding_outpoint, prev_funding_script);
+}
+
+#[test]
+fn test_stale_announcement_signatures_ignored_after_splice_lock() {
+	// Regression test: a peer may transmit `announcement_signatures` signed over a pre-splice
+	// `short_channel_id` (for example, a stale retransmission or a peer implementation that
+	// hasn't yet caught up to our post-splice promotion). Verifying those sigs against the
+	// post-splice `UnsignedChannelAnnouncement` will always fail the hash check, but that is not
+	// a protocol violation — the spec permits ignoring and the channel should stay open.
+	let chanmon_cfgs = create_chanmon_cfgs(2);
+	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let mut config = test_default_channel_config();
+	config.channel_handshake_config.announced_channel_max_inbound_htlc_value_in_flight_percentage =
+		100;
+	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(config)]);
+	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;
+	// Use the lower-level helper so we get the signed `ChannelAnnouncement` back — the test
+	// needs node 1's pre-splice announcement signatures to replay later.
+	let chan_announcement =
+		create_chan_between_nodes_with_value(&nodes[0], &nodes[1], initial_channel_value_sat, 0);
+	let channel_id = chan_announcement.3;
+	update_nodes_with_chan_announce(
+		&nodes,
+		0,
+		1,
+		&chan_announcement.0,
+		&chan_announcement.1,
+		&chan_announcement.2,
+	);
+
+	// Extract node 1's pre-splice signatures from the ChannelAnnouncement. `UnsignedChannelAnnouncement`
+	// orders `node_id_1`/`node_id_2` by serialized pubkey; node 1's sigs are in slot 1 iff node 1's
+	// pubkey is lexicographically smaller.
+	let node_1_is_node_one = node_id_1.serialize() < node_id_0.serialize();
+	let (stale_node_sig, stale_bitcoin_sig) = if node_1_is_node_one {
+		(chan_announcement.0.node_signature_1, chan_announcement.0.bitcoin_signature_1)
+	} else {
+		(chan_announcement.0.node_signature_2, chan_announcement.0.bitcoin_signature_2)
+	};
+
+	// Capture the pre-splice `short_channel_id` — this is the scid the stale sigs sign over.
+	let pre_splice_scid = nodes[0].node.list_channels()[0].short_channel_id.unwrap();
+
+	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);
+	mine_transaction(&nodes[0], &splice_tx);
+	mine_transaction(&nodes[1], &splice_tx);
+	lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1);
+
+	// The post-splice scid is now different; confirm that.
+	let post_splice_scid = nodes[0].node.list_channels()[0].short_channel_id.unwrap();
+	assert_ne!(pre_splice_scid, post_splice_scid);
+
+	// Replay node 1's pre-splice announcement signatures, now stale (the current scid is the
+	// post-splice one). This is the exact shape of message a peer would send if it retransmitted
+	// an old `announcement_signatures` across a splice handoff.
+	let stale_sigs = msgs::AnnouncementSignatures {
+		channel_id,
+		short_channel_id: pre_splice_scid,
+		node_signature: stale_node_sig,
+		bitcoin_signature: stale_bitcoin_sig,
+	};
+	nodes[0].node.handle_announcement_signatures(node_id_1, &stale_sigs);
+
+	// No force-close, no outbound error, no events. The channel must still be listed and usable.
+	assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
+	assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
+	assert_eq!(nodes[0].node.list_channels().len(), 1);
+	send_payment(&nodes[0], &[&nodes[1]], 1_000_000);
+}
+
 #[test]
 fn test_propose_splice_while_disconnected() {
 	do_test_propose_splice_while_disconnected(false);