From 2e6a06beb9aeafd22b6ab37eecd097dcc977c71f Mon Sep 17 00:00:00 2001 From: Dori Medini Date: Mon, 21 Oct 2024 11:31:00 +0300 Subject: [PATCH] feat(blockifier): keep revert error structure in TransactionExecutionInfo --- crates/batcher/src/block_builder_test.rs | 4 +- .../src/concurrency/worker_logic_test.rs | 2 +- .../blockifier/src/execution/stack_trace.rs | 70 +++++++++++++++---- crates/blockifier/src/fee/fee_checks.rs | 3 +- .../src/transaction/account_transaction.rs | 15 ++-- .../transaction/account_transactions_test.rs | 27 +++++-- crates/blockifier/src/transaction/errors.rs | 15 +--- .../src/transaction/execution_flavors_test.rs | 22 ++++-- crates/blockifier/src/transaction/objects.rs | 30 +++++++- .../src/transaction/post_execution_test.rs | 15 +++- .../src/transaction/transactions_test.rs | 9 ++- .../src/py_block_executor.rs | 2 +- .../papyrus_execution/src/execution_test.rs | 11 +-- crates/papyrus_execution/src/lib.rs | 5 +- crates/papyrus_execution/src/objects.rs | 2 +- 15 files changed, 171 insertions(+), 61 deletions(-) diff --git a/crates/batcher/src/block_builder_test.rs b/crates/batcher/src/block_builder_test.rs index 9c7de8a1f61..61bc8c039c0 100644 --- a/crates/batcher/src/block_builder_test.rs +++ b/crates/batcher/src/block_builder_test.rs @@ -3,7 +3,7 @@ use blockifier::blockifier::transaction_executor::{ TransactionExecutorResult, }; use blockifier::bouncer::BouncerWeights; -use blockifier::transaction::objects::TransactionExecutionInfo; +use blockifier::transaction::objects::{RevertError, TransactionExecutionInfo}; use blockifier::transaction::transaction_execution::Transaction as BlockifierTransaction; use indexmap::IndexMap; use rstest::{fixture, rstest}; @@ -191,5 +191,5 @@ fn block_builder_expected_output( } fn execution_info() -> TransactionExecutionInfo { - TransactionExecutionInfo { revert_error: Some("Test string".to_string()), ..Default::default() } + TransactionExecutionInfo { revert_error: Some(RevertError::default()), ..Default::default() } } diff --git a/crates/blockifier/src/concurrency/worker_logic_test.rs b/crates/blockifier/src/concurrency/worker_logic_test.rs index 134324d0bf9..bc6530f30d9 100644 --- a/crates/blockifier/src/concurrency/worker_logic_test.rs +++ b/crates/blockifier/src/concurrency/worker_logic_test.rs @@ -611,7 +611,7 @@ fn test_deploy_before_declare( let execution_output = worker_executor.execution_outputs[1].lock().unwrap(); let tx_execution_info = execution_output.as_ref().unwrap().result.as_ref().unwrap(); assert!(tx_execution_info.is_reverted()); - assert!(tx_execution_info.revert_error.clone().unwrap().contains("not declared.")); + assert!(tx_execution_info.revert_error.clone().unwrap().to_string().contains("not declared.")); drop(execution_output); // Creates 2 active tasks. diff --git a/crates/blockifier/src/execution/stack_trace.rs b/crates/blockifier/src/execution/stack_trace.rs index 9c12308a813..3165f1e32bc 100644 --- a/crates/blockifier/src/execution/stack_trace.rs +++ b/crates/blockifier/src/execution/stack_trace.rs @@ -23,6 +23,7 @@ pub const TRACE_LENGTH_CAP: usize = 15000; pub const TRACE_EXTRA_CHARS_SLACK: usize = 100; #[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))] +#[derive(Clone, Debug, PartialEq)] pub enum PreambleType { CallContract, LibraryCall, @@ -39,7 +40,9 @@ impl PreambleType { } } +#[cfg_attr(any(test, feature = "testing"), derive(Clone))] #[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))] +#[derive(Debug, PartialEq)] pub struct EntryPointErrorFrame { pub depth: usize, pub preamble_type: PreambleType, @@ -71,7 +74,9 @@ impl From<&EntryPointErrorFrame> for String { } } +#[cfg_attr(any(test, feature = "testing"), derive(Clone))] #[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))] +#[derive(Debug, PartialEq)] pub struct VmExceptionFrame { pub pc: Relocatable, pub error_attr_value: Option, @@ -94,7 +99,9 @@ impl From<&VmExceptionFrame> for String { } } +#[cfg_attr(any(test, feature = "testing"), derive(Clone))] #[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))] +#[derive(Debug, PartialEq)] pub enum Frame { EntryPoint(EntryPointErrorFrame), Cairo1Tail(Cairo1RevertStack), @@ -138,24 +145,52 @@ impl From for Frame { } #[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))] -#[derive(Default)] +#[derive(Clone, Debug, Default, PartialEq)] +pub enum ErrorStackHeader { + Constructor, + Execution, + Validation, + #[default] + None, +} + +impl Display for ErrorStackHeader { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!( + f, + "{}", + match self { + Self::Constructor => "Contract constructor execution has failed:\n", + Self::Execution => "Transaction execution has failed:\n", + Self::Validation => "Transaction validation has failed:\n", + Self::None => "", + } + ) + } +} + +#[cfg_attr(any(test, feature = "testing"), derive(Clone))] +#[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))] +#[derive(Debug, Default, PartialEq)] pub struct ErrorStack { + pub header: ErrorStackHeader, pub stack: Vec, } -impl From for String { - fn from(value: ErrorStack) -> Self { - let error_stack_str = value.stack.iter().map(String::from).join("\n"); +impl Display for ErrorStack { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + let error_stack_str = self.stack.iter().map(String::from).join("\n"); // When the trace string is too long, trim it in a way that keeps both the beginning and // end. - if error_stack_str.len() > TRACE_LENGTH_CAP + TRACE_EXTRA_CHARS_SLACK { + let final_str = if error_stack_str.len() > TRACE_LENGTH_CAP + TRACE_EXTRA_CHARS_SLACK { error_stack_str[..(TRACE_LENGTH_CAP / 2)].to_string() + "\n\n...\n\n" + &error_stack_str[(error_stack_str.len() - TRACE_LENGTH_CAP / 2)..] } else { error_stack_str - } + }; + write!(f, "{}{}", self.header, final_str) } } @@ -164,8 +199,9 @@ impl ErrorStack { self.stack.push(frame); } } + #[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))] -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] pub struct Cairo1RevertFrame { pub contract_address: ContractAddress, pub class_hash: Option, @@ -198,7 +234,7 @@ impl Display for Cairo1RevertFrame { } #[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))] -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, PartialEq)] pub enum Cairo1RevertHeader { Execution, Validation, @@ -218,7 +254,7 @@ impl Display for Cairo1RevertHeader { } #[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))] -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] pub struct Cairo1RevertStack { pub header: Cairo1RevertHeader, pub stack: Vec, @@ -314,13 +350,21 @@ pub fn gen_tx_execution_error_trace(error: &TransactionExecutionError) -> ErrorS class_hash, storage_address, selector, - } - | TransactionExecutionError::ValidateTransactionError { + } => gen_error_trace_from_entry_point_error( + ErrorStackHeader::Execution, + error, + storage_address, + class_hash, + Some(selector), + PreambleType::CallContract, + ), + TransactionExecutionError::ValidateTransactionError { error, class_hash, storage_address, selector, } => gen_error_trace_from_entry_point_error( + ErrorStackHeader::Validation, error, storage_address, class_hash, @@ -335,6 +379,7 @@ pub fn gen_tx_execution_error_trace(error: &TransactionExecutionError) -> ErrorS constructor_selector, }, ) => gen_error_trace_from_entry_point_error( + ErrorStackHeader::Constructor, error, storage_address, class_hash, @@ -357,13 +402,14 @@ pub fn gen_tx_execution_error_trace(error: &TransactionExecutionError) -> ErrorS /// Generate error stack from top-level entry point execution error. fn gen_error_trace_from_entry_point_error( + header: ErrorStackHeader, error: &EntryPointExecutionError, storage_address: &ContractAddress, class_hash: &ClassHash, entry_point_selector: Option<&EntryPointSelector>, preamble_type: PreambleType, ) -> ErrorStack { - let mut error_stack: ErrorStack = ErrorStack::default(); + let mut error_stack = ErrorStack { header, ..Default::default() }; let depth = 0; error_stack.push( EntryPointErrorFrame { diff --git a/crates/blockifier/src/fee/fee_checks.rs b/crates/blockifier/src/fee/fee_checks.rs index 50a3820b1b8..6be4380dc26 100644 --- a/crates/blockifier/src/fee/fee_checks.rs +++ b/crates/blockifier/src/fee/fee_checks.rs @@ -11,7 +11,8 @@ use crate::state::state_api::StateReader; use crate::transaction::errors::TransactionExecutionError; use crate::transaction::objects::{FeeType, TransactionExecutionResult, TransactionInfo}; -#[derive(Clone, Copy, Debug, Error)] +#[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))] +#[derive(Clone, Copy, Debug, Error, PartialEq)] pub enum FeeCheckError { #[error( "Insufficient max {resource}: max amount: {max_amount}, actual used: {actual_amount}." diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index b25c1238bbd..c4beb1c1ca0 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -26,7 +26,11 @@ use crate::context::{BlockContext, TransactionContext}; use crate::execution::call_info::{CallInfo, Retdata}; use crate::execution::contract_class::ContractClass; use crate::execution::entry_point::{CallEntryPoint, CallType, EntryPointExecutionContext}; -use crate::execution::stack_trace::{extract_trailing_cairo1_revert_trace, Cairo1RevertHeader}; +use crate::execution::stack_trace::{ + extract_trailing_cairo1_revert_trace, + gen_tx_execution_error_trace, + Cairo1RevertHeader, +}; use crate::fee::fee_checks::{FeeCheckReportFields, PostExecutionReport}; use crate::fee::fee_utils::{ get_fee_by_gas_vector, @@ -47,6 +51,7 @@ use crate::transaction::errors::{ use crate::transaction::objects::{ DeprecatedTransactionInfo, HasRelatedFeeType, + RevertError, TransactionExecutionInfo, TransactionExecutionResult, TransactionInfo, @@ -720,7 +725,7 @@ impl AccountTransaction { execution_state.abort(); Ok(ValidateExecuteCallInfo::new_reverted( validate_call_info, - post_execution_error.to_string(), + post_execution_error.into(), TransactionReceipt { fee: post_execution_report.recommended_fee(), ..revert_cost @@ -745,7 +750,7 @@ impl AccountTransaction { PostExecutionReport::new(state, &tx_context, &revert_cost, charge_fee)?; Ok(ValidateExecuteCallInfo::new_reverted( validate_call_info, - execution_error.to_string(), + gen_tx_execution_error_trace(&execution_error).into(), TransactionReceipt { fee: post_execution_report.recommended_fee(), ..revert_cost @@ -868,7 +873,7 @@ impl TransactionInfoCreator for AccountTransaction { struct ValidateExecuteCallInfo { validate_call_info: Option, execute_call_info: Option, - revert_error: Option, + revert_error: Option, final_cost: TransactionReceipt, } @@ -883,7 +888,7 @@ impl ValidateExecuteCallInfo { pub fn new_reverted( validate_call_info: Option, - revert_error: String, + revert_error: RevertError, final_cost: TransactionReceipt, ) -> Self { Self { diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index 8d377e80c7b..ab304dc000f 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -279,7 +279,7 @@ fn test_invoke_tx_from_non_deployed_account( match tx_result { Ok(info) => { // Make sure the error is because the account wasn't deployed. - assert!(info.revert_error.is_some_and(|err_str| err_str.contains(expected_error))); + assert!(info.revert_error.unwrap().to_string().contains(expected_error)); } Err(err) => { // Make sure the error is because the account wasn't deployed. @@ -339,6 +339,7 @@ fn test_infinite_recursion( tx_execution_info .revert_error .unwrap() + .to_string() .contains("RunResources has no remaining steps.") ); } @@ -546,7 +547,14 @@ fn test_recursion_depth_exceeded( InvokeTxArgs { calldata, nonce: nonce_manager.next(account_address), ..invoke_args }; let tx_execution_info = run_invoke_tx(&mut state, &block_context, invoke_args); - assert!(tx_execution_info.unwrap().revert_error.unwrap().contains("recursion depth exceeded")); + assert!( + tx_execution_info + .unwrap() + .revert_error + .unwrap() + .to_string() + .contains("recursion depth exceeded") + ); } #[rstest] @@ -1128,6 +1136,7 @@ fn test_insufficient_max_fee_reverts( tx_execution_info2 .revert_error .unwrap() + .to_string() .starts_with(&format!("Insufficient max {resource}", resource = Resource::L1Gas)) ); @@ -1148,7 +1157,11 @@ fn test_insufficient_max_fee_reverts( assert!(tx_execution_info3.is_reverted()); assert!(tx_execution_info3.receipt.fee == actual_fee_depth1); assert!( - tx_execution_info3.revert_error.unwrap().contains("RunResources has no remaining steps.") + tx_execution_info3 + .revert_error + .unwrap() + .to_string() + .contains("RunResources has no remaining steps.") ); } @@ -1544,7 +1557,13 @@ fn test_revert_in_execute( .unwrap(); assert!(tx_execution_info.is_reverted()); - assert!(tx_execution_info.revert_error.unwrap().contains("Failed to deserialize param #1")); + assert!( + tx_execution_info + .revert_error + .unwrap() + .to_string() + .contains("Failed to deserialize param #1") + ); } #[rstest] diff --git a/crates/blockifier/src/transaction/errors.rs b/crates/blockifier/src/transaction/errors.rs index 62c717a2686..6bb1b9d9d97 100644 --- a/crates/blockifier/src/transaction/errors.rs +++ b/crates/blockifier/src/transaction/errors.rs @@ -82,17 +82,11 @@ pub enum TransactionExecutionError { version {cairo_version:?}.", **declare_version )] ContractClassVersionMismatch { declare_version: TransactionVersion, cairo_version: u64 }, - #[error( - "Contract constructor execution has failed:\n{}", - String::from(gen_tx_execution_error_trace(self)) - )] + #[error("{}", gen_tx_execution_error_trace(self))] ContractConstructorExecutionFailed(#[from] ConstructorEntryPointExecutionError), #[error("Class with hash {:#064x} is already declared.", **class_hash)] DeclareTransactionError { class_hash: ClassHash }, - #[error( - "Transaction execution has failed:\n{}", - String::from(gen_tx_execution_error_trace(self)) - )] + #[error("{}", gen_tx_execution_error_trace(self))] ExecutionError { error: EntryPointExecutionError, class_hash: ClassHash, @@ -127,10 +121,7 @@ pub enum TransactionExecutionError { transaction size: {}.", *max_capacity, *tx_size )] TransactionTooLarge { max_capacity: Box, tx_size: Box }, - #[error( - "Transaction validation has failed:\n{}", - String::from(gen_tx_execution_error_trace(self)) - )] + #[error("{}", gen_tx_execution_error_trace(self))] ValidateTransactionError { error: EntryPointExecutionError, class_hash: ClassHash, diff --git a/crates/blockifier/src/transaction/execution_flavors_test.rs b/crates/blockifier/src/transaction/execution_flavors_test.rs index 0e965d44a06..3a66a5dcbe9 100644 --- a/crates/blockifier/src/transaction/execution_flavors_test.rs +++ b/crates/blockifier/src/transaction/execution_flavors_test.rs @@ -632,7 +632,14 @@ fn test_simulate_validate_charge_fee_mid_execution( .unwrap(); assert_eq!(tx_execution_info.is_reverted(), charge_fee); if charge_fee { - assert!(tx_execution_info.revert_error.clone().unwrap().contains("no remaining steps")); + assert!( + tx_execution_info + .revert_error + .clone() + .unwrap() + .to_string() + .contains("no remaining steps") + ); } check_gas_and_fee( &block_context, @@ -676,7 +683,9 @@ fn test_simulate_validate_charge_fee_mid_execution( }) .execute(&mut state, &low_step_block_context, charge_fee, validate) .unwrap(); - assert!(tx_execution_info.revert_error.clone().unwrap().contains("no remaining steps")); + assert!( + tx_execution_info.revert_error.clone().unwrap().to_string().contains("no remaining steps") + ); // Complete resources used are reported as receipt.resources; but only the charged // final fee is shown in actual_fee. As a sanity check, verify that the fee derived directly // from the consumed resources is also equal to the expected fee. @@ -764,11 +773,9 @@ fn test_simulate_validate_charge_fee_post_execution( if charge_fee { let expected_error_prefix = &format!("Insufficient max {resource}", resource = Resource::L1Gas); - assert!(tx_execution_info.revert_error.clone().unwrap().starts_with(if is_deprecated { - "Insufficient max fee" - } else { - expected_error_prefix - })); + assert!(tx_execution_info.revert_error.clone().unwrap().to_string().starts_with( + if is_deprecated { "Insufficient max fee" } else { expected_error_prefix } + )); } check_gas_and_fee( @@ -830,6 +837,7 @@ fn test_simulate_validate_charge_fee_post_execution( .revert_error .clone() .unwrap() + .to_string() .contains("Insufficient fee token balance.") ); } diff --git a/crates/blockifier/src/transaction/objects.rs b/crates/blockifier/src/transaction/objects.rs index f3a7ed7cf4c..ab0e8996619 100644 --- a/crates/blockifier/src/transaction/objects.rs +++ b/crates/blockifier/src/transaction/objects.rs @@ -24,6 +24,8 @@ use strum_macros::EnumIter; use crate::abi::constants as abi_constants; use crate::blockifier::block::BlockInfo; use crate::execution::call_info::{CallInfo, ExecutionSummary}; +use crate::execution::stack_trace::ErrorStack; +use crate::fee::fee_checks::FeeCheckError; use crate::fee::fee_utils::get_fee_by_gas_vector; use crate::fee::receipt::TransactionReceipt; use crate::transaction::errors::{TransactionExecutionError, TransactionPreValidationError}; @@ -136,6 +138,32 @@ pub struct CommonAccountFields { pub only_query: bool, } +#[cfg_attr(any(test, feature = "testing"), derive(Clone))] +#[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))] +#[derive(Debug, derive_more::Display, PartialEq)] +pub enum RevertError { + Execution(ErrorStack), + PostExecution(FeeCheckError), +} + +impl From for RevertError { + fn from(stack: ErrorStack) -> Self { + Self::Execution(stack) + } +} + +impl From for RevertError { + fn from(error: FeeCheckError) -> Self { + Self::PostExecution(error) + } +} + +impl Default for RevertError { + fn default() -> Self { + Self::Execution(ErrorStack::default()) + } +} + /// Contains the information gathered by the execution of a transaction. #[cfg_attr(any(test, feature = "testing"), derive(Clone))] #[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))] @@ -147,7 +175,7 @@ pub struct TransactionExecutionInfo { pub execute_call_info: Option, /// Fee transfer call info; [None] for `L1Handler`. pub fee_transfer_call_info: Option, - pub revert_error: Option, + pub revert_error: Option, /// The receipt of the transaction. /// Including the actual fee that was charged (in units of the relevant fee token), /// actual gas consumption the transaction is charged for data availability, diff --git a/crates/blockifier/src/transaction/post_execution_test.rs b/crates/blockifier/src/transaction/post_execution_test.rs index 1ac3270c3c8..e99ec2f0be4 100644 --- a/crates/blockifier/src/transaction/post_execution_test.rs +++ b/crates/blockifier/src/transaction/post_execution_test.rs @@ -182,7 +182,13 @@ fn test_revert_on_overdraft( // Verify the execution was reverted (including nonce bump) with the correct error. assert!(execution_info.is_reverted()); - assert!(execution_info.revert_error.unwrap().starts_with("Insufficient fee token balance")); + assert!( + execution_info + .revert_error + .unwrap() + .to_string() + .starts_with("Insufficient fee token balance") + ); assert_eq!(state.get_nonce_at(account_address).unwrap(), nonce_manager.next(account_address)); // Verify the storage key/value were not updated in the last tx. @@ -307,7 +313,12 @@ fn test_revert_on_resource_overuse( // Assert the transaction was reverted with the correct error. if is_revertible { assert!( - execution_info_result.unwrap().revert_error.unwrap().starts_with(expected_error_prefix) + execution_info_result + .unwrap() + .revert_error + .unwrap() + .to_string() + .starts_with(expected_error_prefix) ); } else { assert_matches!( diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index 1e16dffe9ab..48edb3514dd 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -1210,7 +1210,11 @@ fn test_actual_fee_gt_resource_bounds( let execution_result = invalid_tx.execute(state, block_context, true, true).unwrap(); let execution_error = execution_result.revert_error.unwrap(); // Test error. - assert!(execution_error.starts_with(&format!("Insufficient max {resource}", resource = L1Gas))); + assert!( + execution_error + .to_string() + .starts_with(&format!("Insufficient max {resource}", resource = L1Gas)) + ); // Test that fee was charged. let minimal_fee = minimal_l1_gas .checked_mul( @@ -2223,6 +2227,7 @@ fn test_execute_tx_with_invalid_tx_version( execution_info .revert_error .unwrap() + .to_string() .contains(format!("ASSERT_EQ instruction failed: {} != 3.", invalid_version).as_str()) ); } @@ -2316,7 +2321,7 @@ fn test_emit_event_exceeds_limit( let execution_info = account_tx.execute(state, block_context, true, true).unwrap(); match &expected_error { Some(expected_error) => { - let error_string = execution_info.revert_error.unwrap(); + let error_string = execution_info.revert_error.unwrap().to_string(); assert!(error_string.contains(&format!("{}", expected_error))); } None => { diff --git a/crates/native_blockifier/src/py_block_executor.rs b/crates/native_blockifier/src/py_block_executor.rs index 7bf28040a1a..ccfed9da11a 100644 --- a/crates/native_blockifier/src/py_block_executor.rs +++ b/crates/native_blockifier/src/py_block_executor.rs @@ -68,7 +68,7 @@ impl ThinTransactionExecutionInfo { actual_resources: ThinTransactionExecutionInfo::receipt_to_resources_mapping( &tx_execution_info.receipt, ), - revert_error: tx_execution_info.revert_error, + revert_error: tx_execution_info.revert_error.map(|error| error.to_string()), total_gas: tx_execution_info.receipt.gas, } } diff --git a/crates/papyrus_execution/src/execution_test.rs b/crates/papyrus_execution/src/execution_test.rs index f3d2605bc9d..b9c8ff52545 100644 --- a/crates/papyrus_execution/src/execution_test.rs +++ b/crates/papyrus_execution/src/execution_test.rs @@ -814,11 +814,7 @@ fn blockifier_error_mapping() { storage_address, selector, }; - let expected = format!( - "Transaction execution has failed:\n{}", - // TODO: consider adding ErrorStack display instead. - String::from(gen_tx_execution_error_trace(&blockifier_err)) - ); + let expected = format!("{}", gen_tx_execution_error_trace(&blockifier_err)); let err = ExecutionError::from((0, blockifier_err)); let ExecutionError::TransactionExecutionError { transaction_index, execution_error } = err else { @@ -834,10 +830,7 @@ fn blockifier_error_mapping() { storage_address, selector, }; - let expected = format!( - "Transaction validation has failed:\n{}", - String::from(gen_tx_execution_error_trace(&blockifier_err)) - ); + let expected = format!("{}", gen_tx_execution_error_trace(&blockifier_err)); let err = ExecutionError::from((0, blockifier_err)); let ExecutionError::TransactionExecutionError { transaction_index, execution_error } = err else { diff --git a/crates/papyrus_execution/src/lib.rs b/crates/papyrus_execution/src/lib.rs index 97338954c89..f57792e41b9 100644 --- a/crates/papyrus_execution/src/lib.rs +++ b/crates/papyrus_execution/src/lib.rs @@ -568,7 +568,10 @@ pub fn estimate_fee( for (index, tx_execution_output) in txs_execution_info.into_iter().enumerate() { // If the transaction reverted, fail the entire estimation. if let Some(revert_reason) = tx_execution_output.execution_info.revert_error { - return Ok(Err(RevertedTransaction { index, revert_reason })); + return Ok(Err(RevertedTransaction { + index, + revert_reason: revert_reason.to_string(), + })); } else { result .push(tx_execution_output_to_fee_estimation(&tx_execution_output, &block_context)?); diff --git a/crates/papyrus_execution/src/objects.rs b/crates/papyrus_execution/src/objects.rs index 9898c3527d2..3f17c3492ca 100644 --- a/crates/papyrus_execution/src/objects.rs +++ b/crates/papyrus_execution/src/objects.rs @@ -122,7 +122,7 @@ impl TryFrom for InvokeTransactionTrace { fn try_from(transaction_execution_info: TransactionExecutionInfo) -> ExecutionResult { let execute_invocation = match transaction_execution_info.revert_error { Some(revert_error) => { - FunctionInvocationResult::Err(RevertReason::RevertReason(revert_error)) + FunctionInvocationResult::Err(RevertReason::RevertReason(revert_error.to_string())) } None => FunctionInvocationResult::Ok( (