fuzz: remove macros from chanmon_consistency#4571
fuzz: remove macros from chanmon_consistency#4571joostjager wants to merge 15 commits intolightningdevkit:mainfrom
Conversation
|
👋 I see @valentinewallace was un-assigned. |
joostjager
force-pushed
the
fuzz-no-macros
branch
from
dede551 to
54a3434
Compare
|
Pushed result of experimental automatic commit splitting using a stop hook. |
joostjager
force-pushed
the
fuzz-no-macros
branch
4 times, most recently
from
5209d09 to
6d3d4e6
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.
joostjager
force-pushed
the
fuzz-no-macros
branch
2 times, most recently
from
f370956 to
4e3cce7
Compare
|
Rebased, cleaned up auto-splitted commits, minimized diffs. Currently fuzz CI fails and seems to be a regression. |
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 the action helpers onto `HarnessNode` methods. Node-local operations now live with the state they mutate, which reduces argument threading through the fuzz loop.
Replace the four directional message vectors with one queue owner. The fuzz loop now uses that owner at send, receive, drain, and reload sites while preserving the existing routing behavior.
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
force-pushed
the
fuzz-no-macros
branch
from
4e3cce7 to
66dc37b
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4571 +/- ##
==========================================
- Coverage 87.10% 87.08% -0.03%
==========================================
Files 161 161
Lines 109246 109246
Branches 109246 109246
==========================================
- Hits 95162 95132 -30
- Misses 11600 11630 +30
Partials 2484 2484
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:
|
| ); | ||
| for (id, data) in state.pending_monitors.drain(..) { | ||
| self.monitor.chain_monitor.channel_monitor_updated(*chan_id, id).unwrap(); | ||
| if id > state.persisted_monitor_id { |
There was a problem hiding this comment.
Nit: This uses > (strict) while complete_all_pending_monitor_updates at line 1083 uses >=. When watch_channel returns InProgress, persisted_monitor_id is set equal to the pending monitor's id (and persisted_monitor is empty Vec::new()). If this method were ever called on a channel whose initial watch_channel InProgress entry hasn't been completed yet, > would skip updating persisted_monitor, leaving it as an empty vec.
In practice this path isn't reachable today because make_channel always calls complete_all_pending_monitor_updates (with >=) first. But using >= here (matching the other method) would be safer and less surprising for future maintenance.
Same applies to complete_monitor_update at line 1116.
There was a problem hiding this comment.
Leaving as is to keep this PR a strict refactor.
|
Good. Review SummaryPreviously reported issues (still unresolved):
No new issues found on this re-review pass. The rest of the refactoring faithfully reproduces the original behavior:
|
|
Verified coverage in codecov. It is equal to main. Still reaching the same code paths after the refactor. |
joostjager
force-pushed
the
fuzz-no-macros
branch
from
66dc37b to
6e05ce1
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`.
joostjager
force-pushed
the
fuzz-no-macros
branch
from
6e05ce1 to
6feccc1
Compare
3 participants
This PR removes the macro-heavy structure from
chanmon_consistency.rsand rewrites it as explicit Rust code.The main reason is compile time. The macros in this harness slow builds down enough that it becomes very noticeable during iteration. In follow-up force-close fuzzing work, the
chanmon_consistencybuild time increased to around 5 minutes on my machine. That is too expensive for a fuzz target that needs frequent rebuilds.The macros also make the file harder to read and reason about. Replacing them with normal types and methods should make the harness easier to maintain while also reducing the macro expansion cost.
Builds on #4565