From ef8f923e4340462750f5daafbdf93b4fe037d6ab Mon Sep 17 00:00:00 2001 From: Meshi Peled <141231558+meship-starkware@users.noreply.github.com> Date: Mon, 18 Nov 2024 13:54:51 +0200 Subject: [PATCH] refactor(blockifier): share get_block_hash syscall code between native and casm (#2107) --- .../src/execution/native/syscall_handler.rs | 33 +------------- .../blockifier/src/execution/syscalls/mod.rs | 34 +++----------- .../src/execution/syscalls/syscall_base.rs | 44 +++++++++++++++++++ .../syscalls/syscall_tests/get_block_hash.rs | 11 +++-- 4 files changed, 58 insertions(+), 64 deletions(-) create mode 100644 crates/blockifier/src/execution/syscalls/syscall_base.rs diff --git a/crates/blockifier/src/execution/native/syscall_handler.rs b/crates/blockifier/src/execution/native/syscall_handler.rs index 8b3d4c5b9a..40a3e83b27 100644 --- a/crates/blockifier/src/execution/native/syscall_handler.rs +++ b/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 { 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())), } diff --git a/crates/blockifier/src/execution/syscalls/mod.rs b/crates/blockifier/src/execution/syscalls/mod.rs index 1f84bfa4f7..64c65f7d84 100644 --- a/crates/blockifier/src/execution/syscalls/mod.rs +++ b/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 = Result; 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 { - 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 }) } diff --git a/crates/blockifier/src/execution/syscalls/syscall_base.rs b/crates/blockifier/src/execution/syscalls/syscall_base.rs new file mode 100644 index 0000000000..15ce73bbad --- /dev/null +++ b/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 = Result; + +pub fn get_block_hash_base( + context: &EntryPointExecutionContext, + requested_block_number: u64, + state: &dyn State, +) -> SyscallResult { + 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)?) +} diff --git a/crates/blockifier/src/execution/syscalls/syscall_tests/get_block_hash.rs b/crates/blockifier/src/execution/syscalls/syscall_tests/get_block_hash.rs index 12af5bcbec..360b124be1 100644 --- a/crates/blockifier/src/execution/syscalls/syscall_tests/get_block_hash.rs +++ b/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); }