From b50728a6bea08abf47d58a1b060de3cd35f9ce23 Mon Sep 17 00:00:00 2001 From: Ayelet Zilber Date: Tue, 15 Oct 2024 11:13:16 +0300 Subject: [PATCH] refactor(batcher): make versioned_constants_overrides field optional in BlockBuilderConfig --- config/mempool/default_config.json | 11 +++-- crates/batcher/src/block_builder.rs | 16 +++++--- crates/blockifier/src/versioned_constants.rs | 40 ++++++++----------- .../src/versioned_constants_test.rs | 4 +- .../src/py_block_executor.rs | 5 ++- crates/native_blockifier/src/py_validator.rs | 5 ++- 6 files changed, 43 insertions(+), 38 deletions(-) diff --git a/config/mempool/default_config.json b/config/mempool/default_config.json index 4463567f33..cb9cebf277 100644 --- a/config/mempool/default_config.json +++ b/config/mempool/default_config.json @@ -119,20 +119,25 @@ "privacy": "Public", "value": true }, + "batcher_config.block_builder_config.versioned_constants_overrides.#is_none": { + "description": "Flag for an optional field.", + "privacy": "TemporaryValue", + "value": true + }, "batcher_config.block_builder_config.versioned_constants_overrides.invoke_tx_max_n_steps": { "description": "Maximum number of steps the invoke function is allowed to run.", "privacy": "Public", - "value": 10000000 + "value": 0 }, "batcher_config.block_builder_config.versioned_constants_overrides.max_recursion_depth": { "description": "Maximum recursion depth for nested calls during blockifier validation.", "privacy": "Public", - "value": 50 + "value": 0 }, "batcher_config.block_builder_config.versioned_constants_overrides.validate_max_n_steps": { "description": "Maximum number of steps the validation function is allowed to run.", "privacy": "Public", - "value": 1000000 + "value": 0 }, "batcher_config.outstream_content_buffer_size": { "description": "Maximum items to add to the outstream buffer before blocking further filling of the stream.", diff --git a/crates/batcher/src/block_builder.rs b/crates/batcher/src/block_builder.rs index 98c25d25cc..63340c3f67 100644 --- a/crates/batcher/src/block_builder.rs +++ b/crates/batcher/src/block_builder.rs @@ -22,7 +22,12 @@ use blockifier::versioned_constants::{VersionedConstants, VersionedConstantsOver use indexmap::IndexMap; #[cfg(test)] use mockall::automock; -use papyrus_config::dumping::{append_sub_config_name, ser_param, SerializeConfig}; +use papyrus_config::dumping::{ + append_sub_config_name, + ser_optional_sub_config, + ser_param, + SerializeConfig, +}; use papyrus_config::{ParamPath, ParamPrivacyInput, SerializedParam}; use papyrus_storage::StorageReader; use serde::{Deserialize, Serialize}; @@ -103,7 +108,7 @@ impl Default for BlockBuilderConfig { sequencer_address: ContractAddress::default(), use_kzg_da: true, tx_chunk_size: 100, - versioned_constants_overrides: VersionedConstantsOverrides::default(), + versioned_constants_overrides: None, } } } @@ -131,8 +136,8 @@ impl SerializeConfig for BlockBuilderConfig { "The size of the transaction chunk.", ParamPrivacyInput::Public, )])); - dump.append(&mut append_sub_config_name( - self.versioned_constants_overrides.dump(), + dump.append(&mut ser_optional_sub_config( + &self.versioned_constants_overrides, "versioned_constants_overrides", )); dump @@ -235,8 +240,7 @@ pub struct BlockBuilderConfig { pub sequencer_address: ContractAddress, pub use_kzg_da: bool, pub tx_chunk_size: usize, - // TODO(Ayelet): Make this field optional. - pub versioned_constants_overrides: VersionedConstantsOverrides, + pub versioned_constants_overrides: Option, } pub struct BlockBuilderFactory { diff --git a/crates/blockifier/src/versioned_constants.rs b/crates/blockifier/src/versioned_constants.rs index d6433df961..9fd24469b0 100644 --- a/crates/blockifier/src/versioned_constants.rs +++ b/crates/blockifier/src/versioned_constants.rs @@ -308,18 +308,22 @@ impl VersionedConstants { /// Returns the latest versioned constants after applying the given overrides. pub fn get_versioned_constants( - versioned_constants_overrides: VersionedConstantsOverrides, + versioned_constants_overrides: Option, ) -> Self { - let VersionedConstantsOverrides { - validate_max_n_steps, - max_recursion_depth, - invoke_tx_max_n_steps, - } = versioned_constants_overrides; - Self { - validate_max_n_steps, - max_recursion_depth, - invoke_tx_max_n_steps, - ..Self::latest_constants().clone() + if let Some(overrides) = versioned_constants_overrides { + let VersionedConstantsOverrides { + validate_max_n_steps, + max_recursion_depth, + invoke_tx_max_n_steps, + } = overrides; + Self { + validate_max_n_steps, + max_recursion_depth, + invoke_tx_max_n_steps, + ..Self::latest_constants().clone() + } + } else { + Self::latest_constants().clone() } } @@ -824,24 +828,14 @@ pub struct ResourcesByVersion { pub deprecated_resources: ResourcesParams, } -#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +// TODO(Ayelet): Make fields optional. +#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] pub struct VersionedConstantsOverrides { pub validate_max_n_steps: u32, pub max_recursion_depth: usize, pub invoke_tx_max_n_steps: u32, } -impl Default for VersionedConstantsOverrides { - // TODO: update the default values once the actual values are known. - fn default() -> Self { - Self { - validate_max_n_steps: 1000000, - max_recursion_depth: 50, - invoke_tx_max_n_steps: 10000000, - } - } -} - impl SerializeConfig for VersionedConstantsOverrides { fn dump(&self) -> BTreeMap { BTreeMap::from_iter([ diff --git a/crates/blockifier/src/versioned_constants_test.rs b/crates/blockifier/src/versioned_constants_test.rs index 7947afc52a..5204d860db 100644 --- a/crates/blockifier/src/versioned_constants_test.rs +++ b/crates/blockifier/src/versioned_constants_test.rs @@ -45,11 +45,11 @@ fn test_versioned_constants_overrides() { let updated_max_recursion_depth = versioned_constants.max_recursion_depth + 1; // Create a versioned constants copy with overriden values. - let result = VersionedConstants::get_versioned_constants(VersionedConstantsOverrides { + let result = VersionedConstants::get_versioned_constants(Some(VersionedConstantsOverrides { validate_max_n_steps: updated_validate_max_n_steps, max_recursion_depth: updated_max_recursion_depth, invoke_tx_max_n_steps: updated_invoke_tx_max_n_steps, - }); + })); // Assert the new values are used. assert_eq!(result.invoke_tx_max_n_steps, updated_invoke_tx_max_n_steps); diff --git a/crates/native_blockifier/src/py_block_executor.rs b/crates/native_blockifier/src/py_block_executor.rs index 7bf28040a1..aa9cd93e5d 100644 --- a/crates/native_blockifier/src/py_block_executor.rs +++ b/crates/native_blockifier/src/py_block_executor.rs @@ -141,8 +141,9 @@ impl PyBlockExecutor { log::debug!("Initializing Block Executor..."); let storage = PapyrusStorage::new(target_storage_config).expect("Failed to initialize storage."); - let versioned_constants = - VersionedConstants::get_versioned_constants(py_versioned_constants_overrides.into()); + let versioned_constants = VersionedConstants::get_versioned_constants(Some( + py_versioned_constants_overrides.into(), + )); log::debug!("Initialized Block Executor."); Self { diff --git a/crates/native_blockifier/src/py_validator.rs b/crates/native_blockifier/src/py_validator.rs index e035f0c4ef..3a2620621a 100644 --- a/crates/native_blockifier/src/py_validator.rs +++ b/crates/native_blockifier/src/py_validator.rs @@ -41,8 +41,9 @@ impl PyValidator { let state = CachedState::new(state_reader); // Create the block context. - let versioned_constants = - VersionedConstants::get_versioned_constants(py_versioned_constants_overrides.into()); + let versioned_constants = VersionedConstants::get_versioned_constants(Some( + py_versioned_constants_overrides.into(), + )); let block_context = BlockContext::new( next_block_info.try_into().expect("Failed to convert block info."), os_config.into_chain_info(),