diff --git a/crates/blockifier/src/execution/errors.rs b/crates/blockifier/src/execution/errors.rs index 9c5e0e5d11..922508b83f 100644 --- a/crates/blockifier/src/execution/errors.rs +++ b/crates/blockifier/src/execution/errors.rs @@ -80,7 +80,7 @@ impl From for PostExecutionError { pub enum EntryPointExecutionError { #[error(transparent)] CairoRunError(#[from] CairoRunError), - #[error("Execution failed. Failure reason: {error_trace}.")] + #[error("Execution failed. Failure reason:\n{error_trace}.")] ExecutionFailed { error_trace: String }, #[error("Internal error: {0}")] InternalError(String), diff --git a/crates/blockifier/src/execution/stack_trace.rs b/crates/blockifier/src/execution/stack_trace.rs index 0c02444da8..9380217e67 100644 --- a/crates/blockifier/src/execution/stack_trace.rs +++ b/crates/blockifier/src/execution/stack_trace.rs @@ -5,9 +5,10 @@ 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 starknet_types_core::felt::Felt; use super::deprecated_syscalls::hint_processor::DeprecatedSyscallExecutionError; -use super::syscalls::hint_processor::SyscallExecutionError; +use super::syscalls::hint_processor::{SyscallExecutionError, ENTRYPOINT_FAILED_ERROR}; use crate::execution::call_info::CallInfo; use crate::execution::errors::{ConstructorEntryPointExecutionError, EntryPointExecutionError}; use crate::transaction::errors::TransactionExecutionError; @@ -154,8 +155,65 @@ impl ErrorStack { } } -pub fn extract_trailing_cairo1_revert_trace(root_callinfo: &CallInfo) -> String { - format_panic_data(&root_callinfo.execution.retdata.0) +pub fn extract_trailing_cairo1_revert_trace(root_call: &CallInfo) -> String { + let fallback_value = format_panic_data(&root_call.execution.retdata.0); + let entrypoint_failed_felt = Felt::from_hex(ENTRYPOINT_FAILED_ERROR) + .unwrap_or_else(|_| panic!("{ENTRYPOINT_FAILED_ERROR} does not fit in a felt.")); + + // Compute the failing call chain. + let mut error_calls: Vec<&CallInfo> = vec![]; + let mut call = root_call; + // It is possible that a failing contract managed to call another (non-failing) contract + // before hitting an error; stop iteration if the current call was successful. + while call.execution.failed { + error_calls.push(call); + // If the last felt in the retdata is not the failure felt, stop iteration. + // Even if the next inner call is also in failed state, assume a scenario where the current + // call panicked after ignoring the error result of the inner call. + let retdata = &call.execution.retdata.0; + if retdata.last() != Some(&entrypoint_failed_felt) { + break; + } + // Select the next inner failure, if it exists and is unique. + // Consider the following scenario: + // ``` + // let A = call_contract(...) + // let B = call_contract(...) + // X.unwrap_syscall(...) + // ``` + // where X is either A or B. If both A and B are calls to different contracts, which fail + // for different reasons but return the same failure reasons, we cannot distinguish between + // them - i.e. we cannot distinguish between the case X=A or X=B. + // To avoid returning misleading data, we revert to the fallback value in such cases. + // If the source of failure can be identified in the inner calls, iterate. + let expected_inner_retdata = &retdata[..(retdata.len() - 1)]; + let mut potential_inner_failures_iter = call.inner_calls.iter().filter(|inner_call| { + inner_call.execution.failed + && &inner_call.execution.retdata.0[..] == expected_inner_retdata + }); + call = match potential_inner_failures_iter.next() { + Some(unique_inner_failure) if potential_inner_failures_iter.next().is_none() => { + unique_inner_failure + } + // Inner failure is either not unique, or does not exist (malformed retdata). + _ => return fallback_value, + }; + } + + // 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 }; + error_calls + .iter() + .map(|call_info| { + format!( + "Error in contract (contract address: {:#064x}, selector: {:#064x}):", + call_info.call.storage_address.0.key(), + call_info.call.entry_point_selector.0, + ) + }) + .chain([format_panic_data(&last_call.execution.retdata.0)]) + .join("\n") } /// Extracts the error trace from a `TransactionExecutionError`. This is a top level function. diff --git a/crates/blockifier/src/execution/stack_trace_test.rs b/crates/blockifier/src/execution/stack_trace_test.rs index dff6a9804c..7d5b8e1d20 100644 --- a/crates/blockifier/src/execution/stack_trace_test.rs +++ b/crates/blockifier/src/execution/stack_trace_test.rs @@ -213,8 +213,14 @@ An ASSERT_EQ instruction failed: 1 != 0. "Transaction execution has failed: 0: Error in the called contract (contract address: {account_address_felt:#064x}, class hash: \ {account_contract_hash:#064x}, selector: {execute_selector_felt:#064x}): -Execution failed. Failure reason: (0x6661696c ('fail'), 0x454e545259504f494e545f4641494c4544 \ - ('ENTRYPOINT_FAILED'), 0x454e545259504f494e545f4641494c4544 ('ENTRYPOINT_FAILED')). +Execution failed. Failure reason: +Error in contract (contract address: {account_address_felt:#064x}, selector: \ + {execute_selector_felt:#064x}): +Error in contract (contract address: {test_contract_address_felt:#064x}, selector: \ + {external_entry_point_selector_felt:#064x}): +Error in contract (contract address: {test_contract_address_2_felt:#064x}, selector: \ + {inner_entry_point_selector_felt:#064x}): +0x6661696c ('fail'). " ); @@ -330,9 +336,14 @@ Unknown location (pc=0:{expected_pc1}) "Transaction execution has failed: 0: Error in the called contract (contract address: {account_address_felt:#064x}, class hash: \ {account_contract_hash:#064x}, selector: {execute_selector_felt:#064x}): -Execution failed. Failure reason: ({expected_error}, 0x454e545259504f494e545f4641494c4544 \ - ('ENTRYPOINT_FAILED'), 0x454e545259504f494e545f4641494c4544 \ - ('ENTRYPOINT_FAILED')). +Execution failed. Failure reason: +Error in contract (contract address: {account_address_felt:#064x}, selector: \ + {execute_selector_felt:#064x}): +Error in contract (contract address: {contract_address_felt:#064x}, selector: \ + {invoke_call_chain_selector_felt:#064x}): +Error in contract (contract address: {contract_address_felt:#064x}, selector: \ + {invoke_call_chain_selector_felt:#064x}): +{expected_error}. " ) } @@ -478,10 +489,16 @@ Unknown location (pc=0:{expected_pc3}) "Transaction execution has failed: 0: Error in the called contract (contract address: {account_address_felt:#064x}, class hash: \ {account_contract_hash:#064x}, selector: {execute_selector_felt:#064x}): -Execution failed. Failure reason: ({expected_error}, 0x454e545259504f494e545f4641494c4544 \ - ('ENTRYPOINT_FAILED'), 0x454e545259504f494e545f4641494c4544 \ - ('ENTRYPOINT_FAILED'), 0x454e545259504f494e545f4641494c4544 \ - ('ENTRYPOINT_FAILED')). +Execution failed. Failure reason: +Error in contract (contract address: {account_address_felt:#064x}, selector: \ + {execute_selector_felt:#064x}): +Error in contract (contract address: {address_felt:#064x}, selector: \ + {invoke_call_chain_selector_felt:#064x}): +Error in contract (contract address: {address_felt:#064x}, selector: \ + {invoke_call_chain_selector_felt:#064x}): +Error in contract (contract address: {address_felt:#064x}, selector: \ + {last_func_selector_felt:#064x}): +{expected_error}. " ) } @@ -578,9 +595,11 @@ An ASSERT_EQ instruction failed: 1 != 0. ", class_hash.0 ), - CairoVersion::Cairo1 => "The `validate` entry point panicked with \ - 0x496e76616c6964207363656e6172696f ('Invalid scenario')." - .into(), + CairoVersion::Cairo1 => format!( + "The `validate` entry point panicked with: +Error in contract (contract address: {contract_address:#064x}, selector: {selector:#064x}): +0x496e76616c6964207363656e6172696f ('Invalid scenario')." + ), }; // Clean pc locations from the trace. @@ -633,22 +652,26 @@ fn test_account_ctor_frame_stack_trace( hash: {:#064x}, selector: {expected_selector:#064x}): ", class_hash.0 - ) + match cairo_version { - CairoVersion::Cairo0 => { - "Error at pc=0:223: + ) + .to_string() + + &match cairo_version { + CairoVersion::Cairo0 => "Error at pc=0:223: Cairo traceback (most recent call last): Unknown location (pc=0:195) Unknown location (pc=0:179) An ASSERT_EQ instruction failed: 1 != 0. " - } - CairoVersion::Cairo1 => { - "Execution failed. Failure reason: 0x496e76616c6964207363656e6172696f ('Invalid \ - scenario'). + .to_string(), + CairoVersion::Cairo1 => format!( + "Execution failed. Failure reason: +Error in contract (contract address: {expected_address:#064x}, selector: \ + {expected_selector:#064x}): +0x496e76616c6964207363656e6172696f ('Invalid scenario'). " - } - }; + ) + .to_string(), + }; // Compare expected and actual error. let error = deploy_account_tx.execute(state, &block_context, true, true).unwrap_err(); @@ -774,10 +797,13 @@ Error at pc=0:{}: {frame_1} Error at pc=0:{}: {frame_2} -Execution failed. Failure reason: 0x496e76616c6964207363656e6172696f ('Invalid scenario'). +Execution failed. Failure reason: +Error in contract (contract address: {expected_address:#064x}, selector: {:#064x}): +0x496e76616c6964207363656e6172696f ('Invalid scenario'). ", execute_offset + 205, - deploy_offset + 194 + deploy_offset + 194, + ctor_selector.0 ) } }; 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 803da94381..ccb823cf1e 100644 --- a/crates/blockifier/src/execution/syscalls/syscall_tests/failure_format.rs +++ b/crates/blockifier/src/execution/syscalls/syscall_tests/failure_format.rs @@ -1,5 +1,5 @@ -use cairo_lang_utils::byte_array::BYTE_ARRAY_MAGIC; -use starknet_types_core::felt::Felt; +use starknet_api::core::{ContractAddress, EntryPointSelector}; +use starknet_api::felt; use crate::execution::call_info::{CallExecution, CallInfo, Retdata}; use crate::execution::errors::EntryPointExecutionError; @@ -7,21 +7,8 @@ use crate::execution::stack_trace::extract_trailing_cairo1_revert_trace; #[test] fn test_syscall_failure_format() { - 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 execution_failure = "0x457865637574696f6e206661696c757265"; + let error_data = Retdata(vec![felt!(execution_failure)]); let callinfo = CallInfo { execution: CallExecution { retdata: error_data, failed: true, ..Default::default() }, ..Default::default() @@ -29,5 +16,14 @@ fn test_syscall_failure_format() { let error = EntryPointExecutionError::ExecutionFailed { error_trace: extract_trailing_cairo1_revert_trace(&callinfo), }; - assert_eq!(error.to_string(), "Execution failed. Failure reason: \"Execution failure\"."); + assert_eq!( + error.to_string(), + format!( + "Execution failed. Failure reason: +Error in contract (contract address: {:#064x}, selector: {:#064x}): +{execution_failure} ('Execution failure').", + ContractAddress::default().0.key(), + EntryPointSelector::default().0 + ) + ); } diff --git a/crates/blockifier/src/transaction/errors.rs b/crates/blockifier/src/transaction/errors.rs index f0f0066039..adcf00744f 100644 --- a/crates/blockifier/src/transaction/errors.rs +++ b/crates/blockifier/src/transaction/errors.rs @@ -103,7 +103,7 @@ pub enum TransactionExecutionError { FeeCheckError(#[from] FeeCheckError), #[error(transparent)] FromStr(#[from] FromStrError), - #[error("The `validate` entry point panicked with {panic_reason}.")] + #[error("The `validate` entry point panicked with:\n{panic_reason}.")] PanicInValidate { panic_reason: String }, #[error("The `validate` entry point should return `VALID`. Got {actual:?}.")] InvalidValidateReturnData { actual: Retdata },