Feature: support reverted transactions (#334)

Problem: we panic explicitly if a block contains a reverted transaction,
and the OS fails if we remove these panics.

Solution: Fix support for reverted transactions. This PR simply
implements the `IS_REVERTED` hint correctly and removes
panic/unimplemented calls.

.github/workflows/prove_blocks.yml

@@ -53,4 +53,5 @@ jobs:
           # * 76766 / 76775: additional basic blocks
           # * 86507 / 124533: a failing assert that happened because we used the wrong VersionedConstants
           # * 87019: diff assert values in contract subcall
-          bash scripts/prove-blocks.sh -p ${{ secrets.PATHFINDER_RPC_URL }} -b 76793,76766,76775,86507,87019,124533
+          # * 76832: contains a reverted tx
+          bash scripts/prove-blocks.sh -p ${{ secrets.PATHFINDER_RPC_URL }} -b 76793,76766,76775,76832,86507,87019,124533

crates/bin/prove_block/src/reexecute.rs

@@ -58,15 +58,14 @@ pub fn reexecute_transactions_with_blockifier<S: StateReader>(
                 }
                 Ok(info) => {
                     if info.is_reverted() {
-                        log::error!(
+                        log::warn!(
                             "Transaction {} ({}/{}) reverted: {:?}",
                             tx_hash,
                             index + 1,
                             n_txs,
                             info.revert_error
                         );
                         log::warn!("TransactionExecutionInfo: {:?}", info);
-                        panic!("A transaction reverted during execution: {:?}", info);
                     }
                     info
                 }

crates/bin/prove_block/src/utils.rs

@@ -12,8 +12,6 @@ use starknet_api::state::StorageKey;
 // TODO: check if we can handle this just reexecuting tx using blockifier
 pub(crate) fn get_subcalled_contracts_from_tx_traces(traces: &[TransactionTraceWithHash]) -> HashSet<Felt252> {
     let mut contracts_subcalled: HashSet<Felt252> = HashSet::new();
-    // let traces = provider.trace_block_transactions(block_id).await.expect("Failed to get block tx
-    // traces");
     for trace in traces {
         match &trace.trace_root {
             TransactionTrace::Invoke(invoke_trace) => {
@@ -24,7 +22,9 @@ pub(crate) fn get_subcalled_contracts_from_tx_traces(traces: &[TransactionTraceW
                     ExecuteInvocation::Success(inv) => {
                         process_function_invocations(inv, &mut contracts_subcalled);
                     }
-                    ExecuteInvocation::Reverted(_) => unimplemented!("handle reverted invoke trace"),
+                    ExecuteInvocation::Reverted(_) => {
+                        // Nothing to do
+                    }
                 }
                 if let Some(inv) = &invoke_trace.fee_transfer_invocation {
                     process_function_invocations(inv, &mut contracts_subcalled);

crates/starknet-os/src/hints/execution.rs

@@ -894,18 +894,27 @@ where
 }
 
 pub const IS_REVERTED: &str = "memory[ap] = to_felt_or_relocatable(execution_helper.tx_execution_info.is_reverted)";
-pub fn is_reverted(
+pub fn is_reverted<PCS>(
     vm: &mut VirtualMachine,
-    _exec_scopes: &mut ExecutionScopes,
+    exec_scopes: &mut ExecutionScopes,
     _ids_data: &HashMap<String, HintReference>,
     _ap_tracking: &ApTracking,
     _constants: &HashMap<String, Felt252>,
-) -> Result<(), HintError> {
-    // TODO: implement is_reverted when tx_execution_info abstraction is ready
-    // let execution_helper =
-    // exec_scopes.get::<ExecutionHelperWrapper>(vars::scopes::EXECUTION_HELPER)?;
-    // insert_value_into_ap(vm, Felt252::from(execution_helper. tx_execution_info.is_reverted))
-    insert_value_into_ap(vm, Felt252::ZERO)
+) -> Result<(), HintError>
+where
+    PCS: PerContractStorage + 'static,
+{
+    let execution_helper = exec_scopes.get::<ExecutionHelperWrapper<PCS>>(vars::scopes::EXECUTION_HELPER)?;
+    let is_reverted = execute_coroutine(async {
+        let eh_ref = execution_helper.execution_helper.read().await;
+        eh_ref
+            .tx_execution_info
+            .as_ref()
+            .ok_or(HintError::CustomHint("ExecutionHelper should have tx_execution_info".to_owned().into_boxed_str()))
+            .map(|tx_execution_info| tx_execution_info.is_reverted())
+    })??;
+
+    insert_value_into_ap(vm, Felt252::from(is_reverted))
 }
 
 pub const CHECK_EXECUTION: &str = indoc! {r#"

crates/starknet-os/src/hints/mod.rs

@@ -132,7 +132,7 @@ fn hints<PCS>() -> HashMap<String, HintImpl> where
     hints.insert(execution::GET_OLD_BLOCK_NUMBER_AND_HASH.into(), execution::get_old_block_number_and_hash::<PCS>);
     hints.insert(execution::INITIAL_GE_REQUIRED_GAS.into(), execution::initial_ge_required_gas);
     hints.insert(execution::IS_DEPRECATED.into(), execution::is_deprecated);
-    hints.insert(execution::IS_REVERTED.into(), execution::is_reverted);
+    hints.insert(execution::IS_REVERTED.into(), execution::is_reverted::<PCS>);
     hints.insert(execution::LOAD_NEXT_TX.into(), execution::load_next_tx);
     hints.insert(execution::LOG_ENTER_SYSCALL.into(), execution::log_enter_syscall);
     hints.insert(execution::OS_CONTEXT_SEGMENTS.into(), execution::os_context_segments);