Remove p22/immutable mode from module cache, only support p23/mutable

.github/workflows/rust.yml

@@ -73,7 +73,7 @@ jobs:
     strategy:
       matrix:
         rust: [msrv, latest]
-        test-protocol: [22]
+        test-protocol: [23]
         sys:
         - os: ubuntu-latest
           target: wasm32-unknown-unknown

Makefile

@@ -6,8 +6,8 @@ test:
 test-opt:
 	cargo hack --locked --each-feature test --profile test-opt
 
-MIN_PROTOCOL := 22
-MAX_PROTOCOL := 22
+MIN_PROTOCOL := 23
+MAX_PROTOCOL := 23
 
 test-all-protocols:
 	for i in $$(seq $(MIN_PROTOCOL) $$(($(MAX_PROTOCOL) + 1))); do \

soroban-env-common/src/meta.rs

@@ -49,7 +49,7 @@ pub const ENV_META_V0_SECTION_NAME: &str = "contractenvmetav0";
 // protocol number for testing purposes.
 #[cfg(feature = "next")]
 soroban_env_macros::generate_env_meta_consts!(
-    ledger_protocol_version: 23,
+    ledger_protocol_version: 24,
     pre_release_version: 1,
 );
 
@@ -59,6 +59,6 @@ soroban_env_macros::generate_env_meta_consts!(
 // network protocol number.
 #[cfg(not(feature = "next"))]
 soroban_env_macros::generate_env_meta_consts!(
-    ledger_protocol_version: 22,
+    ledger_protocol_version: 23,
     pre_release_version: 0,
 );

soroban-env-host/src/host.rs

@@ -82,12 +82,11 @@ pub struct CoverageScoreboard {
     pub vm_to_vm_calls: usize,
 }
 
-// The soroban 22.x host only supports protocol 22 and later, having
-// adopted a new version of wasmi with a new fuel metering system, it
-// cannot accurately replay earlier contracts. Earlier protocols
-// must run on Soroban 21.x or earlier.
+// The soroban 23.x host only supports protocol 23 and later, having adopted a
+// new module caching system. Earlier protocols must run on Soroban 22.x or
+// earlier.
 
-pub(crate) const MIN_LEDGER_PROTOCOL_VERSION: u32 = 22;
+pub(crate) const MIN_LEDGER_PROTOCOL_VERSION: u32 = 23;
 
 #[derive(Clone, Default)]
 struct HostImpl {
@@ -163,14 +162,6 @@ struct HostImpl {
     #[cfg(any(test, feature = "recording_mode"))]
     suppress_diagnostic_events: RefCell<bool>,
 
-    // This flag marks the call of `build_module_cache` that would happen
-    // in enforcing mode. In recording mode we need to use this flag to
-    // determine whether we need to rebuild module cache after the host
-    // invocation has been done.
-    #[doc(hidden)]
-    #[cfg(any(test, feature = "recording_mode"))]
-    need_to_build_module_cache: RefCell<bool>,
-
     #[cfg(any(test, feature = "testutils"))]
     pub(crate) invocation_meter: RefCell<InvocationMeter>,
 }
@@ -324,14 +315,6 @@ impl_checked_borrow_helpers!(
     try_borrow_suppress_diagnostic_events_mut
 );
 
-#[cfg(any(test, feature = "recording_mode"))]
-impl_checked_borrow_helpers!(
-    need_to_build_module_cache,
-    bool,
-    try_borrow_need_to_build_module_cache,
-    try_borrow_need_to_build_module_cache_mut
-);
-
 impl Debug for HostImpl {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(f, "HostImpl(...)")
@@ -380,35 +363,26 @@ impl Host {
             coverage_scoreboard: Default::default(),
             #[cfg(any(test, feature = "recording_mode"))]
             suppress_diagnostic_events: RefCell::new(false),
-            #[cfg(any(test, feature = "recording_mode"))]
-            need_to_build_module_cache: RefCell::new(false),
             #[cfg(any(test, feature = "testutils"))]
             invocation_meter: Default::default(),
         }))
     }
 
+    #[cfg(any(test, feature="testutils"))]
+    // This builds a module cache instance for just the contracts stored
+    // in the host's storage map, and is used only in testing.
     pub fn build_module_cache_if_needed(&self) -> Result<(), HostError> {
         if self.try_borrow_module_cache()?.is_none() {
-            let cache = ModuleCache::new(self)?;
+            let cache = ModuleCache::new_for_stored_contracts(self)?;
             *self.try_borrow_module_cache_mut()? = Some(cache);
         }
         Ok(())
     }
 
     // Install a module cache from _outside_ the Host. Doing this is potentially
     // delicate: the cache must contain all contracts that will be run by the
-    // host, and will not be further populated during execution. This is
-    // only allowed if the cache is of "reusable" type, i.e. it was created
-    // using `ModuleCache::new_reusable`.
+    // host, and will not be further populated during execution.
     pub fn set_module_cache(&self, cache: ModuleCache) -> Result<(), HostError> {
-        if !cache.is_reusable() {
-            return Err(self.err(
-                ScErrorType::Context,
-                ScErrorCode::InternalError,
-                "module cache not reusable",
-                &[],
-            ));
-        }
         *self.try_borrow_module_cache_mut()? = Some(cache);
         Ok(())
     }
@@ -438,14 +412,14 @@ impl Host {
     }
 
     #[cfg(any(test, feature = "recording_mode"))]
-    pub fn clear_module_cache(&self) -> Result<(), HostError> {
+    pub fn forget_module_cache(&self) -> Result<(), HostError> {
         *self.try_borrow_module_cache_mut()? = None;
         Ok(())
     }
 
     #[cfg(any(test, feature = "recording_mode"))]
     pub fn rebuild_module_cache(&self) -> Result<(), HostError> {
-        self.clear_module_cache()?;
+        self.forget_module_cache()?;
         self.build_module_cache_if_needed()
     }
 

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

@@ -217,19 +217,6 @@ 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.
-            if self.in_storage_recording_mode()? {
-                if *self.try_borrow_need_to_build_module_cache()? {
-                    // Host function calls that upload Wasm and create contracts
-                    // don't use the module cache and thus don't need to have it
-                    // rebuilt.
-                    self.rebuild_module_cache()?;
-                }
-            }
-            // Reset the flag for building the module cache. This is only relevant
-            // for tests that keep reusing the same host for several invocations.
-            *(self.try_borrow_need_to_build_module_cache_mut()?) = false;
         }
         let mut auth_snapshot = None;
         if let Some(rp) = orp {
@@ -701,16 +688,6 @@ impl Host {
     }
 
     fn instantiate_vm(&self, id: &Hash, wasm_hash: &Hash) -> Result<Rc<Vm>, HostError> {
-        #[cfg(any(test, feature = "recording_mode"))]
-        {
-            if !self.in_storage_recording_mode()? {
-                self.build_module_cache_if_needed()?;
-            } else {
-                *(self.try_borrow_need_to_build_module_cache_mut()?) = true;
-            }
-        }
-        #[cfg(not(any(test, feature = "recording_mode")))]
-        self.build_module_cache_if_needed()?;
         let contract_id = id.metered_clone(self)?;
         let parsed_module = if let Some(cache) = &*self.try_borrow_module_cache()? {
             // Check that storage thinks the entry exists before
@@ -721,7 +698,7 @@ impl Host {
                 .try_borrow_storage_mut()?
                 .has_with_host(&wasm_key, self, None)?
             {
-                cache.get_module(self, wasm_hash)?
+                cache.get_module(wasm_hash)?
             } else {
                 None
             }
@@ -733,8 +710,7 @@ impl Host {
         };
         // We can get here a few ways:
         //
-        //   1. We are running/replaying a protocol that has no
-        //      module cache.
+        //   1. We are in simulation so don't have a module cache.
         //
         //   2. We have a module cache, but it somehow doesn't have
         //      the module requested. This in turn has two
@@ -745,8 +721,10 @@ impl Host {
         //
         //     - User uploaded the wasm _in this transaction_ so we
         //       didn't cache it when starting the transaction (and
-        //       couldn't due to wasmi locking its engine while
-        //       running).
+        //       couldn't add it: additions use a shared wasmi engine
+        //       that owns all the cache entries, and that engine is
+        //       locked while we're running; uploads use a throwaway
+        //       engine for validation purposes).
         //
         //   3. Even more pathological: the module cache was built,
         //      and contained the module, but someone _removed_ the
@@ -767,9 +745,7 @@ impl Host {
         #[cfg(any(test, feature = "recording_mode"))]
         // In recording mode: if a contract was present in the initial snapshot image, it is part of
         // the set of contracts that would have been built into a module cache in enforcing mode;
-        // we want to defer the cost of parsing those (simulating them as cache hits) and then charge
-        // once for each such contract the simulated-module-cache-build that happens at the end of
-        // the frame in [`Self::pop_context`].
+        // we want to suppress the cost of parsing those (simulating them as cache hits).
         //
         // If a contract is _not_ in the initial snapshot image, it's because someone just uploaded
         // it during execution. Those would be cache misses in enforcing mode, and so it is right to
@@ -783,7 +759,7 @@ impl Host {
                 .get_snapshot_value(self, &contact_code_key)?
                 .is_some()
             {
-                crate::vm::ModuleParseCostMode::PossiblyDeferredIfRecording
+                crate::vm::ModuleParseCostMode::SuppressedToEmulateCacheHit
             } else {
                 crate::vm::ModuleParseCostMode::Normal
             }

soroban-env-host/src/test/budget_metering.rs

@@ -128,7 +128,7 @@ fn test_vm_fuel_metering() -> Result<(), HostError> {
         .test_budget(cpu_required, mem_required)
         .enable_model(ContractCostType::WasmInsnExec, 6, 0, 0, 0)
         .enable_model(ContractCostType::MemAlloc, 0, 0, 0, 1);
-    host.clear_module_cache()?;
+    host.forget_module_cache()?;
     host.call(id_obj, sym, args)?;
     host.with_budget(|budget| {
         assert_eq!(budget.get_cpu_insns_consumed()?, cpu_required);
@@ -142,7 +142,7 @@ fn test_vm_fuel_metering() -> Result<(), HostError> {
         .test_budget(cpu_required, mem_required)
         .enable_model(ContractCostType::WasmInsnExec, 6, 0, 0, 0)
         .enable_model(ContractCostType::MemAlloc, 0, 0, 0, 1);
-    host.clear_module_cache()?;
+    host.forget_module_cache()?;
     let res = host.try_call(id_obj, sym, args);
     assert!(HostError::result_matches_err(res, budget_err));
     host.with_budget(|budget| {
@@ -157,7 +157,7 @@ fn test_vm_fuel_metering() -> Result<(), HostError> {
         .test_budget(cpu_required, mem_required)
         .enable_model(ContractCostType::WasmInsnExec, 6, 0, 0, 0)
         .enable_model(ContractCostType::MemAlloc, 0, 0, 0, 1);
-    host.clear_module_cache()?;
+    host.forget_module_cache()?;
     let res = host.try_call(id_obj, sym, args);
     assert!(HostError::result_matches_err(res, budget_err));
     host.with_budget(|budget| {

soroban-env-host/src/test/bytes.rs

@@ -494,7 +494,7 @@ fn instantiate_oversized_bytes_from_linear_memory() -> Result<(), HostError> {
     // constructing a big bytes will cause budget limit exceeded error
     let num_bytes: u32 = 16_000_000;
     let wasm_long = wasm::wasm_module_with_large_bytes_from_linear_memory(num_bytes, 7);
-    host.clear_module_cache()?;
+    host.forget_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()?;

soroban-env-host/src/test/hostile.rs

@@ -606,7 +606,7 @@ fn excessive_logging() -> Result<(), HostError> {
 
     // moderate logging
     {
-        host.clear_module_cache()?;
+        host.forget_module_cache()?;
         host.budget_ref().reset_limits(2_000_000, 500_000)?;
         let res = host.call(
             contract_id_obj,
@@ -626,7 +626,7 @@ fn excessive_logging() -> Result<(), HostError> {
 
     // excessive logging
     {
-        host.clear_module_cache()?;
+        host.forget_module_cache()?;
         host.budget_ref().reset_limits(2_000_000, 500_000)?;
         let res = host.call(
             contract_id_obj,
@@ -648,7 +648,7 @@ fn excessive_logging() -> Result<(), HostError> {
 
     // increasing the shadow budget should make everything happy again
     {
-        host.clear_module_cache()?;
+        host.forget_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(

soroban-env-host/src/test/invocation.rs

@@ -218,7 +218,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.build_module_cache_if_needed()?;
         host.as_budget().reset_default()?;
         let res = host.try_call(id0_obj, sym, args);
         HostError::result_matches_err(

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

@@ -1093,7 +1093,7 @@ mod cap_54_55_56 {
         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());
+            assert!(module_cache.get_module(&wasm).is_ok());
         } else {
             panic!("expected module cache");
         }
@@ -1409,7 +1409,7 @@ mod cap_54_55_56 {
 
         // Check that the module cache did not get populated with the new wasm.
         if let Some(module_cache) = &*host.try_borrow_module_cache()? {
-            assert!(module_cache.get_module(&*host, &wasm_hash)?.is_none());
+            assert!(module_cache.get_module(&wasm_hash)?.is_none());
         } else {
             panic!("expected module cache");
         }

soroban-env-host/src/test/map.rs

@@ -487,7 +487,7 @@ fn instantiate_oversized_map_from_linear_memory() -> Result<(), HostError> {
     let num_vals = 400_000;
     let wasm_long =
         wasm::wasm_module_with_large_map_from_linear_memory(num_vals, U32Val::from(7).to_val());
-    host.clear_module_cache()?;
+    host.forget_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()?;

soroban-env-host/src/test/vec.rs

@@ -457,7 +457,7 @@ fn instantiate_oversized_vec_from_linear_memory() -> Result<(), HostError> {
     let num_vals = 1_100_000;
     let wasm_long =
         wasm::wasm_module_with_large_vector_from_linear_memory(num_vals, U32Val::from(7).to_val());
-    host.clear_module_cache()?;
+    host.forget_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()?;

soroban-env-host/src/testutils.rs

@@ -1,3 +1,4 @@
+use crate::budget::AsBudget;
 use crate::e2e_invoke::ledger_entry_to_ledger_key;
 use crate::storage::EntryWithLiveUntil;
 use crate::ErrorHandler;
@@ -311,6 +312,9 @@ impl Host {
             self.set_source_account(prev_account)?;
         }
         self.set_auth_manager(prev_auth_manager)?;
+        self.as_budget().with_shadow_mode(|| {
+            self.rebuild_module_cache()
+        });
         Ok(contract_address)
     }