lightningd: don't force-close when fulfilled HTLC removal is in progress - #8940

Closed
vincenzopalazzo wants to merge 4 commits into
ElementsProject:masterfrom
vincenzopalazzo:test/reproduce-issue-8899-fulfilled-htlc-deadline
Closed

lightningd: don't force-close when fulfilled HTLC removal is in progress#8940
vincenzopalazzo wants to merge 4 commits into
ElementsProject:masterfrom
vincenzopalazzo:test/reproduce-issue-8899-fulfilled-htlc-deadline

Conversation

@vincenzopalazzo

@vincenzopalazzovincenzopalazzo commented Mar 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes#8899: CLN force-closes a channel with Fulfilled HTLC SENT_REMOVE_HTLC cltv hit deadline even though the node has the preimage and the fulfill is already in progress.

The bug

In htlcs_notify_new_block(), when a fulfilled incoming HTLC hits the cltv deadline, CLN force-closes the channel without checking whether the HTLC removal is already in progress. The HTLC state machine shows the fulfill was queued to channeld (SENT_REMOVE_HTLC), but the force-close fires before the upstream peer receives update_fulfill_htlc.

This was reported in the wild: the upstream peer was connected and exchanging pings, but CLN force-closed instead of sending the fulfill message.

The fix

When the HTLC is in SENT_REMOVE_HTLC or later state (>= SENT_REMOVE_HTLC), skip the force-close and log a warning instead. This is safe because:

  1. The preimage is persisted to DB — onchaind can claim on-chain if needed
  2. On reconnect, channeld will resend update_fulfill_htlc
  3. If the peer goes on-chain themselves, onchaind handles it with the known preimage
  4. The cooperative path (reconnect + fulfill) is cheaper and preserves the channel

Commits

  1. pytest: reproduce issue FC due to Fulfilled HTLC $ID SENT_REMOVE_HTLC cltv $CLTV hit deadline without attempt to claim #8899 — Test that triggers the buggy force-close (l2 disconnects from l1 before sending update_fulfill_htlc, then mining blocks to the deadline)
  2. lightningd: fix — Skip force-close when hin->hstate >= SENT_REMOVE_HTLC, log log_unusual instead

Changelog-Fixed: Don't force-close channel when fulfilled HTLC hits deadline but removal is already in progress.

Test plan

  • test_fulfilled_htlc_deadline_no_force_close reproduces the bug (first commit)
  • With the fix, l2 logs "but removal already in progress" instead of force-closing
  • CI passes (Valgrind, ASan, integration tests)

🤖 Generated with Claude Code

@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from d6d8693 to e316362CompareMarch 13, 2026 21:04
@vincenzopalazzovincenzopalazzo changed the title pytest: reproduce issue #8899 fulfilled HTLC SENT_REMOVE_HTLC deadline force-closepytest: reproduce issue #8899 fulfilled HTLC deadline force-closeMar 14, 2026
…e force-close
Add test_fulfilled_htlc_deadline_no_force_close to reproduce the bug
where CLN force-closes a channel with "Fulfilled HTLC SENT_REMOVE_HTLC
cltv hit deadline" even though it has the preimage and just needs to
reconnect to send update_fulfill_htlc upstream.
The test sets up l1->l2->l3, sends a payment, and disconnects l2 from
l1 right before update_fulfill_htlc is sent (-WIRE_UPDATE_FULFILL_HTLC).
This leaves the incoming HTLC on the l1-l2 channel stuck in
SENT_REMOVE_HTLC (or SENT_REMOVE_COMMIT under Valgrind). Mining blocks
to the deadline triggers the buggy force-close.
Reproduces: ElementsProject/lightning#8899
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 3ad64fb to cbcc032CompareMarch 15, 2026 00:25
@vincenzopalazzovincenzopalazzo changed the title pytest: reproduce issue #8899 fulfilled HTLC deadline force-closelightningd: don't force-close when fulfilled HTLC removal is in progressMar 15, 2026
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 4cf3776 to 066b3c7CompareMarch 15, 2026 00:35
When an incoming HTLC has been fulfilled (preimage known) and is in
SENT_REMOVE_HTLC or later state, the removal is already in progress:
channeld has been told to send update_fulfill_htlc upstream (or will
be told on reconnect). Force-closing the channel is counterproductive
because:
1. The preimage is persisted to DB and onchaind can claim on-chain
2. The cooperative path (reconnect + fulfill) is cheaper and faster
3. If the peer goes on-chain themselves, onchaind handles it
Instead of force-closing, log a warning and let the removal complete
through the normal state machine.
CI run of the reproducer test (cbcc032) confirms the bug exists.
Before this fix, the force-close fires:
lightningd: Peer permanent failure in CHANNELD_NORMAL:
Fulfilled HTLC 0 SENT_REMOVE_HTLC cltv 119 hit deadline
After this fix, the force-close is skipped and a warning is logged:
lightningd: UNUSUAL: Fulfilled HTLC 0 SENT_REMOVE_HTLC cltv 119
hit deadline, but removal already in progress
Also updates test_htlc_no_force_close and test_htlc_in_timeout which
depended on the old force-close behavior: l3/l2 no longer force-close
for fulfilled HTLCs past deadline; instead the offering peer (l2/l1)
force-closes for the offered HTLC timeout, and the fulfilling node
claims on-chain via onchaind using the preimage.
Changelog-Fixed: Don't force-close channel when fulfilled HTLC hits deadline but removal is already in progress.
Fixes: ElementsProject/lightning#8899
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 5ec99ea to c95850fCompareMarch 15, 2026 10:21
vincenzopalazzoand others added 2 commits March 25, 2026 11:00
When a BOLT 12 invoice is created via createinvoice and is associated
with a local offer, include the offer_id in the invoice_creation event
notification. This allows plugins to determine which offer triggered
an invoice creation without having to call listinvoices for every
created invoice.
For bolt11 invoices and bolt12 invoices not associated with a local
offer, the field is omitted.
Fixes #8191
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ting test_fulfilled_htlc_deadline_no_force_close
* on-chain, onchaind will claim using the
* preimage.
* See https://github.com/ElementsProject/lightning/issues/8899 */
if (hin->hstate >= SENT_REMOVE_HTLC) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isnt the new guard hin->hstate >= SENT_REMOVE_HTLC is unconditionally true at the point it's checked, so channel_fail_permanent() for a fulfilled HTLC becomes dead code - not just for the #8899 race, but for every case?

I mean, from what i see on master:

  1. htlcs_notify_new_block() already requires hin->preimage != NULL to reach this code (lightningd/peer_htlcs.c at line 2998 - if (!hin->preimage) continue;)
  2. hin->preimage is set in exactly one place, fulfill_htlc() (lightningd/peer_htlcs.c at line 397), and the very next line unconditionally advances state - htlc_in_update_state(channel, hin, SENT_REMOVE_HTLC) (line 400), with the comment "We update state now to signal it's in progress, for persistence." No other call site assigns hin->preimage (the only other match, htlc_end.c at line 158, sets it to NULL).
    So hin->preimage != NULL implies hin->hstate >= SENT_REMOVE_HTLC always? The new if block will always be taken, and the original channel_fail_permanent("Fulfilled HTLC /// hit deadline") can never fire again

Comment threadtests/test_closing.py


@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors unsupported')
def test_fulfilled_htlc_deadline_no_force_close(node_factory, bitcoind):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And after the forst comment, test_fulfilled_htlc_deadline_no_force_close and test_fulfilled_htlc_deadline_reconnect don't actually test "removal in progress -> skip" vs. "removal stuck -> still force-close". Once comment is fixed with a real distinguishing condition, these tests should be extended to also cover the case where the peer never reconnects and the channel should still eventually force-close

Sign up for freeto 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.

FC due to Fulfilled HTLC $ID SENT_REMOVE_HTLC cltv $CLTV hit deadline without attempt to claim

4 participants

@vincenzopalazzo@Andezion@madelinevibes@ScuttoZ
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

lightningd: don't force-close when fulfilled HTLC removal is in progress - #8940

Closed
vincenzopalazzo wants to merge 4 commits into
ElementsProject:masterfrom
vincenzopalazzo:test/reproduce-issue-8899-fulfilled-htlc-deadline
Closed

lightningd: don't force-close when fulfilled HTLC removal is in progress#8940
vincenzopalazzo wants to merge 4 commits into
ElementsProject:masterfrom
vincenzopalazzo:test/reproduce-issue-8899-fulfilled-htlc-deadline

Conversation

@vincenzopalazzo

@vincenzopalazzovincenzopalazzo commented Mar 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes#8899: CLN force-closes a channel with Fulfilled HTLC SENT_REMOVE_HTLC cltv hit deadline even though the node has the preimage and the fulfill is already in progress.

The bug

In htlcs_notify_new_block(), when a fulfilled incoming HTLC hits the cltv deadline, CLN force-closes the channel without checking whether the HTLC removal is already in progress. The HTLC state machine shows the fulfill was queued to channeld (SENT_REMOVE_HTLC), but the force-close fires before the upstream peer receives update_fulfill_htlc.

This was reported in the wild: the upstream peer was connected and exchanging pings, but CLN force-closed instead of sending the fulfill message.

The fix

When the HTLC is in SENT_REMOVE_HTLC or later state (>= SENT_REMOVE_HTLC), skip the force-close and log a warning instead. This is safe because:

  1. The preimage is persisted to DB — onchaind can claim on-chain if needed
  2. On reconnect, channeld will resend update_fulfill_htlc
  3. If the peer goes on-chain themselves, onchaind handles it with the known preimage
  4. The cooperative path (reconnect + fulfill) is cheaper and preserves the channel

Commits

  1. pytest: reproduce issue FC due to Fulfilled HTLC $ID SENT_REMOVE_HTLC cltv $CLTV hit deadline without attempt to claim #8899 — Test that triggers the buggy force-close (l2 disconnects from l1 before sending update_fulfill_htlc, then mining blocks to the deadline)
  2. lightningd: fix — Skip force-close when hin->hstate >= SENT_REMOVE_HTLC, log log_unusual instead

Changelog-Fixed: Don't force-close channel when fulfilled HTLC hits deadline but removal is already in progress.

Test plan

  • test_fulfilled_htlc_deadline_no_force_close reproduces the bug (first commit)
  • With the fix, l2 logs "but removal already in progress" instead of force-closing
  • CI passes (Valgrind, ASan, integration tests)

🤖 Generated with Claude Code

@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from d6d8693 to e316362CompareMarch 13, 2026 21:04
@vincenzopalazzovincenzopalazzo changed the title pytest: reproduce issue #8899 fulfilled HTLC SENT_REMOVE_HTLC deadline force-closepytest: reproduce issue #8899 fulfilled HTLC deadline force-closeMar 14, 2026
…e force-close
Add test_fulfilled_htlc_deadline_no_force_close to reproduce the bug
where CLN force-closes a channel with "Fulfilled HTLC SENT_REMOVE_HTLC
cltv hit deadline" even though it has the preimage and just needs to
reconnect to send update_fulfill_htlc upstream.
The test sets up l1->l2->l3, sends a payment, and disconnects l2 from
l1 right before update_fulfill_htlc is sent (-WIRE_UPDATE_FULFILL_HTLC).
This leaves the incoming HTLC on the l1-l2 channel stuck in
SENT_REMOVE_HTLC (or SENT_REMOVE_COMMIT under Valgrind). Mining blocks
to the deadline triggers the buggy force-close.
Reproduces: ElementsProject/lightning#8899
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 3ad64fb to cbcc032CompareMarch 15, 2026 00:25
@vincenzopalazzovincenzopalazzo changed the title pytest: reproduce issue #8899 fulfilled HTLC deadline force-closelightningd: don't force-close when fulfilled HTLC removal is in progressMar 15, 2026
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 4cf3776 to 066b3c7CompareMarch 15, 2026 00:35
When an incoming HTLC has been fulfilled (preimage known) and is in
SENT_REMOVE_HTLC or later state, the removal is already in progress:
channeld has been told to send update_fulfill_htlc upstream (or will
be told on reconnect). Force-closing the channel is counterproductive
because:
1. The preimage is persisted to DB and onchaind can claim on-chain
2. The cooperative path (reconnect + fulfill) is cheaper and faster
3. If the peer goes on-chain themselves, onchaind handles it
Instead of force-closing, log a warning and let the removal complete
through the normal state machine.
CI run of the reproducer test (cbcc032) confirms the bug exists.
Before this fix, the force-close fires:
lightningd: Peer permanent failure in CHANNELD_NORMAL:
Fulfilled HTLC 0 SENT_REMOVE_HTLC cltv 119 hit deadline
After this fix, the force-close is skipped and a warning is logged:
lightningd: UNUSUAL: Fulfilled HTLC 0 SENT_REMOVE_HTLC cltv 119
hit deadline, but removal already in progress
Also updates test_htlc_no_force_close and test_htlc_in_timeout which
depended on the old force-close behavior: l3/l2 no longer force-close
for fulfilled HTLCs past deadline; instead the offering peer (l2/l1)
force-closes for the offered HTLC timeout, and the fulfilling node
claims on-chain via onchaind using the preimage.
Changelog-Fixed: Don't force-close channel when fulfilled HTLC hits deadline but removal is already in progress.
Fixes: ElementsProject/lightning#8899
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 5ec99ea to c95850fCompareMarch 15, 2026 10:21
vincenzopalazzoand others added 2 commits March 25, 2026 11:00
When a BOLT 12 invoice is created via createinvoice and is associated
with a local offer, include the offer_id in the invoice_creation event
notification. This allows plugins to determine which offer triggered
an invoice creation without having to call listinvoices for every
created invoice.
For bolt11 invoices and bolt12 invoices not associated with a local
offer, the field is omitted.
Fixes #8191
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ting test_fulfilled_htlc_deadline_no_force_close
* on-chain, onchaind will claim using the
* preimage.
* See https://github.com/ElementsProject/lightning/issues/8899 */
if (hin->hstate >= SENT_REMOVE_HTLC) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isnt the new guard hin->hstate >= SENT_REMOVE_HTLC is unconditionally true at the point it's checked, so channel_fail_permanent() for a fulfilled HTLC becomes dead code - not just for the #8899 race, but for every case?

I mean, from what i see on master:

  1. htlcs_notify_new_block() already requires hin->preimage != NULL to reach this code (lightningd/peer_htlcs.c at line 2998 - if (!hin->preimage) continue;)
  2. hin->preimage is set in exactly one place, fulfill_htlc() (lightningd/peer_htlcs.c at line 397), and the very next line unconditionally advances state - htlc_in_update_state(channel, hin, SENT_REMOVE_HTLC) (line 400), with the comment "We update state now to signal it's in progress, for persistence." No other call site assigns hin->preimage (the only other match, htlc_end.c at line 158, sets it to NULL).
    So hin->preimage != NULL implies hin->hstate >= SENT_REMOVE_HTLC always? The new if block will always be taken, and the original channel_fail_permanent("Fulfilled HTLC /// hit deadline") can never fire again

Comment threadtests/test_closing.py


@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors unsupported')
def test_fulfilled_htlc_deadline_no_force_close(node_factory, bitcoind):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And after the forst comment, test_fulfilled_htlc_deadline_no_force_close and test_fulfilled_htlc_deadline_reconnect don't actually test "removal in progress -> skip" vs. "removal stuck -> still force-close". Once comment is fixed with a real distinguishing condition, these tests should be extended to also cover the case where the peer never reconnects and the channel should still eventually force-close

Sign up for freeto 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.

FC due to Fulfilled HTLC $ID SENT_REMOVE_HTLC cltv $CLTV hit deadline without attempt to claim

4 participants

@vincenzopalazzo@Andezion@madelinevibes@ScuttoZ
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

lightningd: don't force-close when fulfilled HTLC removal is in progress - #8940

Closed
vincenzopalazzo wants to merge 4 commits into
ElementsProject:masterfrom
vincenzopalazzo:test/reproduce-issue-8899-fulfilled-htlc-deadline
Closed

lightningd: don't force-close when fulfilled HTLC removal is in progress#8940
vincenzopalazzo wants to merge 4 commits into
ElementsProject:masterfrom
vincenzopalazzo:test/reproduce-issue-8899-fulfilled-htlc-deadline

Conversation

@vincenzopalazzo

@vincenzopalazzovincenzopalazzo commented Mar 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes#8899: CLN force-closes a channel with Fulfilled HTLC SENT_REMOVE_HTLC cltv hit deadline even though the node has the preimage and the fulfill is already in progress.

The bug

In htlcs_notify_new_block(), when a fulfilled incoming HTLC hits the cltv deadline, CLN force-closes the channel without checking whether the HTLC removal is already in progress. The HTLC state machine shows the fulfill was queued to channeld (SENT_REMOVE_HTLC), but the force-close fires before the upstream peer receives update_fulfill_htlc.

This was reported in the wild: the upstream peer was connected and exchanging pings, but CLN force-closed instead of sending the fulfill message.

The fix

When the HTLC is in SENT_REMOVE_HTLC or later state (>= SENT_REMOVE_HTLC), skip the force-close and log a warning instead. This is safe because:

  1. The preimage is persisted to DB — onchaind can claim on-chain if needed
  2. On reconnect, channeld will resend update_fulfill_htlc
  3. If the peer goes on-chain themselves, onchaind handles it with the known preimage
  4. The cooperative path (reconnect + fulfill) is cheaper and preserves the channel

Commits

  1. pytest: reproduce issue FC due to Fulfilled HTLC $ID SENT_REMOVE_HTLC cltv $CLTV hit deadline without attempt to claim #8899 — Test that triggers the buggy force-close (l2 disconnects from l1 before sending update_fulfill_htlc, then mining blocks to the deadline)
  2. lightningd: fix — Skip force-close when hin->hstate >= SENT_REMOVE_HTLC, log log_unusual instead

Changelog-Fixed: Don't force-close channel when fulfilled HTLC hits deadline but removal is already in progress.

Test plan

  • test_fulfilled_htlc_deadline_no_force_close reproduces the bug (first commit)
  • With the fix, l2 logs "but removal already in progress" instead of force-closing
  • CI passes (Valgrind, ASan, integration tests)

🤖 Generated with Claude Code

@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from d6d8693 to e316362CompareMarch 13, 2026 21:04
@vincenzopalazzovincenzopalazzo changed the title pytest: reproduce issue #8899 fulfilled HTLC SENT_REMOVE_HTLC deadline force-closepytest: reproduce issue #8899 fulfilled HTLC deadline force-closeMar 14, 2026
…e force-close
Add test_fulfilled_htlc_deadline_no_force_close to reproduce the bug
where CLN force-closes a channel with "Fulfilled HTLC SENT_REMOVE_HTLC
cltv hit deadline" even though it has the preimage and just needs to
reconnect to send update_fulfill_htlc upstream.
The test sets up l1->l2->l3, sends a payment, and disconnects l2 from
l1 right before update_fulfill_htlc is sent (-WIRE_UPDATE_FULFILL_HTLC).
This leaves the incoming HTLC on the l1-l2 channel stuck in
SENT_REMOVE_HTLC (or SENT_REMOVE_COMMIT under Valgrind). Mining blocks
to the deadline triggers the buggy force-close.
Reproduces: ElementsProject/lightning#8899
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 3ad64fb to cbcc032CompareMarch 15, 2026 00:25
@vincenzopalazzovincenzopalazzo changed the title pytest: reproduce issue #8899 fulfilled HTLC deadline force-closelightningd: don't force-close when fulfilled HTLC removal is in progressMar 15, 2026
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 4cf3776 to 066b3c7CompareMarch 15, 2026 00:35
When an incoming HTLC has been fulfilled (preimage known) and is in
SENT_REMOVE_HTLC or later state, the removal is already in progress:
channeld has been told to send update_fulfill_htlc upstream (or will
be told on reconnect). Force-closing the channel is counterproductive
because:
1. The preimage is persisted to DB and onchaind can claim on-chain
2. The cooperative path (reconnect + fulfill) is cheaper and faster
3. If the peer goes on-chain themselves, onchaind handles it
Instead of force-closing, log a warning and let the removal complete
through the normal state machine.
CI run of the reproducer test (cbcc032) confirms the bug exists.
Before this fix, the force-close fires:
lightningd: Peer permanent failure in CHANNELD_NORMAL:
Fulfilled HTLC 0 SENT_REMOVE_HTLC cltv 119 hit deadline
After this fix, the force-close is skipped and a warning is logged:
lightningd: UNUSUAL: Fulfilled HTLC 0 SENT_REMOVE_HTLC cltv 119
hit deadline, but removal already in progress
Also updates test_htlc_no_force_close and test_htlc_in_timeout which
depended on the old force-close behavior: l3/l2 no longer force-close
for fulfilled HTLCs past deadline; instead the offering peer (l2/l1)
force-closes for the offered HTLC timeout, and the fulfilling node
claims on-chain via onchaind using the preimage.
Changelog-Fixed: Don't force-close channel when fulfilled HTLC hits deadline but removal is already in progress.
Fixes: ElementsProject/lightning#8899
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 5ec99ea to c95850fCompareMarch 15, 2026 10:21
vincenzopalazzoand others added 2 commits March 25, 2026 11:00
When a BOLT 12 invoice is created via createinvoice and is associated
with a local offer, include the offer_id in the invoice_creation event
notification. This allows plugins to determine which offer triggered
an invoice creation without having to call listinvoices for every
created invoice.
For bolt11 invoices and bolt12 invoices not associated with a local
offer, the field is omitted.
Fixes #8191
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ting test_fulfilled_htlc_deadline_no_force_close
* on-chain, onchaind will claim using the
* preimage.
* See https://github.com/ElementsProject/lightning/issues/8899 */
if (hin->hstate >= SENT_REMOVE_HTLC) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isnt the new guard hin->hstate >= SENT_REMOVE_HTLC is unconditionally true at the point it's checked, so channel_fail_permanent() for a fulfilled HTLC becomes dead code - not just for the #8899 race, but for every case?

I mean, from what i see on master:

  1. htlcs_notify_new_block() already requires hin->preimage != NULL to reach this code (lightningd/peer_htlcs.c at line 2998 - if (!hin->preimage) continue;)
  2. hin->preimage is set in exactly one place, fulfill_htlc() (lightningd/peer_htlcs.c at line 397), and the very next line unconditionally advances state - htlc_in_update_state(channel, hin, SENT_REMOVE_HTLC) (line 400), with the comment "We update state now to signal it's in progress, for persistence." No other call site assigns hin->preimage (the only other match, htlc_end.c at line 158, sets it to NULL).
    So hin->preimage != NULL implies hin->hstate >= SENT_REMOVE_HTLC always? The new if block will always be taken, and the original channel_fail_permanent("Fulfilled HTLC /// hit deadline") can never fire again

Comment threadtests/test_closing.py


@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors unsupported')
def test_fulfilled_htlc_deadline_no_force_close(node_factory, bitcoind):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And after the forst comment, test_fulfilled_htlc_deadline_no_force_close and test_fulfilled_htlc_deadline_reconnect don't actually test "removal in progress -> skip" vs. "removal stuck -> still force-close". Once comment is fixed with a real distinguishing condition, these tests should be extended to also cover the case where the peer never reconnects and the channel should still eventually force-close

Sign up for freeto 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.

FC due to Fulfilled HTLC $ID SENT_REMOVE_HTLC cltv $CLTV hit deadline without attempt to claim

4 participants

@vincenzopalazzo@Andezion@madelinevibes@ScuttoZ
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

lightningd: don't force-close when fulfilled HTLC removal is in progress - #8940

Closed
vincenzopalazzo wants to merge 4 commits into
ElementsProject:masterfrom
vincenzopalazzo:test/reproduce-issue-8899-fulfilled-htlc-deadline
Closed

lightningd: don't force-close when fulfilled HTLC removal is in progress#8940
vincenzopalazzo wants to merge 4 commits into
ElementsProject:masterfrom
vincenzopalazzo:test/reproduce-issue-8899-fulfilled-htlc-deadline

Conversation

@vincenzopalazzo

@vincenzopalazzovincenzopalazzo commented Mar 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes#8899: CLN force-closes a channel with Fulfilled HTLC SENT_REMOVE_HTLC cltv hit deadline even though the node has the preimage and the fulfill is already in progress.

The bug

In htlcs_notify_new_block(), when a fulfilled incoming HTLC hits the cltv deadline, CLN force-closes the channel without checking whether the HTLC removal is already in progress. The HTLC state machine shows the fulfill was queued to channeld (SENT_REMOVE_HTLC), but the force-close fires before the upstream peer receives update_fulfill_htlc.

This was reported in the wild: the upstream peer was connected and exchanging pings, but CLN force-closed instead of sending the fulfill message.

The fix

When the HTLC is in SENT_REMOVE_HTLC or later state (>= SENT_REMOVE_HTLC), skip the force-close and log a warning instead. This is safe because:

  1. The preimage is persisted to DB — onchaind can claim on-chain if needed
  2. On reconnect, channeld will resend update_fulfill_htlc
  3. If the peer goes on-chain themselves, onchaind handles it with the known preimage
  4. The cooperative path (reconnect + fulfill) is cheaper and preserves the channel

Commits

  1. pytest: reproduce issue FC due to Fulfilled HTLC $ID SENT_REMOVE_HTLC cltv $CLTV hit deadline without attempt to claim #8899 — Test that triggers the buggy force-close (l2 disconnects from l1 before sending update_fulfill_htlc, then mining blocks to the deadline)
  2. lightningd: fix — Skip force-close when hin->hstate >= SENT_REMOVE_HTLC, log log_unusual instead

Changelog-Fixed: Don't force-close channel when fulfilled HTLC hits deadline but removal is already in progress.

Test plan

  • test_fulfilled_htlc_deadline_no_force_close reproduces the bug (first commit)
  • With the fix, l2 logs "but removal already in progress" instead of force-closing
  • CI passes (Valgrind, ASan, integration tests)

🤖 Generated with Claude Code

@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from d6d8693 to e316362CompareMarch 13, 2026 21:04
@vincenzopalazzovincenzopalazzo changed the title pytest: reproduce issue #8899 fulfilled HTLC SENT_REMOVE_HTLC deadline force-closepytest: reproduce issue #8899 fulfilled HTLC deadline force-closeMar 14, 2026
…e force-close
Add test_fulfilled_htlc_deadline_no_force_close to reproduce the bug
where CLN force-closes a channel with "Fulfilled HTLC SENT_REMOVE_HTLC
cltv hit deadline" even though it has the preimage and just needs to
reconnect to send update_fulfill_htlc upstream.
The test sets up l1->l2->l3, sends a payment, and disconnects l2 from
l1 right before update_fulfill_htlc is sent (-WIRE_UPDATE_FULFILL_HTLC).
This leaves the incoming HTLC on the l1-l2 channel stuck in
SENT_REMOVE_HTLC (or SENT_REMOVE_COMMIT under Valgrind). Mining blocks
to the deadline triggers the buggy force-close.
Reproduces: ElementsProject/lightning#8899
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 3ad64fb to cbcc032CompareMarch 15, 2026 00:25
@vincenzopalazzovincenzopalazzo changed the title pytest: reproduce issue #8899 fulfilled HTLC deadline force-closelightningd: don't force-close when fulfilled HTLC removal is in progressMar 15, 2026
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 4cf3776 to 066b3c7CompareMarch 15, 2026 00:35
When an incoming HTLC has been fulfilled (preimage known) and is in
SENT_REMOVE_HTLC or later state, the removal is already in progress:
channeld has been told to send update_fulfill_htlc upstream (or will
be told on reconnect). Force-closing the channel is counterproductive
because:
1. The preimage is persisted to DB and onchaind can claim on-chain
2. The cooperative path (reconnect + fulfill) is cheaper and faster
3. If the peer goes on-chain themselves, onchaind handles it
Instead of force-closing, log a warning and let the removal complete
through the normal state machine.
CI run of the reproducer test (cbcc032) confirms the bug exists.
Before this fix, the force-close fires:
lightningd: Peer permanent failure in CHANNELD_NORMAL:
Fulfilled HTLC 0 SENT_REMOVE_HTLC cltv 119 hit deadline
After this fix, the force-close is skipped and a warning is logged:
lightningd: UNUSUAL: Fulfilled HTLC 0 SENT_REMOVE_HTLC cltv 119
hit deadline, but removal already in progress
Also updates test_htlc_no_force_close and test_htlc_in_timeout which
depended on the old force-close behavior: l3/l2 no longer force-close
for fulfilled HTLCs past deadline; instead the offering peer (l2/l1)
force-closes for the offered HTLC timeout, and the fulfilling node
claims on-chain via onchaind using the preimage.
Changelog-Fixed: Don't force-close channel when fulfilled HTLC hits deadline but removal is already in progress.
Fixes: ElementsProject/lightning#8899
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 5ec99ea to c95850fCompareMarch 15, 2026 10:21
vincenzopalazzoand others added 2 commits March 25, 2026 11:00
When a BOLT 12 invoice is created via createinvoice and is associated
with a local offer, include the offer_id in the invoice_creation event
notification. This allows plugins to determine which offer triggered
an invoice creation without having to call listinvoices for every
created invoice.
For bolt11 invoices and bolt12 invoices not associated with a local
offer, the field is omitted.
Fixes #8191
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ting test_fulfilled_htlc_deadline_no_force_close
* on-chain, onchaind will claim using the
* preimage.
* See https://github.com/ElementsProject/lightning/issues/8899 */
if (hin->hstate >= SENT_REMOVE_HTLC) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isnt the new guard hin->hstate >= SENT_REMOVE_HTLC is unconditionally true at the point it's checked, so channel_fail_permanent() for a fulfilled HTLC becomes dead code - not just for the #8899 race, but for every case?

I mean, from what i see on master:

  1. htlcs_notify_new_block() already requires hin->preimage != NULL to reach this code (lightningd/peer_htlcs.c at line 2998 - if (!hin->preimage) continue;)
  2. hin->preimage is set in exactly one place, fulfill_htlc() (lightningd/peer_htlcs.c at line 397), and the very next line unconditionally advances state - htlc_in_update_state(channel, hin, SENT_REMOVE_HTLC) (line 400), with the comment "We update state now to signal it's in progress, for persistence." No other call site assigns hin->preimage (the only other match, htlc_end.c at line 158, sets it to NULL).
    So hin->preimage != NULL implies hin->hstate >= SENT_REMOVE_HTLC always? The new if block will always be taken, and the original channel_fail_permanent("Fulfilled HTLC /// hit deadline") can never fire again

Comment threadtests/test_closing.py


@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors unsupported')
def test_fulfilled_htlc_deadline_no_force_close(node_factory, bitcoind):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And after the forst comment, test_fulfilled_htlc_deadline_no_force_close and test_fulfilled_htlc_deadline_reconnect don't actually test "removal in progress -> skip" vs. "removal stuck -> still force-close". Once comment is fixed with a real distinguishing condition, these tests should be extended to also cover the case where the peer never reconnects and the channel should still eventually force-close

Sign up for freeto 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.

FC due to Fulfilled HTLC $ID SENT_REMOVE_HTLC cltv $CLTV hit deadline without attempt to claim

4 participants

@vincenzopalazzo@Andezion@madelinevibes@ScuttoZ
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

lightningd: don't force-close when fulfilled HTLC removal is in progress - #8940

Closed
vincenzopalazzo wants to merge 4 commits into
ElementsProject:masterfrom
vincenzopalazzo:test/reproduce-issue-8899-fulfilled-htlc-deadline
Closed

lightningd: don't force-close when fulfilled HTLC removal is in progress#8940
vincenzopalazzo wants to merge 4 commits into
ElementsProject:masterfrom
vincenzopalazzo:test/reproduce-issue-8899-fulfilled-htlc-deadline

Conversation

@vincenzopalazzo

@vincenzopalazzovincenzopalazzo commented Mar 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes#8899: CLN force-closes a channel with Fulfilled HTLC SENT_REMOVE_HTLC cltv hit deadline even though the node has the preimage and the fulfill is already in progress.

The bug

In htlcs_notify_new_block(), when a fulfilled incoming HTLC hits the cltv deadline, CLN force-closes the channel without checking whether the HTLC removal is already in progress. The HTLC state machine shows the fulfill was queued to channeld (SENT_REMOVE_HTLC), but the force-close fires before the upstream peer receives update_fulfill_htlc.

This was reported in the wild: the upstream peer was connected and exchanging pings, but CLN force-closed instead of sending the fulfill message.

The fix

When the HTLC is in SENT_REMOVE_HTLC or later state (>= SENT_REMOVE_HTLC), skip the force-close and log a warning instead. This is safe because:

  1. The preimage is persisted to DB — onchaind can claim on-chain if needed
  2. On reconnect, channeld will resend update_fulfill_htlc
  3. If the peer goes on-chain themselves, onchaind handles it with the known preimage
  4. The cooperative path (reconnect + fulfill) is cheaper and preserves the channel

Commits

  1. pytest: reproduce issue FC due to Fulfilled HTLC $ID SENT_REMOVE_HTLC cltv $CLTV hit deadline without attempt to claim #8899 — Test that triggers the buggy force-close (l2 disconnects from l1 before sending update_fulfill_htlc, then mining blocks to the deadline)
  2. lightningd: fix — Skip force-close when hin->hstate >= SENT_REMOVE_HTLC, log log_unusual instead

Changelog-Fixed: Don't force-close channel when fulfilled HTLC hits deadline but removal is already in progress.

Test plan

  • test_fulfilled_htlc_deadline_no_force_close reproduces the bug (first commit)
  • With the fix, l2 logs "but removal already in progress" instead of force-closing
  • CI passes (Valgrind, ASan, integration tests)

🤖 Generated with Claude Code

@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from d6d8693 to e316362CompareMarch 13, 2026 21:04
@vincenzopalazzovincenzopalazzo changed the title pytest: reproduce issue #8899 fulfilled HTLC SENT_REMOVE_HTLC deadline force-closepytest: reproduce issue #8899 fulfilled HTLC deadline force-closeMar 14, 2026
…e force-close
Add test_fulfilled_htlc_deadline_no_force_close to reproduce the bug
where CLN force-closes a channel with "Fulfilled HTLC SENT_REMOVE_HTLC
cltv hit deadline" even though it has the preimage and just needs to
reconnect to send update_fulfill_htlc upstream.
The test sets up l1->l2->l3, sends a payment, and disconnects l2 from
l1 right before update_fulfill_htlc is sent (-WIRE_UPDATE_FULFILL_HTLC).
This leaves the incoming HTLC on the l1-l2 channel stuck in
SENT_REMOVE_HTLC (or SENT_REMOVE_COMMIT under Valgrind). Mining blocks
to the deadline triggers the buggy force-close.
Reproduces: ElementsProject/lightning#8899
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 3ad64fb to cbcc032CompareMarch 15, 2026 00:25
@vincenzopalazzovincenzopalazzo changed the title pytest: reproduce issue #8899 fulfilled HTLC deadline force-closelightningd: don't force-close when fulfilled HTLC removal is in progressMar 15, 2026
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 4cf3776 to 066b3c7CompareMarch 15, 2026 00:35
When an incoming HTLC has been fulfilled (preimage known) and is in
SENT_REMOVE_HTLC or later state, the removal is already in progress:
channeld has been told to send update_fulfill_htlc upstream (or will
be told on reconnect). Force-closing the channel is counterproductive
because:
1. The preimage is persisted to DB and onchaind can claim on-chain
2. The cooperative path (reconnect + fulfill) is cheaper and faster
3. If the peer goes on-chain themselves, onchaind handles it
Instead of force-closing, log a warning and let the removal complete
through the normal state machine.
CI run of the reproducer test (cbcc032) confirms the bug exists.
Before this fix, the force-close fires:
lightningd: Peer permanent failure in CHANNELD_NORMAL:
Fulfilled HTLC 0 SENT_REMOVE_HTLC cltv 119 hit deadline
After this fix, the force-close is skipped and a warning is logged:
lightningd: UNUSUAL: Fulfilled HTLC 0 SENT_REMOVE_HTLC cltv 119
hit deadline, but removal already in progress
Also updates test_htlc_no_force_close and test_htlc_in_timeout which
depended on the old force-close behavior: l3/l2 no longer force-close
for fulfilled HTLCs past deadline; instead the offering peer (l2/l1)
force-closes for the offered HTLC timeout, and the fulfilling node
claims on-chain via onchaind using the preimage.
Changelog-Fixed: Don't force-close channel when fulfilled HTLC hits deadline but removal is already in progress.
Fixes: ElementsProject/lightning#8899
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 5ec99ea to c95850fCompareMarch 15, 2026 10:21
vincenzopalazzoand others added 2 commits March 25, 2026 11:00
When a BOLT 12 invoice is created via createinvoice and is associated
with a local offer, include the offer_id in the invoice_creation event
notification. This allows plugins to determine which offer triggered
an invoice creation without having to call listinvoices for every
created invoice.
For bolt11 invoices and bolt12 invoices not associated with a local
offer, the field is omitted.
Fixes #8191
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ting test_fulfilled_htlc_deadline_no_force_close
* on-chain, onchaind will claim using the
* preimage.
* See https://github.com/ElementsProject/lightning/issues/8899 */
if (hin->hstate >= SENT_REMOVE_HTLC) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isnt the new guard hin->hstate >= SENT_REMOVE_HTLC is unconditionally true at the point it's checked, so channel_fail_permanent() for a fulfilled HTLC becomes dead code - not just for the #8899 race, but for every case?

I mean, from what i see on master:

  1. htlcs_notify_new_block() already requires hin->preimage != NULL to reach this code (lightningd/peer_htlcs.c at line 2998 - if (!hin->preimage) continue;)
  2. hin->preimage is set in exactly one place, fulfill_htlc() (lightningd/peer_htlcs.c at line 397), and the very next line unconditionally advances state - htlc_in_update_state(channel, hin, SENT_REMOVE_HTLC) (line 400), with the comment "We update state now to signal it's in progress, for persistence." No other call site assigns hin->preimage (the only other match, htlc_end.c at line 158, sets it to NULL).
    So hin->preimage != NULL implies hin->hstate >= SENT_REMOVE_HTLC always? The new if block will always be taken, and the original channel_fail_permanent("Fulfilled HTLC /// hit deadline") can never fire again

Comment threadtests/test_closing.py


@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors unsupported')
def test_fulfilled_htlc_deadline_no_force_close(node_factory, bitcoind):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And after the forst comment, test_fulfilled_htlc_deadline_no_force_close and test_fulfilled_htlc_deadline_reconnect don't actually test "removal in progress -> skip" vs. "removal stuck -> still force-close". Once comment is fixed with a real distinguishing condition, these tests should be extended to also cover the case where the peer never reconnects and the channel should still eventually force-close

Sign up for freeto 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.

FC due to Fulfilled HTLC $ID SENT_REMOVE_HTLC cltv $CLTV hit deadline without attempt to claim

4 participants

@vincenzopalazzo@Andezion@madelinevibes@ScuttoZ
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

lightningd: don't force-close when fulfilled HTLC removal is in progress - #8940

Closed
vincenzopalazzo wants to merge 4 commits into
ElementsProject:masterfrom
vincenzopalazzo:test/reproduce-issue-8899-fulfilled-htlc-deadline
Closed

lightningd: don't force-close when fulfilled HTLC removal is in progress#8940
vincenzopalazzo wants to merge 4 commits into
ElementsProject:masterfrom
vincenzopalazzo:test/reproduce-issue-8899-fulfilled-htlc-deadline

Conversation

@vincenzopalazzo

@vincenzopalazzovincenzopalazzo commented Mar 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes#8899: CLN force-closes a channel with Fulfilled HTLC SENT_REMOVE_HTLC cltv hit deadline even though the node has the preimage and the fulfill is already in progress.

The bug

In htlcs_notify_new_block(), when a fulfilled incoming HTLC hits the cltv deadline, CLN force-closes the channel without checking whether the HTLC removal is already in progress. The HTLC state machine shows the fulfill was queued to channeld (SENT_REMOVE_HTLC), but the force-close fires before the upstream peer receives update_fulfill_htlc.

This was reported in the wild: the upstream peer was connected and exchanging pings, but CLN force-closed instead of sending the fulfill message.

The fix

When the HTLC is in SENT_REMOVE_HTLC or later state (>= SENT_REMOVE_HTLC), skip the force-close and log a warning instead. This is safe because:

  1. The preimage is persisted to DB — onchaind can claim on-chain if needed
  2. On reconnect, channeld will resend update_fulfill_htlc
  3. If the peer goes on-chain themselves, onchaind handles it with the known preimage
  4. The cooperative path (reconnect + fulfill) is cheaper and preserves the channel

Commits

  1. pytest: reproduce issue FC due to Fulfilled HTLC $ID SENT_REMOVE_HTLC cltv $CLTV hit deadline without attempt to claim #8899 — Test that triggers the buggy force-close (l2 disconnects from l1 before sending update_fulfill_htlc, then mining blocks to the deadline)
  2. lightningd: fix — Skip force-close when hin->hstate >= SENT_REMOVE_HTLC, log log_unusual instead

Changelog-Fixed: Don't force-close channel when fulfilled HTLC hits deadline but removal is already in progress.

Test plan

  • test_fulfilled_htlc_deadline_no_force_close reproduces the bug (first commit)
  • With the fix, l2 logs "but removal already in progress" instead of force-closing
  • CI passes (Valgrind, ASan, integration tests)

🤖 Generated with Claude Code

@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from d6d8693 to e316362CompareMarch 13, 2026 21:04
@vincenzopalazzovincenzopalazzo changed the title pytest: reproduce issue #8899 fulfilled HTLC SENT_REMOVE_HTLC deadline force-closepytest: reproduce issue #8899 fulfilled HTLC deadline force-closeMar 14, 2026
…e force-close
Add test_fulfilled_htlc_deadline_no_force_close to reproduce the bug
where CLN force-closes a channel with "Fulfilled HTLC SENT_REMOVE_HTLC
cltv hit deadline" even though it has the preimage and just needs to
reconnect to send update_fulfill_htlc upstream.
The test sets up l1->l2->l3, sends a payment, and disconnects l2 from
l1 right before update_fulfill_htlc is sent (-WIRE_UPDATE_FULFILL_HTLC).
This leaves the incoming HTLC on the l1-l2 channel stuck in
SENT_REMOVE_HTLC (or SENT_REMOVE_COMMIT under Valgrind). Mining blocks
to the deadline triggers the buggy force-close.
Reproduces: ElementsProject/lightning#8899
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 3ad64fb to cbcc032CompareMarch 15, 2026 00:25
@vincenzopalazzovincenzopalazzo changed the title pytest: reproduce issue #8899 fulfilled HTLC deadline force-closelightningd: don't force-close when fulfilled HTLC removal is in progressMar 15, 2026
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 4cf3776 to 066b3c7CompareMarch 15, 2026 00:35
When an incoming HTLC has been fulfilled (preimage known) and is in
SENT_REMOVE_HTLC or later state, the removal is already in progress:
channeld has been told to send update_fulfill_htlc upstream (or will
be told on reconnect). Force-closing the channel is counterproductive
because:
1. The preimage is persisted to DB and onchaind can claim on-chain
2. The cooperative path (reconnect + fulfill) is cheaper and faster
3. If the peer goes on-chain themselves, onchaind handles it
Instead of force-closing, log a warning and let the removal complete
through the normal state machine.
CI run of the reproducer test (cbcc032) confirms the bug exists.
Before this fix, the force-close fires:
lightningd: Peer permanent failure in CHANNELD_NORMAL:
Fulfilled HTLC 0 SENT_REMOVE_HTLC cltv 119 hit deadline
After this fix, the force-close is skipped and a warning is logged:
lightningd: UNUSUAL: Fulfilled HTLC 0 SENT_REMOVE_HTLC cltv 119
hit deadline, but removal already in progress
Also updates test_htlc_no_force_close and test_htlc_in_timeout which
depended on the old force-close behavior: l3/l2 no longer force-close
for fulfilled HTLCs past deadline; instead the offering peer (l2/l1)
force-closes for the offered HTLC timeout, and the fulfilling node
claims on-chain via onchaind using the preimage.
Changelog-Fixed: Don't force-close channel when fulfilled HTLC hits deadline but removal is already in progress.
Fixes: ElementsProject/lightning#8899
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 5ec99ea to c95850fCompareMarch 15, 2026 10:21
vincenzopalazzoand others added 2 commits March 25, 2026 11:00
When a BOLT 12 invoice is created via createinvoice and is associated
with a local offer, include the offer_id in the invoice_creation event
notification. This allows plugins to determine which offer triggered
an invoice creation without having to call listinvoices for every
created invoice.
For bolt11 invoices and bolt12 invoices not associated with a local
offer, the field is omitted.
Fixes #8191
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ting test_fulfilled_htlc_deadline_no_force_close
* on-chain, onchaind will claim using the
* preimage.
* See https://github.com/ElementsProject/lightning/issues/8899 */
if (hin->hstate >= SENT_REMOVE_HTLC) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isnt the new guard hin->hstate >= SENT_REMOVE_HTLC is unconditionally true at the point it's checked, so channel_fail_permanent() for a fulfilled HTLC becomes dead code - not just for the #8899 race, but for every case?

I mean, from what i see on master:

  1. htlcs_notify_new_block() already requires hin->preimage != NULL to reach this code (lightningd/peer_htlcs.c at line 2998 - if (!hin->preimage) continue;)
  2. hin->preimage is set in exactly one place, fulfill_htlc() (lightningd/peer_htlcs.c at line 397), and the very next line unconditionally advances state - htlc_in_update_state(channel, hin, SENT_REMOVE_HTLC) (line 400), with the comment "We update state now to signal it's in progress, for persistence." No other call site assigns hin->preimage (the only other match, htlc_end.c at line 158, sets it to NULL).
    So hin->preimage != NULL implies hin->hstate >= SENT_REMOVE_HTLC always? The new if block will always be taken, and the original channel_fail_permanent("Fulfilled HTLC /// hit deadline") can never fire again

Comment threadtests/test_closing.py


@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors unsupported')
def test_fulfilled_htlc_deadline_no_force_close(node_factory, bitcoind):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And after the forst comment, test_fulfilled_htlc_deadline_no_force_close and test_fulfilled_htlc_deadline_reconnect don't actually test "removal in progress -> skip" vs. "removal stuck -> still force-close". Once comment is fixed with a real distinguishing condition, these tests should be extended to also cover the case where the peer never reconnects and the channel should still eventually force-close

Sign up for freeto 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.

FC due to Fulfilled HTLC $ID SENT_REMOVE_HTLC cltv $CLTV hit deadline without attempt to claim

4 participants

@vincenzopalazzo@Andezion@madelinevibes@ScuttoZ
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

lightningd: don't force-close when fulfilled HTLC removal is in progress - #8940

Closed
vincenzopalazzo wants to merge 4 commits into
ElementsProject:masterfrom
vincenzopalazzo:test/reproduce-issue-8899-fulfilled-htlc-deadline
Closed

lightningd: don't force-close when fulfilled HTLC removal is in progress#8940
vincenzopalazzo wants to merge 4 commits into
ElementsProject:masterfrom
vincenzopalazzo:test/reproduce-issue-8899-fulfilled-htlc-deadline

Conversation

@vincenzopalazzo

@vincenzopalazzovincenzopalazzo commented Mar 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes#8899: CLN force-closes a channel with Fulfilled HTLC SENT_REMOVE_HTLC cltv hit deadline even though the node has the preimage and the fulfill is already in progress.

The bug

In htlcs_notify_new_block(), when a fulfilled incoming HTLC hits the cltv deadline, CLN force-closes the channel without checking whether the HTLC removal is already in progress. The HTLC state machine shows the fulfill was queued to channeld (SENT_REMOVE_HTLC), but the force-close fires before the upstream peer receives update_fulfill_htlc.

This was reported in the wild: the upstream peer was connected and exchanging pings, but CLN force-closed instead of sending the fulfill message.

The fix

When the HTLC is in SENT_REMOVE_HTLC or later state (>= SENT_REMOVE_HTLC), skip the force-close and log a warning instead. This is safe because:

  1. The preimage is persisted to DB — onchaind can claim on-chain if needed
  2. On reconnect, channeld will resend update_fulfill_htlc
  3. If the peer goes on-chain themselves, onchaind handles it with the known preimage
  4. The cooperative path (reconnect + fulfill) is cheaper and preserves the channel

Commits

  1. pytest: reproduce issue FC due to Fulfilled HTLC $ID SENT_REMOVE_HTLC cltv $CLTV hit deadline without attempt to claim #8899 — Test that triggers the buggy force-close (l2 disconnects from l1 before sending update_fulfill_htlc, then mining blocks to the deadline)
  2. lightningd: fix — Skip force-close when hin->hstate >= SENT_REMOVE_HTLC, log log_unusual instead

Changelog-Fixed: Don't force-close channel when fulfilled HTLC hits deadline but removal is already in progress.

Test plan

  • test_fulfilled_htlc_deadline_no_force_close reproduces the bug (first commit)
  • With the fix, l2 logs "but removal already in progress" instead of force-closing
  • CI passes (Valgrind, ASan, integration tests)

🤖 Generated with Claude Code

@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from d6d8693 to e316362CompareMarch 13, 2026 21:04
@vincenzopalazzovincenzopalazzo changed the title pytest: reproduce issue #8899 fulfilled HTLC SENT_REMOVE_HTLC deadline force-closepytest: reproduce issue #8899 fulfilled HTLC deadline force-closeMar 14, 2026
…e force-close
Add test_fulfilled_htlc_deadline_no_force_close to reproduce the bug
where CLN force-closes a channel with "Fulfilled HTLC SENT_REMOVE_HTLC
cltv hit deadline" even though it has the preimage and just needs to
reconnect to send update_fulfill_htlc upstream.
The test sets up l1->l2->l3, sends a payment, and disconnects l2 from
l1 right before update_fulfill_htlc is sent (-WIRE_UPDATE_FULFILL_HTLC).
This leaves the incoming HTLC on the l1-l2 channel stuck in
SENT_REMOVE_HTLC (or SENT_REMOVE_COMMIT under Valgrind). Mining blocks
to the deadline triggers the buggy force-close.
Reproduces: ElementsProject/lightning#8899
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 3ad64fb to cbcc032CompareMarch 15, 2026 00:25
@vincenzopalazzovincenzopalazzo changed the title pytest: reproduce issue #8899 fulfilled HTLC deadline force-closelightningd: don't force-close when fulfilled HTLC removal is in progressMar 15, 2026
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 4cf3776 to 066b3c7CompareMarch 15, 2026 00:35
When an incoming HTLC has been fulfilled (preimage known) and is in
SENT_REMOVE_HTLC or later state, the removal is already in progress:
channeld has been told to send update_fulfill_htlc upstream (or will
be told on reconnect). Force-closing the channel is counterproductive
because:
1. The preimage is persisted to DB and onchaind can claim on-chain
2. The cooperative path (reconnect + fulfill) is cheaper and faster
3. If the peer goes on-chain themselves, onchaind handles it
Instead of force-closing, log a warning and let the removal complete
through the normal state machine.
CI run of the reproducer test (cbcc032) confirms the bug exists.
Before this fix, the force-close fires:
lightningd: Peer permanent failure in CHANNELD_NORMAL:
Fulfilled HTLC 0 SENT_REMOVE_HTLC cltv 119 hit deadline
After this fix, the force-close is skipped and a warning is logged:
lightningd: UNUSUAL: Fulfilled HTLC 0 SENT_REMOVE_HTLC cltv 119
hit deadline, but removal already in progress
Also updates test_htlc_no_force_close and test_htlc_in_timeout which
depended on the old force-close behavior: l3/l2 no longer force-close
for fulfilled HTLCs past deadline; instead the offering peer (l2/l1)
force-closes for the offered HTLC timeout, and the fulfilling node
claims on-chain via onchaind using the preimage.
Changelog-Fixed: Don't force-close channel when fulfilled HTLC hits deadline but removal is already in progress.
Fixes: ElementsProject/lightning#8899
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 5ec99ea to c95850fCompareMarch 15, 2026 10:21
vincenzopalazzoand others added 2 commits March 25, 2026 11:00
When a BOLT 12 invoice is created via createinvoice and is associated
with a local offer, include the offer_id in the invoice_creation event
notification. This allows plugins to determine which offer triggered
an invoice creation without having to call listinvoices for every
created invoice.
For bolt11 invoices and bolt12 invoices not associated with a local
offer, the field is omitted.
Fixes #8191
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ting test_fulfilled_htlc_deadline_no_force_close
* on-chain, onchaind will claim using the
* preimage.
* See https://github.com/ElementsProject/lightning/issues/8899 */
if (hin->hstate >= SENT_REMOVE_HTLC) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isnt the new guard hin->hstate >= SENT_REMOVE_HTLC is unconditionally true at the point it's checked, so channel_fail_permanent() for a fulfilled HTLC becomes dead code - not just for the #8899 race, but for every case?

I mean, from what i see on master:

  1. htlcs_notify_new_block() already requires hin->preimage != NULL to reach this code (lightningd/peer_htlcs.c at line 2998 - if (!hin->preimage) continue;)
  2. hin->preimage is set in exactly one place, fulfill_htlc() (lightningd/peer_htlcs.c at line 397), and the very next line unconditionally advances state - htlc_in_update_state(channel, hin, SENT_REMOVE_HTLC) (line 400), with the comment "We update state now to signal it's in progress, for persistence." No other call site assigns hin->preimage (the only other match, htlc_end.c at line 158, sets it to NULL).
    So hin->preimage != NULL implies hin->hstate >= SENT_REMOVE_HTLC always? The new if block will always be taken, and the original channel_fail_permanent("Fulfilled HTLC /// hit deadline") can never fire again

Comment threadtests/test_closing.py


@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors unsupported')
def test_fulfilled_htlc_deadline_no_force_close(node_factory, bitcoind):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And after the forst comment, test_fulfilled_htlc_deadline_no_force_close and test_fulfilled_htlc_deadline_reconnect don't actually test "removal in progress -> skip" vs. "removal stuck -> still force-close". Once comment is fixed with a real distinguishing condition, these tests should be extended to also cover the case where the peer never reconnects and the channel should still eventually force-close

Sign up for freeto 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.

FC due to Fulfilled HTLC $ID SENT_REMOVE_HTLC cltv $CLTV hit deadline without attempt to claim

4 participants

@vincenzopalazzo@Andezion@madelinevibes@ScuttoZ
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

lightningd: don't force-close when fulfilled HTLC removal is in progress - #8940

Closed
vincenzopalazzo wants to merge 4 commits into
ElementsProject:masterfrom
vincenzopalazzo:test/reproduce-issue-8899-fulfilled-htlc-deadline
Closed

lightningd: don't force-close when fulfilled HTLC removal is in progress#8940
vincenzopalazzo wants to merge 4 commits into
ElementsProject:masterfrom
vincenzopalazzo:test/reproduce-issue-8899-fulfilled-htlc-deadline

Conversation

@vincenzopalazzo

@vincenzopalazzovincenzopalazzo commented Mar 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes#8899: CLN force-closes a channel with Fulfilled HTLC SENT_REMOVE_HTLC cltv hit deadline even though the node has the preimage and the fulfill is already in progress.

The bug

In htlcs_notify_new_block(), when a fulfilled incoming HTLC hits the cltv deadline, CLN force-closes the channel without checking whether the HTLC removal is already in progress. The HTLC state machine shows the fulfill was queued to channeld (SENT_REMOVE_HTLC), but the force-close fires before the upstream peer receives update_fulfill_htlc.

This was reported in the wild: the upstream peer was connected and exchanging pings, but CLN force-closed instead of sending the fulfill message.

The fix

When the HTLC is in SENT_REMOVE_HTLC or later state (>= SENT_REMOVE_HTLC), skip the force-close and log a warning instead. This is safe because:

  1. The preimage is persisted to DB — onchaind can claim on-chain if needed
  2. On reconnect, channeld will resend update_fulfill_htlc
  3. If the peer goes on-chain themselves, onchaind handles it with the known preimage
  4. The cooperative path (reconnect + fulfill) is cheaper and preserves the channel

Commits

  1. pytest: reproduce issue FC due to Fulfilled HTLC $ID SENT_REMOVE_HTLC cltv $CLTV hit deadline without attempt to claim #8899 — Test that triggers the buggy force-close (l2 disconnects from l1 before sending update_fulfill_htlc, then mining blocks to the deadline)
  2. lightningd: fix — Skip force-close when hin->hstate >= SENT_REMOVE_HTLC, log log_unusual instead

Changelog-Fixed: Don't force-close channel when fulfilled HTLC hits deadline but removal is already in progress.

Test plan

  • test_fulfilled_htlc_deadline_no_force_close reproduces the bug (first commit)
  • With the fix, l2 logs "but removal already in progress" instead of force-closing
  • CI passes (Valgrind, ASan, integration tests)

🤖 Generated with Claude Code

@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from d6d8693 to e316362CompareMarch 13, 2026 21:04
@vincenzopalazzovincenzopalazzo changed the title pytest: reproduce issue #8899 fulfilled HTLC SENT_REMOVE_HTLC deadline force-closepytest: reproduce issue #8899 fulfilled HTLC deadline force-closeMar 14, 2026
…e force-close
Add test_fulfilled_htlc_deadline_no_force_close to reproduce the bug
where CLN force-closes a channel with "Fulfilled HTLC SENT_REMOVE_HTLC
cltv hit deadline" even though it has the preimage and just needs to
reconnect to send update_fulfill_htlc upstream.
The test sets up l1->l2->l3, sends a payment, and disconnects l2 from
l1 right before update_fulfill_htlc is sent (-WIRE_UPDATE_FULFILL_HTLC).
This leaves the incoming HTLC on the l1-l2 channel stuck in
SENT_REMOVE_HTLC (or SENT_REMOVE_COMMIT under Valgrind). Mining blocks
to the deadline triggers the buggy force-close.
Reproduces: ElementsProject/lightning#8899
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 3ad64fb to cbcc032CompareMarch 15, 2026 00:25
@vincenzopalazzovincenzopalazzo changed the title pytest: reproduce issue #8899 fulfilled HTLC deadline force-closelightningd: don't force-close when fulfilled HTLC removal is in progressMar 15, 2026
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 4cf3776 to 066b3c7CompareMarch 15, 2026 00:35
When an incoming HTLC has been fulfilled (preimage known) and is in
SENT_REMOVE_HTLC or later state, the removal is already in progress:
channeld has been told to send update_fulfill_htlc upstream (or will
be told on reconnect). Force-closing the channel is counterproductive
because:
1. The preimage is persisted to DB and onchaind can claim on-chain
2. The cooperative path (reconnect + fulfill) is cheaper and faster
3. If the peer goes on-chain themselves, onchaind handles it
Instead of force-closing, log a warning and let the removal complete
through the normal state machine.
CI run of the reproducer test (cbcc032) confirms the bug exists.
Before this fix, the force-close fires:
lightningd: Peer permanent failure in CHANNELD_NORMAL:
Fulfilled HTLC 0 SENT_REMOVE_HTLC cltv 119 hit deadline
After this fix, the force-close is skipped and a warning is logged:
lightningd: UNUSUAL: Fulfilled HTLC 0 SENT_REMOVE_HTLC cltv 119
hit deadline, but removal already in progress
Also updates test_htlc_no_force_close and test_htlc_in_timeout which
depended on the old force-close behavior: l3/l2 no longer force-close
for fulfilled HTLCs past deadline; instead the offering peer (l2/l1)
force-closes for the offered HTLC timeout, and the fulfilling node
claims on-chain via onchaind using the preimage.
Changelog-Fixed: Don't force-close channel when fulfilled HTLC hits deadline but removal is already in progress.
Fixes: ElementsProject/lightning#8899
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vincenzopalazzo
vincenzopalazzoforce-pushed the test/reproduce-issue-8899-fulfilled-htlc-deadline branch from 5ec99ea to c95850fCompareMarch 15, 2026 10:21
vincenzopalazzoand others added 2 commits March 25, 2026 11:00
When a BOLT 12 invoice is created via createinvoice and is associated
with a local offer, include the offer_id in the invoice_creation event
notification. This allows plugins to determine which offer triggered
an invoice creation without having to call listinvoices for every
created invoice.
For bolt11 invoices and bolt12 invoices not associated with a local
offer, the field is omitted.
Fixes #8191
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ting test_fulfilled_htlc_deadline_no_force_close
* on-chain, onchaind will claim using the
* preimage.
* See https://github.com/ElementsProject/lightning/issues/8899 */
if (hin->hstate >= SENT_REMOVE_HTLC) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isnt the new guard hin->hstate >= SENT_REMOVE_HTLC is unconditionally true at the point it's checked, so channel_fail_permanent() for a fulfilled HTLC becomes dead code - not just for the #8899 race, but for every case?

I mean, from what i see on master:

  1. htlcs_notify_new_block() already requires hin->preimage != NULL to reach this code (lightningd/peer_htlcs.c at line 2998 - if (!hin->preimage) continue;)
  2. hin->preimage is set in exactly one place, fulfill_htlc() (lightningd/peer_htlcs.c at line 397), and the very next line unconditionally advances state - htlc_in_update_state(channel, hin, SENT_REMOVE_HTLC) (line 400), with the comment "We update state now to signal it's in progress, for persistence." No other call site assigns hin->preimage (the only other match, htlc_end.c at line 158, sets it to NULL).
    So hin->preimage != NULL implies hin->hstate >= SENT_REMOVE_HTLC always? The new if block will always be taken, and the original channel_fail_permanent("Fulfilled HTLC /// hit deadline") can never fire again

Comment threadtests/test_closing.py


@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors unsupported')
def test_fulfilled_htlc_deadline_no_force_close(node_factory, bitcoind):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And after the forst comment, test_fulfilled_htlc_deadline_no_force_close and test_fulfilled_htlc_deadline_reconnect don't actually test "removal in progress -> skip" vs. "removal stuck -> still force-close". Once comment is fixed with a real distinguishing condition, these tests should be extended to also cover the case where the peer never reconnects and the channel should still eventually force-close

Sign up for freeto 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.

FC due to Fulfilled HTLC $ID SENT_REMOVE_HTLC cltv $CLTV hit deadline without attempt to claim

4 participants

@vincenzopalazzo@Andezion@madelinevibes@ScuttoZ