Remove unnecessary next_funding check
What the developer wrote
Authored by Jeffrey Czyz
Remove unnecessary next_funding check
During channel_reestablish handling, next_local_commitment_number can
never be zero. Remove the unnecessary check conditioning on next_funding
not being set. Additionally, remove the spec requirement in the comment
that follows since it does not exist.
What changed, and why it matters
This commit tightens a validation check during Lightning channel reconnection. It removes a special case that allowed a peer to send a zero commitment counter without also providing next_funding data. The change makes the code reject that zero-counter case outright, which is described as safe because the counter should never be zero at this stage. The commit also deletes an incorrect comment that cited a non-existent specification requirement.
Treat as a minor hardening patch. Reviewers should confirm that next_local_commitment_number can indeed never be zero at this point in all channel states (including V1 and V2 channels, and any pending funding flows), and that removing the next_funding condition does not change behavior for valid peers. No urgent action is indicated absent additional context.
Security signals we found
Validation logic tightened: zero next_local_commitment_number now always rejected during channel_reestablish
Comment citing a non-existent spec requirement removed
No CVE, advisory, or researcher attribution present in commit or supplied references
Change is small and framed by the author as cleanup/hardening, not a security fix
Evidence from the diff
In channel.rs, the channel_reestablish handler’s validation of msg.next_local_commitment_number and msg.next_remote_commitment_number is refactored. Previously, next_local_commitment_number == 0 was only an error if next_funding was None. Now, next_local_commitment_number == 0 is always an error. The removed comment incorrectly attributed behavior to the V2 channel establishment spec; the commit notes that requirement does not exist. The error message remains the same. This is a hardening change rather than a clear vulnerability fix, and the commit message frames it as removing an unnecessary check.
Changed components
lightning/src/ln/channel.rschannel_reestablish message handlingInspect captured patch +4 / −5
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 3015bd4..08054a8 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -9120,11 +9120,10 @@ where
return Err(ChannelError::close("Peer sent a loose channel_reestablish not after reconnect".to_owned()));
}
- if msg.next_local_commitment_number >= INITIAL_COMMITMENT_NUMBER || msg.next_remote_commitment_number >= INITIAL_COMMITMENT_NUMBER ||
- (msg.next_local_commitment_number == 0 && msg.next_funding.is_none()) {
- // Note: This also covers the following case in the V2 channel establishment specification:
- // if `next_funding` is not set, and `next_commitment_number` is zero:
- // MUST immediately fail the channel and broadcast any relevant latest commitment transaction.
+ if msg.next_local_commitment_number == 0
+ || msg.next_local_commitment_number >= INITIAL_COMMITMENT_NUMBER
+ || msg.next_remote_commitment_number >= INITIAL_COMMITMENT_NUMBER
+ {
return Err(ChannelError::close("Peer sent an invalid channel_reestablish to force close in a non-standard way".to_owned()));
}
Why this scored 26/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.