diff --git a/soroban-env-host/src/e2e_invoke.rs b/soroban-env-host/src/e2e_invoke.rs index 81a141d92..1387bc53f 100644 --- a/soroban-env-host/src/e2e_invoke.rs +++ b/soroban-env-host/src/e2e_invoke.rs @@ -337,6 +337,7 @@ pub fn invoke_host_function_with_trace_hook, I: ExactSizeIterator let source_account: AccountId = host.metered_from_xdr(encoded_source_account.as_ref())?; host.set_source_account(source_account)?; host.set_ledger_info(ledger_info)?; + host.maybe_add_module_cache()?; host.set_authorization_entries(auth_entries)?; let seed32: [u8; 32] = base_prng_seed.as_ref().try_into().map_err(|_| { host.err( @@ -533,6 +534,10 @@ pub fn invoke_host_function_in_recording_mode( let invoke_result = host.invoke_function(host_function); let mut contract_events_and_return_value_size = 0_u32; if let Ok(res) = &invoke_result { + // See explanation for this line in [crate::vm::Vm::parse_module] -- it exists + // to add-back module-parsing costs that were suppressed during the invocation. + host.maybe_add_module_cache()?; + let mut encoded_result_sc_val = vec![]; metered_write_xdr(&budget, res, &mut encoded_result_sc_val)?; contract_events_and_return_value_size = contract_events_and_return_value_size diff --git a/soroban-env-host/src/test/e2e_tests.rs b/soroban-env-host/src/test/e2e_tests.rs index ffd45e32a..3b1c1d46f 100644 --- a/soroban-env-host/src/test/e2e_tests.rs +++ b/soroban-env-host/src/test/e2e_tests.rs @@ -1737,3 +1737,74 @@ fn test_classic_account_auth_using_simulation() { .unwrap(); assert!(res.invoke_result.is_ok()); } + +#[cfg(feature = "next")] +mod cap_54_55_56 { + + use super::*; + use more_asserts::assert_lt; + use pretty_assertions::assert_eq; + + // Test that when running on a protocol that supports the ModuleCache, when + // doing work that would be significantly different under cached instantiation, + // we get a cost estimate from recording mode that still matches the cost of the + // actual execution. + #[test] + fn test_module_cache_recording_fidelity() { + let cd = CreateContractData::new([111; 32], ADD_I32); + let mut ledger_info = default_ledger_info(); + ledger_info.protocol_version = crate::vm::ModuleCache::MIN_LEDGER_VERSION; + let host_fn = invoke_contract_host_fn( + &cd.contract_address, + "add", + vec![ScVal::I32(1), ScVal::I32(2)], + ); + let ledger_entries_with_ttl = vec![ + ( + cd.wasm_entry.clone(), + Some(ledger_info.sequence_number + 100), + ), + ( + cd.contract_entry.clone(), + Some(ledger_info.sequence_number + 1000), + ), + ]; + let res = invoke_host_function_recording_helper( + true, + &host_fn, + &cd.deployer, + None, + &ledger_info, + ledger_entries_with_ttl.clone(), + &prng_seed(), + None, + ) + .unwrap(); + assert_eq!(res.invoke_result.unwrap(), ScVal::I32(3)); + + let resources = res.resources; + let auth_entries = res.auth; + + let res = invoke_host_function_helper( + true, + &host_fn, + &resources, + &cd.deployer, + auth_entries, + &ledger_info, + ledger_entries_with_ttl, + &prng_seed(), + ) + .unwrap(); + assert_eq!(res.invoke_result.unwrap(), ScVal::I32(3)); + + let insns_recording = resources.instructions as f64; + let insns_enforcing = res.budget.get_cpu_insns_consumed().unwrap() as f64; + let insns_delta = (insns_recording - insns_enforcing).abs(); + let rel_delta_pct = 100.0 * (insns_delta / insns_enforcing); + + // Check that the recording-mode module cache hack puts us within a + // half-percent of the right number. + assert_lt!(rel_delta_pct, 0.5); + } +} diff --git a/soroban-env-host/src/test/lifecycle.rs b/soroban-env-host/src/test/lifecycle.rs index bc4f64171..8cce56ac9 100644 --- a/soroban-env-host/src/test/lifecycle.rs +++ b/soroban-env-host/src/test/lifecycle.rs @@ -72,6 +72,7 @@ fn test_host() -> Host { ..Default::default() }) .unwrap(); + host.maybe_add_module_cache().unwrap(); host } @@ -571,6 +572,8 @@ fn test_large_contract() { #[allow(dead_code)] mod cap_54_55_56 { + use more_asserts::assert_gt; + use super::*; use crate::{ storage::{FootprintMap, StorageMap}, @@ -610,6 +613,22 @@ mod cap_54_55_56 { InstantiateWasmDataSegmentBytes, ]; + fn is_instantiation_cost(ct: ContractCostType) -> bool { + match ct { + InstantiateWasmInstructions + | InstantiateWasmFunctions + | InstantiateWasmGlobals + | InstantiateWasmTableEntries + | InstantiateWasmTypes + | InstantiateWasmDataSegments + | InstantiateWasmElemSegments + | InstantiateWasmImports + | InstantiateWasmExports + | InstantiateWasmDataSegmentBytes => true, + _ => false, + } + } + fn new_host_with_protocol_and_uploaded_contract( hostname: &'static str, proto: u32, @@ -723,12 +742,29 @@ mod cap_54_55_56 { ContractAndWasmEntries::from_contract_addr(&host, contract) } - fn upload_and_call( + fn observed_test_host_with_storage_and_budget( + hostname: &'static str, + proto: u32, + storage: Storage, + budget: Budget, + ) -> Result { + let host = Host::with_storage_and_budget(storage, budget); + let host = ObservedHost::new(hostname, host); + host.enable_debug()?; + host.set_ledger_info(LedgerInfo { + protocol_version: proto, + ..Default::default() + })?; + host.maybe_add_module_cache()?; + Ok(host) + } + + fn upload_and_make_host_for_next_ledger( upload_hostname: &'static str, upload_proto: u32, - call_hostname: &'static str, - call_proto: u32, - ) -> Result<(Budget, Storage), HostError> { + second_hostname: &'static str, + second_proto: u32, + ) -> Result<(ObservedHost, Hash), HostError> { // Phase 1: upload contract, tear down host, "close the ledger" and possibly change protocol. let (host, contract) = new_host_with_protocol_and_uploaded_contract(upload_hostname, upload_proto)?; @@ -738,17 +774,31 @@ mod cap_54_55_56 { let (storage, _events) = realhost.try_finish()?; // Phase 2: build new host with previous ledger output as storage, call contract. Possibly on new protocol. - let host = Host::with_storage_and_budget(storage, Budget::default()); - host.enable_debug()?; - let host = ObservedHost::new(call_hostname, host); - host.set_ledger_info(LedgerInfo { - protocol_version: call_proto, - ..Default::default() - })?; + let host = observed_test_host_with_storage_and_budget( + second_hostname, + second_proto, + storage, + Budget::default(), + )?; + Ok((host, contract_id)) + } + + fn upload_and_call( + upload_hostname: &'static str, + upload_proto: u32, + call_hostname: &'static str, + call_proto: u32, + ) -> Result<(Budget, Storage), HostError> { + let (host, contract_id) = upload_and_make_host_for_next_ledger( + upload_hostname, + upload_proto, + call_hostname, + call_proto, + )?; let contract = host.add_host_object(crate::xdr::ScAddress::Contract(contract_id))?; let _ = host.call( contract, - Symbol::try_from_small_str("test").unwrap(), + Symbol::try_from_small_str("test")?, host.vec_new()?, )?; let realhost = host.clone(); @@ -769,6 +819,8 @@ mod cap_54_55_56 { false } + // region: CAP-0054 refined cost model + // Test that running on protocol vOld only charges the VmInstantiation cost // type. #[test] @@ -865,12 +917,12 @@ mod cap_54_55_56 { // make a new storage map for a new run let budget = Budget::default(); let storage = entries.read_only_storage(&budget); - let host = Host::with_storage_and_budget(storage, budget); - let host = ObservedHost::new("test_v_old_no_rewrite_call", host); - host.set_ledger_info(LedgerInfo { - protocol_version: V_OLD, - ..Default::default() - })?; + let host = observed_test_host_with_storage_and_budget( + "test_v_old_no_rewrite_call", + V_OLD, + storage, + budget, + )?; host.upload_contract_wasm(wasm_module_with_a_bit_of_everything(V_OLD))?; Ok(()) } @@ -885,12 +937,12 @@ mod cap_54_55_56 { // make a new storage map for a new upload but with read-only footprint -- this should fail let budget = Budget::default(); let storage = entries.read_only_storage(&budget); - let host = Host::with_storage_and_budget(storage, budget); - let host = ObservedHost::new("test_v_new_rewrite_call_fail", host); - host.set_ledger_info(LedgerInfo { - protocol_version: V_NEW, - ..Default::default() - })?; + let host = observed_test_host_with_storage_and_budget( + "test_v_new_rewrite_call_fail", + V_NEW, + storage, + budget, + )?; let wasm_blob = match &entries.wasm_entry.data { LedgerEntryData::ContractCode(cce) => cce.code.to_vec(), _ => panic!("expected ContractCode"), @@ -902,12 +954,12 @@ mod cap_54_55_56 { // make a new storage map for a new upload but with read-write footprint -- this should pass let budget = Budget::default(); let storage = entries.wasm_writing_storage(&budget); - let host = Host::with_storage_and_budget(storage, budget); - let host = ObservedHost::new("test_v_new_rewrite_call_succeed", host); - host.set_ledger_info(LedgerInfo { - protocol_version: V_NEW, - ..Default::default() - })?; + let host = observed_test_host_with_storage_and_budget( + "test_v_new_rewrite_call_succeed", + V_NEW, + storage, + budget, + )?; host.upload_contract_wasm(wasm_blob)?; let entries = entries.reload(&host)?; assert!(code_entry_has_cost_inputs(&entries.wasm_entry)); @@ -926,12 +978,12 @@ mod cap_54_55_56 { // make a new storage map for a new upload but with read-only footprint -- this should pass let budget = Budget::default(); let storage = entries.read_only_storage(&budget); - let host = Host::with_storage_and_budget(storage, budget); - let host = ObservedHost::new("test_v_new_no_rewrite_call_pass", host); - host.set_ledger_info(LedgerInfo { - protocol_version: V_NEW, - ..Default::default() - })?; + let host = observed_test_host_with_storage_and_budget( + "test_v_new_no_rewrite_call_pass", + V_NEW, + storage, + budget, + )?; let wasm_blob = match &entries.wasm_entry.data { LedgerEntryData::ContractCode(cce) => cce.code.to_vec(), _ => panic!("expected ContractCode"), @@ -942,4 +994,148 @@ mod cap_54_55_56 { Ok(()) } + // endregion: CAP-0054 refined cost model + + // region: CAP-0056 ModuleCache related tests + + // Test that running on protocol vOld does not make a ModuleCache at all. + #[test] + fn test_v_old_no_module_cache() -> Result<(), HostError> { + let host = upload_and_make_host_for_next_ledger( + "test_v_old_no_module_cache_upload", + V_OLD, + "test_v_old_no_module_cache_check", + V_OLD, + )? + .0; + let module_cache = host.try_borrow_module_cache()?; + assert!(module_cache.is_none()); + Ok(()) + } + + // Test that running on protocol vNew does add ModuleCache entries. + #[test] + fn test_v_new_module_cache() -> Result<(), HostError> { + let (host, contract_id) = upload_and_make_host_for_next_ledger( + "test_v_new_module_cache_upload", + V_OLD, + "test_v_new_module_cache_check", + V_NEW, + )?; + let wasm = get_contract_wasm_ref(&host, contract_id); + let module_cache = host.try_borrow_module_cache()?; + if let Some(module_cache) = &*module_cache { + assert!(module_cache.get_module(&*host, &wasm).is_ok()); + } else { + panic!("expected module cache"); + } + Ok(()) + } + + // Test that, when running on protocol vNew instantiating a contract without + // ContractCodeCostInputs, repeated invocations of the same contract + // increase the VmCachedInstantiation costs but do not increase the + // VmInstantiation costs. + #[test] + fn test_v_new_no_contract_code_cost_inputs_cached_instantiation() -> Result<(), HostError> { + let (host, contract_id) = upload_and_make_host_for_next_ledger( + "test_v_new_no_contract_code_cost_inputs_cached_instantiation_upload", + V_OLD, + "test_v_new_no_contract_code_cost_inputs_cached_instantiation_call", + V_NEW, + )?; + + let contract = host.add_host_object(crate::xdr::ScAddress::Contract(contract_id))?; + let test_symbol = Symbol::try_from_small_str("test")?; + let args = host.vec_new()?; + let _ = host.call(contract, test_symbol, args)?; + + let budget = host.budget_cloned(); + + // Double check we're not charging the new cost types + for ct in NEW_COST_TYPES { + assert_eq!(budget.get_tracker(*ct)?.cpu, 0); + } + + // Check that we're charging both the old cost types + let first_call_vm_instantiation = budget.get_tracker(VmInstantiation)?.cpu; + let first_call_vm_cached_instantiation = budget.get_tracker(VmCachedInstantiation)?.cpu; + + assert_ne!(first_call_vm_instantiation, 0); + assert_ne!(first_call_vm_cached_instantiation, 0); + + // Do a second call and check that it only increased the cached cost type. + let _ = host.call(contract, test_symbol, args)?; + + assert_eq!( + budget.get_tracker(VmInstantiation)?.cpu, + first_call_vm_instantiation + ); + assert_gt!( + budget.get_tracker(VmCachedInstantiation)?.cpu, + first_call_vm_cached_instantiation + ); + + Ok(()) + } + + // Test that, when running on protocol vNew instantiating a contract with + // ContractCodeCostInputs, repeated invocations of the same contract + // increase the new refined cost model InstantiateWasm* cost types but + // do not increase the ParseWasm* cost types. + + #[test] + fn test_v_new_with_contract_code_cost_inputs_cached_instantiation() -> Result<(), HostError> { + let (host, contract_id) = upload_and_make_host_for_next_ledger( + "test_v_new_with_contract_code_cost_inputs_cached_instantiation_upload", + V_NEW, + "test_v_new_with_contract_code_cost_inputs_cached_instantiation_call", + V_NEW, + )?; + + let contract = host.add_host_object(crate::xdr::ScAddress::Contract(contract_id))?; + let test_symbol = Symbol::try_from_small_str("test")?; + let args = host.vec_new()?; + let _ = host.call(contract, test_symbol, args)?; + + let budget = host.budget_cloned(); + + // Check that we're not charging the old cost types + assert_eq!(budget.get_tracker(VmInstantiation)?.cpu, 0); + assert_eq!(budget.get_tracker(VmCachedInstantiation)?.cpu, 0); + + let mut first_costs = Vec::new(); + + for ct in NEW_COST_TYPES { + let cost = budget.get_tracker(*ct)?.cpu; + first_costs.push(cost); + if *ct == InstantiateWasmTypes { + // This is a zero-cost type in the current calibration of the + // new model -- and exceptional case in this test -- though we + // keep it in case it becomes nonzero at some point (it's + // credible that it would). + continue; + } + assert_ne!(cost, 0); + } + + // Do a second call and check that it only increased the cached cost type. + let _ = host.call(contract, test_symbol, args)?; + + for (ct, first_cost) in NEW_COST_TYPES.iter().zip(first_costs.iter()) { + if *ct == InstantiateWasmTypes { + continue; + } + let cost = budget.get_tracker(*ct)?.cpu; + if is_instantiation_cost(*ct) { + assert_gt!(cost, *first_cost); + } else { + assert_eq!(cost, *first_cost); + } + } + + Ok(()) + } + + // endregion: CAP-0056 ModuleCache related tests } diff --git a/soroban-env-host/src/testutils.rs b/soroban-env-host/src/testutils.rs index c305cfa8d..8b0b69ba0 100644 --- a/soroban-env-host/src/testutils.rs +++ b/soroban-env-host/src/testutils.rs @@ -202,6 +202,7 @@ impl Host { max_entry_ttl: 6_312_000, }) .unwrap(); + host.maybe_add_module_cache().unwrap(); host } @@ -1133,7 +1134,7 @@ pub(crate) mod wasm { let (mut me, fid) = fe.finish(); me.export_func(fid, "test"); me.define_elem_funcs(&[fid]); - me.define_data_segment(0x1234, vec![0; 8]); + me.define_data_segment(0x1234, vec![0; 512]); me.finish() } } diff --git a/soroban-env-host/src/vm.rs b/soroban-env-host/src/vm.rs index e5d014902..2fb96fc69 100644 --- a/soroban-env-host/src/vm.rs +++ b/soroban-env-host/src/vm.rs @@ -260,10 +260,77 @@ impl Vm { VmInstantiationTimer::new(host.clone()); let config = get_wasmi_config(host.as_budget())?; let engine = wasmi::Engine::new(&config); - let parsed_module = ParsedModule::new(host, &engine, wasm, cost_inputs)?; + let parsed_module = Self::parse_module(host, &engine, wasm, cost_inputs)?; Self::instantiate(host, contract_id, parsed_module) } + #[cfg(not(any(test, feature = "recording_mode")))] + fn parse_module( + host: &Host, + engine: &wasmi::Engine, + wasm: &[u8], + cost_inputs: VersionedContractCodeCostInputs, + ) -> Result, HostError> { + ParsedModule::new(host, engine, wasm, cost_inputs) + } + + /// This method exists to support [crate::storage::FootprintMode::Recording] + /// when running in protocol versions that feature the [ModuleCache]. + /// + /// There are two ways we can get to here: + /// + /// 1. When we're running in a protocol that doesn't support the + /// [ModuleCache] at all. In this case, we just parse the module and + /// charge for it as normal. + /// + /// 2. When we're in a protocol that _does_ support the [ModuleCache] but + /// are _also_ in [crate::storage::FootprintMode::Recording] mode. Then + /// the [ModuleCache] _does not get built_ during host setup (because we + /// have no footprint yet to buid the cache from), so our caller + /// [Host::call_contract_fn] sees no module cache, and so each call winds + /// up calling us here, reparsing each module as it's called, and then + /// throwing it away. + /// + /// When we are in case 2, we don't want to charge for all those reparses: + /// we want to charge only for the post-parse instantiations _as if_ we had + /// had the cache. The cache will actually be added in + /// [crate::e2e_invoke::invoke_host_function_in_recording_mode] _after_ the + /// invocation completes, by reading the storage and parsing all the modules + /// in it, in order to charge for parsing each used module _once_ and + /// thereby produce a mostly-correct total cost. + /// + /// We still charge the reparses to the shadow budget, to avoid a DoS risk, + /// and we still charge the instantiations to the real budget, to behave the + /// same as if we had a cache. + /// + /// Note that this will only produce "mostly correct" costs for recording in + /// the case of normal completion of the contract. If the contract traps or + /// otherwise fails to complete, the cost will be incorrect, because the + /// cache will not be built and the parse costs will never be charged. + /// + /// Finally, for those scratching their head about the overall structure: + /// all of this happens as a result of the "module cache" not being + /// especially cache-like (i.e. not being populated lazily, on-access). It's + /// populated all at once, up front, because wasmi does not allow adding + /// modules to an engine that's currently running. + #[cfg(any(test, feature = "recording_mode"))] + fn parse_module( + host: &Host, + engine: &wasmi::Engine, + wasm: &[u8], + cost_inputs: VersionedContractCodeCostInputs, + ) -> Result, HostError> { + use crate::storage::FootprintMode; + if host.get_ledger_protocol_version()? >= ModuleCache::MIN_LEDGER_VERSION { + if let FootprintMode::Recording(_) = host.try_borrow_storage()?.mode { + return host.budget_ref().with_observable_shadow_mode(|| { + ParsedModule::new(host, engine, wasm, cost_inputs) + }); + } + } + ParsedModule::new(host, engine, wasm, cost_inputs) + } + pub(crate) fn get_memory(&self, host: &Host) -> Result { match self.memory { Some(mem) => Ok(mem),