From 6fd50ef214cb0be07b8a23a8714036b07f1baaf9 Mon Sep 17 00:00:00 2001 From: Dori Medini Date: Sat, 19 Oct 2024 17:55:14 +0300 Subject: [PATCH] feat(blockifier): add address and selector for intermediate cairo1 revert errors --- crates/blockifier/src/execution/call_info.rs | 21 +++++ crates/blockifier/src/execution/errors.rs | 2 +- .../blockifier/src/execution/stack_trace.rs | 12 ++- .../src/execution/stack_trace_test.rs | 91 ++++++++++++++----- .../syscalls/syscall_tests/failure_format.rs | 12 ++- crates/blockifier/src/transaction/errors.rs | 2 +- 6 files changed, 113 insertions(+), 27 deletions(-) diff --git a/crates/blockifier/src/execution/call_info.rs b/crates/blockifier/src/execution/call_info.rs index d538c72d7d2..d7fee8f3156 100644 --- a/crates/blockifier/src/execution/call_info.rs +++ b/crates/blockifier/src/execution/call_info.rs @@ -117,6 +117,11 @@ impl CallInfo { CallInfoIter { call_infos } } + /// Iterator over the rightmost branch of the callinfo tree rooted at self. + pub fn tail_iter(&self) -> CallInfoTailIter<'_> { + CallInfoTailIter { call_infos: vec![self] } + } + pub fn summarize(&self) -> ExecutionSummary { let mut executed_class_hashes: HashSet = HashSet::new(); let mut visited_storage_entries: HashSet = HashSet::new(); @@ -184,3 +189,19 @@ impl<'a> Iterator for CallInfoIter<'a> { Some(call_info) } } + +pub struct CallInfoTailIter<'a> { + call_infos: Vec<&'a CallInfo>, +} + +impl<'a> Iterator for CallInfoTailIter<'a> { + type Item = &'a CallInfo; + + fn next(&mut self) -> Option { + let call_info = self.call_infos.pop()?; + if let Some(next) = call_info.inner_calls.last() { + self.call_infos.push(next); + } + Some(call_info) + } +} diff --git a/crates/blockifier/src/execution/errors.rs b/crates/blockifier/src/execution/errors.rs index a5434201a54..ec4aa81eada 100644 --- a/crates/blockifier/src/execution/errors.rs +++ b/crates/blockifier/src/execution/errors.rs @@ -79,7 +79,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 0c02444da89..9f0043e5534 100644 --- a/crates/blockifier/src/execution/stack_trace.rs +++ b/crates/blockifier/src/execution/stack_trace.rs @@ -155,7 +155,17 @@ impl ErrorStack { } pub fn extract_trailing_cairo1_revert_trace(root_callinfo: &CallInfo) -> String { - format_panic_data(&root_callinfo.execution.retdata.0) + root_callinfo + .tail_iter() + .map(|callinfo| { + format!( + "Error in contract (contract address: {:#064x}, selector: {:#064x}):\n{}", + callinfo.call.storage_address.0.key(), + callinfo.call.entry_point_selector.0, + format_panic_data(&callinfo.execution.retdata.0) + ) + }) + .join("\n\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 bee3b05bcb9..5fd4cffa5be 100644 --- a/crates/blockifier/src/execution/stack_trace_test.rs +++ b/crates/blockifier/src/execution/stack_trace_test.rs @@ -214,8 +214,19 @@ 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}): +(0x6661696c ('fail'), 0x454e545259504f494e545f4641494c4544 ('ENTRYPOINT_FAILED'), \ + 0x454e545259504f494e545f4641494c4544 ('ENTRYPOINT_FAILED')) + +Error in contract (contract address: {test_contract_address_felt:#064x}, selector: \ + {external_entry_point_selector_felt:#064x}): +(0x6661696c ('fail'), 0x454e545259504f494e545f4641494c4544 ('ENTRYPOINT_FAILED')) + +Error in contract (contract address: {test_contract_address_2_felt:#064x}, selector: \ + {inner_entry_point_selector_felt:#064x}): +0x6661696c ('fail'). " ); @@ -331,9 +342,19 @@ 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}): +({expected_error}, 0x454e545259504f494e545f4641494c4544 ('ENTRYPOINT_FAILED'), \ + 0x454e545259504f494e545f4641494c4544 ('ENTRYPOINT_FAILED')) + +Error in contract (contract address: {contract_address_felt:#064x}, selector: \ + {invoke_call_chain_selector_felt:#064x}): +({expected_error}, 0x454e545259504f494e545f4641494c4544 ('ENTRYPOINT_FAILED')) + +Error in contract (contract address: {contract_address_felt:#064x}, selector: \ + {invoke_call_chain_selector_felt:#064x}): +{expected_error}. " ) } @@ -479,10 +500,25 @@ 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}): +({expected_error}, 0x454e545259504f494e545f4641494c4544 ('ENTRYPOINT_FAILED'), \ + 0x454e545259504f494e545f4641494c4544 ('ENTRYPOINT_FAILED'), \ + 0x454e545259504f494e545f4641494c4544 ('ENTRYPOINT_FAILED')) + +Error in contract (contract address: {address_felt:#064x}, selector: \ + {invoke_call_chain_selector_felt:#064x}): +({expected_error}, 0x454e545259504f494e545f4641494c4544 ('ENTRYPOINT_FAILED'), \ + 0x454e545259504f494e545f4641494c4544 ('ENTRYPOINT_FAILED')) + +Error in contract (contract address: {address_felt:#064x}, selector: \ + {invoke_call_chain_selector_felt:#064x}): +({expected_error}, 0x454e545259504f494e545f4641494c4544 ('ENTRYPOINT_FAILED')) + +Error in contract (contract address: {address_felt:#064x}, selector: \ + {last_func_selector_felt:#064x}): +{expected_error}. " ) } @@ -579,9 +615,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. @@ -634,22 +672,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(); @@ -775,10 +817,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 803da943815..fd582b4c7bd 100644 --- a/crates/blockifier/src/execution/syscalls/syscall_tests/failure_format.rs +++ b/crates/blockifier/src/execution/syscalls/syscall_tests/failure_format.rs @@ -1,4 +1,5 @@ use cairo_lang_utils::byte_array::BYTE_ARRAY_MAGIC; +use starknet_api::core::{ContractAddress, EntryPointSelector}; use starknet_types_core::felt::Felt; use crate::execution::call_info::{CallExecution, CallInfo, Retdata}; @@ -29,5 +30,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\".", + ContractAddress::default().0.key(), + EntryPointSelector::default().0 + ) + ); } diff --git a/crates/blockifier/src/transaction/errors.rs b/crates/blockifier/src/transaction/errors.rs index f0f0066039a..adcf00744f6 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 },