Duplicate Delayed Holder HTLC Claim Replay After Force-Close

[joostjager]

This document was drafted from AI-assisted analysis and test exploration.

Summary

OnchainTxHandler::update_claims_view_from_requests can accept the same logical holder HTLC timeout claim twice after a force-close on an anchor channel.

The failing shape is:

1. A holder commitment confirms on-chain.
2. Two offered holder HTLC outputs are turned into two single-outpoint claim requests.
3. OnchainTxHandler merges those requests into one delayed package and parks it in locktimed_packages until the CLTV height.
4. Before the CLTV height is reached, a later replay through normal production code rebuilds the same two single-outpoint requests from the same confirmed holder commitment.
5. The current delayed-claim dedupe only checks for an exact outpoint-set match, so it does not treat those replayed single-outpoint requests as duplicates of the already-merged two-outpoint delayed package.
6. The replayed requests are merged again into a second identical delayed package.
7. At CLTV maturity, both delayed packages are restored and try to register the same ClaimId.

In debug builds this panics in lightning/src/chain/onchaintx.rs with:

assertion failed: self.pending_claim_requests.get(&claim_id).is_none()

Why This Replay Happens In Reality

This is not a fuzz-only shape.

There is a current production path where it happens:

1. A node force-closes an anchor channel with outbound offered HTLCs.
2. Its holder commitment confirms on-chain.
3. ChannelMonitor::check_spend_holder_transaction calls get_broadcasted_holder_claims, which yields one single-outpoint request per holder HTLC output.
4. OnchainTxHandler::update_claims_view_from_requests merges the two outbound timeout requests into one delayed package because their CLTV is still in the future.
5. Later, the user claims an inbound HTLC that was already emitted as PaymentClaimable.
6. ChannelManager::claim_funds generates a real ChannelMonitorUpdateStep::PaymentPreimage.
7. Applying that monitor update calls ChannelMonitor::provide_payment_preimage.
8. When the holder commitment is already confirmed, provide_payment_preimage calls get_broadcasted_holder_claims again, which rebuilds all holder HTLC claim requests for that commitment, including the same outbound timeout requests that were already delayed.

That makes the replay realistic even without any restart-only or legacy-only API.

This affects both current anchor variants:

• keyed anchors
• zero-fee commitments

Root Cause

The delayed-claim dedupe in OnchainTxHandler::update_claims_view_from_requests is too strict.

Today it effectively asks:

• "Do I already have a delayed package with exactly the same outpoint set as this new request?"

That is not sufficient once earlier requests have already been merged. After two single-outpoint requests have been merged into one delayed two-outpoint package, replaying either original single-outpoint request should still be treated as duplicate.

The correct question is:

• "Is every outpoint in this new request already covered by an existing delayed package?"

Affected Production Code

• lightning/src/chain/channelmonitor.rs check_spend_holder_transaction
• lightning/src/chain/channelmonitor.rs get_broadcasted_holder_claims
• lightning/src/chain/channelmonitor.rs provide_payment_preimage
• lightning/src/chain/onchaintx.rs update_claims_view_from_requests

Impact

Debug Builds

Debug builds panic when the second restored delayed package tries to register the same ClaimId.

Release Builds

The assertion is debug-only, so release builds do not panic at that point.

From the current code, the expected release-build behavior is:

• the same logical HTLC claim event is yielded twice with the same ClaimId
• pending_claim_requests.insert(claim_id, req) overwrites the previous request entry for that ClaimId
• consumers may process duplicate HTLCResolution events for the same logical claim

This looks like a correctness and liveness bug, not a direct fund-theft issue by itself.

There is some blast-radius reduction because the event pipeline and coin selection code already key work off ClaimId, so repeated handling of the same claim tends to reuse the same claim identity instead of creating an unrelated second claim. Still, duplicate claim generation is wrong and can create redundant or conflicting downstream work.

Severity

This should be treated as high priority claim-handling breakage, but not as security-critical by itself.

Reproduction

The repro test code is pushed here:

• https://github.com/lightningdevkit/rust-lightning/compare/main...joostjager:rust-lightning:onchain-tx-bug?expand=0

That compare contains both reproducers described below.

Focused OnchainTxHandler Unit Test

File:

• lightning/src/chain/onchaintx.rs

Compare:

• https://github.com/lightningdevkit/rust-lightning/compare/main...joostjager:rust-lightning:onchain-tx-bug?expand=0#diff-7f78085af9a08cd6f49f44c28f9b4b52644ad1d71d0d2d64f5b0b70f2f5982f5

Test:

• test_duplicate_pending_claim_request_after_force_close_replay

What it does:

1. Builds two offered holder HTLC requests with the same future CLTV.
2. Runs update_claims_view_from_requests once, which merges them into one delayed package.
3. Replays the same requests again before the locktime.
4. Advances to the locktime.

Buggy result:

• the same delayed package is restored twice
• the second restore hits the duplicate ClaimId assertion

Higher-Level ChannelMonitor Test Through Current Prod Code

File:

• lightning/src/ln/monitor_tests.rs

Compare:

• https://github.com/lightningdevkit/rust-lightning/compare/main...joostjager:rust-lightning:onchain-tx-bug?expand=0#diff-10ce5452ca31592777dcf9963756763af597357f3471a1518952aa71f6a5758d

Test:

• test_duplicate_delayed_holder_htlc_claims_after_claim_funds_replay

What it does:

1. Opens an anchor channel.
2. Adds two outbound offered HTLCs from node 0 to node 1.
3. Adds one inbound HTLC to node 0 and leaves it at PaymentClaimable.
4. Force-closes node 0.
5. Confirms node 0's holder commitment, which creates the initial delayed timeout package.
6. Calls claim_funds for the inbound HTLC, which creates a real payment-preimage monitor update and replays holder claim generation through ChannelMonitor::provide_payment_preimage.
7. Advances to the CLTV height.

Buggy result:

• the same duplicate ClaimId assertion is hit in OnchainTxHandler

This reproducer does not use provide_payment_preimage_unsafe_legacy.

Expected Behavior

Once a delayed package already covers an outpoint, replaying a fresh single-outpoint request for that outpoint should be ignored.

At CLTV maturity, the delayed package should be restored exactly once and should yield exactly one logical HTLC claim event for that outpoint set.

Proposed Fix Direction

Update delayed-claim dedupe in OnchainTxHandler::update_claims_view_from_requests so that a new request is rejected when all of its outpoints are already covered by an existing delayed package, even if the existing package was formed by merging earlier requests.

In practice:

• replace exact delayed-package equality with covering-package detection
• keep the dedupe narrow so legitimately new outpoints still pass through

As defense in depth, keeping pending claim events keyed by ClaimId remains useful, but it does not address the root cause here. The primary bug is the duplicate delayed package replay.

[joostjager]

@TheBlueMatt, this was found during force close fuzzing. Does it look real to you?

[joostjager]

I think this is real. Will proceed with a fix of this and some adjacent things.