Enable browser WASM support for shared post-quantum crypto - #7
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| - uses: actions/checkout@v4 | ||
| - uses: dtolnay/rust-toolchain@stable | ||
| with: | ||
| targets: wasm32-unknown-unknown | ||
| - uses: Swatinem/rust-cache@v2 |
There was a problem hiding this comment.
The new workflow references actions/checkout, dtolnay/rust-toolchain, and Swatinem/rust-cache through mutable tags. If an upstream tag is moved or compromised, this job could execute code that was not reviewed in this PR with access to the workflow runner and token. Pin each action to a full commit SHA.
How this was verified: All three action references in the new workflow use tags rather than immutable commit SHAs.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/wasm.yml
Line: 10-14
Comment:
**Actions Use Mutable Tags**
The new workflow references `actions/checkout`, `dtolnay/rust-toolchain`, and `Swatinem/rust-cache` through mutable tags. If an upstream tag is moved or compromised, this job could execute code that was not reviewed in this PR with access to the workflow runner and token. Pin each action to a full commit SHA.
**How this was verified:** All three action references in the new workflow use tags rather than immutable commit SHAs.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| This does not provide a WASI entropy backend. JavaScript hosts must provide secure | ||
| Web Crypto randomness. Generated ant-core WASM integration tests exercise the | ||
| shared crypto through signed quotes and authenticated ML-KEM sessions. |
There was a problem hiding this comment.
The documentation says generated ant-core WASM tests exercise signed quotes and authenticated ML-KEM sessions, but it provides no ant-core reference, external test location, generation command, or browser test workflow. Either link and document the downstream test suite and revision or remove this claim so readers do not mistake native tests and the compile-only WASM check for runtime integration coverage.
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/wasm.md
Line: 13-15
Comment:
**Test Claim Is Unverifiable**
The documentation says generated ant-core WASM tests exercise signed quotes and authenticated ML-KEM sessions, but it provides no ant-core reference, external test location, generation command, or browser test workflow. Either link and document the downstream test suite and revision or remove this claim so readers do not mistake native tests and the compile-only WASM check for runtime integration coverage.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
dirvine
left a comment
dirvine
left a comment
There was a problem hiding this comment.
Verdict: ✅ Approve
I verified the risky parts locally (worktree at 29a2b27) rather than trusting the green CI — the PR does what it claims.
Independently verified
- Removed dependencies are dead. Grepped
src/,tests/,benches/forfutures,rayon,libc— zero usages. The one remainingtokioreference insrc/pqc/memory_pool.rsis inside a#[cfg(test)]module, so the dev-dependency placement is correct; the tokio-using test passes (9/9 memory_pool tests locally). - WASM compilation works.
cargo check --lib --target wasm32-unknown-unknownpasses locally, including--all-featuresand--no-default-features— matching the validation claims in the PR description. - Entropy story is sound. Every reachable randomness path (rand 0.8, rand_core 0.6, fips203/204/205) funnels into
getrandom 0.2.17, which gets thejsfeature via the correctly-scopedcfg(all(target_arch = "wasm32", target_os = "unknown"))target dependency. Native builds are untouched.
One thing worth knowing (non-blocking)
The lock file contains three getrandom versions (0.2.17, 0.3.4, 0.4.2), and only 0.2.17 has the JS backend enabled. I traced the 0.3.4 path: it comes via rand_core 0.9.5 ← the external hpke v0.13 crate. If that path were ever called on WASM it would fail at runtime despite compiling fine. However, the external hpke crate is completely unused — no source file references it (the api::hpke module is a hand-rolled ML-KEM-based construction that doesn't use the crate), so getrandom 0.3 is unreachable dead weight, likely pre-existing on main. Follow-up worth filing: drop hpke from [dependencies], which would also remove p256, rand_core 0.9, and getrandom 0.3 from the WASM graph entirely.
Minor notes (non-blocking)
wasm.ymltriggers on pushes toweb-supportand onpull_request, so every push runs the job twice (visible as duplicatewasmentries in the checks).masteris also listed but doesn't exist in this repo.- The CI job only checks default features; adding
--all-featuresto the check would lock in what was validated manually. - On Greptile's mutable-action-tags P2: valid in general, but consistent with all existing workflows in this repo — repo-wide SHA pinning should be a separate chore, not a blocker here.
docs/wasm.mdis honest about scope (compile-only CI, no browser runtime tests, no WASI backend) — the second Greptile finding was addressed by the docs commit.
No source file references the external hpke crate; the api::hpke module is an independent ML-KEM-based construction. hpke was the only path pulling rand_core 0.9 and getrandom 0.3 into the wasm32-unknown-unknown graph, where their lack of a JavaScript backend would fail at runtime if ever reached. Drops 14 now-unused crates (p256, x25519-dalek, crypto-bigint, primeorder, ...) from the lock file. Verified: cargo check --all-targets, WASM checks with --all-features and --no-default-features, and the full native lib test suite (156 tests).
grumbach
left a comment
grumbach
left a comment
There was a problem hiding this comment.
Review at d9da931d
Verdict: looks good to merge.
Checked:
- The removed dependencies (futures, rayon, libc,
hpke) are not used anywhere insrc/; tokio is only used by a test and is now a dev-dependency. getrandom's JavaScript backend is target-scoped towasm32-unknown-unknownand is not enabled for native targets.cargo check --locked --lib --target wasm32-unknown-unknownpasses locally at this head.
One release note: every consumer in the stack (transport, core, protocol, node, client) currently pins 29a2b272, one commit before this head. That last commit drops hpke and with it its elliptic-curve dependencies (p256, x25519-dalek, curve25519-dalek and a few more), so consumers' dependency graphs will change. The release candidate should be built and tested with this head rather than the pinned commit.
Small follow-up, not blocking: the WASM job only checks default features; adding --all-features would lock in what the PR description says was validated by hand.
Testnet evidence — DEV-03 run 591 (2026-09-17),
|
| 30-min slot from T0 (UTC) | native uploads | WASM uploads | native downloads | WASM downloads |
|---|---|---|---|---|
| T0 → +30m (19:16) | 356/356 (29.8 GiB) | 12/13 (1.4 GiB) | 42/42 (9.8 GiB) | 13/13 (0.5 GiB) |
| +30m → +60m | 371/371 (30.9 GiB) | 16/16 (2.6 GiB) | 48/48 (8.9 GiB) | 15/15 (0.5 GiB) |
| +60m → +90m window | 385/385 (32.3 GiB) | 16/16 (2.0 GiB) | 42/42 (12.8 GiB) | 7/7 (0.3 GiB) |
| +90m → +120m window | 392/392 (33.4 GiB) | 16/16 (2.6 GiB) | 50/50 (10.3 GiB) | 11/11 (1.1 GiB) |
| +120m → +150m window | 391/391 (32.4 GiB) | 16/16 (2.6 GiB) | 48/48 (10.7 GiB) | 14/14 (0.5 GiB) |
| +150m → +180m window | 388/388 (32.3 GiB) | 18/18 (2.6 GiB) | 49/49 (11.2 GiB) | 11/11 (0.6 GiB) |
| +180m → +210m window | 388/389 (32.7 GiB) | 16/16 (1.7 GiB) | 53/53 (9.3 GiB) | 11/11 (0.6 GiB) |
| +210m → +240m window | 392/393 (33.8 GiB) | 17/17 (2.6 GiB) | 50/50 (9.9 GiB) | 15/15 (0.6 GiB) |
No decay in rate or success on either transport from the first half-hour to the last. Measurement-window totals: native uploads 2336/2338 (99.91%), native downloads 292/292, WASM uploads 99/99, WASM downloads 69/69. Whole run (incl. warm-up): native 3085/3087 up / 386/386 down; WASM 127/128 up / 98/98 down. WASM covered 20 MB, 300 MB (single payment) and 900 MB (merkle) uploads, and downloads up to 500 MB, all paid through the JS callback (approve + payForQuotes) with the payment network taken from the node's HELLO.
Durations (ok ops, window): native upload p50 24.4 s / p95 126.5 s, WASM upload p50 146.1 s / p95 1021.8 s; native download p50 17.2 s / p95 72.2 s, WASM download p50 110.8 s / p95 236.3 s. Size-matched 20 MB: upload 16.8 s native vs 137.5 s WASM, download 15.5 s vs 96.6 s — the browser path is slower, as expected, but consistently so.
Failures, all three of them (out of 3,699 transfers)
- WASM upload feat: add deterministic ML-DSA keypair generation from seed #1 at T0 on the 20 MB uploader:
Witnessed close group inconclusive before payment: got 0/1 quorum-recognised peers— first-op-after-start transient, pre-payment, nothing spent; feat: strip tracing from release binaries #2 succeeded three minutes later and nothing recurred in 127 further uploads. - Native sgp/50 MB #336 (22:19Z):
witnessed close group initial lookup found 6 peers, need 7— DHT transient, pre-payment. - Native sjc/100 MB #418 (23:06Z):
payment error: Could not get current gas price: HTTP error 429from the public Arbitrum Sepolia RPC, pre-payment, nothing spent, next upload fine.
That last one (plus one identical -32000 context deadline exceeded on the same call five minutes after the window) technically fails the run's zero-tolerance "no RPC-transport payment failure" criterion, so the issue is scored 10/11. It is not a significant finding: 2 in 3,087 native uploads (0.065%), never post-payment, never money lost, and the cause is narrow — get_gas_price is the one read in the payment prepare path that evmlib ccd65f18 did not put behind retry() (estimate_gas, get_transaction_count, get_chain_id, get_transaction_receipt, get_block_by_number are). Zero evmlib::retry warnings appeared anywhere in the run, i.e. the public RPC was otherwise quiet. A one-line retry wrap closes it; it says nothing about the transport, the node, or the WASM path.
Also clean: 0 clock-skew signatures on either tier against the +3 h node (unsubmitted payment quotes expired / paid proof expired), 0 awaiting chain finality, 0 partial uploads, 0 not within its local closest peers, 0 invalid public file size (the WASM downloader skipped 111 over-cap entries by design), 0 OOM (WASM downloader peak 2.45 GB on the 500 MB pick), and a fleet-wide sweep of 1,009 ant-* units on 85 hosts found no panic, restart or replaced binary.
Node tier under this load: CPU mean 23.5% per host, per-service RSS p50 262 MB / p95 283 MB; steady-state 35.4 GB egress per GB uploaded, 8.7 GB disk per GB stored — in line with the previous (contaminated) run and not split by transport, since one fleet served both.
4 participants
Linear issue
Closes V2-803 — Linear issue
Summary
Enables the shared post-quantum crypto library on
wasm32-unknown-unknown: JavaScript-backedgetrandomentropy, Tokio restricted to development dependencies, and unused native runtime dependencies removed. Native and browser clients use the same ML-DSA, ML-KEM, and symmetric-crypto implementations.The current branch also clarifies validation scope in docs/wasm.md: the WASM CI job checks compilation, while ordinary Cargo crypto tests run on the native host. JavaScript hosts must provide secure Web Crypto randomness; this is not a WASI entropy backend.
Compatibility
No cryptographic wire format or public operation changes. Browser entropy configuration is target-specific. The Rust implementation is unchanged since
6d57e9d;29a2b27updates documentation.Validation
Previously reported implementation validation at
6d57e9d:During the current stack refresh, native and WASM builds of saorsa-core and ant-protocol pass using the current
29a2b27Git revision. The portable protocol tests pass as well (83 tests). These are downstream checks, separate from this repository's compile-only WASM CI.Coordinated consumers
saorsa-transport#160, saorsa-core#158, ant-protocol#29, ant-node#220, and ant-client#186 use this shared library through immutable Git revisions.
Review follow-up
The documentation now explicitly distinguishes native tests from WASM compilation. Pinning CI action tags and committed browser-runtime regression coverage remain follow-up work. The automated review below refers to its stated older reviewed commit.
Current stack revisions
All PRs link to V2-803. Immutable Git revisions used by the validated stack:
29a2b272cf424c046f0b1f620df7853ecdae7d1926f5fb79f1303e1aEVM recovery now retains ambiguous journals until finalized failure/replacement evidence is available. The protocol and native client include the matching consumer changes. Existing browser wire and payment formats are retained.
The code changes appear safe to merge, with non-blocking follow-up needed to pin CI actions and make the documented external integration-test claim verifiable.
Findings
Fix with agent prompt
Summary
This PR makes the shared cryptographic library compile for
wasm32-unknown-unknownby enabling JavaScript-backed entropy, moving Tokio to test-only dependencies, removing unused native runtime dependencies, and adding WASM portability CI and documentation.getrandom 0.2with its JavaScript backend.Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR Browser[Browser JavaScript host] --> WebCrypto[Web Crypto entropy] WebCrypto --> GetRandom[getrandom 0.2 + js] GetRandom --> RandCore[rand_core 0.6 OsRng] RandCore --> FIPS[FIPS ML-KEM / ML-DSA / SLH-DSA] RandCore --> Symmetric[Symmetric key and nonce generation] CI[WASM portability workflow] --> Check[cargo check --lib] Check --> Target[wasm32-unknown-unknown]Reviews (1) · Last reviewed commit: "Make shared post-quantum crypto availabl..."