-
Notifications
You must be signed in to change notification settings - Fork 170
[storage/fuzz] fix mmr_journaled fuzz test #2746
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Deploying monorepo with
|
| Latest commit: |
650ceb4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ca8863de.monorepo-eu0.pages.dev |
| Branch Preview URL: | https://fuzz-work.monorepo-eu0.pages.dev |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
commonware-mcp | 650ceb4 | Jan 08 2026, 03:16 PM |
| // Truncate tracking variables to match recovered state | ||
| let recovered_leaves = new_mmr.leaves().as_u64() as usize; | ||
| leaves.truncate(recovered_leaves); | ||
| historical_sizes.truncate(recovered_leaves); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong tracking data after pop-add-reinit sequence
Low Severity
The truncation logic assumes leaves[0..recovered_leaves] matches the recovered MMR's leaves. This breaks after a Pop-Add sequence without sync. For example: sync with [A,B], pop one leaf giving [A], add C giving [A,C], then Reinit recovers 2 leaves. leaves.truncate(2) keeps [A,C] but the recovered MMR has [A,B]. Subsequent Proof operations use wrong element data (C instead of B), causing spurious verification failures that misleadingly suggest bugs in the MMR implementation.
🔬 Verification Test
Why verification test was not possible: This is a fuzz test infrastructure issue in a Rust project that requires the full commonware-storage crate and its dependencies to compile and run. The bug manifests as spurious assertion failures during fuzz testing when a specific operation sequence (sync → pop → add → reinit) occurs, which cannot be easily unit tested in isolation without the full fuzzing framework.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pop does a sync, so this should be fine.
|
The crash is not reproduced anymore |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #2746 +/- ##
=======================================
Coverage 93.20% 93.20%
=======================================
Files 372 372
Lines 113359 113359
=======================================
Hits 105655 105655
Misses 7704 7704 Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
No description provided.