From 7f02198963dc5bfc0d08ec32b8d91f9a792648ff Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Tue, 16 Sep 2025 23:53:14 -0700 Subject: [PATCH 1/6] Return early on duplicate calls to `funding_transaction_signed` We may produce duplicate `FundingTransactionReadyForSigning` events if the user has processed an initial event but has not yet called back with `funding_transaction_signed` and a peer reconnection occurs. If the user also handles the duplicate events, any duplicate calls to `funding_transaction_signed` after an initial successful one would return an error. This doesn't make sense, as the API should remain idempotent, so we return early on any duplicate calls. --- lightning/src/ln/channel.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index f063c6275f6..2a8fd8eb434 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -8678,8 +8678,19 @@ where .unwrap_or(false)); } + if signing_session.holder_tx_signatures().is_some() { + // Our `tx_signatures` either should've been the first time we processed them, + // or we're waiting for our counterparty to send theirs first. + return Ok((None, None)); + } + signing_session } else { + if Some(funding_txid_signed) == self.funding.get_funding_txid() { + // We may be handling a duplicate call and the funding was already locked so we + // no longer have the signing session present. + return Ok((None, None)); + } let err = format!("Channel {} not expecting funding signatures", self.context.channel_id); return Err(APIError::APIMisuseError { err }); From 1f3c51985f06b3f4300d9e164dab6d106090a735 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Tue, 16 Sep 2025 23:55:22 -0700 Subject: [PATCH 2/6] Wait for inbound commitment_signed before producing tx_signatures We only want to produce `tx_signatures` once we know that the monitor update (either the initial one for a dual-funded channel, or a `RenegotiatedFunding` one for a splice) has been persisted. If we haven't received the counterparty's `commitment_signed` yet, then the monitor update hasn't been created, leading us to pass the `!awaiting_monitor_update` condition and produce a holder `tx_signatures` message. --- lightning/src/ln/channelmanager.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 222da4b7626..e448802ba5e 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -9391,6 +9391,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if let Some(signing_session) = (!channel.is_awaiting_monitor_update()) .then(|| ()) .and_then(|_| channel.context.interactive_tx_signing_session.as_mut()) + .filter(|signing_session| signing_session.has_received_commitment_signed()) .filter(|signing_session| signing_session.holder_tx_signatures().is_none()) { if signing_session.has_local_contribution() { From fd4d052caefbcbd21a646bfb43345b9661f3ca58 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 1 Oct 2025 06:06:18 -0700 Subject: [PATCH 3/6] Avoid initial commitment channel_ready retransmission while splicing If nodes have started a splice, this means they have both sent and received `channel_ready` already: in that case, it's unnecessary to retransmit `channel_ready` on reconnection. --- lightning/src/ln/channel.rs | 10 ++++++++-- lightning/src/ln/splicing_tests.rs | 1 - 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 2a8fd8eb434..4eb551354e1 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -9693,12 +9693,18 @@ where // A node: // - if `next_commitment_number` is 1 in both the `channel_reestablish` it - // sent and received: + // sent and received, and none of those `channel_reestablish` messages + // contain `my_current_funding_locked` or `next_funding` for a splice transaction: // - MUST retransmit `channel_ready`. // - otherwise: // - MUST NOT retransmit `channel_ready`, but MAY send `channel_ready` with // a different `short_channel_id` `alias` field. - let channel_ready = if msg.next_local_commitment_number == 1 && INITIAL_COMMITMENT_NUMBER - self.holder_commitment_point.next_transaction_number() == 1 { + let both_sides_on_initial_commitment_number = msg.next_local_commitment_number == 1 + && INITIAL_COMMITMENT_NUMBER - self.holder_commitment_point.next_transaction_number() == 1; + let channel_ready = if both_sides_on_initial_commitment_number + && self.pending_splice.is_none() + && self.funding.channel_transaction_parameters.splice_parent_funding_txid.is_none() + { // We should never have to worry about MonitorUpdateInProgress resending ChannelReady self.get_channel_ready(logger) } else { None }; diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 88271d71930..a7d5744390f 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -497,7 +497,6 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { } let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); - reconnect_args.send_channel_ready = (true, true); reconnect_nodes(reconnect_args); mine_transaction(&nodes[0], &splice_tx); From 535e2c609370648d3c8d4dfee06b7db3d0c5f70e Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 1 Oct 2025 08:00:17 -0700 Subject: [PATCH 4/6] Avoid resetting splice state on FundingNegotiation::AwaitingSignatures Otherwise, we won't ever be able to resume a pending negotiation after a reconnection via `channel_reestablish`. Along the way, we also merge `should_reset_pending_splice_state` into `PendingFunding::can_abandon_state` to simplify the logic around when we're able to reset specific parts of the pending splice state. --- lightning/src/ln/channel.rs | 114 ++++++++++++++--------------- lightning/src/ln/splicing_tests.rs | 21 ++---- 2 files changed, 59 insertions(+), 76 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 4eb551354e1..d66ddc97c45 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1694,8 +1694,7 @@ where pending_v2_channel.interactive_tx_constructor.take(); }, ChannelPhase::Funded(funded_channel) => { - if funded_channel.should_reset_pending_splice_funding_negotiation().unwrap_or(true) - { + if funded_channel.should_reset_pending_splice_state() { funded_channel.reset_pending_splice_state(); } else { debug_assert!(false, "We should never fail an interactive funding negotiation once we're exchanging tx_signatures"); @@ -1829,18 +1828,15 @@ where pending_v2_channel.interactive_tx_constructor.take().is_some() }, ChannelPhase::Funded(funded_channel) => { - if let Some(should_reset) = - funded_channel.should_reset_pending_splice_funding_negotiation() - { - if should_reset { - // We may have still tracked the pending funding negotiation state, so we - // should ack with our own `tx_abort`. - funded_channel.reset_pending_splice_state() - } else { - return Err(ChannelError::close( - "Received tx_abort while awaiting tx_signatures exchange".to_owned(), - )); - } + if funded_channel.has_pending_splice_awaiting_signatures() { + return Err(ChannelError::close( + "Received tx_abort while awaiting tx_signatures exchange".to_owned(), + )); + } + if funded_channel.should_reset_pending_splice_state() { + let has_funding_negotiation = funded_channel.reset_pending_splice_state(); + debug_assert!(has_funding_negotiation); + true } else { // We were not tracking the pending funding negotiation state anymore, likely // due to a disconnection or already having sent our own `tx_abort`. @@ -2583,13 +2579,17 @@ impl FundingNegotiation { } impl PendingFunding { - fn can_abandon_funding_negotiation(&self) -> bool { + fn can_abandon_state(&self) -> bool { self.funding_negotiation .as_ref() .map(|funding_negotiation| { !matches!(funding_negotiation, FundingNegotiation::AwaitingSignatures { .. }) }) - .unwrap_or(true) + .unwrap_or_else(|| { + let has_negotiated_candidates = !self.negotiated_candidates.is_empty(); + debug_assert!(has_negotiated_candidates); + !has_negotiated_candidates + }) } fn check_get_splice_locked( @@ -6773,40 +6773,35 @@ where ) } - /// Returns `None` if there is no [`FundedChannel::pending_splice`], otherwise a boolean - /// indicating whether we should reset the splice's [`PendingFunding::funding_negotiation`]. - fn should_reset_pending_splice_funding_negotiation(&self) -> Option { - self.pending_splice.as_ref().map(|pending_splice| { - if pending_splice.can_abandon_funding_negotiation() { - true - } else { - self.context - .interactive_tx_signing_session - .as_ref() - .map(|signing_session| !signing_session.has_received_commitment_signed()) - .unwrap_or_else(|| { - debug_assert!(false); - false - }) - } - }) + fn has_pending_splice_awaiting_signatures(&self) -> bool { + self.pending_splice + .as_ref() + .and_then(|pending_splice| pending_splice.funding_negotiation.as_ref()) + .map(|funding_negotiation| { + matches!(funding_negotiation, FundingNegotiation::AwaitingSignatures { .. }) + }) + .unwrap_or(false) } + /// Returns a boolean indicating whether we should reset the splice's + /// [`PendingFunding::funding_negotiation`]. fn should_reset_pending_splice_state(&self) -> bool { - self.should_reset_pending_splice_funding_negotiation().unwrap_or(true) - && self.pending_funding().is_empty() + self.pending_splice + .as_ref() + .map(|pending_splice| pending_splice.can_abandon_state()) + .unwrap_or(false) } fn reset_pending_splice_state(&mut self) -> bool { - debug_assert!(self.should_reset_pending_splice_funding_negotiation().unwrap_or(true)); + debug_assert!(self.should_reset_pending_splice_state()); + debug_assert!(self.context.interactive_tx_signing_session.is_none()); self.context.channel_state.clear_quiescent(); - self.context.interactive_tx_signing_session.take(); let has_funding_negotiation = self .pending_splice .as_mut() .and_then(|pending_splice| pending_splice.funding_negotiation.take()) .is_some(); - if self.should_reset_pending_splice_state() { + if self.pending_funding().is_empty() { self.pending_splice.take(); } has_funding_negotiation @@ -8948,13 +8943,16 @@ where } self.context.channel_state.clear_local_stfu_sent(); self.context.channel_state.clear_remote_stfu_sent(); - if self.should_reset_pending_splice_funding_negotiation().unwrap_or(true) { - // If we were in quiescence but a splice was never negotiated, or the negotiation - // failed due to disconnecting, we shouldn't be quiescent anymore upon reconnecting. - // If there was a pending splice negotiation that has failed due to disconnecting, - // we also take the opportunity to clean up our state. + if self.should_reset_pending_splice_state() { + // If there was a pending splice negotiation that failed due to disconnecting, we + // also take the opportunity to clean up our state. self.reset_pending_splice_state(); debug_assert!(!self.context.channel_state.is_quiescent()); + } else if !self.has_pending_splice_awaiting_signatures() { + // 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 + self.context.channel_state.clear_quiescent(); } } @@ -13993,10 +13991,13 @@ where } channel_state.clear_local_stfu_sent(); channel_state.clear_remote_stfu_sent(); - if self.should_reset_pending_splice_funding_negotiation().unwrap_or(true) { - // If we were in quiescence but a splice was never negotiated, or the - // negotiation failed due to disconnecting, we shouldn't be quiescent - // anymore upon reconnecting. + if self.should_reset_pending_splice_state() + || !self.has_pending_splice_awaiting_signatures() + { + // 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 channel_state.clear_quiescent(); } }, @@ -14361,19 +14362,10 @@ where let holder_commitment_point_next = self.holder_commitment_point.next_point(); let holder_commitment_point_pending_next = self.holder_commitment_point.pending_next_point; - let interactive_tx_signing_session = - if self.should_reset_pending_splice_funding_negotiation().unwrap_or(false) { - None - } else { - self.context.interactive_tx_signing_session.as_ref() - }; - let pending_splice = if self.should_reset_pending_splice_state() { - None - } else { - // We don't have to worry about resetting the pending `FundingNegotiation` because we - // can only read `FundingNegotiation::AwaitingSignatures` variants anyway. - self.pending_splice.as_ref() - }; + // We don't have to worry about resetting the pending `FundingNegotiation` because we + // can only read `FundingNegotiation::AwaitingSignatures` variants anyway. + let pending_splice = + self.pending_splice.as_ref().filter(|_| !self.should_reset_pending_splice_state()); write_tlv_fields!(writer, { (0, self.context.announcement_sigs, option), @@ -14418,7 +14410,7 @@ where (53, funding_tx_broadcast_safe_event_emitted, option), // Added in 0.0.124 (55, removed_htlc_attribution_data, optional_vec), // Added in 0.2 (57, holding_cell_attribution_data, optional_vec), // Added in 0.2 - (58, interactive_tx_signing_session, option), // Added in 0.2 + (58, self.context.interactive_tx_signing_session, option), // Added in 0.2 (59, self.funding.minimum_depth_override, option), // Added in 0.2 (60, self.context.historical_scids, optional_vec), // Added in 0.2 (61, fulfill_attribution_data, optional_vec), // Added in 0.2 diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index a7d5744390f..1384281e644 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -414,9 +414,9 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { ) .unwrap(); - // Attempt a splice negotiation that only goes up to exchanging `tx_complete`. Reconnecting - // should implicitly abort the negotiation and reset the splice state such that we're able to - // retry another splice later. + // Attempt a splice negotiation that ends mid-construction of the funding transaction. + // Reconnecting should implicitly abort the negotiation and reset the splice state such that + // we're able to retry another splice later. let stfu = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); nodes[1].node.handle_stfu(node_id_0, &stfu); let stfu = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0); @@ -427,18 +427,9 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { let splice_ack = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceAck, node_id_0); nodes[0].node.handle_splice_ack(node_id_1, &splice_ack); - let new_funding_script = chan_utils::make_funding_redeemscript( - &splice_init.funding_pubkey, - &splice_ack.funding_pubkey, - ) - .to_p2wsh(); - let _ = complete_interactive_funding_negotiation( - &nodes[0], - &nodes[1], - channel_id, - contribution.clone(), - new_funding_script, - ); + let tx_add_input = get_event_msg!(nodes[0], MessageSendEvent::SendTxAddInput, node_id_1); + nodes[1].node.handle_tx_add_input(node_id_0, &tx_add_input); + let _ = get_event_msg!(nodes[1], MessageSendEvent::SendTxComplete, node_id_0); if reload { let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode(); From ac6be6834916c707f3070eb1adeb75f5ef18d90c Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 17 Sep 2025 11:43:38 -0700 Subject: [PATCH 5/6] Capture announcement signatures resend in reconnection tests We'll use this in the next commit to test the resend logic for `announcement_signatures` when reestablishing a channel that had a pending splice become locked. --- lightning/src/ln/async_signer_tests.rs | 17 ++++--- lightning/src/ln/chanmon_update_fail_tests.rs | 1 + lightning/src/ln/functional_test_utils.rs | 49 +++++++++++++++++-- lightning/src/ln/functional_tests.rs | 35 ++++++++++++- lightning/src/ln/payment_tests.rs | 1 + lightning/src/ln/quiescence_tests.rs | 1 + lightning/src/ln/reorg_tests.rs | 1 + lightning/src/ln/splicing_tests.rs | 3 ++ 8 files changed, 95 insertions(+), 13 deletions(-) diff --git a/lightning/src/ln/async_signer_tests.rs b/lightning/src/ln/async_signer_tests.rs index de4ee805808..ff4ef50205e 100644 --- a/lightning/src/ln/async_signer_tests.rs +++ b/lightning/src/ln/async_signer_tests.rs @@ -596,7 +596,7 @@ fn do_test_async_raa_peer_disconnect( } // Expect the RAA - let (_, revoke_and_ack, commitment_signed, resend_order) = + let (_, revoke_and_ack, commitment_signed, resend_order, _) = handle_chan_reestablish_msgs!(dst, src); if test_case == UnblockSignerAcrossDisconnectCase::AtEnd { assert!(revoke_and_ack.is_none()); @@ -612,14 +612,14 @@ fn do_test_async_raa_peer_disconnect( dst.node.signer_unblocked(Some((src_node_id, chan_id))); if test_case == UnblockSignerAcrossDisconnectCase::AtEnd { - let (_, revoke_and_ack, commitment_signed, resend_order) = + let (_, revoke_and_ack, commitment_signed, resend_order, _) = handle_chan_reestablish_msgs!(dst, src); assert!(revoke_and_ack.is_some()); assert!(commitment_signed.is_some()); assert!(resend_order == RAACommitmentOrder::RevokeAndACKFirst); } else { // Make sure we don't double send the RAA. - let (_, revoke_and_ack, commitment_signed, _) = handle_chan_reestablish_msgs!(dst, src); + let (_, revoke_and_ack, commitment_signed, _, _) = handle_chan_reestablish_msgs!(dst, src); assert!(revoke_and_ack.is_none()); assert!(commitment_signed.is_none()); } @@ -745,7 +745,7 @@ fn do_test_async_commitment_signature_peer_disconnect( } // Expect the RAA - let (_, revoke_and_ack, commitment_signed, _) = handle_chan_reestablish_msgs!(dst, src); + let (_, revoke_and_ack, commitment_signed, _, _) = handle_chan_reestablish_msgs!(dst, src); assert!(revoke_and_ack.is_some()); if test_case == UnblockSignerAcrossDisconnectCase::AtEnd { assert!(commitment_signed.is_none()); @@ -758,11 +758,11 @@ fn do_test_async_commitment_signature_peer_disconnect( dst.node.signer_unblocked(Some((src_node_id, chan_id))); if test_case == UnblockSignerAcrossDisconnectCase::AtEnd { - let (_, _, commitment_signed, _) = handle_chan_reestablish_msgs!(dst, src); + let (_, _, commitment_signed, _, _) = handle_chan_reestablish_msgs!(dst, src); assert!(commitment_signed.is_some()); } else { // Make sure we don't double send the CS. - let (_, _, commitment_signed, _) = handle_chan_reestablish_msgs!(dst, src); + let (_, _, commitment_signed, _, _) = handle_chan_reestablish_msgs!(dst, src); assert!(commitment_signed.is_none()); } } @@ -877,6 +877,7 @@ fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool) { assert!(as_resp.0.is_none()); assert!(as_resp.1.is_none()); assert!(as_resp.2.is_none()); + assert!(as_resp.4.is_none()); if monitor_update_failure { chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed); @@ -896,6 +897,7 @@ fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool) { assert!(as_resp.0.is_none()); assert!(as_resp.1.is_none()); assert!(as_resp.2.is_none()); + assert!(as_resp.4.is_none()); nodes[0].enable_channel_signer_op(&node_b_id, &chan_id, SignerOp::SignCounterpartyCommitment); nodes[0].node.signer_unblocked(Some((node_b_id, chan_id))); @@ -912,6 +914,9 @@ fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool) { assert!(as_resp.3 == RAACommitmentOrder::CommitmentFirst); + assert!(as_resp.4.is_none()); + assert!(bs_resp.4.is_none()); + // Now that everything is restored, get the CS + RAA and handle them. nodes[1] .node diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index 1bc1bfbc2ff..1a9af4f2071 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -206,6 +206,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) { nodes[1].node.peer_disconnected(node_a_id); 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); } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 6bde99bb59b..aa2c45ca3a5 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -4796,11 +4796,14 @@ macro_rules! handle_chan_reestablish_msgs { None }; - if let Some(&MessageSendEvent::SendAnnouncementSignatures { ref node_id, msg: _ }) = + let mut announcement_sigs = None; // May be now or later + if let Some(&MessageSendEvent::SendAnnouncementSignatures { ref node_id, ref msg }) = msg_events.get(idx) { idx += 1; assert_eq!(*node_id, $dst_node.node.get_our_node_id()); + assert!(announcement_sigs.is_none()); + announcement_sigs = Some(msg.clone()); } let mut had_channel_update = false; // ChannelUpdate may be now or later, but not both @@ -4859,6 +4862,15 @@ macro_rules! handle_chan_reestablish_msgs { } } + if let Some(&MessageSendEvent::SendAnnouncementSignatures { ref node_id, ref msg }) = + msg_events.get(idx) + { + idx += 1; + assert_eq!(*node_id, $dst_node.node.get_our_node_id()); + assert!(announcement_sigs.is_none()); + announcement_sigs = Some(msg.clone()); + } + if let Some(&MessageSendEvent::SendChannelUpdate { ref node_id, .. }) = msg_events.get(idx) { assert_eq!(*node_id, $dst_node.node.get_our_node_id()); @@ -4866,9 +4878,9 @@ macro_rules! handle_chan_reestablish_msgs { assert!(!had_channel_update); } - assert_eq!(msg_events.len(), idx); + assert_eq!(msg_events.len(), idx, "{msg_events:?}"); - (channel_ready, revoke_and_ack, commitment_update, order) + (channel_ready, revoke_and_ack, commitment_update, order, announcement_sigs) }}; } @@ -4876,6 +4888,7 @@ pub struct ReconnectArgs<'a, 'b, 'c, 'd> { pub node_a: &'a Node<'b, 'c, 'd>, pub node_b: &'a Node<'b, 'c, 'd>, pub send_channel_ready: (bool, bool), + pub send_announcement_sigs: (bool, bool), pub pending_responding_commitment_signed: (bool, bool), /// Indicates that the pending responding commitment signed will be a dup for the recipient, /// and no monitor update is expected @@ -4894,6 +4907,7 @@ impl<'a, 'b, 'c, 'd> ReconnectArgs<'a, 'b, 'c, 'd> { node_a, node_b, send_channel_ready: (false, false), + send_announcement_sigs: (false, false), pending_responding_commitment_signed: (false, false), pending_responding_commitment_signed_dup_monitor: (false, false), pending_htlc_adds: (0, 0), @@ -4913,6 +4927,7 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) { node_a, node_b, send_channel_ready, + send_announcement_sigs, pending_htlc_adds, pending_htlc_claims, pending_htlc_fails, @@ -4994,7 +5009,7 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) { && pending_cell_htlc_fails.1 == 0) ); - for chan_msgs in resp_1.drain(..) { + for mut chan_msgs in resp_1.drain(..) { if send_channel_ready.0 { node_a.node.handle_channel_ready(node_b_id, &chan_msgs.0.unwrap()); let announcement_event = node_a.node.get_and_clear_pending_msg_events(); @@ -5009,6 +5024,18 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) { } else { assert!(chan_msgs.0.is_none()); } + if send_announcement_sigs.0 { + let announcement_sigs = chan_msgs.4.take().unwrap(); + node_a.node.handle_announcement_signatures(node_b_id, &announcement_sigs); + let msg_events = node_a.node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1, "{msg_events:?}"); + if let MessageSendEvent::BroadcastChannelAnnouncement { .. } = msg_events[0] { + } else { + panic!("Unexpected event! {:?}", msg_events[0]); + } + } else { + assert!(chan_msgs.4.is_none()); + } if pending_raa.0 { assert!(chan_msgs.3 == RAACommitmentOrder::RevokeAndACKFirst); node_a.node.handle_revoke_and_ack(node_b_id, &chan_msgs.1.unwrap()); @@ -5073,7 +5100,7 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) { } } - for chan_msgs in resp_2.drain(..) { + for mut chan_msgs in resp_2.drain(..) { if send_channel_ready.1 { node_b.node.handle_channel_ready(node_a_id, &chan_msgs.0.unwrap()); let announcement_event = node_b.node.get_and_clear_pending_msg_events(); @@ -5088,6 +5115,18 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) { } else { assert!(chan_msgs.0.is_none()); } + if send_announcement_sigs.1 { + let announcement_sigs = chan_msgs.4.take().unwrap(); + node_b.node.handle_announcement_signatures(node_a_id, &announcement_sigs); + let mut msg_events = node_b.node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1, "{msg_events:?}"); + if let MessageSendEvent::BroadcastChannelAnnouncement { .. } = msg_events.remove(0) { + } else { + panic!(); + } + } else { + assert!(chan_msgs.4.is_none()); + } if pending_raa.1 { assert!(chan_msgs.3 == RAACommitmentOrder::RevokeAndACKFirst); node_b.node.handle_revoke_and_ack(node_a_id, &chan_msgs.1.unwrap()); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index d1c0ac8f12b..d79b3074cc7 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -2559,6 +2559,7 @@ pub fn test_simple_peer_disconnect() { nodes[1].node.peer_disconnected(node_a_id); 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); let payment_preimage_1 = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000).0; @@ -2716,22 +2717,29 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken // received on either side, both sides will need to resend them. let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); reconnect_args.send_channel_ready = (true, true); + if simulate_broken_lnd || messages_delivered > 0 { + reconnect_args.send_announcement_sigs.0 = true; + } + reconnect_args.send_announcement_sigs.1 = true; reconnect_args.pending_htlc_adds.1 = 1; reconnect_nodes(reconnect_args); } else if messages_delivered == 3 { // nodes[0] still wants its RAA + commitment_signed let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); + reconnect_args.send_announcement_sigs = (true, true); reconnect_args.pending_responding_commitment_signed.0 = true; reconnect_args.pending_raa.0 = true; reconnect_nodes(reconnect_args); } else if messages_delivered == 4 { // nodes[0] still wants its commitment_signed let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); + reconnect_args.send_announcement_sigs.0 = true; reconnect_args.pending_responding_commitment_signed.0 = true; reconnect_nodes(reconnect_args); } else if messages_delivered == 5 { // nodes[1] still wants its final RAA let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); + reconnect_args.send_announcement_sigs.0 = true; reconnect_args.pending_raa.1 = true; reconnect_nodes(reconnect_args); } else if messages_delivered == 6 { @@ -2752,7 +2760,16 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken nodes[0].node.peer_disconnected(node_b_id); nodes[1].node.peer_disconnected(node_a_id); - reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1])); + let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); + if !simulate_broken_lnd + && (messages_delivered == 0 || (messages_delivered > 2 && messages_delivered < 6)) + { + reconnect_args.send_announcement_sigs.0 = true; + } + if messages_delivered < 4 { + reconnect_args.send_announcement_sigs.1 = true; + } + reconnect_nodes(reconnect_args); nodes[1].node.process_pending_htlc_forwards(); @@ -2850,6 +2867,10 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken nodes[1].node.peer_disconnected(node_a_id); if messages_delivered < 2 { let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); + if !simulate_broken_lnd && messages_delivered == 0 { + reconnect_args.send_announcement_sigs.0 = true; + } + reconnect_args.send_announcement_sigs.1 = true; reconnect_args.pending_htlc_claims.0 = 1; reconnect_nodes(reconnect_args); if messages_delivered < 1 { @@ -2860,12 +2881,14 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken } else if messages_delivered == 2 { // nodes[0] still wants its RAA + commitment_signed let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); + reconnect_args.send_announcement_sigs.1 = true; reconnect_args.pending_responding_commitment_signed.1 = true; reconnect_args.pending_raa.1 = true; reconnect_nodes(reconnect_args); } else if messages_delivered == 3 { // nodes[0] still wants its commitment_signed let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); + reconnect_args.send_announcement_sigs.1 = true; reconnect_args.pending_responding_commitment_signed.1 = true; reconnect_nodes(reconnect_args); } else if messages_delivered == 4 { @@ -2885,7 +2908,15 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken nodes[0].node.peer_disconnected(node_b_id); nodes[1].node.peer_disconnected(node_a_id); } - reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1])); + let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); + if !simulate_broken_lnd { + if messages_delivered == 0 { + reconnect_args.send_announcement_sigs.0 = true; + } else if messages_delivered == 2 || messages_delivered == 3 { + reconnect_args.send_announcement_sigs.1 = true; + } + } + reconnect_nodes(reconnect_args); if messages_delivered > 2 { expect_payment_path_successful!(nodes[0]); diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index d2479bbb0e5..9eb85173a83 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -4858,6 +4858,7 @@ fn do_test_payment_metadata_consistency(do_reload: bool, do_modify: bool) { } let mut reconnect_args = ReconnectArgs::new(&nodes[2], &nodes[3]); reconnect_args.send_channel_ready = (true, true); + reconnect_args.send_announcement_sigs = (true, true); reconnect_nodes(reconnect_args); // Create a new channel between C and D as A will refuse to retry on the existing one because diff --git a/lightning/src/ln/quiescence_tests.rs b/lightning/src/ln/quiescence_tests.rs index 63c50094a67..7a5d35b12a5 100644 --- a/lightning/src/ln/quiescence_tests.rs +++ b/lightning/src/ln/quiescence_tests.rs @@ -755,6 +755,7 @@ fn do_test_quiescence_termination_on_disconnect(reload: bool) { 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); send_payment(&nodes[0], &[&nodes[1]], 1_000_000); diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs index 90cd459938e..b040f454e61 100644 --- a/lightning/src/ln/reorg_tests.rs +++ b/lightning/src/ln/reorg_tests.rs @@ -333,6 +333,7 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool, use_funding_ // generate an error message we can handle below. 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); } } diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 1384281e644..fc062d614d0 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -401,6 +401,7 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { 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); nodes[0] @@ -457,6 +458,7 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { 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); // Attempt a splice negotiation that completes, (i.e. `tx_signatures` are exchanged). Reconnecting @@ -488,6 +490,7 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { } let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); + reconnect_args.send_announcement_sigs = (true, true); reconnect_nodes(reconnect_args); mine_transaction(&nodes[0], &splice_tx); From 5594f9b66e6ff11d661e118bc090fb3527e27990 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Tue, 16 Sep 2025 23:56:47 -0700 Subject: [PATCH 6/6] Test channel reestablish during splice lifecycle This test captures all the new spec requirements introduced for a splice to the channel reestablish flow. --- lightning/src/ln/async_signer_tests.rs | 18 +- lightning/src/ln/functional_test_utils.rs | 61 +++- lightning/src/ln/splicing_tests.rs | 343 +++++++++++++++++++--- 3 files changed, 376 insertions(+), 46 deletions(-) diff --git a/lightning/src/ln/async_signer_tests.rs b/lightning/src/ln/async_signer_tests.rs index ff4ef50205e..de5103aeba9 100644 --- a/lightning/src/ln/async_signer_tests.rs +++ b/lightning/src/ln/async_signer_tests.rs @@ -596,7 +596,7 @@ fn do_test_async_raa_peer_disconnect( } // Expect the RAA - let (_, revoke_and_ack, commitment_signed, resend_order, _) = + let (_, revoke_and_ack, commitment_signed, resend_order, _, _) = handle_chan_reestablish_msgs!(dst, src); if test_case == UnblockSignerAcrossDisconnectCase::AtEnd { assert!(revoke_and_ack.is_none()); @@ -612,14 +612,15 @@ fn do_test_async_raa_peer_disconnect( dst.node.signer_unblocked(Some((src_node_id, chan_id))); if test_case == UnblockSignerAcrossDisconnectCase::AtEnd { - let (_, revoke_and_ack, commitment_signed, resend_order, _) = + let (_, revoke_and_ack, commitment_signed, resend_order, _, _) = handle_chan_reestablish_msgs!(dst, src); assert!(revoke_and_ack.is_some()); assert!(commitment_signed.is_some()); assert!(resend_order == RAACommitmentOrder::RevokeAndACKFirst); } else { // Make sure we don't double send the RAA. - let (_, revoke_and_ack, commitment_signed, _, _) = handle_chan_reestablish_msgs!(dst, src); + let (_, revoke_and_ack, commitment_signed, _, _, _) = + handle_chan_reestablish_msgs!(dst, src); assert!(revoke_and_ack.is_none()); assert!(commitment_signed.is_none()); } @@ -745,7 +746,7 @@ fn do_test_async_commitment_signature_peer_disconnect( } // Expect the RAA - let (_, revoke_and_ack, commitment_signed, _, _) = handle_chan_reestablish_msgs!(dst, src); + let (_, revoke_and_ack, commitment_signed, _, _, _) = handle_chan_reestablish_msgs!(dst, src); assert!(revoke_and_ack.is_some()); if test_case == UnblockSignerAcrossDisconnectCase::AtEnd { assert!(commitment_signed.is_none()); @@ -758,11 +759,11 @@ fn do_test_async_commitment_signature_peer_disconnect( dst.node.signer_unblocked(Some((src_node_id, chan_id))); if test_case == UnblockSignerAcrossDisconnectCase::AtEnd { - let (_, _, commitment_signed, _, _) = handle_chan_reestablish_msgs!(dst, src); + let (_, _, commitment_signed, _, _, _) = handle_chan_reestablish_msgs!(dst, src); assert!(commitment_signed.is_some()); } else { // Make sure we don't double send the CS. - let (_, _, commitment_signed, _, _) = handle_chan_reestablish_msgs!(dst, src); + let (_, _, commitment_signed, _, _, _) = handle_chan_reestablish_msgs!(dst, src); assert!(commitment_signed.is_none()); } } @@ -878,6 +879,7 @@ fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool) { assert!(as_resp.1.is_none()); assert!(as_resp.2.is_none()); assert!(as_resp.4.is_none()); + assert!(as_resp.5.is_none()); if monitor_update_failure { chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed); @@ -898,6 +900,7 @@ fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool) { assert!(as_resp.1.is_none()); assert!(as_resp.2.is_none()); assert!(as_resp.4.is_none()); + assert!(as_resp.5.is_none()); nodes[0].enable_channel_signer_op(&node_b_id, &chan_id, SignerOp::SignCounterpartyCommitment); nodes[0].node.signer_unblocked(Some((node_b_id, chan_id))); @@ -917,6 +920,9 @@ fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool) { assert!(as_resp.4.is_none()); assert!(bs_resp.4.is_none()); + assert!(as_resp.5.is_none()); + assert!(bs_resp.5.is_none()); + // Now that everything is restored, get the CS + RAA and handle them. nodes[1] .node diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index aa2c45ca3a5..fbfe320d9ea 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -4862,6 +4862,15 @@ macro_rules! handle_chan_reestablish_msgs { } } + let mut tx_signatures = None; + if let Some(&MessageSendEvent::SendTxSignatures { ref node_id, ref msg }) = + msg_events.get(idx) + { + assert_eq!(*node_id, $dst_node.node.get_our_node_id()); + tx_signatures = Some(msg.clone()); + idx += 1; + } + if let Some(&MessageSendEvent::SendAnnouncementSignatures { ref node_id, ref msg }) = msg_events.get(idx) { @@ -4880,7 +4889,7 @@ macro_rules! handle_chan_reestablish_msgs { assert_eq!(msg_events.len(), idx, "{msg_events:?}"); - (channel_ready, revoke_and_ack, commitment_update, order, announcement_sigs) + (channel_ready, revoke_and_ack, commitment_update, order, announcement_sigs, tx_signatures) }}; } @@ -4889,6 +4898,9 @@ pub struct ReconnectArgs<'a, 'b, 'c, 'd> { pub node_b: &'a Node<'b, 'c, 'd>, pub send_channel_ready: (bool, bool), pub send_announcement_sigs: (bool, bool), + pub send_interactive_tx_commit_sig: (bool, bool), + pub send_interactive_tx_sigs: (bool, bool), + pub expect_renegotiated_funding_locked_monitor_update: (bool, bool), pub pending_responding_commitment_signed: (bool, bool), /// Indicates that the pending responding commitment signed will be a dup for the recipient, /// and no monitor update is expected @@ -4908,6 +4920,9 @@ impl<'a, 'b, 'c, 'd> ReconnectArgs<'a, 'b, 'c, 'd> { node_b, send_channel_ready: (false, false), send_announcement_sigs: (false, false), + send_interactive_tx_commit_sig: (false, false), + send_interactive_tx_sigs: (false, false), + expect_renegotiated_funding_locked_monitor_update: (false, false), pending_responding_commitment_signed: (false, false), pending_responding_commitment_signed_dup_monitor: (false, false), pending_htlc_adds: (0, 0), @@ -4928,6 +4943,9 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) { node_b, send_channel_ready, send_announcement_sigs, + send_interactive_tx_commit_sig, + send_interactive_tx_sigs, + expect_renegotiated_funding_locked_monitor_update, pending_htlc_adds, pending_htlc_claims, pending_htlc_fails, @@ -4978,7 +4996,11 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) { node_b.node.handle_channel_reestablish(node_a_id, &msg); resp_1.push(handle_chan_reestablish_msgs!(node_b, node_a)); } - if pending_cell_htlc_claims.0 != 0 || pending_cell_htlc_fails.0 != 0 { + + if pending_cell_htlc_claims.0 != 0 + || pending_cell_htlc_fails.0 != 0 + || expect_renegotiated_funding_locked_monitor_update.1 + { check_added_monitors!(node_b, 1); } else { check_added_monitors!(node_b, 0); @@ -4989,7 +5011,10 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) { node_a.node.handle_channel_reestablish(node_b_id, &msg); resp_2.push(handle_chan_reestablish_msgs!(node_a, node_b)); } - if pending_cell_htlc_claims.1 != 0 || pending_cell_htlc_fails.1 != 0 { + if pending_cell_htlc_claims.1 != 0 + || pending_cell_htlc_fails.1 != 0 + || expect_renegotiated_funding_locked_monitor_update.0 + { check_added_monitors!(node_a, 1); } else { check_added_monitors!(node_a, 0); @@ -5036,6 +5061,21 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) { } else { assert!(chan_msgs.4.is_none()); } + if send_interactive_tx_commit_sig.0 { + assert!(chan_msgs.1.is_none()); + let commitment_update = chan_msgs.2.take().unwrap(); + assert_eq!(commitment_update.commitment_signed.len(), 1); + node_a.node.handle_commitment_signed_batch_test( + node_b_id, + &commitment_update.commitment_signed, + ) + } + if send_interactive_tx_sigs.0 { + let tx_signatures = chan_msgs.5.take().unwrap(); + node_a.node.handle_tx_signatures(node_b_id, &tx_signatures); + } else { + assert!(chan_msgs.5.is_none()); + } if pending_raa.0 { assert!(chan_msgs.3 == RAACommitmentOrder::RevokeAndACKFirst); node_a.node.handle_revoke_and_ack(node_b_id, &chan_msgs.1.unwrap()); @@ -5127,6 +5167,21 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) { } else { assert!(chan_msgs.4.is_none()); } + if send_interactive_tx_commit_sig.1 { + assert!(chan_msgs.1.is_none()); + let commitment_update = chan_msgs.2.take().unwrap(); + assert_eq!(commitment_update.commitment_signed.len(), 1); + node_b.node.handle_commitment_signed_batch_test( + node_a_id, + &commitment_update.commitment_signed, + ) + } + if send_interactive_tx_sigs.1 { + let tx_signatures = chan_msgs.5.take().unwrap(); + node_b.node.handle_tx_signatures(node_a_id, &tx_signatures); + } else { + assert!(chan_msgs.5.is_none()); + } if pending_raa.1 { assert!(chan_msgs.3 == RAACommitmentOrder::RevokeAndACKFirst); node_b.node.handle_revoke_and_ack(node_a_id, &chan_msgs.1.unwrap()); diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index fc062d614d0..62e1064acc0 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -10,6 +10,7 @@ use crate::chain::chaininterface::FEERATE_FLOOR_SATS_PER_KW; use crate::chain::channelmonitor::{ANTI_REORG_DELAY, LATENCY_GRACE_PERIOD_BLOCKS}; use crate::chain::transaction::OutPoint; +use crate::chain::ChannelMonitorUpdateStatus; use crate::events::bump_transaction::sync::WalletSourceSync; use crate::events::{ClosureReason, Event, FundingInfo, HTLCHandlingFailureType}; use crate::ln::chan_utils; @@ -63,6 +64,49 @@ fn test_v1_splice_in_negative_insufficient_inputs() { } } +fn negotiate_splice_tx<'a, 'b, 'c, 'd>( + initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, + initiator_contribution: SpliceContribution, +) -> msgs::CommitmentSigned { + let node_id_initiator = initiator.node.get_our_node_id(); + let node_id_acceptor = acceptor.node.get_our_node_id(); + + initiator + .node + .splice_channel( + &channel_id, + &node_id_acceptor, + initiator_contribution.clone(), + FEERATE_FLOOR_SATS_PER_KW, + None, + ) + .unwrap(); + + let stfu_init = get_event_msg!(initiator, MessageSendEvent::SendStfu, node_id_acceptor); + acceptor.node.handle_stfu(node_id_initiator, &stfu_init); + let stfu_ack = get_event_msg!(acceptor, MessageSendEvent::SendStfu, node_id_initiator); + initiator.node.handle_stfu(node_id_acceptor, &stfu_ack); + + let splice_init = get_event_msg!(initiator, MessageSendEvent::SendSpliceInit, node_id_acceptor); + acceptor.node.handle_splice_init(node_id_initiator, &splice_init); + let splice_ack = get_event_msg!(acceptor, MessageSendEvent::SendSpliceAck, node_id_initiator); + initiator.node.handle_splice_ack(node_id_acceptor, &splice_ack); + + let new_funding_script = chan_utils::make_funding_redeemscript( + &splice_init.funding_pubkey, + &splice_ack.funding_pubkey, + ) + .to_p2wsh(); + + complete_interactive_funding_negotiation( + initiator, + acceptor, + channel_id, + initiator_contribution, + new_funding_script, + ) +} + fn complete_interactive_funding_negotiation<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, initiator_contribution: SpliceContribution, new_funding_script: ScriptBuf, @@ -202,43 +246,8 @@ fn splice_channel<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, initiator_contribution: SpliceContribution, ) -> Transaction { - let node_id_initiator = initiator.node.get_our_node_id(); - let node_id_acceptor = acceptor.node.get_our_node_id(); - - initiator - .node - .splice_channel( - &channel_id, - &node_id_acceptor, - initiator_contribution.clone(), - FEERATE_FLOOR_SATS_PER_KW, - None, - ) - .unwrap(); - - let stfu_init = get_event_msg!(initiator, MessageSendEvent::SendStfu, node_id_acceptor); - acceptor.node.handle_stfu(node_id_initiator, &stfu_init); - let stfu_ack = get_event_msg!(acceptor, MessageSendEvent::SendStfu, node_id_initiator); - initiator.node.handle_stfu(node_id_acceptor, &stfu_ack); - - let splice_init = get_event_msg!(initiator, MessageSendEvent::SendSpliceInit, node_id_acceptor); - acceptor.node.handle_splice_init(node_id_initiator, &splice_init); - let splice_ack = get_event_msg!(acceptor, MessageSendEvent::SendSpliceAck, node_id_initiator); - initiator.node.handle_splice_ack(node_id_acceptor, &splice_ack); - - let new_funding_script = chan_utils::make_funding_redeemscript( - &splice_init.funding_pubkey, - &splice_ack.funding_pubkey, - ) - .to_p2wsh(); - - let initial_commit_sig_for_acceptor = complete_interactive_funding_negotiation( - initiator, - acceptor, - channel_id, - initiator_contribution, - new_funding_script, - ); + let initial_commit_sig_for_acceptor = + negotiate_splice_tx(initiator, acceptor, channel_id, initiator_contribution); sign_interactive_funding_transaction(initiator, acceptor, initial_commit_sig_for_acceptor); let splice_tx = { @@ -785,3 +794,263 @@ fn do_test_splice_commitment_broadcast(splice_status: SpliceStatus, claim_htlcs: } } } + +#[test] +fn test_splice_reestablish() { + do_test_splice_reestablish(false, false); + do_test_splice_reestablish(false, true); + do_test_splice_reestablish(true, false); + do_test_splice_reestablish(true, true); +} + +fn do_test_splice_reestablish(reload: bool, async_monitor_update: bool) { + // Test that we're able to reestablish the channel succesfully throughout the lifecycle of a splice. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let (persister_0a, persister_0b, persister_1a, persister_1b); + let (chain_monitor_0a, chain_monitor_0b, chain_monitor_1a, chain_monitor_1b); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let (node_0a, node_0b, node_1a, node_1b); + 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[0], channel_id).get_funding_txo(); + let prev_funding_script = get_monitor!(nodes[0], channel_id).get_funding_script(); + + // Keep a pending HTLC throughout the reestablish flow to make sure we can handle them. + route_payment(&nodes[0], &[&nodes[1]], 1_000_000); + + // Negotiate the splice up until the nodes exchange `tx_complete`. + let initiator_contribution = SpliceContribution::SpliceOut { + 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 initial_commit_sig_for_acceptor = + negotiate_splice_tx(&nodes[0], &nodes[1], channel_id, initiator_contribution); + assert_eq!(initial_commit_sig_for_acceptor.htlc_signatures.len(), 1); + let initial_commit_sig_for_initiator = get_htlc_update_msgs!(&nodes[1], node_id_0); + assert_eq!(initial_commit_sig_for_initiator.commitment_signed.len(), 1); + assert_eq!(initial_commit_sig_for_initiator.commitment_signed[0].htlc_signatures.len(), 1); + + macro_rules! reconnect_nodes { + ($f: expr) => { + nodes[0].node.peer_disconnected(node_id_1); + nodes[1].node.peer_disconnected(node_id_0); + let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); + $f(&mut reconnect_args); + reconnect_nodes(reconnect_args); + }; + } + + // Reestablishing now should force both nodes to retransmit their initial `commitment_signed` + // message as they were never delivered. + if reload { + let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode(); + reload_node!( + nodes[0], + nodes[0].node.encode(), + &[&encoded_monitor_0], + persister_0a, + chain_monitor_0a, + node_0a + ); + let encoded_monitor_1 = get_monitor!(nodes[1], channel_id).encode(); + reload_node!( + nodes[1], + nodes[1].node.encode(), + &[&encoded_monitor_1], + persister_1a, + chain_monitor_1a, + node_1a + ); + if async_monitor_update { + persister_0a.set_update_ret(ChannelMonitorUpdateStatus::InProgress); + persister_1a.set_update_ret(ChannelMonitorUpdateStatus::InProgress); + } + } else { + nodes[0].node.peer_disconnected(node_id_1); + nodes[1].node.peer_disconnected(node_id_0); + if async_monitor_update { + chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress); + chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress); + } + } + + let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); + reconnect_args.send_interactive_tx_commit_sig = (true, true); + reconnect_nodes(reconnect_args); + + // The `commitment_signed` messages were delivered in the reestablishment, so we should expect + // to see a `RenegotiatedFunding` monitor update on both nodes. + check_added_monitors(&nodes[0], 1); + check_added_monitors(&nodes[1], 1); + + if async_monitor_update { + // Reconnecting again should result in no messages/events being generated as the monitor + // update is pending. + reconnect_nodes!(|_| {}); + 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()); + nodes[0].chain_monitor.complete_sole_pending_chan_update(&channel_id); + nodes[1].chain_monitor.complete_sole_pending_chan_update(&channel_id); + chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed); + chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed); + } + + // Node 0 should have a signing event to handle since they had a contribution in the splice. + // Node 1 won't and will immediately send `tx_signatures`. + let _ = get_event!(nodes[0], Event::FundingTransactionReadyForSigning); + assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); + let _ = get_event_msg!(nodes[1], MessageSendEvent::SendTxSignatures, node_id_0); + + // Reconnecting now should force node 1 to retransmit their `tx_signatures` since it was never + // delivered. Node 0 still hasn't called back with `funding_transaction_signed`, so its + // `tx_signatures` is not ready yet. + reconnect_nodes!(|reconnect_args: &mut ReconnectArgs| { + reconnect_args.send_interactive_tx_sigs = (true, false); + }); + let _ = get_event!(nodes[0], Event::FundingTransactionReadyForSigning); + + // Reconnect again to make sure node 1 doesn't retransmit `tx_signatures` unnecessarily as it + // was delivered in the previous reestablishment. + reconnect_nodes!(|_| {}); + + // Have node 0 sign, we should see its `tx_signatures` go out. + let event = get_event!(nodes[0], Event::FundingTransactionReadyForSigning); + if let Event::FundingTransactionReadyForSigning { unsigned_transaction, .. } = event { + let tx = nodes[0].wallet_source.sign_tx(unsigned_transaction).unwrap(); + nodes[0].node.funding_transaction_signed(&channel_id, &node_id_1, tx).unwrap(); + } + let _ = get_event_msg!(nodes[0], MessageSendEvent::SendTxSignatures, node_id_1); + + // Reconnect to make sure node 0 retransmits its `tx_signatures` as it was never delivered. + reconnect_nodes!(|reconnect_args: &mut ReconnectArgs| { + reconnect_args.send_interactive_tx_sigs = (false, true); + }); + + // Reestablish the channel again to make sure node 0 doesn't retransmit `tx_signatures` + // unnecessarily as it was delivered in the previous reestablishment. + if reload { + let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode(); + reload_node!( + nodes[0], + nodes[0].node.encode(), + &[&encoded_monitor_0], + persister_0b, + chain_monitor_0b, + node_0b + ); + let encoded_monitor_1 = get_monitor!(nodes[1], channel_id).encode(); + reload_node!( + nodes[1], + nodes[1].node.encode(), + &[&encoded_monitor_1], + persister_1b, + chain_monitor_1b, + node_1b + ); + } else { + nodes[0].node.peer_disconnected(node_id_1); + nodes[1].node.peer_disconnected(node_id_0); + } + reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1])); + + // The channel should no longer be quiescent with `tx_signatures` exchanged. We should expect to + // see the splice transaction broadcast. + let splice_tx = { + let mut txn_0 = nodes[0].tx_broadcaster.txn_broadcast(); + assert_eq!(txn_0.len(), 1); + let txn_1 = nodes[1].tx_broadcaster.txn_broadcast(); + assert_eq!(txn_0, txn_1); + txn_0.remove(0) + }; + + // Make sure we can still send payments. + send_payment(&nodes[0], &[&nodes[1]], 1_000_000); + + // Lock in the splice on node 0. We should see its `splice_locked` sent. + confirm_transaction(&nodes[0], &splice_tx); + let _ = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceLocked, node_id_1); + + // Confirm the splice but with one less confirmation than required on node 1. Its + // `splice_locked` should no be sent yet. + mine_transaction(&nodes[1], &splice_tx); + connect_blocks(&nodes[1], ANTI_REORG_DELAY - 2); + assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); + + // Reconnect the nodes. Node 1 should assume node 0's `splice_locked` via + // `ChannelReestablish::my_current_funding_locked`. + reconnect_nodes!(|_| {}); + + if async_monitor_update { + chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress); + chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress); + } + + // Mine the remaining block on node 1 for the splice to be locked. Since `splice_locked` has now + // been exchanged on node 1, we should see its `announcement_signatures` sent as well, and the + // `RenegotiatedFundingLocked` monitor update. + connect_blocks(&nodes[1], 1); + check_added_monitors(&nodes[1], 1); + let mut msg_events = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 2, "{msg_events:?}"); + if let MessageSendEvent::SendSpliceLocked { .. } = msg_events.remove(0) { + } else { + panic!() + } + if let MessageSendEvent::SendAnnouncementSignatures { .. } = msg_events.remove(0) { + } else { + panic!() + } + expect_channel_ready_event(&nodes[1], &node_id_0); + + // Reconnect the nodes to ensure node 1 retransmits its `splice_locked` (implicitly via + // `my_current_funding_locked`) and `announcement_signatures` to node 0. + reconnect_nodes!(|reconnect_args: &mut ReconnectArgs| { + reconnect_args.expect_renegotiated_funding_locked_monitor_update = (true, false); + reconnect_args.send_announcement_sigs = (true, true); + }); + expect_channel_ready_event(&nodes[0], &node_id_1); + + if async_monitor_update { + nodes[0].chain_monitor.complete_sole_pending_chan_update(&channel_id); + nodes[1].chain_monitor.complete_sole_pending_chan_update(&channel_id); + chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed); + chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed); + } + + // We shouldn't have any further events or messages to process. + 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()); + + // Make sure we can still send payments. + send_payment(&nodes[0], &[&nodes[1]], 1_000_000); + + // Remove the previous funding info the chain source was watching to avoid failing the + // end-of-test sanity checks. + 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); +}