From ddc8f516c3befa92ccfbd3382b8d5b718fe9bb98 Mon Sep 17 00:00:00 2001 From: amosStarkware <88497213+amosStarkware@users.noreply.github.com> Date: Mon, 26 Aug 2024 13:03:12 +0300 Subject: [PATCH 1/4] chore(native_blockifier): convert general config to os config (#561) --- .../src/py_block_executor.rs | 28 ++++++------------- .../src/py_block_executor_test.rs | 4 +-- crates/native_blockifier/src/py_validator.rs | 8 +++--- 3 files changed, 15 insertions(+), 25 deletions(-) diff --git a/crates/native_blockifier/src/py_block_executor.rs b/crates/native_blockifier/src/py_block_executor.rs index bf3c244eba..06cefba144 100644 --- a/crates/native_blockifier/src/py_block_executor.rs +++ b/crates/native_blockifier/src/py_block_executor.rs @@ -91,11 +91,11 @@ pub struct PyBlockExecutor { #[pymethods] impl PyBlockExecutor { #[new] - #[pyo3(signature = (bouncer_config, concurrency_config, general_config, global_contract_cache_size, target_storage_config, py_versioned_constants_overrides))] + #[pyo3(signature = (bouncer_config, concurrency_config, os_config, global_contract_cache_size, target_storage_config, py_versioned_constants_overrides))] pub fn create( bouncer_config: PyBouncerConfig, concurrency_config: PyConcurrencyConfig, - general_config: PyGeneralConfig, + os_config: PyOsConfig, global_contract_cache_size: usize, target_storage_config: StorageConfig, py_versioned_constants_overrides: PyVersionedConstantsOverrides, @@ -112,7 +112,7 @@ impl PyBlockExecutor { tx_executor_config: TransactionExecutorConfig { concurrency_config: concurrency_config.into(), }, - chain_info: general_config.starknet_os_config.into_chain_info(), + chain_info: os_config.into_chain_info(), versioned_constants, tx_executor: None, storage: Box::new(storage), @@ -333,11 +333,11 @@ impl PyBlockExecutor { } #[cfg(any(feature = "testing", test))] - #[pyo3(signature = (concurrency_config, general_config, path, max_state_diff_size))] + #[pyo3(signature = (concurrency_config, os_config, path, max_state_diff_size))] #[staticmethod] fn create_for_testing( concurrency_config: PyConcurrencyConfig, - general_config: PyGeneralConfig, + os_config: PyOsConfig, path: std::path::PathBuf, max_state_diff_size: usize, ) -> Self { @@ -357,11 +357,8 @@ impl PyBlockExecutor { tx_executor_config: TransactionExecutorConfig { concurrency_config: concurrency_config.into(), }, - storage: Box::new(PapyrusStorage::new_for_testing( - path, - &general_config.starknet_os_config.chain_id, - )), - chain_info: general_config.starknet_os_config.into_chain_info(), + storage: Box::new(PapyrusStorage::new_for_testing(path, &os_config.chain_id)), + chain_info: os_config.into_chain_info(), versioned_constants, tx_executor: None, global_contract_cache: GlobalContractCache::new(GLOBAL_CONTRACT_CACHE_SIZE_FOR_TEST), @@ -401,21 +398,14 @@ impl PyBlockExecutor { #[cfg(test)] pub(crate) fn native_create_for_testing( concurrency_config: PyConcurrencyConfig, - general_config: PyGeneralConfig, + os_config: PyOsConfig, path: std::path::PathBuf, max_state_diff_size: usize, ) -> Self { - Self::create_for_testing(concurrency_config, general_config, path, max_state_diff_size) + Self::create_for_testing(concurrency_config, os_config, path, max_state_diff_size) } } -#[derive(Default, FromPyObject)] -pub struct PyGeneralConfig { - pub starknet_os_config: PyOsConfig, - pub invoke_tx_max_n_steps: u32, - pub validate_max_n_steps: u32, -} - #[derive(Clone, FromPyObject)] pub struct PyOsConfig { #[pyo3(from_py_with = "int_to_chain_id")] diff --git a/crates/native_blockifier/src/py_block_executor_test.rs b/crates/native_blockifier/src/py_block_executor_test.rs index ffc5908ad8..b3cbca87c0 100644 --- a/crates/native_blockifier/src/py_block_executor_test.rs +++ b/crates/native_blockifier/src/py_block_executor_test.rs @@ -10,7 +10,7 @@ use starknet_api::core::ClassHash; use starknet_api::{class_hash, felt}; use starknet_types_core::felt::Felt; -use crate::py_block_executor::{PyBlockExecutor, PyGeneralConfig}; +use crate::py_block_executor::{PyBlockExecutor, PyOsConfig}; use crate::py_objects::PyConcurrencyConfig; use crate::py_state_diff::{PyBlockInfo, PyStateDiff}; use crate::py_utils::PyFelt; @@ -26,7 +26,7 @@ fn global_contract_cache_update() { let temp_storage_path = tempfile::tempdir().unwrap().into_path(); let mut block_executor = PyBlockExecutor::create_for_testing( PyConcurrencyConfig::default(), - PyGeneralConfig::default(), + PyOsConfig::default(), temp_storage_path, 4000, ); diff --git a/crates/native_blockifier/src/py_validator.rs b/crates/native_blockifier/src/py_validator.rs index 8398e48c1a..e035f0c4ef 100644 --- a/crates/native_blockifier/src/py_validator.rs +++ b/crates/native_blockifier/src/py_validator.rs @@ -12,7 +12,7 @@ use starknet_api::transaction::TransactionHash; use starknet_types_core::felt::Felt; use crate::errors::NativeBlockifierResult; -use crate::py_block_executor::PyGeneralConfig; +use crate::py_block_executor::PyOsConfig; use crate::py_objects::PyVersionedConstantsOverrides; use crate::py_state_diff::PyBlockInfo; use crate::py_transaction::{py_account_tx, PyClassInfo, PY_TX_PARSING_ERR}; @@ -28,9 +28,9 @@ pub struct PyValidator { #[pymethods] impl PyValidator { #[new] - #[pyo3(signature = (general_config, state_reader_proxy, next_block_info, max_nonce_for_validation_skip, py_versioned_constants_overrides))] + #[pyo3(signature = (os_config, state_reader_proxy, next_block_info, max_nonce_for_validation_skip, py_versioned_constants_overrides))] pub fn create( - general_config: PyGeneralConfig, + os_config: PyOsConfig, state_reader_proxy: &PyAny, next_block_info: PyBlockInfo, max_nonce_for_validation_skip: PyFelt, @@ -45,7 +45,7 @@ impl PyValidator { VersionedConstants::get_versioned_constants(py_versioned_constants_overrides.into()); let block_context = BlockContext::new( next_block_info.try_into().expect("Failed to convert block info."), - general_config.starknet_os_config.into_chain_info(), + os_config.into_chain_info(), versioned_constants, BouncerConfig::max(), ); From 29f90d322ee7514e12daff504f4c4dc8d14d6c6f Mon Sep 17 00:00:00 2001 From: amosStarkware <88497213+amosStarkware@users.noreply.github.com> Date: Mon, 26 Aug 2024 14:04:31 +0300 Subject: [PATCH 2/4] chore(blockifier, native_blockifier): updated versioned constants (#559) --- crates/blockifier/src/versioned_constants.rs | 29 +++++---------- .../src/versioned_constants_test.rs | 27 +++++++------- crates/native_blockifier/src/py_objects.rs | 35 ++++--------------- 3 files changed, 30 insertions(+), 61 deletions(-) diff --git a/crates/blockifier/src/versioned_constants.rs b/crates/blockifier/src/versioned_constants.rs index a975ab0e98..48824fe197 100644 --- a/crates/blockifier/src/versioned_constants.rs +++ b/crates/blockifier/src/versioned_constants.rs @@ -224,32 +224,21 @@ impl VersionedConstants { Self { validate_max_n_steps, max_recursion_depth, ..Self::latest_constants().clone() } } - // TODO(Amos, 1/8/2024): Remove the explicit `validate_max_n_steps` & `max_recursion_depth`, - // they should be part of the general override. - /// `versioned_constants_base_overrides` are used if they are provided, otherwise the latest - /// versioned constants are used. `validate_max_n_steps` & `max_recursion_depth` override both. + /// 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, - versioned_constants_base_overrides, + invoke_tx_max_n_steps, } = versioned_constants_overrides; - let base_overrides = match versioned_constants_base_overrides { - Some(versioned_constants_base_overrides) => { - log::debug!( - "Using provided `versioned_constants_base_overrides` (with additional \ - overrides)." - ); - versioned_constants_base_overrides - } - None => { - log::debug!("Using latest versioned constants (with additional overrides)."); - Self::latest_constants().clone() - } - }; - Self { validate_max_n_steps, max_recursion_depth, ..base_overrides } + Self { + validate_max_n_steps, + max_recursion_depth, + invoke_tx_max_n_steps, + ..Self::latest_constants().clone() + } } } @@ -746,5 +735,5 @@ pub struct ResourcesByVersion { pub struct VersionedConstantsOverrides { pub validate_max_n_steps: u32, pub max_recursion_depth: usize, - pub versioned_constants_base_overrides: Option, + pub invoke_tx_max_n_steps: u32, } diff --git a/crates/blockifier/src/versioned_constants_test.rs b/crates/blockifier/src/versioned_constants_test.rs index ff6dd2d4d2..7a6f9f545d 100644 --- a/crates/blockifier/src/versioned_constants_test.rs +++ b/crates/blockifier/src/versioned_constants_test.rs @@ -92,24 +92,25 @@ fn get_json_value_without_defaults() -> serde_json::Value { json_value_without_defaults } -/// Assert `versioned_constants_base_overrides` are used when provided. +/// Assert versioned constants overrides are used when provided. #[test] -fn test_versioned_constants_base_overrides() { - // Create a versioned constants copy with a modified value for `invoke_tx_max_n_steps`. - let mut versioned_constants_base_overrides = VERSIONED_CONSTANTS_LATEST.clone(); - versioned_constants_base_overrides.invoke_tx_max_n_steps += 1; +fn test_versioned_constants_overrides() { + let versioned_constants = VERSIONED_CONSTANTS_LATEST.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. let result = VersionedConstants::get_versioned_constants(VersionedConstantsOverrides { - validate_max_n_steps: versioned_constants_base_overrides.validate_max_n_steps, - max_recursion_depth: versioned_constants_base_overrides.max_recursion_depth, - versioned_constants_base_overrides: Some(versioned_constants_base_overrides.clone()), + 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 value is used. - assert_eq!( - result.invoke_tx_max_n_steps, - versioned_constants_base_overrides.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); + assert_eq!(result.validate_max_n_steps, updated_validate_max_n_steps); + assert_eq!(result.max_recursion_depth, updated_max_recursion_depth); } #[test] diff --git a/crates/native_blockifier/src/py_objects.rs b/crates/native_blockifier/src/py_objects.rs index 4cb6716015..8212a3fd3a 100644 --- a/crates/native_blockifier/src/py_objects.rs +++ b/crates/native_blockifier/src/py_objects.rs @@ -3,10 +3,9 @@ use std::collections::HashMap; use blockifier::abi::constants; use blockifier::blockifier::config::ConcurrencyConfig; use blockifier::bouncer::{BouncerConfig, BouncerWeights, BuiltinCount, HashMapWrapper}; -use blockifier::versioned_constants::{VersionedConstants, VersionedConstantsOverrides}; +use blockifier::versioned_constants::VersionedConstantsOverrides; use cairo_vm::types::builtin_name::BuiltinName; use cairo_vm::vm::runners::cairo_runner::ExecutionResources; -use pyo3::exceptions::PyValueError; use pyo3::prelude::*; use crate::errors::{ @@ -50,30 +49,19 @@ impl From for PyExecutionResources { pub struct PyVersionedConstantsOverrides { pub validate_max_n_steps: u32, pub max_recursion_depth: usize, - pub versioned_constants_base_overrides: Option, + pub invoke_tx_max_n_steps: u32, } #[pymethods] impl PyVersionedConstantsOverrides { #[new] - #[pyo3(signature = (validate_max_n_steps, max_recursion_depth, versioned_constants_base_overrides))] + #[pyo3(signature = (validate_max_n_steps, max_recursion_depth, invoke_tx_max_n_steps))] pub fn create( validate_max_n_steps: u32, max_recursion_depth: usize, - versioned_constants_base_overrides: Option, + invoke_tx_max_n_steps: u32, ) -> Self { - Self { validate_max_n_steps, max_recursion_depth, versioned_constants_base_overrides } - } - - #[staticmethod] - pub fn assert_versioned_consts_load_successfully( - versioned_constants_str: &str, - ) -> PyResult<()> { - if serde_json::from_str::(versioned_constants_str).is_ok() { - Ok(()) - } else { - Err(PyValueError::new_err("Failed to parse `versioned_constants_str`.")) - } + Self { validate_max_n_steps, max_recursion_depth, invoke_tx_max_n_steps } } } @@ -82,18 +70,9 @@ impl From for VersionedConstantsOverrides { let PyVersionedConstantsOverrides { validate_max_n_steps, max_recursion_depth, - versioned_constants_base_overrides, + invoke_tx_max_n_steps, } = py_versioned_constants_overrides; - let base_overrides = - versioned_constants_base_overrides.map(|versioned_constants_base_overrides| { - serde_json::from_str(&versioned_constants_base_overrides) - .expect("Versioned constants JSON file is malformed.") - }); - Self { - validate_max_n_steps, - max_recursion_depth, - versioned_constants_base_overrides: base_overrides, - } + Self { validate_max_n_steps, max_recursion_depth, invoke_tx_max_n_steps } } } From 87d874fcd6c79ecfe625d6fd0af5e9a61163e141 Mon Sep 17 00:00:00 2001 From: dorimedini-starkware Date: Tue, 27 Aug 2024 14:50:30 +0300 Subject: [PATCH 3/4] chore(ci): verify GH client is installed in merge_branches.py (#616) Signed-off-by: Dori Medini --- .github/workflows/merge_paths_ci.yml | 4 ++-- scripts/merge_branches.py | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/.github/workflows/merge_paths_ci.yml b/.github/workflows/merge_paths_ci.yml index ebe68a84cc..36cfdf391a 100644 --- a/.github/workflows/merge_paths_ci.yml +++ b/.github/workflows/merge_paths_ci.yml @@ -9,7 +9,7 @@ on: - v[0-9].** paths: - '.github/workflows/merge_paths_ci.yml' - - 'scripts/merge_branches.json' + - 'scripts/merge_branches.py' - 'scripts/merge_paths.json' - 'scripts/merge_paths_test.py' - 'scripts/merge_status.py' @@ -23,7 +23,7 @@ on: - edited paths: - '.github/workflows/merge_paths_ci.yml' - - 'scripts/merge_branches.json' + - 'scripts/merge_branches.py' - 'scripts/merge_paths.json' - 'scripts/merge_paths_test.py' - 'scripts/merge_status.py' diff --git a/scripts/merge_branches.py b/scripts/merge_branches.py index 6ae3c49338..8b960f0460 100755 --- a/scripts/merge_branches.py +++ b/scripts/merge_branches.py @@ -74,11 +74,31 @@ def dstdiff(source_branch: str, destination_branch: Optional[str], files: List[s ) +def verify_gh_client_status(): + try: + run_command("gh --version") + except subprocess.CalledProcessError: + print( + "GitHub CLI not found. Please install it from " + "https://github.com/cli/cli/blob/trunk/docs/install_linux.md#installing-gh-on-linux-and-bsd" + ) + exit(1) + try: + run_command("gh auth status") + except subprocess.CalledProcessError: + print( + "GitHub CLI not authenticated. Please authenticate using `gh auth login` " + "and follow the instructions." + ) + exit(1) + + def merge_branches(src_branch: str, dst_branch: Optional[str]): """ Merge source branch into destination branch. If no destination branch is passed, the destination branch is taken from state on repo. """ + verify_gh_client_status() user = os.environ["USER"] dst_branch = get_dst_branch(src_branch=src_branch, dst_branch_override=dst_branch) From 79f81fd7519c01868cbbe774af5f3ab650978063 Mon Sep 17 00:00:00 2001 From: Dori Medini Date: Tue, 27 Aug 2024 15:26:53 +0300 Subject: [PATCH 4/4] No conflicts in main-v0.13.2 -> main merge, this commit is for any change needed to pass the CI.