diff --git a/crates/blockifier/src/execution/entry_point.rs b/crates/blockifier/src/execution/entry_point.rs index e251e15bff..3f92c3b06c 100644 --- a/crates/blockifier/src/execution/entry_point.rs +++ b/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, + ), }); } } diff --git a/crates/blockifier/src/execution/errors.rs b/crates/blockifier/src/execution/errors.rs index 208daefccc..43e4885832 100644 --- a/crates/blockifier/src/execution/errors.rs +++ b/crates/blockifier/src/execution/errors.rs @@ -82,7 +82,7 @@ impl From 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), diff --git a/crates/blockifier/src/execution/execution_utils.rs b/crates/blockifier/src/execution/execution_utils.rs index a17b2a71ba..c8a33fc11b 100644 --- a/crates/blockifier/src/execution/execution_utils.rs +++ b/crates/blockifier/src/execution/execution_utils.rs @@ -21,27 +21,26 @@ use starknet_api::deprecated_contract_class::Program as DeprecatedProgram; use starknet_api::transaction::Calldata; use starknet_types_core::felt::Felt; -use super::call_info::CallExecution; -use super::entry_point::ConstructorEntryPointExecutionResult; -use super::errors::{ - ConstructorEntryPointExecutionError, - EntryPointExecutionError, - PreExecutionError, -}; -use super::syscalls::hint_processor::ENTRYPOINT_NOT_FOUND_ERROR; -use crate::execution::call_info::{CallInfo, Retdata}; +use crate::execution::call_info::{CallExecution, CallInfo, Retdata}; use crate::execution::contract_class::{RunnableContractClass, TrackedResource}; use crate::execution::entry_point::{ execute_constructor_entry_point, CallEntryPoint, ConstructorContext, + ConstructorEntryPointExecutionResult, EntryPointExecutionContext, EntryPointExecutionResult, }; -use crate::execution::errors::PostExecutionError; +use crate::execution::errors::{ + ConstructorEntryPointExecutionError, + EntryPointExecutionError, + PostExecutionError, + PreExecutionError, +}; #[cfg(feature = "cairo_native")] use crate::execution::native::entry_point_execution as native_entry_point_execution; -use crate::execution::stack_trace::extract_trailing_cairo1_revert_trace; +use crate::execution::stack_trace::{extract_trailing_cairo1_revert_trace, Cairo1RevertHeader}; +use crate::execution::syscalls::hint_processor::ENTRYPOINT_NOT_FOUND_ERROR; use crate::execution::{deprecated_entry_point_execution, entry_point_execution}; use crate::state::errors::StateError; use crate::state::state_api::State; @@ -91,7 +90,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, + ), }); } update_remaining_gas(remaining_gas, &call_info); diff --git a/crates/blockifier/src/execution/stack_trace.rs b/crates/blockifier/src/execution/stack_trace.rs index 2a52ea831d..00c003b9a5 100644 --- a/crates/blockifier/src/execution/stack_trace.rs +++ b/crates/blockifier/src/execution/stack_trace.rs @@ -98,6 +98,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), } @@ -106,6 +107,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(), } @@ -114,19 +116,25 @@ impl From<&Frame> for String { impl From for Frame { fn from(value: EntryPointErrorFrame) -> Self { - Frame::EntryPoint(value) + Self::EntryPoint(value) + } +} + +impl From for Frame { + fn from(value: Cairo1RevertStack) -> Self { + Self::Cairo1Tail(value) } } impl From for Frame { fn from(value: VmExceptionFrame) -> Self { - Frame::Vm(value) + Self::Vm(value) } } impl From for Frame { fn from(value: String) -> Self { - Frame::StringFrame(value) + Self::StringFrame(value) } } @@ -157,7 +165,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, @@ -199,8 +208,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, pub last_retdata: Retdata, } @@ -218,15 +249,20 @@ impl Display for Cairo1RevertStack { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { // Total string length is limited by TRACE_LENGTH_CAP. + let header = format!("{}", self.header); + let tail = ".\n"; + // Prioritize the failure reason felts over the frames. - // If the failure reason is too long to include a minimal frame trace, display only the - // failure reason (truncated if necessary). + // If the failure reason is too long to include a minimal frame trace + header + newline, + // display only the failure reason (truncated if necessary). let failure_reason = format_panic_data(&self.last_retdata.0); - if failure_reason.len() >= TRACE_LENGTH_CAP - *MIN_CAIRO1_FRAMES_STACK_LENGTH { - let output = if failure_reason.len() <= TRACE_LENGTH_CAP { - failure_reason + let string_without_frames = + [header.clone(), failure_reason.clone(), tail.into()].join("\n"); + if string_without_frames.len() >= TRACE_LENGTH_CAP - *MIN_CAIRO1_FRAMES_STACK_LENGTH - 1 { + let output = if string_without_frames.len() <= TRACE_LENGTH_CAP { + string_without_frames } else { - failure_reason + string_without_frames .chars() .take(TRACE_LENGTH_CAP - Self::TRUNCATION_SEPARATOR.len()) .collect::() @@ -235,12 +271,12 @@ impl Display for Cairo1RevertStack { return write!(f, "{}", output); } - let untruncated_string = self - .stack - .iter() - .map(|frame| frame.to_string()) + let untruncated_string = [header.clone()] + .into_iter() + .chain(self.stack.iter().map(|frame| frame.to_string())) .chain([failure_reason.clone()]) - .join("\n"); + .join("\n") + + tail; if untruncated_string.len() <= TRACE_LENGTH_CAP { return write!(f, "{}", untruncated_string); } @@ -255,15 +291,16 @@ impl Display for Cairo1RevertStack { let final_string = match (self.stack.get(..self.stack.len() - n_frames_to_drop - 1), self.stack.last()) { (Some(frames), Some(last_frame)) => { - let combined_string = frames - .iter() - .map(|frame| frame.to_string()) + let combined_string = [header] + .into_iter() + .chain(frames.iter().map(|frame| frame.to_string())) .chain([ String::from(Self::TRUNCATION_SEPARATOR), last_frame.to_string(), failure_reason, ]) - .join("\n"); + .join("\n") + + tail; if combined_string.len() <= TRACE_LENGTH_CAP { combined_string } else { @@ -286,9 +323,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.")); @@ -336,6 +379,7 @@ pub fn extract_trailing_cairo1_revert_trace(root_call: &CallInfo) -> Cairo1Rever // If error_calls is empty, that means the root call is non-failing; return the fallback value. 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(), } @@ -376,6 +420,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(); @@ -636,6 +685,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()), } } diff --git a/crates/blockifier/src/execution/stack_trace_test.rs b/crates/blockifier/src/execution/stack_trace_test.rs index f7e3ac97af..7eac1835c2 100644 --- a/crates/blockifier/src/execution/stack_trace_test.rs +++ b/crates/blockifier/src/execution/stack_trace_test.rs @@ -27,6 +27,7 @@ use crate::execution::entry_point::CallEntryPoint; use crate::execution::errors::EntryPointExecutionError; use crate::execution::stack_trace::{ extract_trailing_cairo1_revert_trace, + Cairo1RevertHeader, Cairo1RevertStack, MIN_CAIRO1_FRAME_LENGTH, TRACE_LENGTH_CAP, @@ -617,7 +618,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 ), }; @@ -840,7 +842,7 @@ Error in contract (contract address: {expected_address:#064x}, class hash: {:#06 #[test] fn test_min_cairo1_frame_length() { let failure_hex = "0xdeadbeef"; - let call_info = CallInfo { + let call_info_1_frame = CallInfo { call: CallEntryPoint { class_hash: Some(ClassHash::default()), storage_address: ContractAddress::default(), @@ -854,11 +856,27 @@ fn test_min_cairo1_frame_length() { }, ..Default::default() }; - let error_stack = extract_trailing_cairo1_revert_trace(&call_info); - let error_string = error_stack.to_string(); - // Frame, newline, failure reason. - // Min frame length includes the newline. - assert_eq!(error_string.len(), *MIN_CAIRO1_FRAME_LENGTH + failure_hex.len()); + let call_info_2_frames = CallInfo { + call: CallEntryPoint { + class_hash: Some(ClassHash::default()), + storage_address: ContractAddress::default(), + entry_point_selector: EntryPointSelector::default(), + ..Default::default() + }, + execution: CallExecution { + retdata: Retdata(vec![felt!(failure_hex), felt!(ENTRYPOINT_FAILED_ERROR)]), + failed: true, + ..Default::default() + }, + inner_calls: vec![call_info_1_frame.clone()], + ..Default::default() + }; + let error_stack_1_frame = + extract_trailing_cairo1_revert_trace(&call_info_1_frame, Cairo1RevertHeader::Execution); + let error_stack_2_frames = + extract_trailing_cairo1_revert_trace(&call_info_2_frames, Cairo1RevertHeader::Execution); + let diff = error_stack_2_frames.to_string().len() - error_stack_1_frame.to_string().len(); + assert_eq!(diff, *MIN_CAIRO1_FRAME_LENGTH); } #[rstest] @@ -901,46 +919,49 @@ fn test_cairo1_revert_error_truncation( } // Check that the error message is structured as expected. - let error_stack = extract_trailing_cairo1_revert_trace(&next_call_info); + let header_type = Cairo1RevertHeader::Execution; + let header_str = header_type.to_string(); + let tail_str = ".\n"; + let error_stack = extract_trailing_cairo1_revert_trace(&next_call_info, header_type); let error_string = error_stack.to_string(); let first_frame = error_stack.stack.first().unwrap().to_string(); let last_frame = error_stack.stack.last().unwrap().to_string(); - match scenario { + let (expected_head, expected_tail) = match scenario { // Frames truncated, entire failure reason (a single felt) is output. "too_many_frames" => { - assert!(error_string.starts_with(&format!("{first_frame}\n"))); - assert!( - error_string.ends_with( - &[ - Cairo1RevertStack::TRUNCATION_SEPARATOR.into(), - last_frame, - // One failure felt. - failure_felt.to_string(), - ] - .join("\n") - ) - ); + ( + format!("{header_str}\n{first_frame}\n"), + [ + Cairo1RevertStack::TRUNCATION_SEPARATOR.into(), + last_frame, + // One failure felt. + format!("{failure_felt}{tail_str}"), + ] + .join("\n"), + ) } // A single frame, but failure reason itself is too long. No frames printed. - "too_much_retdata" => { - assert!(error_string.starts_with(&format!("({failure_felt}"))); - assert!(error_string.ends_with(Cairo1RevertStack::TRUNCATION_SEPARATOR)); - } + "too_much_retdata" => ( + format!("{header_str}\n({failure_felt}"), + Cairo1RevertStack::TRUNCATION_SEPARATOR.into(), + ), // Too many frames and too much retdata - retdata takes precedence. "both_too_much" => { - assert!(error_string.starts_with(&format!("{first_frame}\n"))); - let retdata_tail = - format!("({}{failure_felt})", format!("{failure_felt}, ").repeat(n_retdata - 1)); - assert!( - error_string.ends_with( - &[Cairo1RevertStack::TRUNCATION_SEPARATOR.into(), last_frame, retdata_tail] - .join("\n") - ) + let retdata_tail = format!( + "({}{failure_felt}){tail_str}", + format!("{failure_felt}, ").repeat(n_retdata - 1) ); + ( + format!("{header_str}\n{first_frame}\n"), + [Cairo1RevertStack::TRUNCATION_SEPARATOR.into(), last_frame, retdata_tail] + .join("\n"), + ) } _ => panic!("Test not implemented for {n_frames} frames."), - } + }; assert!(error_string.len() <= TRACE_LENGTH_CAP); + assert_eq!(error_string[..expected_head.len()], expected_head); + assert_eq!(error_string[error_string.len() - expected_tail.len()..], expected_tail); } #[test] @@ -953,14 +974,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}): -{failure_reason_str}.", +{failure_reason_str}. +", ContractAddress::default().0.key(), EntryPointSelector::default().0 ) @@ -991,8 +1013,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 ); } @@ -1032,8 +1054,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 ); } @@ -1050,8 +1072,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 ); } diff --git a/crates/blockifier/src/execution/syscalls/syscall_tests/failure_format.rs b/crates/blockifier/src/execution/syscalls/syscall_tests/failure_format.rs index aed94c351f..022555e141 100644 --- a/crates/blockifier/src/execution/syscalls/syscall_tests/failure_format.rs +++ b/crates/blockifier/src/execution/syscalls/syscall_tests/failure_format.rs @@ -3,7 +3,7 @@ use starknet_api::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 ) diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index 76bad0b168..29d5d4b92d 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -26,7 +26,7 @@ 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; +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, + ), }); } diff --git a/crates/blockifier/src/transaction/errors.rs b/crates/blockifier/src/transaction/errors.rs index 180bb9b710..05325ce62c 100644 --- a/crates/blockifier/src/transaction/errors.rs +++ b/crates/blockifier/src/transaction/errors.rs @@ -90,7 +90,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 },