fuzz: cover deferred writing in chanmon_consistency by joostjager · Pull Request #4465 · lightningdevkit/rust-lightning

joostjager · 2026-03-06T11:07:37Z

Adds fuzz coverage for #4351

ldk-reviews-bot · 2026-03-06T11:07:40Z

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

codecov · 2026-03-06T13:25:43Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.11%. Comparing base (75ac90a) to head (5eb96ab).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4465      +/-   ##
==========================================
- Coverage   86.12%   86.11%   -0.02%     
==========================================
  Files         157      157              
  Lines      108824   108871      +47     
  Branches   108824   108871      +47     
==========================================
+ Hits        93727    93755      +28     
- Misses      12480    12501      +21     
+ Partials     2617     2615       -2

Flag	Coverage Δ
tests	`86.11% <ø> (-0.02%)`	⬇️

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.

joostjager · 2026-03-19T07:52:16Z

Rebased

joostjager · 2026-03-19T11:01:31Z

Will hold off on this until pre-existing fuzz failures #4496 and #4472 are in

joostjager · 2026-03-20T17:53:19Z

Prerequisites are in, rebased.

TheBlueMatt · 2026-03-22T17:54:23Z

This LGTM, why is it draft?

joostjager · 2026-03-22T18:00:40Z

I first wanted to do a serious local run, but then it turned out there are so many pre-existing fuzz failures that it is hard to see what's new. I've bisected the failures to the various PRs that introduced them.

joostjager · 2026-03-22T20:01:05Z

Although for this PR I could just see if anything pops up that doesnt repro on main. Will do that.

joostjager · 2026-03-23T08:23:03Z

Several newly introduced fuzz failures to address:

7084a32219ffa61b1919192119197084a32219ff4400001d228012ffab
10fa0040801170704040198040111911191f1f1170bbff1170ffb3b2b6
10000150804211be707040198011191f1119401f401f0aa97210b6ff25
10000150804211707040198011191f11191f1a4040a91f7210b6ff25

joostjager · 2026-03-26T10:35:59Z

Zoomed in on one of those sequences, and it seems it is reproducible with another string without deferred mode too.

0270801109191109191f1f10b6ff

joostjager · 2026-04-15T08:40:24Z

Dependency: #4520

joostjager · 2026-05-07T08:19:52Z

Interestingly the non-deferred mode reproducer 0270801109191109191f1f10b6ff doesn't fail on main anymore, with #4520 not yet merged. Bisected the fixing PR to #4529 (@tankyleo).

It seems the increased headroom made the bug unobservable to the fuzzer?

I ran the fuzzer on this branch overnight, and no failures were found.

joostjager · 2026-05-07T08:59:29Z

I'll try to add a new invariant to the fuzzer so it catches the problem in a more robust way.

joostjager · 2026-05-07T11:29:12Z

Invariant addition: #4601

joostjager · 2026-05-07T13:23:31Z

Will rebase this after #4571

ldk-claude-review-bot · 2026-05-07T13:40:30Z

I've completed a thorough re-review of the entire PR diff, including:

Tracing the full lifecycle of LatestMonitorState (pending_monitors vs pending_monitor_completions)
Verifying the deferred flush mechanism (checkpoint_manager_persistence → flush → Persist callbacks)
Checking that assert_eq!(pending_operation_count(), 0) invariants hold at all call sites
Verifying the reload path with deferred + InProgress combinations
Confirming the settle loop (process_all_events) converges correctly with deferred mode
Checking make_channel setup with deferred mode
Reviewing all fuzz command handlers (0x00-0xff) for deferred-mode safety

No new issues found beyond what was already noted in the prior review pass.

Prior review status:

Line 367 (docstring accuracy): Still applies but is a documentation nit, not a bug.
Line 386 (drain_all for Completed): Still applies as a fragility note. The invariant that update_ret is constant during a node's lifetime ensures correctness.
Line 421 (mark_persisted bug claim): Confirmed false positive as noted in the prior summary. pending_monitors and pending_monitor_completions are independent lists, and out-of-order completion works correctly.
Line 1179 (update_ret ordering): Confirmed false positive as noted in the prior summary. The fresh persister is created with Completed at line 1108, the flush at line 1176 runs while update_ret is still Completed, and only line 1181 changes it.

No issues found.

Replace the chanmon consistency harness' Watch wrapper with a Persist implementation backed by HarnessPersister. Monitor writes now flow through the real ChainMonitor persistence hooks. Track restart candidates separately from monitor completion callbacks. A monitor can stop being a valid reload candidate once a newer baseline is durable, while its callback may still be needed to unblock the live ChainMonitor. On reload, choose the durable baseline, first pending snapshot, or last pending snapshot. Startup monitor registration completes immediately before the configured persistence style is restored.

Enable deferred ChainMonitor writes in chanmon_consistency. Checkpoint the ChannelManager before flushing captured monitor writes. Treat checkpoint-only progress as progress during settle_all.

joostjager · 2026-05-11T14:21:16Z

I reworked this PR and based it on a preparatory commit that makes the fuzzer's persistence model much more understandable, in my opinion. The prep work was taken from the force-close fuzzing PR #4381 that is still WIP, so it should be useful there as well.

joostjager · 2026-05-12T05:15:53Z

Fuzzed this overnight, no failures other than what is pre-existing in main.

joostjager force-pushed the chain-mon-internal-deferred-writes-with-fuzz branch from 51afc25 to 0aebb10 Compare March 19, 2026 07:51

joostjager force-pushed the chain-mon-internal-deferred-writes-with-fuzz branch from 0aebb10 to 6274ba0 Compare March 20, 2026 17:52

joostjager force-pushed the chain-mon-internal-deferred-writes-with-fuzz branch 2 times, most recently from c6da16e to d4bf3e0 Compare April 15, 2026 04:36

joostjager self-assigned this Apr 16, 2026

joostjager added this to Weekly Goals Apr 16, 2026

joostjager force-pushed the chain-mon-internal-deferred-writes-with-fuzz branch from d4bf3e0 to a798d74 Compare May 6, 2026 11:42

joostjager marked this pull request as ready for review May 7, 2026 13:23

ldk-reviews-bot requested a review from wpaulino May 7, 2026 13:23

joostjager removed the request for review from wpaulino May 7, 2026 13:23

ldk-claude-review-bot reviewed May 7, 2026

View reviewed changes

Comment thread fuzz/src/chanmon_consistency.rs Outdated

ldk-claude-review-bot reviewed May 7, 2026

View reviewed changes

Comment thread fuzz/src/chanmon_consistency.rs Outdated

joostjager force-pushed the chain-mon-internal-deferred-writes-with-fuzz branch from a798d74 to 20275f6 Compare May 8, 2026 14:30

ldk-claude-review-bot reviewed May 8, 2026

View reviewed changes

Comment thread fuzz/src/chanmon_consistency.rs Outdated

joostjager force-pushed the chain-mon-internal-deferred-writes-with-fuzz branch 2 times, most recently from af7e8ee to 53cd7c9 Compare May 11, 2026 12:58

ldk-claude-review-bot reviewed May 11, 2026

View reviewed changes

Comment thread fuzz/src/chanmon_consistency.rs Outdated

joostjager force-pushed the chain-mon-internal-deferred-writes-with-fuzz branch from 53cd7c9 to e72ec58 Compare May 11, 2026 13:50

joostjager added 2 commits May 11, 2026 16:08

fuzz: add deferred chanmon checkpoints

5eb96ab

Enable deferred ChainMonitor writes in chanmon_consistency. Checkpoint the ChannelManager before flushing captured monitor writes. Treat checkpoint-only progress as progress during settle_all.

joostjager force-pushed the chain-mon-internal-deferred-writes-with-fuzz branch from e72ec58 to 5eb96ab Compare May 11, 2026 14:19

Conversation

joostjager commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldk-reviews-bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

joostjager commented Mar 19, 2026

Uh oh!

joostjager commented Mar 19, 2026

Uh oh!

joostjager commented Mar 20, 2026

Uh oh!

TheBlueMatt commented Mar 22, 2026

Uh oh!

joostjager commented Mar 22, 2026

Uh oh!

joostjager commented Mar 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joostjager commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joostjager commented Mar 26, 2026

Uh oh!

joostjager commented Apr 15, 2026

Uh oh!

joostjager commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joostjager commented May 7, 2026

Uh oh!

joostjager commented May 7, 2026

Uh oh!

joostjager commented May 7, 2026

Uh oh!

Uh oh!

Uh oh!

ldk-claude-review-bot commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

joostjager commented May 11, 2026

Uh oh!

joostjager commented May 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

joostjager commented Mar 6, 2026 •

edited

Loading

ldk-reviews-bot commented Mar 6, 2026 •

edited

Loading

codecov Bot commented Mar 6, 2026 •

edited

Loading

joostjager commented Mar 22, 2026 •

edited

Loading

joostjager commented Mar 23, 2026 •

edited

Loading

joostjager commented May 7, 2026 •

edited

Loading

ldk-claude-review-bot commented May 7, 2026 •

edited

Loading