Split out remaining uses of `is_pre_funded_state` and rename it
What changed, and why it matters
This commit is a code-cleanup refactor in a Bitcoin Lightning Network library. It splits one overloaded channel-state check into three more precise checks so that saving channels to disk, deciding when to broadcast force-close transactions, and emitting 'channel is opening' events no longer rely on a single ambiguous flag. The change appears aimed at preventing subtle bugs in dual-funded channel opening, but the commit itself does not claim to fix a known security vulnerability and no exploit is described.
Treat as a maintainability and robustness improvement rather than an urgent security patch. Reviewers should verify that `can_resume_on_restart` correctly handles batch-open channels (where some sub-channels must not be persisted independently) and that `is_funding_broadcastable` does not delay `ChannelPending` or force-close in ways that violate protocol expectations. Consider adding/adjusting tests for dual-funded open persistence and event timing. No immediate user action is required unless this commit is part of a larger disclosed security fix.
Security signals we found
State-machine predicate split reduces conflation of persistence, event emission, and force-close broadcast decisions
Dual-funded/interactive signing channels now persisted earlier than before (once signature exchange begins)
Force-close broadcast and ChannelPending event now gated by `is_funding_broadcastable`, which waits until our tx signatures are ready
No explicit security bug, CVE, or exploit described in commit message or diff
Change is defensive/refactoring; correctness depends on whether new predicates cover all edge cases
Evidence from the diff
The patch renames ChannelState::is_pre_funded_state to can_resume_on_reconnect and introduces two new predicates: Channel::can_resume_on_restart (used to decide persistence to disk) and Channel::is_funding_broadcastable (used for ChannelPending event emission and force-close broadcast decisions). Previously these three concerns were all gated by is_funding_broadcast, which conflated ‘funding signatures exchanged’ with ‘funding actually broadcast’. The new logic allows dual-funded channels to be persisted once funding signature exchange begins, while delaying ChannelReady/force-close until after the funding transaction could actually be broadcast. This reduces state-machine ambiguity around batch opens and interactive (dual-funded) opens.
Changed components
lightning/src/ln/channel.rslightning/src/ln/channelmanager.rsChannel state machine (NegotiatingFunding, FundingNegotiated, AwaitingChannelReady)Dual-funded/interactive channel openingChannel persistence logicChannelPending event emissionForce-close transaction broadcast logicInspect captured patch +30 / −13
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index da4175d..db8a36b 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -806,11 +806,11 @@ impl ChannelState {
}
}
- fn is_pre_funded_state(&self) -> bool {
+ fn can_resume_on_reconnect(&self) -> bool {
match self {
- ChannelState::NegotiatingFunding(_) => true,
- ChannelState::FundingNegotiated(flags) => !flags.is_interactive_signing(),
- _ => false,
+ ChannelState::NegotiatingFunding(_) => false,
+ ChannelState::FundingNegotiated(flags) => flags.is_interactive_signing(),
+ _ => true,
}
}
@@ -4001,7 +4001,7 @@ where
// Checks whether we should emit a `ChannelPending` event.
pub(crate) fn should_emit_channel_pending_event(&mut self) -> bool {
- self.is_funding_broadcast() && !self.channel_pending_event_emitted
+ self.is_funding_broadcastable() && !self.channel_pending_event_emitted
}
// Returns whether we already emitted a `ChannelPending` event.
@@ -4082,11 +4082,28 @@ where
self.is_manual_broadcast = true;
}
+ /// Returns true if this channel can be resume after a restart, implying its past the initial
+ /// funding negotiation stages (and any assocated batch channels are similarly past initial
+ /// funding negotiation).
+ ///
+ /// This is equivalent to saying the channel can be persisted to disk.
+ pub fn can_resume_on_restart(&self) -> bool {
+ self.channel_state.can_resume_on_reconnect()
+ && match self.channel_state {
+ ChannelState::AwaitingChannelReady(flags) => !flags.is_waiting_for_batch(),
+ _ => true,
+ }
+ }
+
/// Returns true if funding_signed was sent/received and the
/// funding transaction has been broadcast if necessary.
- pub fn is_funding_broadcast(&self) -> bool {
- !self.channel_state.is_pre_funded_state()
- && !matches!(self.channel_state, ChannelState::AwaitingChannelReady(flags) if flags.is_set(AwaitingChannelReadyFlags::WAITING_FOR_BATCH))
+ fn is_funding_broadcastable(&self) -> bool {
+ match self.channel_state {
+ ChannelState::NegotiatingFunding(_) => false,
+ ChannelState::FundingNegotiated(flags) => !flags.is_our_tx_signatures_ready(),
+ ChannelState::AwaitingChannelReady(flags) => !flags.is_waiting_for_batch(),
+ _ => true,
+ }
}
#[rustfmt::skip]
@@ -5432,7 +5449,7 @@ where
// be delayed in being processed! See the docs for `ChannelManagerReadArgs` for more.
assert!(!matches!(self.channel_state, ChannelState::ShutdownComplete));
- let broadcast = self.is_funding_broadcast();
+ let broadcast = self.is_funding_broadcastable();
// We go ahead and "free" any holding cell HTLCs or HTLCs we haven't yet committed to and
// return them to fail the payment.
@@ -8306,12 +8323,12 @@ where
#[rustfmt::skip]
fn remove_uncommitted_htlcs_and_mark_paused<L: Deref>(&mut self, logger: &L) -> Result<(), ()> where L::Target: Logger {
assert!(!matches!(self.context.channel_state, ChannelState::ShutdownComplete));
- if self.context.channel_state.is_pre_funded_state() {
+ if !self.context.channel_state.can_resume_on_reconnect() {
return Err(())
}
// We only clear `peer_disconnected` if we were able to reestablish the channel. We always
- // reset our awaiting response in case we failed reestablishment and are disconnecting.
+ // reset our awaiting response in case we failed reestablishment and are disconnecting.
self.context.sent_message_awaiting_response = None;
if self.context.channel_state.is_peer_disconnected() {
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index c953e39..bd09b82 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -15242,7 +15242,7 @@ where
number_of_funded_channels += peer_state.channel_by_id
.values()
.filter_map(Channel::as_funded)
- .filter(|chan| chan.context.is_funding_broadcast())
+ .filter(|chan| chan.context.can_resume_on_restart())
.count();
}
@@ -15254,7 +15254,7 @@ where
for channel in peer_state.channel_by_id
.values()
.filter_map(Channel::as_funded)
- .filter(|channel| channel.context.is_funding_broadcast())
+ .filter(|channel| channel.context.can_resume_on_restart())
{
channel.write(writer)?;
}
Why this scored 30/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.