From 89213420bdd7fccdccc5bfdc21f28f682f8afb0f Mon Sep 17 00:00:00 2001 From: Dori Medini Date: Sat, 19 Oct 2024 16:45:09 +0300 Subject: [PATCH] refactor(blockifier): format retdata in ExecutionFailed and PanicInValidate errors --- .../blockifier/src/execution/entry_point.rs | 5 +-- .../src/execution/entry_point_execution.rs | 3 +- crates/blockifier/src/execution/errors.rs | 6 +-- .../blockifier/src/execution/stack_trace.rs | 6 +++ .../syscalls/syscall_tests/failure_format.rs | 38 ++++++++++++------- .../src/transaction/account_transaction.rs | 3 +- crates/blockifier/src/transaction/errors.rs | 5 +-- 7 files changed, 40 insertions(+), 26 deletions(-) diff --git a/crates/blockifier/src/execution/entry_point.rs b/crates/blockifier/src/execution/entry_point.rs index 9493bf105a..e251e15bff 100644 --- a/crates/blockifier/src/execution/entry_point.rs +++ b/crates/blockifier/src/execution/entry_point.rs @@ -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; @@ -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), }); } } diff --git a/crates/blockifier/src/execution/entry_point_execution.rs b/crates/blockifier/src/execution/entry_point_execution.rs index 08ebbb2b79..535c89a9ee 100644 --- a/crates/blockifier/src/execution/entry_point_execution.rs +++ b/crates/blockifier/src/execution/entry_point_execution.rs @@ -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; @@ -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), }); } diff --git a/crates/blockifier/src/execution/errors.rs b/crates/blockifier/src/execution/errors.rs index ab867ff320..a5434201a5 100644 --- a/crates/blockifier/src/execution/errors.rs +++ b/crates/blockifier/src/execution/errors.rs @@ -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; @@ -81,8 +79,8 @@ impl From for PostExecutionError { pub enum EntryPointExecutionError { #[error(transparent)] CairoRunError(#[from] CairoRunError), - #[error("Execution failed. Failure reason: {}.", format_panic_data(.error_data))] - ExecutionFailed { error_data: Vec }, + #[error("Execution failed. Failure reason: {error_trace}.")] + ExecutionFailed { error_trace: String }, #[error("Internal error: {0}")] InternalError(String), #[error("Invalid input: {input_descriptor}; {info}")] diff --git a/crates/blockifier/src/execution/stack_trace.rs b/crates/blockifier/src/execution/stack_trace.rs index dedf766f02..0c02444da8 100644 --- a/crates/blockifier/src/execution/stack_trace.rs +++ b/crates/blockifier/src/execution/stack_trace.rs @@ -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; @@ -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 { diff --git a/crates/blockifier/src/execution/syscalls/syscall_tests/failure_format.rs b/crates/blockifier/src/execution/syscalls/syscall_tests/failure_format.rs index 27891c2f2d..803da94381 100644 --- a/crates/blockifier/src/execution/syscalls/syscall_tests/failure_format.rs +++ b/crates/blockifier/src/execution/syscalls/syscall_tests/failure_format.rs @@ -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\"."); } diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index 8ae5e146c7..58c48bf084 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -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, @@ -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), }); } diff --git a/crates/blockifier/src/transaction/errors.rs b/crates/blockifier/src/transaction/errors.rs index fd861cfb7e..f0f0066039 100644 --- a/crates/blockifier/src/transaction/errors.rs +++ b/crates/blockifier/src/transaction/errors.rs @@ -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; @@ -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(