Skip to content

Commit 0cbeb51

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 5efe13b commit 0cbeb51

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()
@@ -6071,9 +6083,6 @@ where
60716083
funding
60726084
.channel_transaction_parameters.funding_outpoint = Some(outpoint);
60736085

6074-
self.channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new());
6075-
self.channel_state.set_interactive_signing();
6076-
60776086
if is_splice {
60786087
debug_assert_eq!(
60796088
holder_commitment_transaction_number,
@@ -6121,6 +6130,12 @@ where
61216130
SP::Target: SignerProvider,
61226131
L::Target: Logger,
61236132
{
6133+
let is_quiescent = matches!(
6134+
self.channel_state,
6135+
ChannelState::ChannelReady(f) if f.is_set(ChannelReadyFlags::QUIESCENT)
6136+
);
6137+
debug_assert!(self.channel_state.is_interactive_signing() || is_quiescent);
6138+
61246139
let mut commitment_number = self.counterparty_next_commitment_transaction_number;
61256140
let mut commitment_point = self.counterparty_next_commitment_point.unwrap();
61266141

@@ -6167,10 +6182,6 @@ where
61676182
SP::Target: SignerProvider,
61686183
L::Target: Logger,
61696184
{
6170-
assert!(
6171-
matches!(self.channel_state, ChannelState::FundingNegotiated(flags) if flags.is_interactive_signing())
6172-
);
6173-
61746185
let signature = self.get_initial_counterparty_commitment_signature(funding, logger);
61756186
if let Some(signature) = signature {
61766187
log_info!(
@@ -6201,6 +6212,8 @@ where
62016212
SP::Target: SignerProvider,
62026213
L::Target: Logger,
62036214
{
6215+
self.channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new());
6216+
self.channel_state.set_interactive_signing();
62046217
self.counterparty_next_commitment_point = Some(counterparty_next_commitment_point_override);
62056218
self.get_initial_counterparty_commitment_signature(funding, logger)
62066219
}
@@ -8581,6 +8594,10 @@ where
85818594
format!("Channel {} not expecting funding signatures", self.context.channel_id);
85828595
return Err(APIError::APIMisuseError { err });
85838596
}
8597+
debug_assert_eq!(
8598+
self.has_pending_splice_awaiting_signatures(),
8599+
matches!(self.context.channel_state, ChannelState::ChannelReady(f) if f.is_set(ChannelReadyFlags::QUIESCENT))
8600+
);
85848601

85858602
let signing_session =
85868603
if let Some(signing_session) = self.interactive_tx_signing_session.as_mut() {
@@ -8631,9 +8648,25 @@ where
86318648
.map_err(|err| APIError::APIMisuseError { err })?;
86328649

86338650
if funding_tx_opt.is_some() {
8634-
self.funding.funding_transaction = funding_tx_opt.clone();
8635-
self.context.channel_state =
8636-
ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
8651+
debug_assert!(tx_signatures_opt.is_some());
8652+
debug_assert!(!self.context.channel_state.is_monitor_update_in_progress());
8653+
debug_assert!(!self.context.channel_state.is_awaiting_remote_revoke());
8654+
8655+
if let Some(pending_splice) = self.pending_splice.as_mut() {
8656+
if let Some(FundingNegotiation::AwaitingSignatures(mut funding)) =
8657+
pending_splice.funding_negotiation.take()
8658+
{
8659+
funding.funding_transaction = funding_tx_opt.clone();
8660+
self.pending_funding.push(funding);
8661+
} else {
8662+
debug_assert!(false, "We checked we were in the right state above");
8663+
}
8664+
self.context.channel_state.clear_quiescent();
8665+
} else {
8666+
self.funding.funding_transaction = funding_tx_opt.clone();
8667+
self.context.channel_state =
8668+
ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
8669+
}
86378670
}
86388671

86398672
Ok((tx_signatures_opt, funding_tx_opt))
@@ -8646,6 +8679,10 @@ where
86468679
{
86478680
return Err(ChannelError::Ignore("Ignoring unexpected tx_signatures".to_owned()));
86488681
}
8682+
debug_assert_eq!(
8683+
self.has_pending_splice_awaiting_signatures(),
8684+
matches!(self.context.channel_state, ChannelState::ChannelReady(f) if f.is_set(ChannelReadyFlags::QUIESCENT))
8685+
);
86498686

86508687
let signing_session = if let Some(signing_session) = self.interactive_tx_signing_session.as_mut() {
86518688
if signing_session.has_received_tx_signatures() {
@@ -8678,12 +8715,24 @@ where
86788715
.map_err(|msg| ChannelError::Warn(msg))?;
86798716

86808717
if funding_tx_opt.is_some() {
8681-
// TODO(splicing): Transition back to `ChannelReady` and not `AwaitingChannelReady`
8682-
// We will also need to use the pending `FundingScope` in the splicing case.
8683-
//
8684-
// We have a finalized funding transaction, so we can set the funding transaction.
8685-
self.funding.funding_transaction = funding_tx_opt.clone();
8686-
self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
8718+
debug_assert!(!self.context.channel_state.is_monitor_update_in_progress());
8719+
debug_assert!(!self.context.channel_state.is_awaiting_remote_revoke());
8720+
8721+
if let Some(pending_splice) = self.pending_splice.as_mut() {
8722+
if let Some(FundingNegotiation::AwaitingSignatures(mut funding)) =
8723+
pending_splice.funding_negotiation.take()
8724+
{
8725+
funding.funding_transaction = funding_tx_opt.clone();
8726+
self.pending_funding.push(funding);
8727+
} else {
8728+
debug_assert!(false, "We checked we were in the right state above");
8729+
}
8730+
self.context.channel_state.clear_quiescent();
8731+
} else {
8732+
self.funding.funding_transaction = funding_tx_opt.clone();
8733+
self.context.channel_state =
8734+
ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
8735+
}
86878736
}
86888737

86898738
Ok((holder_tx_signatures_opt, funding_tx_opt))

0 commit comments

Comments
 (0)