From ecc2f07ea34e70528ec3f45ad95c2e0578f8c5ac Mon Sep 17 00:00:00 2001 From: Sergey Kuznetsov Date: Tue, 11 Aug 2026 17:36:52 +0100 Subject: [PATCH] Add internal fatal error code --- crates/xrpl-host-functions/src/lib.rs | 3 ++ crates/xrpl-host-functions/src/macros.rs | 14 +++--- .../xrpl-host-functions/tests/host_errors.rs | 44 +++++++++++++------ crates/xrpl-wasm-vm-ffi/src/lib.rs | 23 +++++++--- crates/xrpl-wasm-vm/src/abi.rs | 13 +++--- crates/xrpl-wasm-vm/tests/host_calls.rs | 2 +- include/xrpl/tx/wasm/WasmCommon.h | 9 ++++ src/libxrpl/tx/wasm/HostContext.cpp | 9 +--- src/libxrpl/tx/wasm/HostFuncImplGetter.cpp | 2 +- src/tests/libxrpl/tx/wasm/WasmVM.cpp | 5 ++- 10 files changed, 83 insertions(+), 41 deletions(-) diff --git a/crates/xrpl-host-functions/src/lib.rs b/crates/xrpl-host-functions/src/lib.rs index eef04de229..f3c5eb34a1 100644 --- a/crates/xrpl-host-functions/src/lib.rs +++ b/crates/xrpl-host-functions/src/lib.rs @@ -41,6 +41,9 @@ host_errors! { IndexOutOfBounds = -18, FloatInputMalformed = -19, FloatComputationError = -20, + /// Internal fatal error. + /// User code will never see this error but keep it reserved to not rely on the value. + InternalFatal = -2147483648, } /// 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 a08e044e8e..b077526784 100644 --- a/crates/xrpl-host-functions/src/macros.rs +++ b/crates/xrpl-host-functions/src/macros.rs @@ -34,7 +34,8 @@ macro_rules! host_errors { /// split iterates this and a code added to the ABI cannot slip past it. pub const ALL: &'static [HostError] = &[$(HostError::$variant,)+]; - /// The negative wire value the guest sees as the function's return code. + /// The negative wire value a failed call returns. Every code but + /// `InternalFatal` is one a guest reads off that value. #[inline] pub const fn code(self) -> i32 { self as i32 @@ -42,14 +43,15 @@ macro_rules! host_errors { /// 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. + /// A code this ABI does not define is `InternalFatal`: an answer the + /// caller cannot act on is the call not having been served, and that is + /// the variant which says so. 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::Unimplemented, + _ => HostError::InternalFatal, } } } diff --git a/crates/xrpl-host-functions/tests/host_errors.rs b/crates/xrpl-host-functions/tests/host_errors.rs index 2a242ed93c..7e77fcdc56 100644 --- a/crates/xrpl-host-functions/tests/host_errors.rs +++ b/crates/xrpl-host-functions/tests/host_errors.rs @@ -39,23 +39,36 @@ fn the_error_table_matches_the_declarations() { (HostError::IndexOutOfBounds, -18), (HostError::FloatInputMalformed, -19), (HostError::FloatComputationError, -20), + (HostError::InternalFatal, i32::MIN), ] ); } -/// 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. +/// The guest-facing set is `-1 ..= -20` and nothing else: those entries are xrpld's +/// `HostFunctionError`, and each is a code some contract may read. +/// +/// `InternalFatal` is the one deliberate exception, exempted by name rather than by +/// widening the range: a condition with no number a contract can act on needs no number +/// in the range a contract reads, and holding it at `i32::MIN` is what keeps it from +/// ever colliding with a code appended to xrpld's list. #[test] -fn every_code_is_in_the_shared_range() { - let outside: Vec = HostError::ALL +fn every_code_but_the_sentinel_is_in_the_shared_range() { + let shared: Vec = HostError::ALL + .iter() + .copied() + .filter(|&error| error != HostError::InternalFatal) + .collect(); + + let outside: Vec = shared .iter() .copied() .filter(|error| !(-20..=-1).contains(&error.code())) .collect(); assert!(outside.is_empty(), "outside -1..=-20: {outside:?}"); - assert_eq!(HostError::ALL.len(), 20); + assert_eq!(shared.len(), 20); + assert_eq!(HostError::InternalFatal.code(), i32::MIN); + assert_eq!(HostError::ALL.len(), 21); } /// Every code a guest can be handed comes back as the error that produced it, so a @@ -69,17 +82,20 @@ fn every_wire_code_round_trips_back_to_its_error() { } } -/// 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. +/// A code from outside the set is `InternalFatal`: 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. +/// +/// `-21` is the code xrpld would append next, so it is the one that decides whether a +/// list this crate has not caught up with reaches a guest or stops the run. `i32::MIN + +/// 1` is next to the sentinel and unassigned, which is what makes the sentinel a value +/// rather than a range. #[test] -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] { +fn a_code_outside_the_set_is_internal_fatal() { + for code in [-21, i32::MIN + 1, 0, 1, i32::MAX] { assert_eq!( HostError::from_code(code), - HostError::Unimplemented, + HostError::InternalFatal, "{code}" ); } diff --git a/crates/xrpl-wasm-vm-ffi/src/lib.rs b/crates/xrpl-wasm-vm-ffi/src/lib.rs index 419009850f..1a358c036e 100644 --- a/crates/xrpl-wasm-vm-ffi/src/lib.rs +++ b/crates/xrpl-wasm-vm-ffi/src/lib.rs @@ -561,13 +561,24 @@ mod tests { assert_eq!(bytes_written(-14), Err(HostError::NoMemExported)); } - /// 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. + /// An exception caught on the C++ side arrives as `InternalFatal`, the code + /// `HostContext` answers with when a body throws. The engine stops the run on it and + /// the transaction is `tecINTERNAL`, rather than the contract being handed a code to + /// interpret. + /// + /// It arrives through the sign test like any other code, which is the point of + /// choosing a negative sentinel: `usize::try_from` rejects it, so this needs no case + /// of its own here and a positive length cannot be mistaken for it. #[test] - fn a_caught_cxx_exception_arrives_as_unimplemented() { - assert_eq!(bytes_written(-1), Err(HostError::Unimplemented)); + fn a_caught_cxx_exception_arrives_as_internal_fatal() { + assert_eq!(bytes_written(i32::MIN), Err(HostError::InternalFatal)); + } + + /// A code the ABI does not define goes the same way, so a C++ list this crate has + /// not caught up with stops the run rather than reaching the guest. + #[test] + fn an_undefined_code_arrives_as_internal_fatal() { + assert_eq!(bytes_written(-21), Err(HostError::InternalFatal)); } // ----------------------------------------------------------------------- diff --git a/crates/xrpl-wasm-vm/src/abi.rs b/crates/xrpl-wasm-vm/src/abi.rs index 3f30425076..5a8cedd348 100644 --- a/crates/xrpl-wasm-vm/src/abi.rs +++ b/crates/xrpl-wasm-vm/src/abi.rs @@ -41,13 +41,15 @@ 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. +/// Three codes stop the run instead of reaching the contract that asked. Each says the +/// call was not served at all — the host could not do it, it has not been wired, or +/// there is nowhere to put the answer — and a contract has no business interpreting +/// any of them, 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::InternalFatal => CallError::Fatal(Fault::Internal), HostError::Unimplemented => CallError::Fatal(Fault::Internal), HostError::NoMemExported => CallError::Fatal(Fault::NoMemory), code => CallError::Code(code), @@ -307,7 +309,8 @@ mod tests { /// 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] = [ + const STOPS_THE_RUN: [(HostError, Fault); 3] = [ + (HostError::InternalFatal, Fault::Internal), (HostError::Unimplemented, Fault::Internal), (HostError::NoMemExported, Fault::NoMemory), ]; diff --git a/crates/xrpl-wasm-vm/tests/host_calls.rs b/crates/xrpl-wasm-vm/tests/host_calls.rs index 06fd0279f7..4ae880758f 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::Unimplemented); + let host = FakeHost::new().failing_trace(HostError::InternalFatal); let wat = module( &[import::TRACE, ONE_PAGE], diff --git a/include/xrpl/tx/wasm/WasmCommon.h b/include/xrpl/tx/wasm/WasmCommon.h index fa651bef10..1bf3c93379 100644 --- a/include/xrpl/tx/wasm/WasmCommon.h +++ b/include/xrpl/tx/wasm/WasmCommon.h @@ -44,6 +44,15 @@ enum class HostFunctionError : int32_t { IndexOutOfBounds = -18, FloatInputMalformed = -19, FloatComputationError = -20, + + // The call was not served at all, so the engine stops the run and the transaction is + // tecINTERNAL rather than the contract being handed a code to interpret. `guarded` + // answers it for a host body that throws. + // + // Outside the -1 ..= -20 range that a contract reads, and the only entry that is: it + // needs no number in that range, and INT32_MIN cannot collide with a code appended + // above. Negative so that a reader treating it as an ordinary failure is still right. + InternalFatal = std::numeric_limits::min(), }; template diff --git a/src/libxrpl/tx/wasm/HostContext.cpp b/src/libxrpl/tx/wasm/HostContext.cpp index 1a0215af29..55d136f0c5 100644 --- a/src/libxrpl/tx/wasm/HostContext.cpp +++ b/src/libxrpl/tx/wasm/HostContext.cpp @@ -29,13 +29,8 @@ 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 converts -1 into a fault, -// stops the run and reports `tecINTERNAL`, rather than handing the code to the contract. -// -// 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); +// to `guarded` as the answer for a body that throws. +constexpr std::int32_t kHostInternal = hfErrorToInt(HostFunctionError::InternalFatal); // Copy `value` into `out` only if the whole of it fits, and answer its true length either // way. A value too large for the guest's buffer must reach it in no part: a prefix would diff --git a/src/libxrpl/tx/wasm/HostFuncImplGetter.cpp b/src/libxrpl/tx/wasm/HostFuncImplGetter.cpp index 3d13997f17..27b0370171 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 -1, which stops the run -> + // xrpld bug, and `guarded` turns the throw into `InternalFatal`, 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 8dfdfb2c93..54e2f89848 100644 --- a/src/tests/libxrpl/tx/wasm/WasmVM.cpp +++ b/src/tests/libxrpl/tx/wasm/WasmVM.cpp @@ -292,7 +292,10 @@ TEST_F(WasmVMTest, FatalHostErrorStopsRun) return std::unexpected(refused); }); - for (auto const error : {HostFunctionError::Unimplemented, HostFunctionError::NoMemExported}) + for (auto const error : + {HostFunctionError::InternalFatal, + HostFunctionError::Unimplemented, + HostFunctionError::NoMemExported}) { refused = error;