mirror of
https://github.com/XRPLF/rippled.git
synced 2026-08-21 14:20:56 +00:00
Fixes
This commit is contained in:
@@ -95,6 +95,8 @@ mod ffi {
|
||||
Import,
|
||||
/// No export of that name with signature `() -> i32`.
|
||||
EntryPoint,
|
||||
/// The module asks for more linear memory than the engine grants.
|
||||
Memory,
|
||||
/// The engine panicked. A defect in this crate or the one below it, and
|
||||
/// not a fault in the module — which is why it is a status of its own
|
||||
/// rather than one more way a contract can be malformed.
|
||||
@@ -377,6 +379,7 @@ impl From<&CheckError> for ffi::CheckStatus {
|
||||
CheckError::Compile(_) => ffi::CheckStatus::Compile,
|
||||
CheckError::Import(_) => ffi::CheckStatus::Import,
|
||||
CheckError::EntryPoint(_) => ffi::CheckStatus::EntryPoint,
|
||||
CheckError::Memory(_) => ffi::CheckStatus::Memory,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -559,6 +562,7 @@ mod tests {
|
||||
CheckError::Compile(String::new()),
|
||||
CheckError::Import(String::new()),
|
||||
CheckError::EntryPoint(String::new()),
|
||||
CheckError::Memory(String::new()),
|
||||
]
|
||||
}
|
||||
|
||||
|
||||
@@ -7,18 +7,21 @@
|
||||
//! makes it callable from a transaction's preflight, which has no ledger to serve
|
||||
//! host calls from.
|
||||
//!
|
||||
//! Two things it deliberately does not screen. A module exporting no linear
|
||||
//! Two things it deliberately does not screen. A module exporting **no** linear
|
||||
//! memory passes: a contract that makes no host call needs none, and one that
|
||||
//! does is refused at the call and charged for what it burned. A start section
|
||||
//! passes: it is guest code, and executing it is the one thing a check must not
|
||||
//! do.
|
||||
//! passes: it is guest code, and executing it is the one thing a check must not do
|
||||
//! — a trap in one is charged to the contract like any other trap.
|
||||
//!
|
||||
//! One thing it screens that a run can only discover: an exported memory larger
|
||||
//! than the engine grants. See [`check_memory`] for what stays invisible.
|
||||
|
||||
use std::fmt;
|
||||
use wasmi::{ExternType, FuncType, Module, ValType};
|
||||
use xrpl_host_functions::HostFunctionSpec;
|
||||
|
||||
use crate::register::HOST_MODULE;
|
||||
use crate::vm::compile;
|
||||
use crate::vm::{MAX_MEMORY_PAGES, compile};
|
||||
|
||||
/// Why a module cannot be run. One variant per stage, since the caller maps the
|
||||
/// stages separately.
|
||||
@@ -32,6 +35,8 @@ pub enum CheckError {
|
||||
Import(String),
|
||||
/// No export named `function_name` with signature `() -> i32`.
|
||||
EntryPoint(String),
|
||||
/// The module asks for more linear memory than the engine grants.
|
||||
Memory(String),
|
||||
}
|
||||
|
||||
impl fmt::Display for CheckError {
|
||||
@@ -42,16 +47,23 @@ impl fmt::Display for CheckError {
|
||||
// The detail says which of the entry point's failures this is, since
|
||||
// "no entry point" would be wrong for an export of the wrong type.
|
||||
CheckError::EntryPoint(detail) => write!(f, "{detail}"),
|
||||
CheckError::Memory(detail) => write!(f, "memory: {detail}"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Screen `wasm`: it must compile, import only what the engine serves, and export
|
||||
/// `function_name` as `() -> i32`.
|
||||
/// Screen `wasm`: it must compile, import only what the engine serves, export
|
||||
/// `function_name` as `() -> i32`, and ask for no more memory than it may have.
|
||||
///
|
||||
/// The stages are ordered by how much of the module each explains. An import fault
|
||||
/// is reported before a missing entry point because the imports are what the rest of
|
||||
/// the module is built on; memory comes last, being a resource request rather than a
|
||||
/// mistake about the ABI.
|
||||
pub fn check(wasm: &[u8], function_name: &str) -> Result<(), CheckError> {
|
||||
let module = compile(wasm).map_err(CheckError::Compile)?;
|
||||
check_imports(&module)?;
|
||||
check_entry_point(&module, function_name)
|
||||
check_entry_point(&module, function_name)?;
|
||||
check_memory(&module)
|
||||
}
|
||||
|
||||
/// Every import must be one the linker defines. The first that is not ends the
|
||||
@@ -104,6 +116,38 @@ fn is_entry_point(ty: &FuncType) -> bool {
|
||||
ty.params().is_empty() && matches!(ty.results(), [ValType::I32])
|
||||
}
|
||||
|
||||
/// A module may not declare more linear memory than the engine grants.
|
||||
///
|
||||
/// Only what it *exports* is visible here. A memory a module keeps to itself is not
|
||||
/// in its exports, and the store's limiter is what refuses that one — at
|
||||
/// instantiation, where the run is charged nothing and the caller cannot tell it
|
||||
/// from any other resource failure. Screening the exported case covers every
|
||||
/// contract built against the guest SDK, since a contract needs an exported memory
|
||||
/// to make a host call at all.
|
||||
fn check_memory(module: &Module) -> Result<(), CheckError> {
|
||||
for export in module.exports() {
|
||||
if let ExternType::Memory(ty) = export.ty() {
|
||||
check_initial_pages(ty.minimum()).map_err(CheckError::Memory)?;
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Whether the engine will grant a memory of this declared initial size.
|
||||
///
|
||||
/// The *minimum* only: a declared maximum past the cap is legal and simply
|
||||
/// unreachable, which `vm_limits::a_declared_maximum_past_the_cap_is_allowed_but_
|
||||
/// unreachable` pins on the run side. Refusing it here would turn a runnable
|
||||
/// contract away.
|
||||
fn check_initial_pages(pages: u64) -> Result<(), String> {
|
||||
if pages > u64::from(MAX_MEMORY_PAGES) {
|
||||
return Err(format!(
|
||||
"initial memory of {pages} pages is past the {MAX_MEMORY_PAGES}-page cap"
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// How an entry-point lookup failed, in the words both stages use: a check and a
|
||||
/// run describe the same module the same way, and "no entry point" would send a
|
||||
/// contract author looking for a function they already have.
|
||||
@@ -250,6 +294,22 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// The cap itself is granted; one page past it is not. The boundary is the whole
|
||||
/// rule, and it is the same boundary the store's limiter applies at
|
||||
/// instantiation.
|
||||
#[test]
|
||||
fn the_initial_memory_may_reach_the_cap_but_not_pass_it() {
|
||||
assert_eq!(check_initial_pages(0), Ok(()));
|
||||
assert_eq!(check_initial_pages(u64::from(MAX_MEMORY_PAGES)), Ok(()));
|
||||
|
||||
let past = u64::from(MAX_MEMORY_PAGES) + 1;
|
||||
let refusal = check_initial_pages(past).expect_err("one page past the cap");
|
||||
assert_eq!(
|
||||
refusal,
|
||||
format!("initial memory of {past} pages is past the {MAX_MEMORY_PAGES}-page cap")
|
||||
);
|
||||
}
|
||||
|
||||
/// The bridge logs this string and the C++ tests match on it, so the stage's
|
||||
/// prefix is part of the interface rather than a debugging aid.
|
||||
#[test]
|
||||
@@ -258,6 +318,10 @@ mod tests {
|
||||
CheckError::Compile("bad magic".to_string()).to_string(),
|
||||
"compile: bad magic"
|
||||
);
|
||||
assert_eq!(
|
||||
CheckError::Memory("initial memory of 129 pages".to_string()).to_string(),
|
||||
"memory: initial memory of 129 pages"
|
||||
);
|
||||
assert_eq!(
|
||||
CheckError::Import("no host function 'x'".to_string()).to_string(),
|
||||
"import: no host function 'x'"
|
||||
|
||||
@@ -83,8 +83,9 @@ pub struct RunOutcome {
|
||||
pub enum RunError {
|
||||
/// `wasm` is not a valid module under this engine's configuration.
|
||||
Compile(String),
|
||||
/// The module compiled but would not instantiate: an import the linker does
|
||||
/// not define, an initial memory past the page cap, a trapping start section.
|
||||
/// The module compiled but the engine would not accept it: an import the
|
||||
/// linker does not define, or an initial memory past the page cap. Not guest
|
||||
/// code failing — a start section that traps is [`RunError::Trap`].
|
||||
Instantiate(String),
|
||||
/// No export named `function_name` with signature `() -> i32`: absent, not a
|
||||
/// function, or a function of another type — which the detail tells apart.
|
||||
@@ -99,7 +100,8 @@ pub enum RunError {
|
||||
/// to resolve the memory from.
|
||||
NoMemory,
|
||||
/// The guest trapped: `unreachable`, division by zero, an out-of-bounds
|
||||
/// access, or `memory.grow` past the page cap.
|
||||
/// access, or `memory.grow` past the page cap. Wherever the guest was
|
||||
/// executing, including a start section during instantiation.
|
||||
Trap(String),
|
||||
}
|
||||
|
||||
@@ -187,6 +189,22 @@ fn guest_halted(error: &wasmi::Error) -> Option<RunError> {
|
||||
(error.as_trap_code() == Some(TrapCode::OutOfFuel)).then_some(RunError::OutOfGas)
|
||||
}
|
||||
|
||||
/// Why instantiation failed, once [`guest_halted`] has ruled out the two conditions
|
||||
/// that can arise anywhere.
|
||||
///
|
||||
/// A start section is guest code, so it can trap on its own — `unreachable`, a
|
||||
/// division by zero, an out-of-bounds access — and a trap is the guest's fault
|
||||
/// wherever it happens. Naming that after the *stage* would file it beside the
|
||||
/// module faults a caller treats as its own defect, and charge nothing for
|
||||
/// instructions the contract burned. What is left for [`RunError::Instantiate`] is a
|
||||
/// module the linker or the store would not accept at all.
|
||||
fn instantiation_failure(error: &wasmi::Error) -> RunError {
|
||||
match error.as_trap_code() {
|
||||
Some(_) => RunError::Trap(error.to_string()),
|
||||
None => RunError::Instantiate(error.to_string()),
|
||||
}
|
||||
}
|
||||
|
||||
/// The outcome a host-fatal `HostError` is.
|
||||
///
|
||||
/// Exhaustive rather than closed with a wildcard, so a variant added to the ABI
|
||||
@@ -306,7 +324,7 @@ pub fn run<'h>(
|
||||
let instance = match linker.instantiate_and_start(&mut store, &module) {
|
||||
Ok(instance) => instance,
|
||||
Err(e) => {
|
||||
let error = guest_halted(&e).unwrap_or_else(|| RunError::Instantiate(e.to_string()));
|
||||
let error = guest_halted(&e).unwrap_or_else(|| instantiation_failure(&e));
|
||||
return Err(failed(&store, gas, error));
|
||||
}
|
||||
};
|
||||
|
||||
@@ -375,42 +375,86 @@ fn screening_and_a_run_agree() {
|
||||
}
|
||||
}
|
||||
|
||||
/// The gap, listed rather than described. A start section is guest code, and
|
||||
/// running it is what screening must not do; a memory the module keeps to itself
|
||||
/// is not in its exports. Both leave a module that passes screening and then fails
|
||||
/// to instantiate, which is why a run's own refusal cannot be treated as the
|
||||
/// node's fault.
|
||||
/// A module asking for more memory than the engine grants is refused, so the
|
||||
/// contract that could never run does not reach the ledger. The cap itself passes.
|
||||
#[test]
|
||||
fn an_exported_memory_past_the_cap_does_not_pass() {
|
||||
let wat = module(
|
||||
&[&format!(
|
||||
r#"(memory (export "memory") {})"#,
|
||||
MAX_MEMORY_PAGES + 1
|
||||
)],
|
||||
"(i32.const 0)",
|
||||
);
|
||||
let refusal = assert_stage!(refusal(&wat), CheckError::Memory(_)).to_string();
|
||||
assert!(refusal.contains("past the 128-page cap"), "{refusal}");
|
||||
|
||||
passes(&module(
|
||||
&[&format!(r#"(memory (export "memory") {MAX_MEMORY_PAGES})"#)],
|
||||
"(i32.const 0)",
|
||||
));
|
||||
}
|
||||
|
||||
/// A declared *maximum* past the cap is legal and simply unreachable, so screening
|
||||
/// must not turn it away: `vm_limits` runs this very module to completion.
|
||||
#[test]
|
||||
fn a_declared_maximum_past_the_cap_still_passes() {
|
||||
passes(&module(
|
||||
&[&format!(
|
||||
r#"(memory (export "memory") 1 {})"#,
|
||||
MAX_MEMORY_PAGES + 1
|
||||
)],
|
||||
"(i32.const 0)",
|
||||
));
|
||||
}
|
||||
|
||||
/// The gap, listed rather than described, and now one entry long. A memory a module
|
||||
/// keeps to itself is not in its exports, so this is the one module that passes
|
||||
/// screening and then fails to *instantiate* — which is why a run's refusal at that
|
||||
/// stage cannot be read as the node's fault.
|
||||
///
|
||||
/// A contract needs an exported memory to make any host call, so a module of this
|
||||
/// shape can do nothing but compute; the SDK does not produce one.
|
||||
#[test]
|
||||
fn what_static_screening_cannot_see() {
|
||||
let host = FakeHost::new();
|
||||
let wat = format!(
|
||||
r#"(module (memory {})
|
||||
(func (export "finish") (result i32) (i32.const 0)))"#,
|
||||
MAX_MEMORY_PAGES + 1
|
||||
);
|
||||
|
||||
for (label, wat) in [
|
||||
(
|
||||
"a start section that traps",
|
||||
format!(
|
||||
r#"(module {ONE_PAGE}
|
||||
(func $init (unreachable))
|
||||
(start $init)
|
||||
(func (export "finish") (result i32) (i32.const 0)))"#
|
||||
),
|
||||
),
|
||||
(
|
||||
"an unexported memory past the cap",
|
||||
format!(
|
||||
r#"(module (memory {})
|
||||
(func (export "finish") (result i32) (i32.const 0)))"#,
|
||||
MAX_MEMORY_PAGES + 1
|
||||
),
|
||||
),
|
||||
] {
|
||||
let wasm = assemble(&wat);
|
||||
passes(&wat);
|
||||
passes(&wat);
|
||||
|
||||
let failure = xrpl_wasm_vm::run(&wasm, PLENTY_OF_GAS, &host, ENTRY)
|
||||
.expect_err(&format!("{label}: expected the run to refuse it"));
|
||||
assert!(
|
||||
matches!(failure.error, RunError::Instantiate(_)),
|
||||
"{label}: {failure}"
|
||||
);
|
||||
}
|
||||
let failure = xrpl_wasm_vm::run(&assemble(&wat), PLENTY_OF_GAS, &host, ENTRY)
|
||||
.expect_err("the store's limiter must refuse the memory");
|
||||
assert!(
|
||||
matches!(failure.error, RunError::Instantiate(_)),
|
||||
"{failure}"
|
||||
);
|
||||
}
|
||||
|
||||
/// A start section is guest code, so screening cannot see whether it traps — but it
|
||||
/// no longer has to. A trap is the guest's fault wherever it happens, so the run
|
||||
/// charges the contract for what it burned instead of reporting a module the node
|
||||
/// should have screened.
|
||||
#[test]
|
||||
fn a_start_section_screening_cannot_see_is_charged_as_a_trap() {
|
||||
let host = FakeHost::new();
|
||||
let wat = format!(
|
||||
r#"(module {ONE_PAGE}
|
||||
(func $init (unreachable))
|
||||
(start $init)
|
||||
(func (export "finish") (result i32) (i32.const 0)))"#
|
||||
);
|
||||
|
||||
passes(&wat);
|
||||
|
||||
let failure = xrpl_wasm_vm::run(&assemble(&wat), PLENTY_OF_GAS, &host, ENTRY)
|
||||
.expect_err("a start section that traps must not complete the run");
|
||||
assert!(matches!(failure.error, RunError::Trap(_)), "{failure}");
|
||||
assert!(
|
||||
failure.fuel_used > 0,
|
||||
"charged for what it burned: {failure}"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -372,8 +372,14 @@ fn an_unused_import_is_still_linked() {
|
||||
/// is even looked up, and `set_fuel` and the memory limiter are both installed by
|
||||
/// then — so it is metered like any other guest code, and a run it stops is
|
||||
/// charged for what it burned.
|
||||
///
|
||||
/// Reported as a **trap**, not as a module that would not instantiate: a trap is the
|
||||
/// guest's fault wherever it happens, and the stage a run stopped at is not what the
|
||||
/// caller maps. Filing it under the stage would put a contract's own defect among the
|
||||
/// faults a caller treats as the node's, and charge nothing for the instructions the
|
||||
/// contract burned reaching it.
|
||||
#[test]
|
||||
fn a_trapping_start_section_fails_instantiation_and_is_charged() {
|
||||
fn a_trapping_start_section_is_a_guest_trap_and_is_charged() {
|
||||
let host = FakeHost::new();
|
||||
|
||||
let wat = format!(
|
||||
@@ -384,8 +390,8 @@ fn a_trapping_start_section_fails_instantiation_and_is_charged() {
|
||||
);
|
||||
let failure = assert_stage!(
|
||||
run_with_gas(&wat, PLENTY_OF_GAS, &host)
|
||||
.expect_err("a start section that traps must not instantiate"),
|
||||
RunError::Instantiate(_)
|
||||
.expect_err("a start section that traps must not complete the run"),
|
||||
RunError::Trap(_)
|
||||
);
|
||||
assert!(
|
||||
failure.fuel_used > 0,
|
||||
@@ -393,6 +399,31 @@ fn a_trapping_start_section_fails_instantiation_and_is_charged() {
|
||||
);
|
||||
}
|
||||
|
||||
/// What `RunError::Instantiate` is left to mean: a module the linker or the store
|
||||
/// would not accept, rather than one whose guest code failed. Its two shapes, so the
|
||||
/// variant is not left standing for nothing.
|
||||
#[test]
|
||||
fn instantiation_failure_is_a_module_the_engine_will_not_accept() {
|
||||
let host = FakeHost::new();
|
||||
|
||||
// The linker defines no such import.
|
||||
let wat = module(
|
||||
&[
|
||||
r#"(import "host_lib" "no_such_function" (func $f (result i32)))"#,
|
||||
ONE_PAGE,
|
||||
],
|
||||
"(call $f)",
|
||||
);
|
||||
assert_stage!(failure(&wat, &host), RunError::Instantiate(_));
|
||||
|
||||
// The store's limiter will not grant the memory, and does not trap to say so.
|
||||
let wat = module(
|
||||
&[&format!("(memory {})", MAX_MEMORY_PAGES + 1)],
|
||||
"(i32.const 0)",
|
||||
);
|
||||
assert_stage!(failure(&wat, &host), RunError::Instantiate(_));
|
||||
}
|
||||
|
||||
/// A start section that runs out of gas is reported as out of gas, not as a module
|
||||
/// that would not instantiate. The stage a run stopped at is not what the caller
|
||||
/// maps — the reason is — and gas exhaustion is one outcome wherever the guest
|
||||
|
||||
@@ -59,14 +59,20 @@ node's, and charging a transaction for a node's defect would write that defect i
|
||||
|---|---|---|
|
||||
| `Ok` | — | `gas_used` |
|
||||
| `OutOfGas` | `tecOUT_OF_GAS` | `gas_used` |
|
||||
| `Trap`, `NoMemory` | `tecFAILED_PROCESSING` | `gas_used` |
|
||||
| `Compile`, `Instantiate`, `EntryPoint` | `tecINTERNAL` | none |
|
||||
| `Trap`, `NoMemory`, `Instantiate` | `tecFAILED_PROCESSING` | `gas_used` |
|
||||
| `Compile`, `EntryPoint` | `tecINTERNAL` | none |
|
||||
| `Internal`, `Panic` | `tecINTERNAL` | none |
|
||||
|
||||
The `Compile`/`Instantiate`/`EntryPoint` row is `tecINTERNAL` because preflight is meant to
|
||||
have refused such a module with `temBAD_WASM` long before apply. **That row is now known to
|
||||
be wrong for `Instantiate`** — see below. `NoMemory` had no old TER to match (it used to
|
||||
reach the guest as code -14); `tecFAILED_PROCESSING` treats it as the contract fault it is.
|
||||
`Compile` and `EntryPoint` are `tecINTERNAL` because preflight decides both from the same
|
||||
bytes and the same engine, so agreement is not a matter of degree: reaching apply means the
|
||||
screening did not happen. `Instantiate` is **not** in that row, and the reason is the point of
|
||||
the whole arrangement — see below. `NoMemory` had no old TER to match (it used to reach the
|
||||
guest as code -14); `tecFAILED_PROCESSING` treats it as the contract fault it is.
|
||||
|
||||
One thing to settle before a long-lived escrow exists: `Compile` is only a node fault while
|
||||
the engine's configuration never changes. A contract created under one feature set and
|
||||
finished under another could legitimately fail to compile at apply, so either the config is
|
||||
amendment-gated or `Compile` joins the charged row.
|
||||
|
||||
`gas <= 0` is refused as `temBAD_AMOUNT` before the engine is called, restoring what
|
||||
`WasmiEngine::run` did — see [open-questions.md](open-questions.md).
|
||||
@@ -79,7 +85,7 @@ one answer, because a caller's only decision is whether the transaction may proc
|
||||
| `CheckStatus` | `NotTEC` |
|
||||
|---|---|
|
||||
| `Ok` | `tesSUCCESS` |
|
||||
| `Compile`, `Import`, `EntryPoint` | `temBAD_WASM` |
|
||||
| `Compile`, `Import`, `EntryPoint`, `Memory` | `temBAD_WASM` |
|
||||
| `Panic` | `telFAILED_PROCESSING` |
|
||||
|
||||
The statuses stay distinct anyway: the *detail* is what a contract author needs, and one
|
||||
@@ -99,19 +105,23 @@ std::string_view) -> NotTEC`, with no `HostFunctions&`. The deleted `preflightEs
|
||||
took one and could therefore never have been called from a real `preflight()` —
|
||||
`PreflightContext` has no view to build a host over.
|
||||
|
||||
## Why `Instantiate` should stop being `tecINTERNAL`
|
||||
## Why `Instantiate` is the contract's fault
|
||||
|
||||
`check` closes compile, imports and the entry point, but two ways instantiation fails are
|
||||
invisible to it: a start section that traps, and a linear memory over the page cap that the
|
||||
module does not export ([engine.md](engine.md)). Both are deterministic properties of the
|
||||
module, so a contract can pass preflight, be escrowed, and then fail to instantiate at
|
||||
apply — where the map currently blames the node and charges nothing.
|
||||
The map must not depend on preflight being exhaustive, because it cannot be. `check` closes
|
||||
compile, imports, the entry point and an exported memory over the page cap — but a memory a
|
||||
module *keeps to itself* is absent from its exports, so such a module passes screening and
|
||||
then fails to instantiate ([engine.md](engine.md)). That is a deterministic property of the
|
||||
code, identical on every node, and nothing this node did; charging it as
|
||||
`tecFAILED_PROCESSING` says so, where `tecINTERNAL` would blame the node and forgive the gas.
|
||||
|
||||
The fix is two lines and its own change: report `Instantiate` as `tecFAILED_PROCESSING`
|
||||
with its gas, and in `vm::run` classify a failure carrying a trap code (`e.as_trap_code()`)
|
||||
as `Trap` rather than `Instantiate`, since a start section trapping is guest code trapping.
|
||||
`tecINTERNAL` then means what it says — `Internal` and `Panic`, the node's own defects — and
|
||||
the map stops depending on preflight's completeness for its correctness.
|
||||
The other half is in the engine rather than the map: `vm::instantiation_failure` reports a
|
||||
failure carrying a trap code as `RunError::Trap`, because a start section that traps is guest
|
||||
code trapping, and a trap is the guest's fault wherever it happens. What is left for
|
||||
`Instantiate` is a module the linker or the store would not accept at all —
|
||||
`vm_limits::instantiation_failure_is_a_module_the_engine_will_not_accept` pins both shapes.
|
||||
|
||||
So `tecINTERNAL` now means what it says: `Internal` and `Panic`, the node's own defects, plus
|
||||
the two stages preflight decides exactly.
|
||||
|
||||
## The one copy left on the byte path, and why it needs `HostFunctions` to change
|
||||
|
||||
|
||||
@@ -30,7 +30,9 @@ cost that cannot be read becomes `RunError::Internal` rather than a number — `
|
||||
forgive a run its whole cost and `gas` would charge an untouched one for everything.
|
||||
`guest_halted` asks "did the guest halt?" at *every* stage from instantiation on, so a start
|
||||
section that burns the limit is `OutOfGas`, not `Instantiate`: the stage a run stopped at is
|
||||
not what the caller maps.
|
||||
not what the caller maps. `instantiation_failure` finishes the thought — a failure carrying a
|
||||
trap code is `Trap`, since a start section that traps is guest code trapping — leaving
|
||||
`Instantiate` to mean a module the linker or the store would not accept.
|
||||
|
||||
**Two ways a byte answer reaches the guest**, both taking a `Region`:
|
||||
|
||||
@@ -129,12 +131,20 @@ unit tests state each rule, its precedence and its wording on inputs built direc
|
||||
an import breaking two rules reports the namespace, which is what explains the module's other
|
||||
imports too.
|
||||
|
||||
What it cannot see is guest behaviour and anything absent from the module's exports: a start
|
||||
section that traps, and a linear memory over the page cap that the module keeps to itself.
|
||||
Both pass the check and then fail instantiation, which is why a run's own refusal at that
|
||||
stage cannot be read as the node's fault. `what_static_screening_cannot_see` lists them and
|
||||
`screening_and_a_run_agree` pins the equivalence everywhere else — in both directions, so a
|
||||
rule that refused a contract the engine would have served fails too.
|
||||
A fourth stage screens what a module *declares*: an exported linear memory whose initial size
|
||||
is past the page cap is refused, since the store's limiter would refuse it anyway. The
|
||||
**minimum** only — a declared maximum past the cap is legal and simply unreachable. Only the
|
||||
exported memory is visible, which is enough for every contract the guest SDK produces, since
|
||||
a contract needs an exported memory to make a host call at all.
|
||||
|
||||
What stays invisible is one module: a memory the module keeps to itself, over the cap, which
|
||||
passes the check and then fails instantiation — the reason a run's refusal at that stage is
|
||||
charged to the contract rather than blamed on the node ([bridge.md](bridge.md)). A start
|
||||
section is invisible too, but no longer matters: a trap in one is reported as
|
||||
`RunError::Trap` and charged like any other trap.
|
||||
`what_static_screening_cannot_see` is that one module, and `screening_and_a_run_agree` pins
|
||||
the equivalence everywhere else — in both directions, so a rule that refused a contract the
|
||||
engine would have served fails too.
|
||||
|
||||
Import **signatures** are the deliberate gap: `check` compares names and kinds, not types, so
|
||||
a mistyped import still parts a module from the engine at instantiation. Closing it needs the
|
||||
|
||||
@@ -46,7 +46,8 @@ than guessing. See [history.md](history.md) for what is worth recovering.
|
||||
- `xrpl-host-functions-macros/` — the proc macro. An implementation detail of the crate
|
||||
above, deliberately not re-exported: the ABI has one declaration site.
|
||||
- `xrpl-wasm-vm/` — the wasmi wrapper. `vm.rs` (engine, store, `run`), `preflight.rs`
|
||||
(`check` — compile, imports, entry point, with no host, store or gas), `abi.rs` (gas,
|
||||
(`check` — compile, imports, entry point, declared memory, with no host, store or
|
||||
gas), `abi.rs` (gas,
|
||||
transfer budget, guest-memory marshaling), `region.rs` (the `(ptr, len)` type),
|
||||
`register.rs` (one `func_wrap` per host function). See [engine.md](engine.md).
|
||||
- `xrpl-wasm-vm-ffi/` — the cxx bridge, all three crossings. `RunStatus`/`RunResult` and
|
||||
@@ -70,10 +71,10 @@ than guessing. See [history.md](history.md) for what is worth recovering.
|
||||
## Current state (2026-08-04)
|
||||
|
||||
**The whole workspace is green**: `cargo test --workspace`, `clippy --workspace
|
||||
--all-targets`, `fmt`, and `cargo doc -p xrpl-wasm-vm --no-deps`. **168 tests** — 33 macro,
|
||||
12 facade, 1 doctest, **105 in `xrpl-wasm-vm`** (19 unit; 86 integration — 13 `budgets`,
|
||||
12 `host_calls`, 23 `memory_policy`, 17 `preflight`, 21 `vm_limits`), 15 in
|
||||
`xrpl-wasm-vm-ffi`, 2 in `xrpl-wasm-testkit`. On the C++ side, **37 tests over the whole
|
||||
--all-targets`, `fmt`, and `cargo doc -p xrpl-wasm-vm --no-deps`. **173 tests** — 33 macro,
|
||||
12 facade, 1 doctest, **110 in `xrpl-wasm-vm`** (20 unit; 90 integration — 13 `budgets`,
|
||||
12 `host_calls`, 23 `memory_policy`, 20 `preflight`, 22 `vm_limits`), 15 in
|
||||
`xrpl-wasm-vm-ffi`, 2 in `xrpl-wasm-testkit`. On the C++ side, **40 tests over the whole
|
||||
loop** in seven fixtures: `./xrpl_tests
|
||||
--gtest_filter='WasmVMTest.*:*Call.*:PreflightTest.*'`.
|
||||
|
||||
@@ -89,12 +90,7 @@ functions are registered (`ldgr_index`, `home_le_field`, `sha512_half`, `trace`,
|
||||
|
||||
## Next
|
||||
|
||||
1. **Move `Instantiate` off `tecINTERNAL`**, and report a trapping start section as `Trap`
|
||||
rather than as a module that would not instantiate. Two lines plus their tests, and it is
|
||||
what actually removes the papering-over: `check` cannot see either of the two remaining
|
||||
instantiate faults, so the apply-side map must stop depending on preflight's
|
||||
completeness. [bridge.md](bridge.md) has the reasoning.
|
||||
2. **A caller.** `EscrowFinish.cpp` still has no wasm reference, so `runEscrowWasm` and
|
||||
1. **A caller.** `EscrowFinish.cpp` still has no wasm reference, so `runEscrowWasm` and
|
||||
`preflightEscrowWasm` are reached only from `src/tests/libxrpl/tx/wasm/`. Wiring it up is
|
||||
what makes `WasmHostFunctionsImpl` (over a real `ApplyContext`) the host in production
|
||||
rather than in principle. **Blocked on the protocol fields**: `FinishFunction` and
|
||||
@@ -102,16 +98,16 @@ functions are registered (`ldgr_index`, `home_le_field`, `sha512_half`, `trace`,
|
||||
`FinishFunction` also has to answer the **contract code-size cap** — there is none, and
|
||||
preflight's cost is linear in the blob (a 249 KB module of duplicate imports measures
|
||||
1.5 ms, mostly wasmi's own parse).
|
||||
3. **Import signatures at preflight.** `check` compares an import's namespace, name and
|
||||
2. **Import signatures at preflight.** `check` compares an import's namespace, name and
|
||||
kind, not its type, so a mistyped import still parts a module from the engine at
|
||||
instantiation. Deferred to when `host_functions!` generates the wasm-level lowering,
|
||||
which the C header and the typed `link_*` shims in [abi.md](abi.md) both want anyway.
|
||||
Note the deleted C++ `check` did not compare signatures either, so this is inherited
|
||||
rather than new.
|
||||
4. **A gas parity oracle.** `Wasm_test.cpp` asserts exact gas numbers (e.g. 29'502) and is
|
||||
3. **A gas parity oracle.** `Wasm_test.cpp` asserts exact gas numbers (e.g. 29'502) and is
|
||||
the best oracle we have, but it is commented out and its fixtures cannot run on this
|
||||
engine — see the `env` finding in [testing.md](testing.md).
|
||||
5. **The `Bytes`-by-value copy in `HostFunctions`** — [bridge.md](bridge.md). A
|
||||
4. **The `Bytes`-by-value copy in `HostFunctions`** — [bridge.md](bridge.md). A
|
||||
49-signature sweep, so it wants a caller to measure against first.
|
||||
|
||||
Also open: the two performance items and the ABI questions in
|
||||
|
||||
@@ -51,14 +51,20 @@ outcome(rs::wasm_vm::RunResult const& run)
|
||||
// its host calls need - so it is charged for what it burned reaching that point.
|
||||
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.
|
||||
case RunStatus::Instantiate:
|
||||
return std::unexpected(WasmTER{.ter = tecFAILED_PROCESSING, .cost = cost});
|
||||
|
||||
// A module that will not compile, instantiate, or expose the entry point should
|
||||
// have been refused at preflight with `temBAD_WASM`. Reaching apply means the
|
||||
// screening did not happen, which is a node-side fault rather than the
|
||||
// transaction's.
|
||||
// A module that will not compile, or does not expose the entry point, should have
|
||||
// been refused at preflight with `temBAD_WASM`: screening decides both from the
|
||||
// same bytes and the same engine, so agreeing here is not a matter of degree.
|
||||
// Reaching apply means the screening did not happen, which is a node-side fault
|
||||
// rather than the transaction's.
|
||||
case RunStatus::Compile:
|
||||
case RunStatus::Instantiate:
|
||||
case RunStatus::EntryPoint:
|
||||
// The host could not serve a call, or it threw and `HostContext` caught it.
|
||||
case RunStatus::Internal:
|
||||
@@ -117,11 +123,13 @@ verdict(CheckStatus status)
|
||||
case CheckStatus::Ok:
|
||||
return tesSUCCESS;
|
||||
|
||||
// The module will not compile, imports what no engine of this ABI serves, or
|
||||
// does not export the entry point as `() -> i32`.
|
||||
// The module will not compile, imports what no engine of this ABI serves, does
|
||||
// not export the entry point as `() -> i32`, or asks for more linear memory than
|
||||
// it may have.
|
||||
case CheckStatus::Compile:
|
||||
case CheckStatus::Import:
|
||||
case CheckStatus::EntryPoint:
|
||||
case CheckStatus::Memory:
|
||||
return temBAD_WASM;
|
||||
|
||||
// The engine panicked: a defect in the engine, reported rather than fatal to
|
||||
|
||||
@@ -103,6 +103,28 @@ TEST_F(PreflightTest, ImportFromAnotherModuleIsRefused)
|
||||
EXPECT_THAT(logged(), testing::HasSubstr("is not from 'host_lib'"));
|
||||
}
|
||||
|
||||
// A contract asking for more linear memory than the engine grants can never run, so it is
|
||||
// refused before it can be escrowed. The cap itself is granted.
|
||||
TEST_F(PreflightTest, MemoryPastTheCapIsRefused)
|
||||
{
|
||||
constexpr std::string_view tooMuch = R"wat(
|
||||
(module
|
||||
(memory (export "memory") 129)
|
||||
(func (export "escrow_finish") (result i32) (i32.const 0)))
|
||||
)wat";
|
||||
|
||||
EXPECT_EQ(preflight(tooMuch), temBAD_WASM);
|
||||
EXPECT_THAT(logged(), testing::HasSubstr("memory: initial memory of 129 pages"));
|
||||
|
||||
constexpr std::string_view atTheCap = R"wat(
|
||||
(module
|
||||
(memory (export "memory") 128)
|
||||
(func (export "escrow_finish") (result i32) (i32.const 0)))
|
||||
)wat";
|
||||
|
||||
EXPECT_EQ(preflight(atTheCap), tesSUCCESS);
|
||||
}
|
||||
|
||||
TEST_F(PreflightTest, MissingEntryPointIsRefused)
|
||||
{
|
||||
constexpr std::string_view wat = R"wat(
|
||||
|
||||
@@ -120,6 +120,49 @@ TEST_F(WasmVMTest, HostCallWithNoExportedMemoryFails)
|
||||
EXPECT_TRUE(outcome.error().cost.has_value());
|
||||
}
|
||||
|
||||
// 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.
|
||||
TEST_F(WasmVMTest, ModuleThatWillNotInstantiateIsChargedToTheContract)
|
||||
{
|
||||
// 129 pages, not exported, so nothing outside the module declares it.
|
||||
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";
|
||||
|
||||
auto const outcome = run(wat);
|
||||
|
||||
ASSERT_FALSE(outcome.has_value());
|
||||
EXPECT_EQ(outcome.error().ter, tecFAILED_PROCESSING);
|
||||
EXPECT_TRUE(outcome.error().cost.has_value());
|
||||
}
|
||||
|
||||
// A start section is guest code, so a trap in one is the contract's fault wherever it
|
||||
// happens - charged for what it burned, rather than reported as a module the node should
|
||||
// have screened.
|
||||
TEST_F(WasmVMTest, TrappingStartSectionIsChargedToTheContract)
|
||||
{
|
||||
constexpr std::string_view wat = R"wat(
|
||||
(module
|
||||
(memory (export "memory") 1)
|
||||
(func $init (unreachable))
|
||||
(start $init)
|
||||
(func (export "escrow_finish") (result i32) (i32.const 0)))
|
||||
)wat";
|
||||
|
||||
auto const outcome = run(wat);
|
||||
|
||||
ASSERT_FALSE(outcome.has_value());
|
||||
EXPECT_EQ(outcome.error().ter, tecFAILED_PROCESSING);
|
||||
ASSERT_TRUE(outcome.error().cost.has_value());
|
||||
EXPECT_GT(*outcome.error().cost, 0) << "the start section's instructions are metered";
|
||||
}
|
||||
|
||||
// Preflight is meant to refuse these with `temBAD_WASM`; reaching apply means the screening
|
||||
// did not happen, which is the node's fault and not the transaction's.
|
||||
TEST_F(WasmVMTest, UnrunnableModuleIsNodeSideFault)
|
||||
|
||||
Reference in New Issue
Block a user