From 9f90914b07a92f492d1908bec96e2358262cc99b Mon Sep 17 00:00:00 2001 From: Piotr Figiela <77412592+Draggu@users.noreply.github.com> Date: Wed, 21 Feb 2024 11:56:49 +0100 Subject: [PATCH] Raise steps limit (#1735) Closes #1652 ## Introduced changes - Added option `--max-n-steps` that allow changing steps limit (3_000_000, now default) ## Checklist - [x] Linked relevant issue - [x] Updated relevant documentation - [x] Added relevant tests - [x] Performed self-review of the code - [x] Added changes to `CHANGELOG.md` --- CHANGELOG.md | 1 + crates/forge-runner/src/lib.rs | 26 ++-- crates/forge-runner/src/running.rs | 7 +- crates/forge/src/main.rs | 20 ++- crates/forge/src/scarb.rs | 1 + crates/forge/src/scarb/config.rs | 5 + crates/forge/test_utils/src/running_tests.rs | 1 + crates/forge/tests/data/steps/Scarb.toml | 12 ++ crates/forge/tests/data/steps/src/lib.cairo | 63 ++++++++++ crates/forge/tests/e2e/mod.rs | 1 + crates/forge/tests/e2e/steps.rs | 125 +++++++++++++++++++ crates/forge/tests/integration/setup_fork.rs | 1 + crates/runtime/src/starknet/context.rs | 18 ++- docs/src/appendix/snforge/test.md | 4 + 14 files changed, 265 insertions(+), 20 deletions(-) create mode 100644 crates/forge/tests/data/steps/Scarb.toml create mode 100644 crates/forge/tests/data/steps/src/lib.cairo create mode 100644 crates/forge/tests/e2e/steps.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 412155716b..666678d61b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 #### Added - contract names to call trace +- `--max-n-steps` argument that allows setting own steps limit #### Changed diff --git a/crates/forge-runner/src/lib.rs b/crates/forge-runner/src/lib.rs index 1ba5dee48f..fe080680d0 100644 --- a/crates/forge-runner/src/lib.rs +++ b/crates/forge-runner/src/lib.rs @@ -17,7 +17,6 @@ use contracts_data::ContractsData; use futures::stream::FuturesUnordered; use futures::StreamExt; -use once_cell::sync::Lazy; use smol_str::SmolStr; use trace_data::save_trace_data; @@ -42,18 +41,16 @@ mod running; pub const CACHE_DIR: &str = ".snfoundry_cache"; -pub static BUILTINS: Lazy> = Lazy::new(|| { - vec![ - "Pedersen", - "RangeCheck", - "Bitwise", - "EcOp", - "Poseidon", - "SegmentArena", - "GasBuiltin", - "System", - ] -}); +pub const BUILTINS: [&str; 8] = [ + "Pedersen", + "RangeCheck", + "Bitwise", + "EcOp", + "Poseidon", + "SegmentArena", + "GasBuiltin", + "System", +]; /// Configuration of the test runner #[derive(Debug, PartialEq)] @@ -65,6 +62,7 @@ pub struct RunnerConfig { pub fuzzer_seed: u64, pub detailed_resources: bool, pub save_trace_data: bool, + pub max_n_steps: Option, } impl RunnerConfig { @@ -77,6 +75,7 @@ impl RunnerConfig { fuzzer_seed: u64, detailed_resources: bool, save_trace_data: bool, + max_n_steps: Option, ) -> Self { Self { workspace_root, @@ -85,6 +84,7 @@ impl RunnerConfig { fuzzer_seed, detailed_resources, save_trace_data, + max_n_steps, } } } diff --git a/crates/forge-runner/src/running.rs b/crates/forge-runner/src/running.rs index 6bcd22adf5..cc61f4cedc 100644 --- a/crates/forge-runner/src/running.rs +++ b/crates/forge-runner/src/running.rs @@ -40,7 +40,7 @@ use cheatnet::runtime_extensions::forge_runtime_extension::{ }; use cheatnet::state::{BlockInfoReader, CallTrace, CheatnetState, ExtendedStateReader}; use itertools::chain; -use runtime::starknet::context::build_context; +use runtime::starknet::context::{build_context, set_max_steps}; use runtime::{ExtendedRuntime, StarknetRuntime}; use tokio::sync::mpsc::Sender; use tokio::task::JoinHandle; @@ -188,6 +188,11 @@ pub fn run_test_case( let block_info = state_reader.get_block_info()?; let mut context = build_context(block_info); + + if let Some(max_n_steps) = runner_config.max_n_steps { + set_max_steps(&mut context, max_n_steps); + } + let mut execution_resources = ExecutionResources::default(); let mut cached_state = CachedState::from(state_reader); let syscall_handler = build_syscall_handler( diff --git a/crates/forge/src/main.rs b/crates/forge/src/main.rs index 33c6fd6dae..f2c628e41f 100644 --- a/crates/forge/src/main.rs +++ b/crates/forge/src/main.rs @@ -108,6 +108,10 @@ struct TestArgs { /// Save execution traces of all test which have passed and are not fuzz tests #[arg(long)] save_trace_data: bool, + + /// Number of maximum steps during a single test. For fuzz tests this value is applied to each subtest separately. + #[arg(long)] + max_n_steps: Option, } fn validate_fuzzer_runs_value(val: &str) -> Result { @@ -134,6 +138,7 @@ fn extract_failed_tests(tests_summaries: Vec) -> Vec, detailed_resources: bool, save_trace_data: bool, + max_n_steps: Option, forge_config: &ForgeConfig, ) -> RunnerConfig { RunnerConfig::new( @@ -154,6 +160,7 @@ fn combine_configs( .unwrap_or_else(|| thread_rng().next_u64()), detailed_resources || forge_config.detailed_resources, save_trace_data || forge_config.save_trace_data, + max_n_steps.or(forge_config.max_n_steps), ) } @@ -241,6 +248,7 @@ fn test_workspace(args: TestArgs) -> Result { args.fuzzer_seed, args.detailed_resources, args.save_trace_data, + args.max_n_steps, &forge_config, )); let runner_params = @@ -326,6 +334,7 @@ mod tests { None, false, false, + None, &Default::default(), ); let config2 = combine_configs( @@ -335,6 +344,7 @@ mod tests { None, false, false, + None, &Default::default(), ); @@ -353,6 +363,7 @@ mod tests { None, false, false, + None, &Default::default(), ); assert_eq!( @@ -364,6 +375,7 @@ mod tests { config.fuzzer_seed, false, false, + None ) ); } @@ -377,6 +389,7 @@ mod tests { fuzzer_seed: Some(500), detailed_resources: true, save_trace_data: true, + max_n_steps: Some(1_000_000), }; let workspace_root: Utf8PathBuf = Default::default(); @@ -387,11 +400,12 @@ mod tests { None, false, false, + None, &config_from_scarb, ); assert_eq!( config, - RunnerConfig::new(workspace_root, true, 1234, 500, true, true) + RunnerConfig::new(workspace_root, true, 1234, 500, true, true, Some(1_000_000)) ); } @@ -406,6 +420,7 @@ mod tests { fuzzer_seed: Some(1000), detailed_resources: false, save_trace_data: false, + max_n_steps: Some(1234), }; let config = combine_configs( &workspace_root, @@ -414,12 +429,13 @@ mod tests { Some(32), true, true, + Some(1_000_000), &config_from_scarb, ); assert_eq!( config, - RunnerConfig::new(workspace_root, true, 100, 32, true, true) + RunnerConfig::new(workspace_root, true, 100, 32, true, true, Some(1_000_000)) ); } } diff --git a/crates/forge/src/scarb.rs b/crates/forge/src/scarb.rs index d6ae75e252..242ccd1589 100644 --- a/crates/forge/src/scarb.rs +++ b/crates/forge/src/scarb.rs @@ -176,6 +176,7 @@ mod tests { ], fuzzer_runs: None, fuzzer_seed: None, + max_n_steps: None, detailed_resources: false, save_trace_data: false } diff --git a/crates/forge/src/scarb/config.rs b/crates/forge/src/scarb/config.rs index a2a8a4f2bb..9218080a4d 100644 --- a/crates/forge/src/scarb/config.rs +++ b/crates/forge/src/scarb/config.rs @@ -19,6 +19,8 @@ pub struct ForgeConfig { pub save_trace_data: bool, /// Fork configuration profiles pub fork: Vec, + /// Limit of steps + pub max_n_steps: Option, } #[derive(Debug, PartialEq, Clone)] @@ -63,6 +65,8 @@ pub(crate) struct RawForgeConfig { #[serde(default)] /// Fork configuration profiles pub fork: Vec, + /// Limit of steps + pub max_n_steps: Option, } #[derive(Deserialize, Debug, PartialEq, Default, Clone)] @@ -130,6 +134,7 @@ impl TryFrom for ForgeConfig { detailed_resources: value.detailed_resources, save_trace_data: value.save_trace_data, fork: fork_targets, + max_n_steps: value.max_n_steps, }) } } diff --git a/crates/forge/test_utils/src/running_tests.rs b/crates/forge/test_utils/src/running_tests.rs index 2e0e175292..898c01d169 100644 --- a/crates/forge/test_utils/src/running_tests.rs +++ b/crates/forge/test_utils/src/running_tests.rs @@ -37,6 +37,7 @@ pub fn run_test_case(test: &TestCase) -> Vec { 12345, false, false, + None, )), Arc::new(RunnerParams::new( ContractsData::try_from(test.contracts().unwrap()).unwrap(), diff --git a/crates/forge/tests/data/steps/Scarb.toml b/crates/forge/tests/data/steps/Scarb.toml new file mode 100644 index 0000000000..2af22b0283 --- /dev/null +++ b/crates/forge/tests/data/steps/Scarb.toml @@ -0,0 +1,12 @@ +[package] +name = "steps" +version = "0.1.0" + +# See more keys and their definitions at https://docs.swmansion.com/scarb/docs/reference/manifest.html + +[dependencies] +starknet = "2.4.0" +snforge_std = { path = "../../../../../snforge_std" } + +[[target.starknet-contract]] +sierra = true diff --git a/crates/forge/tests/data/steps/src/lib.cairo b/crates/forge/tests/data/steps/src/lib.cairo new file mode 100644 index 0000000000..5ce747df48 --- /dev/null +++ b/crates/forge/tests/data/steps/src/lib.cairo @@ -0,0 +1,63 @@ +#[cfg(test)] +mod tests { + #[test] + // requires 570031 steps + fn steps_570031() { + let mut i = 0; + + loop { + if i == 30_000 { + break; + } + + i = i + 1; + + assert(1 + 1 == 2, 'who knows?'); + } + } + + #[test] + fn steps_5700031() { + let mut i = 0; + + loop { + if i == 300_000 { + break; + } + + i = i + 1; + + assert(1 + 1 == 2, 'who knows?'); + } + } + + #[test] + fn steps_2999998() { + let mut i = 0; + + loop { + if i == 157_893 { + break; + } + + i = i + 1; + + assert(1 + 1 == 2, 'who knows?'); + } + } + + #[test] + fn steps_3000017() { + let mut i = 0; + + loop { + if i == 157_894 { + break; + } + + i = i + 1; + + assert(1 + 1 == 2, 'who knows?'); + } + } +} diff --git a/crates/forge/tests/e2e/mod.rs b/crates/forge/tests/e2e/mod.rs index 9cb015248a..68fb42d4bc 100644 --- a/crates/forge/tests/e2e/mod.rs +++ b/crates/forge/tests/e2e/mod.rs @@ -10,6 +10,7 @@ mod forking; mod fuzzing; mod io_operations; mod running; +mod steps; mod trace_data; mod trace_print; mod trace_resources; diff --git a/crates/forge/tests/e2e/steps.rs b/crates/forge/tests/e2e/steps.rs new file mode 100644 index 0000000000..97cca6eece --- /dev/null +++ b/crates/forge/tests/e2e/steps.rs @@ -0,0 +1,125 @@ +use crate::e2e::common::runner::{setup_package, test_runner}; +use indoc::indoc; +use test_utils::output_assert::assert_stdout_contains; + +#[test] +fn should_allow_less_than_default() { + let temp = setup_package("steps"); + let snapbox = test_runner(); + + let output = snapbox + .current_dir(&temp) + .args(["--max-n-steps", "100000"]) + .assert() + .code(1); + + assert_stdout_contains( + output, + indoc!( + r" + [..]Compiling[..] + [..]Finished[..] + + + Collected 4 test(s) from steps package + Running 4 test(s) from src/ + [FAIL] steps::tests::steps_3000017 + + Failure data: + Could not reach the end of the program. RunResources has no remaining steps. + + [FAIL] steps::tests::steps_5700031 + + Failure data: + Could not reach the end of the program. RunResources has no remaining steps. + + [FAIL] steps::tests::steps_2999998 + + Failure data: + Could not reach the end of the program. RunResources has no remaining steps. + + [FAIL] steps::tests::steps_570031 + + Failure data: + Could not reach the end of the program. RunResources has no remaining steps. + + Tests: 0 passed, 4 failed, 0 skipped, 0 ignored, 0 filtered out + + Failures: + steps::tests::steps_3000017 + steps::tests::steps_5700031 + steps::tests::steps_2999998 + steps::tests::steps_570031 + " + ), + ); +} +#[test] +// 4_000_000 is blockifier limit we want to omit +fn should_allow_more_than_4kk() { + let temp = setup_package("steps"); + let snapbox = test_runner(); + + let output = snapbox + .current_dir(&temp) + .args(["--max-n-steps", "5700031"]) + .assert() + .code(0); + + assert_stdout_contains( + output, + indoc!( + r" + [..]Compiling[..] + [..]Finished[..] + + + Collected 4 test(s) from steps package + Running 4 test(s) from src/ + [PASS] steps::tests::steps_570031 [..] + [PASS] steps::tests::steps_3000017 [..] + [PASS] steps::tests::steps_2999998 [..] + [PASS] steps::tests::steps_5700031 [..] + Tests: 4 passed, 0 failed, 0 skipped, 0 ignored, 0 filtered out + " + ), + ); +} +#[test] +fn should_default_to_3kk() { + let temp = setup_package("steps"); + let snapbox = test_runner(); + + let output = snapbox.current_dir(&temp).assert().code(1); + + assert_stdout_contains( + output, + indoc!( + r" + [..]Compiling[..] + [..]Finished[..] + + + Collected 4 test(s) from steps package + Running 4 test(s) from src/ + [PASS] steps::tests::steps_570031 [..] + [FAIL] steps::tests::steps_3000017 + + Failure data: + Could not reach the end of the program. RunResources has no remaining steps. + + [FAIL] steps::tests::steps_5700031 + + Failure data: + Could not reach the end of the program. RunResources has no remaining steps. + + [PASS] steps::tests::steps_2999998 [..] + Tests: 2 passed, 2 failed, 0 skipped, 0 ignored, 0 filtered out + + Failures: + steps::tests::steps_3000017 + steps::tests::steps_5700031 + " + ), + ); +} diff --git a/crates/forge/tests/integration/setup_fork.rs b/crates/forge/tests/integration/setup_fork.rs index b5663692bf..ce48c4ba91 100644 --- a/crates/forge/tests/integration/setup_fork.rs +++ b/crates/forge/tests/integration/setup_fork.rs @@ -125,6 +125,7 @@ fn fork_aliased_decorator() { 12345, false, false, + None, )), Arc::new(RunnerParams::new( ContractsData::try_from(test.contracts().unwrap()).unwrap(), diff --git a/crates/runtime/src/starknet/context.rs b/crates/runtime/src/starknet/context.rs index e3099a92d4..8ae6b2fa0a 100644 --- a/crates/runtime/src/starknet/context.rs +++ b/crates/runtime/src/starknet/context.rs @@ -16,7 +16,9 @@ use cairo_vm::vm::runners::builtin_runner::{ OUTPUT_BUILTIN_NAME, POSEIDON_BUILTIN_NAME, RANGE_CHECK_BUILTIN_NAME, SEGMENT_ARENA_BUILTIN_NAME, SIGNATURE_BUILTIN_NAME, }; +use cairo_vm::vm::runners::cairo_runner::RunResources; use serde::{Deserialize, Serialize}; +use starknet_api::contract_address; use starknet_api::data_availability::DataAvailabilityMode; use starknet_api::block::{BlockNumber, BlockTimestamp}; @@ -31,11 +33,12 @@ use starknet_api::{ pub const SEQUENCER_ADDRESS: &str = "0x1000"; pub const ERC20_CONTRACT_ADDRESS: &str = "0x1001"; pub const STEP_RESOURCE_COST: f64 = 0.005_f64; +pub const DEFAULT_MAX_N_STEPS: u32 = 3_000_000; // HOW TO FIND: // 1. https://docs.starknet.io/documentation/architecture_and_concepts/Network_Architecture/fee-mechanism/#calculation_of_computation_costs #[must_use] -pub fn build_block_context(block_info: BlockInfo) -> BlockContext { +fn build_block_context(block_info: BlockInfo) -> BlockContext { // blockifier::test_utils::create_for_account_testing let vm_resource_fee_cost = Arc::new(HashMap::from([ (constants::N_STEPS_RESOURCE.to_string(), STEP_RESOURCE_COST), @@ -77,12 +80,12 @@ pub fn build_block_context(block_info: BlockInfo) -> BlockContext { block_timestamp: block_info.timestamp, sequencer_address: block_info.sequencer_address, vm_resource_fee_cost, - invoke_tx_max_n_steps: 3_000_000, + invoke_tx_max_n_steps: DEFAULT_MAX_N_STEPS, validate_max_n_steps: 1_000_000, max_recursion_depth: 50, fee_token_addresses: FeeTokenAddresses { - strk_fee_token_address: ContractAddress(patricia_key!(ERC20_CONTRACT_ADDRESS)), - eth_fee_token_address: ContractAddress(patricia_key!(ERC20_CONTRACT_ADDRESS)), + strk_fee_token_address: contract_address!(ERC20_CONTRACT_ADDRESS), + eth_fee_token_address: contract_address!(ERC20_CONTRACT_ADDRESS), }, gas_prices: GasPrices { eth_l1_gas_price: 100 * u128::pow(10, 9), @@ -140,6 +143,13 @@ pub fn build_context(block_info: BlockInfo) -> EntryPointExecutionContext { .unwrap() } +pub fn set_max_steps(entry_point_ctx: &mut EntryPointExecutionContext, max_n_steps: u32) { + entry_point_ctx.block_context.invoke_tx_max_n_steps = max_n_steps; + + // override it to omit [`EntryPointExecutionContext::max_steps`] restrictions + entry_point_ctx.vm_run_resources = RunResources::new(max_n_steps as usize); +} + #[derive(Copy, Clone, Serialize, Deserialize, Debug)] pub struct BlockInfo { pub block_number: BlockNumber, diff --git a/docs/src/appendix/snforge/test.md b/docs/src/appendix/snforge/test.md index 1f48bb4967..7fe5d39639 100644 --- a/docs/src/appendix/snforge/test.md +++ b/docs/src/appendix/snforge/test.md @@ -60,6 +60,10 @@ Display additional info about used resources for passed tests. Saves execution traces of test cases which have passed and are not fuzz tests to files. Traces can be used for profiling purposes. +## `--max-n-steps` `` + +Number of maximum steps during a single test. For fuzz tests this value is applied to each subtest separately. + ## `-h`, `--help` Print help.