feat(blockifier): keep revert error structure in TransactionExecution…

…Info

crates/blockifier/src/concurrency/worker_logic_test.rs

@@ -598,7 +598,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.

crates/blockifier/src/execution/stack_trace.rs

@@ -24,6 +24,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,
@@ -40,7 +41,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,
@@ -72,7 +75,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<String>,
@@ -95,8 +100,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(derive_more::From)]
+#[derive(Debug, PartialEq, derive_more::From)]
 pub enum ErrorStackSegment {
     EntryPoint(EntryPointErrorFrame),
     Cairo1RevertSummary(Cairo1RevertSummary),
@@ -118,24 +124,52 @@ impl From<&ErrorStackSegment> for String {
 }
 
 #[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<ErrorStackSegment>,
 }
 
-impl From<ErrorStack> 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)
     }
 }
 
@@ -144,8 +178,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<ClassHash>,
@@ -188,7 +223,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,
@@ -208,7 +243,7 @@ impl Display for Cairo1RevertHeader {
 }
 
 #[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))]
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, PartialEq)]
 pub struct Cairo1RevertSummary {
     pub header: Cairo1RevertHeader,
     pub stack: Vec<Cairo1RevertFrame>,
@@ -372,13 +407,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,
@@ -393,6 +436,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,
@@ -415,13 +459,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 {

crates/blockifier/src/fee/fee_checks.rs

@@ -16,7 +16,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}."

crates/blockifier/src/transaction/account_transaction.rs

@@ -31,7 +31,11 @@ use crate::context::{BlockContext, TransactionContext};
 use crate::execution::call_info::CallInfo;
 use crate::execution::contract_class::RunnableContractClass;
 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,
@@ -52,6 +56,7 @@ use crate::transaction::errors::{
 use crate::transaction::objects::{
     DeprecatedTransactionInfo,
     HasRelatedFeeType,
+    RevertError,
     TransactionExecutionInfo,
     TransactionExecutionResult,
     TransactionInfo,
@@ -704,7 +709,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
@@ -729,7 +734,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
@@ -852,7 +857,7 @@ impl TransactionInfoCreator for AccountTransaction {
 struct ValidateExecuteCallInfo {
     validate_call_info: Option<CallInfo>,
     execute_call_info: Option<CallInfo>,
-    revert_error: Option<String>,
+    revert_error: Option<RevertError>,
     final_cost: TransactionReceipt,
 }
 
@@ -867,7 +872,7 @@ impl ValidateExecuteCallInfo {
 
     pub fn new_reverted(
         validate_call_info: Option<CallInfo>,
-        revert_error: String,
+        revert_error: RevertError,
         final_cost: TransactionReceipt,
     ) -> Self {
         Self {

crates/blockifier/src/transaction/account_transactions_test.rs

@@ -361,7 +361,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.
@@ -422,6 +422,7 @@ fn test_infinite_recursion(
             tx_execution_info
                 .revert_error
                 .unwrap()
+                .to_string()
                 .contains("RunResources has no remaining steps.")
         );
     }
@@ -630,7 +631,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]
@@ -1232,6 +1240,7 @@ fn test_insufficient_max_fee_reverts(
         tx_execution_info2
             .revert_error
             .unwrap()
+            .to_string()
             .contains(&format!("Insufficient max {overdraft_resource}"))
     );
 
@@ -1252,7 +1261,7 @@ fn test_insufficient_max_fee_reverts(
     assert!(tx_execution_info3.is_reverted());
     assert_eq!(tx_execution_info3.receipt.da_gas, tx_execution_info1.receipt.da_gas);
     assert_eq!(tx_execution_info3.receipt.fee, tx_execution_info1.receipt.fee);
-    assert!(tx_execution_info3.revert_error.unwrap().contains("no remaining steps"));
+    assert!(tx_execution_info3.revert_error.unwrap().to_string().contains("no remaining steps"));
 }
 
 #[rstest]
@@ -1742,7 +1751,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]

crates/blockifier/src/transaction/errors.rs

@@ -70,17 +70,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,
@@ -115,10 +109,7 @@ pub enum TransactionExecutionError {
          transaction size: {}.", *max_capacity, *tx_size
     )]
     TransactionTooLarge { max_capacity: Box<BouncerWeights>, tx_size: Box<BouncerWeights> },
-    #[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,

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.")
         );
     }