From 48a08625dc0095fffdf26727fa98eaceb73d5e4a Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 9 Jul 2025 16:12:35 -0500 Subject: [PATCH 1/4] Enter FundingNegotiated state after constructing funding tx The ChannelState::NegotiatingFunding assertion check in ChannelContext::get_initial_commitment_signed will fail when implementing splicing's channel_reestablish logic. In order to support it and channel establishment, enter ChannelState::FundingNegotiated prior to calling the method and update the assertion accordingly. Also allows writing a channel in the FundedNegotiated state when an interactive signing session is active. This is necessary as it indicates a previously funded channel being spliced. --- lightning/src/ln/channel.rs | 19 +++++-------------- lightning/src/ln/splicing_tests.rs | 24 +----------------------- 2 files changed, 6 insertions(+), 37 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 0796369d4c6..bab0ae684cd 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -5517,6 +5517,9 @@ where funding .channel_transaction_parameters.funding_outpoint = Some(outpoint); + self.channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new()); + self.channel_state.set_interactive_signing(); + if is_splice { debug_assert_eq!( holder_commitment_transaction_number, @@ -5580,10 +5583,6 @@ where }); }; - let mut channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new()); - channel_state.set_interactive_signing(); - self.channel_state = channel_state; - Ok((commitment_signed, funding_ready_for_sig_event)) } @@ -5636,16 +5635,7 @@ where SP::Target: SignerProvider, L::Target: Logger { - if !matches!( - self.channel_state, ChannelState::NegotiatingFunding(flags) - if flags == (NegotiatingFundingFlags::OUR_INIT_SENT | NegotiatingFundingFlags::THEIR_INIT_SENT) - ) { - debug_assert!(false); - let msg = "Tried to get an initial commitment_signed messsage at a time other than \ - immediately after initial handshake completion (or tried to get funding_created twice)"; - let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; - return Err(ChannelError::Close((msg.to_owned(), reason))); - } + assert!(matches!(self.channel_state, ChannelState::FundingNegotiated(flags) if flags.is_interactive_signing())); let signature = match self.get_initial_counterparty_commitment_signature(funding, logger) { Ok(res) => res, @@ -12880,6 +12870,7 @@ where channel_state.clear_remote_stfu_sent(); channel_state.clear_quiescent(); }, + ChannelState::FundingNegotiated(flags) if flags.is_interactive_signing() => {}, _ => debug_assert!(false, "Pre-funded/shutdown channels should not be written"), } channel_state.set_peer_disconnected(); diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index a88a5f76c7e..51f9f2e387e 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -274,29 +274,7 @@ fn test_v1_splice_in() { _ => panic!("Unexpected event {:?}", events[1]), } - // TODO(splicing): Continue with commitment flow, new tx confirmation - - // === Close channel, cooperatively - initiator_node.node.close_channel(&channel_id, &acceptor_node.node.get_our_node_id()).unwrap(); - let node0_shutdown_message = get_event_msg!( - initiator_node, - MessageSendEvent::SendShutdown, - acceptor_node.node.get_our_node_id() - ); - acceptor_node - .node - .handle_shutdown(initiator_node.node.get_our_node_id(), &node0_shutdown_message); - let nodes_1_shutdown = get_event_msg!( - acceptor_node, - MessageSendEvent::SendShutdown, - initiator_node.node.get_our_node_id() - ); - initiator_node.node.handle_shutdown(acceptor_node.node.get_our_node_id(), &nodes_1_shutdown); - let _ = get_event_msg!( - initiator_node, - MessageSendEvent::SendClosingSigned, - acceptor_node.node.get_our_node_id() - ); + // TODO(splicing): Continue with commitment flow, new tx confirmation, and shutdown } #[test] From 29081e7a1b9090b378ad25bc780bae53bf3819e0 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 9 Jul 2025 16:22:27 -0500 Subject: [PATCH 2/4] Return an Option from ChannelContext::get_initial_commitment_signed When ChannelContext::get_initial_commitment_signed is called for V2 channel establishment, any errors should result in closing the channel. However, in the future, when this is used for splicing it should abort instead of closing the channel. Move the error construction to the call sites in anticipation of this. --- lightning/src/ln/channel.rs | 73 +++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 40 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index bab0ae684cd..be7dbaa23dc 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1009,16 +1009,6 @@ impl ChannelError { pub(super) fn close(err: String) -> Self { ChannelError::Close((err.clone(), ClosureReason::ProcessingError { err })) } - - pub(super) fn message(&self) -> &str { - match self { - &ChannelError::Ignore(ref e) => &e, - &ChannelError::Warn(ref e) => &e, - &ChannelError::WarnAndDisconnect(ref e) => &e, - &ChannelError::Close((ref e, _)) => &e, - &ChannelError::SendError(ref e) => &e, - } - } } pub(super) struct WithChannelContext<'a, L: Deref> @@ -5536,12 +5526,13 @@ where let commitment_signed = self.get_initial_commitment_signed(&funding, logger); let commitment_signed = match commitment_signed { - Ok(commitment_signed) => commitment_signed, - Err(e) => { + Some(commitment_signed) => commitment_signed, + // TODO(splicing): Support async signing + None => { funding.channel_transaction_parameters.funding_outpoint = None; return Err(msgs::TxAbort { channel_id: self.channel_id(), - data: e.message().to_owned().into_bytes(), + data: "Failed to get signature for commitment_signed".to_owned().into_bytes(), }); }, }; @@ -5600,7 +5591,7 @@ where #[rustfmt::skip] fn get_initial_counterparty_commitment_signature( &self, funding: &FundingScope, logger: &L - ) -> Result + ) -> Option where SP::Target: SignerProvider, L::Target: Logger @@ -5615,11 +5606,7 @@ where let channel_parameters = &funding.channel_transaction_parameters; ecdsa.sign_counterparty_commitment(channel_parameters, &counterparty_initial_commitment_tx, Vec::new(), Vec::new(), &self.secp_ctx) .map(|(signature, _)| signature) - .map_err(|()| { - let msg = "Failed to get signatures for new commitment_signed"; - let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; - ChannelError::Close((msg.to_owned(), reason)) - }) + .ok() }, // TODO (taproot|arik) #[cfg(taproot)] @@ -5630,38 +5617,35 @@ where #[rustfmt::skip] fn get_initial_commitment_signed( &mut self, funding: &FundingScope, logger: &L - ) -> Result + ) -> Option where SP::Target: SignerProvider, L::Target: Logger { assert!(matches!(self.channel_state, ChannelState::FundingNegotiated(flags) if flags.is_interactive_signing())); - let signature = match self.get_initial_counterparty_commitment_signature(funding, logger) { - Ok(res) => res, - Err(e) => { - log_error!(logger, "Got bad signatures: {:?}!", e); - return Err(e); - } - }; - - log_info!(logger, "Generated commitment_signed for peer for channel {}", &self.channel_id()); - - Ok(msgs::CommitmentSigned { - channel_id: self.channel_id, - htlc_signatures: vec![], - signature, - funding_txid: funding.get_funding_txo().map(|funding_txo| funding_txo.txid), - #[cfg(taproot)] - partial_signature_with_nonce: None, - }) + let signature = self.get_initial_counterparty_commitment_signature(funding, logger); + if let Some(signature) = signature { + log_info!(logger, "Generated commitment_signed for peer for channel {}", &self.channel_id()); + Some(msgs::CommitmentSigned { + channel_id: self.channel_id, + htlc_signatures: vec![], + signature, + funding_txid: funding.get_funding_txo().map(|funding_txo| funding_txo.txid), + #[cfg(taproot)] + partial_signature_with_nonce: None, + }) + } else { + // TODO(splicing): Support async signing + None + } } #[cfg(all(test))] pub fn get_initial_counterparty_commitment_signature_for_test( &mut self, funding: &mut FundingScope, logger: &L, counterparty_cur_commitment_point_override: PublicKey, - ) -> Result + ) -> Option where SP::Target: SignerProvider, L::Target: Logger, @@ -8814,7 +8798,16 @@ where // if it has not received tx_signatures for that funding transaction AND // if next_commitment_number is zero: // MUST retransmit its commitment_signed for that funding transaction. - let commitment_signed = self.context.get_initial_commitment_signed(&self.funding, logger)?; + let commitment_signed = self.context.get_initial_commitment_signed(&self.funding, logger) + // TODO(splicing): Support async signing + .ok_or_else(|| { + let message = "Failed to get signatures for new commitment_signed".to_owned(); + ChannelError::Close( + ( + message.clone(), + ClosureReason::HolderForceClosed { message, broadcasted_latest_txn: Some(false) }, + ) + )})?; Some(msgs::CommitmentUpdate { commitment_signed: vec![commitment_signed], update_add_htlcs: vec![], From c51b6ff676dd1c826afc29c26dbc8045021faa13 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 11 Jul 2025 12:12:34 -0500 Subject: [PATCH 3/4] Use consistent initial commitment_signed naming --- lightning/src/ln/channel.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index be7dbaa23dc..263a98cf617 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1878,7 +1878,7 @@ where #[cfg(splicing)] pending_splice: None, }; - let res = funded_channel.commitment_signed_initial_v2(msg, best_block, signer_provider, logger) + let res = funded_channel.initial_commitment_signed_v2(msg, best_block, signer_provider, logger) .map(|monitor| (Some(monitor), None)) // TODO: Change to `inspect_err` when MSRV is high enough. .map_err(|err| { @@ -5524,7 +5524,7 @@ where self.assert_no_commitment_advancement(holder_commitment_transaction_number, "initial commitment_signed"); } - let commitment_signed = self.get_initial_commitment_signed(&funding, logger); + let commitment_signed = self.get_initial_commitment_signed_v2(&funding, logger); let commitment_signed = match commitment_signed { Some(commitment_signed) => commitment_signed, // TODO(splicing): Support async signing @@ -5615,7 +5615,7 @@ where } #[rustfmt::skip] - fn get_initial_commitment_signed( + fn get_initial_commitment_signed_v2( &mut self, funding: &FundingScope, logger: &L ) -> Option where @@ -6929,7 +6929,7 @@ where } #[rustfmt::skip] - pub fn commitment_signed_initial_v2( + pub fn initial_commitment_signed_v2( &mut self, msg: &msgs::CommitmentSigned, best_block: BestBlock, signer_provider: &SP, logger: &L ) -> Result::EcdsaSigner>, ChannelError> where L::Target: Logger @@ -8798,7 +8798,7 @@ where // if it has not received tx_signatures for that funding transaction AND // if next_commitment_number is zero: // MUST retransmit its commitment_signed for that funding transaction. - let commitment_signed = self.context.get_initial_commitment_signed(&self.funding, logger) + let commitment_signed = self.context.get_initial_commitment_signed_v2(&self.funding, logger) // TODO(splicing): Support async signing .ok_or_else(|| { let message = "Failed to get signatures for new commitment_signed".to_owned(); From 63604cb08be7d475cc93216931fada6f22249e44 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 4 Aug 2025 18:16:24 -0500 Subject: [PATCH 4/4] Remove some #[rustfmt::skip] from channel.rs --- lightning/src/ln/channel.rs | 57 +++++++++++++++++++++++++------------ 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 263a98cf617..944ed8f52e0 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -5578,33 +5578,49 @@ where } /// Asserts that the commitment tx numbers have not advanced from their initial number. - #[rustfmt::skip] - fn assert_no_commitment_advancement(&self, holder_commitment_transaction_number: u64, msg_name: &str) { - if self.commitment_secrets.get_min_seen_secret() != (1 << 48) || - self.cur_counterparty_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER || - holder_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER { - debug_assert!(false, "Should not have advanced channel commitment tx numbers prior to {}", - msg_name); + fn assert_no_commitment_advancement( + &self, holder_commitment_transaction_number: u64, msg_name: &str, + ) { + if self.commitment_secrets.get_min_seen_secret() != (1 << 48) + || self.cur_counterparty_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER + || holder_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER + { + debug_assert!( + false, + "Should not have advanced channel commitment tx numbers prior to {}", + msg_name + ); } } - #[rustfmt::skip] fn get_initial_counterparty_commitment_signature( - &self, funding: &FundingScope, logger: &L + &self, funding: &FundingScope, logger: &L, ) -> Option where SP::Target: SignerProvider, - L::Target: Logger + L::Target: Logger, { - let commitment_data = self.build_commitment_transaction(funding, + let commitment_data = self.build_commitment_transaction( + funding, self.cur_counterparty_commitment_transaction_number, - &self.counterparty_cur_commitment_point.unwrap(), false, false, logger); + &self.counterparty_cur_commitment_point.unwrap(), + false, + false, + logger, + ); let counterparty_initial_commitment_tx = commitment_data.tx; match self.holder_signer { // TODO (taproot|arik): move match into calling method for Taproot ChannelSignerType::Ecdsa(ref ecdsa) => { let channel_parameters = &funding.channel_transaction_parameters; - ecdsa.sign_counterparty_commitment(channel_parameters, &counterparty_initial_commitment_tx, Vec::new(), Vec::new(), &self.secp_ctx) + ecdsa + .sign_counterparty_commitment( + channel_parameters, + &counterparty_initial_commitment_tx, + Vec::new(), + Vec::new(), + &self.secp_ctx, + ) .map(|(signature, _)| signature) .ok() }, @@ -5614,19 +5630,24 @@ where } } - #[rustfmt::skip] fn get_initial_commitment_signed_v2( - &mut self, funding: &FundingScope, logger: &L + &mut self, funding: &FundingScope, logger: &L, ) -> Option where SP::Target: SignerProvider, - L::Target: Logger + L::Target: Logger, { - assert!(matches!(self.channel_state, ChannelState::FundingNegotiated(flags) if flags.is_interactive_signing())); + assert!( + matches!(self.channel_state, ChannelState::FundingNegotiated(flags) if flags.is_interactive_signing()) + ); let signature = self.get_initial_counterparty_commitment_signature(funding, logger); if let Some(signature) = signature { - log_info!(logger, "Generated commitment_signed for peer for channel {}", &self.channel_id()); + log_info!( + logger, + "Generated commitment_signed for peer for channel {}", + &self.channel_id() + ); Some(msgs::CommitmentSigned { channel_id: self.channel_id, htlc_signatures: vec![],