refactor(blockifier): share get_block_hash syscall code between native and casm

crates/blockifier/src/execution/native/syscall_handler.rs

@@ -33,7 +33,6 @@ use starknet_api::transaction::fields::{Calldata, ContractAddressSalt};
 use starknet_api::transaction::{EventContent, EventData, EventKey, L2ToL1Payload};
 use starknet_types_core::felt::Felt;
 
-use crate::abi::constants;
 use crate::execution::call_info::{
     CallInfo,
     MessageToL1,
@@ -53,13 +52,12 @@ use crate::execution::errors::EntryPointExecutionError;
 use crate::execution::execution_utils::execute_deployment;
 use crate::execution::native::utils::{calculate_resource_bounds, default_tx_v2_info};
 use crate::execution::secp;
-use crate::execution::syscalls::exceeds_event_size_limit;
 use crate::execution::syscalls::hint_processor::{
     SyscallExecutionError,
-    BLOCK_NUMBER_OUT_OF_RANGE_ERROR,
     INVALID_INPUT_LENGTH_ERROR,
     OUT_OF_GAS_ERROR,
 };
+use crate::execution::syscalls::{exceeds_event_size_limit, syscall_base};
 use crate::state::state_api::State;
 use crate::transaction::objects::TransactionInfo;
 
@@ -276,34 +274,7 @@ impl<'state> StarknetSyscallHandler for &mut NativeSyscallHandler<'state> {
     ) -> SyscallResult<Felt> {
         self.pre_execute_syscall(remaining_gas, self.context.gas_costs().get_block_hash_gas_cost)?;
 
-        if self.context.execution_mode == ExecutionMode::Validate {
-            let err = SyscallExecutionError::InvalidSyscallInExecutionMode {
-                syscall_name: "get_block_hash".to_string(),
-                execution_mode: ExecutionMode::Validate,
-            };
-            return Err(self.handle_error(remaining_gas, err));
-        }
-
-        let current_block_number =
-            self.context.tx_context.block_context.block_info().block_number.0;
-        if current_block_number < constants::STORED_BLOCK_HASH_BUFFER
-            || block_number > current_block_number - constants::STORED_BLOCK_HASH_BUFFER
-        {
-            // `panic` is unreachable in this case, also this is covered by tests so we can safely
-            // unwrap
-            let out_of_range_felt = Felt::from_hex(BLOCK_NUMBER_OUT_OF_RANGE_ERROR)
-                .expect("Converting BLOCK_NUMBER_OUT_OF_RANGE_ERROR to Felt should not fail.");
-            let error = SyscallExecutionError::SyscallError { error_data: vec![out_of_range_felt] };
-            return Err(self.handle_error(remaining_gas, error));
-        }
-
-        let key = StorageKey::try_from(Felt::from(block_number))
-            .map_err(|e| self.handle_error(remaining_gas, e.into()))?;
-        let block_hash_contract_address =
-            ContractAddress::try_from(Felt::from(constants::BLOCK_HASH_CONTRACT_ADDRESS))
-                .map_err(|e| self.handle_error(remaining_gas, e.into()))?;
-
-        match self.state.get_storage_at(block_hash_contract_address, key) {
+        match syscall_base::get_block_hash_base(self.context, block_number, self.state) {
             Ok(value) => Ok(value),
             Err(e) => Err(self.handle_error(remaining_gas, e.into())),
         }

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

@@ -27,9 +27,7 @@ use self::hint_processor::{
     EmitEventError,
     SyscallExecutionError,
     SyscallHintProcessor,
-    BLOCK_NUMBER_OUT_OF_RANGE_ERROR,
 };
-use crate::abi::constants;
 use crate::execution::call_info::{MessageToL1, OrderedEvent, OrderedL2ToL1Message};
 use crate::execution::deprecated_syscalls::DeprecatedSyscallSelector;
 use crate::execution::entry_point::{CallEntryPoint, CallType, ConstructorContext};
@@ -41,16 +39,17 @@ use crate::execution::execution_utils::{
     ReadOnlySegment,
 };
 use crate::execution::syscalls::hint_processor::{INVALID_INPUT_LENGTH_ERROR, OUT_OF_GAS_ERROR};
+use crate::execution::syscalls::syscall_base::SyscallResult;
 use crate::transaction::account_transaction::is_cairo1;
 use crate::versioned_constants::{EventLimits, VersionedConstants};
 
 pub mod hint_processor;
 mod secp;
+pub mod syscall_base;
 
 #[cfg(test)]
 pub mod syscall_tests;
 
-pub type SyscallResult<T> = Result<T, SyscallExecutionError>;
 pub type WriteResponseResult = SyscallResult<()>;
 
 pub type SyscallSelector = DeprecatedSyscallSelector;
@@ -392,30 +391,11 @@ pub fn get_block_hash(
     syscall_handler: &mut SyscallHintProcessor<'_>,
     _remaining_gas: &mut u64,
 ) -> SyscallResult<GetBlockHashResponse> {
-    if syscall_handler.is_validate_mode() {
-        return Err(SyscallExecutionError::InvalidSyscallInExecutionMode {
-            syscall_name: "get_block_hash".to_string(),
-            execution_mode: syscall_handler.execution_mode(),
-        });
-    }
-
-    let requested_block_number = request.block_number.0;
-    let current_block_number =
-        syscall_handler.context.tx_context.block_context.block_info.block_number.0;
-
-    if current_block_number < constants::STORED_BLOCK_HASH_BUFFER
-        || requested_block_number > current_block_number - constants::STORED_BLOCK_HASH_BUFFER
-    {
-        let out_of_range_error =
-            Felt::from_hex(BLOCK_NUMBER_OUT_OF_RANGE_ERROR).map_err(SyscallExecutionError::from)?;
-        return Err(SyscallExecutionError::SyscallError { error_data: vec![out_of_range_error] });
-    }
-
-    let key = StorageKey::try_from(Felt::from(requested_block_number))?;
-    let block_hash_contract_address =
-        ContractAddress::try_from(Felt::from(constants::BLOCK_HASH_CONTRACT_ADDRESS))?;
-    let block_hash =
-        BlockHash(syscall_handler.state.get_storage_at(block_hash_contract_address, key)?);
+    let block_hash = BlockHash(syscall_base::get_block_hash_base(
+        syscall_handler.context,
+        request.block_number.0,
+        syscall_handler.state,
+    )?);
     Ok(GetBlockHashResponse { block_hash })
 }
 

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

@@ -0,0 +1,44 @@
+/// This file is for sharing common logic between Native and Casm syscalls implementations.
+use starknet_api::core::ContractAddress;
+use starknet_api::state::StorageKey;
+use starknet_types_core::felt::Felt;
+
+use crate::abi::constants;
+use crate::execution::common_hints::ExecutionMode;
+use crate::execution::entry_point::EntryPointExecutionContext;
+use crate::execution::syscalls::hint_processor::{
+    SyscallExecutionError,
+    BLOCK_NUMBER_OUT_OF_RANGE_ERROR,
+};
+use crate::state::state_api::State;
+
+pub type SyscallResult<T> = Result<T, SyscallExecutionError>;
+
+pub fn get_block_hash_base(
+    context: &EntryPointExecutionContext,
+    requested_block_number: u64,
+    state: &dyn State,
+) -> SyscallResult<Felt> {
+    let execution_mode = context.execution_mode;
+    if execution_mode == ExecutionMode::Validate {
+        return Err(SyscallExecutionError::InvalidSyscallInExecutionMode {
+            syscall_name: "get_block_hash".to_string(),
+            execution_mode,
+        });
+    }
+
+    let current_block_number = context.tx_context.block_context.block_info.block_number.0;
+
+    if current_block_number < constants::STORED_BLOCK_HASH_BUFFER
+        || requested_block_number > current_block_number - constants::STORED_BLOCK_HASH_BUFFER
+    {
+        let out_of_range_error = Felt::from_hex(BLOCK_NUMBER_OUT_OF_RANGE_ERROR)
+            .expect("Converting BLOCK_NUMBER_OUT_OF_RANGE_ERROR to Felt should not fail.");
+        return Err(SyscallExecutionError::SyscallError { error_data: vec![out_of_range_error] });
+    }
+
+    let key = StorageKey::try_from(Felt::from(requested_block_number))?;
+    let block_hash_contract_address =
+        ContractAddress::try_from(Felt::from(constants::BLOCK_HASH_CONTRACT_ADDRESS))?;
+    Ok(state.get_storage_at(block_hash_contract_address, key)?)
+}

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

@@ -117,11 +117,10 @@ fn negative_flow_block_number_out_of_range(test_contract: FeatureContract) {
         ..trivial_external_entry_point_new(test_contract)
     };
 
-    let call_result = entry_point_call.execute_directly(&mut state);
-    let (actual_error_msg, expected_error_msg) = (
-        format_panic_data(&call_result.unwrap().execution.retdata.0),
-        "0x426c6f636b206e756d626572206f7574206f662072616e6765 ('Block number out of range')",
+    let call_info = entry_point_call.execute_directly(&mut state).unwrap();
+    assert!(call_info.execution.failed);
+    assert_eq!(
+        format_panic_data(&call_info.execution.retdata.0),
+        "0x426c6f636b206e756d626572206f7574206f662072616e6765 ('Block number out of range')"
     );
-
-    assert_eq!(actual_error_msg, expected_error_msg);
 }