diff --git a/crates/xrpl-host-functions-macros/src/lib.rs b/crates/xrpl-host-functions-macros/src/lib.rs index 597eb6c84e..694b4a81c2 100644 --- a/crates/xrpl-host-functions-macros/src/lib.rs +++ b/crates/xrpl-host-functions-macros/src/lib.rs @@ -25,25 +25,26 @@ use parsed_host_function::ParsedHostFunction; /// `xrpl-host-functions` as a dependency but no imports from it. /// /// ``` +/// use xrpl_host_functions::HostResult; /// use xrpl_host_functions_macros::host_functions; /// /// host_functions! { /// /// The sequence number of the ledger being built. /// #[gas = 60] /// #[wasm_name = "ldgr_index"] -/// fn get_ledger_sqn(&self) -> [u8; 4]; +/// fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; /// /// /// Writes `msg` to the trace log. /// #[gas = 500] /// #[wasm_name = "trace_num"] -/// fn trace_num(&self, msg: &str, number: i64); +/// fn trace_num(&self, msg: &str, number: i64) -> HostResult<()>; /// } /// /// // A `HostFunctions` trait, holding the declarations verbatim: /// struct Host; /// impl HostFunctions for Host { -/// fn get_ledger_sqn(&self) -> [u8; 4] { 7u32.to_le_bytes() } -/// fn trace_num(&self, _msg: &str, _number: i64) {} +/// fn get_ledger_sqn(&self) -> HostResult<[u8; 4]> { Ok(7u32.to_le_bytes()) } +/// fn trace_num(&self, _msg: &str, _number: i64) -> HostResult<()> { Ok(()) } /// } /// /// // A `HostFunctionSpec` enum carrying the ABI metadata as a `const` table: @@ -52,9 +53,10 @@ use parsed_host_function::ParsedHostFunction; /// assert_eq!(HostFunctionSpec::ALL.len(), 2); /// ``` /// -/// A declaration must be a plain `fn` taking `&self`, with no body and no -/// generics: it maps to exactly one wasm import signature. Two declarations may -/// not share a `wasm_name`, nor collapse to the same PascalCase variant. +/// A declaration must be a plain `fn` taking `&self` and returning +/// `HostResult`, with no body and no generics: it maps to exactly one wasm +/// import signature. Two declarations may not share a `wasm_name`, nor collapse to +/// the same PascalCase variant. #[proc_macro] pub fn host_functions(input: proc_macro::TokenStream) -> proc_macro::TokenStream { expand(input.into()) @@ -114,16 +116,6 @@ fn collisions(functions: &[ParsedHostFunction]) -> Vec { errors } -/// Path to the hand-written `HostFnSpec` the expansion refers to. -/// -/// Absolute, so the generated code resolves whatever the caller has imported and -/// whatever else is named `HostFnSpec` in scope. `xrpl-host-functions` declares -/// `extern crate self as xrpl_host_functions;`, which is what lets this path -/// resolve inside the crate the ABI is declared in. -pub(crate) fn host_fn_spec_path() -> TokenStream { - quote! { ::xrpl_host_functions::HostFnSpec } -} - fn generate(functions: &[ParsedHostFunction]) -> TokenStream { let trait_methods = functions.iter().map(ParsedHostFunction::trait_method); let variants = functions @@ -131,7 +123,6 @@ fn generate(functions: &[ParsedHostFunction]) -> TokenStream { .map(ParsedHostFunction::variant_declaration); let spec_arms = functions.iter().map(ParsedHostFunction::spec_arm); let all = functions.iter().map(|function| &function.variant); - let spec_type = host_fn_spec_path(); quote! { /// The host side of the wasm ABI: one method per function a guest may @@ -146,6 +137,16 @@ fn generate(functions: &[ParsedHostFunction]) -> TokenStream { #(#trait_methods)* } + /// One row of the ABI table: what [`HostFunctionSpec::wasm_name`] and + /// [`HostFunctionSpec::gas`] read from. + /// + /// Private, and the only reason it exists is to keep both of them fed + /// from a single `match` over the declarations. + struct HostFnSpec { + name: &'static str, + gas: u64, + } + /// Identifies one host function, and is the compile-time source of its /// ABI metadata. /// @@ -165,11 +166,8 @@ fn generate(functions: &[ParsedHostFunction]) -> TokenStream { /// be registered for a module that imports it to instantiate. pub const ALL: &'static [Self] = &[#(Self::#all,)*]; - /// This function's import name and base gas cost. - /// - /// Usable in `const` context, so gas tables and import lists can be - /// built at compile time. - pub const fn spec(self) -> #spec_type { + /// This function's row of the ABI table. + const fn spec(self) -> HostFnSpec { match self { #(#spec_arms,)* } @@ -178,7 +176,8 @@ fn generate(functions: &[ParsedHostFunction]) -> TokenStream { /// The name a guest imports this function under. /// /// A guest's import name must match this exactly, or the module - /// fails to instantiate. + /// fails to instantiate. Usable in `const` context, so import lists + /// can be built at compile time. pub const fn wasm_name(self) -> &'static str { self.spec().name } @@ -186,7 +185,8 @@ fn generate(functions: &[ParsedHostFunction]) -> TokenStream { /// Gas charged before the call runs, independent of its arguments. /// /// Consensus-relevant: two nodes that disagree on this value - /// disagree on transaction outcomes. + /// disagree on transaction outcomes. Usable in `const` context, so + /// gas tables can be built at compile time. pub const fn gas(self) -> u64 { self.spec().gas } @@ -221,10 +221,10 @@ mod tests { fn reports_mistakes_from_every_function() { let error = expand(quote! { #[wasm_name = "ldgr_index"] - fn get_ledger_sqn(&self) -> [u8; 4]; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; #[gas = 2000] - fn sha512_half(&self, data: &[u8]) -> [u8; 32]; + fn sha512_half(&self, data: &[u8]) -> HostResult<[u8; 32]>; }) .expect_err("expected parsing to fail"); @@ -253,49 +253,71 @@ mod tests { let generated = expand(quote! { #[gas = 60] #[wasm_name = "ldgr_index"] - fn get_ledger_sqn(&self) -> [u8; 4]; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; #[gas = 500] #[wasm_name = "trace_num"] - fn trace_num(&self, msg: &str, number: i64); + fn trace_num(&self, msg: &str, number: i64) -> HostResult<()>; }) .unwrap() .to_string(); for expected in [ "pub trait HostFunctions", - "fn get_ledger_sqn (& self) -> [u8 ; 4] ;", - "fn trace_num (& self , msg : & str , number : i64) ;", + "fn get_ledger_sqn (& self) -> HostResult < [u8 ; 4] > ;", + "fn trace_num (& self , msg : & str , number : i64) -> HostResult < () > ;", "pub enum HostFunctionSpec { GetLedgerSqn , TraceNum , }", "pub const ALL : & 'static [Self] = & [Self :: GetLedgerSqn , Self :: TraceNum ,]", - "pub const fn spec (self) -> :: xrpl_host_functions :: HostFnSpec", - "Self :: GetLedgerSqn => :: xrpl_host_functions :: HostFnSpec \ - { name : \"ldgr_index\" , gas : 60u64 }", + // The table's row type is generated too, and stays private. + "struct HostFnSpec { name : & 'static str , gas : u64 , }", + "const fn spec (self) -> HostFnSpec", + "Self :: GetLedgerSqn => HostFnSpec { name : \"ldgr_index\" , gas : 60u64 }", + "pub const fn wasm_name (self) -> & 'static str", + "pub const fn gas (self) -> u64", ] { assert!(generated.contains(expected), "missing {expected:?}"); } } - /// The expansion names the types it needs by absolute path, so it cannot pick - /// up a different `HostFnSpec` that happens to be in scope where it lands. + /// The expansion stands alone: every name in it is either generated here or + /// written in the declarations, so it cannot depend on the crate it lands in. #[test] - fn refers_to_the_declaring_crate_by_absolute_path() { + fn names_no_crate_of_its_own() { let generated = expand(quote! { #[gas = 60] #[wasm_name = "ldgr_index"] - fn get_ledger_sqn(&self) -> [u8; 4]; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; }) .unwrap() .to_string(); - assert_eq!(generated.matches("HostFnSpec").count(), 2, "{generated}"); - assert_eq!( - generated - .matches(":: xrpl_host_functions :: HostFnSpec") - .count(), - 2, - "{generated}" - ); + assert!(!generated.contains("xrpl_host_functions"), "{generated}"); + + // `Self::Variant` is the only path the expansion may build: anything else + // would reach out of the generated code. Doc comments spell paths without + // spaces (`Self::ALL`), so they do not match. + for (index, _) in generated.match_indices(" :: ") { + assert!( + generated[..index].ends_with("Self"), + "path out of the expansion at {index}: {generated}" + ); + } + } + + /// `spec` is an implementation detail of the two accessors, so it must not + /// become part of the ABI crate's public surface. + #[test] + fn keeps_the_table_row_private() { + let generated = expand(quote! { + #[gas = 60] + #[wasm_name = "ldgr_index"] + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; + }) + .unwrap() + .to_string(); + + assert!(!generated.contains("pub struct HostFnSpec"), "{generated}"); + assert!(!generated.contains("pub const fn spec"), "{generated}"); } #[test] @@ -303,11 +325,11 @@ mod tests { let messages = messages(quote! { #[gas = 60] #[wasm_name = "trace"] - fn trace(&self, msg: &str); + fn trace(&self, msg: &str) -> HostResult<()>; #[gas = 70] #[wasm_name = "trace"] - fn trace_num(&self, msg: &str, number: i64); + fn trace_num(&self, msg: &str, number: i64) -> HostResult<()>; }); assert_eq!(messages.len(), 1, "{messages:?}"); @@ -323,11 +345,11 @@ mod tests { let messages = messages(quote! { #[gas = 60] #[wasm_name = "a"] - fn get_ledger_sqn(&self) -> [u8; 4]; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; #[gas = 70] #[wasm_name = "b"] - fn get_ledger__sqn(&self) -> [u8; 4]; + fn get_ledger__sqn(&self) -> HostResult<[u8; 4]>; }); assert_eq!(messages.len(), 1, "{messages:?}"); diff --git a/crates/xrpl-host-functions-macros/src/parsed_host_function.rs b/crates/xrpl-host-functions-macros/src/parsed_host_function.rs index 44f93cb38f..813dbc5efc 100644 --- a/crates/xrpl-host-functions-macros/src/parsed_host_function.rs +++ b/crates/xrpl-host-functions-macros/src/parsed_host_function.rs @@ -1,7 +1,8 @@ use proc_macro2::TokenStream; use quote::{format_ident, quote}; use syn::{ - Attribute, Expr, ExprLit, Ident, Lit, LitStr, ReceiverKind, Safety, Signature, TraitItemFn, + Attribute, Expr, ExprLit, Ident, Lit, LitStr, PathArguments, ReceiverKind, ReturnType, Safety, + Signature, TraitItemFn, Type, TypePath, }; use crate::errors; @@ -12,6 +13,8 @@ const GAS: &str = "gas"; const WASM_NAME: &str = "wasm_name"; /// `///` desugars to `#[doc = "..."]` before macro expansion. const DOC: &str = "doc"; +/// The alias every declaration returns its success type through. +const HOST_RESULT: &str = "HostResult"; /// One entry of a `host_functions!` block: its ABI metadata and its signature. pub(crate) struct ParsedHostFunction { @@ -27,7 +30,7 @@ pub(crate) struct ParsedHostFunction { } impl ParsedHostFunction { - /// `#[doc …] fn get_ledger_sqn(&self) -> [u8; 4];` + /// `#[doc …] fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>;` pub(crate) fn trait_method(&self) -> TokenStream { let docs = &self.docs; // The declaration is already a trait method: emitted verbatim, so what @@ -50,7 +53,7 @@ impl ParsedHostFunction { } } - /// `Self::GetLedgerSqn => ::xrpl_host_functions::HostFnSpec { name: "ldgr_index", gas: 60u64 }` + /// `Self::GetLedgerSqn => HostFnSpec { name: "ldgr_index", gas: 60u64 }` pub(crate) fn spec_arm(&self) -> TokenStream { let Self { gas, @@ -58,9 +61,8 @@ impl ParsedHostFunction { variant, .. } = self; - let spec = crate::host_fn_spec_path(); quote! { - Self::#variant => #spec { name: #wasm_name, gas: #gas } + Self::#variant => HostFnSpec { name: #wasm_name, gas: #gas } } } @@ -124,6 +126,7 @@ impl ParsedHostFunction { )); } errors.extend(check_receiver(&function.sig).err()); + errors.extend(check_return_type(&function.sig).err()); if let Some(name) = &wasm_name { errors.extend(check_wasm_name(name).err()); } @@ -186,6 +189,52 @@ fn check_receiver(signature: &Signature) -> syn::Result<()> { Ok(()) } +/// Every declaration returns `HostResult`, including the ones that yield +/// nothing (`HostResult<()>`). +/// +/// One shape for every function is what lets a single dispatch adapter lower them +/// all: lift the arguments out of guest memory, call the host, then turn `Ok(T)` +/// into the wire's non-negative `i32` and `Err(e)` into a negative code or a trap. +/// A function returning a bare `T` would need its own arm. +fn check_return_type(signature: &Signature) -> syn::Result<()> { + const SHAPE: &str = "a host function must return `HostResult` — \ + `HostResult<()>` if it yields nothing"; + + let ReturnType::Type(_, returned) = &signature.output else { + return Err(syn::Error::new_spanned(&signature.ident, SHAPE)); + }; + + let Type::Path(TypePath { + qself: None, path, .. + }) = &**returned + else { + return Err(syn::Error::new_spanned(returned, SHAPE)); + }; + // The last segment only, so `HostResult` may be written qualified. + let Some(last) = path.segments.last() else { + return Err(syn::Error::new_spanned(returned, SHAPE)); + }; + if last.ident != HOST_RESULT { + return Err(syn::Error::new_spanned(returned, SHAPE)); + } + + // `HostResult` without its success type is `HostResult` the alias, which names + // no type; rustc's own message for that is unhelpfully far from the cause. + let PathArguments::AngleBracketed(arguments) = &last.arguments else { + return Err(syn::Error::new_spanned( + returned, + format!("`{HOST_RESULT}` needs its success type: `{HOST_RESULT}`"), + )); + }; + if arguments.args.len() != 1 { + return Err(syn::Error::new_spanned( + arguments, + format!("`{HOST_RESULT}` takes exactly one type: `{HOST_RESULT}`"), + )); + } + Ok(()) +} + /// `const`, `async`, `unsafe`/`safe` and `extern "…"` have no meaning in the /// wasm ABI, and would otherwise pass silently into the generated trait. fn reject_modifiers(signature: &Signature, errors: &mut Vec) { @@ -334,6 +383,7 @@ fn path_name(attr: &Attribute) -> String { #[cfg(test)] mod tests { use super::*; + use quote::ToTokens; use syn::parse_quote; /// The message of every diagnostic recorded by one failed `parse`. @@ -362,7 +412,7 @@ mod tests { let parsed = ParsedHostFunction::parse(parse_quote! { #[gas = 60] #[wasm_name = "ldgr_index"] - fn get_ledger_sqn(&self) -> [u8; 4]; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; }) .unwrap(); @@ -401,7 +451,7 @@ mod tests { let messages = messages(parse_quote! { #[gas = 60] #[wasm_name = "two_factor"] - fn _2fa(&self); + fn _2fa(&self) -> HostResult<()>; }); assert_eq!(messages.len(), 1, "{messages:?}"); @@ -433,7 +483,7 @@ mod tests { let messages = messages(parse_quote! { #[gas = -5] #[wasm_name = "ldgr_index"] - fn get_ledger_sqn(&self) -> [u8; 4]; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; }); assert_eq!(messages.len(), 1, "{messages:?}"); @@ -445,7 +495,7 @@ mod tests { let empty = messages(parse_quote! { #[gas = 60] #[wasm_name = ""] - fn get_ledger_sqn(&self) -> [u8; 4]; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; }); assert_eq!(empty.len(), 1, "{empty:?}"); assert_eq!(empty[0], "the wasm name must not be empty"); @@ -453,7 +503,7 @@ mod tests { let spaced = messages(parse_quote! { #[gas = 60] #[wasm_name = "ldgr index"] - fn get_ledger_sqn(&self) -> [u8; 4]; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; }); assert_eq!(spaced.len(), 1, "{spaced:?}"); assert!(spaced[0].contains("may only contain"), "{spaced:?}"); @@ -462,10 +512,10 @@ mod tests { #[test] fn rejects_signature_modifiers() { for declaration in [ - quote! { unsafe fn get_ledger_sqn(&self) -> [u8; 4]; }, - quote! { async fn get_ledger_sqn(&self) -> [u8; 4]; }, - quote! { const fn get_ledger_sqn(&self) -> [u8; 4]; }, - quote! { extern "C" fn get_ledger_sqn(&self) -> [u8; 4]; }, + quote! { unsafe fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; }, + quote! { async fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; }, + quote! { const fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; }, + quote! { extern "C" fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; }, ] { let function: TraitItemFn = syn::parse2(quote! { #[gas = 60] @@ -486,7 +536,7 @@ mod tests { /// Hashes `data`. #[gas = 2000] #[wasm_name = "sha512_half"] - fn sha512_half(&self, data: &[u8]) -> [u8; 32]; + fn sha512_half(&self, data: &[u8]) -> HostResult<[u8; 32]>; }) .unwrap(); @@ -498,7 +548,8 @@ mod tests { "{method}" ); assert!( - method.contains("fn sha512_half (& self , data : & [u8]) -> [u8 ; 32] ;"), + method + .contains("fn sha512_half (& self , data : & [u8]) -> HostResult < [u8 ; 32] > ;"), "{method}" ); } @@ -508,14 +559,13 @@ mod tests { let parsed = ParsedHostFunction::parse(parse_quote! { #[gas = 60] #[wasm_name = "ldgr_index"] - fn get_ledger_sqn(&self) -> [u8; 4]; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; }) .unwrap(); assert_eq!( parsed.spec_arm().to_string(), - "Self :: GetLedgerSqn => :: xrpl_host_functions :: HostFnSpec \ - { name : \"ldgr_index\" , gas : 60u64 }" + "Self :: GetLedgerSqn => HostFnSpec { name : \"ldgr_index\" , gas : 60u64 }" ); } @@ -527,7 +577,7 @@ mod tests { /// Third line. #[gas = 60] #[wasm_name = "ldgr_index"] - fn get_ledger_sqn(&self) -> [u8; 4]; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; }) .unwrap(); @@ -540,26 +590,32 @@ mod tests { let traced = ParsedHostFunction::parse(parse_quote! { #[gas = 500] #[wasm_name = "trace"] - fn trace(&self, msg: &str, data: &[u8], as_hex: bool); + fn trace(&self, msg: &str, data: &[u8], as_hex: bool) -> HostResult<()>; }) .unwrap(); // The receiver is `inputs[0]`; the three wasm parameters follow it. assert_eq!(traced.signature.inputs.len(), 4); - assert!(matches!(traced.signature.output, syn::ReturnType::Default)); + assert_eq!( + traced.signature.output.to_token_stream().to_string(), + "-> HostResult < () >" + ); let hashed = ParsedHostFunction::parse(parse_quote! { #[gas = 2000] #[wasm_name = "sha512_half"] - fn sha512_half(&self, data: &[u8]) -> [u8; 32]; + fn sha512_half(&self, data: &[u8]) -> HostResult<[u8; HASH_LEN]>; }) .unwrap(); - assert!(matches!(hashed.signature.output, syn::ReturnType::Type(..))); + assert_eq!( + hashed.signature.output.to_token_stream().to_string(), + "-> HostResult < [u8 ; HASH_LEN] >" + ); } #[test] fn reports_both_missing_attributes_at_once() { let messages = messages(parse_quote! { - fn get_ledger_sqn(&self) -> [u8; 4]; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; }); assert_eq!(messages.len(), 2); @@ -572,7 +628,7 @@ mod tests { let messages = messages(parse_quote! { #[gas = 60] #[wsam_name = "typo"] - fn get_ledger_sqn(&self) -> [u8; 4]; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; }); // The typo'd attribute, plus the `wasm_name` it failed to be. @@ -588,7 +644,7 @@ mod tests { let gas = messages(parse_quote! { #[gas = "60"] #[wasm_name = "ldgr_index"] - fn get_ledger_sqn(&self) -> [u8; 4]; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; }); assert_eq!(gas.len(), 1, "{gas:?}"); assert!( @@ -599,7 +655,7 @@ mod tests { let name = messages(parse_quote! { #[gas = 60] #[wasm_name = 7] - fn get_ledger_sqn(&self) -> [u8; 4]; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; }); assert_eq!(name.len(), 1, "{name:?}"); assert!( @@ -613,7 +669,7 @@ mod tests { let messages = messages(parse_quote! { #[gas = 99999999999999999999999] #[wasm_name = "ldgr_index"] - fn get_ledger_sqn(&self) -> [u8; 4]; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; }); assert_eq!(messages.len(), 1, "{messages:?}"); @@ -625,7 +681,7 @@ mod tests { let bare = messages(parse_quote! { #[gas] #[wasm_name = "ldgr_index"] - fn get_ledger_sqn(&self) -> [u8; 4]; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; }); assert_eq!(bare.len(), 1, "{bare:?}"); assert!(bare[0].contains("gas = ..."), "{bare:?}"); @@ -633,7 +689,7 @@ mod tests { let list = messages(parse_quote! { #[gas(60)] #[wasm_name = "ldgr_index"] - fn get_ledger_sqn(&self) -> [u8; 4]; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; }); assert_eq!(list.len(), 1, "{list:?}"); } @@ -645,7 +701,7 @@ mod tests { #[gas = 70] #[wasm_name = "ldgr_index"] #[wasm_name = "ldgr_index"] - fn get_ledger_sqn(&self) -> [u8; 4]; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; }); assert_eq!(messages.len(), 2, "{messages:?}"); @@ -662,7 +718,7 @@ mod tests { let messages = messages(parse_quote! { #[gas = "60"] #[wasm_name = 7] - fn get_ledger_sqn(&self) -> [u8; 4]; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; }); assert_eq!(messages.len(), 2, "{messages:?}"); @@ -677,7 +733,7 @@ mod tests { let messages = messages(parse_quote! { #[gas = 60] #[wasm_name = "ldgr_index"] - fn get_ledger_sqn(&self) -> [u8; 4] { [0; 4] } + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]> { Ok([0; 4]) } }); assert_eq!(messages.len(), 1, "{messages:?}"); @@ -689,7 +745,7 @@ mod tests { let parameter = messages(parse_quote! { #[gas = 60] #[wasm_name = "ldgr_index"] - fn get_ledger_sqn(&self) -> T; + fn get_ledger_sqn(&self) -> HostResult; }); assert_eq!(parameter.len(), 1, "{parameter:?}"); assert!( @@ -700,7 +756,7 @@ mod tests { let clause = messages(parse_quote! { #[gas = 60] #[wasm_name = "ldgr_index"] - fn get_ledger_sqn(&self) -> [u8; 4] where Self: Sized; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]> where Self: Sized; }); assert_eq!(clause.len(), 1, "{clause:?}"); } @@ -710,7 +766,7 @@ mod tests { let messages = messages(parse_quote! { #[gas = 60] #[wasm_name = "ldgr_index"] - fn get_ledger_sqn() -> [u8; 4]; + fn get_ledger_sqn() -> HostResult<[u8; 4]>; }); assert_eq!(messages.len(), 1, "{messages:?}"); @@ -734,7 +790,7 @@ mod tests { let function: TraitItemFn = syn::parse2(quote! { #[gas = 60] #[wasm_name = "ldgr_index"] - fn get_ledger_sqn(#receiver) -> [u8; 4]; + fn get_ledger_sqn(#receiver) -> HostResult<[u8; 4]>; }) .unwrap_or_else(|_| panic!("`{receiver}` should parse")); @@ -746,4 +802,70 @@ mod tests { ); } } + + /// A bare `T` return would need its own lowering arm, so the uniform shape is + /// required rather than inferred. + #[test] + fn rejects_returns_that_are_not_host_result() { + for output in [ + quote! {}, + quote! { -> () }, + quote! { -> [u8; 4] }, + quote! { -> i32 }, + quote! { -> Result<[u8; 4], HostError> }, + quote! { -> impl Iterator }, + ] { + let function: TraitItemFn = syn::parse2(quote! { + #[gas = 60] + #[wasm_name = "ldgr_index"] + fn get_ledger_sqn(&self) #output; + }) + .unwrap_or_else(|_| panic!("`{output}` should parse")); + + let messages = messages(function); + assert_eq!(messages.len(), 1, "`{output}`: {messages:?}"); + assert!( + messages[0].contains("must return `HostResult`"), + "`{output}`: {messages:?}" + ); + } + } + + /// `HostResult` may be written qualified, since the trait method keeps whatever + /// path resolves where the block is written. + #[test] + fn accepts_a_qualified_host_result() { + let parsed = ParsedHostFunction::parse(parse_quote! { + #[gas = 60] + #[wasm_name = "ldgr_index"] + fn get_ledger_sqn(&self) -> xrpl_host_functions::HostResult<[u8; 4]>; + }) + .unwrap(); + + assert!( + parsed + .trait_method() + .to_string() + .contains("xrpl_host_functions :: HostResult < [u8 ; 4] >"), + "{}", + parsed.trait_method() + ); + } + + /// `HostResult` with no success type names no type at all; rustc's own error + /// for that lands on the generated trait, far from the declaration. + #[test] + fn rejects_host_result_without_a_success_type() { + let messages = messages(parse_quote! { + #[gas = 60] + #[wasm_name = "ldgr_index"] + fn get_ledger_sqn(&self) -> HostResult; + }); + + assert_eq!(messages.len(), 1, "{messages:?}"); + assert!( + messages[0].contains("needs its success type"), + "{messages:?}" + ); + } } diff --git a/crates/xrpl-host-functions/src/lib.rs b/crates/xrpl-host-functions/src/lib.rs index c83e8b9c71..597b8d339e 100644 --- a/crates/xrpl-host-functions/src/lib.rs +++ b/crates/xrpl-host-functions/src/lib.rs @@ -2,9 +2,12 @@ //! //! `host_functions!` turns the declaration block at the bottom of this file into the //! [`HostFunctions`] trait a host implements and the [`HostFunctionSpec`] table a -//! wasm engine registers from. Everything the expansion refers to — [`HostFnSpec`], -//! [`HostError`] — is written by hand here, and referred to by absolute path, so the -//! generated code never depends on what a caller happens to have imported. +//! wasm engine registers from. +//! +//! The split: hand-written here is the vocabulary the declarations are written in — +//! [`HostError`], [`HostResult`], [`HASH_LEN`] — and everything derived from the +//! declarations is generated. The expansion names nothing this file does not, so the +//! two sides meet only in the block below. #![no_std] extern crate alloc; @@ -93,40 +96,24 @@ 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; -/// The wasm import name and base gas cost of one host function. -/// -/// The same for every host function, so it is declared here rather than generated; -/// [`HostFunctionSpec::spec`] returns one of these per declaration. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct HostFnSpec { - /// The name a guest imports the function under. - pub name: &'static str, - /// Gas charged before the call runs, independent of its arguments. - pub gas: u64, -} - -// Lets the generated code name this crate (`::xrpl_host_functions::HostFnSpec`) -// even though it is expanded here, inside the crate itself. -extern crate self as xrpl_host_functions; - host_functions! { #[gas = 60] #[wasm_name = "ldgr_index"] - fn get_ledger_sqn(&self) -> [u8; 4]; + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]>; #[gas = 70] #[wasm_name = "home_le_field"] - fn get_current_ledger_obj_field(&self, field: i32) -> Vec; + fn get_current_ledger_obj_field(&self, field: i32) -> HostResult>; #[gas = 2000] #[wasm_name = "sha512_half"] - fn sha512_half(&self, data: &[u8]) -> [u8; 32]; + fn sha512_half(&self, data: &[u8]) -> HostResult<[u8; HASH_LEN]>; #[gas = 500] #[wasm_name = "trace"] - fn trace(&self, msg: &str, data: &[u8], as_hex: bool); + fn trace(&self, msg: &str, data: &[u8], as_hex: bool) -> HostResult<()>; #[gas = 500] #[wasm_name = "trace_num"] - fn trace_num(&self, msg: &str, number: i64); + fn trace_num(&self, msg: &str, number: i64) -> HostResult<()>; } diff --git a/crates/xrpl-host-functions/tests/expansion_hygiene.rs b/crates/xrpl-host-functions/tests/expansion_hygiene.rs index ca665260a4..32854bfd72 100644 --- a/crates/xrpl-host-functions/tests/expansion_hygiene.rs +++ b/crates/xrpl-host-functions/tests/expansion_hygiene.rs @@ -1,32 +1,26 @@ -//! `host_functions!` must work outside the crate that declares the ABI, and must -//! not care what is in scope where it lands. +//! `host_functions!` must work outside the crate that declares the ABI: the only +//! names its expansion needs are the ones the declarations themselves spell. +use xrpl_host_functions::HostResult; use xrpl_host_functions_macros::host_functions; -/// Shadows the name the expansion refers to, while the real one is never imported -/// here. Both are inert: the generated code names the type by absolute path, and a -/// bare `HostFnSpec` in the expansion would fail to compile against this one. -struct HostFnSpec; - host_functions! { /// Answers with the number it was given. #[gas = 7] #[wasm_name = "ping"] - fn ping(&self, number: i32) -> i32; + fn ping(&self, number: i32) -> HostResult; } struct Host; impl HostFunctions for Host { - fn ping(&self, number: i32) -> i32 { - number + fn ping(&self, number: i32) -> HostResult { + Ok(number) } } #[test] -fn the_expansion_ignores_a_conflicting_local_type() { - let _decoy = HostFnSpec; - +fn the_generated_table_stands_on_its_own() { assert_eq!(HostFunctionSpec::ALL.len(), 1); assert_eq!(HostFunctionSpec::Ping.wasm_name(), "ping"); assert_eq!(HostFunctionSpec::Ping.gas(), 7); @@ -36,5 +30,5 @@ fn the_expansion_ignores_a_conflicting_local_type() { /// declaring the ABI in a library at all. #[test] fn the_generated_trait_is_implementable_here() { - assert_eq!(Host.ping(3), 3); + assert_eq!(Host.ping(3), Ok(3)); } diff --git a/crates/xrpl-host-functions/tests/generated_abi.rs b/crates/xrpl-host-functions/tests/generated_abi.rs index 5e0bf564ca..18bff71626 100644 --- a/crates/xrpl-host-functions/tests/generated_abi.rs +++ b/crates/xrpl-host-functions/tests/generated_abi.rs @@ -3,7 +3,7 @@ use std::cell::RefCell; -use xrpl_host_functions::{HASH_LEN, HostFnSpec, HostFunctionSpec, HostFunctions}; +use xrpl_host_functions::{HASH_LEN, HostError, HostFunctionSpec, HostFunctions, HostResult}; /// Records what it was asked to do; enough to prove the trait is usable. /// @@ -15,28 +15,34 @@ struct FakeHost { } impl HostFunctions for FakeHost { - fn get_ledger_sqn(&self) -> [u8; 4] { - 7u32.to_le_bytes() + fn get_ledger_sqn(&self) -> HostResult<[u8; 4]> { + Ok(7u32.to_le_bytes()) } - fn get_current_ledger_obj_field(&self, field: i32) -> Vec { - vec![field as u8] + /// Fails on a field it doesn't know, so the error channel is exercised too. + fn get_current_ledger_obj_field(&self, field: i32) -> HostResult> { + if field < 0 { + return Err(HostError::FieldNotFound); + } + Ok(vec![field as u8]) } - fn sha512_half(&self, data: &[u8]) -> [u8; HASH_LEN] { + fn sha512_half(&self, data: &[u8]) -> HostResult<[u8; HASH_LEN]> { let mut digest = [0; HASH_LEN]; digest[0] = data.len() as u8; - digest + Ok(digest) } - fn trace(&self, msg: &str, data: &[u8], as_hex: bool) { + fn trace(&self, msg: &str, data: &[u8], as_hex: bool) -> HostResult<()> { self.traced .borrow_mut() .push(format!("{msg}/{}/{as_hex}", data.len())); + Ok(()) } - fn trace_num(&self, msg: &str, number: i64) { + fn trace_num(&self, msg: &str, number: i64) -> HostResult<()> { self.traced.borrow_mut().push(format!("{msg}={number}")); + Ok(()) } } @@ -44,15 +50,28 @@ impl HostFunctions for FakeHost { fn the_trait_is_implementable() { let host = FakeHost::default(); - assert_eq!(host.get_ledger_sqn(), [7, 0, 0, 0]); - assert_eq!(host.get_current_ledger_obj_field(3), vec![3]); - assert_eq!(host.sha512_half(b"abc")[0], 3); - host.trace("hello", b"xy", true); - host.trace_num("count", -1); + assert_eq!(host.get_ledger_sqn(), Ok([7, 0, 0, 0])); + assert_eq!(host.get_current_ledger_obj_field(3), Ok(vec![3])); + assert_eq!(host.sha512_half(b"abc").unwrap()[0], 3); + assert_eq!(host.trace("hello", b"xy", true), Ok(())); + assert_eq!(host.trace_num("count", -1), Ok(())); assert_eq!(*host.traced.borrow(), ["hello/2/true", "count=-1"]); } +/// The error channel every declaration carries: an `Err` the VM turns into the +/// wire's negative return code. +#[test] +fn a_failing_call_reports_its_error_code() { + let host = FakeHost::default(); + + assert_eq!( + host.get_current_ledger_obj_field(-1), + Err(HostError::FieldNotFound) + ); + assert_eq!(HostError::FieldNotFound.code(), -2); +} + /// The VM reaches the host as one shared trait object held in the wasmi `Store`, /// which is what the `&self` receivers are for. #[test] @@ -60,8 +79,8 @@ fn the_trait_is_callable_through_a_shared_trait_object() { let fake = FakeHost::default(); let host: &dyn HostFunctions = &fake; - assert_eq!(host.get_ledger_sqn(), [7, 0, 0, 0]); - host.trace_num("count", 1); + assert_eq!(host.get_ledger_sqn(), Ok([7, 0, 0, 0])); + assert_eq!(host.trace_num("count", 1), Ok(())); assert_eq!(*fake.traced.borrow(), ["count=1"]); } @@ -69,13 +88,8 @@ fn the_trait_is_callable_through_a_shared_trait_object() { #[test] fn the_spec_table_matches_the_declarations() { assert_eq!(HostFunctionSpec::ALL.len(), 5); - assert_eq!( - HostFunctionSpec::GetLedgerSqn.spec(), - HostFnSpec { - name: "ldgr_index", - gas: 60 - } - ); + assert_eq!(HostFunctionSpec::GetLedgerSqn.wasm_name(), "ldgr_index"); + assert_eq!(HostFunctionSpec::GetLedgerSqn.gas(), 60); assert_eq!(HostFunctionSpec::Sha512Half.gas(), 2000); assert_eq!( HostFunctionSpec::GetCurrentLedgerObjField.wasm_name(), diff --git a/crates/xrpl-wasm-vm/src/abi.rs b/crates/xrpl-wasm-vm/src/abi.rs index e8e627e875..b49ee89138 100644 --- a/crates/xrpl-wasm-vm/src/abi.rs +++ b/crates/xrpl-wasm-vm/src/abi.rs @@ -42,7 +42,7 @@ pub(crate) fn charged( op: HostFunctionSpec, body: impl FnOnce(&mut Caller<'_, VmState<'_>>) -> HostResult, ) -> HostResult { - charge(caller, op.spec().gas)?; + charge(caller, op.gas())?; body(caller) } diff --git a/crates/xrpl-wasm-vm/src/register.rs b/crates/xrpl-wasm-vm/src/register.rs index 7d5737ab15..f1d9040031 100644 --- a/crates/xrpl-wasm-vm/src/register.rs +++ b/crates/xrpl-wasm-vm/src/register.rs @@ -29,7 +29,7 @@ pub(crate) fn register_host_functions(linker: &mut Linker>) -> Resul match op { HostFunctionSpec::GetLedgerSqn => linker.func_wrap( HOST_MODULE, - op.spec().name, + op.wasm_name(), |mut caller: Caller<'_, VmState<'_>>, out_ptr: i32, out_len: i32| -> i32 { to_wasm_i32(charged(&mut caller, HostFunctionSpec::GetLedgerSqn, |c| { // The host writes the serialized sequence number @@ -41,7 +41,7 @@ pub(crate) fn register_host_functions(linker: &mut Linker>) -> Resul ), HostFunctionSpec::GetCurrentLedgerObjField => linker.func_wrap( HOST_MODULE, - op.spec().name, + op.wasm_name(), |mut caller: Caller<'_, VmState<'_>>, field: i32, out_ptr: i32, @@ -63,7 +63,7 @@ pub(crate) fn register_host_functions(linker: &mut Linker>) -> Resul ), HostFunctionSpec::Sha512Half => linker.func_wrap( HOST_MODULE, - op.spec().name, + op.wasm_name(), |mut caller: Caller<'_, VmState<'_>>, data_ptr: i32, data_len: i32, @@ -87,7 +87,7 @@ pub(crate) fn register_host_functions(linker: &mut Linker>) -> Resul ), HostFunctionSpec::Trace => linker.func_wrap( HOST_MODULE, - op.spec().name, + op.wasm_name(), |mut caller: Caller<'_, VmState<'_>>, msg_ptr: i32, msg_len: i32, @@ -110,7 +110,7 @@ pub(crate) fn register_host_functions(linker: &mut Linker>) -> Resul ), HostFunctionSpec::TraceNum => linker.func_wrap( HOST_MODULE, - op.spec().name, + op.wasm_name(), |mut caller: Caller<'_, VmState<'_>>, msg_ptr: i32, msg_len: i32, diff --git a/docs/claude/redesign_impl.md b/docs/claude/redesign_impl.md index 22dcef3702..5dc0f6be33 100644 --- a/docs/claude/redesign_impl.md +++ b/docs/claude/redesign_impl.md @@ -35,18 +35,23 @@ only for reference. Anything we need about the old semantics is recoverable with re-exported — the ABI has one declaration site, so nothing outside `xrpl-host-functions` should be invoking it. - **Convention: the macro emits what varies per declaration; the facade - hand-writes the invariants and the macro refers to them by absolute path.** - So `HostFunctions`, `HostFunctionSpec` and its spec table are generated, while - `HostError`, `HostResult` and `HostFnSpec` are hand-written (greppable, - documented, testable, one rustdoc page). `host_fn_spec_path()` in the macro is - the single place that path is spelled; `extern crate self as - xrpl_host_functions;` in the facade is what makes it resolve inside the crate - the ABI is declared in. Never emit a bare type name — an absolute path is what - keeps the expansion independent of what the call site imported. + **Convention: the expansion is closed.** Every name in it is either generated + or written in the declarations — `Self::Variant` is the only path it builds, and + a test (`names_no_crate_of_its_own`) enforces that. So the macro owns + `HostFunctions`, `HostFunctionSpec`, `ALL`, `wasm_name()`, `gas()`, and the + private `HostFnSpec` row type that keeps both accessors fed from one `match`. + The facade hand-writes only the *vocabulary the declarations are written in* — + `HostError` (23 codes plus `from_code`, which wants to stay greppable and + testable), `HostResult`, `HASH_LEN`. Those resolve at the call site because the + declarations name them, exactly like `Vec` and `&[u8]`; the macro never + emits them. - The macro crate dev-depends on the facade so its doctest compiles; cargo allows - that cycle because dev-dependencies sit outside the library build graph. + Corollary: `HostFnSpec` and `spec()` are **private** to the ABI crate. Read the + table through `HostFunctionSpec::wasm_name()` / `::gas()`. + + The macro crate dev-depends on the facade so its doctest — whose declarations + name `HostResult` — compiles; cargo allows that cycle because dev-dependencies + sit outside the library build graph. - `crates/xrpl-wasm-vm/` — the wasmi wrapper: `vm.rs` (engine/store/run), `abi.rs` (gas + transfer-limit + guest-memory marshaling), `register.rs` (hand-written `Linker::func_wrap` per host function). @@ -155,18 +160,22 @@ params: i64 -> i64 &[u8], &str -> i32 ptr, i32 len const uint8_t*, int32_t -returns: - [u8; N], Vec -> appends i32 out_ptr, i32 out_len; result i32 = bytes written - i32, bool -> no out params; result i32 = the value - () -> no out params; result i32 = 0 +returns, always `HostResult`; `Err(e)` -> negative code, or a trap when host-fatal: + HostResult<[u8; N]> -> appends i32 out_ptr, i32 out_len; result i32 = bytes written + HostResult> -> same + HostResult, -> no out params; result i32 = the value + HostResult<()> -> no out params; result i32 = 0 ``` Total and unambiguous. **The macro must reject any type not in this table** — that is `WasmImpArgs`' `static_assert`, restored, and it is what guarantees the C API is always surfaceable. -**Validation** — all five current declarations (`xrpl-host-functions/src/lib.rs:87-107`) -lower to exactly the deleted C++ `_proto` aliases: +**Validation** — all five current declarations (the `host_functions!` block at the +bottom of `xrpl-host-functions/src/lib.rs`) lower to exactly the deleted C++ `_proto` +aliases. Abbreviated below: each real declaration reads +`fn f(&self, …) -> HostResult`, and neither the receiver nor the `HostResult` +wrapper contributes a C parameter. | Declaration | Derived C | C++ `_proto` | |---|---|---| @@ -359,17 +368,20 @@ useful for comparison and for the gas assertions in `Wasm_test.cpp` — not gosp ## Current state (2026-07-29) -`crates/` does **not** compile: the macro-generated trait (value-returning, -infallible) and the VM code (fill-caller's-buffer, `HostResult`) are two -different ABI shapes. The 8 errors are all in `xrpl-wasm-vm` — every byte-returning -call site in `register.rs`, plus `?` on the infallible `trace`/`trace_num`. +The trait is settled and declared: `&self`, one method per declaration, uniform +`HostResult` returns, all three checked by the macro. `xrpl-host-functions` and +`xrpl-host-functions-macros` are green (`cargo test` + `clippy` + `fmt`): 32 macro +tests, 8 facade tests, 1 doctest. -`xrpl-host-functions` and `xrpl-host-functions-macros` are green (`cargo test` + -`clippy`): 28 macro tests, 5 ABI tests, 1 doctest. +`crates/` as a whole still does **not** compile: the VM's marshaling is the other +ABI shape (fill the caller's buffer, `HostResult`). All 6 errors are in +`xrpl-wasm-vm/src/register.rs`, at the three byte-returning call sites — `write_into` +and `read_write` pass an `&mut [u8]` the trait no longer takes and expect a +`HostResult` the trait no longer returns. -Resolving that shape is the immediate work. Per the mechanism note above, the -value-returning direction (plus `HostResult`) is the one that composes with -typed/generated registration; the fill-the-guest-buffer shape is what fights it. +Rewriting `abi.rs` to lift → call → lower against the value-returning trait (host +writes into a host-side scratch buffer, one copy into guest memory afterwards, gas +and transfer limit counted there) is the immediate work. Deferred to a later refactor, once there is working code: macro-emitted `link_*` shims, the generated C header, and the probe-module conformance test.