Uh oh!
There was an error while loading. Please reload this page.
Split fuzz runners by hash mode - #4565
Conversation
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
66cea4d to
ba3fdf2CompareCodecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## main #4565 +/- ##
=======================================
Coverage 86.99% 87.00% =======================================
Files 163 163 Lines 109008 109008 Branches 109008 109008 =======================================
+ Hits 94830 94837 +7 + Misses 11692 11684 -8 - Partials 2486 2487 +1
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:
|
joostjager
commented
Apr 14, 2026
Problem to solve with the coverage job, but this draft is sufficient for concept ack. |
4af35d0 to
3770052Comparejoostjager
commented
Apr 15, 2026
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. |
3770052 to
0f6c43fCompare| 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 \ |
I've completed a thorough review of the entire PR diff, examining all changed files including:
Review SummaryPrior flagged issues — status update:
New issues found: None. The PR is well-structured. RUSTFLAGS are consistently correct for each crate, relative paths resolve correctly from sub-crate directories, workspace profile inheritance works, corpus/test_cases sharing via |
0f6c43f to
9ee7692CompareUh oh!
There was an error while loading. Please reload this page.
9ee7692 to
4b2838fCompareMove 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.
4b2838f to
c26f3c7Compareldk-reviews-bot
commented
Apr 18, 2026
🔔 1st Reminder Hey @TheBlueMatt! This PR has been waiting for your review. |
ldk-reviews-bot
commented
Apr 20, 2026
🔔 2nd Reminder Hey @TheBlueMatt! This PR has been waiting for your review. |
| let payment_hash = PaymentHash(Sha256::hash(&[*payment_ctr as u8]).to_byte_array()); | ||
| let mut payment_preimage = PaymentPreimage([0; 32]); | ||
| payment_preimage.0[0..8].copy_from_slice(&payment_ctr.to_be_bytes()); | ||
| let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).to_byte_array()); |
There was a problem hiding this comment.
| let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).to_byte_array()); | |
| let payment_hash:PaymentHash = payment_preimage.into(); |
Uh oh!
There was an error while loading. Please reload this page.
PR lightningdevkit#4565 moved chanmon_consistency_target to the real-hashes runner, so txids no longer collide across unrelated outpoints. The 6-block window added to dodge those collisions is no longer needed; scanning the full block history is correct and matches the behavior before the fake-hashes workaround. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.