fuzz: add force-close support to chanmon_consistency#4381
fuzz: add force-close support to chanmon_consistency#4381joostjager wants to merge 29 commits intolightningdevkit:mainfrom
Conversation
|
👋 I see @wpaulino was un-assigned. |
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.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
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:
|
| }, | ||
| events::Event::SplicePending { .. } => {}, | ||
| events::Event::SpliceFailed { .. } => {}, | ||
| events::Event::ChannelClosed { .. } => {}, |
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()); |
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.
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.
24da03b to
1f6b57a
Compare
|
Needs rebase. Is this stalled waiting on fixes that were discovered by the fuzzer? |
1f6b57a to
633c959
Compare
|
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. |
a1b1367 to
ba6cbfa
Compare
f3cbd42 to
88ee2b7
Compare
|
Rebased onto #4571, because compile times became unworkable with the many macros. |
88ee2b7 to
67bcdec
Compare
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.
135b9e5 to
f39de3e
Compare
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.
f39de3e to
be4dd9c
Compare
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.