From fac00094a4135fa9622df43b484f0478f4f102dd Mon Sep 17 00:00:00 2001 From: Ayelet Zilber Date: Mon, 21 Oct 2024 13:34:45 +0300 Subject: [PATCH] refactor(blockifier): make VersionedConstantsOverrides fields optional --- config/mempool/default_config.json | 8 ++-- crates/blockifier/src/versioned_constants.rs | 48 +++++++------------ .../src/versioned_constants_test.rs | 12 ++--- .../src/stateful_transaction_validator.rs | 13 +++-- crates/native_blockifier/src/py_objects.rs | 6 ++- 5 files changed, 39 insertions(+), 48 deletions(-) diff --git a/config/mempool/default_config.json b/config/mempool/default_config.json index 57527f744c..145aafc0e6 100644 --- a/config/mempool/default_config.json +++ b/config/mempool/default_config.json @@ -122,17 +122,17 @@ "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": null }, "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": null }, "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": null }, "batcher_config.global_contract_cache_size": { "description": "Cache size for the global_class_hash_to_class. Initialized with this size on creation.", @@ -699,4 +699,4 @@ "privacy": "Public", "value": "" } -} +} \ No newline at end of file diff --git a/crates/blockifier/src/versioned_constants.rs b/crates/blockifier/src/versioned_constants.rs index d6433df961..1af691f907 100644 --- a/crates/blockifier/src/versioned_constants.rs +++ b/crates/blockifier/src/versioned_constants.rs @@ -299,28 +299,23 @@ impl VersionedConstants { Self { vm_resource_fee_cost, archival_data_gas_costs, ..latest } } - pub fn latest_constants_with_overrides( - validate_max_n_steps: u32, - max_recursion_depth: usize, - ) -> Self { - Self { validate_max_n_steps, max_recursion_depth, ..Self::latest_constants().clone() } - } - /// Returns the latest versioned constants after applying the given overrides. pub fn get_versioned_constants( versioned_constants_overrides: VersionedConstantsOverrides, ) -> 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() + let mut constants = Self::latest_constants().clone(); + + if let Some(validate_max_n_steps) = versioned_constants_overrides.validate_max_n_steps { + constants.validate_max_n_steps = validate_max_n_steps; } + if let Some(max_recursion_depth) = versioned_constants_overrides.max_recursion_depth { + constants.max_recursion_depth = max_recursion_depth; + } + if let Some(invoke_tx_max_n_steps) = versioned_constants_overrides.invoke_tx_max_n_steps { + constants.invoke_tx_max_n_steps = invoke_tx_max_n_steps; + } + + constants } pub fn get_archival_data_gas_costs( @@ -824,22 +819,11 @@ pub struct ResourcesByVersion { pub deprecated_resources: ResourcesParams, } -#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +#[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, - } - } + pub validate_max_n_steps: Option, + pub max_recursion_depth: Option, + pub invoke_tx_max_n_steps: Option, } impl SerializeConfig for VersionedConstantsOverrides { diff --git a/crates/blockifier/src/versioned_constants_test.rs b/crates/blockifier/src/versioned_constants_test.rs index 7947afc52a..75ef53f056 100644 --- a/crates/blockifier/src/versioned_constants_test.rs +++ b/crates/blockifier/src/versioned_constants_test.rs @@ -40,21 +40,21 @@ fn test_successful_gas_costs_parsing() { #[test] fn test_versioned_constants_overrides() { let versioned_constants = VersionedConstants::latest_constants().clone(); - let updated_invoke_tx_max_n_steps = versioned_constants.invoke_tx_max_n_steps + 1; let updated_validate_max_n_steps = versioned_constants.validate_max_n_steps + 1; let updated_max_recursion_depth = versioned_constants.max_recursion_depth + 1; - // Create a versioned constants copy with overriden values. + // Create a versioned constants copy with overridden values. let result = VersionedConstants::get_versioned_constants(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, + validate_max_n_steps: Some(updated_validate_max_n_steps), + max_recursion_depth: Some(updated_max_recursion_depth), + invoke_tx_max_n_steps: None, }); // Assert the new values are used. - assert_eq!(result.invoke_tx_max_n_steps, updated_invoke_tx_max_n_steps); assert_eq!(result.validate_max_n_steps, updated_validate_max_n_steps); assert_eq!(result.max_recursion_depth, updated_max_recursion_depth); + // Assert this value is not overridden. + assert_eq!(result.invoke_tx_max_n_steps, versioned_constants.invoke_tx_max_n_steps); } #[test] diff --git a/crates/gateway/src/stateful_transaction_validator.rs b/crates/gateway/src/stateful_transaction_validator.rs index 51694eb868..10b4a4045b 100644 --- a/crates/gateway/src/stateful_transaction_validator.rs +++ b/crates/gateway/src/stateful_transaction_validator.rs @@ -7,7 +7,7 @@ use blockifier::bouncer::BouncerConfig; use blockifier::context::{BlockContext, ChainInfo}; use blockifier::state::cached_state::CachedState; use blockifier::transaction::account_transaction::AccountTransaction; -use blockifier::versioned_constants::VersionedConstants; +use blockifier::versioned_constants::{VersionedConstants, VersionedConstantsOverrides}; #[cfg(test)] use mockall::automock; use starknet_api::core::{ContractAddress, Nonce}; @@ -95,10 +95,13 @@ impl StatefulTransactionValidator { let latest_block_info = get_latest_block_info(state_reader_factory)?; let state_reader = state_reader_factory.get_state_reader(latest_block_info.block_number); let state = CachedState::new(state_reader); - let versioned_constants = VersionedConstants::latest_constants_with_overrides( - self.config.validate_max_n_steps, - self.config.max_recursion_depth, - ); + let versioned_constants_overrides = VersionedConstantsOverrides { + validate_max_n_steps: Some(self.config.validate_max_n_steps), + max_recursion_depth: Some(self.config.max_recursion_depth), + invoke_tx_max_n_steps: None, + }; + let versioned_constants = + VersionedConstants::get_versioned_constants(versioned_constants_overrides); let mut block_info = latest_block_info; block_info.block_number = block_info.block_number.unchecked_next(); // TODO(yael 21/4/24): create the block context using pre_process_block once we will be diff --git a/crates/native_blockifier/src/py_objects.rs b/crates/native_blockifier/src/py_objects.rs index 8212a3fd3a..7f0c27ec0f 100644 --- a/crates/native_blockifier/src/py_objects.rs +++ b/crates/native_blockifier/src/py_objects.rs @@ -72,7 +72,11 @@ impl From for VersionedConstantsOverrides { max_recursion_depth, invoke_tx_max_n_steps, } = py_versioned_constants_overrides; - Self { validate_max_n_steps, max_recursion_depth, invoke_tx_max_n_steps } + Self { + validate_max_n_steps: Some(validate_max_n_steps), + max_recursion_depth: Some(max_recursion_depth), + invoke_tx_max_n_steps: Some(invoke_tx_max_n_steps), + } } }