feat: SmtStorageReader and SparseMerkleTreeReader traits by sergerad · Pull Request #967 · 0xMiden/crypto

sergerad · 2026-04-14T23:45:43Z

Description

Extract a new SmtStorageReader trait from SmtStorage, enabling LargeSmt<S> to work with read-only storage for read operations, and only require SmtStorage for mutations.

Extract a new SparseMerkleTreeReader trait from SparseMerkleTree, enabling LargeSmt<S> to implement read-only trait methods with only an S: SmtStorageReader bound.

Motivation

Previously, LargeSmt<S: SmtStorage> required full read+write storage for all operations, including purely read-only ones like get_leaf, get_value, open, and tree loading. This made it impossible to use LargeSmt with a read-only storage backend.

The SparseMerkleTree trait bundled read and write methods together, so impl SparseMerkleTree for LargeSmt<S> required S: SmtStorage. Splitting the trait allows LargeSmt to implement read-only methods with only the SmtStorageReader bound.

In miden-node (PR), we are working towards removing synchronization mechanisms by providing read-only types which can be cloned.

Changes

Storage trait hierarchy (storage/mod.rs):

SmtStorageReader — &self methods (leaf/subtree reads, iterators, get_depth24)
SmtStorage: SmtStorageReader — &mut self methods (inserts, removes, apply)

SMT trait hierarchy (smt/mod.rs):

SparseMerkleTreeReader — read-only methods (root, get_inner_node, get_value, get_leaf, open, get_path, compute_mutations, etc.)
SparseMerkleTree: SparseMerkleTreeReader — write methods (set_root, insert_inner_node, remove_inner_node, insert_value, insert, apply_mutations, etc.)
from_raw_parts removed from the trait and inlined into the inherent methods on Smt and SimpleSmt, since LargeSmt could never meaningfully implement it.

LargeSmt<S> struct bound relaxed (large/mod.rs):

Struct bound changed from S: SmtStorage to S: SmtStorageReader
Impl blocks split by required capability:
- impl<S: SmtStorageReader> — accessors (root, get_leaf, get_value, open, is_empty), iterators, construction from existing storage (new, load, load_with_root)
- impl<S: SmtStorage> — insert, with_entries, batch construction methods

LargeSmt<S> trait impls split (large/smt_trait.rs):

impl<S: SmtStorageReader> SparseMerkleTreeReader for LargeSmt<S> — all read-only trait methods
impl<S: SmtStorage> SparseMerkleTree for LargeSmt<S> — write methods only

Other files updated:

Smt and SimpleSmt — trait impls split into SparseMerkleTreeReader + SparseMerkleTree
batch_ops.rs — read-only qualified calls updated to SparseMerkleTreeReader
construction.rs — split into Reader (load from storage) and Storage (build with entries) blocks
iter.rs — bounded by SmtStorageReader
memory.rs, rocksdb.rs — split impl SmtStorage into impl SmtStorageReader + impl SmtStorage
smt/mod.rs — re-exports SmtStorageReader and SparseMerkleTreeReader

Backwards compatibility

SmtStorage retains all original methods (read + write) via its supertrait SmtStorageReader, so existing code using S: SmtStorage continues to compile unchanged.

Both SparseMerkleTree and SparseMerkleTreeReader are pub(crate), so the trait split has no public API impact. from_raw_parts was only ever called through the inherent methods on Smt and SimpleSmt, which retain the same signature.

iamrecursion

This seems reasonable at first glance, but I am concerned about the code that has been manually inlined from the SparseMerkleTree trait.

Also, what is your plan with regards to the fact that this PR is targeted for main? These changes should also be included on next in my view.

sergerad · 2026-04-15T20:12:57Z

Also, what is your plan with regards to the fact that this PR is targeted for main? These changes should also be included on next in my view.

Will create a PR into next after this is merged. I'm working off of v0.23.0 because that is what miden-node / protocol are currently using.

Co-authored-by: Ara Adkins <[email protected]>

iamrecursion

This looks mostly good to me. I've left a few comments inline and I'd like to take another look before merge.

iamrecursion · 2026-04-16T10:56:56Z

Also, what is your plan with regards to the fact that this PR is targeted for main? These changes should also be included on next in my view.

Will create a PR into next after this is merged. I'm working off of v0.23.0 because that is what miden-node / protocol are currently using.

Souds good to me.

huitseeker

re: #967 (comment)
I have ported the RocksDB changes to the node.

I opened #980 as one way of addressing the issues raised in threads here.

huitseeker

Checkpointing my partial re-review. Overall LGTM except for the line comment, will come back a bit later to stamp.

huitseeker · 2026-04-23T14:34:37Z

+            }
+        }
+
+        Ok(MemoryStorage { leaves, subtrees }.into_snapshot())


Come to think of it: could this snapshot keep the depth-24 cache too?

LargeSmt::load will accept this SmtStorageSnapshot, and because it is backed by MemoryStorage, get_depth24() returns an empty vec even when the snapshot has leaves/subtrees — so a non-empty snapshot panics in debug in initialize_from_storage and reconstructs the wrong top in release.

I opted to add an implementation to get_depth24() for MemoryStorage and added a test. Let me know if you would have approached it differently.

huitseeker

LGTM with one caveat, @sergerad : this looks like a breaking public API change. SmtStorage is re-exported from miden_crypto::merkle::smt, and every downstream impl SmtStorage now needs a new SmtStorageReader impl, type Reader, and reader().

I'd at least call that out as breaking in the PR description and changelog, even if we do expect external backends to adapt quickly. If only to help with versioning.

…mt-reader-trait

sergerad · 2026-04-27T19:58:28Z

Also, what is your plan with regards to the fact that this PR is targeted for main? These changes should also be included on next in my view.

Will create a PR into next after this is merged. I'm working off of v0.23.0 because that is what miden-node / protocol are currently using.

Ended up pointing to next branch because this PR has made breaking changes.

…mt-reader-trait

bobbinth

Looks good! Thank you! I left some comments/questions inline.

bobbinth · 2026-04-30T07:52:35Z

+/// Type alias for boxed storage with boxed reader
+pub type BoxedSmtStorage = Box<dyn SmtStorage<Reader = Box<dyn SmtStorageReader>>>;


If these are for benchmarks only, should we put them behind the testing feature? (not sure if that will work - but would be good to check)

bobbinth · 2026-04-30T08:11:59Z

+/// Implementations may return either a point-in-time snapshot or a live view.
+pub trait SmtStorageReader: 'static + fmt::Debug + Send + Sync {


Related to the previous comment: I think returning point-in-time snapshots is fine - but can we really return readers with "live views"? At the storage level this is possible, but since we clone the in-memory portion of the SMT, the in-memory portion would become inconsistent with the storage portion as soon as the data in the storage changes.

Unless the above reasoning is incorrect, I think we should make it an explicit requirement that storage readers must always be against point-in-time snapshots.

#985 is one way to approach this.

bobbinth · 2026-04-30T08:13:46Z

+    /// Implementations may return either a point-in-time snapshot or a live view. Either way, the
+    /// view must be of consistent / committed state (not partial). Holding the reader must not
+    /// block writes in any way.
+    fn reader(&self) -> Result<Self::Reader, StorageError>;


Same comment as above re "live view".

sergerad · 2026-05-01T00:47:59Z

If these are for benchmarks only, should we put them behind the testing feature? (not sure if that will work - but would be good to check)

This is all already behind the executable feature

#[cfg(feature = "executable")]
mod boxed_storage;

bobbinth

Looks good! Thank you!

I didn't review RocksDbSnapshot and related structs in depth because I'm assuming this will serve more like a guiding implementation for the structs that will live in miden-node.

bobbinth · 2026-05-01T02:31:51Z

+pub struct RocksDbSnapshotStorage {
+    inner: Arc<RocksDbSnapshotInner>,
+}


When we move RocksDB storage into miden-node completely, it would be good to add some integration tests to make sure this all works as expected.

sergerad added 9 commits April 9, 2026 08:58

Add Clone to types

71e667f

Split SmtStorage trait

b8597a7

Split impls

2c5ce2f

Update rand crate

e48479e

RM writer trait

5619869

Changelog

c9bc45e

Rand version again

1a713c3

Fix rand core deny

bcb1daa

Update changelog

22c1b78

sergerad changed the title ~~Sergerad largesmt reader trait~~ feat: SmtStorageReader trait Apr 14, 2026

sergerad commented Apr 14, 2026

View reviewed changes

Comment thread miden-crypto/src/merkle/smt/large_forest/mod.rs

This was referenced Apr 14, 2026

feat: SmtStorageReader trait #958

Closed

feat: AccountTreeBackendReader and NullifierTreeBackendReader traits 0xMiden/protocol#2755

Draft

sergerad requested review from bobbinth and iamrecursion April 14, 2026 23:59

iamrecursion requested changes Apr 15, 2026

View reviewed changes

Comment thread miden-crypto/src/merkle/smt/large/storage/mod.rs Outdated

Comment thread miden-crypto/src/merkle/smt/large/storage/rocksdb.rs

Comment thread miden-crypto/src/merkle/smt/large/mod.rs Outdated

sergerad and others added 4 commits April 16, 2026 08:14

Fix comment grammar

d6961f8

Co-authored-by: Ara Adkins <[email protected]>

Split SparseMerkleTree to reader trait, remove from_raw_parts from trait

83ce9f3

LargeSmtForest manual clone

4661f85

Update changelog

bafcee9

sergerad requested a review from iamrecursion April 15, 2026 21:52

sergerad changed the title ~~feat: SmtStorageReader trait~~ feat: SmtStorageReader and SparseMerkleTreeReader traits Apr 15, 2026

Add with_reader

157f0ed

sergerad commented Apr 16, 2026

View reviewed changes

Comment thread miden-crypto/src/merkle/smt/large/mod.rs

Add reader to trait

4441d9c

iamrecursion reviewed Apr 16, 2026

View reviewed changes

Fix benchmark

b30cb4f

huitseeker reviewed Apr 18, 2026

View reviewed changes

Comment thread miden-crypto/src/merkle/smt/large/storage/rocksdb.rs Outdated

Comment thread miden-crypto/src/merkle/smt/large/mod.rs Outdated

sergerad requested review from bobbinth and iamrecursion April 21, 2026 00:01

Fix simple compute_mutations

450c9b1

huitseeker mentioned this pull request Apr 21, 2026

Harden LargeSmt reader snapshots #980

Merged

huitseeker reviewed Apr 21, 2026

View reviewed changes

Harden LargeSmt reader snapshots (#980)

77bc5c4

sergerad requested a review from huitseeker April 21, 2026 20:10

huitseeker reviewed Apr 23, 2026

View reviewed changes

sergerad added 2 commits April 24, 2026 10:38

Update in memory get_depth24() and add test

99167d2

Lint

4f12f87

sergerad requested a review from huitseeker April 24, 2026 03:51

huitseeker approved these changes Apr 24, 2026

View reviewed changes

sergerad mentioned this pull request Apr 27, 2026

feat: Refactor apply_block() locks 0xMiden/node#1901

Draft

sergerad changed the base branch from main to next April 27, 2026 19:53

Merge branch 'next' of github.com:0xMiden/crypto into sergerad-larges…

a281cb8

…mt-reader-trait

sergerad enabled auto-merge (squash) April 27, 2026 21:22

Merge branch 'next' of github.com:0xMiden/crypto into sergerad-larges…

7ab634d

…mt-reader-trait

bobbinth reviewed Apr 30, 2026

View reviewed changes

huitseeker reviewed Apr 30, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

huitseeker mentioned this pull request Apr 30, 2026

Make LargeSmt readers cheap point-in-time snapshots #985

Merged

Make LargeSmt readers cheap point-in-time snapshots (#985)

7d5daf1

sergerad and others added 2 commits May 1, 2026 12:53

Move snapshot code to separate sections

0a03e85

Merge branch 'next' into sergerad-largesmt-reader-trait

e1abc8d

bobbinth approved these changes May 1, 2026

View reviewed changes

bobbinth disabled auto-merge May 1, 2026 02:36

bobbinth merged commit a7f748e into next May 1, 2026
29 checks passed

bobbinth deleted the sergerad-largesmt-reader-trait branch May 1, 2026 02:36

sergerad mentioned this pull request May 4, 2026

Add BackendReader trait #986

Open

		/// Type alias for boxed storage with boxed reader
		pub type BoxedSmtStorage = Box<dyn SmtStorage<Reader = Box<dyn SmtStorageReader>>>;

		/// Implementations may return either a point-in-time snapshot or a live view.
		pub trait SmtStorageReader: 'static + fmt::Debug + Send + Sync {

Conversation

sergerad commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation

Changes

Backwards compatibility

Uh oh!

Uh oh!

iamrecursion left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sergerad commented Apr 15, 2026

Uh oh!

Uh oh!

iamrecursion left a comment

Choose a reason for hiding this comment

Uh oh!

iamrecursion commented Apr 16, 2026

Uh oh!

Uh oh!

Uh oh!

huitseeker left a comment

Choose a reason for hiding this comment

Uh oh!

huitseeker left a comment

Choose a reason for hiding this comment

Uh oh!

huitseeker Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

sergerad Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

huitseeker left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sergerad commented Apr 27, 2026

Uh oh!

bobbinth left a comment

Choose a reason for hiding this comment

Uh oh!

bobbinth Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bobbinth Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

huitseeker Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

bobbinth Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sergerad commented May 1, 2026

Uh oh!

bobbinth left a comment

Choose a reason for hiding this comment

Uh oh!

bobbinth May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sergerad commented Apr 14, 2026 •

edited

Loading

huitseeker left a comment •

edited

Loading