Split fuzz runners by hash mode#4565
Split fuzz runners by hash mode#4565joostjager wants to merge 2 commits intolightningdevkit:mainfrom
Conversation
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
66cea4d to
ba3fdf2
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4565 +/- ##
==========================================
- Coverage 87.00% 87.00% -0.01%
==========================================
Files 163 163
Lines 109002 109008 +6
Branches 109002 109008 +6
==========================================
+ Hits 94839 94841 +2
- Misses 11678 11680 +2
- Partials 2485 2487 +2
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:
|
|
Problem to solve with the coverage job, but this draft is sufficient for concept ack. |
4af35d0 to
3770052
Compare
|
Coverage job fixed. Now uploading real and fake hashes as separate coverage flags to avoid problems with merging coverage files. It might also be useful to allow separate viewing, and codecov makes it easy to view both too. |
Move shared fuzz logic into the root fuzz crate and generate fake-hashes and real-hashes runner crates. Keep `chanmon_consistency_target` on the real-hashes side, remove the fuzz-local Cargo config, and update scripts, CI, coverage, and docs to use explicit flags for each runner. Generate the hash-mode compile checks in the wrapper bins without a synthetic Cargo feature, while keeping the wrapper template close to its original shape. AI tools were used in preparing this commit.
Store real payment preimages in `chanmon_consistency` and use them when claiming funds, so the real-hashes runner does not treat `payment_hash` bytes as a stand-in preimage. AI tools were used in preparing this commit.
3770052 to
0f6c43f
Compare
| local rustflags="$3" | ||
|
|
||
| cargo llvm-cov clean --workspace | ||
| RUSTFLAGS="$rustflags" cargo llvm-cov --manifest-path "$manifest_path" --codecov \ |
There was a problem hiding this comment.
Nit: The old CI codecov path used cargo llvm-cov -j8 --codecov, but this helper function drops the -j8 parallelism limit. On a CI self-hosted runner with many CPUs this might not matter (or might even be faster), but it's a change from the old behavior that could increase peak memory usage. Consider adding a -j8 (or making it configurable) if the CI runner is memory-constrained.
| RUSTFLAGS="$rustflags" cargo llvm-cov --manifest-path "$manifest_path" --codecov \ | |
| cargo llvm-cov clean --workspace | |
| RUSTFLAGS="$rustflags" cargo llvm-cov -j8 --manifest-path "$manifest_path" --codecov \ |
Review SummaryThorough review of the entire PR diff (all shell scripts, Cargo.toml changes, CI workflows, generated templates, and auto-generated binary target files). Inline comments posted
Cross-cutting observations (no action needed)
No bugs, security issues, or logic errors found beyond the minor performance note above. |
Move the shared fuzz logic into the root fuzz crate and generate thin fake-hashes and real-hashes runner crates. Keep
chanmon_consistency_targeton the real-hashes side, remove the fuzz-local Cargo config, and update scripts, CI, coverage, and docs to invoke each runner with explicit flags.This is needed because force-close fuzzing with fake hashing led to several kinds of hash collisions. In particular,
chanmon_consistency_targetrelies on hash behavior closely enough that the fake-hashes mode can alias distinct objects and txes, which makes the fuzz target exercise collision artifacts instead of the real code paths we want to test. Splitting the runners lets that target use real hashes while keeping the rest of the fuzz setup unchanged.