From d35b5e1ddd8cddabd17c7eef55e3e35bb0e5f798 Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Wed, 27 Mar 2024 11:08:26 -0700 Subject: [PATCH 01/19] Fix #1371 - calibration-typo bug --- soroban-env-host/src/budget.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/soroban-env-host/src/budget.rs b/soroban-env-host/src/budget.rs index 8b3e1ddbb..523b6d92e 100644 --- a/soroban-env-host/src/budget.rs +++ b/soroban-env-host/src/budget.rs @@ -457,7 +457,7 @@ impl Default for BudgetImpl { } #[cfg(feature = "next")] ContractCostType::ParseWasmDataSegmentBytes => { - cpu.const_term = 66075; + cpu.const_term = 0; cpu.lin_term = ScaledU64(28); } #[cfg(feature = "next")] @@ -507,7 +507,7 @@ impl Default for BudgetImpl { } #[cfg(feature = "next")] ContractCostType::InstantiateWasmDataSegmentBytes => { - cpu.const_term = 25191; + cpu.const_term = 0; cpu.lin_term = ScaledU64(14); } } @@ -661,7 +661,7 @@ impl Default for BudgetImpl { } #[cfg(feature = "next")] ContractCostType::ParseWasmDataSegmentBytes => { - mem.const_term = 17580; + mem.const_term = 0; mem.lin_term = ScaledU64(257); } @@ -712,7 +712,7 @@ impl Default for BudgetImpl { } #[cfg(feature = "next")] ContractCostType::InstantiateWasmDataSegmentBytes => { - mem.const_term = 69256; + mem.const_term = 0; mem.lin_term = ScaledU64(126); } } From f03d0e441117131dc9abbb0d8e5ad041a27e2fed Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Wed, 27 Mar 2024 11:33:32 -0700 Subject: [PATCH 02/19] Fix #1372 - give ParsedModule a declared size, meter its Rc alloc --- .../benches/common/cost_types/vm_ops.rs | 6 ++-- .../src/cost_runner/cost_types/vm_ops.rs | 4 +-- soroban-env-host/src/host/declared_size.rs | 4 +++ soroban-env-host/src/vm.rs | 2 +- soroban-env-host/src/vm/module_cache.rs | 2 +- soroban-env-host/src/vm/parsed_module.rs | 33 +++++++++++-------- 6 files changed, 30 insertions(+), 21 deletions(-) diff --git a/soroban-env-host/benches/common/cost_types/vm_ops.rs b/soroban-env-host/benches/common/cost_types/vm_ops.rs index a71c9db84..ae0c32d50 100644 --- a/soroban-env-host/benches/common/cost_types/vm_ops.rs +++ b/soroban-env-host/benches/common/cost_types/vm_ops.rs @@ -6,7 +6,6 @@ use soroban_env_host::{ vm::{ParsedModule, VersionedContractCodeCostInputs}, xdr, Host, }; -use std::rc::Rc; // Protocol 20 coarse cost model. pub(crate) struct VmInstantiationMeasure; @@ -40,10 +39,9 @@ macro_rules! impl_measurement_for_instantiation_cost_type { .unwrap(), ) } - let module = Rc::new( + let module = ParsedModule::new_with_isolated_engine(_host, &wasm, cost_inputs.clone()) - .unwrap(), - ); + .unwrap(); VmInstantiationSample { id: Some(id), wasm, diff --git a/soroban-env-host/src/cost_runner/cost_types/vm_ops.rs b/soroban-env-host/src/cost_runner/cost_types/vm_ops.rs index b21d02c0e..aa6648fcd 100644 --- a/soroban-env-host/src/cost_runner/cost_types/vm_ops.rs +++ b/soroban-env-host/src/cost_runner/cost_types/vm_ops.rs @@ -83,7 +83,7 @@ mod v21 { _iter: u64, sample: Self::SampleType, ) -> Self::RecycledType { - let module = black_box(Rc::new( + let module = black_box( ParsedModule::new( host, sample.module.module.engine(), @@ -91,7 +91,7 @@ mod v21 { sample.module.cost_inputs.clone(), ) .unwrap(), - )); + ); (Some(module), sample.wasm) } diff --git a/soroban-env-host/src/host/declared_size.rs b/soroban-env-host/src/host/declared_size.rs index d6ab4a431..26b75fdce 100644 --- a/soroban-env-host/src/host/declared_size.rs +++ b/soroban-env-host/src/host/declared_size.rs @@ -13,6 +13,7 @@ use crate::{ host::{frame::Context, Events}, host_object::HostObject, storage::AccessType, + vm::ParsedModule, xdr::{ AccountEntry, AccountId, Asset, BytesM, ContractCodeEntry, ContractDataDurability, ContractEvent, ContractExecutable, ContractIdPreimage, CreateContractArgs, Duration, @@ -130,6 +131,7 @@ impl_declared_size_type!(AccountAuthorizationTrackerSnapshot, 40); impl_declared_size_type!(InvokerContractAuthorizationTracker, 192); impl_declared_size_type!(InternalDiagnosticArg, 64); impl_declared_size_type!(InternalDiagnosticEvent, 88); +impl_declared_size_type!(ParsedModule, 304); // xdr types impl_declared_size_type!(TimePoint, 8); @@ -446,6 +448,7 @@ mod test { ); expect!["64"].assert_eq(size_of::().to_string().as_str()); expect!["88"].assert_eq(size_of::().to_string().as_str()); + expect!["304"].assert_eq(size_of::().to_string().as_str()); // xdr types expect!["8"].assert_eq(size_of::().to_string().as_str()); @@ -642,6 +645,7 @@ mod test { assert_mem_size_le_declared_size!(InvokerContractAuthorizationTracker); assert_mem_size_le_declared_size!(InternalDiagnosticArg); assert_mem_size_le_declared_size!(InternalDiagnosticEvent); + assert_mem_size_le_declared_size!(ParsedModule); // xdr types assert_mem_size_le_declared_size!(TimePoint); diff --git a/soroban-env-host/src/vm.rs b/soroban-env-host/src/vm.rs index b48208c2e..e5d014902 100644 --- a/soroban-env-host/src/vm.rs +++ b/soroban-env-host/src/vm.rs @@ -260,7 +260,7 @@ impl Vm { VmInstantiationTimer::new(host.clone()); let config = get_wasmi_config(host.as_budget())?; let engine = wasmi::Engine::new(&config); - let parsed_module = Rc::new(ParsedModule::new(host, &engine, wasm, cost_inputs)?); + let parsed_module = ParsedModule::new(host, &engine, wasm, cost_inputs)?; Self::instantiate(host, contract_id, parsed_module) } diff --git a/soroban-env-host/src/vm/module_cache.rs b/soroban-env-host/src/vm/module_cache.rs index 81facb2c4..d32df301d 100644 --- a/soroban-env-host/src/vm/module_cache.rs +++ b/soroban-env-host/src/vm/module_cache.rs @@ -74,7 +74,7 @@ impl ModuleCache { &[], )); } - let parsed_module = Rc::new(ParsedModule::new(host, &self.engine, &wasm, cost_inputs)?); + let parsed_module = ParsedModule::new(host, &self.engine, &wasm, cost_inputs)?; self.modules = self.modules .insert(contract_id.metered_clone(host)?, parsed_module, host)?; diff --git a/soroban-env-host/src/vm/parsed_module.rs b/soroban-env-host/src/vm/parsed_module.rs index eff960075..0bddf9074 100644 --- a/soroban-env-host/src/vm/parsed_module.rs +++ b/soroban-env-host/src/vm/parsed_module.rs @@ -1,5 +1,6 @@ use crate::{ err, + host::metered_clone::MeteredAlloc, meta::{self, get_ledger_protocol_version}, xdr::{ContractCostType, Limited, ReadXdr, ScEnvMetaEntry, ScErrorCode, ScErrorType}, Host, HostError, DEFAULT_XDR_RW_LIMITS, @@ -8,7 +9,7 @@ use crate::{ use wasmi::{Engine, Module}; use super::{ModuleCache, MAX_VM_ARGS}; -use std::io::Cursor; +use std::{io::Cursor, rc::Rc}; #[derive(Debug, Clone)] pub enum VersionedContractCodeCostInputs { @@ -153,14 +154,17 @@ impl ParsedModule { engine: &Engine, wasm: &[u8], cost_inputs: VersionedContractCodeCostInputs, - ) -> Result { + ) -> Result, HostError> { cost_inputs.charge_for_parsing(host)?; let (module, proto_version) = Self::parse_wasm(host, engine, wasm)?; - Ok(Self { - module, - proto_version, - cost_inputs, - }) + Rc::metered_new( + Self { + module, + proto_version, + cost_inputs, + }, + host, + ) } #[cfg(any(test, feature = "testutils"))] @@ -168,17 +172,20 @@ impl ParsedModule { host: &Host, wasm: &[u8], cost_inputs: VersionedContractCodeCostInputs, - ) -> Result { + ) -> Result, HostError> { use crate::budget::AsBudget; let config = crate::vm::get_wasmi_config(host.as_budget())?; let engine = Engine::new(&config); cost_inputs.charge_for_parsing(host)?; let (module, proto_version) = Self::parse_wasm(host, &engine, wasm)?; - Ok(Self { - module, - proto_version, - cost_inputs, - }) + Rc::metered_new( + Self { + module, + proto_version, + cost_inputs, + }, + host, + ) } /// Parse the Wasm blob into a [Module] and its protocol number, checking its interface version From bcd4546fafd4fe05da9585ba213ad68b5482b318 Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Wed, 27 Mar 2024 23:05:44 -0700 Subject: [PATCH 03/19] Fix #1374 - Wire up module cache and add tests --- .../benches/worst_case_linear_models.rs | 10 +- soroban-env-host/src/budget.rs | 48 ++-- soroban-env-host/src/e2e_invoke.rs | 1 + soroban-env-host/src/e2e_testutils.rs | 34 ++- soroban-env-host/src/host.rs | 14 +- soroban-env-host/src/host/declared_size.rs | 5 +- soroban-env-host/src/host/frame.rs | 3 + soroban-env-host/src/test/e2e_tests.rs | 116 ++++++++ soroban-env-host/src/test/lifecycle.rs | 268 +++++++++++++++--- soroban-env-host/src/testutils.rs | 3 +- soroban-env-host/src/vm.rs | 160 ++++++++--- soroban-env-host/src/vm/func_info.rs | 16 +- soroban-env-host/src/vm/module_cache.rs | 31 +- soroban-env-host/src/vm/parsed_module.rs | 32 ++- soroban-test-wasms/src/lib.rs | 2 + soroban-test-wasms/wasm-workspace/Cargo.lock | 158 +++++++---- soroban-test-wasms/wasm-workspace/Cargo.toml | 7 +- .../opt/curr/example_sum_i32.wasm | Bin 0 -> 689 bytes .../wasm-workspace/sum_i32/Cargo.toml | 19 ++ .../wasm-workspace/sum_i32/src/lib.rs | 22 ++ 20 files changed, 775 insertions(+), 174 deletions(-) create mode 100644 soroban-test-wasms/wasm-workspace/opt/curr/example_sum_i32.wasm create mode 100644 soroban-test-wasms/wasm-workspace/sum_i32/Cargo.toml create mode 100644 soroban-test-wasms/wasm-workspace/sum_i32/src/lib.rs diff --git a/soroban-env-host/benches/worst_case_linear_models.rs b/soroban-env-host/benches/worst_case_linear_models.rs index 87a5f188f..3f0effd26 100644 --- a/soroban-env-host/benches/worst_case_linear_models.rs +++ b/soroban-env-host/benches/worst_case_linear_models.rs @@ -217,6 +217,7 @@ fn write_budget_params_code( ty, ); } + #[cfg(not(feature = "next"))] ContractCostType::VmCachedInstantiation => { println!( " @@ -230,7 +231,7 @@ fn write_budget_params_code( cpu.const_term, cpu.lin_term ), None => println!( - "ContractCostType::VmCachedInstantiation => !todo()" + "ContractCostType::VmCachedInstantiation => todo!()" ), } } @@ -239,7 +240,7 @@ fn write_budget_params_code( "ContractCostType::{:?} => {{ cpu.const_term = {}; cpu.lin_term = {:?}; }}", ty, cpu.const_term, cpu.lin_term ), - None => println!("ContractCostType::VmCachedInstantiation => !todo()"), + None => println!("ContractCostType::{:?} => todo!()", ty), }, } } @@ -292,6 +293,7 @@ fn write_budget_params_code( ty ) } + #[cfg(not(feature = "next"))] ContractCostType::VmCachedInstantiation => { println!( " @@ -305,7 +307,7 @@ fn write_budget_params_code( mem.const_term, mem.lin_term ), None => println!( - "ContractCostType::VmCachedInstantiation => !todo()" + "ContractCostType::VmCachedInstantiation => todo!()" ), } } @@ -314,7 +316,7 @@ fn write_budget_params_code( "ContractCostType::{:?} => {{ mem.const_term = {}; mem.lin_term = {:?}; }}", ty, mem.const_term, mem.lin_term ), - None => println!("ContractCostType::VmCachedInstantiation => !todo()"), + None => println!("ContractCostType::{:?} => todo!()", ty), }, } } diff --git a/soroban-env-host/src/budget.rs b/soroban-env-host/src/budget.rs index 523b6d92e..00d6dae6c 100644 --- a/soroban-env-host/src/budget.rs +++ b/soroban-env-host/src/budget.rs @@ -366,8 +366,8 @@ impl Default for BudgetImpl { cpu.lin_term = ScaledU64(45405); } ContractCostType::VmCachedInstantiation => { - cpu.const_term = 451626; - cpu.lin_term = ScaledU64(45405); + cpu.const_term = 24457; + cpu.lin_term = ScaledU64(634); } ContractCostType::InvokeVmFunction => { cpu.const_term = 1948; @@ -412,48 +412,48 @@ impl Default for BudgetImpl { #[cfg(feature = "next")] ContractCostType::ParseWasmInstructions => { - cpu.const_term = 72736; - cpu.lin_term = ScaledU64(25420); + cpu.const_term = 73261; + cpu.lin_term = ScaledU64(25409); } #[cfg(feature = "next")] ContractCostType::ParseWasmFunctions => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(536688); + cpu.lin_term = ScaledU64(540756); } #[cfg(feature = "next")] ContractCostType::ParseWasmGlobals => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(176902); + cpu.lin_term = ScaledU64(176366); } #[cfg(feature = "next")] ContractCostType::ParseWasmTableEntries => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(29639); + cpu.lin_term = ScaledU64(29989); } #[cfg(feature = "next")] ContractCostType::ParseWasmTypes => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(1048891); + cpu.lin_term = ScaledU64(1061438); } #[cfg(feature = "next")] ContractCostType::ParseWasmDataSegments => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(236970); + cpu.lin_term = ScaledU64(237323); } #[cfg(feature = "next")] ContractCostType::ParseWasmElemSegments => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(317249); + cpu.lin_term = ScaledU64(328468); } #[cfg(feature = "next")] ContractCostType::ParseWasmImports => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(694667); + cpu.lin_term = ScaledU64(701838); } #[cfg(feature = "next")] ContractCostType::ParseWasmExports => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(427037); + cpu.lin_term = ScaledU64(429378); } #[cfg(feature = "next")] ContractCostType::ParseWasmDataSegmentBytes => { @@ -462,23 +462,23 @@ impl Default for BudgetImpl { } #[cfg(feature = "next")] ContractCostType::InstantiateWasmInstructions => { - cpu.const_term = 25059; + cpu.const_term = 26338; cpu.lin_term = ScaledU64(0); } #[cfg(feature = "next")] ContractCostType::InstantiateWasmFunctions => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(7503); + cpu.lin_term = ScaledU64(7580); } #[cfg(feature = "next")] ContractCostType::InstantiateWasmGlobals => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(10761); + cpu.lin_term = ScaledU64(10663); } #[cfg(feature = "next")] ContractCostType::InstantiateWasmTableEntries => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(3211); + cpu.lin_term = ScaledU64(1837); } #[cfg(feature = "next")] ContractCostType::InstantiateWasmTypes => { @@ -488,22 +488,22 @@ impl Default for BudgetImpl { #[cfg(feature = "next")] ContractCostType::InstantiateWasmDataSegments => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(16370); + cpu.lin_term = ScaledU64(22893); } #[cfg(feature = "next")] ContractCostType::InstantiateWasmElemSegments => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(28309); + cpu.lin_term = ScaledU64(41094); } #[cfg(feature = "next")] ContractCostType::InstantiateWasmImports => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(683461); + cpu.lin_term = ScaledU64(691499); } #[cfg(feature = "next")] ContractCostType::InstantiateWasmExports => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(297065); + cpu.lin_term = ScaledU64(297122); } #[cfg(feature = "next")] ContractCostType::InstantiateWasmDataSegmentBytes => { @@ -570,8 +570,8 @@ impl Default for BudgetImpl { mem.lin_term = ScaledU64(5064); } ContractCostType::VmCachedInstantiation => { - mem.const_term = 130065; - mem.lin_term = ScaledU64(5064); + mem.const_term = 68960; + mem.lin_term = ScaledU64(1217); } ContractCostType::InvokeVmFunction => { mem.const_term = 14; @@ -652,7 +652,7 @@ impl Default for BudgetImpl { #[cfg(feature = "next")] ContractCostType::ParseWasmImports => { mem.const_term = 0; - mem.lin_term = ScaledU64(102890); + mem.lin_term = ScaledU64(103229); } #[cfg(feature = "next")] ContractCostType::ParseWasmExports => { @@ -703,7 +703,7 @@ impl Default for BudgetImpl { #[cfg(feature = "next")] ContractCostType::InstantiateWasmImports => { mem.const_term = 0; - mem.lin_term = ScaledU64(77273); + mem.lin_term = ScaledU64(76483); } #[cfg(feature = "next")] ContractCostType::InstantiateWasmExports => { diff --git a/soroban-env-host/src/e2e_invoke.rs b/soroban-env-host/src/e2e_invoke.rs index 81a141d92..aa51bb85a 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( diff --git a/soroban-env-host/src/e2e_testutils.rs b/soroban-env-host/src/e2e_testutils.rs index ef749f57e..a1a83e70d 100644 --- a/soroban-env-host/src/e2e_testutils.rs +++ b/soroban-env-host/src/e2e_testutils.rs @@ -58,6 +58,21 @@ pub fn wasm_entry(wasm: &[u8]) -> LedgerEntry { })) } +#[cfg(feature = "next")] +pub fn wasm_entry_with_refined_contract_cost_inputs(wasm: &[u8]) -> LedgerEntry { + use crate::xdr::ContractCodeEntryV1; + let host = crate::Host::default(); + ledger_entry(LedgerEntryData::ContractCode(ContractCodeEntry { + ext: ContractCodeEntryExt::V1(ContractCodeEntryV1 { + ext: ExtensionPoint::V0, + cost_inputs: crate::vm::ParsedModule::extract_refined_contract_cost_inputs(&host, wasm) + .unwrap(), + }), + hash: get_wasm_hash(wasm).try_into().unwrap(), + code: wasm.try_into().unwrap(), + })) +} + pub fn default_ledger_info() -> LedgerInfo { LedgerInfo { protocol_version: 20, @@ -84,6 +99,13 @@ pub struct CreateContractData { impl CreateContractData { pub fn new(salt: [u8; 32], wasm: &[u8]) -> Self { + Self::new_with_refined_contract_cost_inputs(salt, wasm, false) + } + pub fn new_with_refined_contract_cost_inputs( + salt: [u8; 32], + wasm: &[u8], + _refined_cost_inputs: bool, + ) -> Self { let deployer = get_account_id([123; 32]); let contract_id_preimage = get_contract_id_preimage(&deployer, &salt); @@ -114,10 +136,20 @@ impl CreateContractData { }), })); + #[cfg(not(feature = "next"))] + let wasm_entry = wasm_entry(wasm); + + #[cfg(feature = "next")] + let wasm_entry = if _refined_cost_inputs { + wasm_entry_with_refined_contract_cost_inputs(wasm) + } else { + wasm_entry(wasm) + }; + Self { deployer, wasm_key: get_wasm_key(wasm), - wasm_entry: wasm_entry(wasm), + wasm_entry, contract_key, contract_entry, contract_address, diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index d89447827..460b602ad 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -81,6 +81,7 @@ pub struct CoverageScoreboard { #[derive(Clone, Default)] struct HostImpl { module_cache: RefCell>, + shared_linker: RefCell>>, source_account: RefCell>, ledger: RefCell>, objects: RefCell>, @@ -194,6 +195,12 @@ impl_checked_borrow_helpers!( try_borrow_module_cache, try_borrow_module_cache_mut ); +impl_checked_borrow_helpers!( + shared_linker, + Option>, + try_borrow_linker, + try_borrow_linker_mut +); impl_checked_borrow_helpers!( source_account, Option, @@ -323,6 +330,7 @@ impl Host { let _client = tracy_client::Client::start(); Self(Rc::new(HostImpl { module_cache: RefCell::new(None), + shared_linker: RefCell::new(None), source_account: RefCell::new(None), ledger: RefCell::new(None), objects: Default::default(), @@ -356,8 +364,12 @@ impl Host { pub fn maybe_add_module_cache(&self) -> Result<(), HostError> { if cfg!(feature = "next") && self.get_ledger_protocol_version()? >= ModuleCache::MIN_LEDGER_VERSION + && self.try_borrow_module_cache()?.is_none() { - *self.try_borrow_module_cache_mut()? = Some(ModuleCache::new(self)?); + let cache = ModuleCache::new(self)?; + let linker = cache.make_linker(self)?; + *self.try_borrow_module_cache_mut()? = Some(cache); + *self.try_borrow_linker_mut()? = Some(linker); } Ok(()) } diff --git a/soroban-env-host/src/host/declared_size.rs b/soroban-env-host/src/host/declared_size.rs index 26b75fdce..85c82b2e0 100644 --- a/soroban-env-host/src/host/declared_size.rs +++ b/soroban-env-host/src/host/declared_size.rs @@ -448,7 +448,10 @@ mod test { ); expect!["64"].assert_eq(size_of::().to_string().as_str()); expect!["88"].assert_eq(size_of::().to_string().as_str()); - expect!["304"].assert_eq(size_of::().to_string().as_str()); + + // NB: ParsedModule changes size depending on release or debug builds, + // which is impossible to conditionalize on. We should figure out how to + // check it someday but until we do we'll leaveit out. // xdr types expect!["8"].assert_eq(size_of::().to_string().as_str()); diff --git a/soroban-env-host/src/host/frame.rs b/soroban-env-host/src/host/frame.rs index fea201386..b207f80ac 100644 --- a/soroban-env-host/src/host/frame.rs +++ b/soroban-env-host/src/host/frame.rs @@ -193,6 +193,9 @@ impl Host { // recording auth mode. This is a no-op for the enforcing mode. self.try_borrow_authorization_manager()? .maybe_emulate_authentication(self)?; + // 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. + self.maybe_add_module_cache()?; } let mut auth_snapshot = None; if let Some(rp) = orp { diff --git a/soroban-env-host/src/test/e2e_tests.rs b/soroban-env-host/src/test/e2e_tests.rs index ffd45e32a..c1a31f53c 100644 --- a/soroban-env-host/src/test/e2e_tests.rs +++ b/soroban-env-host/src/test/e2e_tests.rs @@ -1737,3 +1737,119 @@ 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 crate::xdr::ScVec; + use more_asserts::assert_lt; + use pretty_assertions::assert_eq; + use soroban_test_wasms::SUM_I32; + + // 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() { + const V_NEW: u32 = crate::vm::ModuleCache::MIN_LEDGER_VERSION; + const V_OLD: u32 = V_NEW - 1; + + for (proto, refined_cost_inputs) in + [(V_OLD, false), (V_OLD, true), (V_NEW, false), (V_NEW, true)] + { + let add_cd = CreateContractData::new_with_refined_contract_cost_inputs( + [111; 32], + ADD_I32, + refined_cost_inputs, + ); + let sum_cd = CreateContractData::new_with_refined_contract_cost_inputs( + [222; 32], + SUM_I32, + refined_cost_inputs, + ); + let mut ledger_info = default_ledger_info(); + ledger_info.protocol_version = proto; + let host_fn = invoke_contract_host_fn( + &sum_cd.contract_address, + "sum", + vec![ + ScVal::Address(add_cd.contract_address.clone()), + ScVal::Vec(Some(ScVec( + vec![ + ScVal::I32(1), + ScVal::I32(2), + ScVal::I32(3), + ScVal::I32(4), + ScVal::I32(5), + ] + .try_into() + .unwrap(), + ))), + ], + ); + let ledger_entries_with_ttl = vec![ + ( + add_cd.wasm_entry.clone(), + Some(ledger_info.sequence_number + 100), + ), + ( + add_cd.contract_entry.clone(), + Some(ledger_info.sequence_number + 1000), + ), + ( + sum_cd.wasm_entry.clone(), + Some(ledger_info.sequence_number + 100), + ), + ( + sum_cd.contract_entry.clone(), + Some(ledger_info.sequence_number + 1000), + ), + ]; + let res = invoke_host_function_recording_helper( + true, + &host_fn, + &sum_cd.deployer, + None, + &ledger_info, + ledger_entries_with_ttl.clone(), + &prng_seed(), + None, + ) + .unwrap(); + assert_eq!(res.invoke_result.unwrap(), ScVal::I32(15)); + + let resources = res.resources; + let auth_entries = res.auth; + + let res = invoke_host_function_helper( + true, + &host_fn, + &resources, + &sum_cd.deployer, + auth_entries, + &ledger_info, + ledger_entries_with_ttl, + &prng_seed(), + ) + .unwrap(); + assert_eq!(res.invoke_result.unwrap(), ScVal::I32(15)); + + 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); + + dbg!(proto); + dbg!(refined_cost_inputs); + dbg!(insns_recording); + dbg!(insns_enforcing); + dbg!(rel_delta_pct); + + // Check that the recording-mode module cache hack puts us within 1% + // of the right number. + assert_lt!(rel_delta_pct, 1.0); + } + } +} 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..95c8cc837 100644 --- a/soroban-env-host/src/vm.rs +++ b/soroban-env-host/src/vm.rs @@ -100,6 +100,21 @@ impl std::hash::Hash for Vm { } } +impl Host { + pub(crate) fn make_linker( + engine: &wasmi::Engine, + symbols: &BTreeSet<(&str, &str)>, + ) -> Result, HostError> { + let mut linker = Linker::new(&engine); + for hf in HOST_FUNCTIONS { + if symbols.contains(&(hf.mod_str, hf.fn_str)) { + (hf.wrap)(&mut linker).map_err(|le| wasmi::Error::Linker(le))?; + } + } + Ok(linker) + } +} + impl Vm { #[cfg(feature = "testutils")] pub fn get_all_host_functions() -> Vec<(&'static str, &'static str, u32)> { @@ -119,26 +134,15 @@ impl Vm { let _span = tracy_span!("Vm::instantiate"); let engine = parsed_module.module.engine(); - let mut linker = >::new(engine); let mut store = Store::new(engine, host.clone()); parsed_module.cost_inputs.charge_for_instantiation(host)?; store.limiter(|host| host); - let module_imports: BTreeSet<(&str, &str)> = parsed_module - .module - .imports() - .filter(|i| i.ty().func().is_some()) - .map(|i| { - let mod_str = i.module(); - let fn_str = i.name(); - (mod_str, fn_str) - }) - .collect(); - { - // We perform link-time protocol version gating here. + // We perform instantiation-time protocol version gating of + // all module-imported symbols here. // Reasons for doing link-time instead of run-time check: // 1. VM instantiation is performed in both contract upload and // execution, thus any errorous contract will be rejected at @@ -148,46 +152,51 @@ impl Vm { // early is preferred from resource usage perspective. // 3. If a contract contains a call to an non-existent host // function, the current (correct) behavior is to return - // `Wasmi::LinkerError::MissingDefinition` error (which gets + // `Wasmi::errors::LinkerError::MissingDefinition` error (which gets // converted to a `(WasmVm, InvalidAction)`). If that host // function is defined in a later protocol, and we replay that - // contract (in the earlier protocol where it belongs), not - // linking the function preserves the right behavior and error - // code. + // contract (in the earlier protocol where it belongs), we need + // to return the same error. let _span0 = tracy_span!("define host functions"); let ledger_proto = host.with_ledger_info(|li| Ok(li.protocol_version))?; - for hf in HOST_FUNCTIONS { - if let Some(min_proto) = hf.min_proto { - if parsed_module.proto_version < min_proto || ledger_proto < min_proto { - // We skip linking this hf instead of returning an error - // because we have to support old contracts during replay. + parsed_module.with_import_symbols(|module_symbols| { + for hf in HOST_FUNCTIONS { + if !module_symbols.contains(&(hf.mod_str, hf.fn_str)) { continue; } - } - if let Some(max_proto) = hf.max_proto { - if parsed_module.proto_version > max_proto || ledger_proto > max_proto { - // We skip linking this hf instead of returning an error - // because we have to support old contracts during replay. - continue; + if let Some(min_proto) = hf.min_proto { + if parsed_module.proto_version < min_proto || ledger_proto < min_proto { + return Err(host.err( + ScErrorType::WasmVm, + ScErrorCode::InvalidAction, + "contract calls a host function not yet supported by current protocol", + &[], + )); + } + } + if let Some(max_proto) = hf.max_proto { + if parsed_module.proto_version > max_proto || ledger_proto > max_proto { + return Err(host.err( + ScErrorType::WasmVm, + ScErrorCode::InvalidAction, + "contract calls a host function no longer supported in the current protocol", + &[], + )); + } } } - // We only link the functions that are actually used by the - // contract. Linking is quite expensive. - if !module_imports.contains(&(hf.mod_str, hf.fn_str)) { - continue; - } - let func = (hf.wrap)(&mut store); - host.map_err( - linker - .define(hf.mod_str, hf.fn_str, func) - .map_err(|le| wasmi::Error::Linker(le)), - )?; - } + Ok(()) + })?; } let not_started_instance = { let _span0 = tracy_span!("instantiate module"); - host.map_err(linker.instantiate(&mut store, &parsed_module.module))? + if let Some(linker) = &*host.try_borrow_linker()? { + host.map_err(linker.instantiate(&mut store, &parsed_module.module))? + } else { + let linker = parsed_module.make_linker()?; + host.map_err(linker.instantiate(&mut store, &parsed_module.module))? + } }; let instance = host.map_err( @@ -260,10 +269,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 [Host::pop_context] + /// _after_ a top-level recording-mode 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), diff --git a/soroban-env-host/src/vm/func_info.rs b/soroban-env-host/src/vm/func_info.rs index 7914828bb..647c478f8 100644 --- a/soroban-env-host/src/vm/func_info.rs +++ b/soroban-env-host/src/vm/func_info.rs @@ -1,7 +1,7 @@ use super::dispatch; use crate::Host; use soroban_env_common::call_macro_with_all_host_functions; -use wasmi::{Func, Store}; +use wasmi::{errors::LinkerError, Linker}; pub(crate) struct HostFuncInfo { /// String name of the WASM module this host function is importable from. @@ -15,10 +15,10 @@ pub(crate) struct HostFuncInfo { #[allow(dead_code)] pub(crate) arity: u32, - /// Function that takes a wasmi::Store and _wraps_ a dispatch function + /// Function that takes a wasmi::Linker and adds a dispatch function /// for this host function, with the specific type of the dispatch function, - /// into a Func in the Store. - pub(crate) wrap: fn(&mut Store) -> Func, + /// into a Func in the Linker. + pub(crate) wrap: fn(&mut Linker) -> Result<&mut Linker, LinkerError>, /// Minimal supported protocol version of this host function pub(crate) min_proto: Option, @@ -45,7 +45,7 @@ macro_rules! host_function_info_helper { mod_str: $mod_str, fn_str: $fn_id, arity: fn_arity!($args), - wrap: |store| Func::wrap(store, dispatch::$func_id), + wrap: |linker| linker.func_wrap($mod_str, $fn_id, dispatch::$func_id), min_proto: Some($min_proto), max_proto: Some($max_proto), } @@ -55,7 +55,7 @@ macro_rules! host_function_info_helper { mod_str: $mod_str, fn_str: $fn_id, arity: fn_arity!($args), - wrap: |store| Func::wrap(store, dispatch::$func_id), + wrap: |linker| linker.func_wrap($mod_str, $fn_id, dispatch::$func_id), min_proto: Some($min_proto), max_proto: None, } @@ -65,7 +65,7 @@ macro_rules! host_function_info_helper { mod_str: $mod_str, fn_str: $fn_id, arity: fn_arity!($args), - wrap: |store| Func::wrap(store, dispatch::$func_id), + wrap: |linker| linker.func_wrap($mod_str, $fn_id, dispatch::$func_id), min_proto: None, max_proto: Some($max_proto), } @@ -75,7 +75,7 @@ macro_rules! host_function_info_helper { mod_str: $mod_str, fn_str: $fn_id, arity: fn_arity!($args), - wrap: |store| Func::wrap(store, dispatch::$func_id), + wrap: |linker| linker.func_wrap($mod_str, $fn_id, dispatch::$func_id), min_proto: None, max_proto: None, } diff --git a/soroban-env-host/src/vm/module_cache.rs b/soroban-env-host/src/vm/module_cache.rs index d32df301d..416e9f3db 100644 --- a/soroban-env-host/src/vm/module_cache.rs +++ b/soroban-env-host/src/vm/module_cache.rs @@ -1,11 +1,14 @@ -use super::parsed_module::{ParsedModule, VersionedContractCodeCostInputs}; +use super::{ + func_info::HOST_FUNCTIONS, + parsed_module::{ParsedModule, VersionedContractCodeCostInputs}, +}; use crate::{ budget::{get_wasmi_config, AsBudget}, host::metered_clone::MeteredClone, xdr::{Hash, ScErrorCode, ScErrorType}, Host, HostError, MeteredOrdMap, }; -use std::rc::Rc; +use std::{collections::BTreeSet, rc::Rc}; use wasmi::Engine; /// A [ModuleCache] is a cache of a set of Wasm modules that have been parsed @@ -81,6 +84,30 @@ impl ModuleCache { Ok(()) } + pub fn with_import_symbols( + &self, + host: &Host, + callback: impl FnOnce(&BTreeSet<(&str, &str)>) -> Result, + ) -> Result { + let mut import_symbols = BTreeSet::new(); + for module in self.modules.values(host)? { + module.with_import_symbols(|module_symbols| { + for hf in HOST_FUNCTIONS { + let sym = (hf.mod_str, hf.fn_str); + if module_symbols.contains(&sym) { + import_symbols.insert(sym); + } + } + Ok(()) + })?; + } + callback(&import_symbols) + } + + pub fn make_linker(&self, host: &Host) -> Result, HostError> { + self.with_import_symbols(host, |symbols| Host::make_linker(&self.engine, symbols)) + } + pub fn get_module( &self, host: &Host, diff --git a/soroban-env-host/src/vm/parsed_module.rs b/soroban-env-host/src/vm/parsed_module.rs index 0bddf9074..63657bd7a 100644 --- a/soroban-env-host/src/vm/parsed_module.rs +++ b/soroban-env-host/src/vm/parsed_module.rs @@ -9,7 +9,7 @@ use crate::{ use wasmi::{Engine, Module}; use super::{ModuleCache, MAX_VM_ARGS}; -use std::{io::Cursor, rc::Rc}; +use std::{collections::BTreeSet, io::Cursor, rc::Rc}; #[derive(Debug, Clone)] pub enum VersionedContractCodeCostInputs { @@ -167,6 +167,36 @@ impl ParsedModule { ) } + pub fn with_import_symbols( + &self, + callback: impl FnOnce(&BTreeSet<(&str, &str)>) -> Result, + ) -> Result { + // Cap symbols we're willing to import at 10 characters for each of + // module and function name. in practice they are all 1-2 chars, but + // we'll leave some future-proofing room here. The important point + // is to not be introducing a DoS vector. + const SYM_LEN_LIMIT: usize = 10; + let symbols: BTreeSet<(&str, &str)> = self + .module + .imports() + .filter_map(|i| { + if i.ty().func().is_some() { + let mod_str = i.module(); + let fn_str = i.name(); + if mod_str.len() < SYM_LEN_LIMIT && fn_str.len() < SYM_LEN_LIMIT { + return Some((mod_str, fn_str)); + } + } + None + }) + .collect(); + callback(&symbols) + } + + pub fn make_linker(&self) -> Result, HostError> { + self.with_import_symbols(|symbols| Host::make_linker(self.module.engine(), symbols)) + } + #[cfg(any(test, feature = "testutils"))] pub fn new_with_isolated_engine( host: &Host, diff --git a/soroban-test-wasms/src/lib.rs b/soroban-test-wasms/src/lib.rs index abaf0eae5..871dc47d1 100644 --- a/soroban-test-wasms/src/lib.rs +++ b/soroban-test-wasms/src/lib.rs @@ -47,6 +47,8 @@ pub use curr::*; mod curr { pub const ADD_I32: &[u8] = include_bytes!("../wasm-workspace/opt/curr/example_add_i32.wasm").as_slice(); + pub const SUM_I32: &[u8] = + include_bytes!("../wasm-workspace/opt/curr/example_sum_i32.wasm").as_slice(); pub const ADD_F32: &[u8] = include_bytes!("../wasm-workspace/opt/curr/example_add_f32.wasm").as_slice(); pub const ALLOC: &[u8] = diff --git a/soroban-test-wasms/wasm-workspace/Cargo.lock b/soroban-test-wasms/wasm-workspace/Cargo.lock index 717c5a8f8..38daa7fe7 100644 --- a/soroban-test-wasms/wasm-workspace/Cargo.lock +++ b/soroban-test-wasms/wasm-workspace/Cargo.lock @@ -45,7 +45,7 @@ dependencies = [ name = "auth_test_contract" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -165,7 +165,7 @@ checksum = "28c122c3980598d243d63d9a704629a2d748d101f278052ff068be5a4423ab6f" name = "contract_sac_transfer" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -424,7 +424,7 @@ checksum = "b90ca2580b73ab6a1f724b76ca11ab632df820fd6040c336200d2c1df7b3c82c" name = "example_add_f32" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -432,7 +432,7 @@ dependencies = [ name = "example_add_i32" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -440,7 +440,7 @@ dependencies = [ name = "example_alloc" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -448,7 +448,7 @@ dependencies = [ name = "example_complex" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -456,7 +456,7 @@ dependencies = [ name = "example_contract_data" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -464,7 +464,7 @@ dependencies = [ name = "example_create_contract" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -472,7 +472,7 @@ dependencies = [ name = "example_err" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -480,7 +480,7 @@ dependencies = [ name = "example_fannkuch" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -488,7 +488,7 @@ dependencies = [ name = "example_fib" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -496,7 +496,7 @@ dependencies = [ name = "example_hostile" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -504,7 +504,7 @@ dependencies = [ name = "example_invoke_contract" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -512,7 +512,7 @@ dependencies = [ name = "example_linear_memory" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -520,7 +520,15 @@ dependencies = [ name = "example_simple_account" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", + "soroban-sdk", +] + +[[package]] +name = "example_sum_i32" +version = "0.0.0" +dependencies = [ + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -528,7 +536,7 @@ dependencies = [ name = "example_updateable_contract" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -536,7 +544,7 @@ dependencies = [ name = "example_vec" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -643,7 +651,7 @@ dependencies = [ name = "hostile_large_val" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -767,7 +775,7 @@ checksum = "4ec2a862134d2a7d32d7983ddcdd1c4923530833c9f2ea1a44fc5fa473989058" name = "loadgen" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -944,7 +952,7 @@ dependencies = [ name = "recursive_account" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -983,7 +991,7 @@ checksum = "1ad4cc8da4ef723ed60bced201181d83791ad433213d8c24efffda1eec85d741" name = "sac_reentry_account" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -1106,7 +1114,8 @@ checksum = "4dccd0940a2dcdf68d092b8cbab7dc0ad8fa938bf95787e1b916b0e3d0e8e970" [[package]] name = "soroban-builtin-sdk-macros" -version = "20.1.0" +version = "20.2.1" +source = "git+https://github.com/stellar/rs-soroban-env?rev=18a10592853d9edf4e341b565b0b1638f95f0393#18a10592853d9edf4e341b565b0b1638f95f0393" dependencies = [ "itertools", "proc-macro2", @@ -1116,7 +1125,8 @@ dependencies = [ [[package]] name = "soroban-env-common" -version = "20.1.0" +version = "20.2.1" +source = "git+https://github.com/stellar/rs-soroban-env?rev=18a10592853d9edf4e341b565b0b1638f95f0393#18a10592853d9edf4e341b565b0b1638f95f0393" dependencies = [ "arbitrary", "crate-git-revision", @@ -1124,23 +1134,38 @@ dependencies = [ "num-derive", "num-traits", "serde", - "soroban-env-macros", + "soroban-env-macros 20.2.1", "soroban-wasmi", "static_assertions", - "stellar-xdr", + "stellar-xdr 20.1.0 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "soroban-env-common" +version = "20.3.0" +dependencies = [ + "crate-git-revision", + "ethnum", + "num-derive", + "num-traits", + "soroban-env-macros 20.3.0", + "static_assertions", + "stellar-xdr 20.1.0 (git+https://github.com/stellar/rs-stellar-xdr?rev=44b7e2d4cdf27a3611663e82828de56c5274cba0)", ] [[package]] name = "soroban-env-guest" -version = "20.1.0" +version = "20.2.1" +source = "git+https://github.com/stellar/rs-soroban-env?rev=18a10592853d9edf4e341b565b0b1638f95f0393#18a10592853d9edf4e341b565b0b1638f95f0393" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.2.1", "static_assertions", ] [[package]] name = "soroban-env-host" -version = "20.1.0" +version = "20.2.1" +source = "git+https://github.com/stellar/rs-soroban-env?rev=18a10592853d9edf4e341b565b0b1638f95f0393#18a10592853d9edf4e341b565b0b1638f95f0393" dependencies = [ "backtrace", "curve25519-dalek", @@ -1157,7 +1182,7 @@ dependencies = [ "sha2", "sha3", "soroban-builtin-sdk-macros", - "soroban-env-common", + "soroban-env-common 20.2.1", "soroban-wasmi", "static_assertions", "stellar-strkey", @@ -1165,32 +1190,46 @@ dependencies = [ [[package]] name = "soroban-env-macros" -version = "20.1.0" +version = "20.2.1" +source = "git+https://github.com/stellar/rs-soroban-env?rev=18a10592853d9edf4e341b565b0b1638f95f0393#18a10592853d9edf4e341b565b0b1638f95f0393" +dependencies = [ + "itertools", + "proc-macro2", + "quote", + "serde", + "serde_json", + "stellar-xdr 20.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + "syn", +] + +[[package]] +name = "soroban-env-macros" +version = "20.3.0" dependencies = [ "itertools", "proc-macro2", "quote", "serde", "serde_json", - "stellar-xdr", + "stellar-xdr 20.1.0 (git+https://github.com/stellar/rs-stellar-xdr?rev=44b7e2d4cdf27a3611663e82828de56c5274cba0)", "syn", ] [[package]] name = "soroban-ledger-snapshot" -version = "20.1.0" +version = "20.3.0" dependencies = [ "serde", "serde_json", "serde_with", - "soroban-env-common", + "soroban-env-common 20.2.1", "soroban-env-host", "thiserror", ] [[package]] name = "soroban-sdk" -version = "20.1.0" +version = "20.3.0" dependencies = [ "arbitrary", "bytes-lit", @@ -1208,7 +1247,7 @@ dependencies = [ [[package]] name = "soroban-sdk-macros" -version = "20.1.0" +version = "20.3.0" dependencies = [ "crate-git-revision", "darling", @@ -1217,41 +1256,41 @@ dependencies = [ "quote", "rustc_version", "sha2", - "soroban-env-common", + "soroban-env-common 20.2.1", "soroban-spec", "soroban-spec-rust", - "stellar-xdr", + "stellar-xdr 20.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "syn", ] [[package]] name = "soroban-spec" -version = "20.1.0" +version = "20.3.0" dependencies = [ "base64 0.13.1", - "stellar-xdr", + "stellar-xdr 20.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "thiserror", "wasmparser", ] [[package]] name = "soroban-spec-rust" -version = "20.1.0" +version = "20.3.0" dependencies = [ "prettyplease", "proc-macro2", "quote", "sha2", "soroban-spec", - "stellar-xdr", + "stellar-xdr 20.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "syn", "thiserror", ] [[package]] name = "soroban-wasmi" -version = "0.31.1-soroban.20.0.0" -source = "git+https://github.com/stellar/wasmi?rev=ab29800224d85ee64d4ac127bac84cdbb0276721#ab29800224d85ee64d4ac127bac84cdbb0276721" +version = "0.31.1-soroban.20.0.1" +source = "git+https://github.com/stellar/wasmi?rev=0ed3f3dee30dc41ebe21972399e0a73a41944aa0#0ed3f3dee30dc41ebe21972399e0a73a41944aa0" dependencies = [ "smallvec", "spin", @@ -1264,7 +1303,7 @@ dependencies = [ name = "soroban-write-upgrade-bytes-contract" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -1303,9 +1342,9 @@ dependencies = [ [[package]] name = "stellar-xdr" -version = "20.0.2" +version = "20.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e9f00a85bd9b1617d4cb7e741733889c9940e6bdeca360db81752b0ef04fe3a5" +checksum = "e59cdf3eb4467fb5a4b00b52e7de6dca72f67fac6f9b700f55c95a5d86f09c9d" dependencies = [ "arbitrary", "base64 0.13.1", @@ -1317,6 +1356,17 @@ dependencies = [ "stellar-strkey", ] +[[package]] +name = "stellar-xdr" +version = "20.1.0" +source = "git+https://github.com/stellar/rs-stellar-xdr?rev=44b7e2d4cdf27a3611663e82828de56c5274cba0#44b7e2d4cdf27a3611663e82828de56c5274cba0" +dependencies = [ + "crate-git-revision", + "escape-bytes", + "hex", + "stellar-strkey", +] + [[package]] name = "strsim" version = "0.10.0" @@ -1344,7 +1394,7 @@ dependencies = [ name = "test_conditional_account" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -1352,7 +1402,7 @@ dependencies = [ name = "test_delegated_account" version = "0.0.0" dependencies = [ - "soroban-env-common", + "soroban-env-common 20.3.0", "soroban-sdk", ] @@ -1486,12 +1536,12 @@ checksum = "0d046c5d029ba91a1ed14da14dca44b68bf2f124cfbaf741c54151fdb3e0750b" [[package]] name = "wasmi_arena" version = "0.4.0" -source = "git+https://github.com/stellar/wasmi?rev=ab29800224d85ee64d4ac127bac84cdbb0276721#ab29800224d85ee64d4ac127bac84cdbb0276721" +source = "git+https://github.com/stellar/wasmi?rev=0ed3f3dee30dc41ebe21972399e0a73a41944aa0#0ed3f3dee30dc41ebe21972399e0a73a41944aa0" [[package]] name = "wasmi_core" version = "0.13.0" -source = "git+https://github.com/stellar/wasmi?rev=ab29800224d85ee64d4ac127bac84cdbb0276721#ab29800224d85ee64d4ac127bac84cdbb0276721" +source = "git+https://github.com/stellar/wasmi?rev=0ed3f3dee30dc41ebe21972399e0a73a41944aa0#0ed3f3dee30dc41ebe21972399e0a73a41944aa0" dependencies = [ "downcast-rs", "libm", @@ -1588,3 +1638,11 @@ name = "zeroize" version = "1.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2a0956f1ba7c7909bfb66c2e9e4124ab6f6482560f6628b5aaeba39207c9aad9" + +[[patch.unused]] +name = "soroban-env-guest" +version = "20.3.0" + +[[patch.unused]] +name = "soroban-env-host" +version = "20.3.0" diff --git a/soroban-test-wasms/wasm-workspace/Cargo.toml b/soroban-test-wasms/wasm-workspace/Cargo.toml index 8a8423ad3..8c1e2753a 100644 --- a/soroban-test-wasms/wasm-workspace/Cargo.toml +++ b/soroban-test-wasms/wasm-workspace/Cargo.toml @@ -40,7 +40,8 @@ members = [ "sac_reentry_account", "recursive_account", "hostile_large_val", - "contract_sac_transfer" + "contract_sac_transfer", + "sum_i32" ] [profile.release] opt-level = "z" @@ -56,11 +57,11 @@ lto = true rust-version = "1.74.0" [workspace.dependencies.soroban-sdk] -version = "=20.2.0" +version = "=20.3.0" git = "https://github.com/stellar/rs-soroban-sdk" [workspace.dependencies.soroban-env-common] -version = "=20.1.1" +version = "=20.3.0" git = "https://github.com/stellar/rs-soroban-env" # Always build from the local instance of env as we need to rebuild test WASMs diff --git a/soroban-test-wasms/wasm-workspace/opt/curr/example_sum_i32.wasm b/soroban-test-wasms/wasm-workspace/opt/curr/example_sum_i32.wasm new file mode 100644 index 0000000000000000000000000000000000000000..0a6a7b5f56fa2b4cd93a7801294f01afd5484a2b GIT binary patch literal 689 zcmY*X!EVz)5S^LbO`@8BQ4vxvXx;Y0f#Ad~$uY4698vQF?j~MAQzeaTH>gtOL~4Y3 z;x`ZyocICmhzp;=Pn3yk5@lANowxhu&9h}N=@l5}R1f&rCgqup+7 ztkjY;jnD6|k6V5tX!r-6L95m341D7UgI3cF2R3MS+J4Zs#@biGk@bx|>iFhGe3oQa Gtosl7^ob|{ literal 0 HcmV?d00001 diff --git a/soroban-test-wasms/wasm-workspace/sum_i32/Cargo.toml b/soroban-test-wasms/wasm-workspace/sum_i32/Cargo.toml new file mode 100644 index 000000000..953fea5bc --- /dev/null +++ b/soroban-test-wasms/wasm-workspace/sum_i32/Cargo.toml @@ -0,0 +1,19 @@ +[package] +name = "example_sum_i32" +version = "0.0.0" +authors = ["Stellar Development Foundation "] +license = "Apache-2.0" +edition = "2021" +publish = false +rust-version.workspace = true + +[lib] +crate-type = ["cdylib", "rlib"] +doctest = false + +[dependencies] +soroban-sdk = { workspace = true } +soroban-env-common = { workspace = true } + +[features] +next = ["soroban-env-common/next"] diff --git a/soroban-test-wasms/wasm-workspace/sum_i32/src/lib.rs b/soroban-test-wasms/wasm-workspace/sum_i32/src/lib.rs new file mode 100644 index 000000000..0e2854d82 --- /dev/null +++ b/soroban-test-wasms/wasm-workspace/sum_i32/src/lib.rs @@ -0,0 +1,22 @@ +#![no_std] +use soroban_sdk::{contract, contractimpl, symbol_short, vec, Address, Env, IntoVal, Vec}; + +#[contract] +pub struct Contract; + +#[contractimpl] +impl Contract { + pub fn sum(env: Env, add: Address, ints: Vec) -> i32 { + let mut sum: i32 = 0; + for i in ints { + sum = env + .invoke_contract::( + &add, + &symbol_short!("add"), + vec![&env, sum.into_val(&env), i.into_val(&env)], + ) + .into(); + } + sum + } +} From c24e4fbc33fdb5c44c128bea14aadf4931179de1 Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Fri, 29 Mar 2024 01:09:26 -0700 Subject: [PATCH 04/19] Remove Rc::metered_new on ParsedModule -- breaks replay and already covered --- soroban-env-host/src/host/declared_size.rs | 7 ------ soroban-env-host/src/vm/parsed_module.rs | 29 ++++++++-------------- 2 files changed, 11 insertions(+), 25 deletions(-) diff --git a/soroban-env-host/src/host/declared_size.rs b/soroban-env-host/src/host/declared_size.rs index 85c82b2e0..d6ab4a431 100644 --- a/soroban-env-host/src/host/declared_size.rs +++ b/soroban-env-host/src/host/declared_size.rs @@ -13,7 +13,6 @@ use crate::{ host::{frame::Context, Events}, host_object::HostObject, storage::AccessType, - vm::ParsedModule, xdr::{ AccountEntry, AccountId, Asset, BytesM, ContractCodeEntry, ContractDataDurability, ContractEvent, ContractExecutable, ContractIdPreimage, CreateContractArgs, Duration, @@ -131,7 +130,6 @@ impl_declared_size_type!(AccountAuthorizationTrackerSnapshot, 40); impl_declared_size_type!(InvokerContractAuthorizationTracker, 192); impl_declared_size_type!(InternalDiagnosticArg, 64); impl_declared_size_type!(InternalDiagnosticEvent, 88); -impl_declared_size_type!(ParsedModule, 304); // xdr types impl_declared_size_type!(TimePoint, 8); @@ -449,10 +447,6 @@ mod test { expect!["64"].assert_eq(size_of::().to_string().as_str()); expect!["88"].assert_eq(size_of::().to_string().as_str()); - // NB: ParsedModule changes size depending on release or debug builds, - // which is impossible to conditionalize on. We should figure out how to - // check it someday but until we do we'll leaveit out. - // xdr types expect!["8"].assert_eq(size_of::().to_string().as_str()); expect!["8"].assert_eq(size_of::().to_string().as_str()); @@ -648,7 +642,6 @@ mod test { assert_mem_size_le_declared_size!(InvokerContractAuthorizationTracker); assert_mem_size_le_declared_size!(InternalDiagnosticArg); assert_mem_size_le_declared_size!(InternalDiagnosticEvent); - assert_mem_size_le_declared_size!(ParsedModule); // xdr types assert_mem_size_le_declared_size!(TimePoint); diff --git a/soroban-env-host/src/vm/parsed_module.rs b/soroban-env-host/src/vm/parsed_module.rs index 63657bd7a..307821000 100644 --- a/soroban-env-host/src/vm/parsed_module.rs +++ b/soroban-env-host/src/vm/parsed_module.rs @@ -1,6 +1,5 @@ use crate::{ err, - host::metered_clone::MeteredAlloc, meta::{self, get_ledger_protocol_version}, xdr::{ContractCostType, Limited, ReadXdr, ScEnvMetaEntry, ScErrorCode, ScErrorType}, Host, HostError, DEFAULT_XDR_RW_LIMITS, @@ -157,14 +156,11 @@ impl ParsedModule { ) -> Result, HostError> { cost_inputs.charge_for_parsing(host)?; let (module, proto_version) = Self::parse_wasm(host, engine, wasm)?; - Rc::metered_new( - Self { - module, - proto_version, - cost_inputs, - }, - host, - ) + Ok(Rc::new(Self { + module, + proto_version, + cost_inputs, + })) } pub fn with_import_symbols( @@ -197,7 +193,7 @@ impl ParsedModule { self.with_import_symbols(|symbols| Host::make_linker(self.module.engine(), symbols)) } - #[cfg(any(test, feature = "testutils"))] + #[cfg(feature = "bench")] pub fn new_with_isolated_engine( host: &Host, wasm: &[u8], @@ -208,14 +204,11 @@ impl ParsedModule { let engine = Engine::new(&config); cost_inputs.charge_for_parsing(host)?; let (module, proto_version) = Self::parse_wasm(host, &engine, wasm)?; - Rc::metered_new( - Self { - module, - proto_version, - cost_inputs, - }, - host, - ) + Ok(Rc::new(Self { + module, + proto_version, + cost_inputs, + })) } /// Parse the Wasm blob into a [Module] and its protocol number, checking its interface version From fad209b4b0e6bcbf3ddc6b0c10dc3e74be217a00 Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Fri, 29 Mar 2024 01:17:06 -0700 Subject: [PATCH 05/19] Recalibrate --- soroban-env-host/src/budget.rs | 43 +++++++++++++++++----------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/soroban-env-host/src/budget.rs b/soroban-env-host/src/budget.rs index 00d6dae6c..1545f3826 100644 --- a/soroban-env-host/src/budget.rs +++ b/soroban-env-host/src/budget.rs @@ -366,7 +366,7 @@ impl Default for BudgetImpl { cpu.lin_term = ScaledU64(45405); } ContractCostType::VmCachedInstantiation => { - cpu.const_term = 24457; + cpu.const_term = 41142; cpu.lin_term = ScaledU64(634); } ContractCostType::InvokeVmFunction => { @@ -412,18 +412,18 @@ impl Default for BudgetImpl { #[cfg(feature = "next")] ContractCostType::ParseWasmInstructions => { - cpu.const_term = 73261; - cpu.lin_term = ScaledU64(25409); + cpu.const_term = 73077; + cpu.lin_term = ScaledU64(25410); } #[cfg(feature = "next")] ContractCostType::ParseWasmFunctions => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(540756); + cpu.lin_term = ScaledU64(540752); } #[cfg(feature = "next")] ContractCostType::ParseWasmGlobals => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(176366); + cpu.lin_term = ScaledU64(176363); } #[cfg(feature = "next")] ContractCostType::ParseWasmTableEntries => { @@ -433,27 +433,27 @@ impl Default for BudgetImpl { #[cfg(feature = "next")] ContractCostType::ParseWasmTypes => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(1061438); + cpu.lin_term = ScaledU64(1061449); } #[cfg(feature = "next")] ContractCostType::ParseWasmDataSegments => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(237323); + cpu.lin_term = ScaledU64(237336); } #[cfg(feature = "next")] ContractCostType::ParseWasmElemSegments => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(328468); + cpu.lin_term = ScaledU64(328476); } #[cfg(feature = "next")] ContractCostType::ParseWasmImports => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(701838); + cpu.lin_term = ScaledU64(701845); } #[cfg(feature = "next")] ContractCostType::ParseWasmExports => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(429378); + cpu.lin_term = ScaledU64(429383); } #[cfg(feature = "next")] ContractCostType::ParseWasmDataSegmentBytes => { @@ -462,23 +462,23 @@ impl Default for BudgetImpl { } #[cfg(feature = "next")] ContractCostType::InstantiateWasmInstructions => { - cpu.const_term = 26338; + cpu.const_term = 43030; cpu.lin_term = ScaledU64(0); } #[cfg(feature = "next")] ContractCostType::InstantiateWasmFunctions => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(7580); + cpu.lin_term = ScaledU64(7556); } #[cfg(feature = "next")] ContractCostType::InstantiateWasmGlobals => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(10663); + cpu.lin_term = ScaledU64(10711); } #[cfg(feature = "next")] ContractCostType::InstantiateWasmTableEntries => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(1837); + cpu.lin_term = ScaledU64(3300); } #[cfg(feature = "next")] ContractCostType::InstantiateWasmTypes => { @@ -488,22 +488,22 @@ impl Default for BudgetImpl { #[cfg(feature = "next")] ContractCostType::InstantiateWasmDataSegments => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(22893); + cpu.lin_term = ScaledU64(23038); } #[cfg(feature = "next")] ContractCostType::InstantiateWasmElemSegments => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(41094); + cpu.lin_term = ScaledU64(42488); } #[cfg(feature = "next")] ContractCostType::InstantiateWasmImports => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(691499); + cpu.lin_term = ScaledU64(828974); } #[cfg(feature = "next")] ContractCostType::InstantiateWasmExports => { cpu.const_term = 0; - cpu.lin_term = ScaledU64(297122); + cpu.lin_term = ScaledU64(297100); } #[cfg(feature = "next")] ContractCostType::InstantiateWasmDataSegmentBytes => { @@ -570,7 +570,7 @@ impl Default for BudgetImpl { mem.lin_term = ScaledU64(5064); } ContractCostType::VmCachedInstantiation => { - mem.const_term = 68960; + mem.const_term = 69472; mem.lin_term = ScaledU64(1217); } ContractCostType::InvokeVmFunction => { @@ -664,10 +664,9 @@ impl Default for BudgetImpl { mem.const_term = 0; mem.lin_term = ScaledU64(257); } - #[cfg(feature = "next")] ContractCostType::InstantiateWasmInstructions => { - mem.const_term = 70192; + mem.const_term = 70704; mem.lin_term = ScaledU64(0); } #[cfg(feature = "next")] @@ -703,7 +702,7 @@ impl Default for BudgetImpl { #[cfg(feature = "next")] ContractCostType::InstantiateWasmImports => { mem.const_term = 0; - mem.lin_term = ScaledU64(76483); + mem.lin_term = ScaledU64(97637); } #[cfg(feature = "next")] ContractCostType::InstantiateWasmExports => { From 8a2f0711b9de9943c11098a738fae162ee5ecdd1 Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Fri, 29 Mar 2024 01:43:48 -0700 Subject: [PATCH 06/19] Fix more tests --- soroban-env-host/src/host/error.rs | 4 ++++ soroban-env-host/src/test/lifecycle.rs | 3 ++- soroban-env-host/src/testutils.rs | 1 - soroban-env-host/src/vm.rs | 18 ++++++++++-------- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/soroban-env-host/src/host/error.rs b/soroban-env-host/src/host/error.rs index c1ec27c72..57d0bb499 100644 --- a/soroban-env-host/src/host/error.rs +++ b/soroban-env-host/src/host/error.rs @@ -96,6 +96,10 @@ impl DebugInfo { || frame_name_matches(frame, "Host::err") || frame_name_matches(frame, "Host>::err") || frame_name_matches(frame, "::augment_err_result") + || frame_name_matches(frame, "::with_shadow_mode") + || frame_name_matches(frame, "::with_debug_mode") + || frame_name_matches(frame, "::maybe_get_debug_info") + || frame_name_matches(frame, "::map_err") } let mut bt = self.backtrace.clone(); bt.resolve(); diff --git a/soroban-env-host/src/test/lifecycle.rs b/soroban-env-host/src/test/lifecycle.rs index 8cce56ac9..7a9e04303 100644 --- a/soroban-env-host/src/test/lifecycle.rs +++ b/soroban-env-host/src/test/lifecycle.rs @@ -72,7 +72,6 @@ fn test_host() -> Host { ..Default::default() }) .unwrap(); - host.maybe_add_module_cache().unwrap(); host } @@ -220,6 +219,8 @@ fn create_contract_using_parent_id_test() { ); assert_eq!(child_wasm, get_contract_wasm(&host, wasm_hash.clone())); + host.maybe_add_module_cache().unwrap(); + // Now successfully create the child contract itself. host.call( parent_contract_address, diff --git a/soroban-env-host/src/testutils.rs b/soroban-env-host/src/testutils.rs index 8b0b69ba0..981481845 100644 --- a/soroban-env-host/src/testutils.rs +++ b/soroban-env-host/src/testutils.rs @@ -202,7 +202,6 @@ impl Host { max_entry_ttl: 6_312_000, }) .unwrap(); - host.maybe_add_module_cache().unwrap(); host } diff --git a/soroban-env-host/src/vm.rs b/soroban-env-host/src/vm.rs index 95c8cc837..2a8f216ad 100644 --- a/soroban-env-host/src/vm.rs +++ b/soroban-env-host/src/vm.rs @@ -130,6 +130,7 @@ impl Vm { host: &Host, contract_id: Hash, parsed_module: Rc, + linker: &Linker, ) -> Result, HostError> { let _span = tracy_span!("Vm::instantiate"); @@ -191,12 +192,7 @@ impl Vm { let not_started_instance = { let _span0 = tracy_span!("instantiate module"); - if let Some(linker) = &*host.try_borrow_linker()? { - host.map_err(linker.instantiate(&mut store, &parsed_module.module))? - } else { - let linker = parsed_module.make_linker()?; - host.map_err(linker.instantiate(&mut store, &parsed_module.module))? - } + host.map_err(linker.instantiate(&mut store, &parsed_module.module))? }; let instance = host.map_err( @@ -230,7 +226,12 @@ impl Vm { ) -> Result, HostError> { let _span = tracy_span!("Vm::from_parsed_module"); VmInstantiationTimer::new(host.clone()); - Self::instantiate(host, contract_id, parsed_module) + if let Some(linker) = &*host.try_borrow_linker()? { + Self::instantiate(host, contract_id, parsed_module, linker) + } else { + let linker = parsed_module.make_linker()?; + Self::instantiate(host, contract_id, parsed_module, &linker) + } } /// Constructs a new instance of a [Vm] within the provided [Host], @@ -270,7 +271,8 @@ impl Vm { let config = get_wasmi_config(host.as_budget())?; let engine = wasmi::Engine::new(&config); let parsed_module = Self::parse_module(host, &engine, wasm, cost_inputs)?; - Self::instantiate(host, contract_id, parsed_module) + let linker = parsed_module.make_linker()?; + Self::instantiate(host, contract_id, parsed_module, &linker) } #[cfg(not(any(test, feature = "recording_mode")))] From f6be5373740a74b4cc86a9fcb180bd738cfbeb68 Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Mon, 1 Apr 2024 14:39:16 -0700 Subject: [PATCH 07/19] Repair non-next-feature-gated tests --- soroban-env-host/src/budget.rs | 12 +++++ soroban-env-host/src/e2e_invoke.rs | 1 - soroban-env-host/src/host.rs | 12 ++++- soroban-env-host/src/host/frame.rs | 9 +++- soroban-env-host/src/test/basic.rs | 10 ++++- soroban-env-host/src/test/budget_metering.rs | 15 +++---- soroban-env-host/src/test/bytes.rs | 2 +- soroban-env-host/src/test/e2e_tests.rs | 2 +- soroban-env-host/src/test/event.rs | 22 ++++----- soroban-env-host/src/test/lifecycle.rs | 7 +-- soroban-env-host/src/testutils.rs | 47 ++++++++++++++------ soroban-env-host/src/vm.rs | 4 +- 12 files changed, 100 insertions(+), 43 deletions(-) diff --git a/soroban-env-host/src/budget.rs b/soroban-env-host/src/budget.rs index 1545f3826..ec0c30bfb 100644 --- a/soroban-env-host/src/budget.rs +++ b/soroban-env-host/src/budget.rs @@ -365,6 +365,12 @@ impl Default for BudgetImpl { cpu.const_term = 451626; cpu.lin_term = ScaledU64(45405); } + #[cfg(not(feature = "next"))] + ContractCostType::VmCachedInstantiation => { + cpu.const_term = 451626; + cpu.lin_term = ScaledU64(45405); + } + #[cfg(feature = "next")] ContractCostType::VmCachedInstantiation => { cpu.const_term = 41142; cpu.lin_term = ScaledU64(634); @@ -569,6 +575,12 @@ impl Default for BudgetImpl { mem.const_term = 130065; mem.lin_term = ScaledU64(5064); } + #[cfg(not(feature = "next"))] + ContractCostType::VmCachedInstantiation => { + mem.const_term = 130065; + mem.lin_term = ScaledU64(5064); + } + #[cfg(feature = "next")] ContractCostType::VmCachedInstantiation => { mem.const_term = 69472; mem.lin_term = ScaledU64(1217); diff --git a/soroban-env-host/src/e2e_invoke.rs b/soroban-env-host/src/e2e_invoke.rs index aa51bb85a..81a141d92 100644 --- a/soroban-env-host/src/e2e_invoke.rs +++ b/soroban-env-host/src/e2e_invoke.rs @@ -337,7 +337,6 @@ 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( diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index 460b602ad..86c8b98f1 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -361,7 +361,7 @@ impl Host { })) } - pub fn maybe_add_module_cache(&self) -> Result<(), HostError> { + pub fn build_module_cache_if_needed(&self) -> Result<(), HostError> { if cfg!(feature = "next") && self.get_ledger_protocol_version()? >= ModuleCache::MIN_LEDGER_VERSION && self.try_borrow_module_cache()?.is_none() @@ -374,6 +374,16 @@ impl Host { Ok(()) } + #[cfg(any(test, feature = "recording_mode"))] + pub fn rebuild_module_cache(&self) -> Result<(), HostError> { + if cfg!(feature = "next") { + *self.try_borrow_module_cache_mut()? = None; + *self.try_borrow_linker_mut()? = None; + self.build_module_cache_if_needed()?; + } + Ok(()) + } + pub fn set_source_account(&self, source_account: AccountId) -> Result<(), HostError> { *self.try_borrow_source_account_mut()? = Some(source_account); Ok(()) diff --git a/soroban-env-host/src/host/frame.rs b/soroban-env-host/src/host/frame.rs index b207f80ac..5e2634845 100644 --- a/soroban-env-host/src/host/frame.rs +++ b/soroban-env-host/src/host/frame.rs @@ -195,7 +195,7 @@ impl Host { .maybe_emulate_authentication(self)?; // 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. - self.maybe_add_module_cache()?; + self.rebuild_module_cache()?; } let mut auth_snapshot = None; if let Some(rp) = orp { @@ -643,11 +643,18 @@ impl Host { let args_vec = args.to_vec(); match &instance.executable { ContractExecutable::Wasm(wasm_hash) => { + // If the module cache is not yet built, build it now, before first access. + self.build_module_cache_if_needed()?; let contract_id = id.metered_clone(self)?; let vm = if let Some(cache) = &*self.try_borrow_module_cache()? { + // This branch should be taken if we're on a protocol that + // supports the module cache. let module = cache.get_module(self, wasm_hash)?; Vm::from_parsed_module(self, contract_id, module)? } else { + // If we got here we're running/replaying a protocol version + // with no module cache. We'll parse the contract anew and + // throw it away, as we did in that old protocol. let (code, costs) = self.retrieve_wasm_from_storage(&wasm_hash)?; Vm::new_with_cost_inputs(self, contract_id, code.as_slice(), costs)? }; diff --git a/soroban-env-host/src/test/basic.rs b/soroban-env-host/src/test/basic.rs index 4428748a9..53c75054e 100644 --- a/soroban-env-host/src/test/basic.rs +++ b/soroban-env-host/src/test/basic.rs @@ -76,8 +76,14 @@ fn tuple_roundtrip() -> Result<(), HostError> { #[test] fn f32_does_not_work() -> Result<(), HostError> { use soroban_env_common::xdr::Hash; - let host = observe_host!(Host::default()); + let host = observe_host!(Host::test_host()); let hash = Hash::from([0; 32]); - assert!(crate::vm::Vm::new(&host, hash, soroban_test_wasms::ADD_F32).is_err()); + assert!(HostError::result_matches_err( + crate::vm::Vm::new(&host, hash, soroban_test_wasms::ADD_F32), + ( + crate::xdr::ScErrorType::WasmVm, + crate::xdr::ScErrorCode::InvalidAction + ) + )); Ok(()) } diff --git a/soroban-env-host/src/test/budget_metering.rs b/soroban-env-host/src/test/budget_metering.rs index ac623a982..dca985298 100644 --- a/soroban-env-host/src/test/budget_metering.rs +++ b/soroban-env-host/src/test/budget_metering.rs @@ -11,7 +11,7 @@ use soroban_test_wasms::VEC; #[test] fn xdr_object_conversion() -> Result<(), HostError> { - let host = observe_host!(Host::test_host()); + let host = observe_host!(Host::test_host_with_prng()); let _ = host.clone().test_budget(100_000, 100_000).enable_model( ContractCostType::MemCpy, 1, @@ -170,7 +170,7 @@ fn test_vm_fuel_metering() -> Result<(), HostError> { #[test] fn metered_xdr() -> Result<(), HostError> { - let host = Host::test_host() + let host = Host::test_host_with_prng() .test_budget(100_000, 100_000) .enable_model(ContractCostType::ValSer, 0, 10, 0, 1) .enable_model(ContractCostType::ValDeser, 0, 10, 0, 1); @@ -210,10 +210,9 @@ fn metered_xdr() -> Result<(), HostError> { #[test] fn metered_xdr_out_of_budget() -> Result<(), HostError> { - let host = - Host::test_host() - .test_budget(10, 10) - .enable_model(ContractCostType::ValSer, 0, 10, 0, 1); + let host = Host::test_host_with_prng() + .test_budget(10, 10) + .enable_model(ContractCostType::ValSer, 0, 10, 0, 1); let scmap: ScMap = host.map_err( vec![ ScMapEntry { @@ -236,7 +235,7 @@ fn metered_xdr_out_of_budget() -> Result<(), HostError> { #[test] fn map_insert_key_vec_obj() -> Result<(), HostError> { - let mut host = Host::test_host().test_budget(1000, 1000); + let mut host = Host::test_host_with_prng().test_budget(1000, 1000); let mut m = host.map_new()?; let k0 = host.test_vec_obj(&[2, 3])?; let v0: Val = 6_u32.into(); @@ -274,7 +273,7 @@ fn map_insert_key_vec_obj() -> Result<(), HostError> { #[test] fn test_recursive_type_clone() -> Result<(), HostError> { - let host = Host::test_host() + let host = Host::test_host_with_prng() .test_budget(100000, 100000) .enable_model(ContractCostType::MemAlloc, 10, 0, 1, 0) .enable_model(ContractCostType::MemCpy, 10, 0, 1, 0); diff --git a/soroban-env-host/src/test/bytes.rs b/soroban-env-host/src/test/bytes.rs index 855b840c9..5b5f749c9 100644 --- a/soroban-env-host/src/test/bytes.rs +++ b/soroban-env-host/src/test/bytes.rs @@ -267,7 +267,7 @@ fn bytes_xdr_roundtrip() -> Result<(), HostError> { #[test] fn arbitrary_xdr_roundtrips() -> Result<(), HostError> { const ITERATIONS: u32 = 50_000; - let host = Host::test_host(); + let host = Host::test_host_with_prng(); host.budget_ref().reset_unlimited().unwrap(); let mut successes = 0; diff --git a/soroban-env-host/src/test/e2e_tests.rs b/soroban-env-host/src/test/e2e_tests.rs index c1a31f53c..d7f2ff8f1 100644 --- a/soroban-env-host/src/test/e2e_tests.rs +++ b/soroban-env-host/src/test/e2e_tests.rs @@ -99,7 +99,7 @@ fn sign_auth_entry( match &mut out.credentials { SorobanCredentials::SourceAccount => {} SorobanCredentials::Address(creds) => { - let dummy_host = Host::test_host(); + let dummy_host = Host::test_host_with_prng(); let signature_payload_preimage = HashIdPreimage::SorobanAuthorization(HashIdPreimageSorobanAuthorization { network_id: ledger_info.network_id.try_into().unwrap(), diff --git a/soroban-env-host/src/test/event.rs b/soroban-env-host/src/test/event.rs index f19c22375..63b84a436 100644 --- a/soroban-env-host/src/test/event.rs +++ b/soroban-env-host/src/test/event.rs @@ -108,7 +108,7 @@ fn test_event_rollback() -> Result<(), HostError> { #[test] fn test_internal_contract_events_metering_not_free() -> Result<(), HostError> { - let host = observe_host!(Host::test_host()); + let host = observe_host!(Host::test_host_with_prng()); let dummy_id = [0; 32]; let ce = InternalContractEvent { type_: ContractEventType::Contract, @@ -148,7 +148,7 @@ fn test_internal_diagnostic_event_metering_free() -> Result<(), HostError> { args, }); - let host = observe_host!(Host::test_host()); + let host = observe_host!(Host::test_host_with_prng()); let _ = host .clone() .test_budget(100000, 100000) @@ -193,7 +193,7 @@ fn test_diagnostic_events_do_not_affect_metering_with_debug_off() -> Result<(), // DEBUG mode OFF // NB: We don't observe here since the test is sensitive to shadow budget. - let host = Host::test_host(); + let host = Host::test_host_with_prng(); let budget = host.as_budget().clone(); budget.reset_default()?; let evts = log_some_diagnostics(host)?; @@ -211,7 +211,7 @@ fn test_diagnostic_events_do_not_affect_metering_with_debug_on_and_sufficient_bu // DEBUG mode ON, budget sufficient // NB: We don't observe here since the test is sensitive to shadow budget. - let host = Host::test_host(); + let host = Host::test_host_with_prng(); host.enable_debug()?; let budget = host.as_budget().clone(); budget.reset_default()?; @@ -230,7 +230,7 @@ fn test_diagnostic_events_do_not_affect_metering_with_debug_on_and_insufficient_ // DEBUG mode ON, budget insufficient // NB: We don't observe here since the test is sensitive to shadow budget. - let host = Host::test_host(); + let host = Host::test_host_with_prng(); host.enable_debug()?; let budget = host.as_budget().clone(); budget.reset_default()?; @@ -252,7 +252,7 @@ fn test_diagnostic_events_do_not_affect_metering_with_debug_on_and_insufficient_ // diagnostic events for any failed borrows. We actually don't want that to // happen, we want failed borrows from the tracing subsystem to be silent. fn test_observation_does_not_emit_diagnostic_events_from_failed_borrows() -> Result<(), HostError> { - let host = Host::test_host(); + let host = Host::test_host_with_prng(); let obs_host = observe_host!(host.clone()); host.enable_debug()?; let storage = host.try_borrow_storage_mut()?; @@ -267,7 +267,7 @@ fn test_observation_does_not_emit_diagnostic_events_from_failed_borrows() -> Res #[test] fn nonexistent_topic_obj_handle() -> Result<(), HostError> { - let host = Host::test_host(); + let host = Host::test_host_with_prng(); let data = Val::from_void().to_val(); let topics = unsafe { VecObject::from_handle(123) }; // non-existent handle let res = host.contract_event(topics, data); @@ -282,7 +282,7 @@ fn nonexistent_topic_obj_handle() -> Result<(), HostError> { fn too_many_event_topics() -> Result<(), HostError> { let topics: Vec<_> = (0..0x00FFFFFF).map(|u| Val::from_u32(u).to_val()).collect(); // We don't observe this test: it makes way too big a trace. - let host = Host::test_host(); + let host = Host::test_host_with_prng(); let budget = host.as_budget().clone(); budget.reset_unlimited()?; let topics = host.vec_new_from_slice(topics.as_slice())?; @@ -306,7 +306,7 @@ fn too_many_event_topics() -> Result<(), HostError> { #[test] fn too_big_event_topic() -> Result<(), HostError> { - let host = observe_host!(Host::test_host()); + let host = observe_host!(Host::test_host_with_prng()); let budget = host.as_budget().clone(); budget.reset_unlimited()?; let bytes = host.bytes_new_from_slice(&[0; 0x0FFFFFFF])?; @@ -326,7 +326,7 @@ fn too_big_event_topic() -> Result<(), HostError> { } #[test] fn too_big_event_data() -> Result<(), HostError> { - let host = observe_host!(Host::test_host()); + let host = observe_host!(Host::test_host_with_prng()); let budget = host.as_budget().clone(); budget.reset_unlimited()?; let bytes = host.bytes_new_from_slice(&[0; 0x0FFFFFFF])?; @@ -348,7 +348,7 @@ fn too_big_event_data() -> Result<(), HostError> { #[test] fn too_many_events_in_loop() -> Result<(), HostError> { // We don't observe this test: it makes way too big a trace. - let host = Host::test_host(); + let host = Host::test_host_with_prng(); let budget = host.as_budget().clone(); budget.reset_unlimited()?; let topics = host.vec_new_from_slice(&[Val::from_u32(0).to_val()])?; diff --git a/soroban-env-host/src/test/lifecycle.rs b/soroban-env-host/src/test/lifecycle.rs index 7a9e04303..1222b5aef 100644 --- a/soroban-env-host/src/test/lifecycle.rs +++ b/soroban-env-host/src/test/lifecycle.rs @@ -219,8 +219,6 @@ fn create_contract_using_parent_id_test() { ); assert_eq!(child_wasm, get_contract_wasm(&host, wasm_hash.clone())); - host.maybe_add_module_cache().unwrap(); - // Now successfully create the child contract itself. host.call( parent_contract_address, @@ -756,7 +754,6 @@ mod cap_54_55_56 { protocol_version: proto, ..Default::default() })?; - host.maybe_add_module_cache()?; Ok(host) } @@ -1009,6 +1006,8 @@ mod cap_54_55_56 { V_OLD, )? .0; + // force a module-cache build (this normally happens on first VM call) + host.build_module_cache_if_needed()?; let module_cache = host.try_borrow_module_cache()?; assert!(module_cache.is_none()); Ok(()) @@ -1023,6 +1022,8 @@ mod cap_54_55_56 { "test_v_new_module_cache_check", V_NEW, )?; + // force a module-cache build (this normally happens on first VM call) + host.build_module_cache_if_needed()?; let wasm = get_contract_wasm_ref(&host, contract_id); let module_cache = host.try_borrow_module_cache()?; if let Some(module_cache) = &*module_cache { diff --git a/soroban-env-host/src/testutils.rs b/soroban-env-host/src/testutils.rs index 981481845..435a96968 100644 --- a/soroban-env-host/src/testutils.rs +++ b/soroban-env-host/src/testutils.rs @@ -178,21 +178,22 @@ pub(crate) fn interface_meta_with_custom_versions(proto: u32, pre: u32) -> Vec Self { - let host = Host::default(); - host.set_base_prng_seed(*Host::TEST_PRNG_SEED).unwrap(); - host + fn set_test_prng(&self) { + self.set_base_prng_seed(*Self::TEST_PRNG_SEED).unwrap(); } - pub fn test_host_with_recording_footprint() -> Self { - let snapshot_source = Rc::::new(MockSnapshotSource::new()); - let storage = Storage::with_recording_footprint(snapshot_source); - let host = Host::with_storage_and_budget(storage, Budget::default()); - host.set_base_prng_seed(*Host::TEST_PRNG_SEED).unwrap(); - host.set_ledger_info(LedgerInfo { - protocol_version: crate::meta::get_ledger_protocol_version( - crate::meta::INTERFACE_VERSION, - ), + fn current_test_protocol() -> u32 { + use crate::meta::{get_ledger_protocol_version, INTERFACE_VERSION}; + if let Ok(vers) = std::env::var("TEST_PROTOCOL") { + vers.parse().unwrap() + } else { + get_ledger_protocol_version(INTERFACE_VERSION) + } + } + + fn set_test_protocol(&self) { + self.set_ledger_info(LedgerInfo { + protocol_version: Self::current_test_protocol(), sequence_number: 0, timestamp: 0, network_id: [0; 32], @@ -202,6 +203,26 @@ impl Host { max_entry_ttl: 6_312_000, }) .unwrap(); + } + + pub fn test_host() -> Self { + let host = Host::default(); + host.set_test_protocol(); + host + } + + pub fn test_host_with_prng() -> Self { + let host = Self::test_host(); + host.set_test_prng(); + host + } + + pub fn test_host_with_recording_footprint() -> Self { + let snapshot_source = Rc::::new(MockSnapshotSource::new()); + let storage = Storage::with_recording_footprint(snapshot_source); + let host = Host::with_storage_and_budget(storage, Budget::default()); + host.set_test_protocol(); + host.set_test_prng(); host } diff --git a/soroban-env-host/src/vm.rs b/soroban-env-host/src/vm.rs index 2a8f216ad..c9a98bcb8 100644 --- a/soroban-env-host/src/vm.rs +++ b/soroban-env-host/src/vm.rs @@ -332,7 +332,9 @@ impl Vm { cost_inputs: VersionedContractCodeCostInputs, ) -> Result, HostError> { use crate::storage::FootprintMode; - if host.get_ledger_protocol_version()? >= ModuleCache::MIN_LEDGER_VERSION { + if cfg!(feature = "next") + && 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) From c19449060879809a995fdaf6f419728a95ed4148 Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Mon, 1 Apr 2024 15:06:49 -0700 Subject: [PATCH 08/19] Handle test-only empty contracts in module cache --- soroban-env-host/src/vm/module_cache.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/soroban-env-host/src/vm/module_cache.rs b/soroban-env-host/src/vm/module_cache.rs index 416e9f3db..a02154418 100644 --- a/soroban-env-host/src/vm/module_cache.rs +++ b/soroban-env-host/src/vm/module_cache.rs @@ -46,6 +46,16 @@ impl ModuleCache { if let LedgerEntryData::ContractCode(ContractCodeEntry { code, hash, ext }) = &e.data { + // We allow empty contracts in testing mode; they exist + // to exercise as much of the contract-code-storage + // infrastructure as possible, while still redirecting + // the actual execution into a `ContractFunctionSet`. + // They should never be called, so we do not have to go + // as far as making a fake `ParsedModule` for them. + if cfg!(any(test, feature = "testutils")) && code.as_slice().is_empty() { + continue; + } + let code_cost_inputs = match ext { ContractCodeEntryExt::V0 => VersionedContractCodeCostInputs::V0 { wasm_bytes: code.len(), From 9c297243debaacab5e83768dbc6055b45b450573 Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Mon, 1 Apr 2024 15:33:57 -0700 Subject: [PATCH 09/19] Fix more next-gated tests --- soroban-env-host/src/host/frame.rs | 4 +++- soroban-env-host/src/test/invocation.rs | 1 + soroban-env-host/src/test/lifecycle.rs | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/soroban-env-host/src/host/frame.rs b/soroban-env-host/src/host/frame.rs index 5e2634845..4f88c49bf 100644 --- a/soroban-env-host/src/host/frame.rs +++ b/soroban-env-host/src/host/frame.rs @@ -195,7 +195,9 @@ impl Host { .maybe_emulate_authentication(self)?; // 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. - self.rebuild_module_cache()?; + if let crate::storage::FootprintMode::Recording(_) = self.try_borrow_storage()?.mode { + self.rebuild_module_cache()?; + } } let mut auth_snapshot = None; if let Some(rp) = orp { diff --git a/soroban-env-host/src/test/invocation.rs b/soroban-env-host/src/test/invocation.rs index fe58cc5c1..77c25e3b2 100644 --- a/soroban-env-host/src/test/invocation.rs +++ b/soroban-env-host/src/test/invocation.rs @@ -217,6 +217,7 @@ fn contract_failure_with_debug_on_off_affects_no_metering() -> Result<(), HostEr let invoke_cross_contract_indirect_with_err = || -> Result<(u64, u64, u64, u64), HostError> { // try call -- add will trap, and add_with will trap, but we will get an error + host.rebuild_module_cache()?; host.as_budget().reset_default()?; let res = host.try_call(id0_obj, sym, args); HostError::result_matches_err( diff --git a/soroban-env-host/src/test/lifecycle.rs b/soroban-env-host/src/test/lifecycle.rs index 1222b5aef..cd4f1240f 100644 --- a/soroban-env-host/src/test/lifecycle.rs +++ b/soroban-env-host/src/test/lifecycle.rs @@ -770,8 +770,9 @@ mod cap_54_55_56 { let realhost = host.clone(); drop(host); let (storage, _events) = realhost.try_finish()?; + let storage = Storage::with_enforcing_footprint_and_map(storage.footprint, storage.map); - // Phase 2: build new host with previous ledger output as storage, call contract. Possibly on new protocol. + // Phase 2: build new host with previous ledger output as storage. Possibly on new protocol. let host = observed_test_host_with_storage_and_budget( second_hostname, second_proto, From d3ac1a8fffa37d5f6bd5c5be14ef697b2fd02486 Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Mon, 1 Apr 2024 17:08:16 -0700 Subject: [PATCH 10/19] Fix test_module_cache_recording_fidelity test --- soroban-env-host/src/host.rs | 9 +++++++++ soroban-env-host/src/host/frame.rs | 11 ++++++++++- soroban-env-host/src/vm.rs | 3 +-- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index 86c8b98f1..079535786 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -374,6 +374,15 @@ impl Host { Ok(()) } + #[cfg(any(test, feature = "recording_mode"))] + pub fn in_storage_recording_mode(&self) -> Result { + if let crate::storage::FootprintMode::Recording(_) = self.try_borrow_storage()?.mode { + Ok(true) + } else { + Ok(false) + } + } + #[cfg(any(test, feature = "recording_mode"))] pub fn rebuild_module_cache(&self) -> Result<(), HostError> { if cfg!(feature = "next") { diff --git a/soroban-env-host/src/host/frame.rs b/soroban-env-host/src/host/frame.rs index 4f88c49bf..d6146c428 100644 --- a/soroban-env-host/src/host/frame.rs +++ b/soroban-env-host/src/host/frame.rs @@ -195,7 +195,7 @@ impl Host { .maybe_emulate_authentication(self)?; // 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. - if let crate::storage::FootprintMode::Recording(_) = self.try_borrow_storage()?.mode { + if self.in_storage_recording_mode()? { self.rebuild_module_cache()?; } } @@ -646,6 +646,15 @@ impl Host { match &instance.executable { ContractExecutable::Wasm(wasm_hash) => { // If the module cache is not yet built, build it now, before first access. + // Unless we're in recording mode, because in that case the cache is built + // late in [pop_context] after we've determined the transaction footprint. + #[cfg(feature = "recording_mode")] + { + if !self.in_storage_recording_mode()? { + self.build_module_cache_if_needed()?; + } + } + #[cfg(not(feature = "recording_mode"))] self.build_module_cache_if_needed()?; let contract_id = id.metered_clone(self)?; let vm = if let Some(cache) = &*self.try_borrow_module_cache()? { diff --git a/soroban-env-host/src/vm.rs b/soroban-env-host/src/vm.rs index c9a98bcb8..04ae80eaa 100644 --- a/soroban-env-host/src/vm.rs +++ b/soroban-env-host/src/vm.rs @@ -331,11 +331,10 @@ impl Vm { wasm: &[u8], cost_inputs: VersionedContractCodeCostInputs, ) -> Result, HostError> { - use crate::storage::FootprintMode; if cfg!(feature = "next") && host.get_ledger_protocol_version()? >= ModuleCache::MIN_LEDGER_VERSION { - if let FootprintMode::Recording(_) = host.try_borrow_storage()?.mode { + if host.in_storage_recording_mode()? { return host.budget_ref().with_observable_shadow_mode(|| { ParsedModule::new(host, engine, wasm, cost_inputs) }); From b849b34cfd4222c7294f04634a46b0d9b2b53446 Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Mon, 1 Apr 2024 18:00:58 -0700 Subject: [PATCH 11/19] Fix total_amount_charged_from_random_inputs --- soroban-env-host/src/test/budget_metering.rs | 133 +++++++++++-------- 1 file changed, 79 insertions(+), 54 deletions(-) diff --git a/soroban-env-host/src/test/budget_metering.rs b/soroban-env-host/src/test/budget_metering.rs index dca985298..0ddd37d34 100644 --- a/soroban-env-host/src/test/budget_metering.rs +++ b/soroban-env-host/src/test/budget_metering.rs @@ -370,6 +370,31 @@ fn total_amount_charged_from_random_inputs() -> Result<(), HostError> { (1, Some(1)), ]; + #[cfg(feature="next")] + let tracker: Vec<(u64, Option)> = tracker.into_iter().chain(vec![ + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + + (1, None), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, None), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + ].into_iter()).collect(); + for (ty, &(iterations, input)) in tracker.iter().enumerate() { host.with_budget(|b| b.bulk_charge(ContractCostType::VARIANTS[ty], iterations, input))?; } @@ -423,60 +448,60 @@ fn total_amount_charged_from_random_inputs() -> Result<(), HostError> { #[cfg(feature = "next")] let expected = expect![ r#" - ===================================================================================================================================================================== - Cpu limit: 100000000; used: 13060190 - Mem limit: 41943040; used: 273960 - ===================================================================================================================================================================== - CostType iterations input cpu_insns mem_bytes const_term_cpu lin_term_cpu const_term_mem lin_term_mem - WasmInsnExec 246 None 984 0 4 0 0 0 - MemAlloc 1 Some(152) 453 168 434 16 16 128 - MemCpy 1 Some(65) 50 0 42 16 0 0 - MemCmp 1 Some(74) 53 0 44 16 0 0 - DispatchHostFunction 176 None 54560 0 310 0 0 0 - VisitObject 97 None 5917 0 61 0 0 0 - ValSer 1 Some(49) 241 389 230 29 242 384 - ValDeser 1 Some(103) 62271 309 59052 4001 0 384 - ComputeSha256Hash 1 Some(193) 14310 0 3738 7012 0 0 - ComputeEd25519PubKey 226 None 9097178 0 40253 0 0 0 - VerifyEd25519Sig 1 Some(227) 384738 0 377524 4068 0 0 - VmInstantiation 1 Some(147) 503770 135880 451626 45405 130065 5064 - VmCachedInstantiation 1 Some(147) 503770 135880 451626 45405 130065 5064 - InvokeVmFunction 47 None 91556 658 1948 0 14 0 - ComputeKeccak256Hash 1 Some(1) 3812 0 3766 5969 0 0 - ComputeEcdsaSecp256k1Sig 1 None 710 0 710 0 0 0 - RecoverEcdsaSecp256k1Key 1 None 2315295 181 2315295 0 181 0 - Int256AddSub 1 None 4404 99 4404 0 99 0 - Int256Mul 1 None 4947 99 4947 0 99 0 - Int256Div 1 None 4911 99 4911 0 99 0 - Int256Pow 1 None 4286 99 4286 0 99 0 - Int256Shift 1 None 913 99 913 0 99 0 - ChaCha20DrawBytes 1 Some(1) 1061 0 1058 501 0 0 - ParseWasmInstructions 0 Some(0) 0 0 72736 25420 17564 6457 - ParseWasmFunctions 0 Some(0) 0 0 0 536688 0 47464 - ParseWasmGlobals 0 Some(0) 0 0 0 176902 0 13420 - ParseWasmTableEntries 0 Some(0) 0 0 0 29639 0 6285 - ParseWasmTypes 0 Some(0) 0 0 0 1048891 0 64670 - ParseWasmDataSegments 0 Some(0) 0 0 0 236970 0 29074 - ParseWasmElemSegments 0 Some(0) 0 0 0 317249 0 48095 - ParseWasmImports 0 Some(0) 0 0 0 694667 0 102890 - ParseWasmExports 0 Some(0) 0 0 0 427037 0 36394 - ParseWasmDataSegmentBytes0 Some(0) 0 0 66075 28 17580 257 - InstantiateWasmInstructions0 None 0 0 25059 0 70192 0 - InstantiateWasmFunctions 0 Some(0) 0 0 0 7503 0 14613 - InstantiateWasmGlobals 0 Some(0) 0 0 0 10761 0 6833 - InstantiateWasmTableEntries0 Some(0) 0 0 0 3211 0 1025 - InstantiateWasmTypes 0 None 0 0 0 0 0 0 - InstantiateWasmDataSegments0 Some(0) 0 0 0 16370 0 129632 - InstantiateWasmElemSegments0 Some(0) 0 0 0 28309 0 13665 - InstantiateWasmImports 0 Some(0) 0 0 0 683461 0 77273 - InstantiateWasmExports 0 Some(0) 0 0 0 297065 0 9176 - InstantiateWasmDataSegmentBytes0 Some(0) 0 0 25191 14 69256 126 - ===================================================================================================================================================================== - Internal details (diagnostics info, does not affect fees) - Total # times meter was called: 23 - Shadow cpu limit: 100000000; used: 13060190 - Shadow mem limit: 41943040; used: 273960 - ===================================================================================================================================================================== + ===================================================================================================================================================================== + Cpu limit: 100000000; used: 12751453 + Mem limit: 41943040; used: 302115 + ===================================================================================================================================================================== + CostType iterations input cpu_insns mem_bytes const_term_cpu lin_term_cpu const_term_mem lin_term_mem + WasmInsnExec 246 None 984 0 4 0 0 0 + MemAlloc 1 Some(152) 453 168 434 16 16 128 + MemCpy 1 Some(65) 50 0 42 16 0 0 + MemCmp 1 Some(74) 53 0 44 16 0 0 + DispatchHostFunction 176 None 54560 0 310 0 0 0 + VisitObject 97 None 5917 0 61 0 0 0 + ValSer 1 Some(49) 241 389 230 29 242 384 + ValDeser 1 Some(103) 62271 309 59052 4001 0 384 + ComputeSha256Hash 1 Some(193) 14310 0 3738 7012 0 0 + ComputeEd25519PubKey 226 None 9097178 0 40253 0 0 0 + VerifyEd25519Sig 1 Some(227) 384738 0 377524 4068 0 0 + VmInstantiation 1 Some(147) 503770 135880 451626 45405 130065 5064 + VmCachedInstantiation 1 Some(147) 41870 70869 41142 634 69472 1217 + InvokeVmFunction 47 None 91556 658 1948 0 14 0 + ComputeKeccak256Hash 1 Some(1) 3812 0 3766 5969 0 0 + ComputeEcdsaSecp256k1Sig 1 None 710 0 710 0 0 0 + RecoverEcdsaSecp256k1Key 1 None 2315295 181 2315295 0 181 0 + Int256AddSub 1 None 4404 99 4404 0 99 0 + Int256Mul 1 None 4947 99 4947 0 99 0 + Int256Div 1 None 4911 99 4911 0 99 0 + Int256Pow 1 None 4286 99 4286 0 99 0 + Int256Shift 1 None 913 99 913 0 99 0 + ChaCha20DrawBytes 1 Some(1) 1061 0 1058 501 0 0 + ParseWasmInstructions 1 Some(1) 73275 17614 73077 25410 17564 6457 + ParseWasmFunctions 1 Some(1) 4224 370 0 540752 0 47464 + ParseWasmGlobals 1 Some(1) 1377 104 0 176363 0 13420 + ParseWasmTableEntries 1 Some(1) 234 49 0 29989 0 6285 + ParseWasmTypes 1 Some(1) 8292 505 0 1061449 0 64670 + ParseWasmDataSegments 1 Some(1) 1854 227 0 237336 0 29074 + ParseWasmElemSegments 1 Some(1) 2566 375 0 328476 0 48095 + ParseWasmImports 1 Some(1) 5483 806 0 701845 0 103229 + ParseWasmExports 1 Some(1) 3354 284 0 429383 0 36394 + ParseWasmDataSegmentBytes1 Some(1) 0 2 0 28 0 257 + InstantiateWasmInstructions1 None 43030 70704 43030 0 70704 0 + InstantiateWasmFunctions 1 Some(1) 59 114 0 7556 0 14613 + InstantiateWasmGlobals 1 Some(1) 83 53 0 10711 0 6833 + InstantiateWasmTableEntries1 Some(1) 25 8 0 3300 0 1025 + InstantiateWasmTypes 1 None 0 0 0 0 0 0 + InstantiateWasmDataSegments1 Some(1) 179 1012 0 23038 0 129632 + InstantiateWasmElemSegments1 Some(1) 331 106 0 42488 0 13665 + InstantiateWasmImports 1 Some(1) 6476 762 0 828974 0 97637 + InstantiateWasmExports 1 Some(1) 2321 71 0 297100 0 9176 + InstantiateWasmDataSegmentBytes1 Some(1) 0 0 0 14 0 126 + ===================================================================================================================================================================== + Internal details (diagnostics info, does not affect fees) + Total # times meter was called: 43 + Shadow cpu limit: 100000000; used: 12751453 + Shadow mem limit: 41943040; used: 302115 + ===================================================================================================================================================================== "# ]; From 2ef5f3b5addbc457556c7c8800657f6e59b80af9 Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Mon, 1 Apr 2024 20:21:23 -0700 Subject: [PATCH 12/19] Fix excessive_logging under --features=next --- soroban-env-host/src/host.rs | 9 +++++-- soroban-env-host/src/test/hostile.rs | 40 +++++++++++++++------------- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index 079535786..e3f586808 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -384,15 +384,20 @@ impl Host { } #[cfg(any(test, feature = "recording_mode"))] - pub fn rebuild_module_cache(&self) -> Result<(), HostError> { + pub fn clear_module_cache(&self) -> Result<(), HostError> { if cfg!(feature = "next") { *self.try_borrow_module_cache_mut()? = None; *self.try_borrow_linker_mut()? = None; - self.build_module_cache_if_needed()?; } Ok(()) } + #[cfg(any(test, feature = "recording_mode"))] + pub fn rebuild_module_cache(&self) -> Result<(), HostError> { + self.clear_module_cache()?; + self.build_module_cache_if_needed() + } + pub fn set_source_account(&self, source_account: AccountId) -> Result<(), HostError> { *self.try_borrow_source_account_mut()? = Some(source_account); Ok(()) diff --git a/soroban-env-host/src/test/hostile.rs b/soroban-env-host/src/test/hostile.rs index e0c1417a7..99abd7751 100644 --- a/soroban-env-host/src/test/hostile.rs +++ b/soroban-env-host/src/test/hostile.rs @@ -530,13 +530,13 @@ fn excessive_logging() -> Result<(), HostError> { #[cfg(feature = "next")] let expected_budget = expect![[r#" ======================================================= - Cpu limit: 2000000; used: 284819 - Mem limit: 500000; used: 252830 + Cpu limit: 2000000; used: 214592 + Mem limit: 500000; used: 166740 ======================================================= CostType cpu_insns mem_bytes WasmInsnExec 300 0 - MemAlloc 15750 67248 - MemCpy 2345 0 + MemAlloc 16191 67320 + MemCpy 2836 0 MemCmp 696 0 DispatchHostFunction 310 0 VisitObject 244 0 @@ -557,26 +557,26 @@ fn excessive_logging() -> Result<(), HostError> { Int256Pow 0 0 Int256Shift 0 0 ChaCha20DrawBytes 0 0 - ParseWasmInstructions 74324 17967 - ParseWasmFunctions 4192 370 - ParseWasmGlobals 1382 104 - ParseWasmTableEntries 29639 6285 - ParseWasmTypes 8194 505 + ParseWasmInstructions 74665 17967 + ParseWasmFunctions 4224 370 + ParseWasmGlobals 1377 104 + ParseWasmTableEntries 29989 6285 + ParseWasmTypes 8292 505 ParseWasmDataSegments 0 0 ParseWasmElemSegments 0 0 - ParseWasmImports 5427 803 - ParseWasmExports 6672 568 - ParseWasmDataSegmentBytes66075 17580 - InstantiateWasmInstructions25059 70192 - InstantiateWasmFunctions 58 114 - InstantiateWasmGlobals 84 53 - InstantiateWasmTableEntries3211 1025 + ParseWasmImports 5483 806 + ParseWasmExports 6709 568 + ParseWasmDataSegmentBytes0 0 + InstantiateWasmInstructions43030 70704 + InstantiateWasmFunctions 59 114 + InstantiateWasmGlobals 83 53 + InstantiateWasmTableEntries3300 1025 InstantiateWasmTypes 0 0 InstantiateWasmDataSegments0 0 InstantiateWasmElemSegments0 0 - InstantiateWasmImports 5339 603 - InstantiateWasmExports 4641 143 - InstantiateWasmDataSegmentBytes25191 69256 + InstantiateWasmImports 6476 762 + InstantiateWasmExports 4642 143 + InstantiateWasmDataSegmentBytes0 0 ======================================================= "#]]; @@ -636,6 +636,7 @@ fn excessive_logging() -> Result<(), HostError> { // excessive logging { + host.clear_module_cache()?; host.budget_ref().reset_limits(2_000_000, 500_000)?; let res = host.call( contract_id_obj, @@ -657,6 +658,7 @@ fn excessive_logging() -> Result<(), HostError> { // increasing the shadow budget should make everything happy again { + host.clear_module_cache()?; host.budget_ref().reset_limits(2_000_000, 500_000)?; host.set_shadow_budget_limits(2_000_000, 1_000_000)?; let res = host.call( From 0a55fa9fa2e8fe823e59d651498b4a68d4e9cafe Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Mon, 1 Apr 2024 20:42:56 -0700 Subject: [PATCH 13/19] Fix remaining --features=next failures --- soroban-env-host/src/test/budget_metering.rs | 53 +++++++++++--------- soroban-env-host/src/test/bytes.rs | 1 + soroban-env-host/src/test/map.rs | 1 + soroban-env-host/src/test/vec.rs | 1 + 4 files changed, 32 insertions(+), 24 deletions(-) diff --git a/soroban-env-host/src/test/budget_metering.rs b/soroban-env-host/src/test/budget_metering.rs index 0ddd37d34..44b529acc 100644 --- a/soroban-env-host/src/test/budget_metering.rs +++ b/soroban-env-host/src/test/budget_metering.rs @@ -370,30 +370,35 @@ fn total_amount_charged_from_random_inputs() -> Result<(), HostError> { (1, Some(1)), ]; - #[cfg(feature="next")] - let tracker: Vec<(u64, Option)> = tracker.into_iter().chain(vec![ - (1, Some(1)), - (1, Some(1)), - (1, Some(1)), - (1, Some(1)), - (1, Some(1)), - (1, Some(1)), - (1, Some(1)), - (1, Some(1)), - (1, Some(1)), - (1, Some(1)), - - (1, None), - (1, Some(1)), - (1, Some(1)), - (1, Some(1)), - (1, None), - (1, Some(1)), - (1, Some(1)), - (1, Some(1)), - (1, Some(1)), - (1, Some(1)), - ].into_iter()).collect(); + #[cfg(feature = "next")] + let tracker: Vec<(u64, Option)> = tracker + .into_iter() + .chain( + vec![ + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, None), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, None), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + (1, Some(1)), + ] + .into_iter(), + ) + .collect(); for (ty, &(iterations, input)) in tracker.iter().enumerate() { host.with_budget(|b| b.bulk_charge(ContractCostType::VARIANTS[ty], iterations, input))?; diff --git a/soroban-env-host/src/test/bytes.rs b/soroban-env-host/src/test/bytes.rs index 5b5f749c9..c31beec5a 100644 --- a/soroban-env-host/src/test/bytes.rs +++ b/soroban-env-host/src/test/bytes.rs @@ -496,6 +496,7 @@ fn instantiate_oversized_bytes_from_linear_memory() -> Result<(), HostError> { #[cfg(feature = "next")] const TOO_BIG: u32 = 8_000_000; let wasm_long = wasm::wasm_module_with_large_bytes_from_linear_memory(TOO_BIG, 7); + host.clear_module_cache()?; host.budget_ref().reset_unlimited()?; let contract_id_obj2 = host.register_test_contract_wasm(&wasm_long.as_slice()); host.budget_ref().reset_default()?; diff --git a/soroban-env-host/src/test/map.rs b/soroban-env-host/src/test/map.rs index 4e2bb7e57..692098dc9 100644 --- a/soroban-env-host/src/test/map.rs +++ b/soroban-env-host/src/test/map.rs @@ -490,6 +490,7 @@ fn instantiate_oversized_map_from_linear_memory() -> Result<(), HostError> { const TOO_BIG: u32 = 1_000_000; let wasm_long = wasm::wasm_module_with_large_map_from_linear_memory(TOO_BIG, U32Val::from(7).to_val()); + host.clear_module_cache()?; host.budget_ref().reset_unlimited()?; let contract_id_obj2 = host.register_test_contract_wasm(&wasm_long.as_slice()); host.budget_ref().reset_default()?; diff --git a/soroban-env-host/src/test/vec.rs b/soroban-env-host/src/test/vec.rs index 6481aaca3..15a75a2e4 100644 --- a/soroban-env-host/src/test/vec.rs +++ b/soroban-env-host/src/test/vec.rs @@ -459,6 +459,7 @@ fn instantiate_oversized_vec_from_linear_memory() -> Result<(), HostError> { const TOO_BIG: u32 = 1_000_000; let wasm_long = wasm::wasm_module_with_large_vector_from_linear_memory(TOO_BIG, U32Val::from(7).to_val()); + host.clear_module_cache()?; host.budget_ref().reset_unlimited()?; let contract_id_obj2 = host.register_test_contract_wasm(&wasm_long.as_slice()); host.budget_ref().reset_default()?; From f417afc00c866f3d1889d97e7958581ba7b86e05 Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Mon, 1 Apr 2024 20:47:14 -0700 Subject: [PATCH 14/19] Remove misleading paragraph --- soroban-env-host/src/vm.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/soroban-env-host/src/vm.rs b/soroban-env-host/src/vm.rs index 04ae80eaa..edf1759e5 100644 --- a/soroban-env-host/src/vm.rs +++ b/soroban-env-host/src/vm.rs @@ -314,11 +314,6 @@ impl Vm { /// 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 From f5fa8c82ef63ccccfc495e211894c570cb15add5 Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Mon, 1 Apr 2024 23:56:19 -0700 Subject: [PATCH 15/19] Fix corner-case next-and-no-testutils test failure --- soroban-env-host/src/host.rs | 8 ++++++++ soroban-env-host/src/test/map.rs | 11 ++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index e3f586808..8dbb4df7e 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -460,6 +460,14 @@ impl Host { } } + #[cfg(any(test, feature = "recording_mode"))] + pub fn switch_to_enforcing_storage(&self) -> Result<(), HostError> { + self.with_mut_storage(|storage| { + storage.mode = crate::storage::FootprintMode::Enforcing; + Ok(()) + }) + } + #[cfg(any(test, feature = "recording_mode"))] pub fn switch_to_recording_auth(&self, disable_non_root_auth: bool) -> Result<(), HostError> { *self.try_borrow_authorization_manager_mut()? = diff --git a/soroban-env-host/src/test/map.rs b/soroban-env-host/src/test/map.rs index 692098dc9..0b0802a01 100644 --- a/soroban-env-host/src/test/map.rs +++ b/soroban-env-host/src/test/map.rs @@ -487,13 +487,22 @@ fn instantiate_oversized_map_from_linear_memory() -> Result<(), HostError> { #[cfg(not(feature = "next"))] const TOO_BIG: u32 = 20_000; #[cfg(feature = "next")] - const TOO_BIG: u32 = 1_000_000; + const TOO_BIG: u32 = 400_000; let wasm_long = wasm::wasm_module_with_large_map_from_linear_memory(TOO_BIG, U32Val::from(7).to_val()); host.clear_module_cache()?; host.budget_ref().reset_unlimited()?; let contract_id_obj2 = host.register_test_contract_wasm(&wasm_long.as_slice()); host.budget_ref().reset_default()?; + + // We want to observe a failure that happens part way through contract + // instantiation, which means the weird half-deferred charging of + // module-cache construct we do in storage-recording mode will interfere + // with this test in features=next,testutils mode (which turns on + // feature=recording_mode implicitly). The easiest workaround is just to + // switch to enforcing mode. + host.switch_to_enforcing_storage()?; + let res = host.call( contract_id_obj2, Symbol::try_from_small_str("test")?, From 6d748d3922bf96224f326472f17b8c2b2f0317cb Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Tue, 2 Apr 2024 00:26:53 -0700 Subject: [PATCH 16/19] Guard e2e recording-fidelity test on recording mode --- soroban-env-host/src/test/e2e_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/soroban-env-host/src/test/e2e_tests.rs b/soroban-env-host/src/test/e2e_tests.rs index d7f2ff8f1..4b02b99bc 100644 --- a/soroban-env-host/src/test/e2e_tests.rs +++ b/soroban-env-host/src/test/e2e_tests.rs @@ -1738,7 +1738,7 @@ fn test_classic_account_auth_using_simulation() { assert!(res.invoke_result.is_ok()); } -#[cfg(feature = "next")] +#[cfg(all(feature = "next", feature = "recording_mode"))] mod cap_54_55_56 { use super::*; From 661d14623bde6ea50406f05cf305180bc09ae8ef Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Tue, 2 Apr 2024 00:27:47 -0700 Subject: [PATCH 17/19] Fix edge-case configuration of excessive_logging with features=next, not-testutils --- soroban-env-host/src/test/hostile.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/soroban-env-host/src/test/hostile.rs b/soroban-env-host/src/test/hostile.rs index 99abd7751..1c34f114f 100644 --- a/soroban-env-host/src/test/hostile.rs +++ b/soroban-env-host/src/test/hostile.rs @@ -526,18 +526,19 @@ fn excessive_logging() -> Result<(), HostError> { let host = Host::test_host_with_recording_footprint(); host.enable_debug()?; let contract_id_obj = host.register_test_contract_wasm(wasm.as_slice()); + host.switch_to_enforcing_storage()?; #[cfg(feature = "next")] let expected_budget = expect![[r#" ======================================================= - Cpu limit: 2000000; used: 214592 - Mem limit: 500000; used: 166740 + Cpu limit: 2000000; used: 212908 + Mem limit: 500000; used: 166460 ======================================================= CostType cpu_insns mem_bytes WasmInsnExec 300 0 - MemAlloc 16191 67320 - MemCpy 2836 0 - MemCmp 696 0 + MemAlloc 15292 67040 + MemCpy 2331 0 + MemCmp 416 0 DispatchHostFunction 310 0 VisitObject 244 0 ValSer 0 0 @@ -617,6 +618,7 @@ fn excessive_logging() -> Result<(), HostError> { // moderate logging { + host.clear_module_cache()?; host.budget_ref().reset_limits(2_000_000, 500_000)?; let res = host.call( contract_id_obj, From 5af9ff14a157dbbe6c6ce38f0fb08e0376394c21 Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Tue, 2 Apr 2024 00:42:38 -0700 Subject: [PATCH 18/19] qualify previous fix --- soroban-env-host/src/test/hostile.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/soroban-env-host/src/test/hostile.rs b/soroban-env-host/src/test/hostile.rs index 1c34f114f..5a98deabb 100644 --- a/soroban-env-host/src/test/hostile.rs +++ b/soroban-env-host/src/test/hostile.rs @@ -526,6 +526,8 @@ fn excessive_logging() -> Result<(), HostError> { let host = Host::test_host_with_recording_footprint(); host.enable_debug()?; let contract_id_obj = host.register_test_contract_wasm(wasm.as_slice()); + + #[cfg(feature = "next")] host.switch_to_enforcing_storage()?; #[cfg(feature = "next")] From c5a57241bff707e1b315a9fdd20b14c98aa7fc1c Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Tue, 2 Apr 2024 01:03:30 -0700 Subject: [PATCH 19/19] qualify previous fix --- soroban-env-host/src/test/map.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/soroban-env-host/src/test/map.rs b/soroban-env-host/src/test/map.rs index 0b0802a01..936a7beae 100644 --- a/soroban-env-host/src/test/map.rs +++ b/soroban-env-host/src/test/map.rs @@ -501,6 +501,7 @@ fn instantiate_oversized_map_from_linear_memory() -> Result<(), HostError> { // with this test in features=next,testutils mode (which turns on // feature=recording_mode implicitly). The easiest workaround is just to // switch to enforcing mode. + #[cfg(feature = "next")] host.switch_to_enforcing_storage()?; let res = host.call(