fuzz: add force-close support to chanmon_consistency#4381
Draft
joostjager wants to merge 9 commits intolightningdevkit:mainfrom
Draft
fuzz: add force-close support to chanmon_consistency#4381joostjager wants to merge 9 commits intolightningdevkit:mainfrom
joostjager wants to merge 9 commits intolightningdevkit:mainfrom
Conversation
|
👋 I see @wpaulino was un-assigned. |
joostjager
force-pushed
the
fuzz-force-close
branch
from
081de37 to
97e65bc
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4381 +/- ##
==========================================
+ Coverage 87.00% 87.03% +0.03%
==========================================
Files 163 163
Lines 109002 109010 +8
Branches 109002 109010 +8
==========================================
+ Hits 94839 94882 +43
+ Misses 11678 11649 -29
+ Partials 2485 2479 -6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
wpaulino
reviewed
Feb 10, 2026
| }, | ||
| events::Event::SplicePending { .. } => {}, | ||
| events::Event::SpliceFailed { .. } => {}, | ||
| events::Event::ChannelClosed { .. } => {}, |
Contributor
There was a problem hiding this comment.
We should probably open a new channel to replace the force closed one?
|
|
||
| // Only check for no broadcasts if no force-closes happened. | ||
| if !fc_ab && !fc_bc { | ||
| assert!(broadcast.txn_broadcasted.borrow().is_empty()); |
Contributor
There was a problem hiding this comment.
I have some changes that will be going up soon that rework this, you may want to wait until then. Each node will have its own broadcaster, and there's also a concept of a "chain" now so we can mine transactions.
Contributor
Author
There was a problem hiding this comment.
Changes were very useful! The per-node broadcasters (broadcast_a/broadcast_b/broadcast_c) are used to selectively drain and confirm each node's force-close commitment txs, and the ChainState abstraction is used to confirm broadcast transactions and advance block height past HTLC timelocks during settlement.
joostjager
force-pushed
the
fuzz-force-close
branch
5 times, most recently
from
24da03b to
1f6b57a
Compare
Collaborator
|
Needs rebase. Is this stalled waiting on fixes that were discovered by the fuzzer? |
joostjager
force-pushed
the
fuzz-force-close
branch
from
1f6b57a to
633c959
Compare
Contributor
Author
|
I was working on my local branch only for a while. Just pushed what I have. But indeed, the mixed mode failure is also showing up in different ways with fc fuzzing. |
Move shared fuzz logic into the root fuzz crate and generate fake-hashes and real-hashes runner crates. Keep `chanmon_consistency_target` on the real-hashes side, remove the fuzz-local Cargo config, and update scripts, CI, coverage, and docs to use explicit flags for each runner. Generate the hash-mode compile checks in the wrapper bins without a synthetic Cargo feature, while keeping the wrapper template close to its original shape. AI tools were used in preparing this commit.
Store real payment preimages in `chanmon_consistency` and use them when claiming funds, so the real-hashes runner does not treat `payment_hash` bytes as a stand-in preimage. AI tools were used in preparing this commit.
When fuzzing, signatures are trivially small so the actual transaction weight can be significantly less than estimated. Skip the lower-bound checks that verify the estimate is within 1-2% of reality, while keeping the upper-bound assertions that ensure we never underestimate. AI tools were used in preparing this commit.
Add opcodes for force-closing channels (0xd0-0xd3), draining and confirming broadcast transactions (0xd8-0xda), and advancing chain height past HTLC timelocks (0xdc-0xde). Add SignHolderCommitment and SignHolderHtlcTransaction signer ops (0xcc-0xcf) needed to sign commitment and HTLC-timeout transactions during force-close. Supporting changes: - UTXO tracking in ChainState to reject double-spends and phantom output spends caused by fuzz txid hash collisions - Enforce lock_time consensus rules in confirm_tx so timelocked txs (HTLC-timeout) cannot confirm before their timelock expires - Sort broadcast txs by lock_time in the settlement drain loop so preimage claims (lock_time 0) confirm before timeout txs when both compete for the same HTLC output - Sync chain monitors alongside channel managers so monitors see commitment transaction confirmations - Relax exit invariants to allow force-closed channels - Handle force-close-related events (ChannelClosed, DiscardFunding, SpendableOutputs, BumpTransaction) and messages (SendErrorMessage, DisconnectPeer, BroadcastChannelUpdate) - Skip bogus channel_reestablish messages (both commitment numbers at 0) that cause infinite ping-pong between LDK nodes - Filter out channels without SCID in MPP sends to avoid panics on force-closed channels - Use correct persist mode during node reload and mirror pending monitor updates so async monitors don't get stuck - Handle BumpTransaction events for anchor channels in settlement, broadcasting commitment txs directly and using the bump handler for HTLC resolution - Resolve timelocked HTLCs during settlement by advancing chain height in 4 iterations past CLTV expiry and CSV delays - Verify no ClaimableOnChannelClose balances remain after settlement - Handle claimed payments that lose the timeout race - Increase wallet UTXOs from 1 to 50 per node for anchor claims - Run fuzzer twice per input (non-anchor and anchor channels) - Remove trusted open/accept channel variants - Sync stale monitors to chain tip during settlement without lowering node height Includes seed test cases covering basic force-close lifecycle, bidirectional close, HTLC timeout/resolution, three-node preimage on-chain scenarios, and async monitor variants of each. One test case (fc_after_claim_before_forward) currently fails due to an LDK bug where a forwarding node's monitor fails to claim an HTLC with a stored preimage when the counterparty commitment tx is confirmed after the preimage was stored. AI tools were used in preparing this commit.
AI tools were used in preparing this commit.
joostjager
force-pushed
the
fuzz-force-close
branch
from
633c959 to
a5443d8
Compare
Count completed monitor updates as progress. Align persisted monitor snapshots on equal update IDs. Give chain-advance drains more passes. Run rustfmt on the fuzz workspace.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add force-close coverage to the
chanmon_consistencyfuzzer. Previously, the fuzzer only exercised cooperative channel flows. This PR enables the fuzzer to force-close channels and verify that on-chain resolution, HTLC timeouts, and payment preimage propagation all work correctly under channel monitor consistencyconstraints.