Skip to content

Fix cltv_expiry_delta comment#4541

Open
elnosh wants to merge 1 commit intolightningdevkit:mainfrom
elnosh:fix-cltv-comment
Open

Fix cltv_expiry_delta comment#4541
elnosh wants to merge 1 commit intolightningdevkit:mainfrom
elnosh:fix-cltv-comment

Conversation

@elnosh
Copy link
Copy Markdown
Contributor

@elnosh elnosh commented Apr 3, 2026

Fix incorrect comment about cltv_expiry_delta.
The cltv_expiry on the outgoing HTLC must be less than the one in the incoming HTLC.

@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented Apr 3, 2026

I've assigned @wpaulino as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@ldk-claude-review-bot
Copy link
Copy Markdown
Collaborator

ldk-claude-review-bot commented Apr 3, 2026

The implementation confirms the PR's changes are correct. The check at channel.rs:11201 is:

htlc.cltv_expiry < outgoing_cltv_value + cltv_expiry_delta

This means outgoing_cltv_value <= incoming_cltv_expiry - cltv_expiry_delta, so the cltv_expiry_delta determines the maximum outgoing value (not minimum), and with incoming=100000, delta=10, outgoing must be at most 99990 (not "at least 100010").

Looking at the diff again: the PR already changes both "minimum" → "maximum" (line 1494) and "at least 100010" → "at most 99990" (line 1496). My prior review comment was a false positive — the PR already correctly fixes line 1494.

No new issues found. The comment corrections are accurate and consistent with the actual implementation.

Fix incorrect comment about cltv_expiry_delta.
The cltv_expiry on the outgoing HTLC must be less than the
one in the incoming HTLC.
@elnosh elnosh force-pushed the fix-cltv-comment branch from cdb7cce to dc06afa Compare April 3, 2026 17:18
@ldk-reviews-bot ldk-reviews-bot requested a review from wpaulino April 3, 2026 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants