Dont use `is_pre_funded_state` to short-circuit `shutdown` handling
What the developer wrote
Authored by Matt Corallo
Dont use `is_pre_funded_state` to short-circuit `shutdown` handling
`Channel::is_pre_funded_state` is used to mean several different
things. In this case, its used to skip all the `shutdown` logic as
the funding transaction can't possibly have been broadcasted so
there's really no ned to try to sign a transaction spending it.
Here, we really want to capture any channel in `NegotiatingFunding`
or any V1 channel in `FundingNegotiated` or, finally, any V2
channel in `FundingNegotiated` where we haven't yet sent our
signatures (which is not captured in `is_pre_funded_state`).
Instead of a new helper, we just check the states directly in
`shutdown` handling.
What changed, and why it matters
This change fixes a logic bug in how the Lightning node decides whether to accept a 'shutdown' message from a peer. Previously, the code used a broad helper called 'is_pre_funded_state' to skip shutdown handling, but that helper did not correctly cover newer V2-style channels that have negotiated funding but not yet sent the node's own transaction signatures. In those cases, a peer could send shutdown and the node might try to run full shutdown logic (including signing a spending transaction) even though the funding transaction was not actually broadcastable yet. The patch narrows the check to the exact channel states where shutdown should be short-circuited.
Treat as a bug-fix patch that may have mild security implications. Reviewers should verify that the new not_broadcasted predicate exactly matches the protocol states where no funding transaction can be on chain, and that no other shutdown invariants (e.g., HTLC handling, signature generation) are reachable in the previously-missed V2 case. No immediate emergency response is indicated, but users running V2 channels should update.
Security signals we found
State-machine bypass in shutdown handling
Incorrect short-circuit predicate for pre-funding channels
Potential signing or error-handling mismatch in V2 channel establishment
No explicit security or CVE language in commit
Evidence from the diff
In lightning/src/ln/channel.rs, the shutdown handler no longer relies on Channel::is_pre_funded_state. Instead it sets a not_broadcasted flag only for ChannelState::NegotiatingFunding, or ChannelState::FundingNegotiated where is_our_tx_signatures_ready() is false. This captures V1 channels in FundingNegotiated and V2 channels in FundingNegotiated that have not yet sent tx_signatures. The previous helper apparently conflated several meanings and missed the V2 not-yet-signed case, potentially allowing shutdown handling to proceed when no funding tx could exist on chain.
Changed components
lightning/src/ln/channel.rsChannel shutdown message handlingV2 channel establishment (splicing / dual-funded channel logic)ChannelState::FundingNegotiated state machineInspect captured patch +11 / −4
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 559a254..701e269 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -9304,13 +9304,20 @@ where
"Peer sent shutdown when we needed a channel_reestablish".to_owned(),
));
}
- if self.context.channel_state.is_pre_funded_state() {
+ let mut not_broadcasted =
+ matches!(self.context.channel_state, ChannelState::NegotiatingFunding(_));
+ if let ChannelState::FundingNegotiated(flags) = &self.context.channel_state {
+ if !flags.is_our_tx_signatures_ready() {
+ // If we're a V1 channel or we haven't yet sent our `tx_signatures`, the funding tx
+ // couldn't be broadcasted yet, so just short-circuit the shutdown logic.
+ not_broadcasted = true;
+ }
+ }
+ if not_broadcasted {
// Spec says we should fail the connection, not the channel, but that's nonsense, there
// are plenty of reasons you may want to fail a channel pre-funding, and spec says you
// can do that via error message without getting a connection fail anyway...
- return Err(ChannelError::close(
- "Peer sent shutdown pre-funding generation".to_owned(),
- ));
+ return Err(ChannelError::close("Shutdown before funding was broadcasted".to_owned()));
}
for htlc in self.context.pending_inbound_htlcs.iter() {
if let InboundHTLCState::RemoteAnnounced(_) = htlc.state {
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.