Skip to content

Fix misleading comment on counterparty_commitment_txn_on_chain insert#4530

Open
joostjager wants to merge 1 commit intolightningdevkit:mainfrom
joostjager:fix-claim-info-comment
Open

Fix misleading comment on counterparty_commitment_txn_on_chain insert#4530
joostjager wants to merge 1 commit intolightningdevkit:mainfrom
joostjager:fix-claim-info-comment

Conversation

@joostjager
Copy link
Copy Markdown
Contributor

The comment claimed this insert "isn't useful yet" and was only a safety measure for a watchtower race. In practice it is also used critically by provide_payment_preimage to look up the commitment number when a preimage arrives after the counterparty commitment tx is confirmed.

The comment claimed this insert "isn't useful yet" and was only a
safety measure for a watchtower race. In practice it is also used by
provide_payment_preimage to look up the commitment number when a
preimage arrives after the counterparty commitment tx is confirmed.

AI tools were used in preparing this commit.
@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented Apr 1, 2026

👋 Thanks for assigning @TheBlueMatt 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.

@joostjager joostjager requested a review from TheBlueMatt April 1, 2026 07:56
@ldk-claude-review-bot
Copy link
Copy Markdown
Collaborator

I've now reviewed the full context. The PR is a comment-only change. Let me verify the new comment's claims are accurate:

  1. provide_payment_preimage uses counterparty_commitment_txn_on_chain: Confirmed at lines 3806 and 3817 — provide_payment_preimage calls self.counterparty_commitment_txn_on_chain.get(&txid) to look up the commitment number.

  2. The watchtower race scenario: This was already documented in the old comment and remains in the new one.

  3. "the above conditional": The new comment refers to counterparty_commitment_txn_on_chain entry not being generated by the above conditional — this refers to line 4850 where the insert happens inside the revoked commitment branch. This is accurate.

The new comment is factually correct and more informative than the old one. This is a clean, comment-only change with no code modifications.

No issues found.

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