fix(executor/call): properly return error stack information for faile…

…d calls

Use blockifier's code to convert a failed call to a revert summary and
add then convert that to an error stack.

This adds proper error stacks for call failures where the failure does
not happen in the top-level call.

crates/executor/src/call.rs

@@ -6,6 +6,10 @@ use blockifier::execution::entry_point::{
     EntryPointExecutionContext,
     SierraGasRevertTracker,
 };
+use blockifier::execution::stack_trace::{
+    extract_trailing_cairo1_revert_trace,
+    Cairo1RevertHeader,
+};
 use blockifier::state::state_api::StateReader;
 use blockifier::transaction::objects::{DeprecatedTransactionInfo, TransactionInfo};
 use blockifier::versioned_constants::VersionedConstants;
@@ -73,13 +77,7 @@ pub fn call(
             )
         })?;
 
-    let error_stack_call_frame = crate::Frame::CallFrame(crate::CallFrame {
-        storage_address: contract_address,
-        class_hash: pathfinder_common::ClassHash(class_hash.0.into_felt()),
-        selector: Some(entry_point_selector),
-    });
-
-    // Sierra 1.7 classes can return a failure without reverting.
+    // In Starknet 0.13.4 calls return a failure which is not an error.
     if call_info.execution.failed {
         match call_info.execution.retdata.0.as_slice() {
             [error_code]
@@ -90,32 +88,13 @@ pub fn call(
             {
                 return Err(CallError::InvalidMessageSelector);
             }
-            [error_code]
-                if error_code.into_felt()
-                    == felt!(blockifier::execution::syscalls::hint_processor::OUT_OF_GAS_ERROR) =>
-            {
-                let error_message = "Out of gas";
-                let error_stack = crate::ErrorStack(vec![
-                    error_stack_call_frame,
-                    crate::Frame::StringFrame(error_message.to_owned()),
-                ]);
-
-                return Err(CallError::ContractError(
-                    anyhow::anyhow!(error_message),
-                    error_stack,
-                ));
-            }
             _ => {
-                let error_message =
-                    format!("Failed with retdata: {:?}", call_info.execution.retdata);
-                let error_stack = crate::ErrorStack(vec![
-                    error_stack_call_frame,
-                    crate::Frame::StringFrame(error_message.clone()),
-                ]);
+                let revert_trace =
+                    extract_trailing_cairo1_revert_trace(&call_info, Cairo1RevertHeader::Execution);
 
                 return Err(CallError::ContractError(
-                    anyhow::Error::msg(error_message),
-                    error_stack,
+                    anyhow::Error::msg(revert_trace.to_string()),
+                    revert_trace.into(),
                 ));
             }
         }

crates/executor/src/error_stack.rs

@@ -1,5 +1,7 @@
 use blockifier::execution::stack_trace::{
     gen_tx_execution_error_trace,
+    Cairo1RevertFrame,
+    Cairo1RevertSummary,
     ErrorStack as BlockifierErrorStack,
     ErrorStackSegment,
 };
@@ -36,6 +38,21 @@ impl From<RevertError> for ErrorStack {
     }
 }
 
+impl From<Cairo1RevertSummary> for ErrorStack {
+    fn from(value: Cairo1RevertSummary) -> Self {
+        let failure_reason =
+            starknet_api::execution_utils::format_panic_data(&value.last_retdata.0);
+        Self(
+            value
+                .stack
+                .into_iter()
+                .map(Into::into)
+                .chain(std::iter::once(Frame::StringFrame(failure_reason)))
+                .collect(),
+        )
+    }
+}
+
 #[derive(Clone, Debug)]
 pub enum Frame {
     CallFrame(CallFrame),
@@ -59,6 +76,17 @@ impl From<ErrorStackSegment> for Frame {
     }
 }
 
+impl From<Cairo1RevertFrame> for Frame {
+    fn from(value: Cairo1RevertFrame) -> Self {
+        Self::CallFrame(CallFrame {
+            storage_address: ContractAddress(value.contract_address.0.into_felt()),
+            // FIXME: what should we do here if the frame has no class hash?
+            class_hash: ClassHash(value.class_hash.unwrap_or_default().0.into_felt()),
+            selector: Some(EntryPoint(value.selector.0.into_felt())),
+        })
+    }
+}
+
 #[derive(Clone, Debug)]
 pub struct CallFrame {
     pub storage_address: ContractAddress,

crates/rpc/fixtures/contracts/caller/.gitignore

@@ -0,0 +1 @@
+target

crates/rpc/fixtures/contracts/caller/Scarb.lock

@@ -0,0 +1,6 @@
+# Code generated by scarb DO NOT EDIT.
+version = 1
+
+[[package]]
+name = "caller"
+version = "0.1.0"

crates/rpc/fixtures/contracts/caller/Scarb.toml

@@ -0,0 +1,13 @@
+[package]
+name = "caller"
+version = "0.1.0"
+edition = "2024_07"
+description = "A simple contract that calls other contracts."
+
+# See more keys and their definitions at https://docs.swmansion.com/scarb/docs/reference/manifest.html
+
+[dependencies]
+starknet = ">=2.9.2"
+
+[[target.starknet-contract]]
+casm = true

crates/rpc/fixtures/contracts/caller/src/lib.cairo

@@ -0,0 +1,31 @@
+use starknet::account::Call;
+
+#[starknet::interface]
+pub trait ICaller<TContractState> {
+    fn call(ref self: TContractState, calls: Array<Call>) -> Array<Span<felt252>>;
+}
+
+#[starknet::contract]
+pub mod Caller {
+    use starknet::account::Call;
+    use starknet::SyscallResultTrait;
+
+    #[storage]
+    struct Storage {}
+
+    #[abi(embed_v0)]
+    pub impl CallerImpl of super::ICaller<ContractState> {        
+        fn call(ref self: ContractState, calls: Array<Call>) -> Array<Span<felt252>> {
+            let mut res = array![];
+            for call in calls.span() {
+                res.append(execute_single_call(call))
+            };
+            res
+        }        
+    }
+
+    fn execute_single_call(call: @Call) -> Span<felt252> {
+        let Call { to, selector, calldata } = *call;
+        starknet::syscalls::call_contract_syscall(to, selector, calldata).unwrap_syscall()
+    }
+}

crates/rpc/src/method/call.rs

@@ -580,12 +580,8 @@ mod tests {
             };
 
             let error = call(context, input).await;
-            // Expecting retdata with "Failed to deserialize param #1".
-            let expected_error_string =
-                "Failed with retdata: \
-                 Retdata([0x4661696c656420746f20646573657269616c697a6520706172616d202331])";
             assert_matches::assert_matches!(error, Err(CallError::ContractError { revert_error, revert_error_stack }) => {
-                assert_eq!(revert_error, Some(format!("Execution error: {expected_error_string}")));
+                assert_eq!(revert_error, Some("Execution error: Execution failed. Failure reason:\nError in contract (contract address: 0x0000000000000000000000000000000000000000000000000000000000000c01, class hash: 0x019cabebe31b9fb6bf5e7ce9a971bd7d06e9999e0b97eee943869141a46fd978, selector: 0x0162da33a4585851fe8d3af3c2a9c60b557814e221e0d4f30ff0b2189d9c7775):\n0x4661696c656420746f20646573657269616c697a6520706172616d202331 ('Failed to deserialize param #1').\n".to_owned()));
                 assert_eq!(revert_error_stack.0.len(), 2);
                 assert_matches::assert_matches!(&revert_error_stack.0[0], pathfinder_executor::Frame::CallFrame(pathfinder_executor::CallFrame {
                     storage_address,
@@ -597,7 +593,139 @@ mod tests {
                     assert_matches::assert_matches!(selector, Some(entry_point) => assert_eq!(entry_point, &validate_entry_point));
                 });
                 assert_matches::assert_matches!(&revert_error_stack.0[1], pathfinder_executor::Frame::StringFrame(error_string) => {
-                    assert_eq!(error_string, expected_error_string);
+                    assert_eq!(error_string, "0x4661696c656420746f20646573657269616c697a6520706172616d202331 ('Failed to deserialize param #1')");
+                });
+            });
+        }
+
+        #[tokio::test]
+        async fn invalid_params_when_called_via_syscall() {
+            let (
+                storage,
+                last_block_header,
+                _account_contract_address,
+                _universal_deployer_address,
+                _test_storage_value,
+            ) = crate::method::simulate_transactions::tests::setup_storage_with_starknet_version(
+                StarknetVersion::new(0, 13, 4, 0),
+            )
+            .await;
+            let context = RpcContext::for_tests().with_storage(storage);
+
+            let sierra_definition = include_bytes!("../../fixtures/contracts/storage_access.json");
+            let sierra_hash =
+                sierra_hash!("0x03f6241e01a5afcb81f181518d74a1d3c8fc49c2aa583f805b67732e494ba9a8");
+            let casm_definition = include_bytes!("../../fixtures/contracts/storage_access.casm");
+            let casm_hash =
+                casm_hash!("0x069032ff71f77284e1a0864a573007108ca5cc08089416af50f03260f5d6d4d8");
+
+            let block_number = BlockNumber::new_or_panic(last_block_header.number.get() + 1);
+            let contract_address = contract_address!("0xcaaaa");
+            let caller_contract_address = contract_address!("0xccccc");
+            let storage_key = StorageAddress::from_name(b"my_storage_var");
+            let storage_value = storage_value!("0xb");
+
+            let caller_sierra_definition = include_bytes!(
+                "../../fixtures/contracts/caller/target/dev/caller_Caller.contract_class.json"
+            );
+            let caller_sierra_json: serde_json::Value =
+                serde_json::from_slice(caller_sierra_definition).unwrap();
+            let caller_sierra_definition = serde_json::json!({
+                "contract_class_version": caller_sierra_json["contract_class_version"],
+                "sierra_program": caller_sierra_json["sierra_program"],
+                "entry_points_by_type": caller_sierra_json["entry_points_by_type"],
+                "abi": serde_json::to_string(&caller_sierra_json["abi"]).unwrap(),
+            });
+            let caller_sierra_definition = serde_json::to_vec(&caller_sierra_definition).unwrap();
+            let caller_sierra_hash =
+                sierra_hash!("0x050d4827b118b6bef606c6e0ad4f33738b726e387de81b5ce045eb62d161bf9b");
+            let caller_casm_definition = include_bytes!(
+                "../../fixtures/contracts/caller/target/dev/caller_Caller.compiled_contract_class.\
+                 json"
+            );
+            let caller_casm_hash =
+                casm_hash!("0x02027e88d6cde8be7669d1baf9ac51f47fe52e600ced31cafba80eee1972a25b");
+
+            let mut connection = context.storage.connection().unwrap();
+            let tx = connection.transaction().unwrap();
+
+            tx.insert_sierra_class(&sierra_hash, sierra_definition, &casm_hash, casm_definition)
+                .unwrap();
+            tx.insert_sierra_class(
+                &caller_sierra_hash,
+                &caller_sierra_definition,
+                &caller_casm_hash,
+                caller_casm_definition,
+            )
+            .unwrap();
+
+            let header = BlockHeader::child_builder(&last_block_header)
+                .number(block_number)
+                .starknet_version(last_block_header.starknet_version)
+                .finalize_with_hash(block_hash!("0xb02"));
+            tx.insert_block_header(&header).unwrap();
+
+            let state_update = StateUpdate::default()
+                .with_declared_sierra_class(sierra_hash, casm_hash)
+                .with_declared_sierra_class(caller_sierra_hash, caller_casm_hash)
+                .with_deployed_contract(contract_address, ClassHash(*sierra_hash.get()))
+                .with_deployed_contract(
+                    caller_contract_address,
+                    ClassHash(*caller_sierra_hash.get()),
+                )
+                .with_storage_update(contract_address, storage_key, storage_value);
+            tx.insert_state_update(block_number, &state_update).unwrap();
+
+            tx.commit().unwrap();
+            drop(connection);
+
+            // Our test account class is Sierra 1.7, so the easiest is just to call that.
+            let caller_entry_point = EntryPoint::hashed(b"call");
+            let input = Input {
+                request: FunctionCall {
+                    contract_address: caller_contract_address,
+                    entry_point_selector: caller_entry_point,
+                    calldata: vec![
+                        // Number of calls
+                        call_param!("0x1"),
+                        // Called contract address
+                        CallParam(*caller_contract_address.get()),
+                        // Entry point selector for the called contract
+                        CallParam(EntryPoint::hashed(b"call").0),
+                        // Length of the call data for the called contract
+                        call_param!("1"),
+                        // Number of calls, but then no more data; leads to deserailization error
+                        call_param!("0x1"),
+                    ],
+                },
+                block_id: BlockId::Latest,
+            };
+
+            let error = call(context, input).await;
+
+            assert_matches::assert_matches!(error, Err(CallError::ContractError { revert_error, revert_error_stack }) => {
+                assert_eq!(revert_error, Some("Execution error: Execution failed. Failure reason:\nError in contract (contract address: 0x00000000000000000000000000000000000000000000000000000000000ccccc, class hash: 0x050d4827b118b6bef606c6e0ad4f33738b726e387de81b5ce045eb62d161bf9b, selector: 0x031a75a0d711dfe3639aae96eb8f9facc2fd74df5aa611067f2511cc9fefc229):\nError in contract (contract address: 0x00000000000000000000000000000000000000000000000000000000000ccccc, class hash: 0x050d4827b118b6bef606c6e0ad4f33738b726e387de81b5ce045eb62d161bf9b, selector: 0x031a75a0d711dfe3639aae96eb8f9facc2fd74df5aa611067f2511cc9fefc229):\n0x4661696c656420746f20646573657269616c697a6520706172616d202331 ('Failed to deserialize param #1').\n".to_owned()));
+                assert_eq!(revert_error_stack.0.len(), 3);
+                assert_matches::assert_matches!(&revert_error_stack.0[0], pathfinder_executor::Frame::CallFrame(pathfinder_executor::CallFrame {
+                    storage_address,
+                    class_hash,
+                    selector,
+                }) => {
+                    assert_eq!(storage_address, &caller_contract_address);
+                    assert_eq!(class_hash, &ClassHash(caller_sierra_hash.0));
+                    assert_matches::assert_matches!(selector, Some(entry_point) => assert_eq!(entry_point, &caller_entry_point));
+                });
+                assert_matches::assert_matches!(&revert_error_stack.0[1], pathfinder_executor::Frame::CallFrame(pathfinder_executor::CallFrame {
+                    storage_address,
+                    class_hash,
+                    selector,
+                }) => {
+                    assert_eq!(storage_address, &caller_contract_address);
+                    assert_eq!(class_hash, &ClassHash(caller_sierra_hash.0));
+                    assert_matches::assert_matches!(selector, Some(entry_point) => assert_eq!(entry_point, &caller_entry_point));
+                });
+                assert_matches::assert_matches!(&revert_error_stack.0[2], pathfinder_executor::Frame::StringFrame(error_string) => {
+                    assert_eq!(error_string, "0x4661696c656420746f20646573657269616c697a6520706172616d202331 ('Failed to deserialize param #1')");
                 });
             });
         }