Merge pull request #5814 from starkware-libs/aner/global_execute_syscall_fn

refactor(blockifier): global fn execute_syscall_from_selector

Author: aner-starkware

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

@@ -61,7 +61,10 @@ use crate::execution::syscalls::secp::{
     SecpNewResponse,
 };
 use crate::execution::syscalls::syscall_base::SyscallHandlerBase;
-use crate::execution::syscalls::syscall_executor::SyscallExecutor;
+use crate::execution::syscalls::syscall_executor::{
+    execute_syscall_from_selector,
+    SyscallExecutor,
+};
 use crate::execution::syscalls::{
     CallContractRequest,
     CallContractResponse,
@@ -338,7 +341,7 @@ impl<'a> SyscallHintProcessor<'a> {
             self.increment_syscall_count(&selector);
         }
 
-        self.execute_syscall_from_selector(vm, selector)
+        execute_syscall_from_selector(self, vm, selector)
     }
 
     pub fn get_or_allocate_execution_info_segment(

crates/blockifier/src/execution/syscalls/syscall_executor.rs

@@ -57,91 +57,7 @@ use crate::execution::syscalls::{
 };
 use crate::utils::u64_from_usize;
 
-#[allow(dead_code)]
 pub trait SyscallExecutor {
-    fn execute_syscall_from_selector(
-        &mut self,
-        vm: &mut VirtualMachine,
-        selector: SyscallSelector,
-    ) -> HintExecutionResult {
-        match selector {
-            SyscallSelector::CallContract => {
-                self.execute_syscall(vm, selector, Self::call_contract)
-            }
-            SyscallSelector::Deploy => self.execute_syscall(vm, selector, Self::deploy),
-            SyscallSelector::EmitEvent => self.execute_syscall(vm, selector, Self::emit_event),
-            SyscallSelector::GetBlockHash => {
-                self.execute_syscall(vm, selector, Self::get_block_hash)
-            }
-            SyscallSelector::GetClassHashAt => {
-                self.execute_syscall(vm, selector, Self::get_class_hash_at)
-            }
-            SyscallSelector::GetExecutionInfo => {
-                self.execute_syscall(vm, selector, Self::get_execution_info)
-            }
-            SyscallSelector::Keccak => self.execute_syscall(vm, selector, Self::keccak),
-            SyscallSelector::Sha256ProcessBlock => {
-                self.execute_syscall(vm, selector, Self::sha256_process_block)
-            }
-            SyscallSelector::LibraryCall => self.execute_syscall(vm, selector, Self::library_call),
-            SyscallSelector::MetaTxV0 => self.execute_syscall(vm, selector, Self::meta_tx_v0),
-            SyscallSelector::ReplaceClass => {
-                self.execute_syscall(vm, selector, Self::replace_class)
-            }
-            SyscallSelector::Secp256k1Add => {
-                self.execute_syscall(vm, selector, Self::secp256k1_add)
-            }
-            SyscallSelector::Secp256k1GetPointFromX => {
-                self.execute_syscall(vm, selector, Self::secp256k1_get_point_from_x)
-            }
-            SyscallSelector::Secp256k1GetXy => {
-                self.execute_syscall(vm, selector, Self::secp256k1_get_xy)
-            }
-            SyscallSelector::Secp256k1Mul => {
-                self.execute_syscall(vm, selector, Self::secp256k1_mul)
-            }
-            SyscallSelector::Secp256k1New => {
-                self.execute_syscall(vm, selector, Self::secp256k1_new)
-            }
-            SyscallSelector::Secp256r1Add => {
-                self.execute_syscall(vm, selector, Self::secp256r1_add)
-            }
-            SyscallSelector::Secp256r1GetPointFromX => {
-                self.execute_syscall(vm, selector, Self::secp256r1_get_point_from_x)
-            }
-            SyscallSelector::Secp256r1GetXy => {
-                self.execute_syscall(vm, selector, Self::secp256r1_get_xy)
-            }
-            SyscallSelector::Secp256r1Mul => {
-                self.execute_syscall(vm, selector, Self::secp256r1_mul)
-            }
-            SyscallSelector::Secp256r1New => {
-                self.execute_syscall(vm, selector, Self::secp256r1_new)
-            }
-            SyscallSelector::SendMessageToL1 => {
-                self.execute_syscall(vm, selector, Self::send_message_to_l1)
-            }
-            SyscallSelector::StorageRead => self.execute_syscall(vm, selector, Self::storage_read),
-            SyscallSelector::StorageWrite => {
-                self.execute_syscall(vm, selector, Self::storage_write)
-            }
-            // Explicitly list unsupported syscalls, so compiler can catch if a syscall is missing.
-            SyscallSelector::DelegateCall
-            | SyscallSelector::DelegateL1Handler
-            | SyscallSelector::GetBlockNumber
-            | SyscallSelector::GetBlockTimestamp
-            | SyscallSelector::GetCallerAddress
-            | SyscallSelector::GetContractAddress
-            | SyscallSelector::GetSequencerAddress
-            | SyscallSelector::GetTxInfo
-            | SyscallSelector::GetTxSignature
-            | SyscallSelector::KeccakRound
-            | SyscallSelector::LibraryCallL1Handler => Err(HintError::UnknownHint(
-                format!("Unsupported syscall selector {selector:?}.").into(),
-            )),
-        }
-    }
-
     fn get_gas_cost_from_selector(
         &self,
         selector: &SyscallSelector,
@@ -153,89 +69,6 @@ pub trait SyscallExecutor {
 
     fn update_revert_gas_with_next_remaining_gas(&mut self, next_remaining_gas: GasAmount);
 
-    fn execute_syscall<Request, Response, ExecuteCallback>(
-        &mut self,
-        vm: &mut VirtualMachine,
-        selector: SyscallSelector,
-        execute_callback: ExecuteCallback,
-    ) -> HintExecutionResult
-    where
-        Request: SyscallRequest + std::fmt::Debug,
-        Response: SyscallResponse + std::fmt::Debug,
-        ExecuteCallback: FnOnce(
-            Request,
-            &mut VirtualMachine,
-            &mut Self,
-            &mut u64, // Remaining gas.
-        ) -> SyscallResult<Response>,
-    {
-        let syscall_gas_cost = self.get_gas_cost_from_selector(&selector).map_err(|e| {
-            HintError::CustomHint(
-                format!("Failed to get gas cost for syscall selector {selector:?}. Error: {e:?}")
-                    .into(),
-            )
-        })?;
-
-        let SyscallRequestWrapper { gas_counter, request } =
-            SyscallRequestWrapper::<Request>::read(vm, self.get_mut_syscall_ptr())?;
-
-        let syscall_gas_cost =
-            syscall_gas_cost.get_syscall_cost(u64_from_usize(request.get_linear_factor_length()));
-        let syscall_base_cost = self.get_syscall_base_gas_cost();
-
-        // Sanity check for preventing underflow.
-        assert!(
-            syscall_gas_cost >= syscall_base_cost,
-            "Syscall gas cost must be greater than base syscall gas cost"
-        );
-
-        // Refund `SYSCALL_BASE_GAS_COST` as it was pre-charged.
-        let required_gas = syscall_gas_cost - syscall_base_cost;
-
-        if gas_counter < required_gas {
-            //  Out of gas failure.
-            let out_of_gas_error =
-                Felt::from_hex(OUT_OF_GAS_ERROR).map_err(SyscallExecutionError::from)?;
-            let response: SyscallResponseWrapper<Response> =
-                SyscallResponseWrapper::Failure { gas_counter, error_data: vec![out_of_gas_error] };
-            response.write(vm, self.get_mut_syscall_ptr())?;
-
-            return Ok(());
-        }
-
-        // Execute.
-        let mut remaining_gas = gas_counter - required_gas;
-
-        // To support sierra gas charge for blockifier revert flow, we track the remaining gas left
-        // before executing a syscall if the current tracked resource is gas.
-        // 1. If the syscall does not run Cairo code (i.e. not library call, not call contract, and
-        //    not a deploy), any failure will not run in the OS, so no need to charge - the value
-        //    before entering the callback is good enough to charge.
-        // 2. If the syscall runs Cairo code, but the tracked resource is steps (and not gas), the
-        //    additional charge of reverted cairo steps will cover the inner cost, and the outer
-        //    cost we track here will be the additional reverted gas.
-        // 3. If the syscall runs Cairo code and the tracked resource is gas, either the inner
-        //    failure will be a Cairo1 revert (and the gas consumed on the call info will override
-        //    the current tracked value), or we will pass through another syscall before failing -
-        //    and by induction (we will reach this point again), the gas will be charged correctly.
-        self.update_revert_gas_with_next_remaining_gas(GasAmount(remaining_gas));
-
-        let original_response = execute_callback(request, vm, self, &mut remaining_gas);
-        let response = match original_response {
-            Ok(response) => {
-                SyscallResponseWrapper::Success { gas_counter: remaining_gas, response }
-            }
-            Err(SyscallExecutionError::Revert { error_data: data }) => {
-                SyscallResponseWrapper::Failure { gas_counter: remaining_gas, error_data: data }
-            }
-            Err(error) => return Err(error.into()),
-        };
-
-        response.write(vm, self.get_mut_syscall_ptr())?;
-
-        Ok(())
-    }
-
     fn call_contract(
         request: CallContractRequest,
         vm: &mut VirtualMachine,
@@ -404,3 +237,174 @@ pub trait SyscallExecutor {
         remaining_gas: &mut u64,
     ) -> SyscallResult<StorageWriteResponse>;
 }
+
+pub(crate) fn execute_syscall_from_selector<T: SyscallExecutor>(
+    syscall_executor: &mut T,
+    vm: &mut VirtualMachine,
+    selector: SyscallSelector,
+) -> HintExecutionResult {
+    match selector {
+        SyscallSelector::CallContract => {
+            execute_syscall(syscall_executor, vm, selector, T::call_contract)
+        }
+        SyscallSelector::Deploy => execute_syscall(syscall_executor, vm, selector, T::deploy),
+        SyscallSelector::EmitEvent => {
+            execute_syscall(syscall_executor, vm, selector, T::emit_event)
+        }
+        SyscallSelector::GetBlockHash => {
+            execute_syscall(syscall_executor, vm, selector, T::get_block_hash)
+        }
+        SyscallSelector::GetClassHashAt => {
+            execute_syscall(syscall_executor, vm, selector, T::get_class_hash_at)
+        }
+        SyscallSelector::GetExecutionInfo => {
+            execute_syscall(syscall_executor, vm, selector, T::get_execution_info)
+        }
+        SyscallSelector::Keccak => execute_syscall(syscall_executor, vm, selector, T::keccak),
+        SyscallSelector::Sha256ProcessBlock => {
+            execute_syscall(syscall_executor, vm, selector, T::sha256_process_block)
+        }
+        SyscallSelector::LibraryCall => {
+            execute_syscall(syscall_executor, vm, selector, T::library_call)
+        }
+        SyscallSelector::MetaTxV0 => execute_syscall(syscall_executor, vm, selector, T::meta_tx_v0),
+        SyscallSelector::ReplaceClass => {
+            execute_syscall(syscall_executor, vm, selector, T::replace_class)
+        }
+        SyscallSelector::Secp256k1Add => {
+            execute_syscall(syscall_executor, vm, selector, T::secp256k1_add)
+        }
+        SyscallSelector::Secp256k1GetPointFromX => {
+            execute_syscall(syscall_executor, vm, selector, T::secp256k1_get_point_from_x)
+        }
+        SyscallSelector::Secp256k1GetXy => {
+            execute_syscall(syscall_executor, vm, selector, T::secp256k1_get_xy)
+        }
+        SyscallSelector::Secp256k1Mul => {
+            execute_syscall(syscall_executor, vm, selector, T::secp256k1_mul)
+        }
+        SyscallSelector::Secp256k1New => {
+            execute_syscall(syscall_executor, vm, selector, T::secp256k1_new)
+        }
+        SyscallSelector::Secp256r1Add => {
+            execute_syscall(syscall_executor, vm, selector, T::secp256r1_add)
+        }
+        SyscallSelector::Secp256r1GetPointFromX => {
+            execute_syscall(syscall_executor, vm, selector, T::secp256r1_get_point_from_x)
+        }
+        SyscallSelector::Secp256r1GetXy => {
+            execute_syscall(syscall_executor, vm, selector, T::secp256r1_get_xy)
+        }
+        SyscallSelector::Secp256r1Mul => {
+            execute_syscall(syscall_executor, vm, selector, T::secp256r1_mul)
+        }
+        SyscallSelector::Secp256r1New => {
+            execute_syscall(syscall_executor, vm, selector, T::secp256r1_new)
+        }
+        SyscallSelector::SendMessageToL1 => {
+            execute_syscall(syscall_executor, vm, selector, T::send_message_to_l1)
+        }
+        SyscallSelector::StorageRead => {
+            execute_syscall(syscall_executor, vm, selector, T::storage_read)
+        }
+        SyscallSelector::StorageWrite => {
+            execute_syscall(syscall_executor, vm, selector, T::storage_write)
+        }
+        // Explicitly list unsupported syscalls, so compiler can catch if a syscall is missing.
+        SyscallSelector::DelegateCall
+        | SyscallSelector::DelegateL1Handler
+        | SyscallSelector::GetBlockNumber
+        | SyscallSelector::GetBlockTimestamp
+        | SyscallSelector::GetCallerAddress
+        | SyscallSelector::GetContractAddress
+        | SyscallSelector::GetSequencerAddress
+        | SyscallSelector::GetTxInfo
+        | SyscallSelector::GetTxSignature
+        | SyscallSelector::KeccakRound
+        | SyscallSelector::LibraryCallL1Handler => Err(HintError::UnknownHint(
+            format!("Unsupported syscall selector {selector:?}.").into(),
+        )),
+    }
+}
+
+fn execute_syscall<Request, Response, ExecuteCallback, Executor>(
+    syscall_executor: &mut Executor,
+    vm: &mut VirtualMachine,
+    selector: SyscallSelector,
+    execute_callback: ExecuteCallback,
+) -> HintExecutionResult
+where
+    Executor: SyscallExecutor,
+    Request: SyscallRequest + std::fmt::Debug,
+    Response: SyscallResponse + std::fmt::Debug,
+    ExecuteCallback: FnOnce(
+        Request,
+        &mut VirtualMachine,
+        &mut Executor,
+        &mut u64, // Remaining gas.
+    ) -> SyscallResult<Response>,
+{
+    let syscall_gas_cost = syscall_executor.get_gas_cost_from_selector(&selector).map_err(|e| {
+        HintError::CustomHint(
+            format!("Failed to get gas cost for syscall selector {selector:?}. Error: {e:?}")
+                .into(),
+        )
+    })?;
+
+    let SyscallRequestWrapper { gas_counter, request } =
+        SyscallRequestWrapper::<Request>::read(vm, syscall_executor.get_mut_syscall_ptr())?;
+
+    let syscall_gas_cost =
+        syscall_gas_cost.get_syscall_cost(u64_from_usize(request.get_linear_factor_length()));
+    let syscall_base_cost = syscall_executor.get_syscall_base_gas_cost();
+
+    // Sanity check for preventing underflow.
+    assert!(
+        syscall_gas_cost >= syscall_base_cost,
+        "Syscall gas cost must be greater than base syscall gas cost"
+    );
+
+    // Refund `SYSCALL_BASE_GAS_COST` as it was pre-charged.
+    let required_gas = syscall_gas_cost - syscall_base_cost;
+
+    if gas_counter < required_gas {
+        //  Out of gas failure.
+        let out_of_gas_error =
+            Felt::from_hex(OUT_OF_GAS_ERROR).map_err(SyscallExecutionError::from)?;
+        let response: SyscallResponseWrapper<Response> =
+            SyscallResponseWrapper::Failure { gas_counter, error_data: vec![out_of_gas_error] };
+        response.write(vm, syscall_executor.get_mut_syscall_ptr())?;
+
+        return Ok(());
+    }
+
+    // Execute.
+    let mut remaining_gas = gas_counter - required_gas;
+
+    // To support sierra gas charge for blockifier revert flow, we track the remaining gas left
+    // before executing a syscall if the current tracked resource is gas.
+    // 1. If the syscall does not run Cairo code (i.e. not library call, not call contract, and not
+    //    a deploy), any failure will not run in the OS, so no need to charge - the value before
+    //    entering the callback is good enough to charge.
+    // 2. If the syscall runs Cairo code, but the tracked resource is steps (and not gas), the
+    //    additional charge of reverted cairo steps will cover the inner cost, and the outer cost we
+    //    track here will be the additional reverted gas.
+    // 3. If the syscall runs Cairo code and the tracked resource is gas, either the inner failure
+    //    will be a Cairo1 revert (and the gas consumed on the call info will override the current
+    //    tracked value), or we will pass through another syscall before failing - and by induction
+    //    (we will reach this point again), the gas will be charged correctly.
+    syscall_executor.update_revert_gas_with_next_remaining_gas(GasAmount(remaining_gas));
+
+    let original_response = execute_callback(request, vm, syscall_executor, &mut remaining_gas);
+    let response = match original_response {
+        Ok(response) => SyscallResponseWrapper::Success { gas_counter: remaining_gas, response },
+        Err(SyscallExecutionError::Revert { error_data: data }) => {
+            SyscallResponseWrapper::Failure { gas_counter: remaining_gas, error_data: data }
+        }
+        Err(error) => return Err(error.into()),
+    };
+
+    response.write(vm, syscall_executor.get_mut_syscall_ptr())?;
+
+    Ok(())
+}