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

refactor(blockifier): format retdata in ExecutionFailed and PanicInValidate errors #1457

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
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
Loading