diff --git a/CHANGELOG.md b/CHANGELOG.md index 562dff6d..832a19be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,11 @@ -## 0.11.0 (2024-10-30) +## 0.13.0 (TBD) -- [BREAKING] Updated Winterfell dependency to v0.10 (#338). - Fixed a bug in the implementation of `draw_integers` for `RpoRandomCoin` (#343). +- [BREAKING] Refactor error messages and use `thiserror` to derive errors (#344). + +## 0.12.0 (2024-10-30) + +- [BREAKING] Updated Winterfell dependency to v0.10 (#338). ## 0.11.0 (2024-10-17) diff --git a/Cargo.lock b/Cargo.lock index 3e822fb3..ebb0da25 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -78,6 +78,12 @@ version = "0.7.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7c02d123df017efcdfbd739ef81735b36c5ba83ec3c59c80a9d7ecc718f92e50" +[[package]] +name = "assert_matches" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9b34d609dfbaf33d6889b2b7106d3ca345eacad44200913df5ba02bfd31d2ba9" + [[package]] name = "autocfg" version = "1.4.0" @@ -521,6 +527,7 @@ checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" name = "miden-crypto" version = "0.12.0" dependencies = [ + "assert_matches", "blake3", "cc", "clap", @@ -537,6 +544,7 @@ dependencies = [ "seq-macro", "serde", "sha3", + "thiserror", "winter-crypto", "winter-math", "winter-rand-utils", @@ -668,9 +676,9 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.89" +version = "1.0.91" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f139b0662de085916d1fb67d2b4169d1addddda1919e696f3252b740b629986e" +checksum = "307e3004becf10f5a6e0d59d20f3cd28231b0e0827a96cd3e0ce6d14bc1e4bb3" dependencies = [ "unicode-ident", ] @@ -900,9 +908,9 @@ checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" [[package]] name = "syn" -version = "2.0.85" +version = "2.0.89" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5023162dfcd14ef8f32034d8bcd4cc5ddc61ef7a247c024a33e24e1f24d21b56" +checksum = "44d46482f1c1c87acd84dea20c1bf5ebff4c757009ed6bf19cfd36fb10e92c4e" dependencies = [ "proc-macro2", "quote", @@ -922,6 +930,26 @@ dependencies = [ "windows-sys 0.59.0", ] +[[package]] +name = "thiserror" +version = "2.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c006c85c7651b3cf2ada4584faa36773bd07bac24acfb39f3c431b36d7e667aa" +dependencies = [ + "thiserror-impl", +] + +[[package]] +name = "thiserror-impl" +version = "2.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f077553d607adc1caf65430528a576c757a71ed73944b66ebb58ef2bbd243568" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "tinytemplate" version = "1.2.1" diff --git a/Cargo.toml b/Cargo.toml index 5d124c68..2ccfc3f2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -55,11 +55,13 @@ rand_core = { version = "0.6", default-features = false } rand-utils = { version = "0.10", package = "winter-rand-utils", optional = true } serde = { version = "1.0", default-features = false, optional = true, features = ["derive"] } sha3 = { version = "0.10", default-features = false } +thiserror = { version = "2.0", default-features = false } winter-crypto = { version = "0.10", default-features = false } winter-math = { version = "0.10", default-features = false } winter-utils = { version = "0.10", default-features = false } [dev-dependencies] +assert_matches = { version = "1.5.0", default-features = false } criterion = { version = "0.5", features = ["html_reports"] } getrandom = { version = "0.2", features = ["js"] } hex = { version = "0.4", default-features = false, features = ["alloc"] } diff --git a/src/hash/rescue/rpo/digest.rs b/src/hash/rescue/rpo/digest.rs index 67d73e31..44663691 100644 --- a/src/hash/rescue/rpo/digest.rs +++ b/src/hash/rescue/rpo/digest.rs @@ -1,6 +1,8 @@ use alloc::string::String; use core::{cmp::Ordering, fmt::Display, ops::Deref, slice}; +use thiserror::Error; + use super::{Digest, Felt, StarkField, DIGEST_BYTES, DIGEST_SIZE, ZERO}; use crate::{ rand::Randomizable, @@ -127,9 +129,12 @@ impl Randomizable for RpoDigest { // CONVERSIONS: FROM RPO DIGEST // ================================================================================================ -#[derive(Copy, Clone, Debug)] +#[derive(Debug, Error)] pub enum RpoDigestError { - InvalidInteger, + #[error("failed to convert digest field element to {0}")] + TypeConversion(&'static str), + #[error("failed to convert to field element: {0}")] + InvalidFieldElement(String), } impl TryFrom<&RpoDigest> for [bool; DIGEST_SIZE] { @@ -153,10 +158,10 @@ impl TryFrom for [bool; DIGEST_SIZE] { } Ok([ - to_bool(value.0[0].as_int()).ok_or(RpoDigestError::InvalidInteger)?, - to_bool(value.0[1].as_int()).ok_or(RpoDigestError::InvalidInteger)?, - to_bool(value.0[2].as_int()).ok_or(RpoDigestError::InvalidInteger)?, - to_bool(value.0[3].as_int()).ok_or(RpoDigestError::InvalidInteger)?, + to_bool(value.0[0].as_int()).ok_or(RpoDigestError::TypeConversion("bool"))?, + to_bool(value.0[1].as_int()).ok_or(RpoDigestError::TypeConversion("bool"))?, + to_bool(value.0[2].as_int()).ok_or(RpoDigestError::TypeConversion("bool"))?, + to_bool(value.0[3].as_int()).ok_or(RpoDigestError::TypeConversion("bool"))?, ]) } } @@ -174,10 +179,22 @@ impl TryFrom for [u8; DIGEST_SIZE] { fn try_from(value: RpoDigest) -> Result { Ok([ - value.0[0].as_int().try_into().map_err(|_| RpoDigestError::InvalidInteger)?, - value.0[1].as_int().try_into().map_err(|_| RpoDigestError::InvalidInteger)?, - value.0[2].as_int().try_into().map_err(|_| RpoDigestError::InvalidInteger)?, - value.0[3].as_int().try_into().map_err(|_| RpoDigestError::InvalidInteger)?, + value.0[0] + .as_int() + .try_into() + .map_err(|_| RpoDigestError::TypeConversion("u8"))?, + value.0[1] + .as_int() + .try_into() + .map_err(|_| RpoDigestError::TypeConversion("u8"))?, + value.0[2] + .as_int() + .try_into() + .map_err(|_| RpoDigestError::TypeConversion("u8"))?, + value.0[3] + .as_int() + .try_into() + .map_err(|_| RpoDigestError::TypeConversion("u8"))?, ]) } } @@ -195,10 +212,22 @@ impl TryFrom for [u16; DIGEST_SIZE] { fn try_from(value: RpoDigest) -> Result { Ok([ - value.0[0].as_int().try_into().map_err(|_| RpoDigestError::InvalidInteger)?, - value.0[1].as_int().try_into().map_err(|_| RpoDigestError::InvalidInteger)?, - value.0[2].as_int().try_into().map_err(|_| RpoDigestError::InvalidInteger)?, - value.0[3].as_int().try_into().map_err(|_| RpoDigestError::InvalidInteger)?, + value.0[0] + .as_int() + .try_into() + .map_err(|_| RpoDigestError::TypeConversion("u16"))?, + value.0[1] + .as_int() + .try_into() + .map_err(|_| RpoDigestError::TypeConversion("u16"))?, + value.0[2] + .as_int() + .try_into() + .map_err(|_| RpoDigestError::TypeConversion("u16"))?, + value.0[3] + .as_int() + .try_into() + .map_err(|_| RpoDigestError::TypeConversion("u16"))?, ]) } } @@ -216,10 +245,22 @@ impl TryFrom for [u32; DIGEST_SIZE] { fn try_from(value: RpoDigest) -> Result { Ok([ - value.0[0].as_int().try_into().map_err(|_| RpoDigestError::InvalidInteger)?, - value.0[1].as_int().try_into().map_err(|_| RpoDigestError::InvalidInteger)?, - value.0[2].as_int().try_into().map_err(|_| RpoDigestError::InvalidInteger)?, - value.0[3].as_int().try_into().map_err(|_| RpoDigestError::InvalidInteger)?, + value.0[0] + .as_int() + .try_into() + .map_err(|_| RpoDigestError::TypeConversion("u32"))?, + value.0[1] + .as_int() + .try_into() + .map_err(|_| RpoDigestError::TypeConversion("u32"))?, + value.0[2] + .as_int() + .try_into() + .map_err(|_| RpoDigestError::TypeConversion("u32"))?, + value.0[3] + .as_int() + .try_into() + .map_err(|_| RpoDigestError::TypeConversion("u32"))?, ]) } } @@ -343,10 +384,10 @@ impl TryFrom<[u64; DIGEST_SIZE]> for RpoDigest { fn try_from(value: [u64; DIGEST_SIZE]) -> Result { Ok(Self([ - value[0].try_into().map_err(|_| RpoDigestError::InvalidInteger)?, - value[1].try_into().map_err(|_| RpoDigestError::InvalidInteger)?, - value[2].try_into().map_err(|_| RpoDigestError::InvalidInteger)?, - value[3].try_into().map_err(|_| RpoDigestError::InvalidInteger)?, + value[0].try_into().map_err(RpoDigestError::InvalidFieldElement)?, + value[1].try_into().map_err(RpoDigestError::InvalidFieldElement)?, + value[2].try_into().map_err(RpoDigestError::InvalidFieldElement)?, + value[3].try_into().map_err(RpoDigestError::InvalidFieldElement)?, ])) } } diff --git a/src/hash/rescue/rpx/digest.rs b/src/hash/rescue/rpx/digest.rs index 8f953fc6..07ec3c66 100644 --- a/src/hash/rescue/rpx/digest.rs +++ b/src/hash/rescue/rpx/digest.rs @@ -1,6 +1,8 @@ use alloc::string::String; use core::{cmp::Ordering, fmt::Display, ops::Deref, slice}; +use thiserror::Error; + use super::{Digest, Felt, StarkField, DIGEST_BYTES, DIGEST_SIZE, ZERO}; use crate::{ rand::Randomizable, @@ -127,9 +129,12 @@ impl Randomizable for RpxDigest { // CONVERSIONS: FROM RPX DIGEST // ================================================================================================ -#[derive(Copy, Clone, Debug)] +#[derive(Debug, Error)] pub enum RpxDigestError { - InvalidInteger, + #[error("failed to convert digest field element to {0}")] + TypeConversion(&'static str), + #[error("failed to convert to field element: {0}")] + InvalidFieldElement(String), } impl TryFrom<&RpxDigest> for [bool; DIGEST_SIZE] { @@ -153,10 +158,10 @@ impl TryFrom for [bool; DIGEST_SIZE] { } Ok([ - to_bool(value.0[0].as_int()).ok_or(RpxDigestError::InvalidInteger)?, - to_bool(value.0[1].as_int()).ok_or(RpxDigestError::InvalidInteger)?, - to_bool(value.0[2].as_int()).ok_or(RpxDigestError::InvalidInteger)?, - to_bool(value.0[3].as_int()).ok_or(RpxDigestError::InvalidInteger)?, + to_bool(value.0[0].as_int()).ok_or(RpxDigestError::TypeConversion("bool"))?, + to_bool(value.0[1].as_int()).ok_or(RpxDigestError::TypeConversion("bool"))?, + to_bool(value.0[2].as_int()).ok_or(RpxDigestError::TypeConversion("bool"))?, + to_bool(value.0[3].as_int()).ok_or(RpxDigestError::TypeConversion("bool"))?, ]) } } @@ -174,10 +179,22 @@ impl TryFrom for [u8; DIGEST_SIZE] { fn try_from(value: RpxDigest) -> Result { Ok([ - value.0[0].as_int().try_into().map_err(|_| RpxDigestError::InvalidInteger)?, - value.0[1].as_int().try_into().map_err(|_| RpxDigestError::InvalidInteger)?, - value.0[2].as_int().try_into().map_err(|_| RpxDigestError::InvalidInteger)?, - value.0[3].as_int().try_into().map_err(|_| RpxDigestError::InvalidInteger)?, + value.0[0] + .as_int() + .try_into() + .map_err(|_| RpxDigestError::TypeConversion("u8"))?, + value.0[1] + .as_int() + .try_into() + .map_err(|_| RpxDigestError::TypeConversion("u8"))?, + value.0[2] + .as_int() + .try_into() + .map_err(|_| RpxDigestError::TypeConversion("u8"))?, + value.0[3] + .as_int() + .try_into() + .map_err(|_| RpxDigestError::TypeConversion("u8"))?, ]) } } @@ -195,10 +212,22 @@ impl TryFrom for [u16; DIGEST_SIZE] { fn try_from(value: RpxDigest) -> Result { Ok([ - value.0[0].as_int().try_into().map_err(|_| RpxDigestError::InvalidInteger)?, - value.0[1].as_int().try_into().map_err(|_| RpxDigestError::InvalidInteger)?, - value.0[2].as_int().try_into().map_err(|_| RpxDigestError::InvalidInteger)?, - value.0[3].as_int().try_into().map_err(|_| RpxDigestError::InvalidInteger)?, + value.0[0] + .as_int() + .try_into() + .map_err(|_| RpxDigestError::TypeConversion("u16"))?, + value.0[1] + .as_int() + .try_into() + .map_err(|_| RpxDigestError::TypeConversion("u16"))?, + value.0[2] + .as_int() + .try_into() + .map_err(|_| RpxDigestError::TypeConversion("u16"))?, + value.0[3] + .as_int() + .try_into() + .map_err(|_| RpxDigestError::TypeConversion("u16"))?, ]) } } @@ -216,10 +245,22 @@ impl TryFrom for [u32; DIGEST_SIZE] { fn try_from(value: RpxDigest) -> Result { Ok([ - value.0[0].as_int().try_into().map_err(|_| RpxDigestError::InvalidInteger)?, - value.0[1].as_int().try_into().map_err(|_| RpxDigestError::InvalidInteger)?, - value.0[2].as_int().try_into().map_err(|_| RpxDigestError::InvalidInteger)?, - value.0[3].as_int().try_into().map_err(|_| RpxDigestError::InvalidInteger)?, + value.0[0] + .as_int() + .try_into() + .map_err(|_| RpxDigestError::TypeConversion("u32"))?, + value.0[1] + .as_int() + .try_into() + .map_err(|_| RpxDigestError::TypeConversion("u32"))?, + value.0[2] + .as_int() + .try_into() + .map_err(|_| RpxDigestError::TypeConversion("u32"))?, + value.0[3] + .as_int() + .try_into() + .map_err(|_| RpxDigestError::TypeConversion("u32"))?, ]) } } @@ -343,10 +384,10 @@ impl TryFrom<[u64; DIGEST_SIZE]> for RpxDigest { fn try_from(value: [u64; DIGEST_SIZE]) -> Result { Ok(Self([ - value[0].try_into().map_err(|_| RpxDigestError::InvalidInteger)?, - value[1].try_into().map_err(|_| RpxDigestError::InvalidInteger)?, - value[2].try_into().map_err(|_| RpxDigestError::InvalidInteger)?, - value[3].try_into().map_err(|_| RpxDigestError::InvalidInteger)?, + value[0].try_into().map_err(RpxDigestError::InvalidFieldElement)?, + value[1].try_into().map_err(RpxDigestError::InvalidFieldElement)?, + value[2].try_into().map_err(RpxDigestError::InvalidFieldElement)?, + value[3].try_into().map_err(RpxDigestError::InvalidFieldElement)?, ])) } } diff --git a/src/merkle/error.rs b/src/merkle/error.rs index 68e531ba..c7eff709 100644 --- a/src/merkle/error.rs +++ b/src/merkle/error.rs @@ -1,65 +1,34 @@ -use alloc::vec::Vec; -use core::fmt; +use thiserror::Error; -use super::{smt::SmtLeafError, MerklePath, NodeIndex, RpoDigest}; +use super::{NodeIndex, RpoDigest}; -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Debug, Error)] pub enum MerkleError { - ConflictingRoots(Vec), + #[error("expected merkle root {expected_root} found {actual_root}")] + ConflictingRoots { + expected_root: RpoDigest, + actual_root: RpoDigest, + }, + #[error("provided merkle tree depth {0} is too small")] DepthTooSmall(u8), + #[error("provided merkle tree depth {0} is too big")] DepthTooBig(u64), + #[error("multiple values provided for merkle tree index {0}")] DuplicateValuesForIndex(u64), - DuplicateValuesForKey(RpoDigest), - InvalidIndex { depth: u8, value: u64 }, - InvalidDepth { expected: u8, provided: u8 }, - InvalidSubtreeDepth { subtree_depth: u8, tree_depth: u8 }, - InvalidPath(MerklePath), - InvalidNumEntries(usize), - NodeNotInSet(NodeIndex), - NodeNotInStore(RpoDigest, NodeIndex), + #[error("node index value {value} is not valid for depth {depth}")] + InvalidNodeIndex { depth: u8, value: u64 }, + #[error("provided node index depth {provided} does not match expected depth {expected}")] + InvalidNodeIndexDepth { expected: u8, provided: u8 }, + #[error("merkle subtree depth {subtree_depth} exceeds merkle tree depth {tree_depth}")] + SubtreeDepthExceedsDepth { subtree_depth: u8, tree_depth: u8 }, + #[error("number of entries in the merkle tree exceeds the maximum of {0}")] + TooManyEntries(usize), + #[error("node index `{0}` not found in the tree")] + NodeIndexNotFoundInTree(NodeIndex), + #[error("node {0:?} with index `{1}` not found in the store")] + NodeIndexNotFoundInStore(RpoDigest, NodeIndex), + #[error("number of provided merkle tree leaves {0} is not a power of two")] NumLeavesNotPowerOfTwo(usize), + #[error("root {0:?} is not in the store")] RootNotInStore(RpoDigest), - SmtLeaf(SmtLeafError), -} - -impl fmt::Display for MerkleError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - use MerkleError::*; - match self { - ConflictingRoots(roots) => write!(f, "the merkle paths roots do not match {roots:?}"), - DepthTooSmall(depth) => write!(f, "the provided depth {depth} is too small"), - DepthTooBig(depth) => write!(f, "the provided depth {depth} is too big"), - DuplicateValuesForIndex(key) => write!(f, "multiple values provided for key {key}"), - DuplicateValuesForKey(key) => write!(f, "multiple values provided for key {key}"), - InvalidIndex { depth, value } => { - write!(f, "the index value {value} is not valid for the depth {depth}") - }, - InvalidDepth { expected, provided } => { - write!(f, "the provided depth {provided} is not valid for {expected}") - }, - InvalidSubtreeDepth { subtree_depth, tree_depth } => { - write!(f, "tried inserting a subtree of depth {subtree_depth} into a tree of depth {tree_depth}") - }, - InvalidPath(_path) => write!(f, "the provided path is not valid"), - InvalidNumEntries(max) => write!(f, "number of entries exceeded the maximum: {max}"), - NodeNotInSet(index) => write!(f, "the node with index ({index}) is not in the set"), - NodeNotInStore(hash, index) => { - write!(f, "the node {hash:?} with index ({index}) is not in the store") - }, - NumLeavesNotPowerOfTwo(leaves) => { - write!(f, "the leaves count {leaves} is not a power of 2") - }, - RootNotInStore(root) => write!(f, "the root {:?} is not in the store", root), - SmtLeaf(smt_leaf_error) => write!(f, "smt leaf error: {smt_leaf_error}"), - } - } -} - -#[cfg(feature = "std")] -impl std::error::Error for MerkleError {} - -impl From for MerkleError { - fn from(value: SmtLeafError) -> Self { - Self::SmtLeaf(value) - } } diff --git a/src/merkle/index.rs b/src/merkle/index.rs index 104ceb44..d4887b7d 100644 --- a/src/merkle/index.rs +++ b/src/merkle/index.rs @@ -38,7 +38,7 @@ impl NodeIndex { /// Returns an error if the `value` is greater than or equal to 2^{depth}. pub const fn new(depth: u8, value: u64) -> Result { if (64 - value.leading_zeros()) > depth as u32 { - Err(MerkleError::InvalidIndex { depth, value }) + Err(MerkleError::InvalidNodeIndex { depth, value }) } else { Ok(Self { depth, value }) } @@ -182,6 +182,7 @@ impl Deserializable for NodeIndex { #[cfg(test)] mod tests { + use assert_matches::assert_matches; use proptest::prelude::*; use super::*; @@ -190,19 +191,19 @@ mod tests { fn test_node_index_value_too_high() { assert_eq!(NodeIndex::new(0, 0).unwrap(), NodeIndex { depth: 0, value: 0 }); let err = NodeIndex::new(0, 1).unwrap_err(); - assert_eq!(err, MerkleError::InvalidIndex { depth: 0, value: 1 }); + assert_matches!(err, MerkleError::InvalidNodeIndex { depth: 0, value: 1 }); assert_eq!(NodeIndex::new(1, 1).unwrap(), NodeIndex { depth: 1, value: 1 }); let err = NodeIndex::new(1, 2).unwrap_err(); - assert_eq!(err, MerkleError::InvalidIndex { depth: 1, value: 2 }); + assert_matches!(err, MerkleError::InvalidNodeIndex { depth: 1, value: 2 }); assert_eq!(NodeIndex::new(2, 3).unwrap(), NodeIndex { depth: 2, value: 3 }); let err = NodeIndex::new(2, 4).unwrap_err(); - assert_eq!(err, MerkleError::InvalidIndex { depth: 2, value: 4 }); + assert_matches!(err, MerkleError::InvalidNodeIndex { depth: 2, value: 4 }); assert_eq!(NodeIndex::new(3, 7).unwrap(), NodeIndex { depth: 3, value: 7 }); let err = NodeIndex::new(3, 8).unwrap_err(); - assert_eq!(err, MerkleError::InvalidIndex { depth: 3, value: 8 }); + assert_matches!(err, MerkleError::InvalidNodeIndex { depth: 3, value: 8 }); } #[test] diff --git a/src/merkle/mmr/error.rs b/src/merkle/mmr/error.rs index 5bede2d3..f26cc5cb 100644 --- a/src/merkle/mmr/error.rs +++ b/src/merkle/mmr/error.rs @@ -1,41 +1,27 @@ -use core::fmt::{Display, Formatter}; -#[cfg(feature = "std")] -use std::error::Error; +use alloc::string::String; + +use thiserror::Error; use crate::merkle::MerkleError; -#[derive(Debug, PartialEq, Eq, Clone)] +#[derive(Debug, Error)] pub enum MmrError { - InvalidPosition(usize), - InvalidPeaks, - InvalidPeak, - PeakOutOfBounds(usize, usize), + #[error("mmr does not contain position {0}")] + PositionNotFound(usize), + #[error("mmr peaks are invalid: {0}")] + InvalidPeaks(String), + #[error( + "mmr peak does not match the computed merkle root of the provided authentication path" + )] + PeakPathMismatch, + #[error("requested peak index is {peak_idx} but the number of peaks is {peaks_len}")] + PeakOutOfBounds { peak_idx: usize, peaks_len: usize }, + #[error("invalid mmr update")] InvalidUpdate, - UnknownPeak, - MerkleError(MerkleError), -} - -impl Display for MmrError { - fn fmt(&self, fmt: &mut Formatter<'_>) -> Result<(), core::fmt::Error> { - match self { - MmrError::InvalidPosition(pos) => write!(fmt, "Mmr does not contain position {pos}"), - MmrError::InvalidPeaks => write!(fmt, "Invalid peaks count"), - MmrError::InvalidPeak => { - write!(fmt, "Peak values does not match merkle path computed root") - }, - MmrError::PeakOutOfBounds(peak_idx, peaks_len) => write!( - fmt, - "Requested peak index is {} but the number of peaks is {}", - peak_idx, peaks_len - ), - MmrError::InvalidUpdate => write!(fmt, "Invalid Mmr update"), - MmrError::UnknownPeak => { - write!(fmt, "Peak not in Mmr") - }, - MmrError::MerkleError(err) => write!(fmt, "{}", err), - } - } + #[error("mmr does not contain a peak with depth {0}")] + UnknownPeak(u8), + #[error("invalid merkle path")] + InvalidMerklePath(#[source] MerkleError), + #[error("merkle root computation failed")] + MerkleRootComputationFailed(#[source] MerkleError), } - -#[cfg(feature = "std")] -impl Error for MmrError {} diff --git a/src/merkle/mmr/full.rs b/src/merkle/mmr/full.rs index 036cd900..e381f5de 100644 --- a/src/merkle/mmr/full.rs +++ b/src/merkle/mmr/full.rs @@ -99,7 +99,7 @@ impl Mmr { pub fn open_at(&self, pos: usize, forest: usize) -> Result { // find the target tree responsible for the MMR position let tree_bit = - leaf_to_corresponding_tree(pos, forest).ok_or(MmrError::InvalidPosition(pos))?; + leaf_to_corresponding_tree(pos, forest).ok_or(MmrError::PositionNotFound(pos))?; // isolate the trees before the target let forest_before = forest & high_bitmask(tree_bit + 1); @@ -126,7 +126,7 @@ impl Mmr { pub fn get(&self, pos: usize) -> Result { // find the target tree responsible for the MMR position let tree_bit = - leaf_to_corresponding_tree(pos, self.forest).ok_or(MmrError::InvalidPosition(pos))?; + leaf_to_corresponding_tree(pos, self.forest).ok_or(MmrError::PositionNotFound(pos))?; // isolate the trees before the target let forest_before = self.forest & high_bitmask(tree_bit + 1); @@ -174,7 +174,10 @@ impl Mmr { /// Returns an error if the specified `forest` value is not valid for this MMR. pub fn peaks_at(&self, forest: usize) -> Result { if forest > self.forest { - return Err(MmrError::InvalidPeaks); + return Err(MmrError::InvalidPeaks(format!( + "requested forest {forest} exceeds current forest {}", + self.forest + ))); } let peaks: Vec = TrueBitPositionIterator::new(forest) @@ -199,7 +202,7 @@ impl Mmr { /// that have been merged together, followed by the new peaks of the [Mmr]. pub fn get_delta(&self, from_forest: usize, to_forest: usize) -> Result { if to_forest > self.forest || from_forest > to_forest { - return Err(MmrError::InvalidPeaks); + return Err(MmrError::InvalidPeaks(format!("to_forest {to_forest} exceeds the current forest {} or from_forest {from_forest} exceeds to_forest", self.forest))); } if from_forest == to_forest { diff --git a/src/merkle/mmr/partial.rs b/src/merkle/mmr/partial.rs index b2c49f92..5327548d 100644 --- a/src/merkle/mmr/partial.rs +++ b/src/merkle/mmr/partial.rs @@ -145,7 +145,7 @@ impl PartialMmr { /// in the underlying MMR. pub fn open(&self, pos: usize) -> Result, MmrError> { let tree_bit = - leaf_to_corresponding_tree(pos, self.forest).ok_or(MmrError::InvalidPosition(pos))?; + leaf_to_corresponding_tree(pos, self.forest).ok_or(MmrError::PositionNotFound(pos))?; let depth = tree_bit as usize; let mut nodes = Vec::with_capacity(depth); @@ -298,7 +298,7 @@ impl PartialMmr { // invalid. let tree = 1 << path.depth(); if tree & self.forest == 0 { - return Err(MmrError::UnknownPeak); + return Err(MmrError::UnknownPeak(path.depth())); }; if leaf_pos + 1 == self.forest @@ -319,9 +319,11 @@ impl PartialMmr { // Compute the root of the authentication path, and check it matches the current version of // the PartialMmr. - let computed = path.compute_root(path_idx as u64, leaf).map_err(MmrError::MerkleError)?; + let computed = path + .compute_root(path_idx as u64, leaf) + .map_err(MmrError::MerkleRootComputationFailed)?; if self.peaks[peak_pos] != computed { - return Err(MmrError::InvalidPeak); + return Err(MmrError::PeakPathMismatch); } let mut idx = InOrderIndex::from_leaf_pos(leaf_pos); @@ -356,7 +358,10 @@ impl PartialMmr { /// inserted into the partial MMR. pub fn apply(&mut self, delta: MmrDelta) -> Result, MmrError> { if delta.forest < self.forest { - return Err(MmrError::InvalidPeaks); + return Err(MmrError::InvalidPeaks(format!( + "forest of mmr delta {} is less than current forest {}", + delta.forest, self.forest + ))); } let mut inserted_nodes = Vec::new(); diff --git a/src/merkle/mmr/peaks.rs b/src/merkle/mmr/peaks.rs index ae39aafc..a640afda 100644 --- a/src/merkle/mmr/peaks.rs +++ b/src/merkle/mmr/peaks.rs @@ -45,7 +45,11 @@ impl MmrPeaks { /// Returns an error if the number of leaves and the number of peaks are inconsistent. pub fn new(num_leaves: usize, peaks: Vec) -> Result { if num_leaves.count_ones() as usize != peaks.len() { - return Err(MmrError::InvalidPeaks); + return Err(MmrError::InvalidPeaks(format!( + "number of one bits in leaves is {} which does not equal peak length {}", + num_leaves.count_ones(), + peaks.len() + ))); } Ok(Self { num_leaves, peaks }) @@ -77,7 +81,7 @@ impl MmrPeaks { pub fn get_peak(&self, peak_idx: usize) -> Result<&RpoDigest, MmrError> { self.peaks .get(peak_idx) - .ok_or(MmrError::PeakOutOfBounds(peak_idx, self.peaks.len())) + .ok_or(MmrError::PeakOutOfBounds { peak_idx, peaks_len: self.peaks.len() }) } /// Converts this [MmrPeaks] into its components: number of leaves and a vector of peaks of @@ -106,7 +110,7 @@ impl MmrPeaks { opening .merkle_path .verify(opening.relative_pos() as u64, value, root) - .map_err(MmrError::MerkleError) + .map_err(MmrError::InvalidMerklePath) } /// Flattens and pads the peaks to make hashing inside of the Miden VM easier. diff --git a/src/merkle/partial_mt/mod.rs b/src/merkle/partial_mt/mod.rs index ef464327..ae4a04ff 100644 --- a/src/merkle/partial_mt/mod.rs +++ b/src/merkle/partial_mt/mod.rs @@ -116,7 +116,7 @@ impl PartialMerkleTree { // depth of 63 because we consider passing in a vector of size 2^64 infeasible. let max = 2usize.pow(63); if layers.len() > max { - return Err(MerkleError::InvalidNumEntries(max)); + return Err(MerkleError::TooManyEntries(max)); } // Get maximum depth @@ -147,11 +147,12 @@ impl PartialMerkleTree { let index = NodeIndex::new(depth, index_value)?; // get hash of the current node - let node = nodes.get(&index).ok_or(MerkleError::NodeNotInSet(index))?; + let node = + nodes.get(&index).ok_or(MerkleError::NodeIndexNotFoundInTree(index))?; // get hash of the sibling node let sibling = nodes .get(&index.sibling()) - .ok_or(MerkleError::NodeNotInSet(index.sibling()))?; + .ok_or(MerkleError::NodeIndexNotFoundInTree(index.sibling()))?; // get parent hash let parent = Rpo256::merge(&index.build_node(*node, *sibling)); @@ -184,7 +185,10 @@ impl PartialMerkleTree { /// # Errors /// Returns an error if the specified NodeIndex is not contained in the nodes map. pub fn get_node(&self, index: NodeIndex) -> Result { - self.nodes.get(&index).ok_or(MerkleError::NodeNotInSet(index)).copied() + self.nodes + .get(&index) + .ok_or(MerkleError::NodeIndexNotFoundInTree(index)) + .copied() } /// Returns true if provided index contains in the leaves set, false otherwise. @@ -224,7 +228,7 @@ impl PartialMerkleTree { } if !self.nodes.contains_key(&index) { - return Err(MerkleError::NodeNotInSet(index)); + return Err(MerkleError::NodeIndexNotFoundInTree(index)); } let mut path = Vec::new(); @@ -335,15 +339,16 @@ impl PartialMerkleTree { if self.root() == EMPTY_DIGEST { self.nodes.insert(ROOT_INDEX, root); } else if self.root() != root { - return Err(MerkleError::ConflictingRoots([self.root(), root].to_vec())); + return Err(MerkleError::ConflictingRoots { + expected_root: self.root(), + actual_root: root, + }); } Ok(()) } /// Updates value of the leaf at the specified index returning the old leaf value. - /// By default the specified index is assumed to belong to the deepest layer. If the considered - /// node does not belong to the tree, the first node on the way to the root will be changed. /// /// By default the specified index is assumed to belong to the deepest layer. If the considered /// node does not belong to the tree, the first node on the way to the root will be changed. @@ -352,6 +357,7 @@ impl PartialMerkleTree { /// /// # Errors /// Returns an error if: + /// - No entry exists at the specified index. /// - The specified index is greater than the maximum number of nodes on the deepest layer. pub fn update_leaf(&mut self, index: u64, value: Word) -> Result { let mut node_index = NodeIndex::new(self.max_depth(), index)?; @@ -367,7 +373,7 @@ impl PartialMerkleTree { let old_value = self .nodes .insert(node_index, value.into()) - .ok_or(MerkleError::NodeNotInSet(node_index))?; + .ok_or(MerkleError::NodeIndexNotFoundInTree(node_index))?; // if the old value and new value are the same, there is nothing to update if value == *old_value { diff --git a/src/merkle/path.rs b/src/merkle/path.rs index 37dd66bc..7e3f02ea 100644 --- a/src/merkle/path.rs +++ b/src/merkle/path.rs @@ -61,7 +61,10 @@ impl MerklePath { pub fn verify(&self, index: u64, node: RpoDigest, root: &RpoDigest) -> Result<(), MerkleError> { let computed_root = self.compute_root(index, node)?; if &computed_root != root { - return Err(MerkleError::ConflictingRoots(vec![computed_root, *root])); + return Err(MerkleError::ConflictingRoots { + expected_root: *root, + actual_root: computed_root, + }); } Ok(()) diff --git a/src/merkle/smt/full/error.rs b/src/merkle/smt/full/error.rs index 805144a3..7144d45d 100644 --- a/src/merkle/smt/full/error.rs +++ b/src/merkle/smt/full/error.rs @@ -1,86 +1,39 @@ -use alloc::vec::Vec; -use core::fmt; +use thiserror::Error; use crate::{ hash::rpo::RpoDigest, merkle::{LeafIndex, SMT_DEPTH}, - Word, }; // SMT LEAF ERROR // ================================================================================================= -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Debug, Error)] pub enum SmtLeafError { - InconsistentKeys { - entries: Vec<(RpoDigest, Word)>, - key_1: RpoDigest, - key_2: RpoDigest, - }, - InvalidNumEntriesForMultiple(usize), - SingleKeyInconsistentWithLeafIndex { + #[error( + "multiple leaf requires all keys to map to the same leaf index but key1 {key_1} and key2 {key_2} map to different indices" + )] + InconsistentMultipleLeafKeys { key_1: RpoDigest, key_2: RpoDigest }, + #[error("single leaf key {key} maps to {actual_leaf_index:?} but was expected to map to {expected_leaf_index:?}")] + InconsistentSingleLeafIndices { key: RpoDigest, - leaf_index: LeafIndex, + expected_leaf_index: LeafIndex, + actual_leaf_index: LeafIndex, }, - MultipleKeysInconsistentWithLeafIndex { + #[error("supplied leaf index {leaf_index_supplied:?} does not match {leaf_index_from_keys:?} for multiple leaf")] + InconsistentMultipleLeafIndices { leaf_index_from_keys: LeafIndex, leaf_index_supplied: LeafIndex, }, -} - -#[cfg(feature = "std")] -impl std::error::Error for SmtLeafError {} - -impl fmt::Display for SmtLeafError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - use SmtLeafError::*; - match self { - InvalidNumEntriesForMultiple(num_entries) => { - write!(f, "Multiple leaf requires 2 or more entries. Got: {num_entries}") - }, - InconsistentKeys { entries, key_1, key_2 } => { - write!(f, "Multiple leaf requires all keys to map to the same leaf index. Offending keys: {key_1} and {key_2}. Entries: {entries:?}.") - }, - SingleKeyInconsistentWithLeafIndex { key, leaf_index } => { - write!( - f, - "Single key in leaf inconsistent with leaf index. Key: {key}, leaf index: {}", - leaf_index.value() - ) - }, - MultipleKeysInconsistentWithLeafIndex { - leaf_index_from_keys, - leaf_index_supplied, - } => { - write!( - f, - "Keys in entries map to leaf index {}, but leaf index {} was supplied", - leaf_index_from_keys.value(), - leaf_index_supplied.value() - ) - }, - } - } + #[error("multiple leaf requires at least two entries but only {0} were given")] + MultipleLeafRequiresTwoEntries(usize), } // SMT PROOF ERROR // ================================================================================================= -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Debug, Error)] pub enum SmtProofError { - InvalidPathLength(usize), -} - -#[cfg(feature = "std")] -impl std::error::Error for SmtProofError {} - -impl fmt::Display for SmtProofError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - use SmtProofError::*; - match self { - InvalidPathLength(path_length) => { - write!(f, "Invalid Merkle path length. Expected {SMT_DEPTH}, got {path_length}") - }, - } - } + #[error("merkle path length {0} does not match SMT depth {SMT_DEPTH}")] + InvalidMerklePathLength(usize), } diff --git a/src/merkle/smt/full/leaf.rs b/src/merkle/smt/full/leaf.rs index 585fc407..ed3e6b7d 100644 --- a/src/merkle/smt/full/leaf.rs +++ b/src/merkle/smt/full/leaf.rs @@ -31,10 +31,12 @@ impl SmtLeaf { 1 => { let (key, value) = entries[0]; - if LeafIndex::::from(key) != leaf_index { - return Err(SmtLeafError::SingleKeyInconsistentWithLeafIndex { + let computed_index = LeafIndex::::from(key); + if computed_index != leaf_index { + return Err(SmtLeafError::InconsistentSingleLeafIndices { key, - leaf_index, + expected_leaf_index: leaf_index, + actual_leaf_index: computed_index, }); } @@ -46,7 +48,7 @@ impl SmtLeaf { // `new_multiple()` checked that all keys map to the same leaf index. We still need // to ensure that that leaf index is `leaf_index`. if leaf.index() != leaf_index { - Err(SmtLeafError::MultipleKeysInconsistentWithLeafIndex { + Err(SmtLeafError::InconsistentMultipleLeafIndices { leaf_index_from_keys: leaf.index(), leaf_index_supplied: leaf_index, }) @@ -75,7 +77,7 @@ impl SmtLeaf { /// - Returns an error if 2 keys in `entries` map to a different leaf index pub fn new_multiple(entries: Vec<(RpoDigest, Word)>) -> Result { if entries.len() < 2 { - return Err(SmtLeafError::InvalidNumEntriesForMultiple(entries.len())); + return Err(SmtLeafError::MultipleLeafRequiresTwoEntries(entries.len())); } // Check that all keys map to the same leaf index @@ -89,8 +91,7 @@ impl SmtLeaf { let next_leaf_index: LeafIndex = next_key.into(); if next_leaf_index != first_leaf_index { - return Err(SmtLeafError::InconsistentKeys { - entries, + return Err(SmtLeafError::InconsistentMultipleLeafKeys { key_1: first_key, key_2: next_key, }); diff --git a/src/merkle/smt/full/proof.rs b/src/merkle/smt/full/proof.rs index c23b8b4a..84554885 100644 --- a/src/merkle/smt/full/proof.rs +++ b/src/merkle/smt/full/proof.rs @@ -25,7 +25,7 @@ impl SmtProof { pub fn new(path: MerklePath, leaf: SmtLeaf) -> Result { let depth: usize = SMT_DEPTH.into(); if path.len() != depth { - return Err(SmtProofError::InvalidPathLength(path.len())); + return Err(SmtProofError::InvalidMerklePathLength(path.len())); } Ok(Self { path, leaf }) diff --git a/src/merkle/smt/mod.rs b/src/merkle/smt/mod.rs index 056c221c..71d68667 100644 --- a/src/merkle/smt/mod.rs +++ b/src/merkle/smt/mod.rs @@ -267,7 +267,10 @@ pub(crate) trait SparseMerkleTree { // Guard against accidentally trying to apply mutations that were computed against a // different tree, including a stale version of this tree. if old_root != self.root() { - return Err(MerkleError::ConflictingRoots(vec![old_root, self.root()])); + return Err(MerkleError::ConflictingRoots { + expected_root: self.root(), + actual_root: old_root, + }); } for (index, mutation) in node_mutations { @@ -403,7 +406,7 @@ impl TryFrom for LeafIndex { fn try_from(node_index: NodeIndex) -> Result { if node_index.depth() != DEPTH { - return Err(MerkleError::InvalidDepth { + return Err(MerkleError::InvalidNodeIndexDepth { expected: DEPTH, provided: node_index.depth(), }); diff --git a/src/merkle/smt/simple/mod.rs b/src/merkle/smt/simple/mod.rs index 6229ac25..b52aac4b 100644 --- a/src/merkle/smt/simple/mod.rs +++ b/src/merkle/smt/simple/mod.rs @@ -81,7 +81,7 @@ impl SimpleSmt { for (idx, (key, value)) in entries.into_iter().enumerate() { if idx >= max_num_entries { - return Err(MerkleError::InvalidNumEntries(max_num_entries)); + return Err(MerkleError::TooManyEntries(max_num_entries)); } let old_value = tree.insert(LeafIndex::::new(key)?, value); @@ -246,7 +246,7 @@ impl SimpleSmt { subtree: SimpleSmt, ) -> Result { if SUBTREE_DEPTH > DEPTH { - return Err(MerkleError::InvalidSubtreeDepth { + return Err(MerkleError::SubtreeDepthExceedsDepth { subtree_depth: SUBTREE_DEPTH, tree_depth: DEPTH, }); diff --git a/src/merkle/smt/simple/tests.rs b/src/merkle/smt/simple/tests.rs index b1dd28d7..84bad47f 100644 --- a/src/merkle/smt/simple/tests.rs +++ b/src/merkle/smt/simple/tests.rs @@ -1,5 +1,7 @@ use alloc::vec::Vec; +use assert_matches::assert_matches; + use super::{ super::{MerkleError, RpoDigest, SimpleSmt}, NodeIndex, @@ -257,12 +259,12 @@ fn test_simplesmt_fail_on_duplicates() { // consecutive let entries = [(1, *first), (1, *second)]; let smt = SimpleSmt::<64>::with_leaves(entries); - assert_eq!(smt.unwrap_err(), MerkleError::DuplicateValuesForIndex(1)); + assert_matches!(smt.unwrap_err(), MerkleError::DuplicateValuesForIndex(1)); // not consecutive let entries = [(1, *first), (5, int_to_leaf(5)), (1, *second)]; let smt = SimpleSmt::<64>::with_leaves(entries); - assert_eq!(smt.unwrap_err(), MerkleError::DuplicateValuesForIndex(1)); + assert_matches!(smt.unwrap_err(), MerkleError::DuplicateValuesForIndex(1)); } } diff --git a/src/merkle/store/mod.rs b/src/merkle/store/mod.rs index bc7a63f2..f89f7398 100644 --- a/src/merkle/store/mod.rs +++ b/src/merkle/store/mod.rs @@ -136,7 +136,10 @@ impl> MerkleStore { self.nodes.get(&hash).ok_or(MerkleError::RootNotInStore(hash))?; for i in (0..index.depth()).rev() { - let node = self.nodes.get(&hash).ok_or(MerkleError::NodeNotInStore(hash, index))?; + let node = self + .nodes + .get(&hash) + .ok_or(MerkleError::NodeIndexNotFoundInStore(hash, index))?; let bit = (index.value() >> i) & 1; hash = if bit == 0 { node.left } else { node.right } @@ -162,7 +165,10 @@ impl> MerkleStore { self.nodes.get(&hash).ok_or(MerkleError::RootNotInStore(hash))?; for i in (0..index.depth()).rev() { - let node = self.nodes.get(&hash).ok_or(MerkleError::NodeNotInStore(hash, index))?; + let node = self + .nodes + .get(&hash) + .ok_or(MerkleError::NodeIndexNotFoundInStore(hash, index))?; let bit = (index.value() >> i) & 1; hash = if bit == 0 { diff --git a/src/merkle/store/tests.rs b/src/merkle/store/tests.rs index fe9ee0f0..3c94dc0a 100644 --- a/src/merkle/store/tests.rs +++ b/src/merkle/store/tests.rs @@ -1,3 +1,4 @@ +use assert_matches::assert_matches; use seq_macro::seq; #[cfg(feature = "std")] use { @@ -42,14 +43,14 @@ const VALUES8: [RpoDigest; 8] = [ fn test_root_not_in_store() -> Result<(), MerkleError> { let mtree = MerkleTree::new(digests_to_words(&VALUES4))?; let store = MerkleStore::from(&mtree); - assert_eq!( + assert_matches!( store.get_node(VALUES4[0], NodeIndex::make(mtree.depth(), 0)), - Err(MerkleError::RootNotInStore(VALUES4[0])), + Err(MerkleError::RootNotInStore(root)) if root == VALUES4[0], "Leaf 0 is not a root" ); - assert_eq!( + assert_matches!( store.get_path(VALUES4[0], NodeIndex::make(mtree.depth(), 0)), - Err(MerkleError::RootNotInStore(VALUES4[0])), + Err(MerkleError::RootNotInStore(root)) if root == VALUES4[0], "Leaf 0 is not a root" ); @@ -64,46 +65,46 @@ fn test_merkle_tree() -> Result<(), MerkleError> { // STORE LEAVES ARE CORRECT ------------------------------------------------------------------- // checks the leaves in the store corresponds to the expected values assert_eq!( - store.get_node(mtree.root(), NodeIndex::make(mtree.depth(), 0)), - Ok(VALUES4[0]), + store.get_node(mtree.root(), NodeIndex::make(mtree.depth(), 0)).unwrap(), + VALUES4[0], "node 0 must be in the tree" ); assert_eq!( - store.get_node(mtree.root(), NodeIndex::make(mtree.depth(), 1)), - Ok(VALUES4[1]), + store.get_node(mtree.root(), NodeIndex::make(mtree.depth(), 1)).unwrap(), + VALUES4[1], "node 1 must be in the tree" ); assert_eq!( - store.get_node(mtree.root(), NodeIndex::make(mtree.depth(), 2)), - Ok(VALUES4[2]), + store.get_node(mtree.root(), NodeIndex::make(mtree.depth(), 2)).unwrap(), + VALUES4[2], "node 2 must be in the tree" ); assert_eq!( - store.get_node(mtree.root(), NodeIndex::make(mtree.depth(), 3)), - Ok(VALUES4[3]), + store.get_node(mtree.root(), NodeIndex::make(mtree.depth(), 3)).unwrap(), + VALUES4[3], "node 3 must be in the tree" ); // STORE LEAVES MATCH TREE -------------------------------------------------------------------- // sanity check the values returned by the store and the tree assert_eq!( - mtree.get_node(NodeIndex::make(mtree.depth(), 0)), - store.get_node(mtree.root(), NodeIndex::make(mtree.depth(), 0)), + mtree.get_node(NodeIndex::make(mtree.depth(), 0)).unwrap(), + store.get_node(mtree.root(), NodeIndex::make(mtree.depth(), 0)).unwrap(), "node 0 must be the same for both MerkleTree and MerkleStore" ); assert_eq!( - mtree.get_node(NodeIndex::make(mtree.depth(), 1)), - store.get_node(mtree.root(), NodeIndex::make(mtree.depth(), 1)), + mtree.get_node(NodeIndex::make(mtree.depth(), 1)).unwrap(), + store.get_node(mtree.root(), NodeIndex::make(mtree.depth(), 1)).unwrap(), "node 1 must be the same for both MerkleTree and MerkleStore" ); assert_eq!( - mtree.get_node(NodeIndex::make(mtree.depth(), 2)), - store.get_node(mtree.root(), NodeIndex::make(mtree.depth(), 2)), + mtree.get_node(NodeIndex::make(mtree.depth(), 2)).unwrap(), + store.get_node(mtree.root(), NodeIndex::make(mtree.depth(), 2)).unwrap(), "node 2 must be the same for both MerkleTree and MerkleStore" ); assert_eq!( - mtree.get_node(NodeIndex::make(mtree.depth(), 3)), - store.get_node(mtree.root(), NodeIndex::make(mtree.depth(), 3)), + mtree.get_node(NodeIndex::make(mtree.depth(), 3)).unwrap(), + store.get_node(mtree.root(), NodeIndex::make(mtree.depth(), 3)).unwrap(), "node 3 must be the same for both MerkleTree and MerkleStore" ); @@ -115,8 +116,8 @@ fn test_merkle_tree() -> Result<(), MerkleError> { "Value for merkle path at index 0 must match leaf value" ); assert_eq!( - mtree.get_path(NodeIndex::make(mtree.depth(), 0)), - Ok(result.path), + mtree.get_path(NodeIndex::make(mtree.depth(), 0)).unwrap(), + result.path, "merkle path for index 0 must be the same for the MerkleTree and MerkleStore" ); @@ -126,8 +127,8 @@ fn test_merkle_tree() -> Result<(), MerkleError> { "Value for merkle path at index 0 must match leaf value" ); assert_eq!( - mtree.get_path(NodeIndex::make(mtree.depth(), 1)), - Ok(result.path), + mtree.get_path(NodeIndex::make(mtree.depth(), 1)).unwrap(), + result.path, "merkle path for index 1 must be the same for the MerkleTree and MerkleStore" ); @@ -137,8 +138,8 @@ fn test_merkle_tree() -> Result<(), MerkleError> { "Value for merkle path at index 0 must match leaf value" ); assert_eq!( - mtree.get_path(NodeIndex::make(mtree.depth(), 2)), - Ok(result.path), + mtree.get_path(NodeIndex::make(mtree.depth(), 2)).unwrap(), + result.path, "merkle path for index 0 must be the same for the MerkleTree and MerkleStore" ); @@ -148,8 +149,8 @@ fn test_merkle_tree() -> Result<(), MerkleError> { "Value for merkle path at index 0 must match leaf value" ); assert_eq!( - mtree.get_path(NodeIndex::make(mtree.depth(), 3)), - Ok(result.path), + mtree.get_path(NodeIndex::make(mtree.depth(), 3)).unwrap(), + result.path, "merkle path for index 0 must be the same for the MerkleTree and MerkleStore" ); @@ -240,56 +241,56 @@ fn test_sparse_merkle_tree() -> Result<(), MerkleError> { // STORE LEAVES ARE CORRECT ============================================================== // checks the leaves in the store corresponds to the expected values assert_eq!( - store.get_node(smt.root(), NodeIndex::make(SMT_MAX_DEPTH, 0)), - Ok(VALUES4[0]), + store.get_node(smt.root(), NodeIndex::make(SMT_MAX_DEPTH, 0)).unwrap(), + VALUES4[0], "node 0 must be in the tree" ); assert_eq!( - store.get_node(smt.root(), NodeIndex::make(SMT_MAX_DEPTH, 1)), - Ok(VALUES4[1]), + store.get_node(smt.root(), NodeIndex::make(SMT_MAX_DEPTH, 1)).unwrap(), + VALUES4[1], "node 1 must be in the tree" ); assert_eq!( - store.get_node(smt.root(), NodeIndex::make(SMT_MAX_DEPTH, 2)), - Ok(VALUES4[2]), + store.get_node(smt.root(), NodeIndex::make(SMT_MAX_DEPTH, 2)).unwrap(), + VALUES4[2], "node 2 must be in the tree" ); assert_eq!( - store.get_node(smt.root(), NodeIndex::make(SMT_MAX_DEPTH, 3)), - Ok(VALUES4[3]), + store.get_node(smt.root(), NodeIndex::make(SMT_MAX_DEPTH, 3)).unwrap(), + VALUES4[3], "node 3 must be in the tree" ); assert_eq!( - store.get_node(smt.root(), NodeIndex::make(SMT_MAX_DEPTH, 4)), - Ok(RpoDigest::default()), + store.get_node(smt.root(), NodeIndex::make(SMT_MAX_DEPTH, 4)).unwrap(), + RpoDigest::default(), "unmodified node 4 must be ZERO" ); // STORE LEAVES MATCH TREE =============================================================== // sanity check the values returned by the store and the tree assert_eq!( - smt.get_node(NodeIndex::make(SMT_MAX_DEPTH, 0)), - store.get_node(smt.root(), NodeIndex::make(SMT_MAX_DEPTH, 0)), + smt.get_node(NodeIndex::make(SMT_MAX_DEPTH, 0)).unwrap(), + store.get_node(smt.root(), NodeIndex::make(SMT_MAX_DEPTH, 0)).unwrap(), "node 0 must be the same for both SparseMerkleTree and MerkleStore" ); assert_eq!( - smt.get_node(NodeIndex::make(SMT_MAX_DEPTH, 1)), - store.get_node(smt.root(), NodeIndex::make(SMT_MAX_DEPTH, 1)), + smt.get_node(NodeIndex::make(SMT_MAX_DEPTH, 1)).unwrap(), + store.get_node(smt.root(), NodeIndex::make(SMT_MAX_DEPTH, 1)).unwrap(), "node 1 must be the same for both SparseMerkleTree and MerkleStore" ); assert_eq!( - smt.get_node(NodeIndex::make(SMT_MAX_DEPTH, 2)), - store.get_node(smt.root(), NodeIndex::make(SMT_MAX_DEPTH, 2)), + smt.get_node(NodeIndex::make(SMT_MAX_DEPTH, 2)).unwrap(), + store.get_node(smt.root(), NodeIndex::make(SMT_MAX_DEPTH, 2)).unwrap(), "node 2 must be the same for both SparseMerkleTree and MerkleStore" ); assert_eq!( - smt.get_node(NodeIndex::make(SMT_MAX_DEPTH, 3)), - store.get_node(smt.root(), NodeIndex::make(SMT_MAX_DEPTH, 3)), + smt.get_node(NodeIndex::make(SMT_MAX_DEPTH, 3)).unwrap(), + store.get_node(smt.root(), NodeIndex::make(SMT_MAX_DEPTH, 3)).unwrap(), "node 3 must be the same for both SparseMerkleTree and MerkleStore" ); assert_eq!( - smt.get_node(NodeIndex::make(SMT_MAX_DEPTH, 4)), - store.get_node(smt.root(), NodeIndex::make(SMT_MAX_DEPTH, 4)), + smt.get_node(NodeIndex::make(SMT_MAX_DEPTH, 4)).unwrap(), + store.get_node(smt.root(), NodeIndex::make(SMT_MAX_DEPTH, 4)).unwrap(), "node 4 must be the same for both SparseMerkleTree and MerkleStore" ); @@ -385,46 +386,46 @@ fn test_add_merkle_paths() -> Result<(), MerkleError> { // STORE LEAVES ARE CORRECT ============================================================== // checks the leaves in the store corresponds to the expected values assert_eq!( - store.get_node(pmt.root(), NodeIndex::make(pmt.max_depth(), 0)), - Ok(VALUES4[0]), + store.get_node(pmt.root(), NodeIndex::make(pmt.max_depth(), 0)).unwrap(), + VALUES4[0], "node 0 must be in the pmt" ); assert_eq!( - store.get_node(pmt.root(), NodeIndex::make(pmt.max_depth(), 1)), - Ok(VALUES4[1]), + store.get_node(pmt.root(), NodeIndex::make(pmt.max_depth(), 1)).unwrap(), + VALUES4[1], "node 1 must be in the pmt" ); assert_eq!( - store.get_node(pmt.root(), NodeIndex::make(pmt.max_depth(), 2)), - Ok(VALUES4[2]), + store.get_node(pmt.root(), NodeIndex::make(pmt.max_depth(), 2)).unwrap(), + VALUES4[2], "node 2 must be in the pmt" ); assert_eq!( - store.get_node(pmt.root(), NodeIndex::make(pmt.max_depth(), 3)), - Ok(VALUES4[3]), + store.get_node(pmt.root(), NodeIndex::make(pmt.max_depth(), 3)).unwrap(), + VALUES4[3], "node 3 must be in the pmt" ); // STORE LEAVES MATCH PMT ================================================================ // sanity check the values returned by the store and the pmt assert_eq!( - pmt.get_node(NodeIndex::make(pmt.max_depth(), 0)), - store.get_node(pmt.root(), NodeIndex::make(pmt.max_depth(), 0)), + pmt.get_node(NodeIndex::make(pmt.max_depth(), 0)).unwrap(), + store.get_node(pmt.root(), NodeIndex::make(pmt.max_depth(), 0)).unwrap(), "node 0 must be the same for both PartialMerkleTree and MerkleStore" ); assert_eq!( - pmt.get_node(NodeIndex::make(pmt.max_depth(), 1)), - store.get_node(pmt.root(), NodeIndex::make(pmt.max_depth(), 1)), + pmt.get_node(NodeIndex::make(pmt.max_depth(), 1)).unwrap(), + store.get_node(pmt.root(), NodeIndex::make(pmt.max_depth(), 1)).unwrap(), "node 1 must be the same for both PartialMerkleTree and MerkleStore" ); assert_eq!( - pmt.get_node(NodeIndex::make(pmt.max_depth(), 2)), - store.get_node(pmt.root(), NodeIndex::make(pmt.max_depth(), 2)), + pmt.get_node(NodeIndex::make(pmt.max_depth(), 2)).unwrap(), + store.get_node(pmt.root(), NodeIndex::make(pmt.max_depth(), 2)).unwrap(), "node 2 must be the same for both PartialMerkleTree and MerkleStore" ); assert_eq!( - pmt.get_node(NodeIndex::make(pmt.max_depth(), 3)), - store.get_node(pmt.root(), NodeIndex::make(pmt.max_depth(), 3)), + pmt.get_node(NodeIndex::make(pmt.max_depth(), 3)).unwrap(), + store.get_node(pmt.root(), NodeIndex::make(pmt.max_depth(), 3)).unwrap(), "node 3 must be the same for both PartialMerkleTree and MerkleStore" ); @@ -436,8 +437,8 @@ fn test_add_merkle_paths() -> Result<(), MerkleError> { "Value for merkle path at index 0 must match leaf value" ); assert_eq!( - pmt.get_path(NodeIndex::make(pmt.max_depth(), 0)), - Ok(result.path), + pmt.get_path(NodeIndex::make(pmt.max_depth(), 0)).unwrap(), + result.path, "merkle path for index 0 must be the same for the MerkleTree and MerkleStore" ); @@ -447,8 +448,8 @@ fn test_add_merkle_paths() -> Result<(), MerkleError> { "Value for merkle path at index 0 must match leaf value" ); assert_eq!( - pmt.get_path(NodeIndex::make(pmt.max_depth(), 1)), - Ok(result.path), + pmt.get_path(NodeIndex::make(pmt.max_depth(), 1)).unwrap(), + result.path, "merkle path for index 1 must be the same for the MerkleTree and MerkleStore" ); @@ -458,8 +459,8 @@ fn test_add_merkle_paths() -> Result<(), MerkleError> { "Value for merkle path at index 0 must match leaf value" ); assert_eq!( - pmt.get_path(NodeIndex::make(pmt.max_depth(), 2)), - Ok(result.path), + pmt.get_path(NodeIndex::make(pmt.max_depth(), 2)).unwrap(), + result.path, "merkle path for index 0 must be the same for the MerkleTree and MerkleStore" ); @@ -469,8 +470,8 @@ fn test_add_merkle_paths() -> Result<(), MerkleError> { "Value for merkle path at index 0 must match leaf value" ); assert_eq!( - pmt.get_path(NodeIndex::make(pmt.max_depth(), 3)), - Ok(result.path), + pmt.get_path(NodeIndex::make(pmt.max_depth(), 3)).unwrap(), + result.path, "merkle path for index 0 must be the same for the MerkleTree and MerkleStore" ); @@ -498,7 +499,7 @@ fn wont_open_to_different_depth_root() { let store = MerkleStore::from(&mtree); let index = NodeIndex::root(); let err = store.get_node(root, index).err().unwrap(); - assert_eq!(err, MerkleError::RootNotInStore(root)); + assert_matches!(err, MerkleError::RootNotInStore(err_root) if err_root == root); } #[test] @@ -537,7 +538,7 @@ fn test_set_node() -> Result<(), MerkleError> { let value = int_to_node(42); let index = NodeIndex::make(mtree.depth(), 0); let new_root = store.set_node(mtree.root(), index, value)?.root; - assert_eq!(store.get_node(new_root, index), Ok(value), "Value must have changed"); + assert_eq!(store.get_node(new_root, index).unwrap(), value, "value must have changed"); Ok(()) } @@ -745,7 +746,7 @@ fn get_leaf_depth_works_with_depth_8() { // duplicate the tree on `a` and assert the depth is short-circuited by such sub-tree let index = NodeIndex::new(8, a).unwrap(); root = store.set_node(root, index, root).unwrap().root; - assert_eq!(Err(MerkleError::DepthTooBig(9)), store.get_leaf_depth(root, 8, a)); + assert_matches!(store.get_leaf_depth(root, 8, a).unwrap_err(), MerkleError::DepthTooBig(9)); } #[test] diff --git a/src/utils/mod.rs b/src/utils/mod.rs index bf72ef9c..f9da5610 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -1,7 +1,9 @@ //! Utilities used in this crate which can also be generally useful downstream. use alloc::string::String; -use core::fmt::{self, Display, Write}; +use core::fmt::{self, Write}; + +use thiserror::Error; use super::Word; @@ -46,36 +48,20 @@ pub fn bytes_to_hex_string(data: [u8; N]) -> String { } /// Defines errors which can occur during parsing of hexadecimal strings. -#[derive(Debug)] +#[derive(Debug, Error)] pub enum HexParseError { + #[error( + "expected hex data to have length {expected}, including the 0x prefix, found {actual}" + )] InvalidLength { expected: usize, actual: usize }, + #[error("hex encoded data must start with 0x prefix")] MissingPrefix, + #[error("hex encoded data must contain only characters [a-zA-Z0-9]")] InvalidChar, + #[error("hex encoded values of a Digest must be inside the field modulus")] OutOfRange, } -impl Display for HexParseError { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - match self { - HexParseError::InvalidLength { expected, actual } => { - write!(f, "Expected hex data to have length {expected}, including the 0x prefix. Got {actual}") - }, - HexParseError::MissingPrefix => { - write!(f, "Hex encoded data must start with 0x prefix") - }, - HexParseError::InvalidChar => { - write!(f, "Hex encoded data must contain characters [a-zA-Z0-9]") - }, - HexParseError::OutOfRange => { - write!(f, "Hex encoded values of an RpoDigest must be inside the field modulus") - }, - } - } -} - -#[cfg(feature = "std")] -impl std::error::Error for HexParseError {} - /// Parses a hex string into an array of bytes of known size. pub fn hex_to_bytes(value: &str) -> Result<[u8; N], HexParseError> { let expected: usize = (N * 2) + 2;