Skip to content

Commit

Permalink
refactor(blockifier): format retdata in ExecutionFailed and PanicInVa…
Browse files Browse the repository at this point in the history
…lidate errors (#1457)
  • Loading branch information
dorimedini-starkware authored and guy-starkware committed Oct 22, 2024
1 parent bf58cb0 commit ebe9901
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 26 deletions.
5 changes: 2 additions & 3 deletions crates/blockifier/src/execution/entry_point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +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::state::state_api::{State, StateResult};
use crate::transaction::objects::{HasRelatedFeeType, TransactionInfo};
use crate::transaction::transaction_types::TransactionType;
Expand Down Expand Up @@ -180,10 +181,8 @@ impl CallEntryPoint {
if let Ok(call_info) = &execution_result {
// If the execution of the outer call failed, revert the transction.
if call_info.execution.failed {
let retdata = &call_info.execution.retdata.0;

return Err(EntryPointExecutionError::ExecutionFailed {
error_data: retdata.clone(),
error_trace: extract_trailing_cairo1_revert_trace(call_info),
});
}
}
Expand Down
3 changes: 2 additions & 1 deletion crates/blockifier/src/execution/entry_point_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use crate::execution::execution_utils::{
ReadOnlySegments,
SEGMENT_ARENA_BUILTIN_SIZE,
};
use crate::execution::stack_trace::extract_trailing_cairo1_revert_trace;
use crate::execution::syscalls::hint_processor::SyscallHintProcessor;
use crate::state::state_api::State;
use crate::versioned_constants::GasCosts;
Expand Down Expand Up @@ -108,7 +109,7 @@ pub fn execute_entry_point_call(
)?;
if call_info.execution.failed && !context.versioned_constants().enable_reverts {
return Err(EntryPointExecutionError::ExecutionFailed {
error_data: call_info.execution.retdata.0,
error_trace: extract_trailing_cairo1_revert_trace(&call_info),
});
}

Expand Down
6 changes: 2 additions & 4 deletions crates/blockifier/src/execution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ use cairo_vm::vm::errors::vm_errors::VirtualMachineError;
use num_bigint::{BigInt, TryFromBigIntError};
use starknet_api::contract_class::EntryPointType;
use starknet_api::core::{ClassHash, ContractAddress, EntryPointSelector};
use starknet_api::execution_utils::format_panic_data;
use starknet_types_core::felt::Felt;
use thiserror::Error;

use crate::execution::entry_point::ConstructorContext;
Expand Down Expand Up @@ -81,8 +79,8 @@ impl From<RunnerError> for PostExecutionError {
pub enum EntryPointExecutionError {
#[error(transparent)]
CairoRunError(#[from] CairoRunError),
#[error("Execution failed. Failure reason: {}.", format_panic_data(.error_data))]
ExecutionFailed { error_data: Vec<Felt> },
#[error("Execution failed. Failure reason: {error_trace}.")]
ExecutionFailed { error_trace: String },
#[error("Internal error: {0}")]
InternalError(String),
#[error("Invalid input: {input_descriptor}; {info}")]
Expand Down
6 changes: 6 additions & 0 deletions crates/blockifier/src/execution/stack_trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ use cairo_vm::vm::errors::hint_errors::HintError;
use cairo_vm::vm::errors::vm_errors::VirtualMachineError;
use itertools::Itertools;
use starknet_api::core::{ClassHash, ContractAddress, EntryPointSelector};
use starknet_api::execution_utils::format_panic_data;

use super::deprecated_syscalls::hint_processor::DeprecatedSyscallExecutionError;
use super::syscalls::hint_processor::SyscallExecutionError;
use crate::execution::call_info::CallInfo;
use crate::execution::errors::{ConstructorEntryPointExecutionError, EntryPointExecutionError};
use crate::transaction::errors::TransactionExecutionError;

Expand Down Expand Up @@ -152,6 +154,10 @@ impl ErrorStack {
}
}

pub fn extract_trailing_cairo1_revert_trace(root_callinfo: &CallInfo) -> String {
format_panic_data(&root_callinfo.execution.retdata.0)
}

/// Extracts the error trace from a `TransactionExecutionError`. This is a top level function.
pub fn gen_tx_execution_error_trace(error: &TransactionExecutionError) -> ErrorStack {
match error {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,33 @@
use cairo_lang_utils::byte_array::BYTE_ARRAY_MAGIC;
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;

#[test]
fn test_syscall_failure_format() {
let error_data = vec![
// Magic to indicate that this is a byte array.
BYTE_ARRAY_MAGIC,
// The number of full words in the byte array.
"0x00",
// The pending word of the byte array: "Execution failure"
"0x457865637574696f6e206661696c757265",
// The length of the pending word.
"0x11",
]
.into_iter()
.map(|x| Felt::from_hex(x).unwrap())
.collect();
let error = EntryPointExecutionError::ExecutionFailed { error_data };
let error_data = Retdata(
vec![
// Magic to indicate that this is a byte array.
BYTE_ARRAY_MAGIC,
// The number of full words in the byte array.
"0x00",
// The pending word of the byte array: "Execution failure"
"0x457865637574696f6e206661696c757265",
// The length of the pending word.
"0x11",
]
.into_iter()
.map(|x| Felt::from_hex(x).unwrap())
.collect(),
);
let callinfo = CallInfo {
execution: CallExecution { retdata: error_data, failed: true, ..Default::default() },
..Default::default()
};
let error = EntryPointExecutionError::ExecutionFailed {
error_trace: extract_trailing_cairo1_revert_trace(&callinfo),
};
assert_eq!(error.to_string(), "Execution failed. Failure reason: \"Execution failure\".");
}
3 changes: 2 additions & 1 deletion crates/blockifier/src/transaction/account_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +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::fee::fee_checks::{FeeCheckReportFields, PostExecutionReport};
use crate::fee::fee_utils::{
get_fee_by_gas_vector,
Expand Down Expand Up @@ -944,7 +945,7 @@ impl ValidatableTransaction for AccountTransaction {

if validate_call_info.execution.failed {
return Err(TransactionExecutionError::PanicInValidate {
panic_reason: validate_call_info.execution.retdata,
panic_reason: extract_trailing_cairo1_revert_trace(&validate_call_info),
});
}

Expand Down
5 changes: 2 additions & 3 deletions crates/blockifier/src/transaction/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use num_bigint::BigUint;
use starknet_api::block::GasPrice;
use starknet_api::core::{ClassHash, ContractAddress, EntryPointSelector, Nonce};
use starknet_api::execution_resources::GasAmount;
use starknet_api::execution_utils::format_panic_data;
use starknet_api::transaction::{Fee, Resource, TransactionVersion};
use starknet_api::StarknetApiError;
use starknet_types_core::felt::FromStrError;
Expand Down Expand Up @@ -104,8 +103,8 @@ pub enum TransactionExecutionError {
FeeCheckError(#[from] FeeCheckError),
#[error(transparent)]
FromStr(#[from] FromStrError),
#[error("The `validate` entry point panicked with {}.", format_panic_data(&panic_reason.0))]
PanicInValidate { panic_reason: Retdata },
#[error("The `validate` entry point panicked with {panic_reason}.")]
PanicInValidate { panic_reason: String },
#[error("The `validate` entry point should return `VALID`. Got {actual:?}.")]
InvalidValidateReturnData { actual: Retdata },
#[error(
Expand Down

0 comments on commit ebe9901

Please sign in to comment.