fuzz: add force-close support to chanmon_consistency#4381
fuzz: add force-close support to chanmon_consistency#4381joostjager wants to merge 13 commits intolightningdevkit:mainfrom
Conversation
|
👋 Hi! I see this is a draft PR. |
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 85.90% 85.90% -0.01%
==========================================
Files 156 156
Lines 103965 103965
Branches 103965 103965
==========================================
- Hits 89316 89311 -5
- Misses 12128 12133 +5
Partials 2521 2521
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:
|
fuzz/src/chanmon_consistency.rs
Outdated
| }, | ||
| 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?
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()); |
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.
2ac4223 to
24da03b
Compare
Add a section in "A fuzz test failed, what do I do?" explaining how to use the stdin_fuzz feature to reproduce crashes by piping bytes via stdin. This avoids the need to create test case files on disk, which is useful during git bisect or when working with AI agents that can construct and pipe byte sequences directly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add an invariant to the settlement phase: every payment that a receiver claimed (via claim_funds) must result in a PaymentSent event at the sender. This catches bugs where a claimed payment's preimage fails to propagate back to the sender. To support this, change resolved_payments from Vec<PaymentId> to HashMap<PaymentId, Option<PaymentHash>>, storing Some(hash) for PaymentSent and None for PaymentFailed/probes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Look up the splice tx by txid instead of assuming it is the first broadcasted tx. Also skip confirmation if the tx is already confirmed, which can happen when SplicePending fires more than once. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Prepare the chanmon_consistency fuzzer to tolerate force-close scenarios by handling the event and message types that force-closes produce. For HandleError, widen the accepted ErrorAction variants beyond DisconnectPeerWithWarning (timeout) to also accept DisconnectPeer, which is what funded channel force-closes generate. This change applies to all four locations: push_excess_b_events\!, process_msg_events\!, and both arms of drain_msg_events_on_disconnect\!. For events, add no-op handling for Event::ChannelClosed and Event::BumpTransaction in process_events\!. Also handle BroadcastChannelUpdate in push_excess_b_events\!, since force-closing a public channel generates this message type. No behavioral change: without force-close action bytes, none of these new paths will fire. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Force-closing a channel requires signing the holder commitment transaction. Add SignHolderCommitment to SUPPORTED_SIGNER_OPS and add action bytes 0xcc-0xce to enable this op per node, following the existing pattern for other signer operations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extend the sync_with_chain_state closure to also notify the TestChainMonitor of confirmed transactions and new blocks. This is necessary for force-close coverage where the chain monitor needs to see commitment transaction confirmations to process funding spends and trigger HTLC resolution. The monitor is synced before the channel manager at each block height so that monitor events are available when the manager processes the same block. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nels Replace the strict channel count assertions in test_return\! with upper bound checks, and drain broadcasters instead of asserting they are empty. Force-closing channels reduces the channel count and produces broadcast commitment transactions, so the old strict checks would fail. Add a closed_channels set to track which channels have been force-closed. The 0xff settlement check uses this to skip closed channels when verifying that each open channel can still send payments. The settlement loop now also drains broadcast transactions, confirms them on-chain, and syncs all nodes to the chain tip. This allows force-close related monitor events to be fully processed during settlement. With closed_channels initially empty, the relaxed checks are equivalent to the previous strict checks for all existing test scenarios. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add fuzz action bytes to exercise channel force-close scenarios: - 0xd0: Force-close first A-B channel from A's side - 0xd1: Force-close first B-C channel from B's side - 0xd2: Force-close first A-B channel from B's side - 0xd3: Force-close first B-C channel from C's side - 0xd8-0xda: Drain broadcaster and confirm all broadcast transactions for nodes A, B, and C respectively Each force-close action calls force_close_broadcasting_latest_txn and tracks the channel in closed_channels on success. The call may fail if the channel is already closed, which is handled gracefully. The broadcast confirmation actions pick up commitment transactions (or any other broadcast transactions) and add them to the chain state so that subsequent chain syncs make them visible to both the channel manager and chain monitor. The Event::ChannelClosed handler now records the channel_id in closed_channels. Event::DiscardFunding and Event::SpendableOutputs are also handled as no-ops since they may be produced after on-chain confirmation of force-close transactions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add ChainState::advance_height() to append empty blocks to the chain without any transactions. This enables the fuzzer to advance chain height past HTLC cltv_expiry timelocks, which is necessary for the OnchainTxHandler to release timelocked HTLC-timeout claim packages. Action bytes: - 0xdc: Advance chain by 50 empty blocks - 0xdd: Advance chain by 100 empty blocks - 0xde: Advance chain by 200 empty blocks These only extend the chain state. Nodes must still be synced to the new tip via existing 0xa8-0xad actions to observe the new height. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
After a force-close, the channel monitor signs HTLC-timeout and HTLC-success transactions for non-anchor channels. Without SignHolderHtlcTransaction in SUPPORTED_SIGNER_OPS, the signer would block these signatures when signer ops are disabled. Add a single enable action byte (0xcf) that re-enables this op on all three nodes at once, since per-node granularity is less important for post-close operations than for live channel signing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When channels have been force-closed, the 0xff settlement needs to advance chain height past HTLC cltv_expiry timelocks so that the OnchainTxHandler releases timelocked HTLC-timeout claim packages for broadcast. Without this, HTLC-timeout transactions would never be generated and in-flight HTLCs would remain unresolved. The settlement advances in two phases of 250 blocks each: 1. Past cltv_expiry: triggers HTLC-timeout tx broadcasts, which the existing drain-and-confirm loop picks up and confirms on-chain. 2. Past the CSV delay (BREAKDOWN_TIMEOUT=144): allows SpendableOutputs events to fire for both the to_local output and resolved HTLC outputs. Each phase syncs all nodes to the new chain tip and runs process_all_events\!() to drain all resulting messages, events, broadcasts, and monitor updates. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
After the 0xff settlement completes with force-closed channels, check get_claimable_balances() on all chain monitors. Assert that no ClaimableOnChannelClose balances remain, since those indicate the monitor still considers a channel open when it should be closed. This catches state machine bugs where the force-close state transition is not properly reflected in balance tracking. Other balance types (ClaimableAwaitingConfirmations, MaybeTimeoutClaimableHTLC, etc.) are logged but not asserted empty, since anchor channel HTLC resolution is not yet fully handled (the BumpTransaction events are currently dropped). The check passes open channels via the ignored_channels parameter so that only closed channel balances are examined. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add tests that exercise the new force-close fuzzer byte actions and verify they produce the expected behavior by asserting on specific log messages. Tests cover: basic force-close lifecycle, negative case without chain advancement, bidirectional force-close, middle node initiating, HTLC timeout delay, HTLC resolution after height advance, and a three-node scenario where A force-closes during fulfill propagation and learns the preimage on-chain from B's HTLC claim. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
24da03b to
1f6b57a
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.