-
Notifications
You must be signed in to change notification settings - Fork 173
[storage/bmt] Add multi-proof support #2733
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 with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
commonware-mcp | 9d9c468 | Jan 08 2026, 07:29 PM |
Deploying monorepo with
|
| Latest commit: |
9d9c468
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ff003c9e.monorepo-eu0.pages.dev |
| Branch Preview URL: | https://cl-bmt-multiproofs.monorepo-eu0.pages.dev |
d515f7b to
f443733
Compare
MultiProoff938b23 to
26490c6
Compare
9ff2afd to
c4a1377
Compare
| impl<H: Hasher> Read for Proof<H> { | ||
| /// The maximum number of items being proven. | ||
| /// | ||
| /// The upper bound on sibling hashes is derived as `max_items * MAX_LEVELS`. |
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.
I think we're using NonZeroRange now? forgot
c4a1377 to
46eccd1
Compare
46eccd1 to
7abbb18
Compare
| /// The proof of the shard in the [bmt] at the given index. | ||
| proof: bmt::Proof<H>, | ||
| /// The multi-proof of the shard in the [bmt] at the given index. | ||
| proof: bmt::Proof<H::Digest>, |
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.
Chunk could also be parameterized on D: Digest and verify updated to take in a &mut Hasher like the proof verify methods. If we decide to do that, I think it's fine to defer to a future PR to keep this diff smaller.
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.
Let's do that in a follow-up - #2753
|
|
||
| /// Returns the number of levels in a tree with `leaf_count` leaves. | ||
| /// A tree with 1 leaf has 1 level, a tree with 2 leaves has 2 levels, etc. | ||
| const fn levels_in_tree(leaf_count: u32) -> usize { |
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.
It seems we always call this with leaf_count > 0 -- should we make the 0 case unreachable! instead of returning 1? Not a big deal either way but I was a bit surprised that levels_in_tree(0) == 1.
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.
Rationale for levels_in_tree(0) == 1 is that a tree with 0 leaves does still have a root. I think we should probably keep all leaf_counts valid inputs for that reason
8ef1074 to
992e7d2
Compare
992e7d2 to
9d9c468
Compare
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #2733 +/- ##
==========================================
+ Coverage 93.20% 93.22% +0.02%
==========================================
Files 372 372
Lines 113359 114143 +784
==========================================
+ Hits 105655 106413 +758
- Misses 7704 7730 +26
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Overview
Adds multi-proof support to
storage::bmt.This PR also breaks the API of the MMR
Proof'sReadimpl - theRead::Cfgis now the max number of items, rather than the max number of digests in the proof. The upper bound is inferred.closes #2221
closes #2506
zoda::encodemmr -> bmt, large change in perf.
reed_solomon::encodeStill using
bmt, now forced to use the multi-proof path. No significant change in perf.