Skip to content

feat: Add Mmr::nodes_from and Mmr::from_nodes_unchecked - #3585

Merged
sergerad merged 10 commits into
sergerad-mmr-clonefrom
sergerad-mmr-incr-serde
Aug 17, 2026
Merged

feat: Add Mmr::nodes_from and Mmr::from_nodes_unchecked#3585
sergerad merged 10 commits into
sergerad-mmr-clonefrom
sergerad-mmr-incr-serde

Conversation

@sergerad

@sergerad sergerad commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Describe your changes

Unblocks 0xMiden/node#2468 which wants to persist Mmr segments incrementally to a flat file.

Write side: adds Mmr::nodes_from(start), returning an iterator over the MMR's nodes at indices start.. in insertion (postorder) order. The node buffer is append-only, so a consumer that has persisted the first start nodes can sync incrementally by appending only the nodes returned here — this lets miden-node keep a flat file of 32-byte node records instead of rewriting the full MMR or using RocksDB.

Read side: adds Mmr::from_nodes_unchecked(forest, nodes), reconstructing an MMR from its complete postorder node array without re-hashing (the exact cost the flat file exists to avoid — try_from_iter recomputes every merge). The node count is validated against the forest, but node values are taken verbatim, so the caller is responsible for checking the result against a trusted commitment (e.g. peaks()) — same trust model as read_from_bytes, with the _unchecked suffix following Blockchain::from_mmr_unchecked. Deserializable::read_from now delegates to the same validation path instead of duplicating the count check.

The backing NodeStore::iter_from skips directly to the starting chunk. Tests cover chunk-boundary/mid-chunk/past-the-end starts, an incremental-persistence round trip, rebuild-from-nodes round trips (forest/nodes/peaks match and openings verify, including empty-forest and multi-chunk sizes), and count-mismatch rejection (MmrError::InvalidNodeCount).

Checklist before requesting a review

  • Repo forked and branch created from next according to naming convention.
  • Commit messages and codestyle follow conventions.
  • Commits are signed.
  • Relevant issues are linked in the PR description.
  • Tests added for new functionality.
  • Documentation/comments updated according to changes.
  • Updated CHANGELOG.md

@sergerad
sergerad force-pushed the sergerad-mmr-incr-serde branch from 689e1a3 to 83d9f73 Compare August 11, 2026 23:38
@sergerad
sergerad force-pushed the sergerad-mmr-incr-serde branch from 83d9f73 to 5645e98 Compare August 11, 2026 23:40
@sergerad
sergerad marked this pull request as draft August 12, 2026 00:01
@sergerad sergerad changed the title feat: Add Mmr::nodes_from feat: Add Mmr::nodes_from and Mmr::from_nodes_unchecked Aug 12, 2026
@sergerad
sergerad marked this pull request as ready for review August 12, 2026 23:57

@huitseeker huitseeker left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, this looks good, but I think this PR needs one code change and one wording change:

  • Restore the early count check in read_from.
  • State clearly that count validation checks representation shape, while trusted commitment comparison checks the accumulator state. from_nodes_unchecked does neither full rehashing nor full internal node validation.

Comment thread crates/crypto/src/merkle/mmr/full.rs Outdated
Comment thread crates/crypto/src/merkle/mmr/full.rs
Comment thread crates/crypto/src/merkle/mmr/full.rs Outdated
@sergerad
sergerad requested a review from huitseeker August 13, 2026 21:25
///
/// Skips directly to the chunk containing `start` instead of walking from the front. Returns
/// an empty iterator if `start >= self.len()`.
pub fn iter_from(&self, start: usize) -> MmrNodeIter<'_> {

@sergerad sergerad Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noting that this doesn't leak MmrNodeIter into pub api b/c pub(super) struct NodeStore.

Comment thread crates/crypto/src/merkle/mmr/error.rs

@Al-Kindi-0 Al-Kindi-0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@huitseeker huitseeker left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread crates/crypto/src/merkle/mmr/full.rs Outdated
@sergerad
sergerad force-pushed the sergerad-mmr-incr-serde branch from 1801e05 to b6ad5bf Compare August 17, 2026 20:48
@sergerad
sergerad merged commit 4f66f9a into next Aug 17, 2026
32 checks passed
@sergerad
sergerad deleted the sergerad-mmr-incr-serde branch August 17, 2026 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants