diff --git a/Cargo.lock b/Cargo.lock index f13f1556f0..724453228e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1743,6 +1743,7 @@ dependencies = [ "rand", "rand_chacha", "rstest", + "thiserror", "tokio", "winter-rand-utils", "winterfell", diff --git a/crates/miden-testing/Cargo.toml b/crates/miden-testing/Cargo.toml index af565ab386..23bea98751 100644 --- a/crates/miden-testing/Cargo.toml +++ b/crates/miden-testing/Cargo.toml @@ -34,6 +34,7 @@ anyhow = { workspace = true } itertools = { default-features = false, features = ["use_alloc"], version = "0.14" } rand = { features = ["os_rng", "small_rng"], workspace = true } rand_chacha = { workspace = true } +thiserror = { workspace = true } winterfell = { version = "0.13" } [dev-dependencies] diff --git a/crates/miden-testing/src/kernel_tests/tx/test_account.rs b/crates/miden-testing/src/kernel_tests/tx/test_account.rs index d0146be16b..d315b9c7c0 100644 --- a/crates/miden-testing/src/kernel_tests/tx/test_account.rs +++ b/crates/miden-testing/src/kernel_tests/tx/test_account.rs @@ -23,7 +23,8 @@ use miden_protocol::account::{ StorageSlotName, StorageSlotType, }; -use miden_protocol::assembly::diagnostics::{IntoDiagnostic, NamedSource, Report, WrapErr, miette}; +use miden_protocol::assembly::diagnostics::NamedSource; +use miden_protocol::assembly::diagnostics::reporting::PrintDiagnostic; use miden_protocol::assembly::{DefaultSourceManager, Library}; use miden_protocol::asset::{Asset, FungibleAsset}; use miden_protocol::errors::tx_kernel::{ @@ -73,7 +74,7 @@ use crate::{ // ================================================================================================ #[tokio::test] -pub async fn compute_commitment() -> miette::Result<()> { +pub async fn compute_commitment() -> anyhow::Result<()> { let account = Account::mock(ACCOUNT_ID_REGULAR_PRIVATE_ACCOUNT_UPDATABLE_CODE, Auth::IncrNonce); // Precompute a commitment to a changed account so we can assert it during tx script execution. @@ -144,19 +145,13 @@ pub async fn compute_commitment() -> miette::Result<()> { ); let tx_context_builder = TransactionContextBuilder::new(account); - let tx_script = CodeBuilder::with_mock_libraries() - .compile_tx_script(tx_script) - .into_diagnostic()?; - let tx_context = tx_context_builder - .tx_script(tx_script) - .build() - .map_err(|err| miette::miette!("{err}"))?; + let tx_script = CodeBuilder::with_mock_libraries().compile_tx_script(tx_script)?; + let tx_context = tx_context_builder.tx_script(tx_script).build()?; tx_context .execute() .await - .into_diagnostic() - .wrap_err("failed to execute transaction")?; + .map_err(|err| anyhow::anyhow!("failed to execute transaction: {err}"))?; Ok(()) } @@ -165,7 +160,7 @@ pub async fn compute_commitment() -> miette::Result<()> { // ================================================================================================ #[tokio::test] -async fn test_account_type() -> miette::Result<()> { +async fn test_account_type() -> anyhow::Result<()> { let procedures = vec![ ("is_fungible_faucet", AccountType::FungibleFaucet), ("is_non_fungible_faucet", AccountType::NonFungibleFaucet), @@ -197,9 +192,7 @@ async fn test_account_type() -> miette::Result<()> { ); let exec_output = CodeExecutor::with_default_host() - .stack_inputs( - StackInputs::new(vec![account_id.prefix().as_felt()]).into_diagnostic()?, - ) + .stack_inputs(StackInputs::new(vec![account_id.prefix().as_felt()])?) .run(&code) .await?; @@ -225,7 +218,7 @@ async fn test_account_type() -> miette::Result<()> { } #[tokio::test] -async fn test_account_validate_id() -> miette::Result<()> { +async fn test_account_validate_id() -> anyhow::Result<()> { let test_cases = [ (ACCOUNT_ID_REGULAR_PUBLIC_ACCOUNT_IMMUTABLE_CODE, None), (ACCOUNT_ID_REGULAR_PRIVATE_ACCOUNT_UPDATABLE_CODE, None), @@ -275,23 +268,27 @@ async fn test_account_validate_id() -> miette::Result<()> { match (result, expected_error) { (Ok(_), None) => (), (Ok(_), Some(err)) => { - miette::bail!("expected error {err} but validation was successful") + anyhow::bail!("expected error {err} but validation was successful") }, (Err(ExecutionError::FailedAssertion { err_code, err_msg, .. }), Some(err)) => { if err_code != err.code() { - miette::bail!( + anyhow::bail!( "actual error \"{}\" (code: {err_code}) did not match expected error {err}", err_msg.as_ref().map(AsRef::as_ref).unwrap_or("") ); } }, - // Construct Reports to get the diagnostics-based error messages. (Err(err), None) => { - return Err(Report::from(err) - .context("validation is supposed to succeed but error occurred")); + return Err(anyhow::anyhow!( + "validation is supposed to succeed but error occurred: {}", + PrintDiagnostic::new(&err) + )); }, (Err(err), Some(_)) => { - return Err(Report::from(err).context("unexpected different error than expected")); + return Err(anyhow::anyhow!( + "unexpected different error than expected: {}", + PrintDiagnostic::new(&err) + )); }, } } @@ -300,7 +297,7 @@ async fn test_account_validate_id() -> miette::Result<()> { } #[tokio::test] -async fn test_is_faucet_procedure() -> miette::Result<()> { +async fn test_is_faucet_procedure() -> anyhow::Result<()> { let test_cases = [ ACCOUNT_ID_REGULAR_PUBLIC_ACCOUNT_IMMUTABLE_CODE, ACCOUNT_ID_REGULAR_PRIVATE_ACCOUNT_UPDATABLE_CODE, @@ -327,10 +324,9 @@ async fn test_is_faucet_procedure() -> miette::Result<()> { prefix = account_id.prefix().as_felt(), ); - let exec_output = CodeExecutor::with_default_host() - .run(&code) - .await - .wrap_err("failed to execute is_faucet procedure")?; + let exec_output = CodeExecutor::with_default_host().run(&code).await.map_err(|err| { + anyhow::anyhow!("failed to execute is_faucet procedure: {}", PrintDiagnostic::new(&err)) + })?; let is_faucet = account_id.is_faucet(); assert_eq!( @@ -348,7 +344,7 @@ async fn test_is_faucet_procedure() -> miette::Result<()> { // TODO: update this test once the ability to change the account code will be implemented #[tokio::test] -pub async fn test_compute_code_commitment() -> miette::Result<()> { +pub async fn test_compute_code_commitment() -> anyhow::Result<()> { let tx_context = TransactionContextBuilder::with_existing_mock_account().build().unwrap(); let account = tx_context.account(); @@ -377,7 +373,7 @@ pub async fn test_compute_code_commitment() -> miette::Result<()> { // ================================================================================================ #[tokio::test] -async fn test_get_item() -> miette::Result<()> { +async fn test_get_item() -> anyhow::Result<()> { for storage_item in [AccountStorage::mock_value_slot0(), AccountStorage::mock_value_slot1()] { let tx_context = TransactionContextBuilder::with_existing_mock_account().build().unwrap(); @@ -412,7 +408,7 @@ async fn test_get_item() -> miette::Result<()> { } #[tokio::test] -async fn test_get_map_item() -> miette::Result<()> { +async fn test_get_map_item() -> anyhow::Result<()> { let slot = AccountStorage::mock_map_slot(); let account = AccountBuilder::new(ChaCha20Rng::from_os_rng().random()) .with_auth_component(Auth::IncrNonce) @@ -459,7 +455,7 @@ async fn test_get_map_item() -> miette::Result<()> { } #[tokio::test] -async fn test_get_storage_slot_type() -> miette::Result<()> { +async fn test_get_storage_slot_type() -> anyhow::Result<()> { for slot_name in [ AccountStorage::mock_value_slot0().name(), AccountStorage::mock_value_slot1().name(), @@ -604,7 +600,7 @@ async fn test_account_set_item_fails_on_reserved_faucet_metadata_slot() -> anyho } #[tokio::test] -async fn test_is_slot_id_lt() -> miette::Result<()> { +async fn test_is_slot_id_lt() -> anyhow::Result<()> { // Note that the slot IDs derived from the names are essentially randomly sorted, so these cover // "less than" and "greater than" outcomes. let mut test_cases = (0..100) @@ -711,7 +707,7 @@ async fn test_set_item() -> anyhow::Result<()> { } #[tokio::test] -async fn test_set_map_item() -> miette::Result<()> { +async fn test_set_map_item() -> anyhow::Result<()> { let (new_key, new_value) = (Word::from([109, 110, 111, 112u32]), Word::from([9, 10, 11, 12u32])); @@ -1320,7 +1316,7 @@ async fn test_get_init_balance_subtraction() -> anyhow::Result<()> { // ================================================================================================ #[tokio::test] -async fn test_authenticate_and_track_procedure() -> miette::Result<()> { +async fn test_authenticate_and_track_procedure() -> anyhow::Result<()> { let mock_component = MockAccountComponent::with_empty_slots(); let account_code = AccountCode::from_components( @@ -1379,7 +1375,7 @@ async fn test_authenticate_and_track_procedure() -> miette::Result<()> { // ================================================================================================ #[tokio::test] -async fn test_was_procedure_called() -> miette::Result<()> { +async fn test_was_procedure_called() -> anyhow::Result<()> { // Create a standard account using the mock component let mock_component = MockAccountComponent::with_slots(AccountStorage::mock_storage_slots()); let account = AccountBuilder::new(ChaCha20Rng::from_os_rng().random()) @@ -1429,9 +1425,7 @@ async fn test_was_procedure_called() -> miette::Result<()> { ); // Compile the transaction script using the testing assembler with mock account - let tx_script = CodeBuilder::with_mock_libraries() - .compile_tx_script(tx_script_code) - .into_diagnostic()?; + let tx_script = CodeBuilder::with_mock_libraries().compile_tx_script(tx_script_code)?; // Create transaction context and execute let tx_context = TransactionContextBuilder::new(account).tx_script(tx_script).build().unwrap(); @@ -1439,8 +1433,7 @@ async fn test_was_procedure_called() -> miette::Result<()> { tx_context .execute() .await - .into_diagnostic() - .wrap_err("Failed to execute transaction")?; + .map_err(|err| anyhow::anyhow!("Failed to execute transaction: {err}"))?; Ok(()) } @@ -1451,7 +1444,7 @@ async fn test_was_procedure_called() -> miette::Result<()> { /// The call chain and dependency graph in this test is: /// `tx script -> account code -> external library` #[tokio::test] -async fn transaction_executor_account_code_using_custom_library() -> miette::Result<()> { +async fn transaction_executor_account_code_using_custom_library() -> anyhow::Result<()> { let external_library_code = format!( r#" use miden::protocol::native_account @@ -1476,15 +1469,20 @@ async fn transaction_executor_account_code_using_custom_library() -> miette::Res let external_library_source = NamedSource::new("external_library::external_module", external_library_code); - let external_library = - TransactionKernel::assembler().assemble_library([external_library_source])?; + let external_library = TransactionKernel::assembler() + .assemble_library([external_library_source]) + .map_err(|err| { + anyhow::anyhow!("failed to assemble library: {}", PrintDiagnostic::new(&err)) + })?; let mut assembler: miden_protocol::assembly::Assembler = CodeBuilder::with_mock_libraries_with_source_manager(Arc::new( DefaultSourceManager::default(), )) .into(); - assembler.link_static_library(&external_library)?; + assembler.link_static_library(&external_library).map_err(|err| { + anyhow::anyhow!("failed to link static library: {}", PrintDiagnostic::new(&err)) + })?; let account_component_source = NamedSource::new("account_component::account_module", ACCOUNT_COMPONENT_CODE); @@ -1499,29 +1497,25 @@ async fn transaction_executor_account_code_using_custom_library() -> miette::Res end"; let account_component = - AccountComponent::new(account_component_lib.clone(), AccountStorage::mock_storage_slots()) - .into_diagnostic()? + AccountComponent::new(account_component_lib.clone(), AccountStorage::mock_storage_slots())? .with_supports_all_types(); // Build an existing account with nonce 1. let native_account = AccountBuilder::new(ChaCha20Rng::from_os_rng().random()) .with_auth_component(Auth::IncrNonce) .with_component(account_component) - .build_existing() - .into_diagnostic()?; + .build_existing()?; let tx_script = CodeBuilder::default() - .with_dynamically_linked_library(&account_component_lib) - .into_diagnostic()? - .compile_tx_script(tx_script_src) - .into_diagnostic()?; + .with_dynamically_linked_library(&account_component_lib)? + .compile_tx_script(tx_script_src)?; let tx_context = TransactionContextBuilder::new(native_account.clone()) .tx_script(tx_script) .build() .unwrap(); - let executed_tx = tx_context.execute().await.into_diagnostic()?; + let executed_tx = tx_context.execute().await?; // Account's initial nonce of 1 should have been incremented by 1. assert_eq!(executed_tx.account_delta().nonce_delta(), Felt::new(1)); @@ -1565,7 +1559,7 @@ async fn incrementing_nonce_twice_fails() -> anyhow::Result<()> { } #[tokio::test] -async fn test_has_procedure() -> miette::Result<()> { +async fn test_has_procedure() -> anyhow::Result<()> { // Create a standard account using the mock component let mock_component = MockAccountComponent::with_slots(AccountStorage::mock_storage_slots()); let account = AccountBuilder::new(ChaCha20Rng::from_os_rng().random()) @@ -1603,7 +1597,7 @@ async fn test_has_procedure() -> miette::Result<()> { // Compile the transaction script using the testing assembler with mock account let tx_script = CodeBuilder::with_mock_libraries() .compile_tx_script(tx_script_code) - .into_diagnostic()?; + .map_err(|err| anyhow::anyhow!("{err}"))?; // Create transaction context and execute let tx_context = TransactionContextBuilder::new(account).tx_script(tx_script).build().unwrap(); @@ -1611,8 +1605,7 @@ async fn test_has_procedure() -> miette::Result<()> { tx_context .execute() .await - .into_diagnostic() - .wrap_err("Failed to execute transaction")?; + .map_err(|err| anyhow::anyhow!("Failed to execute transaction: {err}"))?; Ok(()) } @@ -1621,7 +1614,7 @@ async fn test_has_procedure() -> miette::Result<()> { // ================================================================================================ #[tokio::test] -async fn test_get_initial_item() -> miette::Result<()> { +async fn test_get_initial_item() -> anyhow::Result<()> { let tx_context = TransactionContextBuilder::with_existing_mock_account().build().unwrap(); // Test that get_initial_item returns the initial value before any changes @@ -1671,7 +1664,7 @@ async fn test_get_initial_item() -> miette::Result<()> { } #[tokio::test] -async fn test_get_initial_map_item() -> miette::Result<()> { +async fn test_get_initial_map_item() -> anyhow::Result<()> { let map_slot = AccountStorage::mock_map_slot(); let account = AccountBuilder::new(ChaCha20Rng::from_os_rng().random()) .with_auth_component(Auth::IncrNonce) diff --git a/crates/miden-testing/src/kernel_tests/tx/test_note.rs b/crates/miden-testing/src/kernel_tests/tx/test_note.rs index 49a5be0c60..6f746e78fb 100644 --- a/crates/miden-testing/src/kernel_tests/tx/test_note.rs +++ b/crates/miden-testing/src/kernel_tests/tx/test_note.rs @@ -6,7 +6,6 @@ use miden_processor::fast::ExecutionOutput; use miden_protocol::account::auth::PublicKeyCommitment; use miden_protocol::account::{AccountBuilder, AccountId}; use miden_protocol::assembly::DefaultSourceManager; -use miden_protocol::assembly::diagnostics::miette::{self, miette}; use miden_protocol::asset::FungibleAsset; use miden_protocol::crypto::dsa::falcon512_rpo::SecretKey; use miden_protocol::crypto::rand::{FeltRng, RpoRandomCoin}; @@ -87,27 +86,23 @@ async fn test_note_setup() -> anyhow::Result<()> { } #[tokio::test] -async fn test_note_script_and_note_args() -> miette::Result<()> { +async fn test_note_script_and_note_args() -> anyhow::Result<()> { let mut tx_context = { let mut builder = MockChain::builder(); - let account = builder.add_existing_wallet(Auth::BasicAuth).map_err(|err| miette!(err))?; - let p2id_note_1 = builder - .add_p2id_note( - ACCOUNT_ID_SENDER.try_into().unwrap(), - account.id(), - &[FungibleAsset::mock(150)], - NoteType::Public, - ) - .map_err(|err| miette!(err))?; - let p2id_note_2 = builder - .add_p2id_note( - ACCOUNT_ID_SENDER.try_into().unwrap(), - account.id(), - &[FungibleAsset::mock(300)], - NoteType::Public, - ) - .map_err(|err| miette!(err))?; - let mut mock_chain = builder.build().map_err(|err| miette!(err))?; + let account = builder.add_existing_wallet(Auth::BasicAuth)?; + let p2id_note_1 = builder.add_p2id_note( + ACCOUNT_ID_SENDER.try_into().unwrap(), + account.id(), + &[FungibleAsset::mock(150)], + NoteType::Public, + )?; + let p2id_note_2 = builder.add_p2id_note( + ACCOUNT_ID_SENDER.try_into().unwrap(), + account.id(), + &[FungibleAsset::mock(300)], + NoteType::Public, + )?; + let mut mock_chain = builder.build()?; mock_chain.prove_next_block().unwrap(); mock_chain @@ -380,12 +375,12 @@ async fn test_compute_inputs_commitment() -> anyhow::Result<()> { } #[tokio::test] -async fn test_build_metadata_header() -> miette::Result<()> { +async fn test_build_metadata_header() -> anyhow::Result<()> { let tx_context = TransactionContextBuilder::with_existing_mock_account().build().unwrap(); let sender = tx_context.account().id(); let receiver = AccountId::try_from(ACCOUNT_ID_REGULAR_PRIVATE_ACCOUNT_UPDATABLE_CODE) - .map_err(|e| miette::miette!("Failed to convert account ID: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to convert account ID: {}", e))?; let test_metadata1 = NoteMetadata::new(sender, NoteType::Private, NoteTag::with_account_target(receiver)); diff --git a/crates/miden-testing/src/lib.rs b/crates/miden-testing/src/lib.rs index fbbd4402b8..e24c159bd4 100644 --- a/crates/miden-testing/src/lib.rs +++ b/crates/miden-testing/src/lib.rs @@ -17,7 +17,7 @@ pub use mock_chain::{ }; mod tx_context; -pub use tx_context::{TransactionContext, TransactionContextBuilder}; +pub use tx_context::{ExecError, TransactionContext, TransactionContextBuilder}; pub mod executor; diff --git a/crates/miden-testing/src/tx_context/context.rs b/crates/miden-testing/src/tx_context/context.rs index 274b16672b..e93c13d61a 100644 --- a/crates/miden-testing/src/tx_context/context.rs +++ b/crates/miden-testing/src/tx_context/context.rs @@ -4,7 +4,7 @@ use alloc::sync::Arc; use alloc::vec::Vec; use miden_processor::fast::ExecutionOutput; -use miden_processor::{ExecutionError, FutureMaybeSend, MastForest, MastForestStore, Word}; +use miden_processor::{FutureMaybeSend, MastForest, MastForestStore, Word}; use miden_protocol::account::{ Account, AccountId, @@ -43,6 +43,7 @@ use miden_tx::{ use crate::executor::CodeExecutor; use crate::mock_host::MockHost; +use crate::tx_context::ExecError; // TRANSACTION CONTEXT // ================================================================================================ @@ -72,10 +73,6 @@ impl TransactionContext { /// is run on a modified [`TransactionExecutorHost`] which is loaded with the procedures exposed /// by the transaction kernel, and also individual kernel functions (not normally exposed). /// - /// To improve the error message quality, convert the returned [`ExecutionError`] into a - /// [`Report`](miden_protocol::assembly::diagnostics::Report) or use `?` with - /// [`miden_protocol::assembly::diagnostics::Result`]. - /// /// # Errors /// /// Returns an error if the assembly or execution of the provided code fails. @@ -83,7 +80,7 @@ impl TransactionContext { /// # Panics /// /// - If the provided `code` is not a valid program. - pub async fn execute_code(&self, code: &str) -> Result { + pub async fn execute_code(&self, code: &str) -> Result { // Fetch all witnesses for note assets and the fee asset. let mut asset_vault_keys = self .tx_inputs @@ -177,6 +174,7 @@ impl TransactionContext { .extend_advice_inputs(advice_inputs) .execute_program(program) .await + .map_err(ExecError::new) } /// Executes the transaction through a [TransactionExecutor] diff --git a/crates/miden-testing/src/tx_context/errors.rs b/crates/miden-testing/src/tx_context/errors.rs new file mode 100644 index 0000000000..c3f1a653d0 --- /dev/null +++ b/crates/miden-testing/src/tx_context/errors.rs @@ -0,0 +1,31 @@ +use alloc::string::ToString; + +use miden_processor::ExecutionError; +use miden_protocol::assembly::diagnostics::reporting::PrintDiagnostic; +use thiserror::Error; + +// EXECUTION ERROR +// ================================================================================================ + +/// A newtype wrapper around [`ExecutionError`] that provides better error messages +/// by using [`PrintDiagnostic`] for display formatting. +#[derive(Debug, Error)] +#[error("{}", PrintDiagnostic::new(.0).to_string())] +pub struct ExecError(pub ExecutionError); + +impl ExecError { + /// Creates a new `ExecError` from an `ExecutionError`. + pub fn new(error: ExecutionError) -> Self { + Self(error) + } + + /// Returns a reference to the inner `ExecutionError`. + pub fn as_execution_error(&self) -> &ExecutionError { + &self.0 + } + + /// Consumes `ExecError` and returns the inner `ExecutionError`. + pub fn into_execution_error(self) -> ExecutionError { + self.0 + } +} diff --git a/crates/miden-testing/src/tx_context/mod.rs b/crates/miden-testing/src/tx_context/mod.rs index 787c13d36e..30cb008889 100644 --- a/crates/miden-testing/src/tx_context/mod.rs +++ b/crates/miden-testing/src/tx_context/mod.rs @@ -1,5 +1,7 @@ mod builder; mod context; +mod errors; pub use builder::TransactionContextBuilder; pub use context::TransactionContext; +pub use errors::ExecError; diff --git a/crates/miden-testing/src/utils.rs b/crates/miden-testing/src/utils.rs index 30aeb8fe76..1732ff3580 100644 --- a/crates/miden-testing/src/utils.rs +++ b/crates/miden-testing/src/utils.rs @@ -19,7 +19,7 @@ use rand::rngs::SmallRng; macro_rules! assert_execution_error { ($execution_result:expr, $expected_err:expr) => { match $execution_result { - Err(miden_processor::ExecutionError::FailedAssertion { label: _, source_file: _, clk: _, err_code, err_msg, err: _ }) => { + Err($crate::ExecError(miden_processor::ExecutionError::FailedAssertion { label: _, source_file: _, clk: _, err_code, err_msg, err: _ })) => { if let Some(ref msg) = err_msg { assert_eq!(msg.as_ref(), $expected_err.message(), "error messages did not match"); }