From 3833083871dc71da097d0dfb323a39b9ed188367 Mon Sep 17 00:00:00 2001 From: Leo-Besancon Date: Wed, 16 Oct 2024 11:19:29 +0200 Subject: [PATCH] Add SC recursion limit (#4729) * Asc message execution - requery message bytecode after each message execution (#4710) * Requery bytecode * cargo fmt * fix call stack inconsistency (#4709) * Improve async message checks (#4706) * Improve async message checks * Change checks for async messages * Add unit tests * Fix ledger change to take into account cancelled message balance change (#4715) * Take again the speculative changes after async message cancellation * use .apply() to merge the two LedgerChanges * Fix: we cannot combine two ledger changes with apply * avoid cloning the changes * Remove comment * Fix async msg same slot (#4718) * fix open rpc spec (#4716) * Add eliminated_new_messages in eliminated_msg --------- Co-authored-by: Modship * Add initial code for recursion limit * Latest runtime * Run CI on PRs based on mainnet_2_3 * fmt * Fix config and add UTs * Update scenarios_mandatories.rs * Review comments (CI for all branches starting with "mainnet_" + comment) * Update ci.yml * Remove manual increment / decrement in interface implementation * fmt + update sc_runtime + fix warning * Update test * Update constants.rs * Updated execution config for tests * Updated usize -> u16 for recursion counter and limits * Update test comments * Add comments regarding the needs of this limits * Update sc-runtime branch --------- Co-authored-by: Modship --- Cargo.lock | 32 +-- massa-execution-exports/src/settings.rs | 3 + .../src/test_exports/config.rs | 1 + massa-execution-worker/src/context.rs | 11 ++ massa-execution-worker/src/interface_impl.rs | 23 +++ .../src/tests/scenarios_mandatories.rs | 183 ++++++++++++++++++ massa-models/src/config/constants.rs | 2 + massa-node/src/main.rs | 7 +- 8 files changed, 243 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6172a1b36d9..051c0afba0b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1120,9 +1120,9 @@ dependencies = [ [[package]] name = "dashmap" -version = "6.0.1" +version = "6.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "804c8821570c3f8b70230c2ba75ffa5c0f9a4189b9a432b6656c536712acae28" +checksum = "5041cc499144891f3790297212f32a74fb938e5136a14943f338ef9e0ae276cf" dependencies = [ "cfg-if", "crossbeam-utils", @@ -1513,9 +1513,9 @@ checksum = "27573eac26f4dd11e2b1916c3fe1baa56407c83c71a773a8ba17ec0bca03b6b7" [[package]] name = "filetime" -version = "0.2.24" +version = "0.2.25" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bf401df4a4e3872c4fe8151134cf483738e74b67fc934d6532c882b3d24a4550" +checksum = "35c0522e981e68cbfa8c3f978441a5f34b30b96e146b33cd3359176b50fe8586" dependencies = [ "cfg-if", "libc", @@ -2532,7 +2532,7 @@ checksum = "c0ff37bd590ca25063e35af745c343cb7a0271906fb7b37e4813e8f79f00268d" dependencies = [ "bitflags 2.4.1", "libc", - "redox_syscall 0.5.3", + "redox_syscall 0.5.7", ] [[package]] @@ -2796,7 +2796,7 @@ dependencies = [ [[package]] name = "massa-sc-runtime" version = "0.10.0" -source = "git+https://github.com/massalabs/massa-sc-runtime?branch=next_breaking_update#e8040e69f3f4f31b7ab57c30af4aa87820788950" +source = "git+https://github.com/massalabs/massa-sc-runtime?branch=next_breaking_update#095d6253bf6c31e725f056c79b4df9994f39380e" dependencies = [ "anyhow", "as-ffi-bindings", @@ -4465,7 +4465,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0c1318b19085f08681016926435853bbf7858f9c082d0999b80550ff5d9abe15" dependencies = [ "bytes", - "heck 0.4.1", + "heck 0.5.0", "itertools 0.12.0", "log", "multimap", @@ -4674,9 +4674,9 @@ dependencies = [ [[package]] name = "redox_syscall" -version = "0.5.3" +version = "0.5.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2a908a6e00f1fdd0dfd9c0eb08ce85126f6d8bbda50017e74bc4a4b7d4a926a4" +checksum = "9b6dfecf2c74bce2466cabf93f6664d6998a69eb21e39f4207930065b27b771f" dependencies = [ "bitflags 2.4.1", ] @@ -5252,9 +5252,9 @@ dependencies = [ [[package]] name = "serde_spanned" -version = "0.6.7" +version = "0.6.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eb5b1b31579f3811bf615c144393417496f152e12ac8b7663bf664f4a815306d" +checksum = "87607cb1398ed59d48732e575a4c28a7a8ebf2454b964fe3f224f2afc07909e1" dependencies = [ "serde", ] @@ -5624,9 +5624,9 @@ checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369" [[package]] name = "tar" -version = "0.4.41" +version = "0.4.42" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cb797dad5fb5b76fcf519e702f4a589483b5ef06567f160c392832c1f5e44909" +checksum = "4ff6c40d3aedb5e06b57c6f669ad17ab063dd1e63d977c6a88e7f4dfa4f04020" dependencies = [ "filetime", "libc", @@ -6570,7 +6570,7 @@ dependencies = [ "cfg-if", "corosensei", "crossbeam-queue", - "dashmap 6.0.1", + "dashmap 6.1.0", "derivative", "enum-iterator", "fnv", @@ -6631,9 +6631,9 @@ dependencies = [ [[package]] name = "webc" -version = "6.0.0-rc2" +version = "6.0.0-rc3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fb3e2ccb43d303c5bd48f31db7a129481a9aaa5343d623f92951751df190df81" +checksum = "b85ffb11d1fabf0ebfc458a3d1d34ccf6d4d9596ca7576370cae4eab554c63d1" dependencies = [ "anyhow", "base64 0.22.1", diff --git a/massa-execution-exports/src/settings.rs b/massa-execution-exports/src/settings.rs index 4998cbc3b24..f62cc08ae01 100644 --- a/massa-execution-exports/src/settings.rs +++ b/massa-execution-exports/src/settings.rs @@ -102,6 +102,9 @@ pub struct ExecutionConfig { pub max_execution_traces_slot_limit: usize, /// Where to dump blocks pub block_dump_folder_path: PathBuf, + /// Max recursive calls depth in SC + /// Used to limit the recursion_counter value in the context, to avoid stack overflow issues. + pub max_recursive_calls_depth: u16, /// Runtime condom middleware limits pub condom_limits: CondomLimits, } diff --git a/massa-execution-exports/src/test_exports/config.rs b/massa-execution-exports/src/test_exports/config.rs index c583cb6d6fb..c0d51bd747c 100644 --- a/massa-execution-exports/src/test_exports/config.rs +++ b/massa-execution-exports/src/test_exports/config.rs @@ -76,6 +76,7 @@ impl Default for ExecutionConfig { broadcast_slot_execution_traces_channel_capacity: 5000, max_execution_traces_slot_limit: 320, block_dump_folder_path, + max_recursive_calls_depth: 25, condom_limits: CondomLimits { max_exports: Some(100), max_functions: Some(100), diff --git a/massa-execution-worker/src/context.rs b/massa-execution-worker/src/context.rs index 60e74219d6f..54f65417f60 100644 --- a/massa-execution-worker/src/context.rs +++ b/massa-execution-worker/src/context.rs @@ -91,6 +91,11 @@ pub struct ExecutionContextSnapshot { /// The gas remaining before the last subexecution. /// so *excluding* the gas used by the last sc call. pub gas_remaining_before_subexecution: Option, + + /// recursion counter, incremented for each new nested call + /// This is used to avoid stack overflow issues in the VM (that would crash the node instead of failing the call), + /// by limiting the depth of recursion contracts can have with the max_recursive_calls_depth value. + pub recursion_counter: u16, } /// An execution context that needs to be initialized before executing bytecode, @@ -179,6 +184,9 @@ pub struct ExecutionContext { /// The gas remaining before the last subexecution. /// so *excluding* the gas used by the last sc call. pub gas_remaining_before_subexecution: Option, + + /// recursion counter, incremented for each new nested call + pub recursion_counter: u16, } impl ExecutionContext { @@ -244,6 +252,7 @@ impl ExecutionContext { address_factory: AddressFactory { mip_store }, execution_trail_hash, gas_remaining_before_subexecution: None, + recursion_counter: 0, } } @@ -265,6 +274,7 @@ impl ExecutionContext { event_count: self.events.0.len(), unsafe_rng: self.unsafe_rng.clone(), gas_remaining_before_subexecution: self.gas_remaining_before_subexecution, + recursion_counter: self.recursion_counter, } } @@ -293,6 +303,7 @@ impl ExecutionContext { self.stack = snapshot.stack; self.unsafe_rng = snapshot.unsafe_rng; self.gas_remaining_before_subexecution = snapshot.gas_remaining_before_subexecution; + self.recursion_counter = snapshot.recursion_counter; // For events, set snapshot delta to error events. for event in self.events.0.range_mut(snapshot.event_count..) { diff --git a/massa-execution-worker/src/interface_impl.rs b/massa-execution-worker/src/interface_impl.rs index 65ae41b08ae..4f55eaef0cc 100644 --- a/massa-execution-worker/src/interface_impl.rs +++ b/massa-execution-worker/src/interface_impl.rs @@ -227,6 +227,29 @@ impl Interface for InterfaceImpl { Ok(()) } + fn increment_recursion_counter(&self) -> Result<()> { + let mut context = context_guard!(self); + + context.recursion_counter += 1; + + if context.recursion_counter > self.config.max_recursive_calls_depth { + bail!("recursion depth limit reached"); + } + + Ok(()) + } + + fn decrement_recursion_counter(&self) -> Result<()> { + let mut context = context_guard!(self); + + match context.recursion_counter.checked_sub(1) { + Some(value) => context.recursion_counter = value, + None => bail!("recursion counter underflow"), + } + + Ok(()) + } + /// Initialize the call when bytecode calls a function from another bytecode /// This function transfers the coins passed as parameter, /// prepares the current execution context by pushing a new element on the top of the call stack, diff --git a/massa-execution-worker/src/tests/scenarios_mandatories.rs b/massa-execution-worker/src/tests/scenarios_mandatories.rs index f9009270085..b811f83235e 100644 --- a/massa-execution-worker/src/tests/scenarios_mandatories.rs +++ b/massa-execution-worker/src/tests/scenarios_mandatories.rs @@ -449,6 +449,189 @@ fn test_nested_call_gas_usage() { ); } +/// Test the recursion depth limit in nested calls using call SC operation +/// +/// We call a smart contract that has a nested function call, while setting the max_recursive_calls_depth to 0. +/// We expect the execution of the smart contract call to fail with a message that the recursion depth limit was reached. +#[test] +fn test_nested_call_recursion_limit_reached() { + // setup the period duration + let exec_cfg = ExecutionConfig { + max_recursive_calls_depth: 0, // This limit will be reached + ..Default::default() + }; + + let finalized_waitpoint = WaitPoint::new(); + let mut foreign_controllers = ExecutionForeignControllers::new_with_mocks(); + selector_boilerplate(&mut foreign_controllers.selector_controller); + + foreign_controllers + .ledger_controller + .set_expectations(|ledger_controller| { + ledger_controller + .expect_get_balance() + .returning(move |_| Some(Amount::from_str("100").unwrap())); + + ledger_controller + .expect_entry_exists() + .times(2) + .returning(move |_| false); + + ledger_controller + .expect_entry_exists() + .times(1) + .returning(move |_| true); + }); + let saved_bytecode = expect_finalize_deploy_and_call_blocks( + Slot::new(1, 0), + Some(Slot::new(1, 1)), + finalized_waitpoint.get_trigger_handle(), + &mut foreign_controllers.final_state, + ); + final_state_boilerplate( + &mut foreign_controllers.final_state, + foreign_controllers.db.clone(), + &foreign_controllers.selector_controller, + &mut foreign_controllers.ledger_controller, + Some(saved_bytecode), + None, + None, + ); + let mut universe = ExecutionTestUniverse::new(foreign_controllers, exec_cfg); + + // load bytecodes + universe.deploy_bytecode_block( + &KeyPair::from_str(TEST_SK_1).unwrap(), + Slot::new(1, 0), + include_bytes!("./wasm/nested_call.wasm"), + include_bytes!("./wasm/test.wasm"), + ); + finalized_waitpoint.wait(); + let address = universe.get_address_sc_deployed(Slot::new(1, 0)); + + // Call the function test of the smart contract + let operation = ExecutionTestUniverse::create_call_sc_operation( + &KeyPair::from_str(TEST_SK_2).unwrap(), + 10000000, + Amount::from_str("0").unwrap(), + Amount::from_str("0").unwrap(), + Address::from_str(&address).unwrap(), + String::from("test"), + address.as_bytes().to_vec(), + ) + .unwrap(); + universe.call_sc_block( + &KeyPair::from_str(TEST_SK_2).unwrap(), + Slot::new(1, 1), + operation, + ); + finalized_waitpoint.wait(); + + // Get the events of the smart contract execution. We expect the call to have failed, so we check for the error message. + let events = universe + .module_controller + .get_filtered_sc_output_event(EventFilter { + start: Some(Slot::new(1, 1)), + ..Default::default() + }); + assert!(events.len() >= 2); + //println!("events: {:?}", events); + assert!(events[1].data.contains("recursion depth limit reached")); +} + +/// Test the recursion depth limit in nested calls using call SC operation +/// +/// We call a smart contract that has a nested function call, while setting the max_recursive_calls_depth to 2. +/// We expect the execution of the smart contract call to succeed as the recursion depth limit was not reached. +#[test] +fn test_nested_call_recursion_limit_not_reached() { + // setup the period duration + let exec_cfg = ExecutionConfig { + max_recursive_calls_depth: 2, // This limit will not be reached + ..Default::default() + }; + + let finalized_waitpoint = WaitPoint::new(); + let mut foreign_controllers = ExecutionForeignControllers::new_with_mocks(); + selector_boilerplate(&mut foreign_controllers.selector_controller); + + foreign_controllers + .ledger_controller + .set_expectations(|ledger_controller| { + ledger_controller + .expect_get_balance() + .returning(move |_| Some(Amount::from_str("100").unwrap())); + + ledger_controller + .expect_entry_exists() + .times(2) + .returning(move |_| false); + + ledger_controller + .expect_entry_exists() + .times(1) + .returning(move |_| true); + }); + let saved_bytecode = expect_finalize_deploy_and_call_blocks( + Slot::new(1, 0), + Some(Slot::new(1, 1)), + finalized_waitpoint.get_trigger_handle(), + &mut foreign_controllers.final_state, + ); + final_state_boilerplate( + &mut foreign_controllers.final_state, + foreign_controllers.db.clone(), + &foreign_controllers.selector_controller, + &mut foreign_controllers.ledger_controller, + Some(saved_bytecode), + None, + None, + ); + let mut universe = ExecutionTestUniverse::new(foreign_controllers, exec_cfg); + + // load bytecodes + universe.deploy_bytecode_block( + &KeyPair::from_str(TEST_SK_1).unwrap(), + Slot::new(1, 0), + include_bytes!("./wasm/nested_call.wasm"), + include_bytes!("./wasm/test.wasm"), + ); + finalized_waitpoint.wait(); + let address = universe.get_address_sc_deployed(Slot::new(1, 0)); + + // Call the function test of the smart contract + let operation = ExecutionTestUniverse::create_call_sc_operation( + &KeyPair::from_str(TEST_SK_2).unwrap(), + 10000000, + Amount::from_str("0").unwrap(), + Amount::from_str("0").unwrap(), + Address::from_str(&address).unwrap(), + String::from("test"), + address.as_bytes().to_vec(), + ) + .unwrap(); + universe.call_sc_block( + &KeyPair::from_str(TEST_SK_2).unwrap(), + Slot::new(1, 1), + operation, + ); + finalized_waitpoint.wait(); + + // Get the events. We expect the call to have succeeded, so we check for the length of the events. + // The smart contract emits 4 events in total, (to check gas usage), so we expect at least 4 events, + // and none of them should contain the error message. + let events = universe + .module_controller + .get_filtered_sc_output_event(EventFilter { + start: Some(Slot::new(1, 1)), + ..Default::default() + }); + assert!(events.len() >= 4); + for event in events.iter() { + assert!(!event.data.contains("recursion depth limit reached")); + } +} + /// Test the ABI get call coins /// /// Deploy an SC with a method `test` that generate an event saying how many coins he received diff --git a/massa-models/src/config/constants.rs b/massa-models/src/config/constants.rs index 39a832f59fe..abaf17faa6f 100644 --- a/massa-models/src/config/constants.rs +++ b/massa-models/src/config/constants.rs @@ -310,6 +310,8 @@ pub const ASYNC_MSG_CST_GAS_COST: u64 = 750_000; pub const BASE_OPERATION_GAS_COST: u64 = 800_000; // approx MAX_GAS_PER_BLOCK / MAX_OPERATIONS_PER_BLOCK /// Maximum event size in bytes pub const MAX_EVENT_DATA_SIZE: usize = 50_000; +/// Maximum number of recursion for calls +pub const MAX_RECURSIVE_CALLS_DEPTH: u16 = 25; // // Constants used in network diff --git a/massa-node/src/main.rs b/massa-node/src/main.rs index c80a594c893..119f901af02 100644 --- a/massa-node/src/main.rs +++ b/massa-node/src/main.rs @@ -88,9 +88,9 @@ use massa_models::config::constants::{ use massa_models::config::{ BASE_OPERATION_GAS_COST, CHAINID, KEEP_EXECUTED_HISTORY_EXTRA_PERIODS, MAX_BOOTSTRAP_FINAL_STATE_PARTS_SIZE, MAX_BOOTSTRAP_VERSIONING_ELEMENTS_SIZE, - MAX_EVENT_DATA_SIZE, MAX_MESSAGE_SIZE, MAX_RUNTIME_MODULE_CUSTOM_SECTION_DATA_LEN, - MAX_RUNTIME_MODULE_CUSTOM_SECTION_LEN, MAX_RUNTIME_MODULE_EXPORTS, - MAX_RUNTIME_MODULE_FUNCTIONS, MAX_RUNTIME_MODULE_FUNCTION_NAME_LEN, + MAX_EVENT_DATA_SIZE, MAX_MESSAGE_SIZE, MAX_RECURSIVE_CALLS_DEPTH, + MAX_RUNTIME_MODULE_CUSTOM_SECTION_DATA_LEN, MAX_RUNTIME_MODULE_CUSTOM_SECTION_LEN, + MAX_RUNTIME_MODULE_EXPORTS, MAX_RUNTIME_MODULE_FUNCTIONS, MAX_RUNTIME_MODULE_FUNCTION_NAME_LEN, MAX_RUNTIME_MODULE_GLOBAL_INITIALIZER, MAX_RUNTIME_MODULE_IMPORTS, MAX_RUNTIME_MODULE_MEMORIES, MAX_RUNTIME_MODULE_NAME_LEN, MAX_RUNTIME_MODULE_PASSIVE_DATA, MAX_RUNTIME_MODULE_PASSIVE_ELEMENT, MAX_RUNTIME_MODULE_SIGNATURE_LEN, MAX_RUNTIME_MODULE_TABLE, @@ -541,6 +541,7 @@ async fn launch( .broadcast_slot_execution_traces_channel_capacity, max_execution_traces_slot_limit: SETTINGS.execution.execution_traces_limit, block_dump_folder_path, + max_recursive_calls_depth: MAX_RECURSIVE_CALLS_DEPTH, condom_limits, };