Skip to content

Commit

Permalink
feat(blockifier): limit characters in cairo1 revert trace
Browse files Browse the repository at this point in the history
  • Loading branch information
dorimedini-starkware committed Oct 23, 2024
1 parent 1ef3528 commit 9f232fc
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 5 deletions.
82 changes: 77 additions & 5 deletions crates/blockifier/src/execution/stack_trace.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::fmt::{Display, Formatter};
use std::sync::LazyLock;

use cairo_vm::types::relocatable::Relocatable;
use cairo_vm::vm::errors::cairo_run_errors::CairoRunError;
Expand Down Expand Up @@ -163,6 +164,16 @@ pub struct Cairo1RevertFrame {
pub selector: EntryPointSelector,
}

pub static MIN_CAIRO1_FRAME_LENGTH: LazyLock<usize> = LazyLock::new(|| {
let frame = Cairo1RevertFrame {
contract_address: ContractAddress::default(),
class_hash: Some(ClassHash::default()),
selector: EntryPointSelector::default(),
};
// +1 for newline.
format!("{}", frame).len() + 1
});

impl From<&&CallInfo> for Cairo1RevertFrame {
fn from(callinfo: &&CallInfo) -> Self {
Self {
Expand Down Expand Up @@ -194,16 +205,77 @@ pub struct Cairo1RevertStack {
pub last_retdata: Retdata,
}

impl Cairo1RevertStack {
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()
});

impl Display for Cairo1RevertStack {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
// Total string length is limited by TRACE_LENGTH_CAP.

// 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).
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
} else {
failure_reason
.chars()
.take(TRACE_LENGTH_CAP - Self::TRUNCATION_SEPARATOR.len())
.collect::<String>()
+ &Self::TRUNCATION_SEPARATOR.to_string()
};
return write!(f, "{}", output);
}

let untruncated_string = self
.stack
.iter()
.map(|frame| frame.to_string())
.chain([failure_reason.clone()])
.join("\n");
if untruncated_string.len() <= TRACE_LENGTH_CAP {
return write!(f, "{}", untruncated_string);
}

// If the number of frames is too large, drop frames above the last frame (two frames are
// not too many, as checked above with MIN_CAIRO1_FRAMES_STACK_LENGTH).
let n_frames_to_drop = (untruncated_string.len() - TRACE_LENGTH_CAP
+ Self::TRUNCATION_SEPARATOR.len())
.div_ceil(*MIN_CAIRO1_FRAME_LENGTH);

// Expect the number of frames to be at least the number of frames to drop + 2. If not,
// fall back to the failure reason.
if self.stack.len() < n_frames_to_drop + 2 {
return write!(f, "{}", failure_reason);
}

write!(
f,
"{}",
self.stack
.iter()
.map(|frame| frame.to_string())
.chain([format_panic_data(&self.last_retdata.0)])
.join("\n")
self.stack[..self.stack.len() - n_frames_to_drop - 1]
.iter()
.map(|frame| frame.to_string())
.chain([
String::from(Self::TRUNCATION_SEPARATOR),
self.stack
.last()
.expect("Stack has at least two frames at this point.")
.to_string(),
failure_reason,
])
.join("\n")
// Truncate again as a failsafe.
.chars()
.take(TRACE_LENGTH_CAP)
.collect::<String>()
)
}
}
Expand Down
33 changes: 33 additions & 0 deletions crates/blockifier/src/execution/stack_trace_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ use starknet_api::{calldata, felt, invoke_tx_args};
use crate::abi::abi_utils::selector_from_name;
use crate::abi::constants::CONSTRUCTOR_ENTRY_POINT_NAME;
use crate::context::{BlockContext, ChainInfo};
use crate::execution::call_info::{CallExecution, CallInfo, Retdata};
use crate::execution::stack_trace::{extract_trailing_cairo1_revert_trace, TRACE_LENGTH_CAP};
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};
use crate::test_utils::{create_calldata, CairoVersion, BALANCE};
Expand Down Expand Up @@ -818,3 +821,33 @@ Error in contract (contract address: {expected_address:#064x}, class hash: {:#06
invoke_deploy_tx.execute(state, &block_context, true, true).unwrap().revert_error.unwrap();
assert_eq!(error.to_string(), expected_error);
}

#[rstest]
// Assume each frame is over 100 chars (contract address + selector are already 128 chars).
#[case::too_many_frames(TRACE_LENGTH_CAP / 100, 1)]
// Each (large) felt should require at least 30 chars.
#[case::too_much_retdata(1, TRACE_LENGTH_CAP / 30)]
#[case::both_too_much(TRACE_LENGTH_CAP / 200, TRACE_LENGTH_CAP / 60)]
fn test_cairo1_revert_error_truncation(#[case] n_frames: usize, #[case] n_retdata: usize) {
let mut retdata = Retdata(vec![felt!("0xbeefbeefbeefbeefbeefbeefbeefbeef"); n_retdata]);
let mut next_call_info = CallInfo {
execution: CallExecution { retdata: retdata.clone(), failed: true, ..Default::default() },
..Default::default()
};
for _ in 1..n_frames {
retdata.0.push(felt!(ENTRYPOINT_FAILED_ERROR));
next_call_info = CallInfo {
inner_calls: vec![next_call_info],
execution: CallExecution {
retdata: retdata.clone(),
failed: true,
..Default::default()
},
..Default::default()
};
}
assert!(
format!("{}", extract_trailing_cairo1_revert_trace(&next_call_info)).len()
<= TRACE_LENGTH_CAP
);
}

0 comments on commit 9f232fc

Please sign in to comment.