[storage/qmdb/current] support split (pyramid) bagging in qmdb-current#3693
Merged
Conversation
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
commonware-mcp | 3a36c86 | May 05 2026, 07:55 PM |
roberto-bayardo
force-pushed
the
pyramid-mmb
branch
from
df9c036 to
5fc0ccd
Compare
roberto-bayardo
force-pushed
the
pyramid-grafting
branch
from
0ba01d6 to
571016e
Compare
Deploying monorepo with
|
| Latest commit: |
3a36c86
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://959a9de5.monorepo-eu0.pages.dev |
| Branch Preview URL: | https://pyramid-grafting.monorepo-eu0.pages.dev |
roberto-bayardo
force-pushed
the
pyramid-mmb
branch
3 times, most recently
from
5bfd620 to
9e588ed
Compare
roberto-bayardo
force-pushed
the
pyramid-grafting
branch
from
571016e to
5aa7634
Compare
roberto-bayardo
force-pushed
the
pyramid-mmb
branch
2 times, most recently
from
debbd6e to
2236918
Compare
roberto-bayardo
force-pushed
the
pyramid-grafting
branch
from
5aa7634 to
4ae6103
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for split-root (inactive prefix) policies and pyramid/backward bagging in qmdb-current proof generation/verification, aligning current DB sync/roots with per-family RootSpec behavior.
Changes:
- Update current DBs and test harnesses to require
qmdb::RootSpecon Merkle families and propagateinactivity_floorinto grafted/root computations. - Rework current range proof generation/verification to support backward-bagged proofs by adding explicit suffix witnesses and a “range-collection-only” generic Merkle proof path.
- Refactor grafted-root folding to compute roots using
RootSpec(split roots + bagging), including chunk-aligned inactive-peak handling.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| storage/src/qmdb/sync/database.rs | Clarifies which root spec current uses when verifying sync ops proofs. |
| storage/src/qmdb/current/unordered/test_trait_impls.rs | Adds RootSpec bound for unordered fixed-partitioned test DB trait impls. |
| storage/src/qmdb/current/unordered/mod.rs | Updates tests for new RangeProof fields (suffix witnesses). |
| storage/src/qmdb/current/unordered/db.rs | Propagates RootSpec bounds into unordered current DB impls. |
| storage/src/qmdb/current/sync/tests.rs | Updates sync harness helpers to require RootSpec. |
| storage/src/qmdb/current/sync/mod.rs | Threads RootSpec bounds + inactivity floor into sync rebuild/root computation. |
| storage/src/qmdb/current/proof.rs | Major update: range proof now supports split roots + backward bagging with prefix/suffix witnesses and range-only proof collection. |
| storage/src/qmdb/current/ordered/test_trait_impls.rs | Adds RootSpec bound for ordered test trait impls. |
| storage/src/qmdb/current/ordered/mod.rs | Updates tests for new RangeProof fields (suffix witnesses). |
| storage/src/qmdb/current/mod.rs | Updates initialization/tests to require RootSpec and to pass inactivity floor into root computation. |
| storage/src/qmdb/current/grafting.rs | Implements grafted root computation using RootSpec + transformed inactive-peak mapping; adds chunk-aligned inactive-peak helper. |
| storage/src/qmdb/current/db.rs | Propagates RootSpec bound and passes inactivity floor through grafted/canonical root computation + proof creation. |
| storage/src/qmdb/current/batch.rs | Updates batch merkleization pipeline to compute canonical roots with inactivity floor and RootSpec bounds. |
| storage/src/merkle/proof.rs | Adds APIs for collecting only range siblings (range-collection proof + verifier path) to avoid unused/malleable generic witnesses. |
| storage/src/merkle/mod.rs | Re-exports new range-collection proof helpers under std. |
| storage/fuzz/fuzz_targets/current_unordered_operations.rs | Extends fuzzing to tamper with new prefix/suffix peak witness vectors. |
| storage/fuzz/fuzz_targets/current_ordered_operations.rs | Extends fuzzing to tamper with new prefix/suffix peak witness vectors. |
roberto-bayardo
force-pushed
the
pyramid-grafting
branch
4 times, most recently
from
ab3f7a1 to
bb50c8b
Compare
roberto-bayardo
added
the
breaking-format
Collaborator
Author
|
bugbot run |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for split (pyramid) bagging for qmdb::current databases by switching sync/proof/root handling to the QMDB per-family root spec (including non-zero inactive peak counts) and extending current proofs to carry the extra witness material needed for grafted-root reconstruction under backward bagging (MMB).
Changes:
- Switch
currentops trees + sync verification to QMDB per-family bagging (qmdb::Bagging) and propagate inactive peak counts end-to-end (sync target checks, proof verification, root computation). - Rework
current::RangeProofgeneration/verification to support backward-bagged proofs (adds suffix witnesses; introduces a “range-collection-only” generic proof path to avoid malleable unused bytes). - Update grafted-root computation to use chunk-aligned inactive peak boundaries and update fuzz/conformance artifacts accordingly.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| storage/src/qmdb/sync/database.rs | Clarifies current sync proofs are verified against the ops-tree spec (not canonical root). |
| storage/src/qmdb/current/unordered/test_trait_impls.rs | Tightens test trait bounds to require Bagging for families. |
| storage/src/qmdb/current/unordered/mod.rs | Updates RangeProof construction in tests for new fields (suffix witnesses). |
| storage/src/qmdb/current/unordered/db.rs | Requires Bagging for current unordered DB variants. |
| storage/src/qmdb/current/sync/tests.rs | Updates sync harness bounds to Graftable + Bagging. |
| storage/src/qmdb/current/sync/mod.rs | Uses QMDB bagging + inactive peaks in sync DB/Resolver implementations and local-target checks. |
| storage/src/qmdb/current/proof.rs | Major proof spec update: suffix witnesses, sparse proof path, inactivity-floor plumbing, and verifier changes. |
| storage/src/qmdb/current/ordered/test_trait_impls.rs | Tightens test trait bounds to require Bagging for families. |
| storage/src/qmdb/current/ordered/mod.rs | Updates RangeProof construction in tests for new fields (suffix witnesses). |
| storage/src/qmdb/current/mod.rs | Forces split-root + QMDB bagging for underlying ops tree; updates docs and init wiring. |
| storage/src/qmdb/current/grafting.rs | Adds chunk-aligned inactive peak logic and generalized grafted root computation under arbitrary bagging. |
| storage/src/qmdb/current/db.rs | Propagates inactivity floor and QMDB bagging into grafted-root and proof-generation APIs. |
| storage/src/qmdb/current/batch.rs | Propagates QMDB bagging/inactivity floor into batch merkleization + root computation paths. |
| storage/src/merkle/proof.rs | Adds “range-collection-only” proof building + reconstruction helpers to support current grafted proofs safely. |
| storage/src/merkle/mod.rs | Re-exports new range-collection proof helpers (std-only). |
| storage/fuzz/fuzz_targets/current_unordered_operations.rs | Extends fuzzing to tamper with new prefix/suffix witness vectors. |
| storage/fuzz/fuzz_targets/current_ordered_operations.rs | Extends fuzzing to tamper with new prefix/suffix witness vectors. |
| storage/conformance.toml | Updates conformance hashes for the new root/proof behavior. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
roberto-bayardo
force-pushed
the
pyramid-mmb
branch
3 times, most recently
from
fb14d2e to
c975533
Compare
roberto-bayardo
force-pushed
the
pyramid-grafting
branch
2 times, most recently
from
a82c6e4 to
2b7da92
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2b7da92. Configure here.
roberto-bayardo
force-pushed
the
pyramid-grafting
branch
2 times, most recently
from
05c8caf to
96635d9
Compare
roberto-bayardo
force-pushed
the
pyramid-grafting
branch
from
96635d9 to
6e2e066
Compare
danlaine
reviewed
May 5, 2026
roberto-bayardo
force-pushed
the
pyramid-grafting
branch
from
06f270b to
14d789e
Compare
roberto-bayardo
force-pushed
the
pyramid-grafting
branch
from
445e505 to
b13c7ba
Compare
danlaine
reviewed
May 5, 2026
| UnmerkleizedBatch { | ||
| inner: self.inner.new_batch(), | ||
| hasher: StandardHasher::new(), | ||
| hasher: StandardHasher::new(ForwardFold), |
Collaborator
There was a problem hiding this comment.
This isn't broken since the UnmerkleizedBatch's use of the hasher isn't affected by the bagging policy, but still seems awkward since we may actually use this with BackwardFold
Collaborator
Author
There was a problem hiding this comment.
If we don't pick a default, what's the alternative though?
Collaborator
There was a problem hiding this comment.
what do you think of this?
diff --git a/storage/src/journal/authenticated.rs b/storage/src/journal/authenticated.rs
index a395eda2b..2a36d5ea9 100644
--- a/storage/src/journal/authenticated.rs
+++ b/storage/src/journal/authenticated.rs
@@ -15,8 +15,7 @@ use crate::{
full::Merkle,
hasher::{Hasher as _, Standard as StandardHasher},
mem::Mem,
- Bagging::ForwardFold,
- Family, Location, Position, Proof, Readable,
+ Bagging, Family, Location, Position, Proof, Readable,
},
Context, Persistable,
};
@@ -109,6 +108,7 @@ impl<F: Family, H: Hasher, Item: Encode + Send + Sync, S: Strategy>
let ancestor_items = Self::collect_ancestor_items(&parent);
Arc::new(MerkleizedBatch {
inner: merkle,
+ bagging: hasher.bagging(),
items,
parent: parent.as_ref().map(Arc::downgrade),
ancestor_items,
@@ -153,6 +153,7 @@ impl<F: Family, H: Hasher, Item: Encode + Send + Sync, S: Strategy>
let ancestor_items = Self::collect_ancestor_items(&self.parent);
Arc::new(MerkleizedBatch {
inner: merkle,
+ bagging: self.hasher.bagging(),
items,
parent: self.parent.as_ref().map(Arc::downgrade),
ancestor_items,
@@ -165,6 +166,8 @@ impl<F: Family, H: Hasher, Item: Encode + Send + Sync, S: Strategy>
pub struct MerkleizedBatch<F: Family, D: Digest, Item: Send + Sync, S: Strategy = Sequential> {
/// The inner batch of Merkle leaf digests.
pub(crate) inner: Arc<batch::MerkleizedBatch<F, D, S>>,
+ /// The peak bagging policy inherited from the parent journal or batch.
+ bagging: Bagging,
/// The items to append from this batch.
items: Arc<Vec<Item>>,
/// This batch's parent, or None if the parent is the journal itself.
@@ -227,7 +230,7 @@ impl<F: Family, D: Digest, Item: Send + Sync, S: Strategy> MerkleizedBatch<F, D,
{
UnmerkleizedBatch {
inner: self.inner.new_batch(),
- hasher: StandardHasher::new(ForwardFold),
+ hasher: StandardHasher::new(self.bagging),
items: Vec::new(),
parent: Some(Arc::clone(self)),
}
@@ -319,7 +322,7 @@ where
let root = self.merkle.to_batch();
UnmerkleizedBatch {
inner: root.new_batch(),
- hasher: StandardHasher::new(ForwardFold),
+ hasher: StandardHasher::new(self.hasher.bagging()),
items: Vec::new(),
parent: None,
}
@@ -337,6 +340,7 @@ where
pub(crate) fn to_merkleized_batch(&self) -> Arc<MerkleizedBatch<F, H::Digest, C::Item, S>> {
Arc::new(MerkleizedBatch {
inner: self.merkle.to_batch(),
+ bagging: self.hasher.bagging(),
items: Arc::new(Vec::new()),
parent: None,
ancestor_items: Vec::new(),
@@ -850,6 +854,7 @@ mod tests {
use crate::{
journal::contiguous::fixed::{Config as JConfig, Journal as ContiguousJournal},
merkle::{
+ Bagging::ForwardFold,
full::{Config as MerkleConfig, Merkle},
mmb, mmr,
},
diff --git a/storage/src/merkle/hasher.rs b/storage/src/merkle/hasher.rs
index e2463399e..4940056c9 100644
--- a/storage/src/merkle/hasher.rs
+++ b/storage/src/merkle/hasher.rs
@@ -162,6 +162,11 @@ impl<H: CHasher> Standard<H> {
}
}
+ /// Return the bagging policy used when folding peaks into a root.
+ pub const fn bagging(&self) -> Bagging {
+ self.bagging
+ }
+
/// Hash an arbitrary sequence of byte slices into a single digest.
pub fn hash<'a>(&self, parts: impl IntoIterator<Item = &'a [u8]>) -> H::Digest {
let mut h = H::new();
danlaine
reviewed
May 5, 2026
| UnmerkleizedBatch { | ||
| inner: root.new_batch(), | ||
| hasher: StandardHasher::new(), | ||
| hasher: StandardHasher::new(ForwardFold), |
Collaborator
There was a problem hiding this comment.
danlaine
reviewed
May 5, 2026
| |op| op.has_floor(), | ||
| ) | ||
| .await?; | ||
| if inactivity_floor_loc > last_commit_loc { |
Collaborator
There was a problem hiding this comment.
This is already checked by find_inactivity_floor_at
danlaine
reviewed
May 5, 2026
|
|
||
| /// Regression: MMB current-db ops proofs verify against the full-forward ops root. | ||
| /// Regression: `ops_historical_proof` must verify with QMDB's ops-tree hasher configuration, | ||
| /// not the Merkle default. |
Collaborator
There was a problem hiding this comment.
there is no default anymore
danlaine
reviewed
May 5, 2026
Comment on lines
+171
to
+176
| Self::new(ForwardFold) | ||
| } | ||
|
|
||
| /// Creates a new [Standard] hasher with backward-fold bagging. | ||
| pub const fn backward() -> Self { | ||
| Self::with_bagging(Bagging::BackwardFold) | ||
| } | ||
|
|
||
| /// Creates a new [Standard] hasher with the given bagging policy. | ||
| pub const fn with_bagging(bagging: Bagging) -> Self { | ||
| Self { | ||
| _hasher: PhantomData, | ||
| bagging, | ||
| } | ||
| Self::new(BackwardFold) |
Collaborator
There was a problem hiding this comment.
Do we need forward() and backward() now that there is no default? Or should user just use new?
Collaborator
Author
There was a problem hiding this comment.
not needed, removed.
danlaine
approved these changes
May 5, 2026
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #3693 +/- ##
==========================================
- Coverage 95.83% 95.83% -0.01%
==========================================
Files 458 458
Lines 180036 180650 +614
Branches 4208 4218 +10
==========================================
+ Hits 172531 173117 +586
- Misses 6166 6187 +21
- Partials 1339 1346 +7
... and 6 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary