fuzz: track chanmon claims on PaymentClaimed events

[joostjager]

Only record receiver claims in the fuzz payment tracker after the node emits PaymentClaimed. This keeps the tracker aligned with durable claim completion instead of a claim_funds call that a restart may roll back.

[ldk-reviews-bot]

👋 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.

[ldk-claude-review-bot]

I've thoroughly reviewed the full diff and the surrounding context. The change is small and correct:

1. Moving claimed_payment_hashes.insert(payment_hash) from the claim_payment method (at claim_funds call time) to a new mark_claimed method called from the PaymentClaimed event handler aligns the tracker with durable state.
2. The mark_claimed method is idempotent (HashSet insert).
3. The rollback logic at line 2953 (claimed_payment_hashes.remove) still correctly handles the case where a sender node is reloaded and its sent payments are rolled back.
4. The receiver-side reload case is now actually handled more correctly — if a receiver node is reloaded before PaymentClaimed fires, the hash was never added to claimed_payment_hashes, so there's nothing stale to clean up.

No issues found.

[ldk-reviews-bot]

🔔 1st Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

[ldk-reviews-bot]

🔔 2nd Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

[TheBlueMatt]

I mean yes, I believe this should be required, but I'm more interested in why we didn't run into failures without this? Any chance we could debug that and figure out what coverage we're missing?

[ldk-reviews-bot]

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

[joostjager]

I mean yes, I believe this should be required, but I'm more interested in why we didn't run into failures without this? Any chance we could debug that and figure out what coverage we're missing?

I did try that: #4631 (comment)

It seems the reload replay prevented failures from happening.

[TheBlueMatt]

I tried to repro that failure, but for the cases I tested, the HTLC is then emitted as PaymentClaimable again after reload, the harness claims it again, and makes the tracking check out?

Ah, so we're kinda indirectly checking the "once its claimable, its guaranteed to remain claimable unless you wait so long the HTLC expires" property - that does kinda seem like a useful thing to continue testing and not entirely sure its worth making payment claiming "async" by queuing them and processing them separately to allow this to be reachable.

[joostjager]

Ah, so we're kinda indirectly checking the "once its claimable, its guaranteed to remain claimable unless you wait so long the HTLC expires" property - that does kinda seem like a useful thing to continue testing and not entirely sure its worth making payment claiming "async" by queuing them and processing them separately to allow this to be reachable.

I think the fuzzer should stay close to the public contract, and calling claim_funds itself does not guarantee that the claim has become durable across reload. I'd leave the PR as is.

[TheBlueMatt]

Right but the other public contract is that if you see an Event::PaymentClaimable it will remain claimable until you claim, fail, or the HTLC times out (per the timeout in the event). We're somewhat accidentally testing that public API contract currently, and that might well be more important to test, no?

[joostjager]

Hm yes, good point. For the force close coverage I can then indeed use the event-given deadline for the assertion.

Can close this PR then.

[joostjager]

For the force-close fuzzer addition, I included claim_deadline in PaymentClaimable handling. The fuzzer can mine blocks before draining events, so a claimable payment may become stale before the harness calls claim_funds; in that case we skip the claim and let LDK’s timeout/failback events drive accounting instead. This keeps the harness within the claim_funds API contract and avoids special-casing PaymentClaimBuffer.