From 8c9bb6e6d70ae25faa3c53e5c7d4193d29ea1de9 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Wed, 27 Nov 2024 10:53:01 +0100 Subject: [PATCH 01/11] feat: Use `thiserror` in `air` crate --- Cargo.lock | 2 +- Cargo.toml | 3 +++ air/Cargo.toml | 4 ++-- air/src/errors.rs | 31 +++++-------------------------- air/src/options.rs | 5 ++++- air/src/trace/rows.rs | 3 ++- 6 files changed, 17 insertions(+), 31 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0597bccfc..6b56d57c5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1005,8 +1005,8 @@ version = "0.11.0" dependencies = [ "criterion", "miden-core", - "miden-thiserror", "proptest", + "thiserror 2.0.3", "winter-air", "winter-prover", "winter-rand-utils", diff --git a/Cargo.toml b/Cargo.toml index 30d5cdbdf..c043adbf7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,3 +32,6 @@ inherits = "release" debug = true debug-assertions = true overflow-checks = true + +[workspace.dependencies] +thiserror = { version = "2.0", default-features = false } diff --git a/air/Cargo.toml b/air/Cargo.toml index 811503a0a..5c17a1594 100644 --- a/air/Cargo.toml +++ b/air/Cargo.toml @@ -27,11 +27,11 @@ harness = false [features] default = ["std"] -std = ["vm-core/std", "winter-air/std", "thiserror/std"] +std = ["vm-core/std", "winter-air/std"] testing = [] [dependencies] -thiserror = { package = "miden-thiserror", version = "1.0", default-features = false } +thiserror = { workspace = true } vm-core = { package = "miden-core", path = "../core", version = "0.11", default-features = false } winter-air = { package = "winter-air", version = "0.11", default-features = false } winter-prover = { package = "winter-prover", version = "0.11", default-features = false } diff --git a/air/src/errors.rs b/air/src/errors.rs index af884215c..03439cad4 100644 --- a/air/src/errors.rs +++ b/air/src/errors.rs @@ -1,33 +1,12 @@ -use alloc::string::String; -use core::fmt::{Display, Formatter}; - use crate::trace::MIN_TRACE_LEN; -// EXECUTION ERROR +// EXECUTION OPTIONS ERROR // ================================================================================================ -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum ExecutionOptionsError { - ExpectedCyclesTooBig(u32, u32), + #[error("expected number of cycles {expected_cycles} must be smaller than the maximum number of cycles {max_cycles}")] + ExpectedCyclesTooBig { max_cycles: u32, expected_cycles: u32 }, + #[error("maximum number of cycles {0} must be greater than the minimum number of cycles {MIN_TRACE_LEN}")] MaxCycleNumTooSmall(u32), - OtherErrors(String), } - -impl Display for ExecutionOptionsError { - fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), core::fmt::Error> { - use ExecutionOptionsError::*; - - match self { - ExpectedCyclesTooBig(max, expected) => { - write!(f, "The expected number of cycles must be smaller than the maximum number of cycles: maximum is {max}, but expectd is {expected}") - }, - MaxCycleNumTooSmall(max) => { - write!(f, "The maximum number of cycles must be greater than the minimum number of cycles: minimum is {MIN_TRACE_LEN}, but maximum is {max}") - }, - OtherErrors(error) => write!(f, "{error}"), - } - } -} - -#[cfg(feature = "std")] -impl std::error::Error for ExecutionOptionsError {} diff --git a/air/src/options.rs b/air/src/options.rs index fbbbbb31c..bd1a5e6e5 100644 --- a/air/src/options.rs +++ b/air/src/options.rs @@ -199,7 +199,10 @@ impl ExecutionOptions { return Err(ExecutionOptionsError::MaxCycleNumTooSmall(expected_cycles)); } if max_cycles < expected_cycles { - return Err(ExecutionOptionsError::ExpectedCyclesTooBig(max_cycles, expected_cycles)); + return Err(ExecutionOptionsError::ExpectedCyclesTooBig { + max_cycles, + expected_cycles, + }); } // Round up the expected number of cycles to the next power of two. If it is smaller than diff --git a/air/src/trace/rows.rs b/air/src/trace/rows.rs index 25f1f9e33..bd2090ae8 100644 --- a/air/src/trace/rows.rs +++ b/air/src/trace/rows.rs @@ -3,11 +3,12 @@ use core::{ ops::{Add, AddAssign, Bound, Index, IndexMut, Mul, RangeBounds, Sub, SubAssign}, }; +use thiserror::Error; use vm_core::Felt; /// Represents the types of errors that can occur when converting from and into [`RowIndex`] and /// using its operations. -#[derive(Debug, thiserror::Error, PartialEq, Eq)] +#[derive(Debug, Error, PartialEq, Eq)] pub enum RowIndexError { #[error("value is too large to be converted into RowIndex: {0}")] InvalidSize(T), From d3878b3ddf81e058c84868071f53a00cb2b042b4 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Wed, 27 Nov 2024 11:28:45 +0100 Subject: [PATCH 02/11] feat: Refactor errors in `assembly` --- Cargo.lock | 2 +- assembly/Cargo.toml | 4 +-- assembly/src/assembler/mast_forest_builder.rs | 34 ++++++++++++++----- assembly/src/errors.rs | 17 ++++++---- assembly/src/library/error.rs | 4 +-- assembly/src/library/mod.rs | 2 +- assembly/src/library/path.rs | 8 +++-- 7 files changed, 47 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6b56d57c5..e60ea9cd1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1021,11 +1021,11 @@ dependencies = [ "lalrpop-util", "miden-core", "miden-miette", - "miden-thiserror", "pretty_assertions", "regex", "rustc_version 0.4.1", "smallvec", + "thiserror 2.0.3", "tracing", "unicode-width 0.2.0", ] diff --git a/assembly/Cargo.toml b/assembly/Cargo.toml index e72f780fb..359d82a11 100644 --- a/assembly/Cargo.toml +++ b/assembly/Cargo.toml @@ -19,7 +19,7 @@ doctest = false [features] default = ["std"] -std = ["aho-corasick/std", "miette/fancy", "miette/std", "thiserror/std", "vm-core/std"] +std = ["aho-corasick/std", "miette/fancy", "miette/std", "vm-core/std"] testing = ["dep:regex"] [dependencies] @@ -31,7 +31,7 @@ miette = { package = "miden-miette", version = "7.1", default-features = false, ] } regex = { version = "1.10", optional = true, default-features = false, features = ["unicode", "perf"] } smallvec = { version = "1.13", features = ["union", "const_generics", "const_new"] } -thiserror = { package = "miden-thiserror", version = "1.0", default-features = false } +thiserror = { workspace = true } tracing = { version = "0.1", default-features = false, features = ["attributes"] } unicode-width = { version = "0.2", features = ["no_std"] } vm-core = { package = "miden-core", path = "../core", version = "0.11", default-features = false, features = [ diff --git a/assembly/src/assembler/mast_forest_builder.rs b/assembly/src/assembler/mast_forest_builder.rs index 96e7d51b5..0a17bd07c 100644 --- a/assembly/src/assembler/mast_forest_builder.rs +++ b/assembly/src/assembler/mast_forest_builder.rs @@ -347,7 +347,9 @@ impl MastForestBuilder { // decorator already exists in the forest; return previously assigned id Ok(*decorator_id) } else { - let new_decorator_id = self.mast_forest.add_decorator(decorator)?; + let new_decorator_id = self.mast_forest.add_decorator(decorator).map_err(|source| { + AssemblyError::forest_error("assembler failed to add new decorator", source) + })?; self.decorator_id_by_fingerprint.insert(decorator_hash, new_decorator_id); Ok(new_decorator_id) @@ -366,7 +368,9 @@ impl MastForestBuilder { // node already exists in the forest; return previously assigned id Ok(*node_id) } else { - let new_node_id = self.mast_forest.add_node(node)?; + let new_node_id = self.mast_forest.add_node(node).map_err(|source| { + AssemblyError::forest_error("assembler failed to add new node", source) + })?; self.node_id_by_fingerprint.insert(node_fingerprint, new_node_id); self.hash_by_node_id.insert(new_node_id, node_fingerprint); @@ -380,7 +384,9 @@ impl MastForestBuilder { operations: Vec, decorators: Option, ) -> Result { - let block = MastNode::new_basic_block(operations, decorators)?; + let block = MastNode::new_basic_block(operations, decorators).map_err(|source| { + AssemblyError::forest_error("assembler failed to add new basic block node", source) + })?; self.ensure_node(block) } @@ -390,7 +396,10 @@ impl MastForestBuilder { left_child: MastNodeId, right_child: MastNodeId, ) -> Result { - let join = MastNode::new_join(left_child, right_child, &self.mast_forest)?; + let join = + MastNode::new_join(left_child, right_child, &self.mast_forest).map_err(|source| { + AssemblyError::forest_error("assembler failed to add new join node", source) + })?; self.ensure_node(join) } @@ -400,25 +409,34 @@ impl MastForestBuilder { if_branch: MastNodeId, else_branch: MastNodeId, ) -> Result { - let split = MastNode::new_split(if_branch, else_branch, &self.mast_forest)?; + let split = + MastNode::new_split(if_branch, else_branch, &self.mast_forest).map_err(|source| { + AssemblyError::forest_error("assembler failed to add new split node", source) + })?; self.ensure_node(split) } /// Adds a loop node to the forest, and returns the [`MastNodeId`] associated with it. pub fn ensure_loop(&mut self, body: MastNodeId) -> Result { - let loop_node = MastNode::new_loop(body, &self.mast_forest)?; + let loop_node = MastNode::new_loop(body, &self.mast_forest).map_err(|source| { + AssemblyError::forest_error("assembler failed to add new loop node", source) + })?; self.ensure_node(loop_node) } /// Adds a call node to the forest, and returns the [`MastNodeId`] associated with it. pub fn ensure_call(&mut self, callee: MastNodeId) -> Result { - let call = MastNode::new_call(callee, &self.mast_forest)?; + let call = MastNode::new_call(callee, &self.mast_forest).map_err(|source| { + AssemblyError::forest_error("assembler failed to add new call node", source) + })?; self.ensure_node(call) } /// Adds a syscall node to the forest, and returns the [`MastNodeId`] associated with it. pub fn ensure_syscall(&mut self, callee: MastNodeId) -> Result { - let syscall = MastNode::new_syscall(callee, &self.mast_forest)?; + let syscall = MastNode::new_syscall(callee, &self.mast_forest).map_err(|source| { + AssemblyError::forest_error("assembler failed to add new syscall node", source) + })?; self.ensure_node(syscall) } diff --git a/assembly/src/errors.rs b/assembly/src/errors.rs index 4fd06609e..bee90c05f 100644 --- a/assembly/src/errors.rs +++ b/assembly/src/errors.rs @@ -4,7 +4,7 @@ use vm_core::mast::MastForestError; use crate::{ ast::QualifiedProcedureName, - diagnostics::{Diagnostic, RelatedError, RelatedLabel, Report, SourceFile}, + diagnostics::{Diagnostic, RelatedError, RelatedLabel, SourceFile}, LibraryNamespace, LibraryPath, SourceSpan, }; @@ -81,13 +81,16 @@ pub enum AssemblyError { }, #[error(transparent)] #[diagnostic(transparent)] - Other(#[from] RelatedError), - #[error(transparent)] - Forest(#[from] MastForestError), + Other(RelatedError), + // Technically MastForestError is the source error here, but since AssemblyError is converted + // into a Report and that doesn't implement core::error::Error, treating MastForestError as a + // source error would effectively swallow it, so we include it in the error message instead. + #[error("{0}: {1}")] + Forest(&'static str, MastForestError), } -impl From for AssemblyError { - fn from(report: Report) -> Self { - Self::Other(RelatedError::new(report)) +impl AssemblyError { + pub(super) fn forest_error(message: &'static str, source: MastForestError) -> Self { + Self::Forest(message, source) } } diff --git a/assembly/src/library/error.rs b/assembly/src/library/error.rs index 3df795ca3..94bc646f1 100644 --- a/assembly/src/library/error.rs +++ b/assembly/src/library/error.rs @@ -9,8 +9,8 @@ pub enum LibraryError { EmptyKernel, #[error("invalid export in kernel library: {procedure_path}")] InvalidKernelExport { procedure_path: QualifiedProcedureName }, - #[error(transparent)] - Kernel(#[from] KernelError), + #[error("failed to convert library into kernel library: {0}")] + KernelConversion(KernelError), #[error("invalid export: no procedure root for {procedure_path} procedure")] NoProcedureRootForExport { procedure_path: QualifiedProcedureName }, } diff --git a/assembly/src/library/mod.rs b/assembly/src/library/mod.rs index 7e2d35789..8f42223c2 100644 --- a/assembly/src/library/mod.rs +++ b/assembly/src/library/mod.rs @@ -375,7 +375,7 @@ impl TryFrom for KernelLibrary { kernel_module.add_procedure(proc_path.name.clone(), proc_digest); } - let kernel = Kernel::new(&proc_digests)?; + let kernel = Kernel::new(&proc_digests).map_err(LibraryError::KernelConversion)?; Ok(Self { kernel, diff --git a/assembly/src/library/path.rs b/assembly/src/library/path.rs index 915bc9aaa..cb3ee9263 100644 --- a/assembly/src/library/path.rs +++ b/assembly/src/library/path.rs @@ -25,11 +25,11 @@ pub enum PathError { #[error("invalid library path component: cannot be empty")] EmptyComponent, #[error("invalid library path component: {0}")] - InvalidComponent(#[from] crate::ast::IdentError), + InvalidComponent(crate::ast::IdentError), #[error("invalid library path: contains invalid utf8 byte sequences")] InvalidUtf8, #[error(transparent)] - InvalidNamespace(#[from] crate::library::LibraryNamespaceError), + InvalidNamespace(crate::library::LibraryNamespaceError), #[error("cannot join a path with reserved name to other paths")] UnsupportedJoin, } @@ -429,7 +429,9 @@ impl<'a> TryFrom>> for LibraryPath { let ns = match iter.next() { None => return Err(PathError::Empty), Some(LibraryPathComponent::Namespace(ns)) => ns.clone(), - Some(LibraryPathComponent::Normal(ident)) => LibraryNamespace::try_from(ident.clone())?, + Some(LibraryPathComponent::Normal(ident)) => { + LibraryNamespace::try_from(ident.clone()).map_err(PathError::InvalidNamespace)? + }, }; let mut components = Components::default(); for component in iter { From 2292155c3893d6eda4da57193179c15366048c56 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Wed, 27 Nov 2024 13:26:12 +0100 Subject: [PATCH 03/11] feat: Refactor `errors` in `core` --- Cargo.lock | 2 +- core/Cargo.toml | 2 +- core/src/debuginfo/source_manager.rs | 32 +++++++++++++++++++++++----- core/src/mast/mod.rs | 18 ++++++---------- 4 files changed, 35 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e60ea9cd1..92d5e3c78 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1040,11 +1040,11 @@ dependencies = [ "miden-crypto", "miden-formatting", "miden-miette", - "miden-thiserror", "num-derive", "num-traits", "parking_lot", "proptest", + "thiserror 2.0.3", "winter-math", "winter-rand-utils", "winter-utils", diff --git a/core/Cargo.toml b/core/Cargo.toml index 8643a81ba..89c861935 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -43,7 +43,7 @@ miette = { package = "miden-miette", version = "7.1", default-features = false, num-derive = { version = "0.4", default-features = false } num-traits = { version = "0.2", default-features = false } parking_lot = { version = "0.12", optional = true } -thiserror = { package = "miden-thiserror", version = "1.0", default-features = false } +thiserror = { workspace = true } winter-utils = { package = "winter-utils", version = "0.11", default-features = false } [dev-dependencies] diff --git a/core/src/debuginfo/source_manager.rs b/core/src/debuginfo/source_manager.rs index e88e96d3d..d4453837d 100644 --- a/core/src/debuginfo/source_manager.rs +++ b/core/src/debuginfo/source_manager.rs @@ -1,9 +1,11 @@ use alloc::{ + boxed::Box, collections::BTreeMap, string::{String, ToString}, sync::Arc, vec::Vec, }; +use core::error::Error; use super::*; @@ -78,10 +80,25 @@ pub enum SourceManagerError { /// An attempt was made to read content using invalid byte indices #[error("attempted to read content out of bounds")] InvalidBounds, - /// An attempt to load a source file failed due to an I/O error - #[cfg(feature = "std")] - #[error(transparent)] - LoadFailed(#[from] std::io::Error), + /// Custom error variant for implementors of the trait. + #[error("{error_msg}")] + Custom { + error_msg: Box, + source: Option>, + }, +} + +impl SourceManagerError { + pub fn custom(message: String) -> Self { + Self::Custom { error_msg: message.into(), source: None } + } + + pub fn custom_with_source(message: String, source: impl Error + Send + Sync + 'static) -> Self { + Self::Custom { + error_msg: message.into(), + source: Some(Box::new(source)), + } + } } pub trait SourceManager { @@ -201,7 +218,12 @@ pub trait SourceManagerExt: SourceManager { let name = Arc::from(name.into_owned().into_boxed_str()); let content = std::fs::read_to_string(path) .map(|s| SourceContent::new(Arc::clone(&name), s.into_boxed_str())) - .map_err(SourceManagerError::LoadFailed)?; + .map_err(|source| { + SourceManagerError::custom_with_source( + format!("failed to load filed at `{}`", path.display()), + source, + ) + })?; Ok(self.load_from_raw_parts(name, content)) } diff --git a/core/src/mast/mod.rs b/core/src/mast/mod.rs index 142c50b48..1d8af06ab 100644 --- a/core/src/mast/mod.rs +++ b/core/src/mast/mod.rs @@ -695,24 +695,18 @@ impl Serializable for DecoratorId { /// Represents the types of errors that can occur when dealing with MAST forest. #[derive(Debug, thiserror::Error, PartialEq)] pub enum MastForestError { - #[error( - "invalid decorator count: MAST forest exceeds the maximum of {} decorators", - u32::MAX - )] + #[error("MAST forest decorator count exceeds the maximum of {} decorators", u32::MAX)] TooManyDecorators, - #[error( - "invalid node count: MAST forest exceeds the maximum of {} nodes", - MastForest::MAX_NODES - )] + #[error("MAST forest node count exceeds the maximum of {} nodes", MastForest::MAX_NODES)] TooManyNodes, - #[error("node id: {0} is greater than or equal to forest length: {1}")] + #[error("node id {0} is greater than or equal to forest length {1}")] NodeIdOverflow(MastNodeId, usize), - #[error("decorator id: {0} is greater than or equal to decorator count: {1}")] + #[error("decorator id {0} is greater than or equal to decorator count {1}")] DecoratorIdOverflow(DecoratorId, usize), #[error("basic block cannot be created from an empty list of operations")] EmptyBasicBlock, - #[error("decorator root of child with node id {0} is missing but required for fingerprint computation")] + #[error("decorator root of child with node id {0} is missing but is required for fingerprint computation")] ChildFingerprintMissing(MastNodeId), - #[error("advice map key already exists when merging forests: {0}")] + #[error("advice map key {0} already exists when merging forests")] AdviceMapKeyCollisionOnMerge(RpoDigest), } From c80f5c78f10f8d1fa0e7f802208486fbc0c51c22 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Wed, 27 Nov 2024 14:55:50 +0100 Subject: [PATCH 04/11] feat: Refactor errors in `processor` --- Cargo.lock | 1 + processor/Cargo.toml | 1 + processor/src/errors.rs | 309 +++++++------------------ processor/src/host/advice/providers.rs | 17 +- processor/src/lib.rs | 6 +- 5 files changed, 100 insertions(+), 234 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 92d5e3c78..cef068d35 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1141,6 +1141,7 @@ dependencies = [ "miden-assembly", "miden-core", "miden-test-utils", + "thiserror 2.0.3", "tracing", "winter-fri", "winter-prover", diff --git a/processor/Cargo.toml b/processor/Cargo.toml index cd52ae38a..f0e6d0710 100644 --- a/processor/Cargo.toml +++ b/processor/Cargo.toml @@ -28,6 +28,7 @@ miden-air = { package = "miden-air", path = "../air", version = "0.11", default- tracing = { version = "0.1", default-features = false, features = ["attributes"] } vm-core = { package = "miden-core", path = "../core", version = "0.11", default-features = false } winter-prover = { package = "winter-prover", version = "0.11", default-features = false } +thiserror = { workspace = true } [dev-dependencies] assembly = { package = "miden-assembly", path = "../assembly", version = "0.11", default-features = false } diff --git a/processor/src/errors.rs b/processor/src/errors.rs index b5c77f86e..251a39264 100644 --- a/processor/src/errors.rs +++ b/processor/src/errors.rs @@ -1,7 +1,5 @@ -use alloc::string::String; -use core::fmt::{Display, Formatter}; -#[cfg(feature = "std")] -use std::error::Error; +use alloc::{boxed::Box, string::String}; +use core::error::Error; use miden_air::RowIndex; use vm_core::{ @@ -21,223 +19,118 @@ use crate::ContextId; // EXECUTION ERROR // ================================================================================================ -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum ExecutionError { + #[error("value for key {} not present in the advice map", to_hex(Felt::elements_as_bytes(.0)))] AdviceMapKeyNotFound(Word), + #[error("value for key {} already present in the advice map", to_hex(Felt::elements_as_bytes(.0)))] AdviceMapKeyAlreadyPresent(Word), + #[error("advice stack read failed at step {0}")] AdviceStackReadFailed(RowIndex), + #[error("instruction `caller` used outside of kernel context")] CallerNotInSyscall, + #[error("external node with mast root {0} resolved to an external node")] CircularExternalNode(Digest), + #[error("exceeded the allowed number of max cycles {0}")] CycleLimitExceeded(u32), - DecoratorNotFoundInForest { - decorator_id: DecoratorId, - }, + #[error("decorator id {decorator_id} does not exist in MAST forest")] + DecoratorNotFoundInForest { decorator_id: DecoratorId }, + #[error("division by zero at clock cycle {0}")] DivideByZero(RowIndex), - DuplicateMemoryAccess { - ctx: ContextId, - addr: u32, - clk: Felt, - }, + #[error("memory address {addr} in context {ctx} accessed twice in clock cycle {clk}")] + DuplicateMemoryAccess { ctx: ContextId, addr: u32, clk: Felt }, + #[error("failed to execute the dynamic code block provided by the stack with root {hex}; the block could not be found", + hex = to_hex(.0.as_bytes()) + )] DynamicNodeNotFound(Digest), - EventError(String), + #[error("error during processing of event in on_event handler")] + EventError(#[source] Box), + #[error("failed to execute Ext2Intt operation: {0}")] Ext2InttError(Ext2InttError), + #[error("assertion failed at clock cycle {clk} with error code {err_code}{}", + match err_msg { + Some(msg) => format!(": {msg}"), + None => "".into() + } + )] FailedAssertion { clk: RowIndex, err_code: u32, err_msg: Option, }, + #[error("failed to generate signature: {0}")] FailedSignatureGeneration(&'static str), + #[error("Updating FMP register from {0} to {1} failed because {1} is outside of {FMP_MIN}..{FMP_MAX}")] InvalidFmpValue(Felt, Felt), + #[error("FRI domain segment value cannot exceed 3, but was {0}")] InvalidFriDomainSegment(u64), + #[error("degree-respecting projection is inconsistent: expected {0} but was {1}")] InvalidFriLayerFolding(QuadFelt, QuadFelt), - InvalidMemoryRange { - start_addr: u64, - end_addr: u64, - }, + #[error( + "memory range start address cannot exceed end address, but was ({start_addr}, {end_addr})" + )] + InvalidMemoryRange { start_addr: u64, end_addr: u64 }, + #[error("when returning from a call, stack depth must be {MIN_STACK_DEPTH}, but was {0}")] InvalidStackDepthOnReturn(usize), - InvalidTreeDepth { - depth: Felt, - }, - InvalidTreeNodeIndex { - depth: Felt, - value: Felt, - }, + #[error("provided merkle tree {depth} is out of bounds and cannot be represented as an unsigned 8-bit integer")] + InvalidMerkleTreeDepth { depth: Felt }, + #[error( + "provided node index {value} is out of bounds for a merkle tree node at depth {depth}" + )] + InvalidMerkleTreeNodeIndex { depth: Felt, value: Felt }, + #[error("attempted to calculate integer logarithm with zero argument at clock cycle {0}")] LogArgumentZero(RowIndex), + #[error("malformed signature key: {0}")] MalformedSignatureKey(&'static str), - MalformedMastForestInHost { - root_digest: Digest, - }, - MastNodeNotFoundInForest { - node_id: MastNodeId, - }, - MastForestNotFound { - root_digest: Digest, - }, + #[error( + "MAST forest in host indexed by procedure root {root_digest} doesn't contain that root" + )] + MalformedMastForestInHost { root_digest: Digest }, + #[error("node id {node_id} does not exist in MAST forest")] + MastNodeNotFoundInForest { node_id: MastNodeId }, + #[error("no MAST forest contains the procedure with root digest {root_digest}")] + NoMastForestWithProcedure { root_digest: Digest }, + #[error("memory address cannot exceed 2^32 but was {0}")] MemoryAddressOutOfBounds(u64), + #[error("merkle path verification failed for value {value} at index {index} in the Merkle tree with root {root} (error code: {err_code})", + value = to_hex(Felt::elements_as_bytes(value)), + root = to_hex(root.as_bytes()), + )] MerklePathVerificationFailed { value: Word, index: Felt, root: Digest, err_code: u32, }, - MerkleStoreLookupFailed(MerkleError), - MerkleStoreMergeFailed(MerkleError), - MerkleStoreUpdateFailed(MerkleError), + #[error("advice provider Merkle store backend lookup failed")] + MerkleStoreLookupFailed(#[source] MerkleError), + #[error("advice provider Merkle store backend merge failed")] + MerkleStoreMergeFailed(#[source] MerkleError), + #[error("advice provider Merkle store backend update failed")] + MerkleStoreUpdateFailed(#[source] MerkleError), + #[error("an operation expected a binary value, but received {0}")] NotBinaryValue(Felt), + #[error("an operation expected a u32 value, but received {0} (error code: {1})")] NotU32Value(Felt, Felt), + #[error("stack should have at most {MIN_STACK_DEPTH} elements at the end of program execution, but had {} elements", MIN_STACK_DEPTH + .0)] OutputStackOverflow(usize), + #[error("a program has already been executed in this process")] ProgramAlreadyExecuted, - ProverError(ProverError), + #[error("proof generation failed")] + ProverError(#[source] ProverError), + #[error("smt node {node_hex} not found", node_hex = to_hex(Felt::elements_as_bytes(.0)))] SmtNodeNotFound(Word), + #[error("expected pre-image length of node {node_hex} to be a multiple of 8 but was {preimage_len}", + node_hex = to_hex(Felt::elements_as_bytes(.0)), + preimage_len = .1 + )] SmtNodePreImageNotValid(Word, usize), + #[error("syscall failed: procedure with root {hex} was not found in the kernel", + hex = to_hex(.0.as_bytes()) + )] SyscallTargetNotInKernel(Digest), } -impl Display for ExecutionError { - fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), core::fmt::Error> { - use ExecutionError::*; - - match self { - AdviceMapKeyNotFound(key) => { - let hex = to_hex(Felt::elements_as_bytes(key)); - write!(f, "Value for key {hex} not present in the advice map") - }, - AdviceMapKeyAlreadyPresent(key) => { - let hex = to_hex(Felt::elements_as_bytes(key)); - write!(f, "Value for key {hex} already present in the advice map") - }, - AdviceStackReadFailed(step) => write!(f, "Advice stack read failed at step {step}"), - CallerNotInSyscall => { - write!(f, "Instruction `caller` used outside of kernel context") - }, - CircularExternalNode(mast_root) => { - write!(f, "External node with root {mast_root} resolved to an external node") - }, - CycleLimitExceeded(max_cycles) => { - write!(f, "Exceeded the allowed number of cycles (max cycles = {max_cycles})") - }, - DecoratorNotFoundInForest { decorator_id } => { - write!(f, "Malformed MAST forest, decorator id {decorator_id} doesn't exist") - }, - DivideByZero(clk) => write!(f, "Division by zero at clock cycle {clk}"), - DuplicateMemoryAccess { ctx, addr, clk } => write!( - f, - "Memory address {addr} in context {ctx} accessed twice in clock cycle {clk}" - ), - DynamicNodeNotFound(digest) => { - let hex = to_hex(digest.as_bytes()); - write!( - f, - "Failed to execute the dynamic code block provided by the stack with root {hex}; the block could not be found" - ) - }, - EventError(error) => write!(f, "Failed to process event - {error}"), - Ext2InttError(err) => write!(f, "Failed to execute Ext2Intt operation: {err}"), - FailedAssertion { clk, err_code, err_msg } => { - if let Some(err_msg) = err_msg { - write!( - f, - "Assertion failed at clock cycle {clk} with error code {err_code}: {err_msg}" - ) - } else { - write!(f, "Assertion failed at clock cycle {clk} with error code {err_code}") - } - }, - FailedSignatureGeneration(signature) => { - write!(f, "Failed to generate signature: {signature}") - }, - InvalidFmpValue(old, new) => { - write!(f, "Updating FMP register from {old} to {new} failed because {new} is outside of {FMP_MIN}..{FMP_MAX}") - }, - InvalidFriDomainSegment(value) => { - write!(f, "FRI domain segment value cannot exceed 3, but was {value}") - }, - InvalidFriLayerFolding(expected, actual) => { - write!(f, "Degree-respecting projection is inconsistent: expected {expected} but was {actual}") - }, - InvalidMemoryRange { start_addr, end_addr } => { - write!(f, "Memory range start address cannot exceed end address, but was ({start_addr}, {end_addr})") - }, - InvalidStackDepthOnReturn(depth) => { - write!(f, "When returning from a call, stack depth must be {MIN_STACK_DEPTH}, but was {depth}") - }, - InvalidTreeDepth { depth } => { - write!(f, "The provided {depth} is out of bounds and cannot be represented as an unsigned 8-bits integer") - }, - InvalidTreeNodeIndex { depth, value } => { - write!(f, "The provided index {value} is out of bounds for a node at depth {depth}") - }, - LogArgumentZero(clk) => { - write!( - f, - "Calculating of the integer logarithm with zero argument at clock cycle {clk}" - ) - }, - MalformedSignatureKey(signature) => write!(f, "Malformed signature key: {signature}"), - MalformedMastForestInHost { root_digest } => { - write!(f, "Malformed host: MAST forest indexed by procedure root {} doesn't contain that root", root_digest) - }, - MastNodeNotFoundInForest { node_id } => { - write!(f, "Malformed MAST forest, node id {node_id} doesn't exist") - }, - MastForestNotFound { root_digest } => { - write!( - f, - "No MAST forest contains the following procedure root digest: {root_digest}" - ) - }, - MemoryAddressOutOfBounds(addr) => { - write!(f, "Memory address cannot exceed 2^32 but was {addr}") - }, - MerklePathVerificationFailed { value, index, root, err_code } => { - let value = to_hex(Felt::elements_as_bytes(value)); - let root = to_hex(root.as_bytes()); - write!(f, "Merkle path verification failed for value {value} at index {index}, in the Merkle tree with root {root} (error code: {err_code})") - }, - MerkleStoreLookupFailed(reason) => { - write!(f, "Advice provider Merkle store backend lookup failed: {reason}") - }, - MerkleStoreMergeFailed(reason) => { - write!(f, "Advice provider Merkle store backend merge failed: {reason}") - }, - MerkleStoreUpdateFailed(reason) => { - write!(f, "Advice provider Merkle store backend update failed: {reason}") - }, - NotBinaryValue(v) => { - write!(f, "An operation expected a binary value, but received {v}") - }, - NotU32Value(v, err_code) => { - write!( - f, - "An operation expected a u32 value, but received {v} (error code: {err_code})" - ) - }, - OutputStackOverflow(n) => { - write!(f, "The stack should have at most {MIN_STACK_DEPTH} elements at the end of program execution, but had {} elements", MIN_STACK_DEPTH + n) - }, - SmtNodeNotFound(node) => { - let node_hex = to_hex(Felt::elements_as_bytes(node)); - write!(f, "Smt node {node_hex} not found") - }, - SmtNodePreImageNotValid(node, preimage_len) => { - let node_hex = to_hex(Felt::elements_as_bytes(node)); - write!(f, "Invalid pre-image for node {node_hex}. Expected pre-image length to be a multiple of 8, but was {preimage_len}") - }, - ProgramAlreadyExecuted => { - write!(f, "a program has already been executed in this process") - }, - ProverError(error) => write!(f, "Proof generation failed: {error}"), - SyscallTargetNotInKernel(proc) => { - let hex = to_hex(proc.as_bytes()); - write!(f, "Syscall failed: procedure with root {hex} was not found in the kernel") - }, - } - } -} - -#[cfg(feature = "std")] -impl Error for ExecutionError {} - impl From for ExecutionError { fn from(value: Ext2InttError) -> Self { Self::Ext2InttError(value) @@ -247,54 +140,22 @@ impl From for ExecutionError { // EXT2INTT ERROR // ================================================================================================ -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, thiserror::Error)] pub enum Ext2InttError { + #[error("input domain size must be a power of two, but was {0}")] DomainSizeNotPowerOf2(u64), + #[error("input domain size ({0} elements) is too small")] DomainSizeTooSmall(u64), + #[error("address of the last input must be smaller than 2^32, but was {0}")] InputEndAddressTooBig(u64), + #[error("input size must be smaller than 2^32, but was {0}")] InputSizeTooBig(u64), + #[error("address of the first input must be smaller than 2^32, but was {0}")] InputStartAddressTooBig(u64), + #[error("output size ({0}) cannot be greater than the input size ({1})")] OutputSizeTooBig(usize, usize), + #[error("output size must be greater than 0")] OutputSizeIsZero, + #[error("uninitialized memory at address {0}")] UninitializedMemoryAddress(u32), } - -impl Display for Ext2InttError { - fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), core::fmt::Error> { - use Ext2InttError::*; - - match self { - DomainSizeNotPowerOf2(v) => { - write!(f, "input domain size must be a power of two, but was {v}") - }, - DomainSizeTooSmall(v) => { - write!(f, "input domain size ({v} elements) is too small") - }, - InputEndAddressTooBig(addr) => { - write!(f, "address of the last input must be smaller than 2^32, but was {addr}") - }, - InputSizeTooBig(size) => { - write!(f, "input size must be smaller than 2^32, but was {size}") - }, - InputStartAddressTooBig(addr) => { - write!(f, "address of the first input must be smaller than 2^32, but was {addr}") - }, - OutputSizeIsZero => { - write!(f, "output size must be greater than 0") - }, - OutputSizeTooBig(output_size, input_size) => { - write!( - f, - "output size ({output_size}) cannot be greater than the input size ({input_size})" - ) - }, - - UninitializedMemoryAddress(address) => { - write!(f, "uninitialized memory at address {address}") - }, - } - } -} - -#[cfg(feature = "std")] -impl Error for Ext2InttError {} diff --git a/processor/src/host/advice/providers.rs b/processor/src/host/advice/providers.rs index 6edcb85ad..bf80b7575 100644 --- a/processor/src/host/advice/providers.rs +++ b/processor/src/host/advice/providers.rs @@ -127,8 +127,9 @@ where depth: &Felt, index: &Felt, ) -> Result { - let index = NodeIndex::from_elements(depth, index) - .map_err(|_| ExecutionError::InvalidTreeNodeIndex { depth: *depth, value: *index })?; + let index = NodeIndex::from_elements(depth, index).map_err(|_| { + ExecutionError::InvalidMerkleTreeNodeIndex { depth: *depth, value: *index } + })?; self.store .get_node(root.into(), index) .map(|v| v.into()) @@ -141,8 +142,9 @@ where depth: &Felt, index: &Felt, ) -> Result { - let index = NodeIndex::from_elements(depth, index) - .map_err(|_| ExecutionError::InvalidTreeNodeIndex { depth: *depth, value: *index })?; + let index = NodeIndex::from_elements(depth, index).map_err(|_| { + ExecutionError::InvalidMerkleTreeNodeIndex { depth: *depth, value: *index } + })?; self.store .get_path(root.into(), index) .map(|value| value.path) @@ -156,7 +158,7 @@ where index: &Felt, ) -> Result { let tree_depth = u8::try_from(tree_depth.as_int()) - .map_err(|_| ExecutionError::InvalidTreeDepth { depth: *tree_depth })?; + .map_err(|_| ExecutionError::InvalidMerkleTreeDepth { depth: *tree_depth })?; self.store .get_leaf_depth(root.into(), tree_depth, index.as_int()) .map_err(ExecutionError::MerkleStoreLookupFailed) @@ -169,8 +171,9 @@ where index: &Felt, value: Word, ) -> Result<(MerklePath, Word), ExecutionError> { - let node_index = NodeIndex::from_elements(depth, index) - .map_err(|_| ExecutionError::InvalidTreeNodeIndex { depth: *depth, value: *index })?; + let node_index = NodeIndex::from_elements(depth, index).map_err(|_| { + ExecutionError::InvalidMerkleTreeNodeIndex { depth: *depth, value: *index } + })?; self.store .set_node(root.into(), node_index, value.into()) .map(|root| (root.path, root.root.into())) diff --git a/processor/src/lib.rs b/processor/src/lib.rs index cf0a80bc9..2f186fd72 100644 --- a/processor/src/lib.rs +++ b/processor/src/lib.rs @@ -278,9 +278,9 @@ impl Process { MastNode::Dyn(node) => self.execute_dyn_node(node, program, host)?, MastNode::External(external_node) => { let node_digest = external_node.digest(); - let mast_forest = host - .get_mast_forest(&node_digest) - .ok_or(ExecutionError::MastForestNotFound { root_digest: node_digest })?; + let mast_forest = host.get_mast_forest(&node_digest).ok_or( + ExecutionError::NoMastForestWithProcedure { root_digest: node_digest }, + )?; // We limit the parts of the program that can be called externally to procedure // roots, even though MAST doesn't have that restriction. From 3c22c587190a2705310516ec820cc05c05b75ea2 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Wed, 27 Nov 2024 15:08:52 +0100 Subject: [PATCH 05/11] feat: Refactor errors for `verifier` --- Cargo.lock | 1 + verifier/Cargo.toml | 1 + verifier/src/lib.rs | 25 +++++++------------------ 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cef068d35..a446adcbe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1227,6 +1227,7 @@ version = "0.11.0" dependencies = [ "miden-air", "miden-core", + "thiserror 2.0.3", "tracing", "winter-verifier", ] diff --git a/verifier/Cargo.toml b/verifier/Cargo.toml index a91742fd3..35e9bdc78 100644 --- a/verifier/Cargo.toml +++ b/verifier/Cargo.toml @@ -26,3 +26,4 @@ air = { package = "miden-air", path = "../air", version = "0.11", default-featur tracing = { version = "0.1", default-features = false, features = ["attributes"] } vm-core = { package = "miden-core", path = "../core", version = "0.11", default-features = false } winter-verifier = { package = "winter-verifier", version = "0.11", default-features = false } +thiserror = { workspace = true } \ No newline at end of file diff --git a/verifier/src/lib.rs b/verifier/src/lib.rs index 0ccf8fda2..03962b607 100644 --- a/verifier/src/lib.rs +++ b/verifier/src/lib.rs @@ -6,7 +6,6 @@ extern crate alloc; extern crate std; use alloc::vec; -use core::fmt; use air::{HashFunction, ProcessorAir, ProvingOptions, PublicInputs}; use vm_core::crypto::{ @@ -62,6 +61,7 @@ pub fn verify( ) -> Result { // get security level of the proof let security_level = proof.security_level(); + let program_hash = *program_info.program_hash(); // build public inputs and try to verify the proof let pub_inputs = PublicInputs::new(program_info, stack_inputs, stack_outputs); @@ -98,7 +98,7 @@ pub fn verify( ) }, } - .map_err(VerificationError::VerifierError)?; + .map_err(|source| VerificationError::ProgramVerificationError(program_hash, source))?; Ok(security_level) } @@ -107,23 +107,12 @@ pub fn verify( // ================================================================================================ /// TODO: add docs -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, thiserror::Error)] pub enum VerificationError { - VerifierError(VerifierError), + #[error("failed to verify proof for program with hash {0}")] + ProgramVerificationError(Digest, #[source] VerifierError), + #[error("the input {0} is not a valid field element")] InputNotFieldElement(u64), + #[error("the output {0} is not a valid field element")] OutputNotFieldElement(u64), } - -impl fmt::Display for VerificationError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - use VerificationError::*; - match self { - VerifierError(e) => write!(f, "{e}"), - InputNotFieldElement(i) => write!(f, "the input {i} is not a valid field element!"), - OutputNotFieldElement(o) => write!(f, "the output {o} is not a valid field element!"), - } - } -} - -#[cfg(feature = "std")] -impl std::error::Error for VerificationError {} From cdb324c952c6a8624d6941383683129babf5ddf8 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Wed, 27 Nov 2024 16:41:39 +0100 Subject: [PATCH 06/11] chore: Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf5c12887..24424450d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - [BREAKING] Updated Winterfell dependency to v0.11 (#1586). - [BREAKING] resolved flag collision in `--verify` command and added functionality for optional input/output files (#1513). - [BREAKING] Cleanup benchmarks and examples in the `miden-vm` crate (#1587) +- [BREAKING] Use `thiserror` 2.0 to derive errors and refactor them (#1588). #### Enhancements - Added `miden_core::mast::MastForest::advice_map` to load it into the advice provider before the `MastForest` execution (#1574). From f5f4ca05af6199585110ccc8bee66b29e007deea Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Wed, 27 Nov 2024 16:56:42 +0100 Subject: [PATCH 07/11] chore: Add `"thiserror/std"` conditional feature --- air/Cargo.toml | 2 +- assembly/Cargo.toml | 2 +- processor/Cargo.toml | 2 +- verifier/Cargo.toml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/air/Cargo.toml b/air/Cargo.toml index 5c17a1594..627cd691b 100644 --- a/air/Cargo.toml +++ b/air/Cargo.toml @@ -27,7 +27,7 @@ harness = false [features] default = ["std"] -std = ["vm-core/std", "winter-air/std"] +std = ["vm-core/std", "winter-air/std", "thiserror/std"] testing = [] [dependencies] diff --git a/assembly/Cargo.toml b/assembly/Cargo.toml index 359d82a11..a912e4879 100644 --- a/assembly/Cargo.toml +++ b/assembly/Cargo.toml @@ -19,7 +19,7 @@ doctest = false [features] default = ["std"] -std = ["aho-corasick/std", "miette/fancy", "miette/std", "vm-core/std"] +std = ["aho-corasick/std", "miette/fancy", "miette/std", "vm-core/std", "thiserror/std"] testing = ["dep:regex"] [dependencies] diff --git a/processor/Cargo.toml b/processor/Cargo.toml index f0e6d0710..a2867798d 100644 --- a/processor/Cargo.toml +++ b/processor/Cargo.toml @@ -20,7 +20,7 @@ doctest = false [features] concurrent = ["std", "winter-prover/concurrent"] default = ["std"] -std = ["vm-core/std", "winter-prover/std"] +std = ["vm-core/std", "winter-prover/std", "thiserror/std"] testing = ["miden-air/testing"] [dependencies] diff --git a/verifier/Cargo.toml b/verifier/Cargo.toml index 35e9bdc78..79c355607 100644 --- a/verifier/Cargo.toml +++ b/verifier/Cargo.toml @@ -19,7 +19,7 @@ doctest = false [features] default = ["std"] -std = ["air/std", "vm-core/std", "winter-verifier/std"] +std = ["air/std", "vm-core/std", "winter-verifier/std", "thiserror/std"] [dependencies] air = { package = "miden-air", path = "../air", version = "0.11", default-features = false } From 823681736fb968868efc4c1609a96f84801a4a74 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Fri, 29 Nov 2024 09:25:54 +0100 Subject: [PATCH 08/11] feat: Add static assertions for errors with `dyn Error` --- core/src/debuginfo/source_manager.rs | 12 ++++++++++++ processor/src/errors.rs | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/core/src/debuginfo/source_manager.rs b/core/src/debuginfo/source_manager.rs index d4453837d..a2176844e 100644 --- a/core/src/debuginfo/source_manager.rs +++ b/core/src/debuginfo/source_manager.rs @@ -389,3 +389,15 @@ impl SourceManager for DefaultSourceManager { .ok_or(SourceManagerError::InvalidBounds) } } + +#[cfg(test)] +mod error_assertions { + use super::*; + + /// Asserts at compile time that the passed error has Send + Sync + 'static bounds. + fn _assert_error_is_send_sync_static(_: E) {} + + fn _assert_source_manager_error_bounds(err: SourceManagerError) { + _assert_error_is_send_sync_static(err); + } +} diff --git a/processor/src/errors.rs b/processor/src/errors.rs index 251a39264..61d731930 100644 --- a/processor/src/errors.rs +++ b/processor/src/errors.rs @@ -159,3 +159,15 @@ pub enum Ext2InttError { #[error("uninitialized memory at address {0}")] UninitializedMemoryAddress(u32), } + +#[cfg(test)] +mod error_assertions { + use super::*; + + /// Asserts at compile time that the passed error has Send + Sync + 'static bounds. + fn _assert_error_is_send_sync_static(_: E) {} + + fn _assert_execution_error_bounds(err: ExecutionError) { + _assert_error_is_send_sync_static(err); + } +} From c59a1715da0711afb6a20b61a22b0d646186a568 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Fri, 29 Nov 2024 09:35:31 +0100 Subject: [PATCH 09/11] chore: Test using miette with thiserror 2 --- Cargo.lock | 106 ++++++++++++++++++-------------------------- assembly/Cargo.toml | 7 ++- core/Cargo.toml | 7 ++- 3 files changed, 54 insertions(+), 66 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a446adcbe..8e32eabae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1020,7 +1020,7 @@ dependencies = [ "lalrpop", "lalrpop-util", "miden-core", - "miden-miette", + "miette", "pretty_assertions", "regex", "rustc_version 0.4.1", @@ -1039,7 +1039,7 @@ dependencies = [ "memchr", "miden-crypto", "miden-formatting", - "miden-miette", + "miette", "num-derive", "num-traits", "parking_lot", @@ -1090,48 +1090,6 @@ dependencies = [ "winter-math", ] -[[package]] -name = "miden-miette" -version = "7.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c532250422d933f15b148fb81e4522a5d649c178ab420d0d596c86228da35570" -dependencies = [ - "backtrace", - "backtrace-ext", - "cfg-if", - "futures", - "indenter", - "lazy_static", - "miden-miette-derive", - "miden-thiserror", - "owo-colors", - "regex", - "rustc_version 0.2.3", - "rustversion", - "serde_json", - "spin", - "strip-ansi-escapes", - "supports-color", - "supports-hyperlinks", - "supports-unicode", - "syn", - "terminal_size", - "textwrap", - "trybuild", - "unicode-width 0.1.14", -] - -[[package]] -name = "miden-miette-derive" -version = "7.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1cc759f0a2947acae217a2f32f722105cacc57d17d5f93bc16362142943a4edd" -dependencies = [ - "proc-macro2", - "quote", - "syn", -] - [[package]] name = "miden-processor" version = "0.11.0" @@ -1201,26 +1159,6 @@ dependencies = [ "winter-rand-utils", ] -[[package]] -name = "miden-thiserror" -version = "1.0.59" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "183ff8de338956ecfde3a38573241eb7a6f3d44d73866c210e5629c07fa00253" -dependencies = [ - "miden-thiserror-impl", -] - -[[package]] -name = "miden-thiserror-impl" -version = "1.0.59" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ee4176a0f2e7d29d2a8ee7e60b6deb14ce67a20e94c3e2c7275cdb8804e1862" -dependencies = [ - "proc-macro2", - "quote", - "syn", -] - [[package]] name = "miden-verifier" version = "0.11.0" @@ -1263,6 +1201,46 @@ dependencies = [ "winter-fri", ] +[[package]] +name = "miette" +version = "7.1.0" +source = "git+https://github.com/0xPolygonMiden/miette?branch=pgackst-thiserror-2#eed93caaf55ca418d09b3afd8b30997bc5e7a405" +dependencies = [ + "backtrace", + "backtrace-ext", + "cfg-if", + "futures", + "indenter", + "lazy_static", + "miette-derive", + "owo-colors", + "regex", + "rustc_version 0.2.3", + "rustversion", + "serde_json", + "spin", + "strip-ansi-escapes", + "supports-color", + "supports-hyperlinks", + "supports-unicode", + "syn", + "terminal_size", + "textwrap", + "thiserror 2.0.3", + "trybuild", + "unicode-width 0.1.14", +] + +[[package]] +name = "miette-derive" +version = "7.1.0" +source = "git+https://github.com/0xPolygonMiden/miette?branch=pgackst-thiserror-2#eed93caaf55ca418d09b3afd8b30997bc5e7a405" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "miniz_oxide" version = "0.8.0" diff --git a/assembly/Cargo.toml b/assembly/Cargo.toml index a912e4879..009234b07 100644 --- a/assembly/Cargo.toml +++ b/assembly/Cargo.toml @@ -25,7 +25,12 @@ testing = ["dep:regex"] [dependencies] aho-corasick = { version = "1.1", default-features = false } lalrpop-util = { version = "0.20", default-features = false } -miette = { package = "miden-miette", version = "7.1", default-features = false, features = [ +# miette = { path = "../../miden-miette/", version = "7.1", default-features = false, features = [ +# "fancy-no-syscall", +# "derive" +# ] } +# TODO: Temporary change to test updated miden-miette. +miette = { version = "7.1", git = "https://github.com/0xPolygonMiden/miette", branch = "pgackst-thiserror-2", default-features = false,features = [ "fancy-no-syscall", "derive" ] } diff --git a/core/Cargo.toml b/core/Cargo.toml index 89c861935..843a0e03f 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -36,7 +36,12 @@ math = { package = "winter-math", version = "0.11", default-features = false } memchr = { version = "2.7", default-features = false } miden-crypto = { version = "0.13", default-features = false } miden-formatting = { version = "0.1", default-features = false } -miette = { package = "miden-miette", version = "7.1", default-features = false, optional = true, features = [ +# miette = { path = "../miden-miette/", version = "7.1", default-features = false, optional = true, features = [ +# "fancy-no-syscall", +# "derive" +# ] } +# TODO: Temporary change to test updated miden-miette. +miette = { version = "7.1", git = "https://github.com/0xPolygonMiden/miette", branch = "pgackst-thiserror-2", default-features = false, optional = true, features = [ "fancy-no-syscall", "derive" ] } From b5fd67faceb9aefe52588c25b05b412ab2e5e542 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Fri, 29 Nov 2024 10:40:29 +0100 Subject: [PATCH 10/11] feat: Remove `PartialEq, Eq, Clone` from errors --- air/src/trace/rows.rs | 3 +-- assembly/src/ast/ident.rs | 4 ++-- assembly/src/library/namespace.rs | 2 +- assembly/src/library/path.rs | 2 +- assembly/src/parser/error.rs | 2 +- verifier/Cargo.toml | 2 +- 6 files changed, 7 insertions(+), 8 deletions(-) diff --git a/air/src/trace/rows.rs b/air/src/trace/rows.rs index bd2090ae8..29411e51a 100644 --- a/air/src/trace/rows.rs +++ b/air/src/trace/rows.rs @@ -3,12 +3,11 @@ use core::{ ops::{Add, AddAssign, Bound, Index, IndexMut, Mul, RangeBounds, Sub, SubAssign}, }; -use thiserror::Error; use vm_core::Felt; /// Represents the types of errors that can occur when converting from and into [`RowIndex`] and /// using its operations. -#[derive(Debug, Error, PartialEq, Eq)] +#[derive(Debug, thiserror::Error)] pub enum RowIndexError { #[error("value is too large to be converted into RowIndex: {0}")] InvalidSize(T), diff --git a/assembly/src/ast/ident.rs b/assembly/src/ast/ident.rs index ea2d13a23..399f63a18 100644 --- a/assembly/src/ast/ident.rs +++ b/assembly/src/ast/ident.rs @@ -8,7 +8,7 @@ use core::{ use crate::{SourceSpan, Span, Spanned}; /// Represents the types of errors that can occur when parsing/validating an [Ident] -#[derive(Debug, thiserror::Error, PartialEq, Eq)] +#[derive(Debug, thiserror::Error)] pub enum IdentError { #[error("invalid identifier: cannot be empty")] Empty, @@ -24,7 +24,7 @@ pub enum IdentError { /// Represents the various types of casing errors that can occur, e.g. using an identifier /// with `SCREAMING_CASE` where one with `snake_case` is expected. -#[derive(Debug, thiserror::Error, PartialEq, Eq)] +#[derive(Debug, thiserror::Error)] pub enum CaseKindError { #[error("only uppercase characters or underscores are allowed, and must start with an alphabetic character")] Screaming, diff --git a/assembly/src/library/namespace.rs b/assembly/src/library/namespace.rs index 529718264..cc254bea0 100644 --- a/assembly/src/library/namespace.rs +++ b/assembly/src/library/namespace.rs @@ -13,7 +13,7 @@ use crate::{ // ================================================================================================ /// Represents an error when parsing or validating a library namespace -#[derive(Debug, thiserror::Error, Diagnostic, PartialEq, Eq)] +#[derive(Debug, thiserror::Error, Diagnostic)] pub enum LibraryNamespaceError { #[error("invalid library namespace name: cannot be a empty")] #[diagnostic()] diff --git a/assembly/src/library/path.rs b/assembly/src/library/path.rs index cb3ee9263..4c8f11007 100644 --- a/assembly/src/library/path.rs +++ b/assembly/src/library/path.rs @@ -18,7 +18,7 @@ use crate::{ }; /// Represents errors that can occur when creating, parsing, or manipulating [LibraryPath]s -#[derive(Debug, PartialEq, Eq, thiserror::Error)] +#[derive(Debug, thiserror::Error)] pub enum PathError { #[error("invalid library path: cannot be empty")] Empty, diff --git a/assembly/src/parser/error.rs b/assembly/src/parser/error.rs index 4b0fd1bf2..1f6278b58 100644 --- a/assembly/src/parser/error.rs +++ b/assembly/src/parser/error.rs @@ -91,7 +91,7 @@ impl fmt::Display for BinErrorKind { // PARSING ERROR // ================================================================================================ -#[derive(Debug, Default, Clone, thiserror::Error, Diagnostic)] +#[derive(Debug, Default, thiserror::Error, Diagnostic)] #[repr(u8)] pub enum ParsingError { #[default] diff --git a/verifier/Cargo.toml b/verifier/Cargo.toml index 79c355607..b83739580 100644 --- a/verifier/Cargo.toml +++ b/verifier/Cargo.toml @@ -26,4 +26,4 @@ air = { package = "miden-air", path = "../air", version = "0.11", default-featur tracing = { version = "0.1", default-features = false, features = ["attributes"] } vm-core = { package = "miden-core", path = "../core", version = "0.11", default-features = false } winter-verifier = { package = "winter-verifier", version = "0.11", default-features = false } -thiserror = { workspace = true } \ No newline at end of file +thiserror = { workspace = true } From 4001a388199c01c4ca8dae44187fa18dbc1c80d2 Mon Sep 17 00:00:00 2001 From: Philipp Gackstatter Date: Mon, 2 Dec 2024 09:14:00 +0100 Subject: [PATCH 11/11] chore: Use miden-miette 8.0 --- Cargo.lock | 86 +++++++++++++++++++++++---------------------- assembly/Cargo.toml | 7 +--- core/Cargo.toml | 7 +--- 3 files changed, 46 insertions(+), 54 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8e32eabae..dbe6e24e5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1020,7 +1020,7 @@ dependencies = [ "lalrpop", "lalrpop-util", "miden-core", - "miette", + "miden-miette", "pretty_assertions", "regex", "rustc_version 0.4.1", @@ -1039,7 +1039,7 @@ dependencies = [ "memchr", "miden-crypto", "miden-formatting", - "miette", + "miden-miette", "num-derive", "num-traits", "parking_lot", @@ -1090,6 +1090,48 @@ dependencies = [ "winter-math", ] +[[package]] +name = "miden-miette" +version = "8.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eef536978f24a179d94fa2a41e4f92b28e7d8aab14b8d23df28ad2a3d7098b20" +dependencies = [ + "backtrace", + "backtrace-ext", + "cfg-if", + "futures", + "indenter", + "lazy_static", + "miden-miette-derive", + "owo-colors", + "regex", + "rustc_version 0.2.3", + "rustversion", + "serde_json", + "spin", + "strip-ansi-escapes", + "supports-color", + "supports-hyperlinks", + "supports-unicode", + "syn", + "terminal_size", + "textwrap", + "thiserror 2.0.3", + "trybuild", + "unicode-width 0.1.14", +] + +[[package]] +name = "miden-miette-derive" +version = "8.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "86a905f3ea65634dd4d1041a4f0fd0a3e77aa4118341d265af1a94339182222f" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "miden-processor" version = "0.11.0" @@ -1201,46 +1243,6 @@ dependencies = [ "winter-fri", ] -[[package]] -name = "miette" -version = "7.1.0" -source = "git+https://github.com/0xPolygonMiden/miette?branch=pgackst-thiserror-2#eed93caaf55ca418d09b3afd8b30997bc5e7a405" -dependencies = [ - "backtrace", - "backtrace-ext", - "cfg-if", - "futures", - "indenter", - "lazy_static", - "miette-derive", - "owo-colors", - "regex", - "rustc_version 0.2.3", - "rustversion", - "serde_json", - "spin", - "strip-ansi-escapes", - "supports-color", - "supports-hyperlinks", - "supports-unicode", - "syn", - "terminal_size", - "textwrap", - "thiserror 2.0.3", - "trybuild", - "unicode-width 0.1.14", -] - -[[package]] -name = "miette-derive" -version = "7.1.0" -source = "git+https://github.com/0xPolygonMiden/miette?branch=pgackst-thiserror-2#eed93caaf55ca418d09b3afd8b30997bc5e7a405" -dependencies = [ - "proc-macro2", - "quote", - "syn", -] - [[package]] name = "miniz_oxide" version = "0.8.0" diff --git a/assembly/Cargo.toml b/assembly/Cargo.toml index 009234b07..86276f8f4 100644 --- a/assembly/Cargo.toml +++ b/assembly/Cargo.toml @@ -25,12 +25,7 @@ testing = ["dep:regex"] [dependencies] aho-corasick = { version = "1.1", default-features = false } lalrpop-util = { version = "0.20", default-features = false } -# miette = { path = "../../miden-miette/", version = "7.1", default-features = false, features = [ -# "fancy-no-syscall", -# "derive" -# ] } -# TODO: Temporary change to test updated miden-miette. -miette = { version = "7.1", git = "https://github.com/0xPolygonMiden/miette", branch = "pgackst-thiserror-2", default-features = false,features = [ +miette = { package = "miden-miette", version = "8.0", default-features = false, features = [ "fancy-no-syscall", "derive" ] } diff --git a/core/Cargo.toml b/core/Cargo.toml index 843a0e03f..e840b429b 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -36,12 +36,7 @@ math = { package = "winter-math", version = "0.11", default-features = false } memchr = { version = "2.7", default-features = false } miden-crypto = { version = "0.13", default-features = false } miden-formatting = { version = "0.1", default-features = false } -# miette = { path = "../miden-miette/", version = "7.1", default-features = false, optional = true, features = [ -# "fancy-no-syscall", -# "derive" -# ] } -# TODO: Temporary change to test updated miden-miette. -miette = { version = "7.1", git = "https://github.com/0xPolygonMiden/miette", branch = "pgackst-thiserror-2", default-features = false, optional = true, features = [ +miette = { package = "miden-miette", version = "8.0", default-features = false, optional = true, features = [ "fancy-no-syscall", "derive" ] }