refactor(execution)!: pre_process_block refactor for full_nodes simul…

…ate transaction (#1387)

crates/blockifier/src/block.rs

@@ -6,8 +6,11 @@ use starknet_api::hash::StarkFelt;
 use starknet_api::state::StorageKey;
 
 use crate::abi::constants;
+use crate::context::{BlockContext, ChainInfo};
+use crate::state::errors::StateError;
 use crate::state::state_api::{State, StateResult};
 use crate::transaction::objects::FeeType;
+use crate::versioned_constants::VersionedConstants;
 
 #[cfg(test)]
 #[path = "block_test.rs"]
@@ -51,23 +54,33 @@ impl GasPrices {
 // Block pre-processing.
 // Writes the hash of the (current_block_number - N) block under its block number in the dedicated
 // contract state, where N=STORED_BLOCK_HASH_BUFFER.
+// NOTE: This function must remain idempotent since full nodes can call it for an already updated
+// block hash table.
 pub fn pre_process_block(
     state: &mut dyn State,
     old_block_number_and_hash: Option<BlockNumberHashPair>,
-) -> StateResult<()> {
+    block_info: BlockInfo,
+    chain_info: ChainInfo,
+    versioned_constants: VersionedConstants,
+) -> StateResult<BlockContext> {
+    let should_block_hash_be_provided =
+        block_info.block_number >= BlockNumber(constants::STORED_BLOCK_HASH_BUFFER);
     if let Some(BlockNumberHashPair { number: block_number, hash: block_hash }) =
         old_block_number_and_hash
     {
+        let block_hash_contract_address =
+            ContractAddress::from(constants::BLOCK_HASH_CONTRACT_ADDRESS);
+        let block_number_as_storage_key = StorageKey::from(block_number.0);
         state.set_storage_at(
-            ContractAddress::try_from(StarkFelt::from(constants::BLOCK_HASH_CONTRACT_ADDRESS))
-                .expect("Failed to convert `BLOCK_HASH_CONTRACT_ADDRESS` to ContractAddress."),
-            StorageKey::try_from(StarkFelt::from(block_number.0))
-                .expect("Failed to convert BlockNumber to StorageKey."),
+            block_hash_contract_address,
+            block_number_as_storage_key,
             block_hash.0,
         )?;
+    } else if should_block_hash_be_provided {
+        return Err(StateError::OldBlockHashNotProvided);
     }
 
-    Ok(())
+    Ok(BlockContext { block_info, chain_info, versioned_constants })
 }
 
 pub struct BlockNumberHashPair {

crates/blockifier/src/block_test.rs

@@ -1,24 +1,68 @@
+use starknet_api::block::BlockNumber;
 use starknet_api::core::ContractAddress;
 use starknet_api::hash::StarkFelt;
 use starknet_api::state::StorageKey;
 
 use crate::abi::constants;
-use crate::block::{pre_process_block, BlockNumberHashPair};
+use crate::block::{pre_process_block, BlockInfo, BlockNumberHashPair};
+use crate::context::ChainInfo;
 use crate::state::state_api::StateReader;
 use crate::test_utils::cached_state::create_test_state;
+use crate::versioned_constants::VersionedConstants;
 
 #[test]
 fn test_pre_process_block() {
     let mut state = create_test_state();
 
-    let block_number: u64 = 10;
+    // Test the positive flow of pre_process_block inside the allowed block number interval
+    let block_number = constants::STORED_BLOCK_HASH_BUFFER;
     let block_hash = StarkFelt::from(20_u8);
-    pre_process_block(&mut state, Some(BlockNumberHashPair::new(block_number, block_hash)))
-        .unwrap();
+    let mut block_info = BlockInfo::create_for_testing();
+    block_info.block_number = BlockNumber(block_number);
+    pre_process_block(
+        &mut state,
+        Some(BlockNumberHashPair::new(block_number, block_hash)),
+        block_info,
+        ChainInfo::default(),
+        VersionedConstants::default(),
+    )
+    .unwrap();
 
     let written_hash = state.get_storage_at(
-        ContractAddress::try_from(StarkFelt::from(constants::BLOCK_HASH_CONTRACT_ADDRESS)).unwrap(),
-        StorageKey::try_from(StarkFelt::from(block_number)).unwrap(),
+        ContractAddress::from(constants::BLOCK_HASH_CONTRACT_ADDRESS),
+        StorageKey::from(block_number),
     );
     assert_eq!(written_hash.unwrap(), block_hash);
+
+    // Test that block pre-process with block hash None is successful only within the allowed
+    // block number interval.
+    let mut block_info = BlockInfo::create_for_testing();
+    block_info.block_number = BlockNumber(constants::STORED_BLOCK_HASH_BUFFER - 1);
+    assert!(
+        pre_process_block(
+            &mut state,
+            None,
+            block_info,
+            ChainInfo::default(),
+            VersionedConstants::default()
+        )
+        .is_ok()
+    );
+
+    let mut block_info = BlockInfo::create_for_testing();
+    block_info.block_number = BlockNumber(constants::STORED_BLOCK_HASH_BUFFER);
+    let error = pre_process_block(
+        &mut state,
+        None,
+        block_info,
+        ChainInfo::default(),
+        VersionedConstants::default(),
+    );
+    assert_eq!(
+        format!(
+            "A block hash must be provided for block number > {}.",
+            constants::STORED_BLOCK_HASH_BUFFER
+        ),
+        format!("{}", error.unwrap_err())
+    );
 }

crates/blockifier/src/context.rs

@@ -70,6 +70,15 @@ impl ChainInfo {
     }
 }
 
+impl Default for ChainInfo {
+    fn default() -> Self {
+        ChainInfo {
+            chain_id: ChainId("0x0".to_string()),
+            fee_token_addresses: FeeTokenAddresses::default(),
+        }
+    }
+}
+
 #[derive(Clone, Debug, Default)]
 pub struct FeeTokenAddresses {
     pub strk_fee_token_address: ContractAddress,

crates/blockifier/src/state/errors.rs

@@ -3,8 +3,15 @@ use starknet_api::core::{ClassHash, ContractAddress};
 use starknet_api::StarknetApiError;
 use thiserror::Error;
 
+use crate::abi::constants;
+
 #[derive(Debug, Error)]
 pub enum StateError {
+    #[error(
+        "A block hash must be provided for block number > {}.",
+        constants::STORED_BLOCK_HASH_BUFFER
+    )]
+    OldBlockHashNotProvided,
     #[error("Cannot deploy contract at address 0.")]
     OutOfRangeContractAddress,
     #[error(transparent)]

crates/native_blockifier/src/py_block_executor.rs

@@ -1,9 +1,14 @@
 use std::collections::HashMap;
 use std::sync::Arc;
 
-use blockifier::block::{BlockInfo, GasPrices};
+use blockifier::block::{
+    pre_process_block as pre_process_block_blockifier, BlockInfo, BlockNumberHashPair, GasPrices,
+};
 use blockifier::context::{BlockContext, ChainInfo, FeeTokenAddresses};
-use blockifier::state::cached_state::{GlobalContractCache, GLOBAL_CONTRACT_CACHE_SIZE_FOR_TEST};
+use blockifier::state::cached_state::{
+    CachedState, GlobalContractCache, GLOBAL_CONTRACT_CACHE_SIZE_FOR_TEST,
+};
+use blockifier::state::state_api::State;
 use blockifier::versioned_constants::VersionedConstants;
 use pyo3::prelude::*;
 use starknet_api::block::{BlockNumber, BlockTimestamp};
@@ -65,20 +70,24 @@ impl PyBlockExecutor {
     // Transaction Execution API.
 
     /// Initializes the transaction executor for the given block.
-    #[pyo3(signature = (next_block_info))]
+    #[pyo3(signature = (next_block_info, old_block_number_and_hash))]
     fn setup_block_execution(
         &mut self,
         next_block_info: PyBlockInfo,
+        old_block_number_and_hash: Option<(u64, PyFelt)>,
     ) -> NativeBlockifierResult<()> {
         let papyrus_reader = self.get_aligned_reader(next_block_info.block_number);
-
-        let tx_executor = TransactionExecutor::new(
-            papyrus_reader,
+        let global_contract_cache = self.global_contract_cache.clone();
+        let mut state = CachedState::new(papyrus_reader, global_contract_cache);
+        let block_context = pre_process_block(
+            &mut state,
+            old_block_number_and_hash,
             &self.general_config,
+            &next_block_info,
             &self.versioned_constants,
-            next_block_info,
-            self.global_contract_cache.clone(),
         )?;
+
+        let tx_executor = TransactionExecutor::new(state, block_context)?;
         self.tx_executor = Some(tx_executor);
 
         Ok(())
@@ -108,14 +117,6 @@ impl PyBlockExecutor {
         finalized_state
     }
 
-    #[pyo3(signature = (old_block_number_and_hash))]
-    pub fn pre_process_block(
-        &mut self,
-        old_block_number_and_hash: Option<(u64, PyFelt)>,
-    ) -> NativeBlockifierResult<()> {
-        self.tx_executor().pre_process_block(old_block_number_and_hash)
-    }
-
     pub fn commit_tx(&mut self) {
         self.tx_executor().commit()
     }
@@ -301,11 +302,10 @@ impl Default for PyOsConfig {
     }
 }
 
-pub fn into_block_context(
+pub fn into_block_context_args(
     general_config: &PyGeneralConfig,
-    versioned_constants: &VersionedConstants,
-    block_info: PyBlockInfo,
-) -> NativeBlockifierResult<BlockContext> {
+    block_info: &PyBlockInfo,
+) -> NativeBlockifierResult<(BlockInfo, ChainInfo)> {
     let chain_info: ChainInfo = general_config.starknet_os_config.clone().try_into()?;
     let block_info = BlockInfo {
         block_number: BlockNumber(block_info.block_number),
@@ -348,5 +348,28 @@ pub fn into_block_context(
         use_kzg_da: block_info.use_kzg_da,
     };
 
-    Ok(BlockContext::new_unchecked(&block_info, &chain_info, versioned_constants))
+    Ok((block_info, chain_info))
+}
+
+// Executes block pre-processing; see `block_execution::pre_process_block` documentation.
+fn pre_process_block(
+    state: &mut dyn State,
+    old_block_number_and_hash: Option<(u64, PyFelt)>,
+    general_config: &PyGeneralConfig,
+    block_info: &PyBlockInfo,
+    versioned_constants: &VersionedConstants,
+) -> NativeBlockifierResult<BlockContext> {
+    let old_block_number_and_hash = old_block_number_and_hash
+        .map(|(block_number, block_hash)| BlockNumberHashPair::new(block_number, block_hash.0));
+
+    let (block_info, chain_info) = into_block_context_args(general_config, block_info)?;
+    let block_context = pre_process_block_blockifier(
+        state,
+        old_block_number_and_hash,
+        block_info,
+        chain_info,
+        versioned_constants.clone(),
+    )?;
+
+    Ok(block_context)
 }

crates/native_blockifier/src/py_block_executor_test.rs

@@ -19,7 +19,10 @@ fn global_contract_cache_update() {
     let temp_storage_path = tempfile::tempdir().unwrap().into_path();
     let mut block_executor =
         PyBlockExecutor::create_for_testing(PyGeneralConfig::default(), temp_storage_path);
-    block_executor.setup_block_execution(PyBlockInfo::default()).unwrap();
+    let sentinel_block_number_and_hash = None; // Information does not exist for block 0.
+    block_executor
+        .setup_block_execution(PyBlockInfo::default(), sentinel_block_number_and_hash)
+        .unwrap();
 
     let class_hash = class_hash!(TEST_CLASS_HASH);
     let contract_class = get_test_contract_class();
@@ -36,7 +39,9 @@ fn global_contract_cache_update() {
     block_executor.teardown_block_execution();
 
     // Finalizing a non-pending block does update the global cache.
-    block_executor.setup_block_execution(PyBlockInfo::default()).unwrap();
+    block_executor
+        .setup_block_execution(PyBlockInfo::default(), sentinel_block_number_and_hash)
+        .unwrap();
     block_executor.tx_executor().state.set_contract_class(class_hash, contract_class).unwrap();
     let is_pending_block = false;
     block_executor.finalize(is_pending_block);

crates/native_blockifier/src/py_validator.rs

@@ -1,8 +1,10 @@
-use blockifier::context::TransactionContext;
+use blockifier::context::{BlockContext, TransactionContext};
 use blockifier::execution::call_info::CallInfo;
 use blockifier::fee::actual_cost::ActualCost;
 use blockifier::fee::fee_checks::PostValidationReport;
-use blockifier::state::cached_state::{GlobalContractCache, GLOBAL_CONTRACT_CACHE_SIZE_FOR_TEST};
+use blockifier::state::cached_state::{
+    CachedState, GlobalContractCache, GLOBAL_CONTRACT_CACHE_SIZE_FOR_TEST,
+};
 use blockifier::state::state_api::StateReader;
 use blockifier::transaction::account_transaction::AccountTransaction;
 use blockifier::transaction::objects::{TransactionExecutionResult, TransactionInfo};
@@ -13,7 +15,7 @@ use starknet_api::core::Nonce;
 use starknet_api::hash::StarkFelt;
 
 use crate::errors::NativeBlockifierResult;
-use crate::py_block_executor::PyGeneralConfig;
+use crate::py_block_executor::{into_block_context_args, PyGeneralConfig};
 use crate::py_state_diff::PyBlockInfo;
 use crate::py_transaction::py_account_tx;
 use crate::py_transaction_execution_info::PyBouncerInfo;
@@ -43,14 +45,16 @@ impl PyValidator {
     ) -> NativeBlockifierResult<Self> {
         let versioned_constants =
             versioned_constants_with_overrides(validate_max_n_steps, max_recursion_depth);
+        let global_contract_cache = GlobalContractCache::new(global_contract_cache_size);
+        let state_reader = PyStateReader::new(state_reader_proxy);
+        let state = CachedState::new(state_reader, global_contract_cache);
+
+        let (block_info, chain_info) = into_block_context_args(&general_config, &next_block_info)?;
+        // TODO(Yael 24/01/24): calc block_context using pre_process_block
+        let block_context =
+            BlockContext::new_unchecked(&block_info, &chain_info, &versioned_constants);
+        let tx_executor = TransactionExecutor::new(state, block_context)?;
 
-        let tx_executor = TransactionExecutor::new(
-            PyStateReader::new(state_reader_proxy),
-            &general_config,
-            &versioned_constants,
-            next_block_info,
-            GlobalContractCache::new(global_contract_cache_size),
-        )?;
         let validator = Self {
             max_nonce_for_validation_skip: Nonce(max_nonce_for_validation_skip.0),
             tx_executor,
@@ -113,13 +117,19 @@ impl PyValidator {
         state_reader_proxy: &PyAny,
         next_block_info: PyBlockInfo,
     ) -> NativeBlockifierResult<Self> {
-        let tx_executor = TransactionExecutor::new(
-            PyStateReader::new(state_reader_proxy),
-            &general_config,
-            &VersionedConstants::latest_constants().clone(),
-            next_block_info,
-            GlobalContractCache::new(GLOBAL_CONTRACT_CACHE_SIZE_FOR_TEST),
-        )?;
+        let state_reader = PyStateReader::new(state_reader_proxy);
+        let global_contract_cache = GlobalContractCache::new(GLOBAL_CONTRACT_CACHE_SIZE_FOR_TEST);
+        let state = CachedState::new(state_reader, global_contract_cache);
+
+        let (block_info, chain_info) = into_block_context_args(&general_config, &next_block_info)?;
+        let block_context = BlockContext::new_unchecked(
+            &block_info,
+            &chain_info,
+            VersionedConstants::latest_constants(),
+        );
+        // TODO(Yael 24/01/24): calc block_context using pre_process_block
+        let tx_executor = TransactionExecutor::new(state, block_context)?;
+
         Ok(Self { max_nonce_for_validation_skip: Nonce(StarkFelt::ONE), tx_executor })
     }