feat: support reverts of inner calls

crates/blockifier/feature_contracts/cairo1/test_contract.cairo

@@ -71,6 +71,41 @@ mod TestContract {
             .span()
     }
 
+
+    #[external(v0)]
+    fn test_call_contract_revert(
+        ref self: ContractState,
+        contract_address: ContractAddress,
+        entry_point_selector: felt252,
+        calldata: Array::<felt252>
+    ) {
+         match syscalls::call_contract_syscall(
+            contract_address, entry_point_selector, calldata.span())
+        {
+            Result::Ok(_) => panic!("Expected revert"),
+            Result::Err(errors) => {
+                let mut error_span = errors.span();
+                assert(
+                    *error_span.pop_back().unwrap() == 'ENTRYPOINT_FAILED',
+                    'Unexpected error',
+                );
+            },
+        };
+        // TODO(Yoni, 1/12/2024): test replace class once get_class_hash_at syscall is supported.
+        assert(self.my_storage_var.read() == 0, 'values should not change.');
+    }
+
+
+    #[external(v0)]
+    fn test_revert_helper(ref self: ContractState, class_hash: ClassHash) {
+        let dummy_span = array![0].span();
+        syscalls::emit_event_syscall(dummy_span, dummy_span).unwrap_syscall();
+        syscalls::replace_class_syscall(class_hash).unwrap_syscall();
+        syscalls::send_message_to_l1_syscall(17.try_into().unwrap(), dummy_span).unwrap_syscall();
+        self.my_storage_var.write(17);
+        panic!("test_revert_helper");
+    }
+
     #[external(v0)]
     fn test_emit_events(
         self: @ContractState, events_number: u64, keys: Array::<felt252>, data: Array::<felt252>

crates/blockifier/src/execution/deprecated_syscalls/hint_processor.rs

@@ -502,8 +502,12 @@ pub fn execute_inner_call(
     vm: &mut VirtualMachine,
     syscall_handler: &mut DeprecatedSyscallHintProcessor<'_>,
 ) -> DeprecatedSyscallResult<ReadOnlySegment> {
-    let call_info =
-        call.execute(syscall_handler.state, syscall_handler.resources, syscall_handler.context)?;
+    // Use `non_reverting_execute` since we don't support reverts here.
+    let call_info = call.non_reverting_execute(
+        syscall_handler.state,
+        syscall_handler.resources,
+        syscall_handler.context,
+    )?;
     let retdata = &call_info.execution.retdata.0;
     let retdata: Vec<MaybeRelocatable> =
         retdata.iter().map(|&x| MaybeRelocatable::from(x)).collect();

crates/blockifier/src/execution/entry_point.rs

@@ -1,12 +1,14 @@
 use std::cell::RefCell;
 use std::cmp::min;
+use std::collections::HashMap;
 use std::sync::Arc;
 
 use cairo_vm::vm::runners::cairo_runner::{ExecutionResources, ResourceTracker, RunResources};
 use num_traits::{Inv, Zero};
 use serde::Serialize;
 use starknet_api::core::{ClassHash, ContractAddress, EntryPointSelector};
 use starknet_api::deprecated_contract_class::EntryPointType;
+use starknet_api::state::StorageKey;
 use starknet_api::transaction::{Calldata, TransactionVersion};
 use starknet_types_core::felt::Felt;
 
@@ -21,7 +23,7 @@ use crate::execution::errors::{
     PreExecutionError,
 };
 use crate::execution::execution_utils::execute_entry_point_call;
-use crate::state::state_api::State;
+use crate::state::state_api::{State, StateResult};
 use crate::transaction::objects::{HasRelatedFeeType, TransactionInfo};
 use crate::transaction::transaction_types::TransactionType;
 use crate::utils::{u128_from_usize, usize_from_u128};
@@ -37,6 +39,43 @@ pub const FAULTY_CLASS_HASH: &str =
 pub type EntryPointExecutionResult<T> = Result<T, EntryPointExecutionError>;
 pub type ConstructorEntryPointExecutionResult<T> = Result<T, ConstructorEntryPointExecutionError>;
 
+/// Holds the the information required to revert the execution of an entry point.
+#[derive(Debug)]
+pub struct EntryPointRevertInfo {
+    // The contract address that the revert info applies to.
+    pub contract_address: ContractAddress,
+    /// The original class hash of the contract that was called.
+    pub original_class_hash: ClassHash,
+    /// The original storage values.
+    pub original_values: HashMap<StorageKey, Felt>,
+    // The number of emitted events before the call.
+    n_emitted_events: usize,
+    // The number of sent messages to L1 before the call.
+    n_sent_messages_to_l1: usize,
+}
+impl EntryPointRevertInfo {
+    pub fn new(
+        contract_address: ContractAddress,
+        original_class_hash: ClassHash,
+        n_emitted_events: usize,
+        n_sent_messages_to_l1: usize,
+    ) -> Self {
+        Self {
+            contract_address,
+            original_class_hash,
+            original_values: HashMap::new(),
+            n_emitted_events,
+            n_sent_messages_to_l1,
+        }
+    }
+}
+
+/// The ExecutionRevertInfo stores a vector of entry point revert infos.
+/// We don't merge infos related same contract as doing it on every nesting level would
+/// result in O(N^2) complexity.
+#[derive(Default, Debug)]
+pub struct ExecutionRevertInfo(pub Vec<EntryPointRevertInfo>);
+
 /// Represents a the type of the call (used for debugging).
 #[cfg_attr(feature = "transaction_serde", derive(serde::Deserialize))]
 #[derive(Clone, Copy, Debug, Default, Eq, Hash, PartialEq, Serialize)]
@@ -102,8 +141,37 @@ impl CallEntryPoint {
         self.class_hash = Some(class_hash);
         let contract_class = state.get_compiled_contract_class(class_hash)?;
 
+        context.revert_infos.0.push(EntryPointRevertInfo::new(
+            self.storage_address,
+            storage_class_hash,
+            context.n_emitted_events,
+            context.n_sent_messages_to_l1,
+        ));
+
         execute_entry_point_call(self, contract_class, state, resources, context)
     }
+
+    /// Similar to `execute`, but returns an error if the outer call is reverted.
+    pub fn non_reverting_execute(
+        self,
+        state: &mut dyn State,
+        resources: &mut ExecutionResources,
+        context: &mut EntryPointExecutionContext,
+    ) -> EntryPointExecutionResult<CallInfo> {
+        let execution_result = self.execute(state, resources, context);
+        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(),
+                });
+            }
+        }
+
+        execution_result
+    }
 }
 
 pub struct ConstructorContext {
@@ -130,6 +198,8 @@ pub struct EntryPointExecutionContext {
 
     // The execution mode affects the behavior of the hint processor.
     pub execution_mode: ExecutionMode,
+
+    pub revert_infos: ExecutionRevertInfo,
 }
 
 impl EntryPointExecutionContext {
@@ -146,6 +216,7 @@ impl EntryPointExecutionContext {
             tx_context: tx_context.clone(),
             current_recursion_depth: Default::default(),
             execution_mode: mode,
+            revert_infos: ExecutionRevertInfo(vec![]),
         }
     }
 
@@ -287,6 +358,24 @@ impl EntryPointExecutionContext {
     pub fn gas_costs(&self) -> &GasCosts {
         &self.versioned_constants().os_constants.gas_costs
     }
+
+    /// Reverts the state back to the way it was when self.revert_infos.0['revert_idx'] was created.
+    pub fn revert(&mut self, revert_idx: usize, state: &mut dyn State) -> StateResult<()> {
+        for contract_revert_info in self.revert_infos.0.drain(revert_idx..).rev() {
+            for (key, value) in contract_revert_info.original_values.iter() {
+                state.set_storage_at(contract_revert_info.contract_address, *key, *value)?;
+            }
+            state.set_class_hash_at(
+                contract_revert_info.contract_address,
+                contract_revert_info.original_class_hash,
+            )?;
+
+            self.n_emitted_events = contract_revert_info.n_emitted_events;
+            self.n_sent_messages_to_l1 = contract_revert_info.n_sent_messages_to_l1;
+        }
+
+        Ok(())
+    }
 }
 
 pub fn execute_constructor_entry_point(
@@ -320,7 +409,7 @@ pub fn execute_constructor_entry_point(
         initial_gas: remaining_gas,
     };
 
-    constructor_call.execute(state, resources, context).map_err(|error| {
+    constructor_call.non_reverting_execute(state, resources, context).map_err(|error| {
         ConstructorEntryPointExecutionError::new(error, &ctor_context, Some(constructor_selector))
     })
 }

crates/blockifier/src/execution/entry_point_execution.rs

@@ -104,11 +104,6 @@ pub fn execute_entry_point_call(
         n_total_args,
         program_extra_data_length,
     )?;
-    if call_info.execution.failed {
-        return Err(EntryPointExecutionError::ExecutionFailed {
-            error_data: call_info.execution.retdata.0,
-        });
-    }
 
     Ok(call_info)
 }
@@ -369,7 +364,7 @@ fn maybe_fill_holes(
 
 pub fn finalize_execution(
     mut runner: CairoRunner,
-    syscall_handler: SyscallHintProcessor<'_>,
+    mut syscall_handler: SyscallHintProcessor<'_>,
     previous_resources: ExecutionResources,
     n_total_args: usize,
     program_extra_data_length: usize,
@@ -409,6 +404,8 @@ pub fn finalize_execution(
     *syscall_handler.resources += &versioned_constants
         .get_additional_os_syscall_resources(&syscall_handler.syscall_counter)?;
 
+    syscall_handler.finalize();
+
     let full_call_resources = &*syscall_handler.resources - &previous_resources;
     Ok(CallInfo {
         call: syscall_handler.call,

crates/blockifier/src/execution/stack_trace_test.rs

@@ -39,8 +39,6 @@ use crate::transaction::test_utils::{
 use crate::transaction::transaction_types::TransactionType;
 use crate::transaction::transactions::ExecutableTransaction;
 
-const INNER_CALL_CONTRACT_IN_CALL_CHAIN_OFFSET: usize = 117;
-
 #[rstest]
 fn test_stack_trace_with_inner_error_msg(block_context: BlockContext) {
     let cairo_version = CairoVersion::Cairo0;
@@ -212,19 +210,16 @@ An ASSERT_EQ instruction failed: 1 != 0.
 "
     );
 
-    let expected_trace_cairo1 = format!(
+    let expected_trace_cairo1 =
         "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}):
-Error at pc=0:767:
-1: Error in the called contract (contract address: {test_contract_address_felt:#064x}, class hash: \
-         {test_contract_hash:#064x}, selector: {external_entry_point_selector_felt:#064x}):
-Error at pc=0:612:
-2: Error in the called contract (contract address: {test_contract_address_2_felt:#064x}, class \
-         hash: {test_contract_hash:#064x}, selector: {inner_entry_point_selector_felt:#064x}):
-Execution failed. Failure reason: 0x6661696c ('fail').
+0: Error in the called contract (contract address: \
+         0x00000000000000000000000000000000000000000000000000000000c0020000, class hash: \
+         0x0000000000000000000000000000000000000000000000000000000080020000, selector: \
+         0x015d40a3d6ca2ac30f4031e42be28da9b056fef9bb7357ac5e85627ee876e5ad):
+Execution failed. Failure reason: (0x6661696c ('fail'), 0x454e545259504f494e545f4641494c4544 \
+         ('ENTRYPOINT_FAILED'), 0x454e545259504f494e545f4641494c4544 ('ENTRYPOINT_FAILED')).
 "
-    );
+        .into();
 
     let expected_trace = match cairo_version {
         CairoVersion::Cairo0 => expected_trace_cairo0,
@@ -334,21 +329,13 @@ Unknown location (pc=0:{expected_pc1})
             )
         }
         CairoVersion::Cairo1 => {
-            let pc_location = entry_point_offset.0 + INNER_CALL_CONTRACT_IN_CALL_CHAIN_OFFSET;
             format!(
                 "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}):
-Error at pc=0:767:
-1: Error in the called contract (contract address: {contract_address_felt:#064x}, class hash: \
-                 {test_contract_hash:#064x}, selector: {invoke_call_chain_selector_felt:#064x}):
-Error at pc=0:9631:
-Cairo traceback (most recent call last):
-Unknown location (pc=0:{pc_location})
-
-2: Error in the called contract (contract address: {contract_address_felt:#064x}, class hash: \
-                 {test_contract_hash:#064x}, selector: {invoke_call_chain_selector_felt:#064x}):
-Execution failed. Failure reason: {expected_error}.
+Execution failed. Failure reason: ({expected_error}, 0x454e545259504f494e545f4641494c4544 \
+                 ('ENTRYPOINT_FAILED'), 0x454e545259504f494e545f4641494c4544 \
+                 ('ENTRYPOINT_FAILED')).
 "
             )
         }
@@ -490,27 +477,16 @@ Unknown location (pc=0:{expected_pc3})
             )
         }
         CairoVersion::Cairo1 => {
-            let pc_location = entry_point_offset.0 + INNER_CALL_CONTRACT_IN_CALL_CHAIN_OFFSET;
-            let (expected_pc0, expected_pc1, _, _) = expected_pcs;
             format!(
                 "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}):
-Error at pc=0:767:
-1: Error in the called contract (contract address: {address_felt:#064x}, class hash: \
-                 {test_contract_hash:#064x}, selector: {invoke_call_chain_selector_felt:#064x}):
-Error at pc=0:{expected_pc0}:
-Cairo traceback (most recent call last):
-Unknown location (pc=0:{pc_location})
-
-2: Error in the called contract (contract address: {address_felt:#064x}, class hash: \
-                 {test_contract_hash:#064x}, selector: {invoke_call_chain_selector_felt:#064x}):
-Error at pc=0:{expected_pc1}:
-Cairo traceback (most recent call last):
-Unknown location (pc=0:{pc_location})
-
-3: {last_call_preamble}:
-Execution failed. Failure reason: {expected_error}.
+0: Error in the called contract (contract address: \
+                 0x00000000000000000000000000000000000000000000000000000000c0020000, class hash: \
+                 0x0000000000000000000000000000000000000000000000000000000080020000, selector: \
+                 0x015d40a3d6ca2ac30f4031e42be28da9b056fef9bb7357ac5e85627ee876e5ad):
+Execution failed. Failure reason: ({expected_error}, 0x454e545259504f494e545f4641494c4544 \
+                 ('ENTRYPOINT_FAILED'), 0x454e545259504f494e545f4641494c4544 \
+                 ('ENTRYPOINT_FAILED'), 0x454e545259504f494e545f4641494c4544 \
+                 ('ENTRYPOINT_FAILED')).
 "
             )
         }
@@ -607,14 +583,9 @@ An ASSERT_EQ instruction failed: 1 != 0.
 ",
             class_hash.0
         ),
-        CairoVersion::Cairo1 => format!(
-            "Transaction validation has failed:
-0: Error in the called contract (contract address: {contract_address:#064x}, class hash: {:#064x}, \
-             selector: {selector:#064x}):
-Execution failed. Failure reason: 0x496e76616c6964207363656e6172696f ('Invalid scenario').
-",
-            class_hash.0
-        ),
+        CairoVersion::Cairo1 => "The `validate` entry point panicked with \
+                                 0x496e76616c6964207363656e6172696f ('Invalid scenario')."
+            .into(),
     };
 
     // Clean pc locations from the trace.