Skip to content

fuzz: add force-close support to chanmon_consistency#4381

Draft
joostjager wants to merge 29 commits intolightningdevkit:mainfrom
joostjager:fuzz-force-close
Draft

fuzz: add force-close support to chanmon_consistency#4381
joostjager wants to merge 29 commits intolightningdevkit:mainfrom
joostjager:fuzz-force-close

Conversation

@joostjager
Copy link
Copy Markdown
Contributor

@joostjager joostjager commented Feb 4, 2026

Add force-close coverage to the chanmon_consistency fuzzer. 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 consistency
constraints.

@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented Feb 4, 2026

👋 I see @wpaulino was un-assigned.
If you'd like another reviewer assignment, please click here.

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.18%. Comparing base (df49980) to head (be4dd9c).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4381      +/-   ##
==========================================
+ Coverage   87.10%   87.18%   +0.07%     
==========================================
  Files         161      161              
  Lines      109246   109278      +32     
  Branches   109246   109278      +32     
==========================================
+ Hits        95162    95274     +112     
+ Misses      11600    11528      -72     
+ Partials     2484     2476       -8     
Flag Coverage Δ
fuzzing-fake-hashes 31.15% <ø> (+0.03%) ⬆️
fuzzing-real-hashes 29.16% <ø> (+6.35%) ⬆️
tests 86.21% <ø> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread fuzz/src/chanmon_consistency.rs Outdated
},
events::Event::SplicePending { .. } => {},
events::Event::SpliceFailed { .. } => {},
events::Event::ChannelClosed { .. } => {},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should probably open a new channel to replace the force closed one?

Comment thread fuzz/src/chanmon_consistency.rs Outdated

// Only check for no broadcasts if no force-closes happened.
if !fc_ab && !fc_bc {
assert!(broadcast.txn_broadcasted.borrow().is_empty());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

@TheBlueMatt
Copy link
Copy Markdown
Collaborator

Needs rebase. Is this stalled waiting on fixes that were discovered by the fuzzer?

@joostjager
Copy link
Copy Markdown
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.

@joostjager joostjager force-pushed the fuzz-force-close branch 2 times, most recently from a1b1367 to ba6cbfa Compare April 16, 2026 13:33
@joostjager joostjager force-pushed the fuzz-force-close branch 2 times, most recently from f3cbd42 to 88ee2b7 Compare April 21, 2026 08:23
@joostjager
Copy link
Copy Markdown
Contributor Author

Rebased onto #4571, because compile times became unworkable with the many macros.

Extract the repeated peer-connection and channel-funding setup into
small helpers. This leaves the fuzz scenario setup behavior unchanged
while making later harness refactors easier to review.
Introduce a small wrapper around each channel manager and its test
resources. This keeps node-local state together before moving more
operations onto the harness.
Move construction of loggers, keys, monitors, broadcasters, wallets,
and fee estimators into node resource setup. This removes ad hoc local
closures while preserving the deterministic test inputs used by the
fuzzer.
Centralize creation of the three chanmon harness nodes. The fuzzer now
initializes the node array through one path, which reduces duplicated
setup before the event and payment helpers are split out.
Move persistence, reload, and chain sync state onto each harness node.
Keeping serialized managers and heights with the node makes restarts and
block updates easier to reason about.
Lift monitor update, splice, and chain sync actions into named helper
functions. This keeps the byte-dispatch loop focused on choosing actions
rather than spelling out each operation.
Move per-node queue draining, middle-node routing, and disconnect
cleanup into EventQueues. The fuzz loop now asks the queue owner to
route remaining messages instead of mutating each directional vector
directly.
Pull message-event delivery into standalone helpers. This keeps the fuzz
dispatch loop smaller while preserving the same corruption and
one-message processing modes.
Represent each channel pair as a peer link with its channel ids and
disconnect state. Link methods now own peer reconnect, disconnect, and
monitor-update operations for that channel group.
Move payment bookkeeping into a payment tracker. Sending, resolving,
claiming, and stuck-payment assertions now share one state owner instead
of borrowing several local maps.
Collect the node, link, queue, chain, and payment setup into a harness
builder. This keeps the initial fuzz scenario construction together and
leaves the action loop with a smaller state surface.
Wrap the chanmon consistency state in a `Harness` struct. The fuzz loop
now accesses nodes, links, queues, payments, and chain state through one
owner while keeping the existing byte actions intact.
@joostjager joostjager force-pushed the fuzz-force-close branch 8 times, most recently from 135b9e5 to f39de3e Compare April 29, 2026 10:33
Move the main fuzz flow onto the harness. This completes the structural
refactor so `do_test` is responsible for constructing and running the
harness rather than managing the full scenario directly.

Keep the final event-settling loop on the harness as well,
avoiding an extra free function once the state it needs is already
owned by `Harness`.
Collect local notes and fuzz inputs used while investigating the
force-close chanmon_consistency work.

These files are marked dropme so they can aid review and reduction
without becoming part of the final patch set.
Remember claim outpoints after their spending transactions have reached
the anti-reorg delay, even after normal claim tracking is cleaned up.

This prevents later claims for newly learned preimages from attempting
to spend outputs that were already irrevocably claimed on chain.
Allow fuzz builds to use cheaper ECDSA signatures and skip assertions
that rely on production signature sizes.

The fuzz signer intentionally trades realistic DER lengths for speed, so
weight lower-bound checks must not assume low-R signatures there.
Replace the test persister with harness-owned monitor persistence that
tracks completed and in-progress updates.

This lets restarts select persisted or pending monitor snapshots and
exercise reload behavior across asynchronous monitor update states.
Track confirmed UTXOs and reject transactions that spend missing,
already spent, or still-timelocked outputs.

The harness now models enough chain state to confirm funding, splice,
and claim transactions without accepting impossible spends.
Teach the harness to deliver additional control and announcement
messages emitted during reconnects and timer-driven state changes.

This keeps delayed message handling from panicking on valid events that
can be produced by the channel manager.
Stop treating every channel close or broadcast transaction as an
immediate invariant failure.

Later commits add explicit force-close coverage, so the baseline harness
must allow channels and broadcaster queues to reflect closure progress.
Return the transactions confirmed from the pending pool and apply their
effects to the harness wallets.

This keeps wallet UTXO state aligned with the fake chain when splice or
other pending transactions are mined by fuzz input.
Map the primary signer-unblock opcodes so nodes 0, 1, and 2 each get
distinct controls for counterparty commitment signing, per-commitment
points, and commitment secret release.

Make node B's primary signer-unblock controls retry all pending
channels once an operation is available, while keeping the older
channel-specific release-secret controls for now.

This avoids duplicated node coverage and makes the byte controls easier
to reason about in reduced test cases.
Make event processing robust to splice, close, spendable-output, and
bump-transaction events that can arise during on-chain cleanup.

Splice pending handling now finds the matching broadcast transaction by
txid instead of assuming queue order.
Add cleanup helpers and fuzz opcodes for monitor bump events,
broadcast confirmation, careful chain advancement, and node resyncs.

The all-events loop now advances messages, node events, monitor events,
pending transactions, and broadcasts until the harness quiesces.
Before final assertions, catch raw monitors up to node height and drive
timer ticks plus block advancement until pending work clears.

The final liveness probe now uses each channel's advertised sendable
range instead of a fixed amount that may be outside its limits.
Track payment hashes, paths, claims, sender outcomes, and closed
channels so settle-all can distinguish unresolved work from valid
force-close outcomes.

This lets the harness accept sender failure for claimed dust paths while
still asserting that observable payment lifecycles complete.
Add explicit force-close fuzz actions for the A-B and B-C channels.

Enable holder commitment and holder HTLC signing together so on-chain
cleanup retries do not split the paired monitor-side signer operations.
The all-node holder-signing byte remains as a compatibility alias for
existing fuzz inputs.

The harness records dust HTLC paths before closing so later payment
resolution checks can account for claims blocked by dust outputs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants