refactor: unify account/nullifier SMT backends into shared SmtBackend traits#3009
Merged
Merged
Conversation
… traits Replaces the near-identical AccountTreeBackend/NullifierTreeBackend trait pairs with a single Word-valued SmtBackendReader + SmtBackend pair, impl'd once for Smt and once for LargeSmt. The NullifierBlock value wrapping moves from the backend into NullifierTree's own accessors, leaving the backend layer value-agnostic. Removes both per-tree backend.rs modules.
Addresses pre-push review findings on the SMT backend unification: - correct the CHANGELOG entry to name the shared SmtBackend/SmtBackendReader traits instead of the removed per-tree reader traits - disambiguate the writer-impl section headers in smt_backend.rs - document that with_storage_from_entries panics (not errors) on storage failures, matching large_smt_error_to_merkle_error behavior - add reader() tests asserting the read-only view shares root and entries
Removes the redundant AccountTreeError::DuplicateEntries variant and maps
the LargeSmt-backed with_storage_from_entries duplicate case through the same
prefix-extracting helper as the Smt-backed with_entries, so both constructors
surface DuplicateStateCommitments { prefix }. Adds a duplicate-entry test,
documents the read-only reader() invariant, and aligns the leaves()/entries()
lifetime style.
These only asserted that a freshly created reader view equals its source, which exercises no meaningful logic. The duplicate-entry test (real error path) is kept.
mmagician
commented
May 29, 2026
Co-authored-by: Marti <marcin.gorny.94@protonmail.com>
mmagician
changed the title
SmtBackend traits
Use inline `[`leaves`](Self::leaves)` / `[`entries`](Self::entries)` links instead of reference-style links, which rustdoc failed to resolve under `-D warnings`.
PhilippGackstatter
approved these changes
Jun 1, 2026
PhilippGackstatter
left a comment
PhilippGackstatter
left a comment
Contributor
There was a problem hiding this comment.
Looks good to me!
I think since the backends did not add much type safety to account tree or nullifier tree, the duplication isn't worth it and deduplicating them makes sense to me.
sergerad
reviewed
Jun 1, 2026
| )] | ||
| DuplicateStateCommitments { prefix: AccountIdPrefix }, | ||
| #[error("entries passed to account tree contain duplicate values")] | ||
| DuplicateEntries(#[source] MerkleError), |
Contributor
There was a problem hiding this comment.
Why don't we use this and instead have duplicate_state_commitment_error()? Understood that this PR only added the fn, not the underlying impl which already existed.
Collaborator
Author
There was a problem hiding this comment.
DuplicateEntries described the same condition as DuplicateStateCommitments (two entries colliding on the same account-ID prefix) - but with a vaguer message (no account id included in the error)
sergerad
approved these changes
Jun 1, 2026
sergerad
left a comment
sergerad
left a comment
Contributor
There was a problem hiding this comment.
LGTM just wondering about the #[source] question which is somewhat tangential to the PR.
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
5 participants
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
Follow-up to #2755
Replaces the two near-identical per-tree backend trait pairs (
AccountTreeBackend/NullifierTreeBackendand their reader variants from #2755) with a single, value-agnostic pair in a newblock/smt_backend.rs:SmtBackendReader(read-only) andSmtBackend: SmtBackendReader(read-write), implemented once forSmtand once forLargeSmt(reader impl bounded onSmtStorageReader, writer impl onSmtStorage).Design notes
leaves/num_leaves/get_leaf; nullifier usesentries/num_entries).num_leavesandnum_entriesare both kept because they differ for the nullifier tree when keys collide into a singleSmtLeaf::Multiple- collapsing them would undercount nullifiers.Open questions
SmtBackend/SmtBackendReadervs something more specific?