From 95526371b0d5c5c95eead6fd0fff8419e3c6e56b Mon Sep 17 00:00:00 2001 From: Sergey Kuznetsov Date: Tue, 11 Aug 2026 15:22:02 +0100 Subject: [PATCH] Update error codes --- crates/xrpl-host-functions/src/lib.rs | 7 +- crates/xrpl-host-functions/src/macros.rs | 19 +- .../xrpl-host-functions/tests/host_errors.rs | 36 ++- crates/xrpl-wasm-vm-ffi/src/lib.rs | 11 +- crates/xrpl-wasm-vm/src/abi.rs | 218 ++++++++++++------ crates/xrpl-wasm-vm/src/register.rs | 5 +- crates/xrpl-wasm-vm/src/vm.rs | 76 ++---- crates/xrpl-wasm-vm/tests/host_calls.rs | 2 +- src/libxrpl/tx/wasm/HostContext.cpp | 11 +- src/libxrpl/tx/wasm/HostFuncImplGetter.cpp | 2 +- src/tests/libxrpl/tx/wasm/WasmVM.cpp | 14 +- 11 files changed, 219 insertions(+), 182 deletions(-) diff --git a/crates/xrpl-host-functions/src/lib.rs b/crates/xrpl-host-functions/src/lib.rs index 06db3c6c59..eef04de229 100644 --- a/crates/xrpl-host-functions/src/lib.rs +++ b/crates/xrpl-host-functions/src/lib.rs @@ -21,7 +21,7 @@ mod macros; use xrpl_host_functions_macros::host_functions; host_errors! { - Internal = -1, + Unimplemented = -1, FieldNotFound = -2, BufferTooSmall = -3, NoArray = -4, @@ -31,7 +31,7 @@ host_errors! { SlotsFull = -8, EmptySlot = -9, LedgerObjNotFound = -10, - Decoding = -11, + OutOfTransferLimit = -11, DataFieldTooLarge = -12, PointerOutOfBounds = -13, NoMemExported = -14, @@ -41,9 +41,6 @@ host_errors! { IndexOutOfBounds = -18, FloatInputMalformed = -19, FloatComputationError = -20, - NoRuntime = -21, - OutOfGas = -22, - OutOfTransferLimit = -23, } /// Convenience alias for the trait's fallible returns. diff --git a/crates/xrpl-host-functions/src/macros.rs b/crates/xrpl-host-functions/src/macros.rs index d720cd2ddc..a08e044e8e 100644 --- a/crates/xrpl-host-functions/src/macros.rs +++ b/crates/xrpl-host-functions/src/macros.rs @@ -16,18 +16,13 @@ /// construction. `HostFunctionSpec::ALL` is complete the same way, from the /// `host_functions!` block. macro_rules! host_errors { - ($($variant:ident = $code:literal,)+) => { + ($($(#[$doc:meta])* $variant:ident = $code:literal,)+) => { /// Error codes a host function may return. /// - /// The discriminants mirror `HostFunctionError` in - /// `include/xrpl/tx/wasm/WasmCommon.h`, so a negative `i32` crossing the wasm - /// boundary means the same thing to the guest, the Rust host, and the existing - /// C++ code. The full set is kept (not just the ones the PoC uses today) to - /// preserve that shared meaning. #[derive(Debug, Clone, Copy, PartialEq, Eq)] #[repr(i32)] pub enum HostError { - $($variant = $code,)+ + $($(#[$doc])* $variant = $code,)+ } impl HostError { @@ -45,12 +40,16 @@ macro_rules! host_errors { self as i32 } - /// Reconstruct a `HostError` from its wire code; unknown/positive values - /// map to `Internal`. + /// Reconstruct a `HostError` from its wire code. + /// + /// A code this ABI does not define is `Unimplemented`: an answer the + /// caller cannot act on is the call not having been served. Positive + /// values are not errors at all and go the same way, since this is + /// reached only once a negative return has been read as a failure. pub const fn from_code(code: i32) -> HostError { match code { $($code => HostError::$variant,)+ - _ => HostError::Internal, + _ => HostError::Unimplemented, } } } diff --git a/crates/xrpl-host-functions/tests/host_errors.rs b/crates/xrpl-host-functions/tests/host_errors.rs index 7815f5c1a6..2a242ed93c 100644 --- a/crates/xrpl-host-functions/tests/host_errors.rs +++ b/crates/xrpl-host-functions/tests/host_errors.rs @@ -19,7 +19,7 @@ fn the_error_table_matches_the_declarations() { assert_eq!( table, [ - (HostError::Internal, -1), + (HostError::Unimplemented, -1), (HostError::FieldNotFound, -2), (HostError::BufferTooSmall, -3), (HostError::NoArray, -4), @@ -29,7 +29,7 @@ fn the_error_table_matches_the_declarations() { (HostError::SlotsFull, -8), (HostError::EmptySlot, -9), (HostError::LedgerObjNotFound, -10), - (HostError::Decoding, -11), + (HostError::OutOfTransferLimit, -11), (HostError::DataFieldTooLarge, -12), (HostError::PointerOutOfBounds, -13), (HostError::NoMemExported, -14), @@ -39,13 +39,25 @@ fn the_error_table_matches_the_declarations() { (HostError::IndexOutOfBounds, -18), (HostError::FloatInputMalformed, -19), (HostError::FloatComputationError, -20), - (HostError::NoRuntime, -21), - (HostError::OutOfGas, -22), - (HostError::OutOfTransferLimit, -23), ] ); } +/// The set is `-1 ..= -20` and nothing else: this enum is xrpld's `HostFunctionError` +/// and every entry is a code some contract may read, so a condition with no number to +/// answer with is not one of these — it is a `Fault` in the engine. +#[test] +fn every_code_is_in_the_shared_range() { + let outside: Vec = HostError::ALL + .iter() + .copied() + .filter(|error| !(-20..=-1).contains(&error.code())) + .collect(); + + assert!(outside.is_empty(), "outside -1..=-20: {outside:?}"); + assert_eq!(HostError::ALL.len(), 20); +} + /// Every code a guest can be handed comes back as the error that produced it, so a /// caller reading a negative return value recovers the condition and not a /// neighbouring one. The table above pins the numbers; this adds only the round @@ -57,14 +69,18 @@ fn every_wire_code_round_trips_back_to_its_error() { } } -/// A code from outside the set is `Internal`: a host that answers something this -/// ABI does not define has failed in a way the caller cannot act on, and success is -/// not an error at all. +/// A code from outside the set is `Unimplemented`: a host answering something this +/// ABI does not define has not served the call, whatever it meant by it, and success +/// is not an error at all. #[test] -fn a_code_outside_the_set_is_internal() { +fn a_code_outside_the_set_is_unimplemented() { let unassigned = -(HostError::ALL.len() as i32) - 1; for code in [unassigned, i32::MIN, 0, 1, i32::MAX] { - assert_eq!(HostError::from_code(code), HostError::Internal, "{code}"); + assert_eq!( + HostError::from_code(code), + HostError::Unimplemented, + "{code}" + ); } } diff --git a/crates/xrpl-wasm-vm-ffi/src/lib.rs b/crates/xrpl-wasm-vm-ffi/src/lib.rs index 5c9b4e9a8c..419009850f 100644 --- a/crates/xrpl-wasm-vm-ffi/src/lib.rs +++ b/crates/xrpl-wasm-vm-ffi/src/lib.rs @@ -561,12 +561,13 @@ mod tests { assert_eq!(bytes_written(-14), Err(HostError::NoMemExported)); } - /// An exception caught on the C++ side arrives as `-1`, which has to reach the - /// engine as a *fatal* error so the run stops and the transaction is - /// `tecINTERNAL` — not as a code handed to the contract to interpret. + /// An exception caught on the C++ side arrives as `-1`, the same code + /// `HostFunctionError` spells `Unimplemented`. The engine stops the run on it and + /// the transaction is `tecINTERNAL`, rather than the contract being handed a code + /// to interpret. #[test] - fn a_caught_cxx_exception_arrives_as_internal() { - assert_eq!(bytes_written(-1), Err(HostError::Internal)); + fn a_caught_cxx_exception_arrives_as_unimplemented() { + assert_eq!(bytes_written(-1), Err(HostError::Unimplemented)); } // ----------------------------------------------------------------------- diff --git a/crates/xrpl-wasm-vm/src/abi.rs b/crates/xrpl-wasm-vm/src/abi.rs index 74a46727ec..3f30425076 100644 --- a/crates/xrpl-wasm-vm/src/abi.rs +++ b/crates/xrpl-wasm-vm/src/abi.rs @@ -3,8 +3,63 @@ use crate::vm::{MAX_FIELD_BYTES, VmState}; use wasmi::{Caller, Memory}; use xrpl_host_functions::{HostError, HostFunctionSpec, HostFunctions, HostResult}; +/// A condition that stops the run. It is a property of the run rather than an answer +/// to a call, so it reaches no guest and carries no wire code — which is why it is +/// not a [`HostError`]: no host can report one and no contract can read one. +/// +/// The three are the outcomes a host call can end a run with, and +/// `From for RunError` in `vm.rs` is where each gets its name. #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(crate) struct FatalHostError(pub(crate) HostError); +pub(crate) enum Fault { + /// This call's charge would take the meter below zero. The guest exhausting the + /// meter with its own instructions reaches [`crate::vm::RunError::OutOfGas`] by + /// wasmi's `OutOfFuel` trap instead, never through here. + OutOfGas, + /// The call could not be served: either the host said so, or this engine's own + /// fuel meter did not answer. + Internal, + /// There is no linear memory to work in — the module exports none, or the call + /// came from a start section, which runs before there is an instance. + NoMemory, +} + +/// How a host call fails: with a code the guest reads off the return value, or with a +/// [`Fault`] that stops the run. +/// +/// **The variant picks the channel.** [`to_wire`] reads it rather than asking a +/// predicate, so the two cannot disagree, and a [`FatalHostError`] cannot be built +/// around something a guest was supposed to see. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum CallError { + Code(HostError), + Fatal(Fault), +} + +/// A host call's result inside the engine: [`HostResult`] plus the faults only the +/// engine can raise. +pub(crate) type CallResult = Result; + +/// Which channel a host's answer takes, decided once, here. +/// +/// Two of the twenty codes stop the run instead of reaching the contract that asked. +/// Both say the call was not served at all — the host could not do it, or there is +/// nowhere to put the answer — and a contract has no business interpreting either, so +/// it is told nothing and the run ends. Every other code is the contract's to read. +impl From for CallError { + fn from(error: HostError) -> CallError { + match error { + HostError::Unimplemented => CallError::Fatal(Fault::Internal), + HostError::NoMemExported => CallError::Fatal(Fault::NoMemory), + code => CallError::Code(code), + } + } +} + +/// The payload a trap carries so [`crate::vm::run`] can name the outcome without +/// parsing a message. Holds a [`Fault`], so by construction no guest-visible code can +/// leave through this channel. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct FatalHostError(pub(crate) Fault); impl wasmi::errors::HostError for FatalHostError {} @@ -14,20 +69,12 @@ impl core::fmt::Display for FatalHostError { } } -/// Whether a [`HostError`] stops the run instead of reaching the guest as a code. -pub(crate) fn is_fatal(error: HostError) -> bool { - matches!( - error, - HostError::OutOfGas | HostError::Internal | HostError::NoMemExported - ) -} - /// Charge the call's gas, run its body, put the result on the wire. The one path /// every registered closure takes, so gas cannot be forgotten. pub(crate) fn charged( caller: &mut Caller<'_, VmState<'_>>, op: HostFunctionSpec, - body: impl FnOnce(&mut Caller<'_, VmState<'_>>) -> HostResult, + body: impl FnOnce(&mut Caller<'_, VmState<'_>>) -> CallResult, ) -> Result { to_wire(charge(caller, op.gas()).and_then(|()| body(caller))) } @@ -40,37 +87,44 @@ pub(crate) fn charged( pub(crate) fn charged_unreported( caller: &mut Caller<'_, VmState<'_>>, op: HostFunctionSpec, - body: impl FnOnce(&mut Caller<'_, VmState<'_>>) -> HostResult<()>, + body: impl FnOnce(&mut Caller<'_, VmState<'_>>) -> CallResult<()>, ) -> Result<(), wasmi::Error> { dropped(charge(caller, op.gas()).and_then(|()| body(caller))) } -/// [`to_wire`] for a call with no result: there is no return value to encode a soft -/// error in, so it is dropped. The host-fatal ones still stop the run — those are a -/// property of the run, not an answer to the call. -fn dropped(result: HostResult<()>) -> Result<(), wasmi::Error> { +/// [`to_wire`] for a call with no result: there is no return value to encode a code +/// in, so it is dropped. A [`Fault`] still stops the run — that is a property of the +/// run, not an answer to the call. +fn dropped(result: CallResult<()>) -> Result<(), wasmi::Error> { match result { - Err(error) if is_fatal(error) => Err(wasmi::Error::host(FatalHostError(error))), + Err(CallError::Fatal(fault)) => Err(wasmi::Error::host(FatalHostError(fault))), _ => Ok(()), } } -fn to_wire(result: HostResult) -> Result { +fn to_wire(result: CallResult) -> Result { match result { Ok(value) => Ok(value), - Err(error) if is_fatal(error) => Err(wasmi::Error::host(FatalHostError(error))), - Err(error) => Ok(error.code()), + Err(CallError::Code(error)) => Ok(error.code()), + Err(CallError::Fatal(fault)) => Err(wasmi::Error::host(FatalHostError(fault))), } } -/// Deduct `cost` fuel; `OutOfGas` if it would go negative. -fn charge(caller: &mut Caller<'_, T>, cost: u64) -> Result<(), HostError> { - let remaining = caller.get_fuel().map_err(|_| HostError::Internal)?; +/// Deduct `cost` fuel; [`Fault::OutOfGas`] if it would go negative. +/// +/// A meter that will not answer is this crate's own defect, not the contract's, so it +/// is [`Fault::Internal`] rather than a number a guest could act on. +fn charge(caller: &mut Caller<'_, T>, cost: u64) -> CallResult<()> { + let remaining = caller + .get_fuel() + .map_err(|_| CallError::Fatal(Fault::Internal))?; match remaining.checked_sub(cost) { - Some(left) => caller.set_fuel(left).map_err(|_| HostError::Internal), + Some(left) => caller + .set_fuel(left) + .map_err(|_| CallError::Fatal(Fault::Internal)), None => { let _ = caller.set_fuel(0); - Err(HostError::OutOfGas) + Err(CallError::Fatal(Fault::OutOfGas)) } } } @@ -87,8 +141,11 @@ fn charge_transfer(state: &VmState<'_>, n: usize) -> Result<(), HostError> { } } -fn memory(caller: &Caller<'_, VmState<'_>>) -> Result { - caller.data().memory.ok_or(HostError::NoMemExported) +fn memory(caller: &Caller<'_, VmState<'_>>) -> CallResult { + caller + .data() + .memory + .ok_or(CallError::Fatal(Fault::NoMemory)) } /// [`Region::read`] of the guest's memory, for a call that reads and writes nothing @@ -96,9 +153,9 @@ fn memory(caller: &Caller<'_, VmState<'_>>) -> Result { pub(crate) fn read_borrowed<'a>( caller: &'a Caller<'_, VmState<'_>>, input: Region, -) -> HostResult<&'a [u8]> { +) -> CallResult<&'a [u8]> { let mem = memory(caller)?; - input.read(mem.data(caller)) + Ok(input.read(mem.data(caller))?) } /// Service a call whose answer is bytes, written straight into the guest's output @@ -112,7 +169,7 @@ pub(crate) fn write_into( caller: &mut Caller<'_, VmState<'_>>, out: Region, fill: impl FnOnce(&dyn HostFunctions, &mut [u8]) -> HostResult, -) -> HostResult { +) -> CallResult { let range = out.range()?; let cap = range.len(); let mem = memory(caller)?; @@ -130,10 +187,10 @@ pub(crate) fn write_into( let n = fill(host, buf)?; if n > MAX_FIELD_BYTES { - return Err(HostError::DataFieldTooLarge); + return Err(HostError::DataFieldTooLarge.into()); } if n > cap { - return Err(HostError::BufferTooSmall); + return Err(HostError::BufferTooSmall.into()); } charge_transfer(caller.data(), n)?; #[expect( @@ -165,7 +222,7 @@ pub(crate) fn write_buffered( caller: &mut Caller<'_, VmState<'_>>, out: Region, call: impl FnOnce(&dyn HostFunctions, &[u8], &mut [u8]) -> HostResult, -) -> HostResult { +) -> CallResult { let mem = memory(caller)?; // One borrow split in two: the guest's bytes for the inputs, the store data for // the output buffer. Taking them together is what keeps the inputs borrowed @@ -180,11 +237,11 @@ pub(crate) fn write_buffered( let range = out.range()?; let cap = range.len(); if n > MAX_FIELD_BYTES { - return Err(HostError::DataFieldTooLarge); + return Err(HostError::DataFieldTooLarge.into()); } let buf = data.get_mut(range).ok_or(HostError::PointerOutOfBounds)?; if n > cap { - return Err(HostError::BufferTooSmall); + return Err(HostError::BufferTooSmall.into()); } charge_transfer(state, n)?; buf[..n].copy_from_slice(&state.out_buffer[..n]); @@ -235,7 +292,7 @@ mod tests { /// `wasmi::Error` is not `PartialEq`, so a test expecting the guest-visible /// channel says so by going through here. - fn wire(result: HostResult) -> i32 { + fn wire(result: CallResult) -> i32 { to_wire(result) .unwrap_or_else(|trap| panic!("expected a guest-visible status, got a trap: {trap}")) } @@ -244,72 +301,85 @@ mod tests { fn a_success_becomes_the_value_and_an_error_becomes_its_code() { assert_eq!(wire(Ok(0)), 0); assert_eq!(wire(Ok(32)), 32); - assert_eq!(wire(Err(HostError::BufferTooSmall)), -3); + assert_eq!(wire(Err(HostError::BufferTooSmall.into())), -3); } - /// The fatal set as the tests *expect* it, not as [`is_fatal`] reports it: - /// deriving it from `is_fatal` would make both tests below vacuous, since a - /// condition wrongly classified as soft would simply be skipped. - const MUST_TRAP: [HostError; 3] = [ - HostError::OutOfGas, - HostError::Internal, - HostError::NoMemExported, + /// The codes a host may answer that a contract must not see, and the fault each + /// becomes. Written out rather than derived from `From`, which is what + /// they are asserting. + const STOPS_THE_RUN: [(HostError, Fault); 2] = [ + (HostError::Unimplemented, Fault::Internal), + (HostError::NoMemExported, Fault::NoMemory), ]; - /// The trap carries the condition, so `run` can name the outcome without - /// parsing a message. + /// Every fault, so the two tests below are the whole set and not a sample. + /// `From for RunError` is what forces a fault added later to be + /// considered; this is what forces it to be tested. + const ALL_FAULTS: [Fault; 3] = [Fault::OutOfGas, Fault::Internal, Fault::NoMemory]; + #[test] - fn a_host_fatal_error_becomes_a_trap_carrying_it() { - for error in MUST_TRAP { - let trap = - to_wire(Err(error)).expect_err("a fatal error must not reach the guest as a code"); - let payload = trap.downcast_ref::().unwrap_or_else(|| { - panic!("{error:?}: expected a FatalHostError payload, got: {trap}") - }); - assert_eq!(*payload, FatalHostError(error)); + fn a_code_that_stops_the_run_converts_to_its_fault() { + for (error, fault) in STOPS_THE_RUN { + assert_eq!(CallError::from(error), CallError::Fatal(fault), "{error:?}"); } } /// Over `HostError::ALL`, so it is the whole ABI and not a sample: a code added - /// to the ABI arrives already asserted to be guest-visible, and making it fatal - /// is then a change someone has to come and make. + /// to the ABI arrives already asserted to reach the guest as itself, and stopping + /// the run on it is then a change someone has to come and make. /// /// `OutOfTransferLimit` is the row worth reading twice: the one budget a /// contract can be expected to handle, so it is told no rather than killed. #[test] - fn only_the_host_fatal_errors_trap() { + fn every_other_code_reaches_the_guest_as_itself() { for &error in HostError::ALL { - if MUST_TRAP.contains(&error) { - assert!(is_fatal(error), "{error:?} must stop the run"); - } else { - assert!(!is_fatal(error), "{error:?} must reach the guest as a code"); - assert_eq!(wire(Err(error)), error.code()); + if STOPS_THE_RUN.iter().any(|&(stops, _)| stops == error) { + continue; } + assert_eq!(CallError::from(error), CallError::Code(error), "{error:?}"); + assert_eq!(wire(Err(error.into())), error.code(), "{error:?}"); } } - /// The result-less path splits the same set differently: the fatal errors still - /// stop the run, and every other one is dropped, since `trace` has no return value - /// to carry it. Over `HostError::ALL` for the reason above — a code added to the - /// ABI arrives asserted against both paths. + /// The trap carries the fault, so `run` can name the outcome without parsing a + /// message. #[test] - fn a_call_with_no_result_drops_a_soft_error_and_traps_on_a_fatal_one() { + fn a_fault_becomes_a_trap_carrying_it() { + for fault in ALL_FAULTS { + let trap = to_wire(Err(CallError::Fatal(fault))) + .expect_err("a fault must not reach the guest as a code"); + let payload = trap.downcast_ref::().unwrap_or_else(|| { + panic!("{fault:?}: expected a FatalHostError payload, got: {trap}") + }); + assert_eq!(*payload, FatalHostError(fault)); + } + } + + /// The result-less path splits the same two channels differently: a fault still + /// stops the run, and every code is dropped, since `trace` has no return value to + /// carry it. Over `HostError::ALL` for the reason above — a code added to the ABI + /// arrives asserted against both paths. + #[test] + fn a_call_with_no_result_drops_a_code_and_traps_on_a_fault() { assert!(dropped(Ok(())).is_ok()); for &error in HostError::ALL { - if MUST_TRAP.contains(&error) { - let trap = dropped(Err(error)).expect_err("a fatal error must stop the run"); - let payload = trap.downcast_ref::().unwrap_or_else(|| { - panic!("{error:?}: expected a FatalHostError payload, got: {trap}") - }); - assert_eq!(*payload, FatalHostError(error)); - } else { + if let CallError::Code(code) = CallError::from(error) { assert!( - dropped(Err(error)).is_ok(), + dropped(Err(CallError::Code(code))).is_ok(), "{error:?} has no channel to the guest and must be dropped" ); } } + + for fault in ALL_FAULTS { + let trap = + dropped(Err(CallError::Fatal(fault))).expect_err("a fault must stop the run"); + let payload = trap.downcast_ref::().unwrap_or_else(|| { + panic!("{fault:?}: expected a FatalHostError payload, got: {trap}") + }); + assert_eq!(*payload, FatalHostError(fault)); + } } #[test] diff --git a/crates/xrpl-wasm-vm/src/register.rs b/crates/xrpl-wasm-vm/src/register.rs index a3fac3824d..99bec20a20 100644 --- a/crates/xrpl-wasm-vm/src/register.rs +++ b/crates/xrpl-wasm-vm/src/register.rs @@ -96,11 +96,12 @@ pub(crate) fn register_host_functions( charged_unreported(&mut caller, HostFunctionSpec::Trace, |c| { let host = c.data().host; let msg = read_borrowed(c, Region::new(msg_ptr, msg_len))?; - let msg = core::str::from_utf8(msg).map_err(|_| HostError::Decoding)?; + let msg = + core::str::from_utf8(msg).map_err(|_| HostError::InvalidParams)?; let data_type = TraceDataType::from_code(data_type).ok_or(HostError::InvalidParams)?; let data = read_borrowed(c, Region::new(data_ptr, data_len))?; - host.trace(msg, data, data_type) + Ok(host.trace(msg, data, data_type)?) }) }, ), diff --git a/crates/xrpl-wasm-vm/src/vm.rs b/crates/xrpl-wasm-vm/src/vm.rs index 28643a819e..0fe2795d2e 100644 --- a/crates/xrpl-wasm-vm/src/vm.rs +++ b/crates/xrpl-wasm-vm/src/vm.rs @@ -5,9 +5,9 @@ use wasmi::{ Config, Engine, Export, Linker, Memory, Module, Store, StoreLimits, StoreLimitsBuilder, TrapCode, }; -use xrpl_host_functions::{HostError, HostFunctions}; +use xrpl_host_functions::HostFunctions; -use crate::abi::FatalHostError; +use crate::abi::{FatalHostError, Fault}; use crate::preflight::entry_point_fault; use crate::register::register_host_functions; @@ -184,7 +184,7 @@ fn failed(store: &Store>, gas: u64, error: RunError) -> RunFailure { /// this before naming a failure after itself. fn guest_halted(error: &wasmi::Error) -> Option { if let Some(fatal) = error.downcast_ref::() { - return Some(host_fatal(fatal.0)); + return Some(fatal.0.into()); } (error.as_trap_code() == Some(TrapCode::OutOfFuel)).then_some(RunError::OutOfGas) } @@ -205,41 +205,19 @@ fn instantiation_failure(error: &wasmi::Error) -> RunError { } } -/// The outcome a host-fatal `HostError` is. +/// The outcome a [`Fault`] is: the one place a stopped call becomes a stopped run. /// -/// Exhaustive rather than closed with a wildcard, so a variant added to the ABI -/// must be placed here before this compiles. That is one direction of the agreement -/// with [`crate::abi::is_fatal`], which picks the channel; the other — an existing -/// variant moved into `is_fatal`'s set, landing in the soft arm and reported as -/// `Internal` — is `tests::every_fatal_error_has_an_outcome_of_its_own`. -/// -/// The soft arm is otherwise unreachable: a guest-visible error is a return code -/// and never becomes a trap for [`guest_halted`] to unwrap. -fn host_fatal(error: HostError) -> RunError { - match error { - HostError::OutOfGas => RunError::OutOfGas, - HostError::Internal => RunError::Internal, - HostError::NoMemExported => RunError::NoMemory, - HostError::FieldNotFound - | HostError::BufferTooSmall - | HostError::NoArray - | HostError::NotLeafField - | HostError::LocatorMalformed - | HostError::SlotOutRange - | HostError::SlotsFull - | HostError::EmptySlot - | HostError::LedgerObjNotFound - | HostError::Decoding - | HostError::DataFieldTooLarge - | HostError::PointerOutOfBounds - | HostError::InvalidParams - | HostError::InvalidAccount - | HostError::InvalidField - | HostError::IndexOutOfBounds - | HostError::FloatInputMalformed - | HostError::FloatComputationError - | HostError::NoRuntime - | HostError::OutOfTransferLimit => RunError::Internal, +/// Total and one arm each, because a `Fault` is only ever a condition that stops the +/// run — the guest-visible codes cannot reach here, which is what +/// [`crate::abi::CallError`] buys. A fault added later has no arm and does not +/// compile. +impl From for RunError { + fn from(fault: Fault) -> RunError { + match fault { + Fault::OutOfGas => RunError::OutOfGas, + Fault::Internal => RunError::Internal, + Fault::NoMemory => RunError::NoMemory, + } } } @@ -357,36 +335,12 @@ pub fn run<'h>( #[cfg(test)] mod tests { use super::*; - use crate::abi::is_fatal; #[test] fn the_engine_is_one_engine() { assert!(Engine::same(wasm_engine(), wasm_engine())); } - #[test] - fn every_fatal_error_has_an_outcome_of_its_own() { - for &error in HostError::ALL { - let named = !matches!(host_fatal(error), RunError::Internal) - || matches!(error, HostError::Internal); - assert_eq!( - is_fatal(error), - named, - "{error:?}: abi::is_fatal {}, host_fatal {}", - if is_fatal(error) { - "traps it" - } else { - "passes it to the guest" - }, - if named { - "names its outcome" - } else { - "groups it with the soft errors" - } - ); - } - } - /// The only place these numbers appear as literals; every other test derives /// them from the constants. #[test] diff --git a/crates/xrpl-wasm-vm/tests/host_calls.rs b/crates/xrpl-wasm-vm/tests/host_calls.rs index b194ba7dad..06fd0279f7 100644 --- a/crates/xrpl-wasm-vm/tests/host_calls.rs +++ b/crates/xrpl-wasm-vm/tests/host_calls.rs @@ -238,7 +238,7 @@ fn a_soft_error_from_a_call_with_no_result_is_dropped() { /// changes nothing: the run stops. #[test] fn a_fatal_error_from_a_call_with_no_result_still_stops_the_run() { - let host = FakeHost::new().failing_trace(HostError::Internal); + let host = FakeHost::new().failing_trace(HostError::Unimplemented); let wat = module( &[import::TRACE, ONE_PAGE], diff --git a/src/libxrpl/tx/wasm/HostContext.cpp b/src/libxrpl/tx/wasm/HostContext.cpp index 1c35789bdf..1a0215af29 100644 --- a/src/libxrpl/tx/wasm/HostContext.cpp +++ b/src/libxrpl/tx/wasm/HostContext.cpp @@ -29,13 +29,12 @@ namespace xrpl { namespace { // What a host call answers when it could not be served at all: every method below hands it -// to `guarded` as the answer for a body that throws. The engine reads -1 as its fatal -// `Internal`, stops the run and reports `tecINTERNAL`. +// to `guarded` as the answer for a body that throws. The engine converts -1 into a fault, +// stops the run and reports `tecINTERNAL`, rather than handing the code to the contract. // -// `HostFunctionError` spells -1 `Unimplemented`, so the two share a code. They also share -// a meaning worth keeping together - "the host could not serve this call, and the contract -// has no business interpreting why" - and they must share a fate. Named here so a call -// site reads as what it is rather than as "unimplemented". +// Named here because `Unimplemented` is not what a thrown exception is. What -1 carries is +// the meaning the two conditions share - "the host could not serve this call, and the +// contract has no business interpreting why" - and it is the fate they share too. constexpr std::int32_t kHostInternal = hfErrorToInt(HostFunctionError::Unimplemented); // Copy `value` into `out` only if the whole of it fits, and answer its true length either diff --git a/src/libxrpl/tx/wasm/HostFuncImplGetter.cpp b/src/libxrpl/tx/wasm/HostFuncImplGetter.cpp index e622e62b0b..3d13997f17 100644 --- a/src/libxrpl/tx/wasm/HostFuncImplGetter.cpp +++ b/src/libxrpl/tx/wasm/HostFuncImplGetter.cpp @@ -124,7 +124,7 @@ getAnyFieldData(FieldValue const& variantObj) return Bytes((*u)->begin(), (*u)->end()); // Unreachable: the variant only holds the two alternatives above. If not, it is an - // xrpld bug, and `guarded` turns the throw into the engine's fatal `Internal` -> + // xrpld bug, and `guarded` turns the throw into -1, which stops the run -> // tecINTERNAL. Throw("field value variant holds neither alternative"); // LCOV_EXCL_LINE } diff --git a/src/tests/libxrpl/tx/wasm/WasmVM.cpp b/src/tests/libxrpl/tx/wasm/WasmVM.cpp index 76c774e3eb..8dfdfb2c93 100644 --- a/src/tests/libxrpl/tx/wasm/WasmVM.cpp +++ b/src/tests/libxrpl/tx/wasm/WasmVM.cpp @@ -234,14 +234,14 @@ TEST_F(WasmVMTest, DirtyHostIsRefusedBeforeContractRuns) // A soft host error is the contract's to interpret, so its code has to cross the boundary // unchanged: the engine must not renumber it, clamp it, or turn it into a failure of its own. // -// Over the whole of `HostFunctionError` rather than a sample, because the C++ and Rust error -// enums are two hand-maintained lists of the same wire numbers and they have already drifted -// once — C++ spells -11 `OutOfTransferLimit` where the Rust ABI spells it `Decoding`. This is -// the test that notices if either side renumbers. +// Over the whole of `HostFunctionError` rather than a sample, because `HostFunctionError` and +// the Rust ABI's `HostError` are two hand-maintained lists of the same wire numbers: -1 +// through -20 have to mean the same thing on both sides, and this is the test that notices if +// either side renumbers. // -// The two exclusions are the codes the Rust engine treats as host-fatal, which stop the run -// instead of reaching the guest: -1 (its `Internal`, which C++ spells `Unimplemented`) and -// -14 `NoMemExported`. +// The two exclusions are the codes the Rust engine converts into a fault, which stops the run +// instead of reaching the guest: -1 `Unimplemented` and -14 `NoMemExported`. Both say the call +// was not served at all. TEST_F(WasmVMTest, SoftHostErrorCodesCrossUnchanged) { static constexpr HostFunctionError kSoftErrors[] = {