diff --git a/AGENTS.md b/AGENTS.md index f9d0c8f6..38c8daa3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -104,7 +104,7 @@ Consequences: - `lib.rs` uses `#![cfg_attr(target_arch = "wasm32", no_std)]` — code is `no_std` only when targeting WASM; native builds get `std` so `cargo test` works. This applies to both `xrpl-common-stdlib` and `xrpl-escrow-stdlib`. - To exercise stdlib code from another crate's tests (e.g. `e2e-tests/`, `xrpl-escrow-stdlib`), enable the `test-host-bindings` feature on `xrpl-common-stdlib` — `dev-dependencies` aren't enough because mockall must be available when the lib is consumed as a regular dep. `xrpl-stdlib-test-utils` wraps this feature and its mocks behind higher-level scenario builders; prefer it over hand-rolling mock setups in new tests. -- Anything new added to `HostBindings` must be implemented in all three files. CI's `scripts/host-function-audit.sh` compares the trait against rippled's exports — keep them in sync. +- Anything new added to `HostBindings` must be implemented in all three files. CI's `scripts/host-function-audit.sh` (`tools/compareHostFunctions.js`) compares all four files (trait + three impls) against rippled's host ABI — keep them in sync. rippled's side of that ABI is Rust, not C++: `crates/xrpl-host-functions/src/lib.rs` holds the `host_functions! { ... }` declaration block (its `#[wasm_name = "..."]` attribute is the import name our bindings must match, e.g. `ldgr_index`), and `crates/xrpl-wasm-vm/src/register.rs` hand-registers each `HostFunctionSpec::` with its real flat wasm signature. The script correlates the two by variant name (PascalCase of the declaration's fn name) and takes params/return **only** from `register.rs` — the declaration's high-level types (`&[u8]`, `i32`) don't predict wire arity (e.g. a `seq: i32` travels as a (ptr, len) region). Comparison is at the wasm-value level (`i32`/`i64`/`void`; our `*const u8`/`*mut u8`/`usize` all map to `i32`). The script fetches the live HEAD of `ripple/se/supported` with no pinned commit, so it breaks whenever upstream reshapes those two files; a genuine set difference (a function on one side only) is a real finding to report, not something to paper over in the script. ## Architecture: layering inside `xrpl-common-stdlib` diff --git a/tools/compareHostFunctions.js b/tools/compareHostFunctions.js index 30369bee..226c226d 100644 --- a/tools/compareHostFunctions.js +++ b/tools/compareHostFunctions.js @@ -10,6 +10,13 @@ if (process.argv.length !== 3) { //////////////////////////////////////////////////////////////////////// const { readFile, readSourceFile: read } = require("./rippledSource") +// rippled's WASM host ABI is declared in Rust (the `xrpl-host-functions` crate) and +// registered with the wasm engine in `xrpl-wasm-vm`. The two files are correlated by +// the `HostFunctionSpec::` identifier, which is the PascalCase form of the +// declaration's Rust function name. +const ABI_DECLARATION_FILE = "crates/xrpl-host-functions/src/lib.rs" +const ABI_REGISTRATION_FILE = "crates/xrpl-wasm-vm/src/register.rs" + function areListsEqual(a, b) { if (a.length !== b.length) return false for (let i = 0; i < a.length; i++) { @@ -20,105 +27,182 @@ function areListsEqual(a, b) { return true } -async function main() { - const wasmImportFile = await read( - process.argv[2], - "src/libxrpl/tx/wasm/WasmVM.cpp", - ) - const hostWrapperFile = await read( - process.argv[2], - "include/xrpl/tx/wasm/HostFuncWrapper.h", - ) - let importHits = [ - // Parse WASM host function imports in `WasmVM.cpp` - ...wasmImportFile.matchAll( - /^ *WASM_IMPORT_FUNC2? *\(\*?i, *([A-Za-z0-9]+), *("([A-Za-z0-9_]+)",)? *&?hfs, *[0-9']+\);$/gm, - ), - ] - console.log( - `\n📝 WasmVM.cpp: Regex matched ${importHits.length} import functions`, - ) - - const imports = importHits - .map((hit) => [hit[1], hit[3] != null ? hit[3] : hit[1]]) - .sort((a, b) => a[0].localeCompare(b[0])) - - let wrapperHits = [ - // Parse the `proto` functions in `HostFuncWrapper.h` - ...hostWrapperFile.matchAll( - /^ *using ([A-Za-z0-9]+)_proto =[ \n]*([A-Za-z0-9_]+)\(([A-Za-z0-9_\* \n,]*)\);$/gm, - ), - ] - console.log( - `📝 HostFuncWrapper.h: Regex matched ${wrapperHits.length} wrapper functions`, - ) +function snakeToPascal(name) { + return name + .split("_") + .map((part) => part.charAt(0).toUpperCase() + part.slice(1)) + .join("") +} - const wrappers = wrapperHits - .map((hit) => [ - hit[1], - hit[2], - hit[3].length == 0 ? [] : hit[3].split(",").map((s) => s.trim()), - ]) - .sort((a, b) => a[0].localeCompare(b[0])) - if ( - !areListsEqual( - imports.map((f) => f[0]), - wrappers.map((f) => f[0]), +/** + * Parses the `host_functions! { ... }` macro block in `crates/xrpl-host-functions/src/lib.rs`. + * + * Only the `#[wasm_name = "..."]` attribute is taken from here: it is the literal import + * name the guest links against. The declaration's parameter types are high-level Rust + * (`&[u8]`, `i32`, ...) and are *not* a reliable guide to the wire signature -- e.g. a + * `seq: i32` is passed on the wire as a (ptr, len) byte region -- so params and return + * type come from the registration file instead. + * + * Returns a Map keyed by `HostFunctionSpec` variant name (e.g. `GetLedgerSqn`) whose + * values are `{ rustName, wasmName }`. + */ +function parseAbiDeclarations(source) { + const blockStart = source.search(/^host_functions!\s*\{/m) + if (blockStart < 0) { + console.error( + `Could not find the host_functions! block in ${ABI_DECLARATION_FILE}`, ) - ) { - console.error("Imports and C++ Host Functions do not match!") - const importsMissing = wrappers.filter( - (f) => !imports.some((func) => func[0] === f[0]), - ) - const hfMissing = imports.filter( - (f) => !wrappers.some((func) => func[0] === f[0]), + process.exit(1) + } + // The macro block is at the top level, so its closing brace is the first + // `}` at column 0 after the opening line. + const blockEnd = source.indexOf("\n}", blockStart) + if (blockEnd < 0) { + console.error( + `Could not find the end of the host_functions! block in ${ABI_DECLARATION_FILE}`, ) - if (importsMissing.length > 0) + process.exit(1) + } + const block = source + .slice(blockStart, blockEnd) + // Strip comments so `fn`/`#[...]` inside prose can't confuse the parser. + .replace(/^\s*\/\/.*$/gm, "") + + const declarations = new Map() + const declRegex = + /((?:\s*#\[[^\]]*\])+)\s*fn\s+([a-z0-9_]+)\s*\(([^)]*)\)\s*->\s*HostResult<([^>]*)>\s*;/g + for (const hit of block.matchAll(declRegex)) { + const [, attrs, rustName] = hit + const wasmNameMatch = attrs.match(/#\[wasm_name\s*=\s*"([A-Za-z0-9_]+)"\]/) + if (!wasmNameMatch) { console.error( - "Missing Imports:", - "\x1b[31m" + - importsMissing.map((func) => func[0]).join(", ") + - "\x1b[0m", + `${ABI_DECLARATION_FILE}: fn ${rustName} has no #[wasm_name = "..."] attribute`, ) - if (hfMissing.length > 0) + process.exit(1) + } + + const variant = snakeToPascal(rustName) + if (declarations.has(variant)) { console.error( - "Missing C++ Host Functions:", - "\x1b[31m" + hfMissing.map((func) => func[0]).join(", ") + "\x1b[0m", + `${ABI_DECLARATION_FILE}: duplicate host function declaration ${rustName}`, ) - process.exit(1) + process.exit(1) + } + declarations.set(variant, { rustName, wasmName: wasmNameMatch[1] }) } + return declarations +} - for (let i = 0; i < imports.length; i++) { - if (imports[i][0] !== wrappers[i][0]) { +/** + * Parses the `match op { HostFunctionSpec::X => linker.func_wrap(...) }` arms in + * `crates/xrpl-wasm-vm/src/register.rs`. Each arm's closure header is the real + * wasm-level import signature: flat `i32`/`i64` params (after the `caller`) and a + * `Result` or `Result<(), _>` return. + * + * Returns a Map keyed by `HostFunctionSpec` variant name whose values are + * `{ params: string[], return: string }` using wasm value types (`i32`, `i64`, `void`). + */ +function parseAbiRegistrations(source) { + const armRegex = + /HostFunctionSpec::([A-Za-z0-9]+)\s*=>\s*linker\.func_wrap\(/g + const armCount = [...source.matchAll(armRegex)].length + + const closureRegex = + /HostFunctionSpec::([A-Za-z0-9]+)\s*=>\s*linker\.func_wrap\(\s*HOST_MODULE,\s*op\.wasm_name\(\),\s*\|\s*mut caller:\s*Caller<'_,\s*VmState<'_>>((?:\s*,\s*[a-z0-9_]+\s*:\s*[a-z0-9]+)*)\s*\|\s*->\s*Result<(\(\)|[a-z0-9]+),\s*wasmi::Error>/g + const registrations = new Map() + for (const hit of source.matchAll(closureRegex)) { + const [, variant, rawParams, rawReturn] = hit + const params = rawParams + .split(",") + .map((s) => s.trim()) + .filter((s) => s.length > 0) + .map((s) => s.slice(s.indexOf(":") + 1).trim()) + if (registrations.has(variant)) { console.error( - "Imports and Host Functions do not match at index " + - i + - ": " + - imports[i][0] + - " !== " + - wrappers[i][0], + `${ABI_REGISTRATION_FILE}: HostFunctionSpec::${variant} is registered twice`, ) process.exit(1) } + registrations.set(variant, { + params, + return: rawReturn === "()" ? "void" : rawReturn, + }) } - const cppHostFunctions = imports - .map((hit, i) => { + if (registrations.size !== armCount) { + console.error( + `${ABI_REGISTRATION_FILE}: found ${armCount} HostFunctionSpec arms but could only parse ${registrations.size} closure signatures; the file's shape has changed and this parser needs updating`, + ) + process.exit(1) + } + return registrations +} + +async function main() { + const abiDeclarationFile = await read(process.argv[2], ABI_DECLARATION_FILE) + const abiRegistrationFile = await read(process.argv[2], ABI_REGISTRATION_FILE) + + const declarations = parseAbiDeclarations(abiDeclarationFile) + console.log( + `\n📝 ${ABI_DECLARATION_FILE}: parsed ${declarations.size} host function declarations`, + ) + const registrations = parseAbiRegistrations(abiRegistrationFile) + console.log( + `📝 ${ABI_REGISTRATION_FILE}: parsed ${registrations.size} host function registrations`, + ) + + const declaredVariants = [...declarations.keys()].sort() + const registeredVariants = [...registrations.keys()].sort() + if (!areListsEqual(declaredVariants, registeredVariants)) { + console.error( + "rippled's host function declarations and registrations do not match!", + ) + const unregistered = declaredVariants.filter((v) => !registrations.has(v)) + const undeclared = registeredVariants.filter((v) => !declarations.has(v)) + if (unregistered.length > 0) + console.error( + `Declared in ${ABI_DECLARATION_FILE} but not registered:`, + "\x1b[31m" + unregistered.join(", ") + "\x1b[0m", + ) + if (undeclared.length > 0) + console.error( + `Registered in ${ABI_REGISTRATION_FILE} but not declared:`, + "\x1b[31m" + undeclared.join(", ") + "\x1b[0m", + ) + process.exit(1) + } + + const rippledHostFunctions = declaredVariants + .map((variant) => { + const reg = registrations.get(variant) return { - name: hit[1], - return: wrappers[i][1], - params: wrappers[i][2], + name: declarations.get(variant).wasmName, + return: reg.return, + params: reg.params, } }) .sort((a, b) => a.name.localeCompare(b.name)) + const duplicateWasmNames = rippledHostFunctions.filter( + (f, i) => i > 0 && rippledHostFunctions[i - 1].name === f.name, + ) + if (duplicateWasmNames.length > 0) { + console.error( + "rippled declares the same wasm_name more than once:", + "\x1b[31m" + duplicateWasmNames.map((f) => f.name).join(", ") + "\x1b[0m", + ) + process.exit(1) + } + + // Our FFI bindings are compared at the wasm-value level, which is what actually has + // to agree for the import to link: pointers and lengths are both `i32` on wasm32. const paramTranslation = { - i32: "int32_t", - u32: "uint32_t", - usize: "int32_t", - i64: "int64_t", - "*const u8": "uint8_t const*", - "*mut u8": "uint8_t*", + i32: "i32", + u32: "i32", + usize: "i32", + i64: "i64", + "*const u8": "i32", + "*mut u8": "i32", } function translateParamType(param) { @@ -130,7 +214,7 @@ async function main() { } // Fire-and-forget host functions (the trace family) have no return value. Rust spells that - // as either an omitted `->` or an explicit `-> ()`; C++ spells it `void`. + // as either an omitted `->` or an explicit `-> ()`; rippled registers them as `Result<(), _>`. function translateReturnType(ret) { if (ret === undefined || ret === "()") { return "void" @@ -139,61 +223,61 @@ async function main() { } function checkHits(fileTitle, rustHostFunctions) { - console.log(`\n🔍 Comparing ${fileTitle} with C++ host functions...`) + console.log(`\n🔍 Comparing ${fileTitle} with rippled host functions...`) console.log(` Found ${rustHostFunctions.length} Rust functions`) - console.log(` Found ${cppHostFunctions.length} C++ functions`) + console.log(` Found ${rippledHostFunctions.length} rippled functions`) if ( !areListsEqual( rustHostFunctions.map((f) => f.name), - cppHostFunctions.map((f) => f.name), + rippledHostFunctions.map((f) => f.name), ) ) { console.error( - `\n❌ ${fileTitle}: Rust Host Functions and C++ Host Functions do not match!`, + `\n❌ ${fileTitle}: Rust Host Functions and rippled Host Functions do not match!`, ) - const rustMissing = cppHostFunctions.filter( + const rustMissing = rippledHostFunctions.filter( (f) => !rustHostFunctions.some((rf) => rf.name === f.name), ) - const cppMissing = rustHostFunctions.filter( - (f) => !cppHostFunctions.some((rf) => rf.name === f.name), + const rippledMissing = rustHostFunctions.filter( + (f) => !rippledHostFunctions.some((rf) => rf.name === f.name), ) if (rustMissing.length > 0) console.error( ` Missing Rust Host Functions in ${fileTitle}:`, "\x1b[31m" + rustMissing.map((f) => f.name).join(", ") + "\x1b[0m", ) - if (cppMissing.length > 0) + if (rippledMissing.length > 0) console.error( - ` Missing C++ Host Functions (extra in ${fileTitle}):`, - "\x1b[31m" + cppMissing.map((f) => f.name).join(", ") + "\x1b[0m", + ` Missing rippled Host Functions (extra in ${fileTitle}):`, + "\x1b[31m" + rippledMissing.map((f) => f.name).join(", ") + "\x1b[0m", ) process.exit(1) } let hasError = false rustHostFunctions.forEach((hit, index) => { - const cppHit = cppHostFunctions[index] - if (hit.name !== cppHit.name) { + const rippledHit = rippledHostFunctions[index] + if (hit.name !== rippledHit.name) { console.error( - `Rust Host Function name mismatch in ${fileTitle}: ${hit.name} !== ${cppHit.name}`, + `Rust Host Function name mismatch in ${fileTitle}: ${hit.name} !== ${rippledHit.name}`, ) hasError = true - } else if (hit.return !== cppHit.return) { + } else if (hit.return !== rippledHit.return) { console.error( - `Rust Host Function return type mismatch in ${fileTitle} for ${hit.name}: ${hit.return} !== ${cppHit.return}`, + `Rust Host Function return type mismatch in ${fileTitle} for ${hit.name}: ${hit.return} !== ${rippledHit.return}`, ) hasError = true - } else if (hit.params.length !== cppHit.params.length) { + } else if (hit.params.length !== rippledHit.params.length) { console.error( - `Rust Host Function parameter count mismatch in ${fileTitle} for ${hit.name}: ${hit.params.length} !== ${cppHit.params.length} (${hit.params.join(", ")}) !== (${cppHit.params.join(", ")})`, + `Rust Host Function parameter count mismatch in ${fileTitle} for ${hit.name}: ${hit.params.length} !== ${rippledHit.params.length} (${hit.params.join(", ")}) !== (${rippledHit.params.join(", ")})`, ) hasError = true } else { hit.params.forEach((param, paramIndex) => { - if (param !== cppHit.params[paramIndex]) { + if (param !== rippledHit.params[paramIndex]) { console.error( - `Rust Host Function parameter type mismatch in ${fileTitle} for ${hit.name}, parameter ${paramIndex}: ${param} !== ${cppHit.params[paramIndex]}`, + `Rust Host Function parameter type mismatch in ${fileTitle} for ${hit.name}, parameter ${paramIndex}: ${param} !== ${rippledHit.params[paramIndex]}`, ) hasError = true } @@ -403,7 +487,9 @@ async function main() { checkHits("host_bindings_empty.rs", rustEmptyHostFunctions) } - console.log("All host functions match between Rust and C++ implementations.") + console.log( + "All host functions match between the Rust bindings and rippled's host ABI.", + ) } main()