[storage/qmdb] Support generalized Merkle family in qmdb state sync#3626
Merged
Conversation
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
commonware-mcp | de3324e | Apr 21 2026, 05:07 PM |
roberto-bayardo
force-pushed
the
mmb-sync-approach-2
branch
from
6be33a5 to
5c7ea5c
Compare
Deploying monorepo with
|
| Latest commit: |
de3324e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://dfaf6c07.monorepo-eu0.pages.dev |
| Branch Preview URL: | https://mmb-sync-approach-2.monorepo-eu0.pages.dev |
roberto-bayardo
force-pushed
the
mmb-sync-approach-2
branch
10 times, most recently
from
43137f8 to
90387dc
Compare
roberto-bayardo
added
the
breaking-format
Contributor
There was a problem hiding this comment.
Pull request overview
Generalizes QMDB state sync to work across the Merkle “family” abstraction (e.g., MMR and MMB), propagating the family type through sync targets, resolvers, engine types, and database/journal interfaces, and expanding test coverage to exercise multiple families.
Changes:
- Make sync core types (
Target,Resolver,Engine, errors, request tracking) generic overmerkle::Family. - Update database/journal sync interfaces to use
NonEmptyRange<Location<F>>and addDatabase::Family, plus aDbResolverhelper trait. - Add/expand sync test harnesses to run suites against both MMR and MMB, and update examples/fuzz targets to the new APIs (including
sync_boundary).
Reviewed changes
Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| storage/src/qmdb/sync/target.rs | Make Target generic over Merkle family and update codec/tests accordingly. |
| storage/src/qmdb/sync/resolver.rs | Add Resolver::Family and thread family type through fetch/proof types and impl macros. |
| storage/src/qmdb/sync/requests.rs | Parameterize request tracking and future types by Merkle family. |
| storage/src/qmdb/sync/mod.rs | Introduce DbResolver and update sync() to return family-parameterized errors. |
| storage/src/qmdb/sync/journal.rs | Make Journal generic over family and switch to NonEmptyRange<Location<F>>. |
| storage/src/qmdb/sync/gaps.rs | Generalize gap detection over Location<F>. |
| storage/src/qmdb/sync/error.rs | Parameterize sync errors and engine errors by Merkle family. |
| storage/src/qmdb/sync/engine.rs | Thread family through engine state, events, requests, retained roots, and config channels. |
| storage/src/qmdb/sync/database.rs | Add Database::Family and update sync rebuild/target checks to use family + NonEmptyRange. |
| storage/src/qmdb/keyless/sync/mod.rs | Move keyless sync impl into module and generalize over family. |
| storage/src/qmdb/keyless/sync.rs | Remove old MMR-only keyless sync implementation (relocated/generalized). |
| storage/src/qmdb/keyless/mod.rs | Add sync_boundary() helper for keyless DBs. |
| storage/src/qmdb/keyless/fixed.rs | Add an end-to-end sync smoke test for fixed-size keyless DB. |
| storage/src/qmdb/immutable/sync/tests.rs | Add generic sync harness/tests for immutable DBs across MMR and MMB. |
| storage/src/qmdb/immutable/sync/mod.rs | Generalize immutable sync rebuild logic over Merkle family. |
| storage/src/qmdb/immutable/sync.rs | Remove old MMR-only immutable sync implementation (replaced by generalized module). |
| storage/src/qmdb/immutable/mod.rs | Add sync_boundary() helper for immutable DBs. |
| storage/src/qmdb/current/sync/tests.rs | Expand current sync tests to cover both MMR/MMB and add focused MMB pruning/auth regression tests. |
| storage/src/qmdb/current/sync/mod.rs | Generalize current sync rebuild logic over Merkle family and NonEmptyRange. |
| storage/src/qmdb/current/ordered/fixed.rs | Update pruning in tests to use sync_boundary(). |
| storage/src/qmdb/current/mod.rs | Replace inactivity-floor pruning usage with sync_boundary() and add boundary-focused tests. |
| storage/src/qmdb/current/grafting.rs | Update docs to reference sync_boundary as the pruning guard. |
| storage/src/qmdb/current/db.rs | Introduce sync_boundary() computation and enforce it as the prune upper bound. |
| storage/src/qmdb/any/unordered/variable.rs | Rename sync test trait assoc type from Mmr to Merkle. |
| storage/src/qmdb/any/unordered/fixed.rs | Rename sync test trait assoc type from Mmr to Merkle. |
| storage/src/qmdb/any/traits.rs | Add DbAny::sync_boundary() and implement via inherent method delegation. |
| storage/src/qmdb/any/sync/mod.rs | Generalize any-db sync helpers over Merkle family and switch to NonEmptyRange. |
| storage/src/qmdb/any/ordered/variable.rs | Rename sync test trait assoc type from Mmr to Merkle. |
| storage/src/qmdb/any/ordered/fixed.rs | Rename sync test trait assoc type from Mmr to Merkle. |
| storage/src/qmdb/any/db.rs | Add sync_boundary() (equal to inactivity floor for any). |
| storage/src/merkle/mod.rs | Require Family::peaks() iterator to be Send. |
| storage/src/merkle/mmr/journaled.rs | Update sync config/tests to use NonEmptyRange. |
| storage/src/merkle/journaled.rs | Change SyncConfig.range to NonEmptyRange and update init/write logic accordingly. |
| storage/fuzz/fuzz_targets/qmdb_any_variable_sync.rs | Switch fuzzing logic from inactivity floor to sync_boundary(). |
| storage/fuzz/fuzz_targets/qmdb_any_fixed_sync.rs | Thread family through fuzz target sync types and use sync_boundary(). |
| storage/fuzz/fuzz_targets/merkle_journaled.rs | Update fuzz sync ranges to NonEmptyRange. |
| storage/fuzz/fuzz_targets/current_unordered_operations.rs | Use sync_boundary() for pruning/proof bounds in fuzzing. |
| storage/fuzz/fuzz_targets/current_ordered_operations.rs | Use sync_boundary() for pruning/proof bounds in fuzzing. |
| storage/fuzz/fuzz_targets/current_mmb_prune_grow.rs | Use sync_boundary() for pruning helper in fuzzing. |
| storage/fuzz/fuzz_targets/current_crash_recovery.rs | Use sync_boundary() for pruning/recovery verification loops. |
| storage/conformance.toml | Update codec conformance entry for family-parameterized Target. |
| examples/sync/src/net/wire.rs | Update wire types to use Target<mmr::Family, D>. |
| examples/sync/src/net/resolver.rs | Implement Resolver::Family and update return types for family-aware fetch results. |
| examples/sync/src/databases/mod.rs | Rename API from inactivity_floor to sync_boundary in the example DB abstraction. |
| examples/sync/src/databases/keyless.rs | Use sync_boundary() in example keyless DB adapter. |
| examples/sync/src/databases/immutable.rs | Use sync_boundary() in example immutable DB adapter. |
| examples/sync/src/databases/current.rs | Use sync_boundary() in example current DB adapter. |
| examples/sync/src/databases/any.rs | Use sync_boundary() in example any DB adapter. |
| examples/sync/src/bin/server.rs | Serve targets using sync_boundary rather than inactivity floor. |
| examples/sync/src/bin/client.rs | Update target update channel/types to include the family parameter. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
roberto-bayardo
force-pushed
the
mmb-sync-approach-2
branch
2 times, most recently
from
bc95804 to
8233b0b
Compare
danlaine
reviewed
Apr 20, 2026
Collaborator
danlaine
left a comment
danlaine
left a comment
There was a problem hiding this comment.
LLM review also pointed out a bunch of mmr -> merkle naming nits:
Stale "MMR" docs in generalized code
All of these now describe logic that applies to both MMR and MMB — update
to "merkle tree" / "merkle family" / "ops tree":
- storage/src/qmdb/current/sync/mod.rs:6-24 — module-level doc: "grafted
MMR root", "standard MMR range proofs", "grafted MMR are reconstructed",
"read directly from the ops MMR", "making the grafted MMR structurally
identical to the ops MMR…"
- storage/src/qmdb/current/sync/mod.rs:91-92 — build_db doc: "ops MMR",
"grafted MMR from the bitmap and ops MMR"
- storage/src/qmdb/current/sync/mod.rs:165-195 — inline comments:
"Extract grafted pinned nodes from the ops MMR", "The grafted MMR's
pinned nodes", "Build grafted MMR"
- storage/src/qmdb/current/sync/mod.rs:314 — "batches against the ops
MMR"
- storage/src/qmdb/any/sync/mod.rs:52 — "operations-MMR layout"
- storage/src/qmdb/sync/engine.rs:169 — "Pinned MMR nodes extracted from
proofs"
- storage/src/qmdb/sync/engine.rs:178 — "the MMR is append-only and
validate_update rejects unchanged roots"
- storage/src/qmdb/sync/resolver.rs:41,73 — "Pinned MMR nodes at the
start location"
also:
Stale README:
examples/sync/README.md:120. Still says "sync target (inactivity floor,
size, and root digest)". Server was changed to send sync_boundary()
instead; these differ for MMB.
| /// known to be inactive. Operations before this point can be safely pruned. | ||
| pub const fn inactivity_floor_loc(&self) -> Location<F> { | ||
| #[cfg(any(test, feature = "test-traits"))] | ||
| pub(crate) const fn inactivity_floor_loc(&self) -> Location<F> { |
Collaborator
There was a problem hiding this comment.
Should we remove this and just use sync_boundary which is already pub?
Collaborator
Author
There was a problem hiding this comment.
I've already replaced most all uses of this, but there are some tests where it's still useful (e.g. floor raising)
Collaborator
Author
There was a problem hiding this comment.
Oh there were more inappropriate uses of prune(inactivity_floor_loc()) that the bot missed, but hopefully fixed now.
Comment on lines
80
to
+76
| // Size + root match implies the last CommitFloor op (and therefore the | ||
| // committed inactivity floor) matches, per the caller contract above. | ||
| // size + root identify a unique state, so if they match the target's we can reuse | ||
| // the persisted DB without fetching boundary pins. |
Collaborator
There was a problem hiding this comment.
nit: (and is never ) closed
| pub(crate) type JournalOf<H> = <DbOf<H> as qmdb::sync::Database>::Journal; | ||
|
|
||
| /// Type alias for the merkle family used by a harness. | ||
| pub(crate) type FamilyOf<H> = <DbOf<H> as qmdb::sync::Database>::Family; |
Collaborator
There was a problem hiding this comment.
I think we can remove this and just do H::Family
roberto-bayardo
force-pushed
the
mmb-sync-approach-2
branch
2 times, most recently
from
a35f784 to
8d8b6e6
Compare
Collaborator
Author
Should be fixed |
roberto-bayardo
force-pushed
the
mmb-sync-approach-2
branch
2 times, most recently
from
3684666 to
93a7705
Compare
danlaine
reviewed
Apr 21, 2026
| pub(crate) type DbOf<H> = <H as SyncTestHarness>::Db; | ||
| pub(crate) type OpOf<H> = <DbOf<H> as qmdb::sync::Database>::Op; | ||
| pub(crate) type ConfigOf<H> = <DbOf<H> as qmdb::sync::Database>::Config; | ||
| pub(crate) type FamilyOf<H> = <DbOf<H> as qmdb::sync::Database>::Family; |
Collaborator
There was a problem hiding this comment.
I think we can remove FamilyOf like we did for qmdb::any
danlaine
reviewed
Apr 21, 2026
| pub(crate) type DbOf<H> = <H as SyncTestHarness>::Db; | ||
| pub(crate) type OpOf<H> = <DbOf<H> as qmdb::sync::Database>::Op; | ||
| pub(crate) type ConfigOf<H> = <DbOf<H> as qmdb::sync::Database>::Config; | ||
| pub(crate) type FamilyOf<H> = <DbOf<H> as qmdb::sync::Database>::Family; |
Collaborator
There was a problem hiding this comment.
I think we can remove FamilyOf like we did for qmdb::any
danlaine
reviewed
Apr 21, 2026
| let client_suffix = context.next_u64().to_string(); | ||
| let client_config = variable_config::<crate::translator::TwoCap>(&client_suffix, &context); | ||
| let target_db = std::sync::Arc::new(target_db); | ||
| // Supply the trusted canonical root so `build_db`'s authentication check actually |
Collaborator
There was a problem hiding this comment.
It's actually the ops root, not the canonical root, I think
Collaborator
Author
There was a problem hiding this comment.
yup good catch, fixed
Collaborator
There was a problem hiding this comment.
This comment is still here -- forgot to push maybe?
danlaine
reviewed
Apr 21, 2026
| let hasher = StandardHasher::<H>::new(); | ||
| let mmr = mmr::journaled::Mmr::init_sync( | ||
| let merkle = Journaled::<F, _, _>::init_sync( | ||
| context.with_label("mmr"), |
Collaborator
There was a problem hiding this comment.
Suggested change
| context.with_label("mmr"), | |
| context.with_label("merkle"), |
roberto-bayardo
force-pushed
the
mmb-sync-approach-2
branch
from
918a0d5 to
7ffaec9
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit de3324e. Configure here.
danlaine
approved these changes
Apr 21, 2026
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #3626 +/- ##
==========================================
- Coverage 95.88% 95.85% -0.04%
==========================================
Files 441 441
Lines 172372 171196 -1176
Branches 4006 4001 -5
==========================================
- Hits 165287 164104 -1183
- Misses 5819 5830 +11
+ Partials 1266 1262 -4
... and 8 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.
Generalize sync protocol on merkle family
Generalizes the QMDB sync protocol over merkle families (MMR/MMB) and tightens the current-DB grafting contract so
pruneand sync targets can't produce state the receiver will fail on.Motivation
The sync protocol was hardcoded to
mmr::Familythroughout.current::Db::prunesilently clippedprune_locto an internal "settled" boundary, so callers could believe they had pruned toprune_locwhile the ops log actually retained an older start — the kind of mismatch that breaks delayed-merge sync targets.What changed
Target,FetchResult,Requests,EngineError,Error,Database,Resolver,Journal<F>and all their impls/macros now carry a family parameter.immutable/keylesssync impls and their resolver macros no longer hardcodemmr::Family.current::Dbgrafting contract: newsync_boundary()walks back from the inactivity floor until chunks are settled and absorbed;prunevalidatesprune_loc <= sync_boundaryand returnsPruneBeyondMinRequiredinstead of silently clipping;rewindgains a symmetric floor check;inactivity_floor_locis test-only.keyless/sync.rsandimmutable/sync.rsconverted tosync/{mod,tests}.rsdirectories, each gaining aSyncTestHarnesstrait with MMR+MMB instantiations (matches the pattern already inany/sync/andcurrent/sync/). Per-family harness duplication collapsed into single generic impls plus type aliases.