Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions miden-crypto/src/merkle/smt/forest/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ impl SmtForest {
/// Inserts the specified key-value pair into an SMT with the specified root. This will also
/// add a new root to the forest. Returns the new root.
///
/// Returns an error if an SMT with the specified root is not in the forest, these is not
/// Returns an error if an SMT with the specified root is not in the forest, there is not
/// enough data in the forest to perform the insert, or if the insert would create a leaf
/// with too many entries.
pub fn insert(&mut self, root: Word, key: Word, value: Word) -> Result<Word, MerkleError> {
Expand All @@ -136,7 +136,7 @@ impl SmtForest {
/// Inserts the specified key-value pairs into an SMT with the specified root. This will also
/// add a single new root to the forest for the entire batch of inserts. Returns the new root.
///
/// Returns an error if an SMT with the specified root is not in the forest, these is not
/// Returns an error if an SMT with the specified root is not in the forest, there is not
/// enough data in the forest to perform the insert, or if the insert would create a leaf
/// with too many entries.
pub fn batch_insert(
Expand Down
112 changes: 112 additions & 0 deletions miden-crypto/src/merkle/smt/large_forest/backend.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
//! This file contains the [`Backend`] trait for the [`LargeSmtForest`] implementation and the
//! supporting types it needs.
use core::{any::Any, error::Error, fmt::Debug};

use crate::{
Map, Word,
merkle::smt::{
SmtProof,
large_forest::{
history::VersionId,
operation::{SmtForestUpdateBatch, SmtUpdateBatch},
},
},
};

// BACKEND
// ================================================================================================

/// The backing storage for the SMT forest, providing the necessary high-level methods for
/// performing operations on the full trees that make up the forest, while allowing the forest
/// itself to be storage agnostic.
///
/// # Backend Data Storage
///
/// Having a generic [`Backend`] provides no guarantees to the user about how it stores data and
/// what patterns are used for data access under the hood. It is, however, guaranteed to store
/// _only_ the data necessary to describe the latest state of each tree in the forest.
///
/// # Errors
///
/// The trait allows each implementation to provide its own error type as [`Self::Error`]. This
/// ensures that each backend can tailor its errors to its operation without having to worry about
/// using a pre-defined error enum. Every method is made to return this error type by default to
/// enable accurate error handling, but not all implementations may need to return an error in all
/// cases.
///
/// As a result, specific errors cannot be documented in the trait method documentation blocks and
/// so are not.
pub trait Backend
where
Self: Debug,
{
/// The error type used by the backend.
type Error: BackendError;

// QUERIES
// ============================================================================================

/// Returns an opening for the specified `key` in the SMT with the specified `root`.
fn open(&self, root: Word, key: Word) -> Result<SmtProof, Self::Error>;

/// Returns the value associated with the provided `key` in the SMT with the provided `root`, or
/// [`None`] if no such value exists.
fn get(&self, root: Word, key: Word) -> Result<Option<Word>, Self::Error>;

/// Returns the version of the tree with the provided `root`.
fn version(&self, root: Word) -> Result<VersionId, Self::Error>;

/// Returns an iterator over all the tree roots and versions that the backend knows about.
///
/// The iteration order is unspecified.
fn versions(&self) -> Result<impl Iterator<Item = (Word, VersionId)>, Self::Error>;

// SINGLE-TREE MODIFIERS
// ============================================================================================

/// Performs the provided `operations` on the tree with the provided `root`, returning the new
/// root.
///
/// Implementations must guarantee the following behavior, with non-conforming implementations
/// considered to be a bug:
///
/// - At most one new root must be added to the forest for the entire batch.
/// - If applying the provided `operations` results in no changes to the tree, no new tree must
/// be allocated.
fn modify_tree(
&mut self,
root: Word,
new_version: VersionId,
operations: SmtUpdateBatch,
) -> Result<Word, Self::Error>;

// MULTI-TREE MODIFIERS
// ============================================================================================

/// Performs the provided `operations` on the forest, returning a mapping from old root to new
/// root.
///
/// Implementations must guarantee the following behaviour, with non-conforming implementations
/// considered to be a bug:
///
/// - At most one new root must be added to the forest for each target root in the provided
/// `operations`.
/// - If applying the provided `operations` results in no changes to a given lineage of trees in
/// the forest, then no new tree must be allocated in that lineage.
fn modify_forest(
Copy link
Contributor

Choose a reason for hiding this comment

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

This new Backend trait is simpler and does not have explicit begin/commit methods. But I am curious how error recovery would work for persistent backends. If modify_forest partially succeeds (say, updates 3 of 5 trees) and then hits an error, what is the expected state? Should the trait doc clarify whether implementations must be atomic (all-or-nothing) or whether partial failures are allowed?

(this is the thrust of my comments on abort /rollback in #700, BTW — though I'm more interested in the atomicity contract in the abstract sense than how that's conveyed in the API, here)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My thinking on this is that atomicity is now an internal concept to the Backend. It was never intended to be user-facing in the previous design, just exposed to the forest itself, as I don't think we ever want to contend with user-facing rollback.

The main reason the concept exists is to prevent data corruption in the persistent backend. If something like your above scenario occurs in the persistent backend the whole transaction will be backed out.

That said, I think documenting something is probably valuable. What would you expect to happen for the in-memory backend should updating the 4th tree fail? To roll back to the previous state, or allow the partial delta? The same question applies if the backend is part-way through applying operations and fails.

I'm not really an API client here, so understanding how this should behave is important to me!

&mut self,
operations: SmtForestUpdateBatch,
) -> Result<Map<Word, Word>, Self::Error>;
}

// BACKEND ERROR
// ================================================================================================

/// A trait that must be implemented by the error types for the [`Backend`], primarily serving to
/// work around the lack of negative impl constraints in Rust.
pub trait BackendError
where
Self: Any + Debug + Error,
{
}
21 changes: 0 additions & 21 deletions miden-crypto/src/merkle/smt/large_forest/error.rs

This file was deleted.

68 changes: 68 additions & 0 deletions miden-crypto/src/merkle/smt/large_forest/error/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
//! This module contains the error types and helpers for working with errors from the large SMT
//! forest.

pub mod subtree;

use thiserror::Error;

use crate::{
Word,
merkle::{
MerkleError,
smt::large_forest::{backend::BackendError, history::error::HistoryError},
},
};

// LARGE SMT FOREST ERROR
// ================================================================================================

/// The type of errors returned by operations on the large SMT forest.
#[derive(Debug, Error)]
pub enum LargeSmtForestError<E: BackendError> {
/// Errors with the storage backend of the forest.
#[error(transparent)]
BackendError(#[from] E),

/// Errors in the history subsystem of the forest.
#[error(transparent)]
HistoryError(#[from] HistoryError),

/// Raised when an attempt is made to modify a frozen tree.
#[error("Attempted to modify non-current tree with root {0}")]
InvalidModification(Word),

/// Errors with the merkle tree operations of the forest.
#[error(transparent)]
MerkleError(#[from] MerkleError),
}

/// The result type for use within the large SMT forest portion of the library.
pub type Result<T, B> = core::result::Result<T, LargeSmtForestError<B>>;

pub mod backend {}

pub mod prefix {
use thiserror::Error;

use crate::{Word, merkle::smt::large_forest::utils::LinearIndex};

#[derive(Debug, Eq, Error, PartialEq)]
pub enum PrefixError {
/// Raised if an indexing operation would be out of bounds.
#[error("Index {0} was out of bounds in a prefix with {1} levels")]
IndexOutOfBounds(LinearIndex, u8),

/// Raised if the forest cannot restore correctly from the saved restoration data.
#[error("Restoration data for tree with root {0} produced root {1}")]
InvalidRestoration(Word, Word),

/// Raised if the number of leaves in the restoration data provided to the prefix is
/// incorrect for the depth of the prefix.
#[error("Was given {0} leaves but expected {1}")]
WrongLeafCount(u64, u64),
}

/// The result type for use within the prefix portion of the library.
#[allow(dead_code)] // Temporary
pub type Result<T> = core::result::Result<T, PrefixError>;
}
36 changes: 36 additions & 0 deletions miden-crypto/src/merkle/smt/large_forest/error/subtree.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
//! Errors in working with subtrees.

use thiserror::Error;

/// Errors raised when encountering an issue when working with subtrees.
#[allow(dead_code)] // Temporary
#[derive(Debug, Error)]
pub enum SubtreeError {
/// Raised when decoding the subtree from bytes and encountering insufficient node data.
#[error("Expected {expected} bytes of node data, found {found} bytes")]
BadHashLen { expected: usize, found: usize },

/// Raised when the left index has an invalid hash.
#[error("Invalid left hash format at local index {index}")]
BadLeft { index: u64 },

/// Raised when the left index has an invalid hash.
#[error("Invalid right hash format at local index {index}")]
BadRight { index: u64 },

/// When extra node data exists in the bytestream after the expected data.
#[error("Found extra node data after bitmask-indicated entries")]
ExtraData,

/// When the node data for the left index of a node.
#[error("Missing left node data at local index {index}")]
MissingLeft { index: u64 },

/// When the node data for the right index of a node.
#[error("Missing right node data at local index {index}")]
MissingRight { index: u64 },

/// Raised when there is insufficient data when decoding the subtree.
#[error("Found {found} bytes when decoding the subtree, but need at least {min} bytes")]
TooShort { found: usize, min: usize },
}
14 changes: 8 additions & 6 deletions miden-crypto/src/merkle/smt/large_forest/history/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use core::fmt::Debug;
use error::{HistoryError, Result};

use crate::{
Map, Set, Word,
Map, Word,
merkle::{
NodeIndex,
smt::{LeafIndex, SMT_DEPTH},
Expand Down Expand Up @@ -136,13 +136,15 @@ impl History {

/// Returns all the roots that the history knows about.
///
/// The iteration order of the roots is guaranteed to move backward in time, with earlier items
/// being roots from versions closer to the present.
///
/// # Complexity
///
/// Calling this method requires a traversal of all the versions and is hence linear in the
/// number of history versions.
#[must_use]
pub fn roots(&self) -> Set<Word> {
self.deltas.iter().map(|d| d.root).collect()
/// Calling this method provides an iterator whose consumption requires a traversal of all the
/// versions. The method's complexity is thus `O(n)` in the number of versions.
pub fn roots(&self) -> impl Iterator<Item = Word> {
self.deltas.iter().rev().map(|d| d.root)
}

/// Returns `true` if `root` is in the history and `false` otherwise.
Expand Down
4 changes: 3 additions & 1 deletion miden-crypto/src/merkle/smt/large_forest/history/tests.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#![cfg(feature = "std")]
//! The functional tests for the history component.

use alloc::vec::Vec;

use p3_field::PrimeCharacteristicRing;

use super::{CompactLeaf, History, LeafChanges, NodeChanges, error::Result};
Expand Down Expand Up @@ -32,7 +34,7 @@ fn roots() -> Result<()> {
history.add_version(root_2, 1, nodes.clone(), leaves.clone())?;

// We should be able to get all the roots.
let roots = history.roots();
let roots = history.roots().collect::<Vec<_>>();
assert_eq!(roots.len(), 2);
assert!(roots.contains(&root_1));
assert!(roots.contains(&root_2));
Expand Down
Loading