From 21fed17c242bc7764522cd965059a40244ed2a57 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 23 Apr 2026 14:54:22 -0500 Subject: [PATCH 1/2] Skip pre-splice announcement_signatures on reestablish MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. In the reestablish handler, `get_announcement_sigs` was called before the inferred `splice_locked` was processed and the splice was promoted, so `self.funding` still pointed to the pre-splice scope. If `announcement_sigs_state` was `NotSent`, the generated `announcement_signatures` carried the pre-splice `short_channel_id` and bitcoin key — which the peer (having already promoted via its own inferred `splice_locked`) would verify against the post-splice `UnsignedChannelAnnouncement`, failing the signature check and force-closing. Skip the pre-promotion call when `my_current_funding_locked` matches the splice we've already confirmed — i.e. `pending_splice.sent_funding_txid` is set and equals the peer's locked txid. `maybe_promote_splice_funding` emits correct post-splice signatures after the inferred `splice_locked` is processed. Co-Authored-By: Claude Opus 4.7 (1M context) --- lightning/src/ln/channel.rs | 18 ++++- lightning/src/ln/splicing_tests.rs | 118 +++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 10801edef01..51e863c4115 100644 --- a/lightning/src/ln/channel.rs +++ b/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; diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index fa22ccb61c7..41903a5b851 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -2183,6 +2183,124 @@ 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| -> 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_propose_splice_while_disconnected() { do_test_propose_splice_while_disconnected(false); From 80528b15975794cbff8f1f2edad551472948618d Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 23 Apr 2026 15:33:59 -0500 Subject: [PATCH 2/2] Ignore stale announcement_signatures instead of force-closing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A peer may transmit `announcement_signatures` signed over a stale `short_channel_id` — most plausibly a retransmission or a peer implementation whose view hasn't caught up to our post-splice promotion. Verifying such sigs against the current `UnsignedChannelAnnouncement` (built from `self.funding`) always fails the hash check, which previously produced a force-close. BOLT #7 does not require closing in this situation; the mismatch is expected across splice handoffs. Short-circuit with `ChannelError::Ignore` when `msg.short_channel_id` doesn't match the current funding's scid, leaving the genuine invalid-signature paths in place for sigs that actually target our current scid. Co-Authored-By: Claude Opus 4.7 (1M context) --- lightning/src/ln/channel.rs | 11 ++++ lightning/src/ln/splicing_tests.rs | 85 ++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 51e863c4115..e62d36f9e70 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -12155,6 +12155,17 @@ where &mut self, node_signer: &NS, chain_hash: ChainHash, best_block_height: u32, msg: &msgs::AnnouncementSignatures, user_config: &UserConfig ) -> Result { + // 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()[..])[..]); diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 41903a5b851..a6b91a63c89 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -2301,6 +2301,91 @@ fn test_splice_confirms_on_both_sides_while_disconnected() { .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);