Skip to content

Adds Rust bindgen/gloo-net based io implementation for JavaScript#1443

Open
Anunayj wants to merge 6 commits intopayjoin:masterfrom
Anunayj:io-javascript
Open

Adds Rust bindgen/gloo-net based io implementation for JavaScript#1443
Anunayj wants to merge 6 commits intopayjoin:masterfrom
Anunayj:io-javascript

Conversation

@Anunayj
Copy link
Copy Markdown

@Anunayj Anunayj commented Mar 25, 2026

Pure Rust implementation of fetch_ohttp_keys using gloo-net and (too much) rust-wasmbindgen.

The Websocket Implementation is quite straightfoward as it uses gloo-net to access the WebSocket API. However, the fetch API does not support Proxy requests, so we bind to undici and (in a quite ugly fashion) make the HTTP(S) request thru the proxy.

It would likely have been more sensible to implement these as JS stubs, but I had quite a lot of fun (actually I did not :P) implementing these. Very overkill, probably pointless but ¯\_(ツ)_/¯

Actually I am unsure if this was a cleanest way to go about this (I still think pure JS stubs would have been easier), maybe this code should instead reside in payjoin-ffi, open to opinions.

AI Disclaimer

AI (Gemini mostly) was used extensively for consulting on how to approach, or implement certain things. However the key implementation was handwritten while consulting snippets of code. Test cases were written with help of AI.

Closes: #1250

Pull Request Checklist

Please confirm the following before requesting review:

Note for Reviewers: Would recommend reviewing commit by commit. I've broken the change down into small set of changes for ease of review.

Anunayj added 6 commits March 26, 2026 01:11
I have been using RustRover for development, and it generates
and saves configurations in .idea/
Also Allow IoError inner message to be read via a uniffi export
Adds pollyfills for WebSocket and File API (required 
by unidici). Node > 21 actually doesn't need these 
pollyfills, neither does the browser. I've not tested
these changes in the browser yet.
Pure Rust implementation of fetch_ohttp_keys using gloo-net and (too
much rust-wasmbindgen).

The Websocket Implementation is quite straightfoward as it uses
gloo-net to access the WebSocket API. However, the fetch API does not
support Proxy requests, so we bind to uncidi and (in a quite ugly
fashion) make the HTTP(S) request thru the proxy. Messy!

It would likely have been more sensible to implement these as JS
stubs, but I had quite a lot of fun implementing these.

lockfiles were generated by using the script in contrib/

+----------------------+
| fetch_ohttp_keys()   |
+----------+-----------+
           |
           v
+----------+-----------+
| Detect JS Runtime    |
+----+------------+----+
     |            |
 (process)    (Browser/Worker)
     |            |
     v            v
+---------+  (fail)  +-------------------+
| undici  |--------->| WebSocket +       |
| Proxy   |          | rustls TLS Stream |
+---------+          +-------------------+
     |                         |
     | (success)               |
     v                         v
+----------------------------------------+
|              OHTTP Relay               |
+----------------------------------------+
           |
           v
+----------------------------------------+
|           Payjoin Directory            |
+----------------------------------------+

Closes: payjoin#1250
service.fetchOhttpKeysWithCert is still using 
NAAPI API. These test check basic functionality.
Copilot AI review requested due to automatic review settings March 25, 2026 21:35
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a WASM/JavaScript-capable IO layer to fetch OHTTP keys (to address the lack of io in JS bindings), and wires it through the UniFFI JavaScript package with new integration tests and required dependencies.

Changes:

  • Introduces a wasm32 IO implementation that fetches OHTTP keys via undici (CONNECT proxy) and a WebSocket/TLS fallback.
  • Refactors IO error types so native/wasm share a single payjoin::io::Error/InternalError surface.
  • Exposes io via payjoin-ffi and updates the JS package (polyfills, deps, tests, CI helper script).

Reviewed changes

Copilot reviewed 12 out of 17 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
payjoin/src/core/io/wasm.rs New wasm32 IO backend using undici + WebSocket/TLS strategy.
payjoin/src/core/io/native.rs Adapts native IO backend to shared error types in io/mod.rs.
payjoin/src/core/io/mod.rs New shared io module defining public Error/InternalError and conversion macro.
payjoin/Cargo.toml Adds wasm32 dependencies needed for the new wasm IO backend.
payjoin-ffi/src/lib.rs Conditionally exports a new io module when the io feature is enabled.
payjoin-ffi/src/io.rs New UniFFI-exported async wrappers for fetching OHTTP keys.
payjoin-ffi/javascript/ubrn.config.yaml Enables io + _manual-tls features for the JS/WASM build.
payjoin-ffi/javascript/test/integration.test.ts Adds JS integration tests exercising the WASM IO path.
payjoin-ffi/javascript/src/polyfills.ts Adds Node polyfills for File and WebSocket.
payjoin-ffi/javascript/src/index.ts Loads polyfills before exporting generated bindings.
payjoin-ffi/javascript/package.json Adds undici and ws dependencies (and type deps).
payjoin-ffi/javascript/package-lock.json Lockfile updates for the new JS dependencies.
payjoin-ffi/javascript/contrib/test.sh New helper script to install deps, generate bindings, and run JS tests.
payjoin-ffi/Cargo.toml Adds io feature that enables payjoin/io.
Cargo-recent.lock Lock updates for new Rust deps (wasm IO + toolchain transitive changes).
Cargo-minimal.lock Lock updates for new Rust deps (wasm IO + toolchain transitive changes).
.gitignore Ignores .idea project files.
Files not reviewed (1)
  • payjoin-ffi/javascript/package-lock.json: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -0,0 +1,23 @@
import buffer from "node:buffer";
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In an ESM build, import buffer from "node:buffer" will fail at runtime because node:buffer does not provide a default export. Use a namespace or named import (e.g., import * as buffer from "node:buffer" or import File directly) to avoid SyntaxError: ... does not provide an export named 'default'.

Suggested change
import buffer from "node:buffer";
import * as buffer from "node:buffer";

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +2
import "./polyfills.js";

Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

polyfills.ts imports Node-only modules (node:buffer, ws) and index.ts now imports it unconditionally. That will break browser consumers (and contradicts the package.json browser field) because those modules can’t be resolved in browser bundlers. Consider a Node-specific entrypoint, conditional import, or an alternate index.web.ts that omits these polyfills.

Copilot uses AI. Check for mistakes.
Comment on lines +41 to 44
"dependencies": {
"undici": "^7.24.3",
"ws": "^8.19.0"
}
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The selected undici version has an engines constraint of node >=20.18.1 (see package-lock), but the new polyfill comments imply support for Node 18/19. Either document/encode the new minimum Node version (e.g., via engines in package.json / CI config) or pin undici to a version that supports the intended Node range.

Copilot uses AI. Check for mistakes.
Comment on lines +5 to +6
// Reqired for Node.JS version < 20,
// buffer.File is not available in all v18 and v19 versions as it was backported
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in comment: ReqiredRequired. (Also consider using Node.js instead of Node.JS for consistency.)

Copilot uses AI. Check for mistakes.
};

let ohttp_key_bytes = {
// call unidici fetch(), and jump through multiple hoops to get a Vec() out of the response
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in comment: unidiciundici.

Suggested change
// call unidici fetch(), and jump through multiple hoops to get a Vec() out of the response
// call undici fetch(), and jump through multiple hoops to get a Vec() out of the response

Copilot uses AI. Check for mistakes.
Comment on lines 60 to +74
[target.'cfg(target_arch = "wasm32")'.dependencies]
web-time = "1.1.0"
getrandom = { version = "0.4.2", features = ["wasm_js"] }
js-sys = "0.3.85"
gloo-net = { version = "0.6.0", features = ["io-util"] }
gloo-utils = { version = "0.2.0", features = ["serde"] }
ring = { version = "0.17.14", default-features = false, features = ["wasm32_unknown_unknown_js"] }
futures = { version = "0.3.28"}
futures-rustls = { version = "0.26.0", default-features = false, features = ["ring"] }
webpki-roots = "1.0.5"
rustls-pki-types = { version = "1.14.0", default-features = false, features = ["web"] }
wasm-bindgen = "0.2"
wasm-bindgen-futures = "0.4"
serde-wasm-bindgen = "0.6"
base64 = "0.21"
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These wasm32-only dependencies are currently unconditional for any wasm32 build of payjoin, even when the io feature is disabled. This increases build time and binary size for WASM consumers that don’t need IO. Consider making them optional = true and enabling them only via the io feature (or a dedicated wasm-io feature), e.g. using feature-gated deps instead of always-on target deps.

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,360 @@
//! WASM IO implementation.

use std::fmt::{Debug, Formatter};
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug is imported but not used (#[derive(Debug)] doesn’t require importing the trait). This will trigger an unused import warning in WASM builds; consider removing Debug from the import list.

Suggested change
use std::fmt::{Debug, Formatter};
use std::fmt::Formatter;

Copilot uses AI. Check for mistakes.
Comment on lines +46 to +53
// if we're not running in a browser
if !matches!(runtime, Runtime::BrowserMain | Runtime::WebWorker) {
if let Ok(keys) = fetch_via_http_connect(&relay, &directory, cert.as_deref()).await {
return Ok(keys);
}
}

fetch_via_websocket(relay, directory, cert.as_deref()).await
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetch_ohttp_keys_strategy discards the concrete error from fetch_via_http_connect and silently falls back to the WebSocket strategy. This can mask real failures (e.g., proxy misconfiguration, TLS failures) and make debugging difficult. Consider preserving the first error and returning a combined error if the fallback also fails (or only falling back for specific error kinds).

Copilot uses AI. Check for mistakes.
Comment on lines +136 to +149
let response = fetch(directory_url.as_str(), &fetch_options)
.map_err(InternalErrorInner::ProxyFetchFailed)?;
let response =
JsFuture::from(response).await.map_err(InternalErrorInner::ProxyFetchFailed)?;
let response: Response = response.unchecked_into();
Uint8Array::new(
&JsFuture::from(response.array_buffer())
.await
.map_err(|_e| InternalErrorInner::InvalidResponse)?,
)
.to_vec()
};

OhttpKeys::decode(&ohttp_key_bytes).map_err(Error::from)
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetch_via_http_connect decodes the response body without checking the HTTP status. This makes the WASM implementation inconsistent with the native one (which returns UnexpectedStatusCode) and may turn server errors into misleading decode errors. Bind the Response.status (and ideally ok) property and return Error::UnexpectedStatusCode on non-success statuses.

Copilot uses AI. Check for mistakes.
Comment on lines +205 to +206
let ohttp_keys_req =
format!("GET /ohttp-keys HTTP/1.1\r\nHost: {}\r\nConnection: close\r\n\r\n", host_header);
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The WebSocket strategy sends GET /ohttp-keys but the rest of the codebase (and the RFC9540 gateway path used elsewhere) fetches keys from /.well-known/ohttp-gateway. This looks like the wrong endpoint for keys retrieval via the relay’s ws-bootstrap mechanism. Update the request line to use the gateway path so it matches the server behavior.

Suggested change
let ohttp_keys_req =
format!("GET /ohttp-keys HTTP/1.1\r\nHost: {}\r\nConnection: close\r\n\r\n", host_header);
let ohttp_keys_req = format!(
"GET /.well-known/ohttp-gateway HTTP/1.1\r\nHost: {}\r\nConnection: close\r\n\r\n",
host_header
);

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

@Anunayj Anunayj Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait right yea, now I'm starting to question how was this working in my testing..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JS bindings should include io functionality

2 participants