Address review comments and CAP updates

soroban-env-host/benches/common/cost_types/vm_ops.rs

@@ -11,8 +11,8 @@ use std::rc::Rc;
 // Protocol 20 coarse cost model.
 pub(crate) struct VmInstantiationMeasure;
 
-// This measures the cost of parsing wasm and/or instantiating a host::Vm on a
-// variety of possible wasm modules, of different sizes.
+// This measures the cost of parsing Wasm and/or instantiating a host::Vm on a
+// variety of possible Wasm modules, of different sizes.
 macro_rules! impl_measurement_for_instantiation_cost_type {
     ($RUNNER:ty, $MEASURE:ty, $BUILD:ident, $USE_REFINED_INPUTS:expr, $MAGNITUDE:expr) => {
         impl HostCostMeasurement for $MEASURE {

soroban-env-host/benches/common/cost_types/wasm_insn_exec.rs

@@ -198,15 +198,15 @@ fn wasm_module_with_mem_grow(n_pages: usize) -> Vec<u8> {
     fe.finish_and_export("test").finish()
 }
 
-// A wasm module with a single const to serve as the baseline
+// A Wasm module with a single const to serve as the baseline
 fn wasm_module_baseline_pass() -> WasmModule {
     let mut fe = ModEmitter::default().func(Arity(0), 0);
     fe.push(Symbol::try_from_small_str("pass").unwrap());
     let wasm = fe.finish_and_export("test").finish();
     WasmModule { wasm, overhead: 0 }
 }
 
-// A wasm module with a single trap to serve as the baseline
+// A Wasm module with a single trap to serve as the baseline
 fn wasm_module_baseline_trap() -> WasmModule {
     let mut fe = ModEmitter::default().func(Arity(0), 0);
     fe.trap();

soroban-env-host/src/host/lifecycle.rs

@@ -216,24 +216,46 @@ impl Host {
             }),
             self,
         )?;
-        if !self
-            .try_borrow_storage_mut()?
+
+        let mut storage = self.try_borrow_storage_mut()?;
+
+        // We will definitely put the contract in the ledger if it isn't there yet.
+        #[allow(unused_mut)]
+        let mut should_put_contract = !storage
             .has(&code_key, self.as_budget())
-            .map_err(|e| self.decorate_contract_code_storage_error(e, &Hash(hash_bytes)))?
+            .map_err(|e| self.decorate_contract_code_storage_error(e, &Hash(hash_bytes)))?;
+
+        // We may also, in the cache-supporting protocol, overwrite the contract if its ext field changed.
+        #[cfg(feature = "next")]
+        if !should_put_contract
+            && self.get_ledger_protocol_version()? >= super::ModuleCache::MIN_LEDGER_VERSION
         {
-            self.with_mut_storage(|storage| {
-                let data = ContractCodeEntry {
-                    hash: Hash(hash_bytes),
-                    ext,
-                    code: wasm_bytes_m,
-                };
-                storage.put(
+            let entry = storage
+                .get(&code_key, self.as_budget())
+                .map_err(|e| self.decorate_contract_code_storage_error(e, &Hash(hash_bytes)))?;
+            if let crate::xdr::LedgerEntryData::ContractCode(ContractCodeEntry {
+                ext: old_ext,
+                ..
+            }) = &entry.data
+            {
+                should_put_contract = *old_ext != ext;
+            }
+        }
+
+        if should_put_contract {
+            let data = ContractCodeEntry {
+                hash: Hash(hash_bytes),
+                ext,
+                code: wasm_bytes_m,
+            };
+            storage
+                .put(
                     &code_key,
                     &Host::new_contract_code(self, data)?,
                     Some(self.get_min_live_until_ledger(ContractDataDurability::Persistent)?),
                     self.as_budget(),
                 )
-            })?;
+                .map_err(|e| self.decorate_contract_code_storage_error(e, &Hash(hash_bytes)))?;
         }
         Ok(hash_obj)
     }

soroban-env-host/src/vm.rs

@@ -45,6 +45,7 @@ use wasmi::{Caller, StoreContextMut};
 impl wasmi::core::HostError for HostError {}
 
 const MAX_VM_ARGS: usize = 32;
+const WASM_STD_MEM_PAGE_SIZE_IN_BYTES: u32 = 0x10000;
 
 struct VmInstantiationTimer {
     host: Host,

soroban-env-host/src/vm/module_cache.rs

@@ -1,21 +1,22 @@
 use super::parsed_module::{ParsedModule, VersionedContractCodeCostInputs};
 use crate::{
     budget::{get_wasmi_config, AsBudget},
+    host::metered_clone::MeteredClone,
     xdr::{Hash, ScErrorCode, ScErrorType},
-    Host, HostError,
+    Host, HostError, MeteredOrdMap,
 };
-use std::{collections::BTreeMap, rc::Rc};
+use std::rc::Rc;
 use wasmi::Engine;
 
-/// A [ModuleCache] is a cache of a set of WASM modules that have been parsed
+/// A [ModuleCache] is a cache of a set of Wasm modules that have been parsed
 /// but not yet instantiated, along with a shared and reusable [Engine] storing
 /// their code. The cache must be populated eagerly with all the contracts in a
 /// single [Host]'s lifecycle (at least) added all at once, since each wasmi
 /// [Engine] is locked during execution and no new modules can be added to it.
 #[derive(Clone, Default)]
 pub struct ModuleCache {
     pub(crate) engine: Engine,
-    modules: BTreeMap<Hash, Rc<ParsedModule>>,
+    modules: MeteredOrdMap<Hash, Rc<ParsedModule>, Host>,
 }
 
 impl ModuleCache {
@@ -25,7 +26,7 @@ impl ModuleCache {
     pub fn new(host: &Host) -> Result<Self, HostError> {
         let config = get_wasmi_config(host.as_budget())?;
         let engine = Engine::new(&config);
-        let modules = BTreeMap::new();
+        let modules = MeteredOrdMap::new();
         #[allow(unused_mut)]
         let mut cache = Self { engine, modules };
         #[cfg(feature = "next")]
@@ -65,7 +66,7 @@ impl ModuleCache {
         wasm: &[u8],
         cost_inputs: VersionedContractCodeCostInputs,
     ) -> Result<(), HostError> {
-        if self.modules.contains_key(contract_id) {
+        if self.modules.contains_key(contract_id, host)? {
             return Err(host.err(
                 ScErrorType::Context,
                 ScErrorCode::InternalError,
@@ -74,7 +75,9 @@ impl ModuleCache {
             ));
         }
         let parsed_module = Rc::new(ParsedModule::new(host, &self.engine, &wasm, cost_inputs)?);
-        self.modules.insert(contract_id.clone(), parsed_module);
+        self.modules =
+            self.modules
+                .insert(contract_id.metered_clone(host)?, parsed_module, host)?;
         Ok(())
     }
 
@@ -83,7 +86,7 @@ impl ModuleCache {
         host: &Host,
         contract_id: &Hash,
     ) -> Result<Rc<ParsedModule>, HostError> {
-        if let Some(m) = self.modules.get(contract_id) {
+        if let Some(m) = self.modules.get(contract_id, host)? {
             return Ok(m.clone());
         } else {
             Err(host.err(

soroban-env-host/src/vm/parsed_module.rs

@@ -1,6 +1,7 @@
 use crate::{
     err,
     meta::{self, get_ledger_protocol_version},
+    vm::WASM_STD_MEM_PAGE_SIZE_IN_BYTES,
     xdr::{ContractCostType, Limited, ReadXdr, ScEnvMetaEntry, ScErrorCode, ScErrorType},
     Host, HostError, DEFAULT_XDR_RW_LIMITS,
 };
@@ -134,7 +135,7 @@ impl VersionedContractCodeCostInputs {
     }
 }
 
-/// A [ParsedModule] contains the parsed [wasmi::Module] for a given wasm blob,
+/// A [ParsedModule] contains the parsed [wasmi::Module] for a given Wasm blob,
 /// as well as a protocol number and set of [ContractCodeCostInputs] extracted
 /// from the module when it was parsed.
 pub struct ParsedModule {
@@ -177,7 +178,7 @@ impl ParsedModule {
         })
     }
 
-    /// Parse the wasm blob into a [Module] and its protocol number, checking check its interface version
+    /// Parse the Wasm blob into a [Module] and its protocol number, checking its interface version
     fn parse_wasm(host: &Host, engine: &Engine, wasm: &[u8]) -> Result<(Module, u32), HostError> {
         let module = {
             let _span0 = tracy_span!("parse module");
@@ -224,7 +225,7 @@ impl ParsedModule {
             //
             // Note that we only enable this check if the "next" feature isn't enabled
             // because a "next" stellar-core can still run a "curr" test using non-finalized
-            // test wasms. The "next" feature isn't safe for production and is meant to
+            // test Wasms. The "next" feature isn't safe for production and is meant to
             // simulate the protocol version after the one currently supported in
             // stellar-core, so bypassing this check for "next" is safe.
             #[cfg(not(feature = "next"))]
@@ -282,7 +283,7 @@ impl ParsedModule {
         })
     }
 
-    /// Returns the raw bytes content of a named custom section from the WASM
+    /// Returns the raw bytes content of a named custom section from the Wasm
     /// module loaded into the [Vm], or `None` if no such custom section exists.
     #[allow(dead_code)]
     pub fn custom_section(&self, name: impl AsRef<str>) -> Option<&[u8]> {
@@ -321,12 +322,20 @@ impl ParsedModule {
         for e in m.exports() {
             match e.ty() {
                 wasmi::ExternType::Func(f) => {
-                    if f.params().len() > MAX_VM_ARGS || f.results().len() > MAX_VM_ARGS {
-                        return Err(host.err(
-                            ScErrorType::WasmVm,
-                            ScErrorCode::InvalidInput,
-                            "Too many arguments or results in wasm export",
-                            &[],
+                    if f.results().len() > MAX_VM_ARGS {
+                        return Err(err!(
+                            host,
+                            (ScErrorType::WasmVm, ScErrorCode::InvalidInput),
+                            "Too many return values in Wasm export",
+                            f.results().len()
+                        ));
+                    }
+                    if f.params().len() > MAX_VM_ARGS {
+                        return Err(err!(
+                            host,
+                            (ScErrorType::WasmVm, ScErrorCode::InvalidInput),
+                            "Too many arguments Wasm export",
+                            f.params().len()
                         ));
                     }
                 }
@@ -336,7 +345,7 @@ impl ParsedModule {
         Ok(())
     }
 
-    // Do a second, manual parse of the wasm blob to extract cost parameters we're
+    // Do a second, manual parse of the Wasm blob to extract cost parameters we're
     // interested in.
     #[cfg(feature = "next")]
     pub fn extract_refined_contract_cost_inputs(
@@ -355,6 +364,7 @@ impl ParsedModule {
         }
 
         let mut costs = crate::xdr::ContractCodeCostInputs {
+            ext: crate::xdr::ExtensionPoint::V0,
             n_instructions: 0,
             n_functions: 0,
             n_globals: 0,
@@ -398,7 +408,7 @@ impl ParsedModule {
                     return Err(host.err(
                         ScErrorType::WasmVm,
                         ScErrorCode::InvalidInput,
-                        "unsupported wasm section",
+                        "unsupported wasm section type",
                         &[],
                     ))
                 }
@@ -491,7 +501,7 @@ impl ParsedModule {
                             return Err(host.err(
                                 ScErrorType::WasmVm,
                                 ScErrorCode::InvalidInput,
-                                "data segment too large",
+                                "data segment exceeds u32::MAX",
                                 &[],
                             ));
                         }
@@ -512,20 +522,25 @@ impl ParsedModule {
                 }
             }
         }
-        if data > costs.n_memory_pages.saturating_mul(0x10000) {
-            return Err(host.err(
-                ScErrorType::WasmVm,
-                ScErrorCode::InvalidInput,
+        let available_memory = costs
+            .n_memory_pages
+            .saturating_mul(WASM_STD_MEM_PAGE_SIZE_IN_BYTES);
+        if data > available_memory {
+            return Err(err!(
+                host,
+                (ScErrorType::WasmVm, ScErrorCode::InvalidInput),
                 "data segments exceed memory size",
-                &[],
+                data,
+                available_memory
             ));
         }
         if elements > costs.n_table_entries {
-            return Err(host.err(
-                ScErrorType::WasmVm,
-                ScErrorCode::InvalidInput,
-                "too many elements in wasm elem section(s)",
-                &[],
+            return Err(err!(
+                host,
+                (ScErrorType::WasmVm, ScErrorCode::InvalidInput),
+                "too many elements in Wasm elem section(s)",
+                elements,
+                costs.n_table_entries
             ));
         }
         Ok(costs)
@@ -542,7 +557,7 @@ impl ParsedModule {
                     return Err(host.err(
                         ScErrorType::WasmVm,
                         ScErrorCode::InvalidInput,
-                        "unsupported complex wasm constant expression",
+                        "unsupported complex Wasm constant expression",
                         &[],
                     ))
                 }