From b02f45bc67e2fd1f956ce099c75f545f1c1eb4b5 Mon Sep 17 00:00:00 2001 From: Yael Doweck Date: Mon, 4 Nov 2024 11:50:35 +0200 Subject: [PATCH] chore(batcher): move papyrus_state_reader to a shared crate --- Cargo.lock | 13 ++ Cargo.toml | 2 + crates/batcher/Cargo.toml | 3 +- crates/batcher/src/block_builder.rs | 2 +- crates/batcher/src/lib.rs | 1 - crates/batcher/src/papyrus_state.rs | 146 ------------------ crates/native_blockifier/Cargo.toml | 1 + .../src/py_block_executor.rs | 2 +- .../src/py_block_executor_test.rs | 48 ++++++ .../large_compiled_contract.json | 0 crates/native_blockifier/src/state_readers.rs | 1 - crates/papyrus_state_reader/Cargo.toml | 18 +++ crates/papyrus_state_reader/src/lib.rs | 1 + .../src}/papyrus_state.rs | 1 - .../src}/papyrus_state_test.rs | 56 +------ 15 files changed, 88 insertions(+), 207 deletions(-) delete mode 100644 crates/batcher/src/papyrus_state.rs rename crates/native_blockifier/src/{state_readers => resources}/large_compiled_contract.json (100%) create mode 100644 crates/papyrus_state_reader/Cargo.toml create mode 100644 crates/papyrus_state_reader/src/lib.rs rename crates/{native_blockifier/src/state_readers => papyrus_state_reader/src}/papyrus_state.rs (99%) rename crates/{native_blockifier/src/state_readers => papyrus_state_reader/src}/papyrus_state_test.rs (60%) diff --git a/Cargo.lock b/Cargo.lock index bf750e8141..cdf85f605d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6642,6 +6642,7 @@ dependencies = [ "indexmap 2.6.0", "log", "num-bigint 0.4.6", + "papyrus_state_reader", "papyrus_storage", "pretty_assertions", "pyo3", @@ -7557,6 +7558,17 @@ dependencies = [ "validator", ] +[[package]] +name = "papyrus_state_reader" +version = "0.0.0" +dependencies = [ + "blockifier", + "indexmap 2.6.0", + "papyrus_storage", + "starknet-types-core", + "starknet_api", +] + [[package]] name = "papyrus_storage" version = "0.0.0" @@ -10093,6 +10105,7 @@ dependencies = [ "mempool_test_utils", "mockall", "papyrus_config", + "papyrus_state_reader", "papyrus_storage", "rstest", "serde", diff --git a/Cargo.toml b/Cargo.toml index 95558bc780..fc37ae2d88 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,6 +34,7 @@ members = [ "crates/papyrus_proc_macros", "crates/papyrus_protobuf", "crates/papyrus_rpc", + "crates/papyrus_state_reader", "crates/papyrus_storage", "crates/papyrus_sync", "crates/papyrus_test_utils", @@ -166,6 +167,7 @@ papyrus_p2p_sync = { path = "crates/papyrus_p2p_sync", version = "0.0.0" } papyrus_proc_macros = { path = "crates/papyrus_proc_macros", version = "0.0.0" } papyrus_protobuf = { path = "crates/papyrus_protobuf", version = "0.0.0" } papyrus_rpc = { path = "crates/papyrus_rpc", version = "0.0.0" } +papyrus_state_reader = { path = "crates/papyrus_state_reader", version = "0.0.0" } papyrus_storage = { path = "crates/papyrus_storage", version = "0.0.0" } papyrus_sync = { path = "crates/papyrus_sync", version = "0.0.0" } papyrus_test_utils = { path = "crates/papyrus_test_utils", version = "0.0.0" } diff --git a/crates/batcher/Cargo.toml b/crates/batcher/Cargo.toml index 0ad59fe65a..49be0836c0 100644 --- a/crates/batcher/Cargo.toml +++ b/crates/batcher/Cargo.toml @@ -14,9 +14,9 @@ blockifier.workspace = true chrono.workspace = true indexmap.workspace = true papyrus_config.workspace = true +papyrus_state_reader.workspace = true papyrus_storage.workspace = true serde.workspace = true -starknet-types-core.workspace = true starknet_api.workspace = true starknet_batcher_types.workspace = true starknet_mempool_types.workspace = true @@ -33,4 +33,5 @@ futures.workspace = true mempool_test_utils.workspace = true mockall.workspace = true rstest.workspace = true +starknet-types-core.workspace = true starknet_api = { workspace = true, features = ["testing"] } diff --git a/crates/batcher/src/block_builder.rs b/crates/batcher/src/block_builder.rs index e7463d8b7d..ae46b43f57 100644 --- a/crates/batcher/src/block_builder.rs +++ b/crates/batcher/src/block_builder.rs @@ -28,6 +28,7 @@ use papyrus_config::dumping::{ SerializeConfig, }; use papyrus_config::{ParamPath, ParamPrivacyInput, SerializedParam}; +use papyrus_state_reader::papyrus_state::PapyrusReader; use papyrus_storage::StorageReader; use serde::{Deserialize, Serialize}; use starknet_api::block::{BlockHashAndNumber, BlockNumber, BlockTimestamp, NonzeroGasPrice}; @@ -38,7 +39,6 @@ use thiserror::Error; use tokio::sync::Mutex; use tracing::{debug, error, info, trace}; -use crate::papyrus_state::PapyrusReader; use crate::transaction_executor::TransactionExecutorTrait; use crate::transaction_provider::{NextTxs, TransactionProvider, TransactionProviderError}; diff --git a/crates/batcher/src/lib.rs b/crates/batcher/src/lib.rs index c9d4fea20c..e3df65d7f0 100644 --- a/crates/batcher/src/lib.rs +++ b/crates/batcher/src/lib.rs @@ -7,7 +7,6 @@ mod block_builder_test; pub mod communication; pub mod config; pub mod fee_market; -pub mod papyrus_state; mod proposal_manager; #[cfg(test)] mod proposal_manager_test; diff --git a/crates/batcher/src/papyrus_state.rs b/crates/batcher/src/papyrus_state.rs deleted file mode 100644 index 2ee1295f74..0000000000 --- a/crates/batcher/src/papyrus_state.rs +++ /dev/null @@ -1,146 +0,0 @@ -// TODO(yael 22/9/2024): This module is copied from native_blockifier, need to how to share it -// between the crates. -use blockifier::execution::contract_class::{ - ContractClassV0, - ContractClassV1, - RunnableContractClass, -}; -use blockifier::state::errors::StateError; -use blockifier::state::global_cache::GlobalContractCache; -use blockifier::state::state_api::{StateReader, StateResult}; -use papyrus_storage::compiled_class::CasmStorageReader; -use papyrus_storage::db::RO; -use papyrus_storage::state::StateStorageReader; -use papyrus_storage::StorageReader; -use starknet_api::block::BlockNumber; -use starknet_api::core::{ClassHash, CompiledClassHash, ContractAddress, Nonce}; -use starknet_api::state::{StateNumber, StorageKey}; -use starknet_types_core::felt::Felt; - -type RawPapyrusReader<'env> = papyrus_storage::StorageTxn<'env, RO>; -pub struct PapyrusReader { - storage_reader: StorageReader, - latest_block: BlockNumber, - global_class_hash_to_class: GlobalContractCache, -} - -impl PapyrusReader { - pub fn new( - storage_reader: StorageReader, - latest_block: BlockNumber, - global_class_hash_to_class: GlobalContractCache, - ) -> Self { - Self { storage_reader, latest_block, global_class_hash_to_class } - } - - fn reader(&self) -> StateResult> { - self.storage_reader - .begin_ro_txn() - .map_err(|error| StateError::StateReadError(error.to_string())) - } - - /// Returns a V1 contract if found, or a V0 contract if a V1 contract is not - /// found, or an `Error` otherwise. - fn get_compiled_contract_class_inner( - &self, - class_hash: ClassHash, - ) -> StateResult { - let state_number = StateNumber(self.latest_block); - let class_declaration_block_number = self - .reader()? - .get_state_reader() - .and_then(|sr| sr.get_class_definition_block_number(&class_hash)) - .map_err(|err| StateError::StateReadError(err.to_string()))?; - let class_is_declared: bool = matches!(class_declaration_block_number, - Some(block_number) if block_number <= state_number.0); - - if class_is_declared { - let casm_contract_class = self - .reader()? - .get_casm(&class_hash) - .map_err(|err| StateError::StateReadError(err.to_string()))? - .expect( - "Should be able to fetch a Casm class if its definition exists, database is \ - inconsistent.", - ); - - return Ok(RunnableContractClass::V1(ContractClassV1::try_from(casm_contract_class)?)); - } - - let v0_contract_class = self - .reader()? - .get_state_reader() - .and_then(|sr| sr.get_deprecated_class_definition_at(state_number, &class_hash)) - .map_err(|err| StateError::StateReadError(err.to_string()))?; - - match v0_contract_class { - Some(starknet_api_contract_class) => { - Ok(ContractClassV0::try_from(starknet_api_contract_class)?.into()) - } - None => Err(StateError::UndeclaredClassHash(class_hash)), - } - } -} - -// Currently unused - will soon replace the same `impl` for `PapyrusStateReader`. -impl StateReader for PapyrusReader { - fn get_storage_at( - &self, - contract_address: ContractAddress, - key: StorageKey, - ) -> StateResult { - let state_number = StateNumber(self.latest_block); - self.reader()? - .get_state_reader() - .and_then(|sr| sr.get_storage_at(state_number, &contract_address, &key)) - .map_err(|error| StateError::StateReadError(error.to_string())) - } - - fn get_nonce_at(&self, contract_address: ContractAddress) -> StateResult { - let state_number = StateNumber(self.latest_block); - match self - .reader()? - .get_state_reader() - .and_then(|sr| sr.get_nonce_at(state_number, &contract_address)) - { - Ok(Some(nonce)) => Ok(nonce), - Ok(None) => Ok(Nonce::default()), - Err(err) => Err(StateError::StateReadError(err.to_string())), - } - } - - fn get_class_hash_at(&self, contract_address: ContractAddress) -> StateResult { - let state_number = StateNumber(self.latest_block); - match self - .reader()? - .get_state_reader() - .and_then(|sr| sr.get_class_hash_at(state_number, &contract_address)) - { - Ok(Some(class_hash)) => Ok(class_hash), - Ok(None) => Ok(ClassHash::default()), - Err(err) => Err(StateError::StateReadError(err.to_string())), - } - } - - fn get_compiled_contract_class( - &self, - class_hash: ClassHash, - ) -> StateResult { - // Assumption: the global cache is cleared upon reverted blocks. - let contract_class = self.global_class_hash_to_class.get(&class_hash); - - match contract_class { - Some(contract_class) => Ok(contract_class), - None => { - let contract_class_from_db = self.get_compiled_contract_class_inner(class_hash)?; - // The class was declared in a previous (finalized) state; update the global cache. - self.global_class_hash_to_class.set(class_hash, contract_class_from_db.clone()); - Ok(contract_class_from_db) - } - } - } - - fn get_compiled_class_hash(&self, _class_hash: ClassHash) -> StateResult { - todo!() - } -} diff --git a/crates/native_blockifier/Cargo.toml b/crates/native_blockifier/Cargo.toml index 63b101f268..4c0ba4ec5d 100644 --- a/crates/native_blockifier/Cargo.toml +++ b/crates/native_blockifier/Cargo.toml @@ -33,6 +33,7 @@ cairo-vm.workspace = true indexmap.workspace = true log.workspace = true num-bigint.workspace = true +papyrus_state_reader.workspace = true papyrus_storage = { workspace = true, features = ["testing"] } pyo3 = { workspace = true, features = ["hashbrown", "num-bigint"] } pyo3-log.workspace = true diff --git a/crates/native_blockifier/src/py_block_executor.rs b/crates/native_blockifier/src/py_block_executor.rs index 7bf28040a1..a7d7f0e48b 100644 --- a/crates/native_blockifier/src/py_block_executor.rs +++ b/crates/native_blockifier/src/py_block_executor.rs @@ -12,6 +12,7 @@ use blockifier::transaction::objects::{ExecutionResourcesTraits, TransactionExec use blockifier::transaction::transaction_execution::Transaction; use blockifier::utils::usize_from_u64; use blockifier::versioned_constants::VersionedConstants; +use papyrus_state_reader::papyrus_state::PapyrusReader; use pyo3::prelude::*; use pyo3::types::{PyBytes, PyList}; use pyo3::{FromPyObject, PyAny, Python}; @@ -27,7 +28,6 @@ use crate::py_objects::{PyBouncerConfig, PyConcurrencyConfig, PyVersionedConstan use crate::py_state_diff::{PyBlockInfo, PyStateDiff}; use crate::py_transaction::{py_tx, PyClassInfo, PY_TX_PARSING_ERR}; use crate::py_utils::{int_to_chain_id, into_block_number_hash_pair, PyFelt}; -use crate::state_readers::papyrus_state::PapyrusReader; use crate::storage::{PapyrusStorage, Storage, StorageConfig}; pub(crate) type RawTransactionExecutionResult = Vec; diff --git a/crates/native_blockifier/src/py_block_executor_test.rs b/crates/native_blockifier/src/py_block_executor_test.rs index ab3fbdd660..826d285396 100644 --- a/crates/native_blockifier/src/py_block_executor_test.rs +++ b/crates/native_blockifier/src/py_block_executor_test.rs @@ -7,6 +7,7 @@ use cached::Cached; use cairo_lang_starknet_classes::casm_contract_class::CasmContractClass; use pretty_assertions::assert_eq; use starknet_api::class_hash; +use starknet_api::deprecated_contract_class::ContractClass as DeprecatedContractClass; use starknet_types_core::felt::Felt; use crate::py_block_executor::{PyBlockExecutor, PyOsConfig}; @@ -15,6 +16,8 @@ use crate::py_state_diff::{PyBlockInfo, PyStateDiff}; use crate::py_utils::PyFelt; use crate::test_utils::MockStorage; +const LARGE_COMPILED_CONTRACT_JSON: &str = include_str!("resources/large_compiled_contract.json"); + #[test] fn global_contract_cache_update() { // Initialize executor and set a contract class on the state. @@ -92,3 +95,48 @@ fn get_block_id() { expected_max_class_hash_as_py_felt ); } + +#[test] +/// Edge case: adding a large contract to the global contract cache. +fn global_contract_cache_update_large_contract() { + let mut raw_contract_class: serde_json::Value = + serde_json::from_str(LARGE_COMPILED_CONTRACT_JSON).unwrap(); + + // ABI is not required for execution. + raw_contract_class + .as_object_mut() + .expect("A compiled contract must be a JSON object.") + .remove("abi"); + + let dep_casm: DeprecatedContractClass = serde_json::from_value(raw_contract_class) + .expect("DeprecatedContractClass is not supported for this contract."); + + let temp_storage_path = tempfile::tempdir().unwrap().into_path(); + let mut block_executor = PyBlockExecutor::native_create_for_testing( + Default::default(), + Default::default(), + temp_storage_path, + 4000, + ); + block_executor + .append_block( + 0, + None, + Default::default(), + Default::default(), + Default::default(), + HashMap::from([(PyFelt::from(1_u8), serde_json::to_string(&dep_casm).unwrap())]), + ) + .unwrap(); + + block_executor + .append_block( + 1, + Some(PyFelt(Felt::ZERO)), + PyBlockInfo { block_number: 1, ..PyBlockInfo::default() }, + Default::default(), + Default::default(), + HashMap::from([(PyFelt::from(1_u8), serde_json::to_string(&dep_casm).unwrap())]), + ) + .unwrap(); +} diff --git a/crates/native_blockifier/src/state_readers/large_compiled_contract.json b/crates/native_blockifier/src/resources/large_compiled_contract.json similarity index 100% rename from crates/native_blockifier/src/state_readers/large_compiled_contract.json rename to crates/native_blockifier/src/resources/large_compiled_contract.json diff --git a/crates/native_blockifier/src/state_readers.rs b/crates/native_blockifier/src/state_readers.rs index f6ca71ad12..c35cda2fec 100644 --- a/crates/native_blockifier/src/state_readers.rs +++ b/crates/native_blockifier/src/state_readers.rs @@ -1,2 +1 @@ -pub mod papyrus_state; pub mod py_state_reader; diff --git a/crates/papyrus_state_reader/Cargo.toml b/crates/papyrus_state_reader/Cargo.toml new file mode 100644 index 0000000000..9360144e04 --- /dev/null +++ b/crates/papyrus_state_reader/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "papyrus_state_reader" +version.workspace = true +edition.workspace = true +repository.workspace = true +license.workspace = true + +[lints] +workspace = true + +[dependencies] +blockifier.workspace = true +papyrus_storage.workspace = true +starknet-types-core.workspace = true +starknet_api.workspace = true + +[dev-dependencies] +indexmap.workspace = true diff --git a/crates/papyrus_state_reader/src/lib.rs b/crates/papyrus_state_reader/src/lib.rs new file mode 100644 index 0000000000..378444d69a --- /dev/null +++ b/crates/papyrus_state_reader/src/lib.rs @@ -0,0 +1 @@ +pub mod papyrus_state; diff --git a/crates/native_blockifier/src/state_readers/papyrus_state.rs b/crates/papyrus_state_reader/src/papyrus_state.rs similarity index 99% rename from crates/native_blockifier/src/state_readers/papyrus_state.rs rename to crates/papyrus_state_reader/src/papyrus_state.rs index bdf38d748c..b957a170b0 100644 --- a/crates/native_blockifier/src/state_readers/papyrus_state.rs +++ b/crates/papyrus_state_reader/src/papyrus_state.rs @@ -20,7 +20,6 @@ use starknet_types_core::felt::Felt; mod test; type RawPapyrusReader<'env> = papyrus_storage::StorageTxn<'env, RO>; - pub struct PapyrusReader { storage_reader: StorageReader, latest_block: BlockNumber, diff --git a/crates/native_blockifier/src/state_readers/papyrus_state_test.rs b/crates/papyrus_state_reader/src/papyrus_state_test.rs similarity index 60% rename from crates/native_blockifier/src/state_readers/papyrus_state_test.rs rename to crates/papyrus_state_reader/src/papyrus_state_test.rs index 5b149df93c..91821d55c9 100644 --- a/crates/native_blockifier/src/state_readers/papyrus_state_test.rs +++ b/crates/papyrus_state_reader/src/papyrus_state_test.rs @@ -1,5 +1,3 @@ -use std::collections::HashMap; - use blockifier::abi::abi_utils::selector_from_name; use blockifier::execution::call_info::CallExecution; use blockifier::execution::entry_point::CallEntryPoint; @@ -13,17 +11,10 @@ use indexmap::IndexMap; use papyrus_storage::class::ClassStorageWriter; use papyrus_storage::state::StateStorageWriter; use starknet_api::block::BlockNumber; -use starknet_api::deprecated_contract_class::ContractClass as DeprecatedContractClass; use starknet_api::state::{StateDiff, StorageKey}; use starknet_api::{calldata, felt}; -use starknet_types_core::felt::Felt; - -use crate::py_block_executor::PyBlockExecutor; -use crate::py_state_diff::PyBlockInfo; -use crate::py_utils::PyFelt; -use crate::state_readers::papyrus_state::PapyrusReader; -const LARGE_COMPILED_CONTRACT_JSON: &str = include_str!("large_compiled_contract.json"); +use crate::papyrus_state::PapyrusReader; #[test] fn test_entry_point_with_papyrus_state() -> papyrus_storage::StorageResult<()> { @@ -79,48 +70,3 @@ fn test_entry_point_with_papyrus_state() -> papyrus_storage::StorageResult<()> { Ok(()) } - -#[test] -/// Edge case: adding a large contract to the global contract cache. -fn global_contract_cache_update_large_contract() { - let mut raw_contract_class: serde_json::Value = - serde_json::from_str(LARGE_COMPILED_CONTRACT_JSON).unwrap(); - - // ABI is not required for execution. - raw_contract_class - .as_object_mut() - .expect("A compiled contract must be a JSON object.") - .remove("abi"); - - let dep_casm: DeprecatedContractClass = serde_json::from_value(raw_contract_class) - .expect("DeprecatedContractClass is not supported for this contract."); - - let temp_storage_path = tempfile::tempdir().unwrap().into_path(); - let mut block_executor = PyBlockExecutor::native_create_for_testing( - Default::default(), - Default::default(), - temp_storage_path, - 4000, - ); - block_executor - .append_block( - 0, - None, - Default::default(), - Default::default(), - Default::default(), - HashMap::from([(PyFelt::from(1_u8), serde_json::to_string(&dep_casm).unwrap())]), - ) - .unwrap(); - - block_executor - .append_block( - 1, - Some(PyFelt(Felt::ZERO)), - PyBlockInfo { block_number: 1, ..PyBlockInfo::default() }, - Default::default(), - Default::default(), - HashMap::from([(PyFelt::from(1_u8), serde_json::to_string(&dep_casm).unwrap())]), - ) - .unwrap(); -}