diff --git a/crates/Cargo.lock b/crates/Cargo.lock index 8654aa6577..20affc58f0 100644 --- a/crates/Cargo.lock +++ b/crates/Cargo.lock @@ -468,6 +468,7 @@ dependencies = [ "proc-macro2", "quote", "syn 3.0.3", + "xrpl-host-functions", ] [[package]] diff --git a/crates/xrpl-host-functions-macros/Cargo.toml b/crates/xrpl-host-functions-macros/Cargo.toml index e5efeda8ea..5b5548bec7 100644 --- a/crates/xrpl-host-functions-macros/Cargo.toml +++ b/crates/xrpl-host-functions-macros/Cargo.toml @@ -10,3 +10,9 @@ proc-macro = true syn = { version = "3", features = ["full"] } quote = "1" proc-macro2 = "1" + +# The expansion names `::xrpl_host_functions::HostFnSpec`, so the doctest needs the +# facade crate. Cargo allows this cycle because dev-dependencies are outside the +# library build graph. +[dev-dependencies] +xrpl-host-functions.path = "../xrpl-host-functions" diff --git a/crates/xrpl-host-functions-macros/src/lib.rs b/crates/xrpl-host-functions-macros/src/lib.rs index 738a84d8ea..597eb6c84e 100644 --- a/crates/xrpl-host-functions-macros/src/lib.rs +++ b/crates/xrpl-host-functions-macros/src/lib.rs @@ -19,6 +19,11 @@ use parsed_host_function::ParsedHostFunction; /// charges before the call and the name the guest imports it under. Doc comments /// are kept and appear on the generated items. /// +/// This crate is an implementation detail of `xrpl-host-functions`, which +/// hand-writes the types the expansion refers to and holds the one declaration +/// block. The expansion names those types by absolute path, so a call site needs +/// `xrpl-host-functions` as a dependency but no imports from it. +/// /// ``` /// use xrpl_host_functions_macros::host_functions; /// @@ -109,6 +114,16 @@ 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 @@ -116,6 +131,7 @@ 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 @@ -130,18 +146,6 @@ fn generate(functions: &[ParsedHostFunction]) -> TokenStream { #(#trait_methods)* } - /// The wasm import name and base gas cost of one host function. - /// - /// Declared by `host_functions!`, and obtained from - /// [`HostFunctionSpec::spec`]. - #[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, - } - /// Identifies one host function, and is the compile-time source of its /// ABI metadata. /// @@ -165,7 +169,7 @@ fn generate(functions: &[ParsedHostFunction]) -> TokenStream { /// /// Usable in `const` context, so gas tables and import lists can be /// built at compile time. - pub const fn spec(self) -> HostFnSpec { + pub const fn spec(self) -> #spec_type { match self { #(#spec_arms,)* } @@ -262,18 +266,38 @@ mod tests { "pub trait HostFunctions", "fn get_ledger_sqn (& self) -> [u8 ; 4] ;", "fn trace_num (& self , msg : & str , number : i64) ;", - "pub struct HostFnSpec", - "pub name : & 'static str", - "pub gas : u64", "pub enum HostFunctionSpec { GetLedgerSqn , TraceNum , }", "pub const ALL : & 'static [Self] = & [Self :: GetLedgerSqn , Self :: TraceNum ,]", - "pub const fn spec (self) -> HostFnSpec", - "Self :: GetLedgerSqn => HostFnSpec { name : \"ldgr_index\" , gas : 60u64 }", + "pub const fn spec (self) -> :: xrpl_host_functions :: HostFnSpec", + "Self :: GetLedgerSqn => :: xrpl_host_functions :: HostFnSpec \ + { name : \"ldgr_index\" , gas : 60u64 }", ] { 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. + #[test] + fn refers_to_the_declaring_crate_by_absolute_path() { + let generated = expand(quote! { + #[gas = 60] + #[wasm_name = "ldgr_index"] + fn get_ledger_sqn(&self) -> [u8; 4]; + }) + .unwrap() + .to_string(); + + assert_eq!(generated.matches("HostFnSpec").count(), 2, "{generated}"); + assert_eq!( + generated + .matches(":: xrpl_host_functions :: HostFnSpec") + .count(), + 2, + "{generated}" + ); + } + #[test] fn rejects_two_functions_that_share_a_wasm_name() { let messages = messages(quote! { 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 de50141ce5..44f93cb38f 100644 --- a/crates/xrpl-host-functions-macros/src/parsed_host_function.rs +++ b/crates/xrpl-host-functions-macros/src/parsed_host_function.rs @@ -50,7 +50,7 @@ impl ParsedHostFunction { } } - /// `Self::GetLedgerSqn => HostFnSpec { name: "ldgr_index", gas: 60u64 }` + /// `Self::GetLedgerSqn => ::xrpl_host_functions::HostFnSpec { name: "ldgr_index", gas: 60u64 }` pub(crate) fn spec_arm(&self) -> TokenStream { let Self { gas, @@ -58,8 +58,9 @@ impl ParsedHostFunction { variant, .. } = self; + let spec = crate::host_fn_spec_path(); quote! { - Self::#variant => HostFnSpec { name: #wasm_name, gas: #gas } + Self::#variant => #spec { name: #wasm_name, gas: #gas } } } @@ -513,7 +514,8 @@ mod tests { assert_eq!( parsed.spec_arm().to_string(), - "Self :: GetLedgerSqn => HostFnSpec { name : \"ldgr_index\" , gas : 60u64 }" + "Self :: GetLedgerSqn => :: xrpl_host_functions :: HostFnSpec \ + { name : \"ldgr_index\" , gas : 60u64 }" ); } diff --git a/crates/xrpl-host-functions/src/lib.rs b/crates/xrpl-host-functions/src/lib.rs index e123bf04fc..c83e8b9c71 100644 --- a/crates/xrpl-host-functions/src/lib.rs +++ b/crates/xrpl-host-functions/src/lib.rs @@ -1,8 +1,17 @@ +//! The wasm host ABI: the one place it is declared. +//! +//! `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. + #![no_std] extern crate alloc; use alloc::vec::Vec; +// Not re-exported: the ABI is declared once, here, and this is the only call site. use xrpl_host_functions_macros::host_functions; /// Error codes a host function may return. @@ -84,6 +93,22 @@ 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"] diff --git a/crates/xrpl-host-functions/tests/expansion_hygiene.rs b/crates/xrpl-host-functions/tests/expansion_hygiene.rs new file mode 100644 index 0000000000..ca665260a4 --- /dev/null +++ b/crates/xrpl-host-functions/tests/expansion_hygiene.rs @@ -0,0 +1,40 @@ +//! `host_functions!` must work outside the crate that declares the ABI, and must +//! not care what is in scope where it lands. + +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; +} + +struct Host; + +impl HostFunctions for Host { + fn ping(&self, number: i32) -> i32 { + number + } +} + +#[test] +fn the_expansion_ignores_a_conflicting_local_type() { + let _decoy = HostFnSpec; + + assert_eq!(HostFunctionSpec::ALL.len(), 1); + assert_eq!(HostFunctionSpec::Ping.wasm_name(), "ping"); + assert_eq!(HostFunctionSpec::Ping.gas(), 7); +} + +/// The generated trait is implementable from another crate, which is the point of +/// declaring the ABI in a library at all. +#[test] +fn the_generated_trait_is_implementable_here() { + assert_eq!(Host.ping(3), 3); +} diff --git a/docs/claude/redesign_impl.md b/docs/claude/redesign_impl.md index 9d44506154..22dcef3702 100644 --- a/docs/claude/redesign_impl.md +++ b/docs/claude/redesign_impl.md @@ -29,7 +29,24 @@ only for reference. Anything we need about the old semantics is recoverable with a declaration reads exactly as the trait method it becomes. `&self` is what lets the VM hold the host as one shared `&dyn HostFunctions` in the wasmi `Store`; a host that needs to mutate uses interior mutability. - - `crates/xrpl-host-functions-macros/` — the `host_functions!` proc macro. + - `crates/xrpl-host-functions-macros/` — the `host_functions!` proc macro. An + implementation detail of the crate above: the dependency arrow runs facade → + macro, and the macro depends on nothing but syn/quote. It is deliberately *not* + 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. + + 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. - `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).