diff --git a/Cargo.lock b/Cargo.lock index 04cefd04e5..411500a011 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1664,7 +1664,6 @@ dependencies = [ "serde", "serde_json", "sha2", - "stacker", "starknet-types-core", "starknet_api", "starknet_infra_utils", @@ -8780,15 +8779,6 @@ version = "2.0.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "33cb294fe86a74cbcf50d4445b37da762029549ebeea341421c7c70370f86cac" -[[package]] -name = "psm" -version = "0.1.24" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "200b9ff220857e53e184257720a14553b2f4aa02577d2ed9842d45d4b9654810" -dependencies = [ - "cc", -] - [[package]] name = "publicsuffix" version = "2.3.0" @@ -10436,19 +10426,6 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" -[[package]] -name = "stacker" -version = "0.1.17" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "799c883d55abdb5e98af1a7b3f23b9b6de8ecada0ecac058672d7635eb48ca7b" -dependencies = [ - "cc", - "cfg-if", - "libc", - "psm", - "windows-sys 0.59.0", -] - [[package]] name = "starknet-core" version = "0.6.1" diff --git a/Cargo.toml b/Cargo.toml index c38f516366..d37f0cd0f8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -212,7 +212,6 @@ serde_yaml = "0.9.16" sha2 = "0.10.8" sha3 = "0.10.8" simple_logger = "4.0.0" -stacker = "0.1.17" starknet-core = "0.6.0" starknet-crypto = "0.7.1" starknet-types-core = "0.1.6" diff --git a/crates/blockifier/Cargo.toml b/crates/blockifier/Cargo.toml index 56ae7f49e6..95598ad8bc 100644 --- a/crates/blockifier/Cargo.toml +++ b/crates/blockifier/Cargo.toml @@ -10,7 +10,7 @@ description = "The transaction-executing component in the Starknet sequencer." workspace = true [features] -cairo_native = ["dep:cairo-native", "dep:stacker", "starknet_sierra_compile/cairo_native"] +cairo_native = ["dep:cairo-native", "starknet_sierra_compile/cairo_native"] jemalloc = ["dep:tikv-jemallocator"] native_blockifier = [] reexecution = ["transaction_serde"] @@ -50,7 +50,6 @@ semver.workspace = true serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true, features = ["arbitrary_precision"] } sha2.workspace = true -stacker = { workspace = true, optional = true } starknet-types-core.workspace = true starknet_api.workspace = true starknet_infra_utils.workspace = true diff --git a/crates/blockifier/src/blockifier/config.rs b/crates/blockifier/src/blockifier/config.rs index 6b64588630..86a57dad02 100644 --- a/crates/blockifier/src/blockifier/config.rs +++ b/crates/blockifier/src/blockifier/config.rs @@ -6,16 +6,22 @@ use serde::{Deserialize, Serialize}; use starknet_sierra_compile::config::SierraCompilationConfig; use crate::state::contract_class_manager::DEFAULT_COMPILATION_REQUEST_CHANNEL_SIZE; +#[cfg(any(test, feature = "testing", feature = "native_blockifier"))] +use crate::state::contract_class_manager::DEFAULT_STACK_SIZE; use crate::state::global_cache::GLOBAL_CONTRACT_CACHE_SIZE_FOR_TEST; #[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] pub struct TransactionExecutorConfig { pub concurrency_config: ConcurrencyConfig, + pub stack_size: usize, } impl TransactionExecutorConfig { #[cfg(any(test, feature = "testing", feature = "native_blockifier"))] pub fn create_for_testing(concurrency_enabled: bool) -> Self { - Self { concurrency_config: ConcurrencyConfig::create_for_testing(concurrency_enabled) } + Self { + concurrency_config: ConcurrencyConfig::create_for_testing(concurrency_enabled), + stack_size: DEFAULT_STACK_SIZE, + } } } diff --git a/crates/blockifier/src/blockifier/transaction_executor.rs b/crates/blockifier/src/blockifier/transaction_executor.rs index d54d9db7ac..bbd85daeef 100644 --- a/crates/blockifier/src/blockifier/transaction_executor.rs +++ b/crates/blockifier/src/blockifier/transaction_executor.rs @@ -131,7 +131,7 @@ impl TransactionExecutor { } } - pub fn execute_txs_sequentially( + pub fn execute_txs_sequentially_inner( &mut self, txs: &[Transaction], ) -> Vec> { @@ -227,6 +227,29 @@ impl TransactionExecutor { } } + pub fn execute_txs_sequentially( + &mut self, + txs: &[Transaction], + ) -> Vec> { + #[cfg(not(feature = "cairo_native"))] + return self.execute_txs_sequentially(txs); + #[cfg(feature = "cairo_native")] + { + // TODO meshi: find a way to access the contract class manager config from transaction + // executor. + let txs = txs.to_vec(); + let mut results = Vec::new(); + std::thread::scope(|s| { + let handle = std::thread::Builder::new() + .stack_size(self.config.stack_size) + .spawn_scoped(s, || self.execute_txs_sequentially_inner(&txs)); + results = + handle.expect("Failed to spawn thread").join().expect("Failed to join thread."); + }); + results + } + } + pub fn execute_chunk( &mut self, chunk: &[Transaction], @@ -250,26 +273,28 @@ impl TransactionExecutor { std::thread::scope(|s| { for _ in 0..self.config.concurrency_config.n_workers { let worker_executor = Arc::clone(&worker_executor); - s.spawn(move || { - // Making sure that the program will abort if a panic accured while halting the - // scheduler. - let abort_guard = AbortIfPanic; - // If a panic is not handled or the handling logic itself panics, then we abort - // the program. - if let Err(err) = catch_unwind(AssertUnwindSafe(|| { - worker_executor.run(); - })) { - // If the program panics here, the abort guard will exit the program. - // In this case, no panic message will be logged. Add the cargo flag - // --nocapture to log the panic message. + let _handle = std::thread::Builder::new() + .stack_size(self.config.stack_size) + .spawn_scoped(s, move || { + // Making sure that the program will abort if a panic accured while halting + // the scheduler. + let abort_guard = AbortIfPanic; + // If a panic is not handled or the handling logic itself panics, then we + // abort the program. + if let Err(err) = catch_unwind(AssertUnwindSafe(|| { + worker_executor.run(); + })) { + // If the program panics here, the abort guard will exit the program. + // In this case, no panic message will be logged. Add the cargo flag + // --nocapture to log the panic message. - worker_executor.scheduler.halt(); - abort_guard.release(); - panic::resume_unwind(err); - } + worker_executor.scheduler.halt(); + abort_guard.release(); + panic::resume_unwind(err); + } - abort_guard.release(); - }); + abort_guard.release(); + }); } }); diff --git a/crates/blockifier/src/execution/native/entry_point_execution.rs b/crates/blockifier/src/execution/native/entry_point_execution.rs index d0ca87b173..56779a7d85 100644 --- a/crates/blockifier/src/execution/native/entry_point_execution.rs +++ b/crates/blockifier/src/execution/native/entry_point_execution.rs @@ -1,7 +1,5 @@ use cairo_native::execution_result::ContractExecutionResult; use cairo_native::utils::BuiltinCosts; -use num_rational::Ratio; -use stacker; use crate::execution::call_info::{CallExecution, CallInfo, Retdata}; use crate::execution::contract_class::TrackedResource; @@ -14,7 +12,6 @@ use crate::execution::errors::{EntryPointExecutionError, PostExecutionError, Pre use crate::execution::native::contract_class::NativeCompiledClassV1; use crate::execution::native::syscall_handler::NativeSyscallHandler; use crate::state::state_api::State; -use crate::versioned_constants::CairoNativeStackConfig; // todo(rodrigo): add an `entry point not found` test for Native pub fn execute_entry_point_call( @@ -66,30 +63,15 @@ pub fn execute_entry_point_call( // This also limits multi-threading, since each thread has its own stack. // If the the free stack size is in the red zone, We will grow the stack to the // target size, relative to reaming gas. - let stack_config = CairoNativeStackConfig { - // TODO(Aviv): Take it from VC. - gas_to_stack_ratio: Ratio::new(1, 20), - max_stack_size: 200 * 1024 * 1024, - min_stack_red_zone: 2 * 1024 * 1024, - buffer_size: 5 * 1024 * 1024, - }; - let stack_size_red_zone = stack_config.get_stack_size_red_zone(call_initial_gas); - let target_stack_size = - usize::try_from(stack_config.get_target_stack_size(stack_size_red_zone)) - .unwrap_or_else(|e| panic!("Failed to convert target stack size to usize: {}", e)); - let stack_size_red_zone = usize::try_from(stack_size_red_zone) - .unwrap_or_else(|e| panic!("Failed to convert stack size red zone to usize: {}", e)); - // Use `maybe_grow` and not `grow` for performance, since in happy flows, only the main call - // should trigger the growth. - let execution_result = stacker::maybe_grow(stack_size_red_zone, target_stack_size, || { - compiled_class.executor.run( - entry_point.selector.0, - &syscall_handler.base.call.calldata.0.clone(), - call_initial_gas, - Some(builtin_costs), - &mut syscall_handler, - ) - }); + + let execution_result = compiled_class.executor.run( + entry_point.selector.0, + &syscall_handler.base.call.calldata.0.clone(), + call_initial_gas, + Some(builtin_costs), + &mut syscall_handler, + ); + syscall_handler.finalize(); let call_result = execution_result.map_err(EntryPointExecutionError::NativeUnexpectedError)?; diff --git a/crates/blockifier/src/execution/syscalls/syscall_tests/out_of_gas.rs b/crates/blockifier/src/execution/syscalls/syscall_tests/out_of_gas.rs index c30bc19a62..1b822c0c9f 100644 --- a/crates/blockifier/src/execution/syscalls/syscall_tests/out_of_gas.rs +++ b/crates/blockifier/src/execution/syscalls/syscall_tests/out_of_gas.rs @@ -3,8 +3,6 @@ use starknet_api::{calldata, felt}; use test_case::test_case; use crate::abi::constants::MAX_POSSIBLE_SIERRA_GAS; -#[cfg(feature = "cairo_native")] -use crate::context::ChainInfo; use crate::execution::call_info::CallExecution; use crate::execution::entry_point::CallEntryPoint; use crate::execution::syscalls::syscall_tests::constants; @@ -12,10 +10,6 @@ use crate::execution::syscalls::syscall_tests::get_block_hash::initialize_state; use crate::execution::syscalls::SyscallSelector; use crate::retdata; use crate::test_utils::contracts::FeatureContract; -#[cfg(feature = "cairo_native")] -use crate::test_utils::initial_test_state::test_state; -#[cfg(feature = "cairo_native")] -use crate::test_utils::BALANCE; use crate::test_utils::{trivial_external_entry_point_new, CairoVersion, RunnableCairo1}; use crate::versioned_constants::VersionedConstants; @@ -64,44 +58,53 @@ fn test_total_tx_limits_less_than_max_sierra_gas() { ); } -#[cfg(feature = "cairo_native")] -#[rstest::rstest] -#[case(MAX_POSSIBLE_SIERRA_GAS, MAX_POSSIBLE_SIERRA_GAS - 2681170910)] -#[case(MAX_POSSIBLE_SIERRA_GAS / 10, 81886490)] -#[case(MAX_POSSIBLE_SIERRA_GAS / 100, 8190940)] -#[case(MAX_POSSIBLE_SIERRA_GAS / 1000, 822890)] -#[case(MAX_POSSIBLE_SIERRA_GAS / 10000, 85440)] -#[case(MAX_POSSIBLE_SIERRA_GAS / 100000, 12340)] -#[case(MAX_POSSIBLE_SIERRA_GAS / 1000000, 0)] -#[case(350, 0)] -#[case(35, 0)] -#[case(0, 0)] -/// Tests that Native can handle deep recursion calls without overflowing the stack. -/// Note that the recursive function must be complicated, since the compiler might transform -/// simple recursions into loops. The tested function was manually tested with higher gas and -/// reached stack overflow. -/// -/// Also, there is no need to test the VM here since it doesn't use the stack. -fn test_stack_overflow(#[case] initial_gas: u64, #[case] gas_consumed: u64) { - let test_contract = FeatureContract::TestContract(CairoVersion::Cairo1(RunnableCairo1::Native)); - let mut state = test_state(&ChainInfo::create_for_testing(), BALANCE, &[(test_contract, 1)]); +// TODO (AvivG) move this test to Transaction executor - let depth = felt!(1000000_u128); - let entry_point_call = CallEntryPoint { - calldata: calldata![depth], - entry_point_selector: selector_from_name("test_stack_overflow"), - initial_gas, - ..trivial_external_entry_point_new(test_contract) - }; - let call_info = entry_point_call.execute_directly(&mut state).unwrap(); - assert_eq!( - call_info.execution, - CallExecution { - // 'Out of gas' - retdata: retdata![felt!["0x4f7574206f6620676173"]], - gas_consumed, - failed: true, - ..Default::default() - } - ); -} +// #[cfg(feature = "cairo_native")] +// #[cfg(feature = "cairo_native")] +// use crate::context::ChainInfo; +// #[cfg(feature = "cairo_native")] +// use crate::test_utils::initial_test_state::test_state; +// #[cfg(feature = "cairo_native")] +// use crate::test_utils::BALANCE; +// #[rstest::rstest] +// #[case(MAX_POSSIBLE_SIERRA_GAS, MAX_POSSIBLE_SIERRA_GAS - 2681170910)] +// #[case(MAX_POSSIBLE_SIERRA_GAS / 10, 81886490)] +// #[case(MAX_POSSIBLE_SIERRA_GAS / 100, 8190940)] +// #[case(MAX_POSSIBLE_SIERRA_GAS / 1000, 822890)] +// #[case(MAX_POSSIBLE_SIERRA_GAS / 10000, 85440)] +// #[case(MAX_POSSIBLE_SIERRA_GAS / 100000, 12340)] +// #[case(MAX_POSSIBLE_SIERRA_GAS / 1000000, 0)] +// #[case(350, 0)] +// #[case(35, 0)] +// #[case(0, 0)] +// /// Tests that Native can handle deep recursion calls without overflowing the stack. +// /// Note that the recursive function must be complicated, since the compiler might transform +// /// simple recursions into loops. The tested function was manually tested with higher gas and +// /// reached stack overflow. +// /// +// /// Also, there is no need to test the VM here since it doesn't use the stack. +// fn test_stack_overflow(#[case] initial_gas: u64, #[case] gas_consumed: u64) { +// let test_contract = +// FeatureContract::TestContract(CairoVersion::Cairo1(RunnableCairo1::Native)); let mut state = +// test_state(&ChainInfo::create_for_testing(), BALANCE, &[(test_contract, 1)]); + +// let depth = felt!(1000000_u128); +// let entry_point_call = CallEntryPoint { +// calldata: calldata![depth], +// entry_point_selector: selector_from_name("test_stack_overflow"), +// initial_gas, +// ..trivial_external_entry_point_new(test_contract) +// }; +// let call_info = entry_point_call.execute_directly(&mut state).unwrap(); +// assert_eq!( +// call_info.execution, +// CallExecution { +// // 'Out of gas' +// retdata: retdata![felt!["0x4f7574206f6620676173"]], +// gas_consumed, +// failed: true, +// ..Default::default() +// } +// ); +// } diff --git a/crates/blockifier/src/state/contract_class_manager.rs b/crates/blockifier/src/state/contract_class_manager.rs index 9e1f54707d..22a6297705 100644 --- a/crates/blockifier/src/state/contract_class_manager.rs +++ b/crates/blockifier/src/state/contract_class_manager.rs @@ -28,6 +28,7 @@ use crate::execution::native::contract_class::NativeCompiledClassV1; use crate::state::global_cache::CachedCairoNative; use crate::state::global_cache::{CachedCasm, ContractCaches}; pub const DEFAULT_COMPILATION_REQUEST_CHANNEL_SIZE: usize = 1000; +pub const DEFAULT_STACK_SIZE: usize = 60 * 1024 * 1024; /// Represents a request to compile a sierra contract class to a native compiled class. /// diff --git a/crates/blockifier/src/test_utils/transfers_generator.rs b/crates/blockifier/src/test_utils/transfers_generator.rs index 954caa75d0..81d7675ac1 100644 --- a/crates/blockifier/src/test_utils/transfers_generator.rs +++ b/crates/blockifier/src/test_utils/transfers_generator.rs @@ -14,6 +14,7 @@ use starknet_types_core::felt::Felt; use crate::blockifier::config::{ConcurrencyConfig, TransactionExecutorConfig}; use crate::blockifier::transaction_executor::TransactionExecutor; use crate::context::{BlockContext, ChainInfo}; +use crate::state::contract_class_manager::DEFAULT_STACK_SIZE; use crate::test_utils::contracts::FeatureContract; use crate::test_utils::dict_state_reader::DictStateReader; use crate::test_utils::initial_test_state::test_state; @@ -37,6 +38,7 @@ pub struct TransfersGeneratorConfig { pub tx_version: TransactionVersion, pub recipient_generator_type: RecipientGeneratorType, pub concurrency_config: ConcurrencyConfig, + pub stack_size: usize, } impl Default for TransfersGeneratorConfig { @@ -51,6 +53,7 @@ impl Default for TransfersGeneratorConfig { tx_version: TRANSACTION_VERSION, recipient_generator_type: RECIPIENT_GENERATOR_TYPE, concurrency_config: ConcurrencyConfig::create_for_testing(false), + stack_size: DEFAULT_STACK_SIZE, } } } @@ -79,8 +82,10 @@ impl TransfersGenerator { let chain_info = block_context.chain_info().clone(); let state = test_state(&chain_info, config.balance, &[(account_contract, config.n_accounts)]); - let executor_config = - TransactionExecutorConfig { concurrency_config: config.concurrency_config.clone() }; + let executor_config = TransactionExecutorConfig { + concurrency_config: config.concurrency_config.clone(), + stack_size: config.stack_size, + }; let executor = TransactionExecutor::new(state, block_context, executor_config); let account_addresses = (0..config.n_accounts) .map(|instance_id| account_contract.get_instance_address(instance_id)) diff --git a/crates/native_blockifier/src/py_block_executor.rs b/crates/native_blockifier/src/py_block_executor.rs index 6fba424188..9b50a8480d 100644 --- a/crates/native_blockifier/src/py_block_executor.rs +++ b/crates/native_blockifier/src/py_block_executor.rs @@ -123,7 +123,7 @@ pub struct PyBlockExecutor { #[pymethods] impl PyBlockExecutor { #[new] - #[pyo3(signature = (bouncer_config, concurrency_config, contract_class_manager_config, os_config, target_storage_config, py_versioned_constants_overrides))] + #[pyo3(signature = (bouncer_config, concurrency_config, contract_class_manager_config, os_config, target_storage_config, py_versioned_constants_overrides, stack_size))] pub fn create( bouncer_config: PyBouncerConfig, concurrency_config: PyConcurrencyConfig, @@ -131,6 +131,7 @@ impl PyBlockExecutor { os_config: PyOsConfig, target_storage_config: StorageConfig, py_versioned_constants_overrides: PyVersionedConstantsOverrides, + stack_size: usize, ) -> Self { log::debug!("Initializing Block Executor..."); let storage = @@ -143,6 +144,7 @@ impl PyBlockExecutor { bouncer_config: bouncer_config.try_into().expect("Failed to parse bouncer config."), tx_executor_config: TransactionExecutorConfig { concurrency_config: concurrency_config.into(), + stack_size, }, chain_info: os_config.into_chain_info(), versioned_constants, @@ -352,7 +354,7 @@ impl PyBlockExecutor { self.storage.close(); } - #[pyo3(signature = (concurrency_config, contract_class_manager_config, os_config, path, max_state_diff_size))] + #[pyo3(signature = (concurrency_config, contract_class_manager_config, os_config, path, max_state_diff_size, stack_size))] #[staticmethod] fn create_for_testing( concurrency_config: PyConcurrencyConfig, @@ -360,6 +362,7 @@ impl PyBlockExecutor { os_config: PyOsConfig, path: std::path::PathBuf, max_state_diff_size: usize, + stack_size: usize, ) -> Self { use blockifier::bouncer::BouncerWeights; // TODO(Meshi, 01/01/2025): Remove this once we fix all python tests that re-declare cairo0 @@ -375,6 +378,7 @@ impl PyBlockExecutor { }, tx_executor_config: TransactionExecutorConfig { concurrency_config: concurrency_config.into(), + stack_size, }, storage: Box::new(PapyrusStorage::new_for_testing(path, &os_config.chain_id)), chain_info: os_config.into_chain_info(), @@ -423,6 +427,7 @@ impl PyBlockExecutor { os_config: PyOsConfig, path: std::path::PathBuf, max_state_diff_size: usize, + stack_size: usize, ) -> Self { Self::create_for_testing( concurrency_config, @@ -430,6 +435,7 @@ impl PyBlockExecutor { os_config, path, max_state_diff_size, + stack_size, ) } } diff --git a/crates/native_blockifier/src/py_block_executor_test.rs b/crates/native_blockifier/src/py_block_executor_test.rs index abbe6ee24e..819ab2abb0 100644 --- a/crates/native_blockifier/src/py_block_executor_test.rs +++ b/crates/native_blockifier/src/py_block_executor_test.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; use blockifier::blockifier::transaction_executor::BLOCK_STATE_ACCESS_ERR; use blockifier::execution::contract_class::{CompiledClassV1, RunnableCompiledClass}; +use blockifier::state::contract_class_manager::DEFAULT_STACK_SIZE; use blockifier::state::state_api::StateReader; use cairo_lang_starknet_classes::casm_contract_class::CasmContractClass; use pretty_assertions::assert_eq; @@ -48,6 +49,7 @@ fn global_contract_cache_update() { PyOsConfig::default(), temp_storage_path, 4000, + DEFAULT_STACK_SIZE, ); block_executor .append_block( @@ -129,6 +131,7 @@ fn global_contract_cache_update_large_contract() { Default::default(), temp_storage_path, 4000, + DEFAULT_STACK_SIZE, ); block_executor .append_block(