Use HolderCommitmentPoint::current_transaction_number
What the developer wrote
Authored by Jeffrey Czyz
Use HolderCommitmentPoint::current_transaction_number
What changed, and why it matters
This tiny code change fixes how rust-lightning calculates which local commitment transaction number to report during Lightning channel re-establishment. The old code used an upcoming ('next') transaction number and subtracted an extra 1, which could produce an off-by-one value. If that value is wrong, a peer could be told we are on a different commitment number than we actually are, potentially causing a force-close or state mismatch during channel recovery. The fix uses the current transaction number directly.
Review the surrounding channel_reestablish logic and add regression tests covering commitment number reporting after holder commitment point transitions. Consider whether any reachable path could exploit the off-by-one to force an incorrect close or state update.
Security signals we found
off-by-one in commitment transaction number
channel_reestablish message handling
potential state desynchronization
Lightning protocol state machine correctness
Evidence from the diff
In channel.rs, the channel_reestablish handling computed ‘our_commitment_transaction’ as INITIAL_COMMITMENT_NUMBER - holder_commitment_point.next_transaction_number() - 1. The patch changes it to INITIAL_COMMITMENT_NUMBER - holder_commitment_point.current_transaction_number(). This removes the extra -1 and aligns the reported next_local_commitment_number with the holder’s actual current commitment point. An incorrect commitment number in channel_reestablish can lead to desynchronization, unnecessary force-closes, or worse if combined with other state bugs.
Changed components
lightning/src/ln/channel.rschannel_reestablish processingHolderCommitmentPointInspect captured patch +1 / −1
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index a5789da..f77c73f 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -9129,7 +9129,7 @@ where
return Err(ChannelError::close("Peer sent an invalid channel_reestablish to force close in a non-standard way".to_owned()));
}
- let our_commitment_transaction = INITIAL_COMMITMENT_NUMBER - self.holder_commitment_point.next_transaction_number() - 1;
+ let our_commitment_transaction = INITIAL_COMMITMENT_NUMBER - self.holder_commitment_point.current_transaction_number();
if msg.next_remote_commitment_number > 0 {
let expected_point = self.context.holder_signer.as_ref()
.get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - msg.next_remote_commitment_number + 1, &self.context.secp_ctx)
Why this scored 54/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.