feat(blockifier): cairo1 revert stack now part of ErrorStack

crates/blockifier/src/execution/entry_point.rs

@@ -31,7 +31,7 @@ use crate::execution::errors::{
     PreExecutionError,
 };
 use crate::execution::execution_utils::execute_entry_point_call_wrapper;
-use crate::execution::stack_trace::extract_trailing_cairo1_revert_trace;
+use crate::execution::stack_trace::{extract_trailing_cairo1_revert_trace, Cairo1RevertHeader};
 use crate::state::state_api::{State, StateResult};
 use crate::transaction::objects::{HasRelatedFeeType, TransactionInfo};
 use crate::transaction::transaction_types::TransactionType;
@@ -182,7 +182,10 @@ impl CallEntryPoint {
             // If the execution of the outer call failed, revert the transction.
             if call_info.execution.failed {
                 return Err(EntryPointExecutionError::ExecutionFailed {
-                    error_trace: extract_trailing_cairo1_revert_trace(call_info),
+                    error_trace: extract_trailing_cairo1_revert_trace(
+                        call_info,
+                        Cairo1RevertHeader::Execution,
+                    ),
                 });
             }
         }

crates/blockifier/src/execution/errors.rs

@@ -81,7 +81,7 @@ impl From<RunnerError> for PostExecutionError {
 pub enum EntryPointExecutionError {
     #[error(transparent)]
     CairoRunError(#[from] CairoRunError),
-    #[error("Execution failed. Failure reason:\n{error_trace}.")]
+    #[error("{error_trace}")]
     ExecutionFailed { error_trace: Cairo1RevertStack },
     #[error("Internal error: {0}")]
     InternalError(String),

crates/blockifier/src/execution/execution_utils.rs

@@ -21,6 +21,7 @@ use starknet_api::deprecated_contract_class::Program as DeprecatedProgram;
 use starknet_api::transaction::Calldata;
 use starknet_types_core::felt::Felt;
 
+use super::stack_trace::Cairo1RevertHeader;
 use crate::execution::call_info::{CallInfo, Retdata};
 use crate::execution::contract_class::{ContractClass, TrackedResource};
 use crate::execution::entry_point::{
@@ -84,7 +85,10 @@ pub fn execute_entry_point_call_wrapper(
         if call_info.execution.failed && !context.versioned_constants().enable_reverts {
             // Reverts are disabled.
             return Err(EntryPointExecutionError::ExecutionFailed {
-                error_trace: extract_trailing_cairo1_revert_trace(call_info),
+                error_trace: extract_trailing_cairo1_revert_trace(
+                    call_info,
+                    Cairo1RevertHeader::Execution,
+                ),
             });
         }
 

crates/blockifier/src/execution/stack_trace.rs

@@ -97,6 +97,7 @@ impl From<&VmExceptionFrame> for String {
 #[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))]
 pub enum Frame {
     EntryPoint(EntryPointErrorFrame),
+    Cairo1Tail(Cairo1RevertStack),
     Vm(VmExceptionFrame),
     StringFrame(String),
 }
@@ -105,6 +106,7 @@ impl From<&Frame> for String {
     fn from(value: &Frame) -> Self {
         match value {
             Frame::EntryPoint(entry_point_frame) => entry_point_frame.into(),
+            Frame::Cairo1Tail(cairo1_revert_stack) => cairo1_revert_stack.to_string(),
             Frame::Vm(vm_exception_frame) => vm_exception_frame.into(),
             Frame::StringFrame(error) => error.clone(),
         }
@@ -113,19 +115,25 @@ impl From<&Frame> for String {
 
 impl From<EntryPointErrorFrame> for Frame {
     fn from(value: EntryPointErrorFrame) -> Self {
-        Frame::EntryPoint(value)
+        Self::EntryPoint(value)
+    }
+}
+
+impl From<Cairo1RevertStack> for Frame {
+    fn from(value: Cairo1RevertStack) -> Self {
+        Self::Cairo1Tail(value)
     }
 }
 
 impl From<VmExceptionFrame> for Frame {
     fn from(value: VmExceptionFrame) -> Self {
-        Frame::Vm(value)
+        Self::Vm(value)
     }
 }
 
 impl From<String> for Frame {
     fn from(value: String) -> Self {
-        Frame::StringFrame(value)
+        Self::StringFrame(value)
     }
 }
 
@@ -156,7 +164,8 @@ impl ErrorStack {
         self.stack.push(frame);
     }
 }
-#[derive(Debug)]
+#[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))]
+#[derive(Clone, Debug)]
 pub struct Cairo1RevertFrame {
     pub contract_address: ContractAddress,
     pub class_hash: Option<ClassHash>,
@@ -188,8 +197,30 @@ impl Display for Cairo1RevertFrame {
     }
 }
 
-#[derive(Debug)]
+#[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))]
+#[derive(Clone, Copy, Debug)]
+pub enum Cairo1RevertHeader {
+    Execution,
+    Validation,
+}
+
+impl Display for Cairo1RevertHeader {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        write!(
+            f,
+            "{}",
+            match self {
+                Self::Execution => "Execution failed. Failure reason:",
+                Self::Validation => "The `validate` entry point panicked with:",
+            }
+        )
+    }
+}
+
+#[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))]
+#[derive(Clone, Debug)]
 pub struct Cairo1RevertStack {
+    pub header: Cairo1RevertHeader,
     pub stack: Vec<Cairo1RevertFrame>,
     pub last_retdata: Retdata,
 }
@@ -198,7 +229,8 @@ impl Display for Cairo1RevertStack {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         write!(
             f,
-            "{}",
+            "{}\n{}.\n",
+            self.header,
             self.stack
                 .iter()
                 .map(|frame| frame.to_string())
@@ -211,9 +243,15 @@ impl Display for Cairo1RevertStack {
     }
 }
 
-pub fn extract_trailing_cairo1_revert_trace(root_call: &CallInfo) -> Cairo1RevertStack {
-    let fallback_value =
-        Cairo1RevertStack { stack: vec![], last_retdata: root_call.execution.retdata.clone() };
+pub fn extract_trailing_cairo1_revert_trace(
+    root_call: &CallInfo,
+    header: Cairo1RevertHeader,
+) -> Cairo1RevertStack {
+    let fallback_value = Cairo1RevertStack {
+        header,
+        stack: vec![],
+        last_retdata: root_call.execution.retdata.clone(),
+    };
     let entrypoint_failed_felt = Felt::from_hex(ENTRYPOINT_FAILED_ERROR)
         .unwrap_or_else(|_| panic!("{ENTRYPOINT_FAILED_ERROR} does not fit in a felt."));
 
@@ -262,6 +300,7 @@ pub fn extract_trailing_cairo1_revert_trace(root_call: &CallInfo) -> Cairo1Rever
     // Add one line per call, and append the failure reason.
     let Some(last_call) = error_calls.last() else { return fallback_value };
     Cairo1RevertStack {
+        header,
         stack: error_calls.iter().map(Cairo1RevertFrame::from).collect(),
         last_retdata: last_call.execution.retdata.clone(),
     }
@@ -302,6 +341,11 @@ pub fn gen_tx_execution_error_trace(error: &TransactionExecutionError) -> ErrorS
             constructor_selector.as_ref(),
             PreambleType::Constructor,
         ),
+        TransactionExecutionError::PanicInValidate { panic_reason } => {
+            let mut stack = ErrorStack::default();
+            stack.push(panic_reason.clone().into());
+            stack
+        }
         _ => {
             // Top-level error is unrelated to Cairo execution, no "real" frames.
             let mut stack = ErrorStack::default();
@@ -562,6 +606,9 @@ fn extract_entry_point_execution_error_into_stack_trace(
         EntryPointExecutionError::CairoRunError(cairo_run_error) => {
             extract_cairo_run_error_into_stack_trace(error_stack, depth, cairo_run_error)
         }
+        EntryPointExecutionError::ExecutionFailed { error_trace } => {
+            error_stack.push(error_trace.clone().into())
+        }
         _ => error_stack.push(format!("{}\n", entry_point_error).into()),
     }
 }

crates/blockifier/src/execution/stack_trace_test.rs

@@ -18,7 +18,11 @@ use crate::abi::constants::CONSTRUCTOR_ENTRY_POINT_NAME;
 use crate::context::{BlockContext, ChainInfo};
 use crate::execution::call_info::{CallExecution, CallInfo, Retdata};
 use crate::execution::errors::EntryPointExecutionError;
-use crate::execution::stack_trace::{extract_trailing_cairo1_revert_trace, Cairo1RevertStack};
+use crate::execution::stack_trace::{
+    extract_trailing_cairo1_revert_trace,
+    Cairo1RevertHeader,
+    Cairo1RevertStack,
+};
 use crate::execution::syscalls::hint_processor::ENTRYPOINT_FAILED_ERROR;
 use crate::test_utils::contracts::FeatureContract;
 use crate::test_utils::initial_test_state::{fund_account, test_state};
@@ -605,7 +609,8 @@ An ASSERT_EQ instruction failed: 1 != 0.
             "The `validate` entry point panicked with:
 Error in contract (contract address: {contract_address:#064x}, class hash: {:#064x}, selector: \
              {selector:#064x}):
-0x496e76616c6964207363656e6172696f ('Invalid scenario').",
+0x496e76616c6964207363656e6172696f ('Invalid scenario').
+",
             class_hash.0
         ),
     };
@@ -837,14 +842,15 @@ fn test_cairo1_stack_extraction_inner_call_successful() {
         ..Default::default()
     };
     let error = EntryPointExecutionError::ExecutionFailed {
-        error_trace: extract_trailing_cairo1_revert_trace(&callinfo),
+        error_trace: extract_trailing_cairo1_revert_trace(&callinfo, Cairo1RevertHeader::Execution),
     };
     assert_eq!(
         error.to_string(),
         format!(
             "Execution failed. Failure reason:
 Error in contract (contract address: {:#064x}, class hash: _, selector: {:#064x}):
-0xdeadbeef.",
+0xdeadbeef.
+",
             ContractAddress::default().0.key(),
             EntryPointSelector::default().0
         )
@@ -873,8 +879,8 @@ fn test_ambiguous_inner_cairo1_failure() {
         ..Default::default()
     };
     assert_matches!(
-        extract_trailing_cairo1_revert_trace(&call_info),
-        Cairo1RevertStack { stack, last_retdata }
+        extract_trailing_cairo1_revert_trace(&call_info, Cairo1RevertHeader::Execution),
+        Cairo1RevertStack { stack, last_retdata, .. }
         if stack.is_empty() && last_retdata == outer_retdata
     );
 }
@@ -913,8 +919,8 @@ fn test_inner_cairo1_failure_not_last(#[values(true, false)] last_is_failed: boo
         ..Default::default()
     };
     assert_matches!(
-        extract_trailing_cairo1_revert_trace(&call_info),
-        Cairo1RevertStack { stack, last_retdata }
+        extract_trailing_cairo1_revert_trace(&call_info, Cairo1RevertHeader::Execution),
+        Cairo1RevertStack { stack, last_retdata, .. }
         if stack.len() == 2 && last_retdata == first_inner_retdata
     );
 }
@@ -929,8 +935,8 @@ fn test_cairo1_stack_extraction_not_failure_fallback() {
         ..Default::default()
     };
     assert_matches!(
-        extract_trailing_cairo1_revert_trace(&successful_call),
-        Cairo1RevertStack { stack, last_retdata }
+        extract_trailing_cairo1_revert_trace(&successful_call, Cairo1RevertHeader::Execution),
+        Cairo1RevertStack { stack, last_retdata, .. }
         if stack.is_empty() && last_retdata == expected_retdata
     );
 }

crates/blockifier/src/execution/syscalls/syscall_tests/failure_format.rs

@@ -3,7 +3,7 @@ use starknet_types_core::felt::Felt;
 
 use crate::execution::call_info::{CallExecution, CallInfo, Retdata};
 use crate::execution::errors::EntryPointExecutionError;
-use crate::execution::stack_trace::extract_trailing_cairo1_revert_trace;
+use crate::execution::stack_trace::{extract_trailing_cairo1_revert_trace, Cairo1RevertHeader};
 
 #[test]
 fn test_syscall_failure_format() {
@@ -14,14 +14,15 @@ fn test_syscall_failure_format() {
         ..Default::default()
     };
     let error = EntryPointExecutionError::ExecutionFailed {
-        error_trace: extract_trailing_cairo1_revert_trace(&callinfo),
+        error_trace: extract_trailing_cairo1_revert_trace(&callinfo, Cairo1RevertHeader::Execution),
     };
     assert_eq!(
         error.to_string(),
         format!(
             "Execution failed. Failure reason:
 Error in contract (contract address: {:#064x}, class hash: _, selector: {:#064x}):
-{execution_failure} ('Execution failure').",
+{execution_failure} ('Execution failure').
+",
             ContractAddress::default().0.key(),
             EntryPointSelector::default().0
         )

crates/blockifier/src/transaction/account_transaction.rs

@@ -26,7 +26,7 @@ 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;
+use crate::execution::stack_trace::{extract_trailing_cairo1_revert_trace, Cairo1RevertHeader};
 use crate::fee::fee_checks::{FeeCheckReportFields, PostExecutionReport};
 use crate::fee::fee_utils::{
     get_fee_by_gas_vector,
@@ -945,7 +945,10 @@ impl ValidatableTransaction for AccountTransaction {
 
             if validate_call_info.execution.failed {
                 return Err(TransactionExecutionError::PanicInValidate {
-                    panic_reason: extract_trailing_cairo1_revert_trace(&validate_call_info),
+                    panic_reason: extract_trailing_cairo1_revert_trace(
+                        &validate_call_info,
+                        Cairo1RevertHeader::Validation,
+                    ),
                 });
             }
 

crates/blockifier/src/transaction/errors.rs

@@ -103,7 +103,7 @@ pub enum TransactionExecutionError {
     FeeCheckError(#[from] FeeCheckError),
     #[error(transparent)]
     FromStr(#[from] FromStrError),
-    #[error("The `validate` entry point panicked with:\n{panic_reason}.")]
+    #[error("{panic_reason}")]
     PanicInValidate { panic_reason: Cairo1RevertStack },
     #[error("The `validate` entry point should return `VALID`. Got {actual:?}.")]
     InvalidValidateReturnData { actual: Retdata },