refactor: Screen wasm modules with a wasmi ModuleInspector instead of a pre-scan

crates/Cargo.lock

@@ -349,8 +349,7 @@ dependencies = [
 [[package]]
 name = "wasmi"
 version = "2.0.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "78693fcdd618e0fc34af59c6b8efa9ac5d58c68df940beff4bedddb6acfe7c27"
+source = "git+https://github.com/TimothyBanks/wasmi.git?branch=parser_callback#1be4a903162990371da09f5224b3351983999b05"
 dependencies = [
  "spin",
  "wasmi_collections",
@@ -362,8 +361,7 @@ dependencies = [
 [[package]]
 name = "wasmi_collections"
 version = "2.0.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8a8be2aa467cf2d29e96ff759472c36eeb44a3c81c67fc9cb76c9a24c519c557"
+source = "git+https://github.com/TimothyBanks/wasmi.git?branch=parser_callback#1be4a903162990371da09f5224b3351983999b05"
 dependencies = [
  "string-interner",
 ]
@@ -371,8 +369,7 @@ dependencies = [
 [[package]]
 name = "wasmi_core"
 version = "2.0.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "69372d5fda3ea3d1e0aa6603c7888110e0187e88ea17cd8fc2e2df0a0e1f37fa"
+source = "git+https://github.com/TimothyBanks/wasmi.git?branch=parser_callback#1be4a903162990371da09f5224b3351983999b05"
 dependencies = [
  "libm",
 ]
@@ -380,8 +377,7 @@ dependencies = [
 [[package]]
 name = "wasmi_ir"
 version = "2.0.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8f17b774caa13c618c7244f1ee51fe23c5e7b8538a471fa46d9949779758aed6"
+source = "git+https://github.com/TimothyBanks/wasmi.git?branch=parser_callback#1be4a903162990371da09f5224b3351983999b05"
 dependencies = [
  "wasmi_core",
 ]
@@ -484,7 +480,6 @@ name = "xrpl-wasm-vm"
 version = "0.1.0"
 dependencies = [
  "wasmi",
- "wasmparser 0.228.0",
  "wat",
  "xrpl-host-functions",
 ]

crates/Cargo.toml

@@ -11,6 +11,15 @@ resolver = "3"
 [workspace.dependencies]
 cxx = { version = "1.0.199", features = ["c++20"] }
 
+# Not for merge: builds against a wasmi fork with `ModuleInspector`
+# (TimothyBanks/wasmi, branch `parser_callback`) until it is accepted upstream.
+# All four crates come from the fork so that there is one `wasmi_core`.
+[patch.crates-io]
+wasmi = { git = "https://github.com/TimothyBanks/wasmi.git", branch = "parser_callback" }
+wasmi_core = { git = "https://github.com/TimothyBanks/wasmi.git", branch = "parser_callback" }
+wasmi_ir = { git = "https://github.com/TimothyBanks/wasmi.git", branch = "parser_callback" }
+wasmi_collections = { git = "https://github.com/TimothyBanks/wasmi.git", branch = "parser_callback" }
+
 [workspace.package]
 edition = "2024"
 

crates/xrpl-wasm-vm/Cargo.toml

@@ -5,7 +5,6 @@ edition.workspace = true
 
 [dependencies]
 wasmi = { version = "2.0.0", default-features = false, features = ["std", "validate", "portable-dispatch"] }
-wasmparser = { version = "0.228", default-features = false, features = ["std"] }
 xrpl-host-functions = { path = "../xrpl-host-functions", features = ["wasmi_glue"] }
 
 [dev-dependencies]

crates/xrpl-wasm-vm/src/preflight/mod.rs

@@ -16,10 +16,9 @@
 //! does is refused at the call and charged for what it burned. A start section needs
 //! no rule of its own — the engine forbids one, so such a module fails to compile.
 //!
-//! Two things it screens that a run can only discover: an exported memory, or an
-//! exported table, larger than the engine grants. Both read the same export list, so
-//! [`check_exported_resources_iter`] is one pass — see it for what stays invisible,
-//! and why the table case leaves much more of it there.
+//! Two things it screens that a run can only discover: a memory, or a table, larger
+//! than the engine grants — exported or not. Both are read from the module's own
+//! declarations in one pass, [`check_declared_resources_iter`].
 //!
 //! Every rule is here but one: [`signature`] holds the comparison of an import's
 //! type against the ABI's, which needs machinery the rest of the stage does not.
@@ -30,7 +29,7 @@ use std::fmt;
 use wasmi::{ExternType, FuncType, Module, ValType};
 use xrpl_host_functions::{HOST_MODULE, HostFunctionSpec};
 
-use crate::vm::{CompileError, MAX_MEMORY_PAGES, MAX_TABLE_ELEMENTS, compile, wasm_engine};
+use crate::vm::{CompileError, MAX_MEMORY_PAGES, MAX_TABLE_ELEMENTS, Screen, compile, wasm_engine};
 use signature::check_signature;
 
 /// Why a module cannot be run. One variant per stage, since the caller maps the
@@ -88,29 +87,33 @@ impl From<CompileError> for CheckError {
 /// `function_name` as `() -> i32`, and ask for no more memory or table than it may
 /// have.
 pub fn check(wasm: &[u8], function_name: &str) -> Result<(), CheckError> {
-    let module = compile(&wasm_engine(), wasm).map_err(CheckError::from)?;
-    check_error_iter(&module, function_name)
+    let compiled = compile(&wasm_engine(), wasm).map_err(CheckError::from)?;
+    check_error_iter(&compiled.module, &compiled.screen, function_name)
         .next()
         .map_or(Ok(()), Err)
 }
 
 /// [`check`], reporting every error found rather than stopping at the first.
 pub fn check_all(wasm: &[u8], function_name: &str) -> Result<(), Vec<CheckError>> {
-    let module = compile(&wasm_engine(), wasm).map_err(|e| vec![CheckError::from(e)])?;
-    let refusals: Vec<CheckError> = check_error_iter(&module, function_name).collect();
+    let compiled = compile(&wasm_engine(), wasm).map_err(|e| vec![CheckError::from(e)])?;
+    let refusals: Vec<CheckError> =
+        check_error_iter(&compiled.module, &compiled.screen, function_name).collect();
     if refusals.is_empty() {
         return Ok(());
     }
     Err(refusals)
 }
 
+/// `screen` is what compiling `module` recorded of its memory and table, which the
+/// module itself shows only through its exports.
 fn check_error_iter<'a>(
     module: &'a Module,
+    screen: &'a Screen,
     function_name: &'a str,
 ) -> impl Iterator<Item = CheckError> + 'a {
     check_imports_iter(module)
         .chain(check_entry_point_iter(module, function_name))
-        .chain(check_exported_resources_iter(module))
+        .chain(check_declared_resources_iter(screen))
 }
 
 fn check_imports_iter(module: &Module) -> impl Iterator<Item = CheckError> + '_ {
@@ -175,29 +178,29 @@ fn is_entry_point(ty: &FuncType) -> bool {
 }
 
 /// A module may declare no more linear memory, and no larger a table, than the
-/// engine grants. One pass over the exports, since both rules read the same list and
-/// the export table is the only place either is visible.
+/// engine grants — whether it exports them or keeps them to itself.
 ///
-/// **A memory or table the module keeps to itself is therefore not screened**: it is
-/// absent from the exports, and the store's limiter is what refuses it, at
-/// instantiation. That gap is wide for tables — Rust exports
-/// `__indirect_function_table` only under `--export-table`, so unexported is the
-/// normal shape — and narrow for memories, since a contract needs an exported one to
-/// make any host call at all.
+/// Read from what [`compile`] recorded, not from [`Module`]: wasmi shows a module's
+/// memories and tables only through its exports, and a table is *normally*
+/// unexported — Rust exports `__indirect_function_table` only under
+/// `--export-table`. Screening the exports alone would pass such a module to the
+/// ledger, where the store's limiter refuses it at every finish. The limiter stays,
+/// for a module that reaches a run unscreened.
 ///
-/// A module faulting on both yields both, in export order. Neither fault explains
-/// the other, so there is no precedence to preserve — only the need for every node to
-/// reach the same verdict, which export order already gives.
-fn check_exported_resources_iter(module: &Module) -> impl Iterator<Item = CheckError> + '_ {
-    module.exports().filter_map(|export| match export.ty() {
-        ExternType::Memory(ty) => check_initial_pages(ty.minimum())
-            .err()
-            .map(CheckError::Memory),
-        ExternType::Table(ty) => check_initial_elements(ty.minimum())
-            .err()
-            .map(CheckError::Table),
-        _ => None,
-    })
+/// A module faulting on both yields both, in section order — the table section
+/// precedes the memory section. Neither fault explains the other, so there is no
+/// precedence to preserve — only the need for every node to reach the same verdict,
+/// which the binary's own order already gives.
+fn check_declared_resources_iter(screen: &Screen) -> impl Iterator<Item = CheckError> {
+    let table = screen
+        .table_elements()
+        .and_then(|elements| check_initial_elements(elements).err())
+        .map(CheckError::Table);
+    let memory = screen
+        .memory_pages()
+        .and_then(|pages| check_initial_pages(pages).err())
+        .map(CheckError::Memory);
+    table.into_iter().chain(memory)
 }
 
 /// Whether the engine will grant a memory of this declared initial size.

crates/xrpl-wasm-vm/src/vm.rs

@@ -1,8 +1,10 @@
 use std::cell::Cell;
 use std::fmt;
+use wasmi::errors::HostError;
 use wasmi::{
-    CompilationMode, Config, CustomFuelCosts, EnforcedLimits, Engine, Export, Linker, Memory,
-    Module, Store, StoreLimits, StoreLimitsBuilder, TrapCode,
+    CompilationMode, Config, CustomFuelCosts, EnforcedLimits, Engine, Export, ExternKind,
+    ImportKind, Linker, Memory, MemoryType, Module, ModuleInspector, SectionKind, Store,
+    StoreLimits, StoreLimitsBuilder, TableType, TrapCode,
 };
 use xrpl_host_functions::HostFunctions;
 
@@ -326,9 +328,23 @@ fn store_limits() -> StoreLimits {
 /// contract is valid at all, so [`run`] and [`crate::check`] must not be able to
 /// compile against different ones. The engine is the caller's because the module it
 /// returns may only be instantiated in a [`Store`] built on that same engine.
-pub(crate) fn compile(engine: &Engine, wasm: &[u8]) -> Result<Module, CompileError> {
-    check_section_counts(wasm).map_err(CompileError::Limit)?;
-    Module::new(engine, wasm).map_err(|e| CompileError::Invalid(e.to_string()))
+pub(crate) fn compile(engine: &Engine, wasm: &[u8]) -> Result<Compiled, CompileError> {
+    let mut screen = Screen::default();
+    let module = Module::new_with_inspector(engine, wasm, &mut screen).map_err(|e| match e
+        .downcast_ref::<PastCap>()
+    {
+        Some(PastCap(detail)) => CompileError::Limit(detail.clone()),
+        None => CompileError::Invalid(e.to_string()),
+    })?;
+    Ok(Compiled { module, screen })
+}
+
+/// What [`compile`] produced: the module, and the [`Screen`] that inspected it.
+pub(crate) struct Compiled {
+    pub(crate) module: Module,
+    /// What the parse recorded that [`Module`] cannot say. Read by [`crate::check`];
+    /// a run ignores it.
+    pub(crate) screen: Screen,
 }
 
 /// Why [`compile`] produced no module.
@@ -340,51 +356,97 @@ pub(crate) fn compile(engine: &Engine, wasm: &[u8]) -> Result<Module, CompileErr
 pub(crate) enum CompileError {
     /// `wasm` is not a valid module under this engine's configuration.
     Invalid(String),
-    /// A section or a name is past its cap — see [`check_section_counts`].
+    /// A section or a name is past its cap — see [`Screen`].
     Limit(String),
 }
 
 /// Refuse a module whose type, import or export section, or any import or export
-/// name, is past its cap — before [`Module::new`] reads a single entry.
+/// name, is past its cap, and record the memories and tables it defines — all from
+/// inside [`Module::new_with_inspector`]'s own parse, so the bytes are walked once.
 ///
-/// Ahead of compiling because compiling is where the cost is: `Module::new`
-/// validates and stores every entry, and no store exists yet to charge it to. Left
-/// to wasmi, the ceilings are wasmparser's — a million entries per section and
+/// Left to wasmi, the ceilings are wasmparser's — a million entries per section and
 /// 100 000 bytes per name — and [`EnforcedLimits`] has no knob for any of them.
 ///
-/// A section's count is in its header, so the counts cost nothing to read; names
-/// are read only once the count is known to be under its cap. Every section
-/// checked here precedes the code section, so the scan stops there.
+/// The section caps are checked **before** wasmi validates the section: a count is in
+/// the section's header, so a module past one is refused before any of its entries
+/// is read. The name caps, and the record, come **after** the section is validated,
+/// as wasmi reads its entries — so the validator has read a long name by the time it
+/// is refused. That cost is bounded by the module's size, which the transaction
+/// already caps.
 ///
-/// Anything that does not parse is passed through rather than refused, so a
-/// malformed module is described by `Module::new` and not by a second parser.
-fn check_section_counts(wasm: &[u8]) -> Result<(), String> {
-    use wasmparser::{Parser, Payload};
+/// One field each for the memory and the table is enough: a module that compiles
+/// defines at most one of each, since the engine disables multi-memory and reference
+/// types, and a [`Screen`] is only read once [`compile`] has succeeded. An imported
+/// memory or table is not recorded — an import that is not a host function is
+/// refused anyway.
+#[derive(Default)]
+pub(crate) struct Screen {
+    memory_pages: Option<u64>,
+    table_elements: Option<u64>,
+}
 
-    for payload in Parser::new(0).parse_all(wasm) {
-        let Ok(payload) = payload else { break };
-        match payload {
-            Payload::TypeSection(section) => {
-                check_count("type", section.count(), MAX_TYPES)?;
-            }
-            Payload::ImportSection(section) => {
-                check_count("import", section.count(), MAX_IMPORTS)?;
-                for import in section.into_iter().map_while(Result::ok) {
-                    check_name("import module", import.module)?;
-                    check_name("import", import.name)?;
-                }
-            }
-            Payload::ExportSection(section) => {
-                check_count("export", section.count(), MAX_EXPORTS)?;
-                for export in section.into_iter().map_while(Result::ok) {
-                    check_name("export", export.name)?;
-                }
-            }
-            Payload::CodeSectionStart { .. } => break,
-            _ => {}
-        }
+impl Screen {
+    /// The initial pages of the memory the module defines, if it defines one —
+    /// exported or not, which [`Module`] cannot say: it shows a memory only through
+    /// its exports. A run leaves the size to the store's limiter.
+    pub(crate) fn memory_pages(&self) -> Option<u64> {
+        self.memory_pages
     }
-    Ok(())
+
+    /// The initial elements of the table the module defines, if it defines one, as
+    /// [`Screen::memory_pages`] is for a memory.
+    pub(crate) fn table_elements(&self) -> Option<u64> {
+        self.table_elements
+    }
+}
+
+impl ModuleInspector for Screen {
+    fn section(&mut self, kind: SectionKind, count: u32) -> Result<(), wasmi::Error> {
+        let (section, cap) = match kind {
+            SectionKind::Type => ("type", MAX_TYPES),
+            SectionKind::Import => ("import", MAX_IMPORTS),
+            SectionKind::Export => ("export", MAX_EXPORTS),
+            _ => return Ok(()),
+        };
+        check_count(section, count, cap).map_err(past_cap)
+    }
+
+    fn import(&mut self, module: &str, name: &str, _: &ImportKind) -> Result<(), wasmi::Error> {
+        check_name("import module", module)
+            .and_then(|()| check_name("import", name))
+            .map_err(past_cap)
+    }
+
+    fn export(&mut self, name: &str, _: ExternKind, _: u32) -> Result<(), wasmi::Error> {
+        check_name("export", name).map_err(past_cap)
+    }
+
+    fn table(&mut self, ty: &TableType) -> Result<(), wasmi::Error> {
+        self.table_elements = Some(ty.minimum());
+        Ok(())
+    }
+
+    fn memory(&mut self, ty: &MemoryType) -> Result<(), wasmi::Error> {
+        self.memory_pages = Some(ty.minimum());
+        Ok(())
+    }
+}
+
+/// A [`Screen`] refusal, carried through wasmi's error so [`compile`] can tell it
+/// from a module wasmi itself rejects.
+#[derive(Debug)]
+struct PastCap(String);
+
+impl fmt::Display for PastCap {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str(&self.0)
+    }
+}
+
+impl HostError for PastCap {}
+
+fn past_cap(detail: String) -> wasmi::Error {
+    wasmi::Error::host(PastCap(detail))
 }
 
 fn check_count(section: &str, count: u32, cap: u32) -> Result<(), String> {
@@ -415,7 +477,7 @@ pub fn run<'h>(
     function_name: &str,
 ) -> Result<RunOutcome, RunFailure> {
     let engine = wasm_engine();
-    let module = compile(&engine, wasm).map_err(|e| {
+    let Compiled { module, .. } = compile(&engine, wasm).map_err(|e| {
         RunFailure::owing_nothing(match e {
             CompileError::Invalid(detail) => RunError::Compile(detail),
             CompileError::Limit(detail) => RunError::Limit(detail),

crates/xrpl-wasm-vm/tests/preflight.rs

@@ -552,8 +552,7 @@ fn modules() -> Vec<(&'static str, String)> {
 
 /// Screening refuses a module exactly when a run would refuse it at one of the
 /// stages screening covers — nothing it rejects would have run, and nothing
-/// it passes stops before the entry point is called. The exceptions are the ones
-/// [`what_static_screening_cannot_see`] lists.
+/// it passes stops before the entry point is called.
 #[test]
 fn screening_and_a_run_agree() {
     let host = FakeHost::new();
@@ -692,7 +691,7 @@ fn an_exported_table_past_the_cap_does_not_pass() {
     ));
 }
 
-/// Both caps are applied in one pass over the exports, so neither may end the walk
+/// Both caps are applied in one pass over the declarations, so neither may end the walk
 /// early: a passing memory must not hide a failing table declared after it, and a
 /// passing table must not hide a failing memory.
 #[test]
@@ -729,36 +728,32 @@ fn a_declared_table_maximum_past_the_cap_still_passes() {
     ));
 }
 
-/// The gap, listed rather than described. A memory or a table a module keeps to
-/// itself is not in its exports, so these are the modules that pass screening and
-/// then fail to *instantiate* — which is why a run's refusal at that stage cannot be
-/// read as the node's fault.
+/// A memory or a table the module keeps to itself is screened as an exported one
+/// is: the caps read the module's declarations, not its exports. The table row is the
+/// one that matters — Rust exports `__indirect_function_table` only under
+/// `--export-table`, so unexported is a table's normal shape, and a contract with
+/// more indirect-call targets than the cap would otherwise reach the ledger and fail
+/// every finish.
 ///
-/// The two entries are not equally remote. A contract needs an exported memory to
-/// make any host call, so the memory row can do nothing but compute and the SDK does
-/// not produce one. A table, though, is *normally* unexported — Rust exports
-/// `__indirect_function_table` only under `--export-table` — so the table row is the
-/// shape a hostile module actually takes, and the store's limiter is the only thing
-/// standing in front of it.
+/// The store's limiter still refuses both at instantiation, for a module that
+/// reaches a run unscreened.
 #[test]
-fn what_static_screening_cannot_see() {
-    let host = FakeHost::new();
-
-    for (label, declaration) in [
-        ("memory", format!("(memory {})", MAX_MEMORY_PAGES + 1)),
-        (
-            "table",
-            format!("(table {} funcref)", MAX_TABLE_ELEMENTS + 1),
-        ),
-    ] {
-        let wat = format!(
+fn a_memory_or_table_kept_to_itself_is_still_screened() {
+    let private = |declaration: String| {
+        format!(
             r#"(module {declaration}
                  (func (export "finish") (result i32) (i32.const 0)))"#
-        );
+        )
+    };
+    let memory = private(format!("(memory {})", MAX_MEMORY_PAGES + 1));
+    let table = private(format!("(table {} funcref)", MAX_TABLE_ELEMENTS + 1));
 
-        passes(&wat);
+    assert_stage!(refusal(&memory), CheckError::Memory(_));
+    assert_stage!(refusal(&table), CheckError::Table(_));
 
-        let failure = match xrpl_wasm_vm::run(&assemble(&wat), PLENTY_OF_GAS, &host, ENTRY) {
+    let host = FakeHost::new();
+    for (label, wat) in [("memory", &memory), ("table", &table)] {
+        let failure = match xrpl_wasm_vm::run(&assemble(wat), PLENTY_OF_GAS, &host, ENTRY) {
             Err(failure) => failure,
             Ok(outcome) => panic!(
                 "the store's limiter must refuse the {label}, but the module returned {}",
@@ -897,8 +892,10 @@ fn check_all_reports_a_refusal_from_every_stage() {
                 CheckError::Import(_),
                 CheckError::Signature(_),
                 CheckError::EntryPoint(_),
-                CheckError::Memory(_),
+                // Section order, not declaration order: the table section precedes
+                // the memory section in the binary.
                 CheckError::Table(_),
+                CheckError::Memory(_),
             ]
         ),
         "{refusals:?}"

crates/xrpl-wasm-vm/tests/vm_limits.rs

@@ -130,8 +130,8 @@ fn an_initial_table_at_the_cap_is_allowed() {
 }
 
 /// The cap binds a table the module keeps to itself, which is the case that matters:
-/// a contract has no reason to export its table, so screening never sees the one a
-/// hostile module declares.
+/// a contract has no reason to export its table. Screening refuses this module too;
+/// the limiter is what stops one that reaches a run unscreened.
 #[test]
 fn the_table_cap_binds_an_unexported_table() {
     let host = FakeHost::new();

src/libxrpl/tx/wasm/WasmVM.cpp

@@ -55,10 +55,10 @@ outcome(rs::wasm_vm::RunResult const& run)
         case RunStatus::Trap:
         case RunStatus::NoMemory:
         // A module that will not instantiate is the contract's fault too. Screening
-        // cannot see every way this happens - a linear memory the module keeps to itself
-        // is absent from its exports - so a module can pass preflight and still be
-        // refused here. It is a deterministic property of the code either way, and one
-        // this node's own conduct had no part in.
+        // refuses the ways this is known to happen - an import the engine does not
+        // serve, a memory or table past its cap - but the store's limiter, not
+        // screening, has the last word. It is a deterministic property of the code
+        // either way, and one this node's own conduct had no part in.
         case RunStatus::Instantiate:
             return std::unexpected{WasmTER{.ter = tecFAILED_PROCESSING, .cost = cost}};
 

src/tests/libxrpl/tx/wasm/Preflight.cpp

@@ -128,8 +128,7 @@ TEST_F(PreflightTest, memory_past_the_cap_is_refused)
 }
 
 // A table is allocated in full at instantiation, before any gas is charged, so an oversized
-// one is refused before it can be escrowed. Screening sees only an *exported* table; the
-// store's limiter is what refuses the table a contract keeps to itself.
+// one is refused before it can be escrowed - exported or not.
 TEST_F(PreflightTest, table_past_the_cap_is_refused)
 {
     constexpr std::string_view tooMuch = R"wat(
@@ -142,6 +141,16 @@ TEST_F(PreflightTest, table_past_the_cap_is_refused)
     EXPECT_EQ(preflight(tooMuch), temINVALID_BYTECODE);
     EXPECT_THAT(logged(), testing::HasSubstr("table: initial table of 1025 elements"));
 
+    // The usual shape: Rust exports `__indirect_function_table` only under --export-table.
+    constexpr std::string_view unexported = R"wat(
+    (module
+      (memory (export "memory") 1)
+      (table 1025 funcref)
+      (func (export "escrow_finish") (result i32) (i32.const 0)))
+    )wat";
+
+    EXPECT_EQ(preflight(unexported), temINVALID_BYTECODE);
+
     constexpr std::string_view atTheCap = R"wat(
     (module
       (memory (export "memory") 1)

src/tests/libxrpl/tx/wasm/WasmVM.cpp

@@ -116,19 +116,19 @@ TEST_F(WasmVMTest, host_call_with_no_exported_memory_fails)
 }
 
 // A module that will not instantiate is the contract's fault and is charged, not the node's.
-// Screening does not see every way this happens - a linear memory the module keeps to itself
-// is absent from its exports - so such a module can pass preflight and still be refused here.
+// Screening refuses this one, but the store's limiter is what refuses it in a run, so the
+// run is checked without relying on screening having happened.
 TEST_F(WasmVMTest, module_that_will_not_instantiate_is_charged_to_the_contract)
 {
-    // 129 pages, not exported, so nothing outside the module declares it.
+    // 129 pages, not exported: screening reads the declaration, not the exports.
     static constexpr std::string_view wat = R"wat(
     (module
       (memory 129)
       (func (export "escrow_finish") (result i32) (i32.const 0)))
     )wat";
 
-    EXPECT_EQ(preflightEscrowWasm(assembleWat(wat), beast::Journal{sink}), tesSUCCESS)
-        << "screening cannot see an unexported memory";
+    EXPECT_EQ(preflightEscrowWasm(assembleWat(wat), beast::Journal{sink}), temINVALID_BYTECODE)
+        << "screening reads an unexported memory's declaration";
 
     auto const outcome = run(wat);