Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading