Skip to content

Commit 23a1c29

Browse files
committed
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.
1 parent e692043 commit 23a1c29

File tree

1 file changed

+65
-16
lines changed

1 file changed

+65
-16
lines changed

lightning/src/ln/channel.rs

Lines changed: 65 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1935,6 +1935,18 @@ where
19351935
let logger = WithChannelContext::from(logger, self.context(), None);
19361936
match &mut self.phase {
19371937
ChannelPhase::UnfundedV2(chan) => {
1938+
debug_assert_eq!(
1939+
chan.context.channel_state,
1940+
ChannelState::NegotiatingFunding(
1941+
NegotiatingFundingFlags::OUR_INIT_SENT
1942+
| NegotiatingFundingFlags::THEIR_INIT_SENT
1943+
),
1944+
);
1945+
1946+
chan.context.channel_state =
1947+
ChannelState::FundingNegotiated(FundingNegotiatedFlags::new());
1948+
chan.context.channel_state.set_interactive_signing();
1949+
19381950
let mut signing_session = chan
19391951
.interactive_tx_constructor
19401952
.take()
@@ -5998,9 +6010,6 @@ where
59986010
funding
59996011
.channel_transaction_parameters.funding_outpoint = Some(outpoint);
60006012

6001-
self.channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new());
6002-
self.channel_state.set_interactive_signing();
6003-
60046013
if is_splice {
60056014
debug_assert_eq!(
60066015
holder_commitment_transaction_number,
@@ -6048,6 +6057,12 @@ where
60486057
SP::Target: SignerProvider,
60496058
L::Target: Logger,
60506059
{
6060+
let is_quiescent = matches!(
6061+
self.channel_state,
6062+
ChannelState::ChannelReady(f) if f.is_set(ChannelReadyFlags::QUIESCENT)
6063+
);
6064+
debug_assert!(self.channel_state.is_interactive_signing() || is_quiescent);
6065+
60516066
let mut commitment_number = self.counterparty_next_commitment_transaction_number;
60526067
let mut commitment_point = self.counterparty_next_commitment_point.unwrap();
60536068

@@ -6094,10 +6109,6 @@ where
60946109
SP::Target: SignerProvider,
60956110
L::Target: Logger,
60966111
{
6097-
assert!(
6098-
matches!(self.channel_state, ChannelState::FundingNegotiated(flags) if flags.is_interactive_signing())
6099-
);
6100-
61016112
let signature = self.get_initial_counterparty_commitment_signature(funding, logger);
61026113
if let Some(signature) = signature {
61036114
log_info!(
@@ -6128,6 +6139,8 @@ where
61286139
SP::Target: SignerProvider,
61296140
L::Target: Logger,
61306141
{
6142+
self.channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new());
6143+
self.channel_state.set_interactive_signing();
61316144
self.counterparty_next_commitment_point = Some(counterparty_next_commitment_point_override);
61326145
self.get_initial_counterparty_commitment_signature(funding, logger)
61336146
}
@@ -8508,6 +8521,10 @@ where
85088521
format!("Channel {} not expecting funding signatures", self.context.channel_id);
85098522
return Err(APIError::APIMisuseError { err });
85108523
}
8524+
debug_assert_eq!(
8525+
self.has_pending_splice_awaiting_signatures(),
8526+
matches!(self.context.channel_state, ChannelState::ChannelReady(f) if f.is_set(ChannelReadyFlags::QUIESCENT))
8527+
);
85118528

85128529
let signing_session =
85138530
if let Some(signing_session) = self.interactive_tx_signing_session.as_mut() {
@@ -8558,9 +8575,25 @@ where
85588575
.map_err(|err| APIError::APIMisuseError { err })?;
85598576

85608577
if funding_tx_opt.is_some() {
8561-
self.funding.funding_transaction = funding_tx_opt.clone();
8562-
self.context.channel_state =
8563-
ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
8578+
debug_assert!(tx_signatures_opt.is_some());
8579+
debug_assert!(!self.context.channel_state.is_monitor_update_in_progress());
8580+
debug_assert!(!self.context.channel_state.is_awaiting_remote_revoke());
8581+
8582+
if let Some(pending_splice) = self.pending_splice.as_mut() {
8583+
if let Some(FundingNegotiation::AwaitingSignatures(mut funding)) =
8584+
pending_splice.funding_negotiation.take()
8585+
{
8586+
funding.funding_transaction = funding_tx_opt.clone();
8587+
self.pending_funding.push(funding);
8588+
} else {
8589+
debug_assert!(false, "We checked we were in the right state above");
8590+
}
8591+
self.context.channel_state.clear_quiescent();
8592+
} else {
8593+
self.funding.funding_transaction = funding_tx_opt.clone();
8594+
self.context.channel_state =
8595+
ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
8596+
}
85648597
}
85658598

85668599
Ok((tx_signatures_opt, funding_tx_opt))
@@ -8573,6 +8606,10 @@ where
85738606
{
85748607
return Err(ChannelError::Ignore("Ignoring unexpected tx_signatures".to_owned()));
85758608
}
8609+
debug_assert_eq!(
8610+
self.has_pending_splice_awaiting_signatures(),
8611+
matches!(self.context.channel_state, ChannelState::ChannelReady(f) if f.is_set(ChannelReadyFlags::QUIESCENT))
8612+
);
85768613

85778614
let signing_session = if let Some(signing_session) = self.interactive_tx_signing_session.as_mut() {
85788615
if signing_session.has_received_tx_signatures() {
@@ -8605,12 +8642,24 @@ where
86058642
.map_err(|msg| ChannelError::Warn(msg))?;
86068643

86078644
if funding_tx_opt.is_some() {
8608-
// TODO(splicing): Transition back to `ChannelReady` and not `AwaitingChannelReady`
8609-
// We will also need to use the pending `FundingScope` in the splicing case.
8610-
//
8611-
// We have a finalized funding transaction, so we can set the funding transaction.
8612-
self.funding.funding_transaction = funding_tx_opt.clone();
8613-
self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
8645+
debug_assert!(!self.context.channel_state.is_monitor_update_in_progress());
8646+
debug_assert!(!self.context.channel_state.is_awaiting_remote_revoke());
8647+
8648+
if let Some(pending_splice) = self.pending_splice.as_mut() {
8649+
if let Some(FundingNegotiation::AwaitingSignatures(mut funding)) =
8650+
pending_splice.funding_negotiation.take()
8651+
{
8652+
funding.funding_transaction = funding_tx_opt.clone();
8653+
self.pending_funding.push(funding);
8654+
} else {
8655+
debug_assert!(false, "We checked we were in the right state above");
8656+
}
8657+
self.context.channel_state.clear_quiescent();
8658+
} else {
8659+
self.funding.funding_transaction = funding_tx_opt.clone();
8660+
self.context.channel_state =
8661+
ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
8662+
}
86148663
}
86158664

86168665
Ok((holder_tx_signatures_opt, funding_tx_opt))

0 commit comments

Comments
 (0)