feat(blockifier): add address and selector for intermediate cairo1 revert errors

crates/blockifier/src/execution/errors.rs

@@ -80,7 +80,7 @@ impl From<RunnerError> 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),

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.

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
             )
         }
     };

crates/blockifier/src/execution/syscalls/syscall_tests/failure_format.rs

@@ -1,33 +1,29 @@
-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;
 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()
     };
     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
+        )
+    );
 }

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 },