From 91a23fc92cf0c7b6da1e8126bb285c3f6ea5c00e Mon Sep 17 00:00:00 2001 From: Sergey Kuznetsov Date: Tue, 11 Aug 2026 14:47:11 +0100 Subject: [PATCH] Update trace method --- crates/xrpl-host-functions/src/lib.rs | 94 ++++++-- .../tests/generated_abi.rs | 50 +++-- crates/xrpl-wasm-vm-ffi/src/lib.rs | 98 ++++++--- crates/xrpl-wasm-vm/src/abi.rs | 55 ++++- crates/xrpl-wasm-vm/src/register.rs | 46 ++-- crates/xrpl-wasm-vm/tests/budgets.rs | 125 +++++++---- crates/xrpl-wasm-vm/tests/host_calls.rs | 127 +++++++---- crates/xrpl-wasm-vm/tests/memory_policy.rs | 116 ++++++---- crates/xrpl-wasm-vm/tests/preflight.rs | 3 +- crates/xrpl-wasm-vm/tests/support/mod.rs | 82 ++++--- include/xrpl/tx/wasm/HostContext.h | 25 ++- src/libxrpl/tx/wasm/HostContext.cpp | 132 ++++++++++-- src/tests/libxrpl/tx/wasm/MockHostFunctions.h | 12 +- .../libxrpl/tx/wasm/host_calls/Trace.cpp | 202 ++++++++++++++++-- .../libxrpl/tx/wasm/host_calls/TraceNum.cpp | 53 ----- 15 files changed, 865 insertions(+), 355 deletions(-) delete mode 100644 src/tests/libxrpl/tx/wasm/host_calls/TraceNum.cpp diff --git a/crates/xrpl-host-functions/src/lib.rs b/crates/xrpl-host-functions/src/lib.rs index c2082a9d0f..80d104301b 100644 --- a/crates/xrpl-host-functions/src/lib.rs +++ b/crates/xrpl-host-functions/src/lib.rs @@ -48,6 +48,12 @@ 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. + #[inline] + pub const fn code(self) -> i32 { + self as i32 + } + /// Reconstruct a `HostError` from its wire code; unknown/positive values /// map to `Internal`. pub const fn from_code(code: i32) -> HostError { @@ -86,20 +92,72 @@ host_errors! { OutOfTransferLimit = -23, } -impl HostError { - /// The negative wire value the guest sees as the function's return code. - #[inline] - pub const fn code(self) -> i32 { - self as i32 - } -} - /// Convenience alias for the trait's fallible returns. pub type HostResult = Result; /// A `sha512Half` digest: the first 32 bytes of a SHA-512, as XRPL uses it. pub const HASH_LEN: usize = 32; +/// Declares [`TraceDataType`] from one list, so [`TraceDataType::ALL`], +/// [`TraceDataType::code`] and [`TraceDataType::from_code`] cannot fall behind the +/// variants — the reason `host_errors!` above is written this way. +macro_rules! trace_data_types { + ($($(#[$doc:meta])* $variant:ident = $code:literal,)+) => { + /// How [`HostFunctions::trace`] is to read its data buffer. + /// + /// The discriminants are wire values shared with the guest stdlib: append only, + /// never renumber. They start at 1, so a zeroed argument names no type rather + /// than the first one. + /// + /// This is the declaration a guest and a host both compile against. The host + /// side needs a second one — `cxx` cannot be a dependency here, since this + /// crate also links into the guest — so `xrpl-wasm-vm-ffi` declares a shared + /// enum for C++ and converts, exhaustively, from this. + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + #[repr(i32)] + pub enum TraceDataType { + $($(#[$doc])* $variant = $code,)+ + } + + impl TraceDataType { + /// Every data type a guest may name, in code order. + pub const ALL: &'static [TraceDataType] = &[$(TraceDataType::$variant,)+]; + + /// The wire value a guest passes to name this type. + #[inline] + pub const fn code(self) -> i32 { + self as i32 + } + + /// The type `code` names, or `None`: the engine drops a call it cannot + /// read rather than guessing at a rendering the guest did not ask for. + pub const fn from_code(code: i32) -> Option { + match code { + $($code => Some(TraceDataType::$variant),)+ + _ => None, + } + } + } + }; +} + +trace_data_types! { + /// 8 little-endian bytes, rendered as a signed decimal. + Int64 = 1, + /// 8 little-endian bytes, rendered as an unsigned decimal. + Uint64 = 2, + /// A serialized XRPL float: 12 bytes, mantissa then exponent. + Xfloat = 3, + /// A 20-byte account ID, rendered as base58. + Account = 4, + /// A serialized `STAmount`. + Amount = 5, + /// Raw bytes, hex-encoded. + AsHex = 6, + /// Bytes rendered verbatim as text. + AsText = 7, +} + host_functions! { /// The sequence number of the ledger being built, as 4 little-endian bytes. #[gas = 60] @@ -116,13 +174,17 @@ host_functions! { #[wasm_name = "sha512_half"] fn sha512_half(&self, data: &[u8], out: &mut [u8]) -> HostResult; - /// Writes `msg` and `data` to the trace log, `data` in hex if `as_hex`. - #[gas = 500] + /// Writes `msg` to the trace log, followed by `data` rendered as `data_type` says. + /// + /// The one declaration whose wasm function has **no result**: this node's own log + /// is its only effect, so a guest is told nothing. An `Err` from a host therefore + /// reaches it in no form, and only the host-fatal ones do anything at all. + /// + /// It is also the one declaration that is **not** the wasm parameter order. + /// `data_type` is the third wasm parameter, between the two regions, because that + /// is where xrpld's `trace_proto` and the guest stdlib put it; `register.rs` takes + /// the arguments in wasm order and calls this in declaration order. + #[gas = 30] #[wasm_name = "trace"] - fn trace(&self, msg: &str, data: &[u8], as_hex: bool) -> HostResult<()>; - - /// Writes `msg` and `number` to the trace log. - #[gas = 500] - #[wasm_name = "trace_num"] - fn trace_num(&self, msg: &str, number: i64) -> HostResult<()>; + fn trace(&self, msg: &str, data: &[u8], data_type: TraceDataType) -> HostResult<()>; } diff --git a/crates/xrpl-host-functions/tests/generated_abi.rs b/crates/xrpl-host-functions/tests/generated_abi.rs index a760a2b3af..c0327f86c7 100644 --- a/crates/xrpl-host-functions/tests/generated_abi.rs +++ b/crates/xrpl-host-functions/tests/generated_abi.rs @@ -4,7 +4,9 @@ use std::cell::RefCell; use std::collections::HashSet; -use xrpl_host_functions::{HASH_LEN, HostError, HostFunctionSpec, HostFunctions, HostResult}; +use xrpl_host_functions::{ + HASH_LEN, HostError, HostFunctionSpec, HostFunctions, HostResult, TraceDataType, +}; /// Records what it was asked to do; enough to prove the trait is usable. /// @@ -45,15 +47,10 @@ impl HostFunctions for FakeHost { put(out, &digest) } - fn trace(&self, msg: &str, data: &[u8], as_hex: bool) -> HostResult<()> { + fn trace(&self, msg: &str, data: &[u8], data_type: TraceDataType) -> HostResult<()> { self.traced .borrow_mut() - .push(format!("{msg}/{}/{as_hex}", data.len())); - Ok(()) - } - - fn trace_num(&self, msg: &str, number: i64) -> HostResult<()> { - self.traced.borrow_mut().push(format!("{msg}={number}")); + .push(format!("{msg}/{data_type:?}/{}", data.len())); Ok(()) } } @@ -69,10 +66,9 @@ fn the_trait_is_implementable() { assert_eq!(out[0], 3); assert_eq!(host.sha512_half(b"abc", &mut out), Ok(HASH_LEN)); assert_eq!(out[0], 3); - assert_eq!(host.trace("hello", b"xy", true), Ok(())); - assert_eq!(host.trace_num("count", -1), Ok(())); + assert_eq!(host.trace("hello", b"xy", TraceDataType::AsHex), Ok(())); - assert_eq!(*host.traced.borrow(), ["hello/2/true", "count=-1"]); + assert_eq!(*host.traced.borrow(), ["hello/AsHex/2"]); } /// The error channel every declaration carries: an `Err` the VM turns into the @@ -113,9 +109,12 @@ fn the_trait_is_callable_through_a_shared_trait_object() { let mut out = [0u8; 4]; assert_eq!(host.get_ledger_sqn(&mut out), Ok(4)); - assert_eq!(host.trace_num("count", 1), Ok(())); + assert_eq!( + host.trace("count", &1i64.to_le_bytes(), TraceDataType::Int64), + Ok(()) + ); - assert_eq!(*fake.traced.borrow(), ["count=1"]); + assert_eq!(*fake.traced.borrow(), ["count/Int64/8"]); } /// The whole table, written out: the one place the ABI's wire names and gas costs @@ -137,12 +136,33 @@ fn the_spec_table_matches_the_declarations() { ("ldgr_index", 60), ("home_le_field", 70), ("sha512_half", 2000), - ("trace", 500), - ("trace_num", 500), + ("trace", 30), ] ); } +/// The other half of the wire vocabulary, and the same change-detector argument: the +/// codes are what a guest passes, so they are pinned as literals here. `ALL` is in code +/// order, so the round trip pins the discriminants and not just the membership. +#[test] +fn every_trace_data_type_survives_the_wire() { + let codes: Vec = TraceDataType::ALL.iter().map(|t| t.code()).collect(); + + assert_eq!(codes, [1, 2, 3, 4, 5, 6, 7]); + for &data_type in TraceDataType::ALL { + assert_eq!(TraceDataType::from_code(data_type.code()), Some(data_type)); + } +} + +/// A code no declaration names is refused rather than read as a neighbouring type. +/// Zero is the one worth naming: it is what a guest sends by omission. +#[test] +fn an_unnamed_trace_data_type_code_is_refused() { + for code in [0, -1, 8, i32::MAX, i32::MIN] { + assert_eq!(TraceDataType::from_code(code), None, "code {code}"); + } +} + /// `ALL` is what a wasm engine iterates to register imports, so no two declarations /// may collapse to the same wire name. The table above pins membership and order; /// this adds only uniqueness, and restates nothing. diff --git a/crates/xrpl-wasm-vm-ffi/src/lib.rs b/crates/xrpl-wasm-vm-ffi/src/lib.rs index 5a4c77048e..5c9b4e9a8c 100644 --- a/crates/xrpl-wasm-vm-ffi/src/lib.rs +++ b/crates/xrpl-wasm-vm-ffi/src/lib.rs @@ -28,7 +28,7 @@ use std::any::Any; use std::panic::{AssertUnwindSafe, catch_unwind}; -use xrpl_host_functions::{HostError, HostFunctions, HostResult}; +use xrpl_host_functions::{HostError, HostFunctions, HostResult, TraceDataType}; use xrpl_wasm_vm::{CheckError, RunError, RunFailure, RunOutcome, check, run}; /// [`guarded`] must be able to stop an unwind. Under `panic = "abort"` it cannot, @@ -111,6 +111,32 @@ mod ffi { detail: String, } + /// How `HostContext::trace` is to read its data buffer. + /// + /// **Declared here so that C++ does not declare it.** A shared enum is emitted into + /// the generated header as `xrpl::TraceDataType`, which is the definition + /// `HostContext.cpp` switches on — so the variants and their wire values are + /// written once, in Rust, for both languages. + /// + /// It is not the same type as [`xrpl_host_functions::TraceDataType`], and cannot + /// be: the ABI crate is `no_std` with no dependencies so that it also links into + /// the guest, and `cxx` is neither. [`crossed`] converts, in a `match` that is + /// exhaustive over the ABI's enum — so a data type added there fails to compile + /// until it is added here, which is the drift check the hand-written C++ copy + /// never had. + #[namespace = "xrpl"] + #[derive(Debug, Hash)] + #[repr(i32)] + enum TraceDataType { + Int64 = 1, + Uint64 = 2, + Xfloat = 3, + Account = 4, + Amount = 5, + AsHex = 6, + AsText = 7, + } + extern "Rust" { /// Run `wasm`'s `function_name` export with `gas` fuel, servicing host calls /// through `host`. @@ -167,14 +193,15 @@ mod ffi { #[cxx_name = "sha512Half"] fn sha512_half(self: &HostContext, data: &[u8], out: &mut [u8]) -> i32; - /// A call with no value to report answers `0`, or a negative `HostError` - /// code. + /// Renders `data` as `data_type` says and writes it to this node's log with + /// `msg`. Answers nothing at all: the guest's wasm function has no result, and + /// C++ swallows a malformed buffer rather than reporting it, so there is no + /// failure for this side to encode. + /// + /// The engine has already refused a code that names no type, so what crosses + /// here is always one of the variants. #[namespace = "xrpl"] - fn trace(self: &HostContext, msg: &str, data: &[u8], as_hex: bool) -> i32; - - #[namespace = "xrpl"] - #[cxx_name = "traceNum"] - fn trace_num(self: &HostContext, msg: &str, number: i64) -> i32; + fn trace(self: &HostContext, msg: &str, data: &[u8], data_type: TraceDataType); } } @@ -191,20 +218,28 @@ struct CxxHost<'a> { /// The conversion *is* the sign test — it fails on exactly the negative values — so /// there is no cast to argue about. /// -/// Named functions rather than `From` impls, and not by preference: every type +/// A named function rather than a `From` impl, and not by preference: every type /// involved — `i32`, `Result`, `HostError` — is foreign to this crate, so the orphan -/// rule forbids the impl. Two readings of the same `i32` would want distinguishing -/// names here in any case. +/// rule forbids the impl. fn bytes_written(n: i32) -> HostResult { usize::try_from(n).map_err(|_| HostError::from_code(n)) } -/// A call with nothing to report: any non-negative answer is success. -fn reported(n: i32) -> HostResult<()> { - if n < 0 { - return Err(HostError::from_code(n)); +/// The ABI's data type as the shared enum C++ was given a definition of. +/// +/// A `match` rather than a cast through `code()`: the cast would compile for a variant +/// nobody added to [`ffi::TraceDataType`] and hand C++ a value its `switch` does not +/// name. This is the whole reason the two lists cannot drift. +fn crossed(data_type: TraceDataType) -> ffi::TraceDataType { + match data_type { + TraceDataType::Int64 => ffi::TraceDataType::Int64, + TraceDataType::Uint64 => ffi::TraceDataType::Uint64, + TraceDataType::Xfloat => ffi::TraceDataType::Xfloat, + TraceDataType::Account => ffi::TraceDataType::Account, + TraceDataType::Amount => ffi::TraceDataType::Amount, + TraceDataType::AsHex => ffi::TraceDataType::AsHex, + TraceDataType::AsText => ffi::TraceDataType::AsText, } - Ok(()) } impl HostFunctions for CxxHost<'_> { @@ -220,12 +255,9 @@ impl HostFunctions for CxxHost<'_> { bytes_written(self.ctx.sha512_half(data, out)) } - fn trace(&self, msg: &str, data: &[u8], as_hex: bool) -> HostResult<()> { - reported(self.ctx.trace(msg, data, as_hex)) - } - - fn trace_num(&self, msg: &str, number: i64) -> HostResult<()> { - reported(self.ctx.trace_num(msg, number)) + fn trace(&self, msg: &str, data: &[u8], data_type: TraceDataType) -> HostResult<()> { + self.ctx.trace(msg, data, crossed(data_type)); + Ok(()) } } @@ -503,13 +535,30 @@ mod tests { assert_eq!(crossed.gas_used, 2); } + /// [`crossed`] being exhaustive makes the two lists hold the same *variants*; + /// this makes them hold the same *numbers*, which is what actually crosses. A + /// `match` arm pointed at the wrong variant would pass the compiler and fail + /// here. + /// + /// Over `TraceDataType::ALL`, so it is the whole set rather than a sample: a data + /// type added to the ABI arrives already asserted against the shared enum. + #[test] + fn every_data_type_crosses_as_the_same_wire_value() { + for &data_type in TraceDataType::ALL { + assert_eq!( + crossed(data_type).repr, + data_type.code(), + "{data_type:?} crosses as a different value than the ABI gives it" + ); + } + } + #[test] fn a_negative_answer_is_an_error_code_and_a_length_is_a_length() { assert_eq!(bytes_written(32), Ok(32)); assert_eq!(bytes_written(0), Ok(0)); assert_eq!(bytes_written(-3), Err(HostError::BufferTooSmall)); - assert_eq!(reported(0), Ok(())); - assert_eq!(reported(-14), Err(HostError::NoMemExported)); + assert_eq!(bytes_written(-14), Err(HostError::NoMemExported)); } /// An exception caught on the C++ side arrives as `-1`, which has to reach the @@ -518,7 +567,6 @@ mod tests { #[test] fn a_caught_cxx_exception_arrives_as_internal() { assert_eq!(bytes_written(-1), Err(HostError::Internal)); - assert_eq!(reported(-1), Err(HostError::Internal)); } // ----------------------------------------------------------------------- diff --git a/crates/xrpl-wasm-vm/src/abi.rs b/crates/xrpl-wasm-vm/src/abi.rs index db6db25fda..74a46727ec 100644 --- a/crates/xrpl-wasm-vm/src/abi.rs +++ b/crates/xrpl-wasm-vm/src/abi.rs @@ -32,6 +32,29 @@ pub(crate) fn charged( to_wire(charge(caller, op.gas()).and_then(|()| body(caller))) } +/// [`charged`] for a call the guest gets no answer from: its wasm function has no +/// result, so a soft error has nowhere to go and is dropped. The gas is charged first +/// and charged whatever happens after, so the cost is all such a call leaves behind. +/// +/// Only `trace` takes this path. +pub(crate) fn charged_unreported( + caller: &mut Caller<'_, VmState<'_>>, + op: HostFunctionSpec, + body: impl FnOnce(&mut Caller<'_, VmState<'_>>) -> HostResult<()>, +) -> 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> { + match result { + Err(error) if is_fatal(error) => Err(wasmi::Error::host(FatalHostError(error))), + _ => Ok(()), + } +} + fn to_wire(result: HostResult) -> Result { match result { Ok(value) => Ok(value), @@ -69,7 +92,7 @@ fn memory(caller: &Caller<'_, VmState<'_>>) -> Result { } /// [`Region::read`] of the guest's memory, for a call that reads and writes nothing -/// back (`trace`, `trace_num`). +/// back (`trace`). pub(crate) fn read_borrowed<'a>( caller: &'a Caller<'_, VmState<'_>>, input: Region, @@ -180,6 +203,7 @@ mod tests { use crate::vm::TRANSFER_LIMIT_BYTES; use std::cell::Cell; use wasmi::StoreLimitsBuilder; + use xrpl_host_functions::TraceDataType; /// `charge_transfer` takes the store data, which has to hold a host. struct UncalledHost; @@ -194,10 +218,7 @@ mod tests { fn sha512_half(&self, _data: &[u8], _out: &mut [u8]) -> HostResult { unreachable!("no unit test in this module calls the host") } - fn trace(&self, _msg: &str, _data: &[u8], _as_hex: bool) -> HostResult<()> { - unreachable!("no unit test in this module calls the host") - } - fn trace_num(&self, _msg: &str, _number: i64) -> HostResult<()> { + fn trace(&self, _msg: &str, _data: &[u8], _data_type: TraceDataType) -> HostResult<()> { unreachable!("no unit test in this module calls the host") } } @@ -267,6 +288,30 @@ mod tests { } } + /// 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. + #[test] + fn a_call_with_no_result_drops_a_soft_error_and_traps_on_a_fatal_one() { + 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 { + assert!( + dropped(Err(error)).is_ok(), + "{error:?} has no channel to the guest and must be dropped" + ); + } + } + } + #[test] fn a_transfer_spends_the_budget() { let state = state(100); diff --git a/crates/xrpl-wasm-vm/src/register.rs b/crates/xrpl-wasm-vm/src/register.rs index ef933f1bf9..a3fac3824d 100644 --- a/crates/xrpl-wasm-vm/src/register.rs +++ b/crates/xrpl-wasm-vm/src/register.rs @@ -1,8 +1,8 @@ -use crate::abi::{charged, read_borrowed, write_buffered, write_into}; +use crate::abi::{charged, charged_unreported, read_borrowed, write_buffered, write_into}; use crate::region::Region; use crate::vm::VmState; use wasmi::{Caller, Linker}; -use xrpl_host_functions::{HostError, HostFunctionSpec}; +use xrpl_host_functions::{HostError, HostFunctionSpec, TraceDataType}; /// The module name the guest imports under (`(import "host_lib" "ldgr_index" …)`), /// as the guest SDK and this fork's fixtures spell it. @@ -73,40 +73,34 @@ pub(crate) fn register_host_functions( }) }, ), + // The one arm with no result: the wasm function is `(param i32 i32 i32 i32 + // i32)` and nothing more, so a malformed call is dropped rather than + // answered — an unreadable region, a `msg` that is not UTF-8 and a + // `data_type` naming no rendering all leave the guest none the wiser, and + // the host uncalled. + // + // Also the one arm whose parameters are not the declaration's order: + // `data_type` arrives third, between the two regions, as xrpld and the + // guest stdlib spell it. The wasm order is this closure's; the declaration + // order is the call's. HostFunctionSpec::Trace => linker.func_wrap( HOST_MODULE, op.wasm_name(), |mut caller: Caller<'_, VmState<'_>>, msg_ptr: i32, msg_len: i32, + data_type: i32, data_ptr: i32, - data_len: i32, - as_hex: i32| - -> Result { - charged(&mut caller, HostFunctionSpec::Trace, |c| { + data_len: i32| + -> Result<(), wasmi::Error> { + 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 data_type = + TraceDataType::from_code(data_type).ok_or(HostError::InvalidParams)?; let data = read_borrowed(c, Region::new(data_ptr, data_len))?; - let msg = core::str::from_utf8(msg).map_err(|_| HostError::Decoding)?; - host.trace(msg, data, as_hex != 0)?; - Ok(0) - }) - }, - ), - HostFunctionSpec::TraceNum => linker.func_wrap( - HOST_MODULE, - op.wasm_name(), - |mut caller: Caller<'_, VmState<'_>>, - msg_ptr: i32, - msg_len: i32, - number: i64| - -> Result { - charged(&mut caller, HostFunctionSpec::TraceNum, |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)?; - host.trace_num(msg, number)?; - Ok(0) + host.trace(msg, data, data_type) }) }, ), diff --git a/crates/xrpl-wasm-vm/tests/budgets.rs b/crates/xrpl-wasm-vm/tests/budgets.rs index 9b7ac613ed..5b5f4a9965 100644 --- a/crates/xrpl-wasm-vm/tests/budgets.rs +++ b/crates/xrpl-wasm-vm/tests/budgets.rs @@ -4,8 +4,11 @@ mod support; -use support::{Answer, FakeHost, ONE_PAGE, PLENTY_OF_GAS, code, import, module, run, run_with_gas}; -use xrpl_host_functions::{HASH_LEN, HostError, HostFunctionSpec}; +use support::{ + Answer, EMPTY_REGION, FakeHost, ONE_PAGE, PLENTY_OF_GAS, code, import, module, run, + run_with_gas, trace_call, +}; +use xrpl_host_functions::{HASH_LEN, HostError, HostFunctionSpec, TraceDataType}; use xrpl_wasm_vm::{MAX_FIELD_BYTES, RunError, TRANSFER_LIMIT_BYTES}; // --------------------------------------------------------------------------- @@ -34,6 +37,11 @@ fn wasmi_call_fuel(small_const_operands: u64) -> u64 { 14 * small_const_operands + 1 } +/// What wasmi charges on top of that for a call to a function with no result — +/// `trace`'s shape, and nothing else in the ABI. Per call, not per module. Measured +/// and pinned like the figures above. +const WASMI_NO_RESULT_FUEL: u64 = 14; + /// wasmi's fuel for one `(drop …)`, which is how a module makes more than one call /// and keeps only the last result. Pinned like the two above. const WASMI_DROP_FUEL: u64 = 21; @@ -44,6 +52,36 @@ struct Call { import: &'static str, call: &'static str, operands: u64, + /// Whether the call leaves an `i32` behind. `trace` does not, which is why + /// [`Call::body`] ends every module with a constant instead of the call. + yields: bool, +} + +impl Call { + /// `n` calls in a row, leaving one `i32` for the module to return: the last + /// answer where there is one, and a constant where the call has none. + fn body(&self, n: usize) -> String { + if self.yields { + format!( + "{}{}", + format!("(drop {}) ", self.call).repeat(n - 1), + self.call + ) + } else { + format!("{}(i32.const 0)", format!("{} ", self.call).repeat(n)) + } + } + + /// What [`Call::body`] burns beside the calls' own gas and the module's floor: + /// one `drop` between consecutive answers, or wasmi's own surcharge on a call + /// that has none. + fn overhead(&self, n: u64) -> u64 { + if self.yields { + (n - 1) * WASMI_DROP_FUEL + } else { + n * WASMI_NO_RESULT_FUEL + } + } } /// The test wasm for each host function. The `match` is exhaustive, so a function @@ -68,19 +106,15 @@ fn call_for(op: HostFunctionSpec) -> Call { ), HostFunctionSpec::Trace => ( import::TRACE, - "(call $trace (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 0))", + "(call $trace (i32.const 0) (i32.const 0) (i32.const 1) (i32.const 0) (i32.const 0))", 5, ), - HostFunctionSpec::TraceNum => ( - import::TRACE_NUM, - "(call $trace_num (i32.const 0) (i32.const 0) (i64.const 0))", - 3, - ), }; Call { import, call, operands, + yields: !matches!(op, HostFunctionSpec::Trace), } } @@ -91,30 +125,27 @@ fn an_empty_module_burns_a_fixed_amount_of_fuel() { } /// Calling a host function `n` times costs `n` times its gas, to the unit. Every -/// other term is known — the module's floor, wasmi's fuel per call, one `drop` -/// between consecutive calls — so the total is a closed form, with the gas read -/// from the spec table rather than restated. `n = 1` pins the charge, `n > 1` pins -/// that it lands on every call rather than once per run. +/// other term is known — the module's floor, wasmi's fuel per call, one `drop` per +/// answered call — so the total is a closed form, with the gas read from the spec +/// table rather than restated. `n = 1` pins the charge, `n > 1` pins that it lands +/// on every call rather than once per run. #[test] fn a_host_call_costs_its_gas_every_time_it_is_called() { let host = FakeHost::new().answering_field(1, Answer::bytes([0xaa])); for &op in HostFunctionSpec::ALL { - let Call { - import, - call, - operands, - } = call_for(op); - let per_call = wasmi_call_fuel(operands) + op.gas(); + let call = call_for(op); + let per_call = wasmi_call_fuel(call.operands) + op.gas(); for n in 1..=3 { - let body = format!("{}{call}", format!("(drop {call}) ").repeat(n - 1)); + let body = call.body(n); let n = n as u64; assert_eq!( - fuel_for(&body, &[import, ONE_PAGE], &host), - EMPTY_MODULE_FUEL + n * per_call + (n - 1) * WASMI_DROP_FUEL, - "{n} x {call}" + fuel_for(&body, &[call.import, ONE_PAGE], &host), + EMPTY_MODULE_FUEL + n * per_call + call.overhead(n), + "{n} x {}", + call.call ); } } @@ -148,13 +179,9 @@ fn a_failing_host_call_costs_exactly_what_a_successful_one_costs() { fn fuel_used_is_what_was_spent_not_what_was_supplied() { let host = FakeHost::new(); let op = HostFunctionSpec::GetLedgerSqn; - let Call { - import, - call, - operands, - } = call_for(op); - let wat = module(&[import, ONE_PAGE], call); - let cost = EMPTY_MODULE_FUEL + wasmi_call_fuel(operands) + op.gas(); + let call = call_for(op); + let wat = module(&[call.import, ONE_PAGE], call.call); + let cost = EMPTY_MODULE_FUEL + wasmi_call_fuel(call.operands) + op.gas(); // Exactly its cost is enough, and no amount above it changes the figure. The // result is checked too, so the figure belongs to a run that did the work @@ -182,10 +209,8 @@ fn fuel_used_is_what_was_spent_not_what_was_supplied() { /// property consensus depends on. #[test] fn the_same_run_burns_the_same_fuel() { - let wat = module( - &[import::TRACE, ONE_PAGE], - "(call $trace (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 0))", - ); + let call = call_for(HostFunctionSpec::Trace); + let wat = module(&[call.import, ONE_PAGE], &call.body(1)); let first = run(&wat, &FakeHost::new()).expect("should run").fuel_used; for _ in 0..4 { @@ -242,23 +267,24 @@ fn an_endless_loop_is_stopped_by_gas() { /// ignore the refusal and carry on, and it is charged the whole limit. /// /// The gas range is every amount that reaches the call and cannot pay for it, so -/// the case is the whole boundary rather than one number. +/// the case is the whole boundary rather than one number. `trace` is the call under +/// it because it is the one that could not report a refusal even if it wanted to: +/// stopping the run is the whole of what the guest sees. #[test] fn a_host_call_refused_its_gas_stops_the_run() { let host = FakeHost::new(); - let op = HostFunctionSpec::TraceNum; - let Call { - import, - call, - operands, - } = call_for(op); - let wat = module(&[import, ONE_PAGE], call); - // What the guest spends getting as far as the call. Below it the meter stops - // the guest's own instructions instead, which is + let op = HostFunctionSpec::Trace; + let call = call_for(op); + let wat = module(&[call.import, ONE_PAGE], &call.body(1)); + // Measured rather than derived: the whole run's cost, less the call's own gas, + // is the least a guest can be given and still reach the call. Below that the + // meter stops the guest's own instructions instead, which is // `a_run_that_cannot_afford_itself_fails`'s case, not this one. - let reaching_the_call = EMPTY_MODULE_FUEL + wasmi_call_fuel(operands); + let cost = run(&wat, &FakeHost::new()) + .expect("the module should run") + .fuel_used; - for gas in reaching_the_call..reaching_the_call + op.gas() { + for gas in cost - op.gas()..cost { let Err(failure) = run_with_gas(&wat, gas, &host) else { panic!("gas {gas}: the run completed, so the guest was handed the refusal"); }; @@ -362,12 +388,17 @@ fn reads_do_not_spend_the_transfer_budget() { const READS: u64 = 4 * TRANSFER_LIMIT_BYTES / MAX_FIELD_BYTES as u64; let host = FakeHost::new().answering_field(1, Answer::filler(MAX_FIELD_BYTES)); + let read = trace_call( + TraceDataType::AsHex, + EMPTY_REGION, + &format!("(i32.const 0) (i32.const {MAX_FIELD_BYTES})"), + ); let wat = module( - &[import::TRACE_NUM, import::HOME_LE_FIELD, ONE_PAGE], + &[import::TRACE, import::HOME_LE_FIELD, ONE_PAGE], &format!( "(local $i i32) (loop $l - (drop (call $trace_num (i32.const 0) (i32.const {MAX_FIELD_BYTES}) (i64.const 0))) + {read} (local.set $i (i32.add (local.get $i) (i32.const 1))) (br_if $l (i32.lt_u (local.get $i) (i32.const {READS})))) (call $home_le_field (i32.const 1) (i32.const 0) (i32.const {MAX_FIELD_BYTES}))" diff --git a/crates/xrpl-wasm-vm/tests/host_calls.rs b/crates/xrpl-wasm-vm/tests/host_calls.rs index d9987d58ea..b194ba7dad 100644 --- a/crates/xrpl-wasm-vm/tests/host_calls.rs +++ b/crates/xrpl-wasm-vm/tests/host_calls.rs @@ -4,8 +4,12 @@ mod support; -use support::{FakeHost, ONE_PAGE, Trace, code, import, module, run, status}; -use xrpl_host_functions::{HASH_LEN, HostError}; +use support::{ + COMPLETED, EMPTY_REGION, FakeHost, ONE_PAGE, Trace, code, failure, import, module, run, status, + traced, +}; +use xrpl_host_functions::{HASH_LEN, HostError, TraceDataType}; +use xrpl_wasm_vm::RunError; /// A value the host writes must be readable by the guest at the pointer it gave, /// and the call's status is the byte count. @@ -127,9 +131,10 @@ fn sha512_half_accepts_an_empty_input() { assert_eq!(*host.digested.borrow(), vec![Vec::::new()]); } -/// `trace` reads two regions and a flag, and yields a status of 0. +/// `trace` reads two regions and a type, and hands the guest back nothing — the +/// module returns a constant of its own, which is what a completed run looks like. #[test] -fn trace_passes_its_message_data_and_flag_through() { +fn trace_passes_its_message_type_and_data_through() { let host = FakeHost::new(); let wat = module( @@ -139,54 +144,59 @@ fn trace_passes_its_message_data_and_flag_through() { r#"(data (i32.const 0) "note")"#, r#"(data (i32.const 16) "\01\02\03")"#, ], - "(call $trace (i32.const 0) (i32.const 4) (i32.const 16) (i32.const 3) (i32.const 1))", + &traced( + TraceDataType::AsHex, + "(i32.const 0) (i32.const 4)", + "(i32.const 16) (i32.const 3)", + ), ); - assert_eq!(status(&wat, &host), 0, "trace yields a status of 0"); + assert_eq!(status(&wat, &host), COMPLETED); assert_eq!( host.traces(), - vec![Trace::Message { + vec![Trace { msg: "note".to_owned(), + data_type: TraceDataType::AsHex, data: vec![1, 2, 3], - as_hex: true, }] ); } -/// The flag is `bool` in the declaration and `i32` on the wire: nonzero is true. +/// The type is the guest's to choose and the host's to act on, so every code the +/// ABI names has to arrive as the type it names. #[test] -fn any_nonzero_flag_is_true() { - for (flag, expected) in [("0", false), ("1", true), ("2", true), ("-1", true)] { +fn every_data_type_reaches_the_host_as_declared() { + for &data_type in TraceDataType::ALL { + let host = FakeHost::new(); + let wat = module( + &[import::TRACE, ONE_PAGE], + &traced(data_type, EMPTY_REGION, EMPTY_REGION), + ); + assert_eq!(status(&wat, &host), COMPLETED, "{data_type:?}"); + assert_eq!( + host.traces().first().map(|t| t.data_type), + Some(data_type), + "{data_type:?}" + ); + } +} + +/// A code no type carries is the guest's mistake, and there is no channel to tell it +/// so: the call is dropped and the run carries on. +#[test] +fn a_code_that_names_no_data_type_drops_the_call() { + for code in [0, -1, 8] { let host = FakeHost::new(); let wat = module( &[import::TRACE, ONE_PAGE], &format!( - "(call $trace (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 0) (i32.const {flag}))" + "(call $trace (i32.const 0) (i32.const 0) (i32.const {code}) (i32.const 0) (i32.const 0)) + (i32.const {COMPLETED})" ), ); - assert_eq!(status(&wat, &host), 0); - let Some(Trace::Message { as_hex, .. }) = host.traces().first().cloned() else { - panic!("expected one traced message"); - }; - assert_eq!(as_hex, expected, "flag {flag}"); - } -} - -/// An `i64` parameter crosses as an `i64`, full width. -#[test] -fn trace_num_carries_a_full_width_i64() { - for number in [0, 1, -1, i64::MAX, i64::MIN] { - let host = FakeHost::new(); - let wat = module( - &[import::TRACE_NUM, ONE_PAGE], - &format!("(call $trace_num (i32.const 0) (i32.const 0) (i64.const {number}))"), - ); - assert_eq!(status(&wat, &host), 0); - assert_eq!( - host.traces(), - vec![Trace::Number { - msg: String::new(), - number, - }] + assert_eq!(status(&wat, &host), COMPLETED, "code {code}"); + assert!( + host.traces().is_empty(), + "code {code}: the host is not called" ); } } @@ -198,17 +208,48 @@ fn a_message_that_is_not_utf8_is_refused() { let host = FakeHost::new(); let wat = module( - &[ - import::TRACE_NUM, - ONE_PAGE, - r#"(data (i32.const 0) "\ff\fe")"#, - ], - "(call $trace_num (i32.const 0) (i32.const 2) (i64.const 0))", + &[import::TRACE, ONE_PAGE, r#"(data (i32.const 0) "\ff\fe")"#], + &traced( + TraceDataType::AsText, + "(i32.const 0) (i32.const 2)", + EMPTY_REGION, + ), ); - assert_eq!(status(&wat, &host), code(HostError::Decoding)); + assert_eq!(status(&wat, &host), COMPLETED); assert!(host.traces().is_empty(), "the host must not be called"); } +/// The error a host with no result to report may still return: a soft one is the +/// engine's to drop, since there is nowhere to put it and the contract asked +/// nothing. +#[test] +fn a_soft_error_from_a_call_with_no_result_is_dropped() { + let host = FakeHost::new().failing_trace(HostError::InvalidParams); + + let wat = module( + &[import::TRACE, ONE_PAGE], + &traced(TraceDataType::AsText, EMPTY_REGION, EMPTY_REGION), + ); + assert_eq!(status(&wat, &host), COMPLETED); + assert_eq!(host.traces().len(), 1, "the host was called and failed"); +} + +/// A host-fatal error is not an answer to the call, so having no answer to give +/// 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 wat = module( + &[import::TRACE, ONE_PAGE], + &traced(TraceDataType::AsText, EMPTY_REGION, EMPTY_REGION), + ); + assert!( + matches!(failure(&wat, &host).error, RunError::Internal), + "a fatal host error must stop the run" + ); +} + /// Several host calls in one run each see their own arguments: the two fields answer /// with distinct marker bytes and `finish` returns their sum, so a value landing in /// the wrong place gives a different total. diff --git a/crates/xrpl-wasm-vm/tests/memory_policy.rs b/crates/xrpl-wasm-vm/tests/memory_policy.rs index 3042165fdc..d8bc344ea4 100644 --- a/crates/xrpl-wasm-vm/tests/memory_policy.rs +++ b/crates/xrpl-wasm-vm/tests/memory_policy.rs @@ -4,8 +4,11 @@ mod support; -use support::{Answer, FakeHost, ONE_PAGE, code, failure, import, module, status}; -use xrpl_host_functions::{HASH_LEN, HostError}; +use support::{ + Answer, COMPLETED, EMPTY_REGION, FakeHost, ONE_PAGE, code, failure, import, module, status, + traced, +}; +use xrpl_host_functions::{HASH_LEN, HostError, TraceDataType}; use xrpl_wasm_vm::{MAX_FIELD_BYTES, RunError}; /// One page, so anything at or past 65536 is out of bounds. @@ -185,7 +188,11 @@ fn the_field_cap_precedes_the_buffer_fit_check() { } // --------------------------------------------------------------------------- -// Input regions (`read_borrowed`, via `trace`) +// Input regions (`Region::read`, via `sha512_half`) +// +// `sha512_half`'s first pair is an input region like any other, and it is the +// input the guest gets a status back from: `trace`, the other reader, answers +// nothing at all. So the codes are pinned here and the silence below. // --------------------------------------------------------------------------- /// An input region is bounds-checked the same way an output region is. Every case @@ -196,15 +203,18 @@ fn an_input_region_running_past_memory_is_refused() { for (ptr, len) in [(PAGE, 1), (PAGE - 3, 4), (PAGE - 1, CAP)] { let wat = module( - &[import::TRACE_NUM, ONE_PAGE], - &format!("(call $trace_num (i32.const {ptr}) (i32.const {len}) (i64.const 0))"), + &[import::SHA512_HALF, ONE_PAGE], + &format!( + "(call $sha512_half (i32.const {ptr}) (i32.const {len}) + (i32.const 0) (i32.const {HASH_LEN}))" + ), ); assert_eq!( status(&wat, &host), code(HostError::PointerOutOfBounds), "ptr {ptr} len {len}" ); - assert!(host.traces().is_empty(), "the host must not be called"); + assert!(host.digested.borrow().is_empty(), "the host is not called"); } } @@ -214,8 +224,11 @@ fn a_negative_input_pointer_or_length_is_refused() { for (ptr, len) in [(-1, 1), (0, -1), (i32::MIN, 1)] { let wat = module( - &[import::TRACE_NUM, ONE_PAGE], - &format!("(call $trace_num (i32.const {ptr}) (i32.const {len}) (i64.const 0))"), + &[import::SHA512_HALF, ONE_PAGE], + &format!( + "(call $sha512_half (i32.const {ptr}) (i32.const {len}) + (i32.const 0) (i32.const {HASH_LEN}))" + ), ); assert_eq!( status(&wat, &host), @@ -229,19 +242,27 @@ fn a_negative_input_pointer_or_length_is_refused() { #[test] fn an_input_past_the_field_cap_is_refused() { let host = FakeHost::new(); + let digest = |len: i64| { + module( + &[import::SHA512_HALF, ONE_PAGE], + &format!( + "(call $sha512_half (i32.const 0) (i32.const {len}) + (i32.const 2048) (i32.const {HASH_LEN}))" + ), + ) + }; - let wat = module( - &[import::TRACE_NUM, ONE_PAGE], - &format!("(call $trace_num (i32.const 0) (i32.const {OVER_CAP}) (i64.const 0))"), + assert_eq!( + status(&digest(OVER_CAP), &host), + code(HostError::DataFieldTooLarge) ); - assert_eq!(status(&wat, &host), code(HostError::DataFieldTooLarge)); - assert!(host.traces().is_empty()); + assert!(host.digested.borrow().is_empty()); - let wat = module( - &[import::TRACE_NUM, ONE_PAGE], - &format!("(call $trace_num (i32.const 0) (i32.const {CAP}) (i64.const 0))"), + assert_eq!( + status(&digest(CAP), &host), + HASH_LEN as i32, + "the cap itself is allowed" ); - assert_eq!(status(&wat, &host), 0, "the cap itself is allowed"); } /// The two directions check in opposite orders: an input's length is known before @@ -252,9 +273,10 @@ fn the_field_cap_precedes_the_bounds_check_on_an_input() { let host = FakeHost::new(); let reading = module( - &[import::TRACE_NUM, ONE_PAGE], + &[import::SHA512_HALF, ONE_PAGE], &format!( - "(call $trace_num (i32.const 0) (i32.const {}) (i64.const 0))", + "(call $sha512_half (i32.const 0) (i32.const {}) + (i32.const 0) (i32.const {HASH_LEN}))", PAGE + 1 ), ); @@ -267,30 +289,46 @@ fn the_field_cap_precedes_the_bounds_check_on_an_input() { assert_eq!(status(&writing, &host), code(HostError::PointerOutOfBounds)); } -/// `trace` reads two regions, and either one being bad refuses the call. +// --------------------------------------------------------------------------- +// The reader with no result (`read_borrowed`, via `trace`) +// --------------------------------------------------------------------------- + +/// `trace` reads two regions and either one being bad refuses the call. The same +/// rule as above, and the guest is told nothing: the refusal is the host not being +/// called, and the run carries on to the constant that follows. #[test] -fn both_of_traces_regions_are_checked() { +fn both_of_traces_regions_are_checked_silently() { let host = FakeHost::new(); - - let bad_msg = module( - &[import::TRACE, ONE_PAGE], - &format!( - "(call $trace (i32.const {PAGE}) (i32.const 1) (i32.const 0) (i32.const 1) (i32.const 0))" + let regions = [ + ( + format!("(i32.const {PAGE}) (i32.const 1)"), + EMPTY_REGION.to_owned(), ), - ); - assert_eq!(status(&bad_msg, &host), code(HostError::PointerOutOfBounds)); - - let bad_data = module( - &[import::TRACE, ONE_PAGE], - &format!( - "(call $trace (i32.const 0) (i32.const 1) (i32.const {PAGE}) (i32.const 1) (i32.const 0))" + ( + EMPTY_REGION.to_owned(), + format!("(i32.const {PAGE}) (i32.const 1)"), ), - ); - assert_eq!( - status(&bad_data, &host), - code(HostError::PointerOutOfBounds) - ); - assert!(host.traces().is_empty()); + ( + EMPTY_REGION.to_owned(), + format!("(i32.const 0) (i32.const {OVER_CAP})"), + ), + ( + "(i32.const -1) (i32.const 1)".to_owned(), + EMPTY_REGION.to_owned(), + ), + ]; + + for (msg, data) in regions { + let wat = module( + &[import::TRACE, ONE_PAGE], + &traced(TraceDataType::AsHex, &msg, &data), + ); + assert_eq!(status(&wat, &host), COMPLETED, "msg {msg} data {data}"); + assert!( + host.traces().is_empty(), + "msg {msg} data {data}: the host must not be called" + ); + } } // --------------------------------------------------------------------------- diff --git a/crates/xrpl-wasm-vm/tests/preflight.rs b/crates/xrpl-wasm-vm/tests/preflight.rs index cfa29883da..8c30390fcf 100644 --- a/crates/xrpl-wasm-vm/tests/preflight.rs +++ b/crates/xrpl-wasm-vm/tests/preflight.rs @@ -98,12 +98,11 @@ fn a_disabled_feature_does_not_pass() { /// Every host function the ABI declares, spelled as a guest imports it. The count /// is asserted against the ABI so a function added to it cannot be left out here. -const ALL_IMPORTS: [&str; 5] = [ +const ALL_IMPORTS: [&str; 4] = [ import::LDGR_INDEX, import::HOME_LE_FIELD, import::SHA512_HALF, import::TRACE, - import::TRACE_NUM, ]; #[test] diff --git a/crates/xrpl-wasm-vm/tests/support/mod.rs b/crates/xrpl-wasm-vm/tests/support/mod.rs index 80ab51395e..32b9dfe83a 100644 --- a/crates/xrpl-wasm-vm/tests/support/mod.rs +++ b/crates/xrpl-wasm-vm/tests/support/mod.rs @@ -10,7 +10,7 @@ use std::cell::RefCell; use std::collections::HashMap; -use xrpl_host_functions::{HostError, HostFunctions, HostResult}; +use xrpl_host_functions::{HostError, HostFunctions, HostResult, TraceDataType}; use xrpl_wasm_vm::{RunFailure, RunOutcome}; /// The entry point every test module exports. @@ -83,18 +83,12 @@ impl Answer { } } -/// One `trace` or `trace_num` call, as the host received it. +/// One `trace` call, as the host received it. #[derive(Clone, Debug, PartialEq, Eq)] -pub enum Trace { - Message { - msg: String, - data: Vec, - as_hex: bool, - }, - Number { - msg: String, - number: i64, - }, +pub struct Trace { + pub msg: String, + pub data_type: TraceDataType, + pub data: Vec, } /// A `HostFunctions` implementation that answers from what the test put in it and @@ -112,8 +106,12 @@ pub struct FakeHost { pub fields_asked: RefCell>, /// Every input `sha512_half` was given. pub digested: RefCell>>, - /// Every `trace`/`trace_num` call, in order. + /// Every `trace` call, in order. pub traces: RefCell>, + /// What `trace` fails with, after recording the call. `trace` has no result, + /// so this is how a test reaches what the engine does with an error it cannot + /// report. + pub trace_failure: Option, } impl Default for FakeHost { @@ -126,6 +124,7 @@ impl Default for FakeHost { fields_asked: RefCell::new(Vec::new()), digested: RefCell::new(Vec::new()), traces: RefCell::new(Vec::new()), + trace_failure: None, } } } @@ -150,6 +149,11 @@ impl FakeHost { self } + pub fn failing_trace(mut self, error: HostError) -> FakeHost { + self.trace_failure = Some(error); + self + } + pub fn traces(&self) -> Vec { self.traces.borrow().clone() } @@ -173,21 +177,18 @@ impl HostFunctions for FakeHost { self.digest.fill(out) } - fn trace(&self, msg: &str, data: &[u8], as_hex: bool) -> HostResult<()> { - self.traces.borrow_mut().push(Trace::Message { + /// Records before failing, so a test can tell a host that was called and then + /// failed from one that was never reached. + fn trace(&self, msg: &str, data: &[u8], data_type: TraceDataType) -> HostResult<()> { + self.traces.borrow_mut().push(Trace { msg: msg.to_owned(), + data_type, data: data.to_vec(), - as_hex, }); - Ok(()) - } - - fn trace_num(&self, msg: &str, number: i64) -> HostResult<()> { - self.traces.borrow_mut().push(Trace::Number { - msg: msg.to_owned(), - number, - }); - Ok(()) + match self.trace_failure { + Some(error) => Err(error), + None => Ok(()), + } } } @@ -203,15 +204,40 @@ pub mod import { r#"(import "host_lib" "ldgr_index" (func $ldgr_index (param i32 i32) (result i32)))"#; pub const HOME_LE_FIELD: &str = r#"(import "host_lib" "home_le_field" (func $home_le_field (param i32 i32 i32) (result i32)))"#; pub const SHA512_HALF: &str = r#"(import "host_lib" "sha512_half" (func $sha512_half (param i32 i32 i32 i32) (result i32)))"#; + /// No result, unlike every other import here: `trace` answers the guest nothing. pub const TRACE: &str = - r#"(import "host_lib" "trace" (func $trace (param i32 i32 i32 i32 i32) (result i32)))"#; - pub const TRACE_NUM: &str = - r#"(import "host_lib" "trace_num" (func $trace_num (param i32 i32 i64) (result i32)))"#; + r#"(import "host_lib" "trace" (func $trace (param i32 i32 i32 i32 i32)))"#; } /// One page of linear memory, exported under the name the engine looks for. pub const ONE_PAGE: &str = r#"(memory (export "memory") 1)"#; +/// What a module returns after a `trace`: the call leaves nothing on the stack, so a +/// test asserting on the run rather than on an answer asserts this. +pub const COMPLETED: i32 = 1; + +/// A `(ptr, len)` pair naming no bytes, for the half of a `trace` a test is not +/// about. +pub const EMPTY_REGION: &str = "(i32.const 0) (i32.const 0)"; + +/// A `trace` of `data` as `data_type`. `msg` and `data` are each a `(ptr, len)` +/// pair. +pub fn trace_call(data_type: TraceDataType, msg: &str, data: &str) -> String { + format!( + "(call $trace {msg} (i32.const {code}) {data})", + code = data_type.code() + ) +} + +/// [`trace_call`] as a whole module body: the call, then the constant that stands +/// in for the status it does not return. +pub fn traced(data_type: TraceDataType, msg: &str, data: &str) -> String { + format!( + "{call}\n (i32.const {COMPLETED})", + call = trace_call(data_type, msg, data) + ) +} + /// A module of `parts`, wrapping `body` in an exported `finish` returning `i32`. pub fn module(parts: &[&str], body: &str) -> String { format!( diff --git a/include/xrpl/tx/wasm/HostContext.h b/include/xrpl/tx/wasm/HostContext.h index 7f0f71f26a..7ba70e0b6e 100644 --- a/include/xrpl/tx/wasm/HostContext.h +++ b/include/xrpl/tx/wasm/HostContext.h @@ -12,6 +12,16 @@ namespace xrpl { // complete type; HostContext.cpp, compiled into libxrpl, includes the real header. class HostFunctions; +// Defined by the cxx bridge, which emits it into `xrpl_wasm_vm_ffi_cxxbridge/lib.h` from the +// declaration in `crates/xrpl-wasm-vm-ffi` - so the data types and their wire values are +// written once, in Rust, rather than kept in step with a copy here. +// +// Forward-declared for the reason `HostFunctions` above is: that generated header includes +// this one, so naming its definition here would be circular. A scoped enum with a fixed +// underlying type needs no definition to appear in a signature; `HostContext.cpp` includes +// the generated header for the `switch`. +enum class TraceDataType : std::int32_t; + // The host handed to the Rust wasm engine: one method per entry in the wasm host ABI, // each forwarding to `xrpl::HostFunctions` - the single source of truth for ledger // access - and lowering its typed `std::expected` result onto the ABI's wire form. @@ -50,12 +60,15 @@ public: [[nodiscard]] std::int32_t sha512Half(rust::Slice data, rust::Slice out) const noexcept; - // A call with no value to report answers 0, or a negative `HostFunctionError` code. - [[nodiscard]] std::int32_t - trace(rust::Str msg, rust::Slice data, bool asHex) const noexcept; - - [[nodiscard]] std::int32_t - traceNum(rust::Str msg, std::int64_t number) const noexcept; + // Renders `data` as `dataType` says, and hands the text to `HostFunctions::trace`, which + // is what puts it in this node's log. + // + // The one call that answers nothing: the guest's wasm function has no result, and this + // node's own log is the only thing a trace touches, so a buffer that does not hold what + // it claims is logged here and dropped rather than reported to a contract. + void + trace(rust::Str msg, rust::Slice data, TraceDataType dataType) + const noexcept; }; } // namespace xrpl diff --git a/src/libxrpl/tx/wasm/HostContext.cpp b/src/libxrpl/tx/wasm/HostContext.cpp index bdf22a802e..1c35789bdf 100644 --- a/src/libxrpl/tx/wasm/HostContext.cpp +++ b/src/libxrpl/tx/wasm/HostContext.cpp @@ -2,16 +2,27 @@ #include #include +#include +#include +#include #include +#include +#include #include #include #include +// For `TraceDataType`, which the bridge declares and this header defines. +#include #include #include #include +#include +#include +#include #include +#include namespace xrpl { @@ -53,6 +64,74 @@ answerScalar(rust::Slice out, T value) return answer(out, reinterpret_cast(&wire), sizeof(wire)); } +// A traced integer, which the guest sends as bytes rather than as a wasm scalar so that one +// import serves every type. `std::nullopt` if the buffer is not the width the type needs. +// +// `memcpy` regardless of alignment, and no `reinterpret_cast` fast path: a trace must cost +// the same whatever address the guest chose for its buffer. +template +std::optional +traceInt(Slice const& data) +{ + static_assert(std::is_integral_v); + if (data.size() != sizeof(T)) + return std::nullopt; + + T x; + std::memcpy(&x, data.data(), sizeof(T)); + return adjustWasmEndianess(x); +} + +// The guest's bytes as the text a log line carries, or `std::nullopt` when they do not hold +// the type they claim. +// +// The engine refuses a code that names no type before it crosses, so `type` is always one of +// the variants; the trailing `return` is what the `switch` owes a scoped enum, not a case +// this can meet. +// +// May throw: `STAmount`'s deserializer rejects malformed input that way. +std::optional +traceFormat(TraceDataType type, Slice const& data) +{ + switch (type) + { + case TraceDataType::Int64: + if (auto const x = traceInt(data)) + return std::to_string(*x); + return std::nullopt; + + case TraceDataType::Uint64: + if (auto const x = traceInt(data)) + return std::to_string(*x); + return std::nullopt; + + case TraceDataType::Xfloat: + return wasm_float::floatToString(data); + + case TraceDataType::Account: + if (data.size() != AccountID::size()) + return std::nullopt; + return toBase58(AccountID::fromVoid(data.data())); + + case TraceDataType::Amount: { + SerialIter iter(data); + STAmount const amount(iter, sfGeneric); + return amount.getFullText(); + } + + case TraceDataType::AsHex: + return strHex(data); + + case TraceDataType::AsText: + // An empty Slice has a null data(), which std::string may not be handed. + if (data.empty()) + return std::string(); + return std::string(reinterpret_cast(data.data()), data.size()); + } + + return std::nullopt; +} + } // namespace HostContext::HostContext(HostFunctions& hostFunctions) : hostFunctions_(hostFunctions) @@ -102,30 +181,43 @@ HostContext::sha512Half(rust::Slice data, rust::Slice data, bool asHex) const noexcept +void +HostContext::trace(rust::Str msg, rust::Slice data, TraceDataType dataType) + const noexcept { - return guarded(hostFunctions_.getJournal(), kHostInternal, [&] { - auto const status = hostFunctions_.trace( - std::string_view{msg.data(), msg.size()}, Slice{data.data(), data.size()}, asHex); - if (!status) - return hfErrorToInt(status.error()); + auto const journal = hostFunctions_.getJournal(); - return *status; - }); -} + // Not `guarded`: a buffer that does not hold what it claims is an ordinary contract + // mistake, so it belongs in the log the contract is writing to rather than in the error + // log as an internal failure - and it must not become one, since there is nothing to + // report it to. + try + { + if (msg.size() + data.size() > kMaxWasmDataLength) + { + JLOG(journal.trace()) << "WasmTrace: message and data too long"; + return; + } -std::int32_t -HostContext::traceNum(rust::Str msg, std::int64_t number) const noexcept -{ - return guarded(hostFunctions_.getJournal(), kHostInternal, [&] { - auto const status = - hostFunctions_.traceNum(std::string_view{msg.data(), msg.size()}, number); - if (!status) - return hfErrorToInt(status.error()); + // Rendered whatever the log level: the level decides what is written, never whether + // the host is called, so a run costs the same on every node. + auto const text = traceFormat(dataType, Slice{data.data(), data.size()}); + if (!text) + { + JLOG(journal.trace()) << "WasmTrace: data does not hold the type it names"; + return; + } - return *status; - }); + hostFunctions_.trace(std::string_view{msg.data(), msg.size()}, *text); + } + catch (std::exception const& e) + { + JLOG(journal.trace()) << "WasmTrace: threw: " << e.what(); + } + catch (...) + { + JLOG(journal.trace()) << "WasmTrace: threw"; + } } } // namespace xrpl diff --git a/src/tests/libxrpl/tx/wasm/MockHostFunctions.h b/src/tests/libxrpl/tx/wasm/MockHostFunctions.h index cde7401020..d75291cc9a 100644 --- a/src/tests/libxrpl/tx/wasm/MockHostFunctions.h +++ b/src/tests/libxrpl/tx/wasm/MockHostFunctions.h @@ -46,16 +46,12 @@ struct MockHostFunctions : HostFunctions (Slice const& data), (const, override)); + // Takes the rendered text, not the guest's buffer: rendering is `HostContext`'s, so what + // a test asserts here is the log line a node would write. MOCK_METHOD( - (std::expected), + void, trace, - (std::string_view const& msg, Slice const& data, bool asHex), - (const, override)); - - MOCK_METHOD( - (std::expected), - traceNum, - (std::string_view const& msg, std::int64_t number), + (std::string_view const& msg, std::string_view const& data), (const, override)); }; diff --git a/src/tests/libxrpl/tx/wasm/host_calls/Trace.cpp b/src/tests/libxrpl/tx/wasm/host_calls/Trace.cpp index c4606717ea..5ed645ae2b 100644 --- a/src/tests/libxrpl/tx/wasm/host_calls/Trace.cpp +++ b/src/tests/libxrpl/tx/wasm/host_calls/Trace.cpp @@ -1,62 +1,220 @@ +#include +#include +#include +#include +#include +#include #include #include #include #include #include +// For `TraceDataType`, which the bridge declares and this header defines. +#include -#include +#include +#include #include #include +#include namespace xrpl::test { -using testing::Return; +namespace { -// trace — two byte inputs and a flag, no output. +// Bytes as a WAT data segment's contents. Hex-escaped throughout, so a buffer needs no +// thought about which of its bytes the text format would otherwise read. +std::string +watBytes(Bytes const& bytes) +{ + std::string escaped; + escaped.reserve(bytes.size() * 4); + for (auto const byte : bytes) + escaped += std::format("\\{:02x}", byte); + return escaped; +} + +Bytes +serialized(STAmount const& amount) +{ + Serializer s; + amount.add(s); + return s.getData(); +} + +} // namespace + +// trace — a message, a data type, and a buffer holding what that type says. One import for +// what were five, so what a test varies is the type rather than the function. +// +// The buffer arrives as bytes and leaves as text: `HostContext` renders it, and the host is +// handed the finished line. So a test says which renderer the type selected. struct TraceCall : HostCallTest { + static constexpr std::int32_t kDataAt = 64; + + // What the guest passes. `typeCode` rather than a `TraceDataType` so a test can send a + // code that names no type, which is the guest's to get wrong. + std::int32_t typeCode{static_cast(TraceDataType::AsText)}; + Bytes data; + + void + traces(TraceDataType type, Bytes bytes) + { + typeCode = static_cast(type); + data = std::move(bytes); + } + + void + traces(TraceDataType type, std::string_view text) + { + traces(type, Bytes{text.begin(), text.end()}); + } + [[nodiscard]] std::string wat() const override { - return std::string{R"wat( + // {0} data offset, {1} the data itself, {2} the type under test, {3} its length, + // {4} a type the constant modules can name, {5} the data cap. + return std::format( + R"wat( (module - (import "host_lib" "trace" (func $trace (param i32 i32 i32 i32 i32) (result i32))) + (import "host_lib" "trace" (func $trace (param i32 i32 i32 i32 i32))) (memory (export "memory") 1) (data (i32.const 0) "note") - (data (i32.const 16) "\07\08") + (data (i32.const {0}) "{1}") (func (export "escrow_finish") (result i32) - (call $trace (i32.const 0) (i32.const 4) (i32.const 16) (i32.const 2) (i32.const 1))) + (call $trace (i32.const 0) (i32.const 4) (i32.const {2}) (i32.const {0}) (i32.const {3})) + (i32.const 1)) - (func (export "not_as_hex") (result i32) - (call $trace (i32.const 0) (i32.const 4) (i32.const 16) (i32.const 2) (i32.const 0)))) -)wat"}; + (func (export "unnamed_type") (result i32) + (call $trace (i32.const 0) (i32.const 4) (i32.const 0) (i32.const {0}) (i32.const 0)) + (i32.const 1)) + + (func (export "past_memory") (result i32) + (call $trace (i32.const 0) (i32.const 4) (i32.const {4}) (i32.const 65536) (i32.const 1)) + (i32.const 1)) + + (func (export "too_long") (result i32) + (call $trace (i32.const 0) (i32.const 4) (i32.const {4}) (i32.const {0}) (i32.const {5})) + (i32.const 1))) +)wat", + kDataAt, + watBytes(data), + typeCode, + data.size(), + static_cast(TraceDataType::AsHex), + kMaxWasmDataLength); + } + + // The line the host was handed, for a run that is expected to reach it. + void + expectTraced(std::string_view text) + { + EXPECT_CALL(host, trace(std::string_view("note"), text)); + + EXPECT_EQ(hostAnswer(), 1) << "the contract runs on past its trace"; } }; -// Two borrowed regions in one call, which is the shape a single-input helper could not -// express — so this pins that both arrive intact, and the flag with them. -TEST_F(TraceCall, MessageDataAndFlagAllArrive) +// The eight-byte types are the pair worth naming: the same bytes, and the type is the whole +// difference between the two readings. +TEST_F(TraceCall, Int64ReadsTheBufferSigned) { - EXPECT_CALL(host, trace(std::string_view("note"), BytesAre("\x07\x08"), true)) - .WillOnce(Return(0)); + traces(TraceDataType::Int64, Bytes(8, 0xff)); - EXPECT_EQ(hostAnswer(), 0) << "a call with nothing to report answers 0"; + expectTraced("-1"); } -TEST_F(TraceCall, HexFlagIsGuestsToChoose) +TEST_F(TraceCall, Uint64ReadsTheSameBufferUnsigned) { - EXPECT_CALL(host, trace(testing::_, testing::_, false)).WillOnce(Return(0)); + traces(TraceDataType::Uint64, Bytes(8, 0xff)); - EXPECT_EQ(hostAnswer("not_as_hex"), 0); + expectTraced("18446744073709551615"); } -TEST_F(TraceCall, HostErrorBecomesContractReturnValue) +TEST_F(TraceCall, AsTextTakesTheBufferVerbatim) { - EXPECT_CALL(host, trace).WillOnce(Return(std::unexpected(HostFunctionError::InvalidParams))); + traces(TraceDataType::AsText, "hello"); - EXPECT_EQ(hostAnswer(), hfErrorToInt(HostFunctionError::InvalidParams)); + expectTraced("hello"); +} + +TEST_F(TraceCall, AsHexEncodesTheBuffer) +{ + traces(TraceDataType::AsHex, Bytes{0x07, 0x08, 0xff}); + + expectTraced("0708FF"); +} + +// The zero account, so the expectation is the well-known base58 rather than a rendering of +// whatever the renderer happened to do. +TEST_F(TraceCall, AccountIsBase58) +{ + traces(TraceDataType::Account, Bytes(AccountID::size(), 0)); + + expectTraced("rrrrrrrrrrrrrrrrrrrrrhoLvTp"); +} + +TEST_F(TraceCall, AmountCarriesItsAssetIntoTheText) +{ + traces(TraceDataType::Amount, serialized(STAmount{XRPAmount{1000}})); + + expectTraced("1000/XRP"); +} + +TEST_F(TraceCall, XfloatIsDecodedToItsValue) +{ + auto const encoded = wasm_float::floatFromIntImpl( + 42, static_cast(Number::RoundingMode::ToNearest)); + ASSERT_TRUE(encoded.has_value()); + traces(TraceDataType::Xfloat, *encoded); + + expectTraced("42"); +} + +// The width is part of the type, and a buffer that is not it holds no value to print. The +// contract is not told: a trace answers nothing at all. +TEST_F(TraceCall, ABufferOfTheWrongWidthIsDropped) +{ + traces(TraceDataType::Int64, Bytes(4, 0xff)); + + EXPECT_CALL(host, trace).Times(0); + EXPECT_EQ(hostAnswer(), 1); +} + +// `STAmount`'s deserializer rejects this by throwing, which must not escape into the run. +TEST_F(TraceCall, AMalformedAmountIsDroppedRatherThanThrown) +{ + traces(TraceDataType::Amount, Bytes(3, 0xff)); + + EXPECT_CALL(host, trace).Times(0); + EXPECT_EQ(hostAnswer(), 1); +} + +// Zero is the code a guest sends by omission, which is why no type carries it. +TEST_F(TraceCall, ACodeThatNamesNoTypeIsDropped) +{ + EXPECT_CALL(host, trace).Times(0); + + EXPECT_EQ(hostAnswer("unnamed_type"), 1); +} + +// The memory policy every input region is held to, on the one call that cannot report it. +TEST_F(TraceCall, ARegionPastMemoryIsDropped) +{ + EXPECT_CALL(host, trace).Times(0); + + EXPECT_EQ(hostAnswer("past_memory"), 1); +} + +TEST_F(TraceCall, AMessageAndBufferPastTheDataCapAreDropped) +{ + EXPECT_CALL(host, trace).Times(0); + + EXPECT_EQ(hostAnswer("too_long"), 1); } } // namespace xrpl::test diff --git a/src/tests/libxrpl/tx/wasm/host_calls/TraceNum.cpp b/src/tests/libxrpl/tx/wasm/host_calls/TraceNum.cpp deleted file mode 100644 index 1e49095ff8..0000000000 --- a/src/tests/libxrpl/tx/wasm/host_calls/TraceNum.cpp +++ /dev/null @@ -1,53 +0,0 @@ -#include - -#include -#include -#include - -#include -#include -#include -#include -#include - -namespace xrpl::test { - -using testing::Return; - -// trace_num — a string and an i64, the ABI's only 64-bit parameter. -struct TraceNumCall : HostCallTest -{ - [[nodiscard]] std::string - wat() const override - { - return std::string{R"wat( -(module - (import "host_lib" "trace_num" (func $trace_num (param i32 i32 i64) (result i32))) - (memory (export "memory") 1) - (data (i32.const 0) "count") - - (func (export "escrow_finish") (result i32) - (call $trace_num (i32.const 0) (i32.const 5) (i64.const -9223372036854775808)))) -)wat"}; - } -}; - -// The extreme value on purpose: an `i64` that a truncating or sign-losing conversion anywhere -// on the wire would visibly mangle. -TEST_F(TraceNumCall, I64ArrivesWholeIncludingMostNegativeValue) -{ - EXPECT_CALL(host, traceNum(std::string_view("count"), std::numeric_limits::min())) - .WillOnce(Return(0)); - - EXPECT_EQ(hostAnswer(), 0); -} - -TEST_F(TraceNumCall, HostErrorBecomesContractReturnValue) -{ - EXPECT_CALL(host, traceNum) - .WillOnce(Return(std::unexpected(HostFunctionError::IndexOutOfBounds))); - - EXPECT_EQ(hostAnswer(), hfErrorToInt(HostFunctionError::IndexOutOfBounds)); -} - -} // namespace xrpl::test