Address ChannelState inconsistency throughout splicing
What the developer wrote
Authored by Wilmer Paulino
Address ChannelState inconsistency throughout splicing
Once a channel open has become locked (i.e., we've entered
`ChannelState::ChannelReady`), the channel is intended to remain within
this state for the rest of its lifetime until shutdown. Previously, we
had assumed a channel being spliced would go through the `ChannelState`
lifecycle again starting from `NegotiatingFunding` but skipping
`AwaitingChannelReady`. This inconsistency departs from what we strive
to achieve with `ChannelState` and also makes the state of a channel
harder to reason about.
This commit ensures a channel undergoing a splice remains in
`ChannelReady`, clearing the quiescent flag once the negotiation is
complete. Dual funding is unaffected by this change as the channel is
being opened and we want to maintain the same `ChannelState` lifecycle.
What changed, and why it matters
This commit fixes a state-machine inconsistency in how the Lightning node tracks the lifecycle of a channel when it is being spliced. Previously, a splice incorrectly moved the channel back through the same states used when a channel is first being opened. The change keeps the channel in its normal operating state during a splice and only clears a temporary 'quiet' flag once negotiation is done. This makes the code easier to reason about and prevents potential bugs where the channel could be treated as not-yet-open when it is actually already active.
Review and merge as a correctness fix. Consider whether any downstream logic depends on a spliced channel briefly entering FundingNegotiated/AwaitingChannelReady, and add regression tests covering splice state transitions and quiescent flag clearing. No immediate emergency response is indicated, but the fix should be included in the next release.
Security signals we found
State machine lifecycle correction for splicing
Prevents ChannelReady channel from being re-entered into pre-ready states during splice
Clears quiescent flag only after splice negotiation completes
Adds debug_assertions to enforce expected state during interactive funding completion
Removes TODO comment indicating prior known splicing state transition issue
Evidence from the diff
The patch changes rust-lightning’s channel state handling so that splicing no longer transitions a ChannelReady channel back into FundingNegotiated/AwaitingChannelReady. Instead, on_tx_signatures_exchange now checks whether a pending_splice exists: if so, it stores the finalized funding transaction in the splice’s negotiated_candidates and clears the quiescent flag while leaving channel_state as ChannelReady; only for non-splice (initial dual-funding) flows does it set AwaitingChannelReady. The interactive funding completion path also moves the FundingNegotiated state assignment into the non-splice branch. Several assertions are relaxed or converted to debug_asserts to match the new splice behavior.
Changed components
lightning/src/ln/channel.rsChannel state machineSplicing flowInteractive transaction signing (TxSignatures handling)Dual-funding channel openingInspect captured patch +73 / −24
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 2aed908..de894de 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -718,9 +718,9 @@ enum ChannelState {
/// `AwaitingChannelReady`. Note that this is nonsense for an inbound channel as we immediately generate
/// `funding_signed` upon receipt of `funding_created`, so simply skip this state.
///
- /// For inbound and outbound interactively funded channels (dual-funding/splicing), this flag indicates
- /// that interactive transaction construction has been completed and we are now interactively signing
- /// the funding/splice transaction.
+ /// For inbound and outbound interactively funded channels (dual-funding), this state indicates
+ /// that interactive transaction construction has been completed and we are now interactively
+ /// signing the initial funding transaction.
FundingNegotiated(FundingNegotiatedFlags),
/// We've received/sent `funding_created` and `funding_signed` and are thus now waiting on the
/// funding transaction to confirm.
@@ -1932,6 +1932,14 @@ where
let logger = WithChannelContext::from(logger, self.context(), None);
match &mut self.phase {
ChannelPhase::UnfundedV2(chan) => {
+ debug_assert_eq!(
+ chan.context.channel_state,
+ ChannelState::NegotiatingFunding(
+ NegotiatingFundingFlags::OUR_INIT_SENT
+ | NegotiatingFundingFlags::THEIR_INIT_SENT
+ ),
+ );
+
let signing_session = chan
.interactive_tx_constructor
.take()
@@ -6116,7 +6124,6 @@ where
funding
.channel_transaction_parameters.funding_outpoint = Some(outpoint);
self.interactive_tx_signing_session = Some(signing_session);
- self.channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new());
if is_splice {
debug_assert_eq!(
@@ -6127,6 +6134,7 @@ where
return Err(AbortReason::InternalError("Splicing not yet supported"));
} else {
self.assert_no_commitment_advancement(holder_commitment_transaction_number, "initial commitment_signed");
+ self.channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new());
}
let commitment_signed = self.get_initial_commitment_signed_v2(&funding, logger);
@@ -6211,9 +6219,7 @@ where
SP::Target: SignerProvider,
L::Target: Logger,
{
- assert!(
- matches!(self.channel_state, ChannelState::FundingNegotiated(_) if self.interactive_tx_signing_session.is_some())
- );
+ debug_assert!(self.interactive_tx_signing_session.is_some());
let signature = self.get_initial_counterparty_commitment_signature(funding, logger);
if let Some(signature) = signature {
@@ -8573,11 +8579,43 @@ where
}
}
+ fn on_tx_signatures_exchange(&mut self, funding_tx: Transaction) {
+ debug_assert!(!self.context.channel_state.is_monitor_update_in_progress());
+ debug_assert!(!self.context.channel_state.is_awaiting_remote_revoke());
+
+ if let Some(pending_splice) = self.pending_splice.as_mut() {
+ if let Some(FundingNegotiation::AwaitingSignatures { mut funding }) =
+ pending_splice.funding_negotiation.take()
+ {
+ funding.funding_transaction = Some(funding_tx);
+ pending_splice.negotiated_candidates.push(funding);
+ } else {
+ debug_assert!(false);
+ }
+ self.context.channel_state.clear_quiescent();
+ } else {
+ self.funding.funding_transaction = Some(funding_tx);
+ self.context.channel_state =
+ ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
+ }
+ }
+
pub fn funding_transaction_signed(
&mut self, funding_txid_signed: Txid, witnesses: Vec<Witness>,
) -> Result<(Option<msgs::TxSignatures>, Option<Transaction>), APIError> {
let signing_session =
if let Some(signing_session) = self.context.interactive_tx_signing_session.as_mut() {
+ if let Some(pending_splice) = self.pending_splice.as_ref() {
+ debug_assert!(pending_splice
+ .funding_negotiation
+ .as_ref()
+ .map(|funding_negotiation| matches!(
+ funding_negotiation,
+ FundingNegotiation::AwaitingSignatures { .. }
+ ))
+ .unwrap_or(false));
+ }
+
signing_session
} else {
let err =
@@ -8620,24 +8658,40 @@ where
.provide_holder_witnesses(tx_signatures, &self.context.secp_ctx)
.map_err(|err| APIError::APIMisuseError { err })?;
- if funding_tx_opt.is_some() {
- self.funding.funding_transaction = funding_tx_opt.clone();
- self.context.channel_state =
- ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
+ if let Some(funding_tx) = funding_tx_opt.clone() {
+ debug_assert!(tx_signatures_opt.is_some());
+ self.on_tx_signatures_exchange(funding_tx);
}
Ok((tx_signatures_opt, funding_tx_opt))
}
- #[rustfmt::skip]
- pub fn tx_signatures(&mut self, msg: &msgs::TxSignatures) -> Result<(Option<msgs::TxSignatures>, Option<Transaction>), ChannelError> {
- let signing_session = if let Some(signing_session) = self.context.interactive_tx_signing_session.as_mut() {
+ pub fn tx_signatures(
+ &mut self, msg: &msgs::TxSignatures,
+ ) -> Result<(Option<msgs::TxSignatures>, Option<Transaction>), ChannelError> {
+ let signing_session = if let Some(signing_session) =
+ self.context.interactive_tx_signing_session.as_mut()
+ {
if signing_session.has_received_tx_signatures() {
return Err(ChannelError::Ignore("Ignoring duplicate tx_signatures".to_owned()));
}
if !signing_session.has_received_commitment_signed() {
- return Err(ChannelError::close("Received tx_signatures before initial commitment_signed".to_owned()));
+ return Err(ChannelError::close(
+ "Received tx_signatures before initial commitment_signed".to_owned(),
+ ));
+ }
+
+ if let Some(pending_splice) = self.pending_splice.as_ref() {
+ debug_assert!(pending_splice
+ .funding_negotiation
+ .as_ref()
+ .map(|funding_negotiation| matches!(
+ funding_negotiation,
+ FundingNegotiation::AwaitingSignatures { .. }
+ ))
+ .unwrap_or(false));
}
+
signing_session
} else {
return Err(ChannelError::Ignore("Ignoring unexpected tx_signatures".to_owned()));
@@ -8657,16 +8711,11 @@ where
}
}
- let (holder_tx_signatures_opt, funding_tx_opt) = signing_session.received_tx_signatures(msg)
- .map_err(|msg| ChannelError::Warn(msg))?;
+ let (holder_tx_signatures_opt, funding_tx_opt) =
+ signing_session.received_tx_signatures(msg).map_err(|msg| ChannelError::Warn(msg))?;
- if funding_tx_opt.is_some() {
- // TODO(splicing): Transition back to `ChannelReady` and not `AwaitingChannelReady`
- // We will also need to use the pending `FundingScope` in the splicing case.
- //
- // We have a finalized funding transaction, so we can set the funding transaction.
- self.funding.funding_transaction = funding_tx_opt.clone();
- self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
+ if let Some(funding_tx) = funding_tx_opt.clone() {
+ self.on_tx_signatures_exchange(funding_tx);
}
Ok((holder_tx_signatures_opt, funding_tx_opt))
Why this scored 42/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.