diff --git a/CONVENTIONS.md b/CONVENTIONS.md index 3c412625246..c4bc12748fd 100644 --- a/CONVENTIONS.md +++ b/CONVENTIONS.md @@ -435,8 +435,57 @@ This keeps the TypeScript side clean (no serialization helpers) while handling t --- +## 10. Use structured errors across WASM boundaries + +**What:** New public wasm-utxo APIs must return `Result` from the WASM binding layer. Use a domain-specific error enum for operations with multiple failure modes, then add it as a typed `WasmUtxoError` variant. + +Leaf error enums should derive `strum::IntoStaticStr`, implement `Display` and `std::error::Error`, and invoke `crate::impl_wasm_error_code!`. The resulting stable code is exposed to JavaScript as `err.code` while the display text remains human-readable. + +**Why:** `WasmUtxoError` is converted into a branded JavaScript `Error` with a stable, branchable code. Returning `String`, `JsValue`, or a new `WasmUtxoError::StringError` from a new public API discards the failure category and forces callers to parse message text. + +**Good:** + +```rust +#[derive(Debug, strum::IntoStaticStr)] +pub enum SignatureVerificationError { + InputIndexOutOfBounds { index: usize }, + MissingSighashContext, +} + +impl std::fmt::Display for SignatureVerificationError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::InputIndexOutOfBounds { index } => write!(f, "input index {index} out of bounds"), + Self::MissingSighashContext => write!(f, "signature sighash context is missing"), + } + } +} + +impl std::error::Error for SignatureVerificationError {} +crate::impl_wasm_error_code!(SignatureVerificationError); + +// Add `SignatureVerification(SignatureVerificationError)` and its `From` impl +// to `WasmUtxoError`, then propagate it from the `src/wasm/` wrapper. +``` + +**Bad:** + +```rust +// Loses the error category and creates only WasmUtxoError.StringError in JS. +pub fn verify_signature(...) -> Result; + +// Do not make JavaScript callers parse error messages. +WasmUtxoError::new(&format!("verification failed: {error}")) +``` + +Keep the result contract explicit: `Ok(false)` means verification was evaluated and the signature is absent or invalid; `Err(WasmUtxoError)` means the operation could not be evaluated because the input, context, or computation was invalid. Existing string-returning APIs are legacy compatibility surfaces and should not be extended by new public methods. + +**See:** `packages/wasm-utxo/src/error.rs`, `packages/wasm-utxo/src/wasm/try_into_js_value.rs`, `packages/wasm-utxo/src/zcash/v6.rs`, `packages/wasm-utxo/src/zcash/ironwood_build.rs` + +--- + ## Summary -These 9 conventions define how BitGoWasm packages structure their APIs. They're architectural patterns enforced in code reviews — not general software practices or build requirements. +These 10 conventions define how BitGoWasm packages structure their APIs. They're architectural patterns enforced in code reviews — not general software practices or build requirements. When in doubt, look at wasm-solana and wasm-utxo — they're the reference implementations. Following these patterns from the start prevents review churn and keeps all packages consistent. diff --git a/packages/wasm-utxo/README.md b/packages/wasm-utxo/README.md index 7ddf0c77469..4084fcfa8de 100644 --- a/packages/wasm-utxo/README.md +++ b/packages/wasm-utxo/README.md @@ -7,6 +7,7 @@ that help verify and co-sign transactions built by the BitGo Wallet Platform API ## Documentation +- **[`../../CONVENTIONS.md`](../../CONVENTIONS.md)** - Repository-wide API and structured-error conventions - **[`src/wasm-bindgen.md`](src/wasm-bindgen.md)** - Guide for creating WASM bindings using the namespace pattern - **[`js/README.md`](js/README.md)** - TypeScript wrapper layer architecture and best practices - **[`cli/README.md`](cli/README.md)** - Command-line interface for address and PSBT operations diff --git a/packages/wasm-utxo/js/README.md b/packages/wasm-utxo/js/README.md index 7a8e806aee8..c9dc78c3eee 100644 --- a/packages/wasm-utxo/js/README.md +++ b/packages/wasm-utxo/js/README.md @@ -33,6 +33,32 @@ This directory implements two complementary patterns to provide cleaner, more ty - Re-exports shared types and classes for top-level access - Augments WASM types with additional TypeScript declarations +### Error Handling + +The WASM boundary exposes failures as branded JavaScript `Error` objects. New Rust/WASM APIs +should return `Result` and use a domain-specific error enum for distinct failure +modes; do not return `Result` or wrap new failures with `WasmUtxoError::new(...)`. + +`WasmUtxoError.code` is the stable machine-readable value. The error message is for diagnostics and +must not be parsed by callers. Use `isWasmUtxoError()` when code needs to distinguish a wasm-utxo +failure from an unrelated exception: + +```typescript +try { + psbt.verifyIronwoodV6SignatureWithPub(inputIndex, key); +} catch (error: unknown) { + if (isWasmUtxoError(error) && error.code === "V6SignatureError.MissingSighashContext") { + // Handle a PSBT that cannot be verified in its current state. + } + throw error; +} +``` + +Use `Ok(false)` for a validly evaluated negative result, such as an absent or cryptographically +invalid signature. Return a structured error when verification cannot be evaluated because the +input, PSBT metadata, derivation, or sighash context is invalid. This keeps the public API +branchable without coupling callers to human-readable text. + ### Pattern 1: Namespace Wrapper Pattern Used for static utility functions (e.g., `address.ts`, `utxolibCompat.ts`). diff --git a/packages/wasm-utxo/src/wasm-bindgen.md b/packages/wasm-utxo/src/wasm-bindgen.md index 5d93ddeb6a6..1dc0c0f1024 100644 --- a/packages/wasm-utxo/src/wasm-bindgen.md +++ b/packages/wasm-utxo/src/wasm-bindgen.md @@ -16,7 +16,7 @@ Create empty structs with `#[wasm_bindgen]` to serve as namespaces, then impleme // wasm/address.rs use wasm_bindgen::prelude::*; -use wasm_bindgen::JsValue; +use crate::error::WasmUtxoError; use crate::address::networks::{ to_output_script_with_coin, from_output_script_with_coin_and_format, }; @@ -29,10 +29,8 @@ impl AddressNamespace { pub fn to_output_script_with_coin( address: &str, coin: &str, - ) -> Result, JsValue> { - to_output_script_with_coin(address, coin) - .map(|script| script.to_bytes()) - .map_err(|e| JsValue::from_str(&e.to_string())) + ) -> Result, WasmUtxoError> { + Ok(to_output_script_with_coin(address, coin)?.to_bytes()) } pub fn from_output_script_with_coin( @@ -70,7 +68,7 @@ pub use fixed_script_wallet::FixedScriptWalletNamespace; 6. **No `js_name` Attributes**: Do NOT use `#[wasm_bindgen(js_name = "...")]` to rename methods - keep Rust names pure and let TypeScript wrappers handle JS naming conventions -7. **Error Handling**: Return `Result` types - `wasm-bindgen` automatically converts these to JavaScript exceptions +7. **Error Handling**: Return `Result` types. `wasm-bindgen` converts the typed error into a JavaScript exception, preserving the stable `err.code` and human-readable message. New public APIs must not return `Result` or manually convert errors to `JsValue`. 8. **Separation**: WASM binding layer delegates to core implementation in domain modules (e.g., `src/address/`, `src/fixed_script_wallet/`) @@ -126,15 +124,15 @@ This layered approach gives us: Common Rust ↔ JavaScript type mappings: -| Rust | JavaScript/TypeScript | Notes | -| ------------------ | --------------------- | ------------------------------ | -| `&str`, `String` | `string` | Strings are copied | -| `&[u8]`, `Vec` | `Uint8Array` | Efficient binary data | -| `u32`, `i32`, etc. | `number` | JavaScript numbers are f64 | -| `bool` | `boolean` | | -| `Option` | `T \| undefined` | Becomes optional parameter | -| `Result` | `T` (throws on Err) | Errors become exceptions | -| Custom structs | `any` (usually) | Reason for TypeScript wrappers | +| Rust | JavaScript/TypeScript | Notes | +| ------------------ | --------------------- | ------------------------------------------------------------------ | +| `&str`, `String` | `string` | Strings are copied | +| `&[u8]`, `Vec` | `Uint8Array` | Efficient binary data | +| `u32`, `i32`, etc. | `number` | JavaScript numbers are f64 | +| `bool` | `boolean` | | +| `Option` | `T \| undefined` | Becomes optional parameter | +| `Result` | `T` (throws on Err) | `WasmUtxoError` becomes a branded exception with stable `err.code` | +| Custom structs | `any` (usually) | Reason for TypeScript wrappers | ## Best Practices @@ -148,7 +146,7 @@ Common Rust ↔ JavaScript type mappings: 5. **Thin binding layer** - WASM methods should delegate to core implementation, only handling type conversions -6. **Return `Result` types** - Let `wasm-bindgen` handle error conversion to JavaScript exceptions +6. **Return typed error results** - Return `Result` so `wasm-bindgen` exposes a branded JavaScript exception with a stable `err.code`; do not flatten new errors into strings or `JsValue` 7. **Avoid complex types in signatures** - Stick to primitives and byte arrays when possible; use `JsValue` for complex types