Skip to content

Commit

Permalink
feat(blockifier): cairo1 revert stack now part of ErrorStack (#1490)
Browse files Browse the repository at this point in the history
  • Loading branch information
dorimedini-starkware authored Nov 17, 2024
1 parent f0436f8 commit 641ba41
Show file tree
Hide file tree
Showing 8 changed files with 181 additions and 102 deletions.
7 changes: 5 additions & 2 deletions crates/blockifier/src/execution/entry_point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
),
});
}
}
Expand Down
6 changes: 3 additions & 3 deletions crates/blockifier/src/execution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use starknet_api::core::{ClassHash, ContractAddress, EntryPointSelector};
use thiserror::Error;

use crate::execution::entry_point::ConstructorContext;
use crate::execution::stack_trace::Cairo1RevertStack;
use crate::execution::stack_trace::Cairo1RevertSummary;
#[cfg(feature = "cairo_native")]
use crate::execution::syscalls::hint_processor::SyscallExecutionError;
use crate::state::errors::StateError;
Expand Down Expand Up @@ -84,8 +84,8 @@ impl From<RunnerError> for PostExecutionError {
pub enum EntryPointExecutionError {
#[error(transparent)]
CairoRunError(#[from] CairoRunError),
#[error("Execution failed. Failure reason:\n{error_trace}.")]
ExecutionFailed { error_trace: Cairo1RevertStack },
#[error("{error_trace}")]
ExecutionFailed { error_trace: Cairo1RevertSummary },
#[error("Internal error: {0}")]
InternalError(String),
#[error("Invalid input: {input_descriptor}; {info}")]
Expand Down
26 changes: 14 additions & 12 deletions crates/blockifier/src/execution/execution_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,27 +21,26 @@ use starknet_api::deprecated_contract_class::Program as DeprecatedProgram;
use starknet_api::transaction::fields::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;
Expand Down Expand Up @@ -92,7 +91,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);
Expand Down
116 changes: 82 additions & 34 deletions crates/blockifier/src/execution/stack_trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,26 +97,30 @@ impl From<&VmExceptionFrame> for String {

#[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))]
#[derive(derive_more::From)]
pub enum Frame {
pub enum ErrorStackSegment {
EntryPoint(EntryPointErrorFrame),
Cairo1RevertSummary(Cairo1RevertSummary),
Vm(VmExceptionFrame),
StringFrame(String),
}

impl From<&Frame> for String {
fn from(value: &Frame) -> Self {
impl From<&ErrorStackSegment> for String {
fn from(value: &ErrorStackSegment) -> Self {
match value {
Frame::EntryPoint(entry_point_frame) => entry_point_frame.into(),
Frame::Vm(vm_exception_frame) => vm_exception_frame.into(),
Frame::StringFrame(error) => error.clone(),
ErrorStackSegment::EntryPoint(entry_point_frame) => entry_point_frame.into(),
ErrorStackSegment::Cairo1RevertSummary(cairo1_revert_stack) => {
cairo1_revert_stack.to_string()
}
ErrorStackSegment::Vm(vm_exception_frame) => vm_exception_frame.into(),
ErrorStackSegment::StringFrame(error) => error.clone(),
}
}
}

#[cfg_attr(feature = "transaction_serde", derive(serde::Serialize, serde::Deserialize))]
#[derive(Default)]
pub struct ErrorStack {
pub stack: Vec<Frame>,
pub stack: Vec<ErrorStackSegment>,
}

impl From<ErrorStack> for String {
Expand All @@ -136,11 +140,12 @@ impl From<ErrorStack> for String {
}

impl ErrorStack {
pub fn push(&mut self, frame: Frame) {
pub fn push(&mut self, frame: ErrorStackSegment) {
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>,
Expand Down Expand Up @@ -182,34 +187,61 @@ impl Display for Cairo1RevertFrame {
}
}

#[derive(Debug)]
pub struct Cairo1RevertStack {
#[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 Cairo1RevertSummary {
pub header: Cairo1RevertHeader,
pub stack: Vec<Cairo1RevertFrame>,
pub last_retdata: Retdata,
}

impl Cairo1RevertStack {
impl Cairo1RevertSummary {
pub const TRUNCATION_SEPARATOR: &'static str = "\n...";
}

pub static MIN_CAIRO1_FRAMES_STACK_LENGTH: LazyLock<usize> = LazyLock::new(|| {
// Two frames (first and last) + separator.
2 * *MIN_CAIRO1_FRAME_LENGTH + Cairo1RevertStack::TRUNCATION_SEPARATOR.len()
2 * *MIN_CAIRO1_FRAME_LENGTH + Cairo1RevertSummary::TRUNCATION_SEPARATOR.len()
});

impl Display for Cairo1RevertStack {
impl Display for Cairo1RevertSummary {
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::<String>()
Expand All @@ -218,12 +250,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);
}
Expand All @@ -238,15 +270,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 {
Expand All @@ -269,9 +302,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,
) -> Cairo1RevertSummary {
let fallback_value = Cairo1RevertSummary {
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."));

Expand Down Expand Up @@ -318,7 +357,8 @@ pub fn extract_trailing_cairo1_revert_trace(root_call: &CallInfo) -> Cairo1Rever
// Add one line per call, and append the failure reason.
// 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 {
Cairo1RevertSummary {
header,
stack: error_calls.iter().map(Cairo1RevertFrame::from).collect(),
last_retdata: last_call.execution.retdata.clone(),
}
Expand Down Expand Up @@ -359,10 +399,15 @@ 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();
stack.push(Frame::StringFrame(error.to_string()));
stack.push(ErrorStackSegment::StringFrame(error.to_string()));
stack
}
}
Expand Down Expand Up @@ -619,6 +664,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()),
}
}
Loading

0 comments on commit 641ba41

Please sign in to comment.