Skip to content

fix(web-client): ship optimized WASM by adding binaryen as devDependency#1994

Merged
WiktorStarczewski merged 2 commits intomainfrom
fix-wasm-size
Apr 12, 2026
Merged

fix(web-client): ship optimized WASM by adding binaryen as devDependency#1994
WiktorStarczewski merged 2 commits intomainfrom
fix-wasm-size

Conversation

@WiktorStarczewski
Copy link
Copy Markdown
Contributor

@WiktorStarczewski WiktorStarczewski commented Apr 10, 2026 •
edited
Loading

Summary

  • The 0.14.0 npm package shipped a 68MB unoptimized WASM instead of the expected ~14MB because wasm-opt was silently failing on CI
  • Root cause: the rollup plugin (@wasm-tool/rollup-plugin-rust) lists binaryen as a peer dependency — yarn doesn't auto-install peer deps. The only wasm-opt on CI was the v108 from apt, which can't parse the WASM (Fatal: error parsing wasm on bulk-memory-opt opcodes). The plugin swallows wasm-opt failures, so the unoptimized DWARF-laden binary was published
  • Fix: add binaryen to devDependencies so yarn install actually installs it, and remove the apt-get install binaryen steps that installed the broken v108

Changes

  • crates/web-client/package.json — add binaryen as devDependency
  • .github/workflows/publish-web-client-{release,next}.yml — remove apt-get install binaryen step
  • .github/workflows/test.yml — remove apt-get install binaryen step

Evidence

CI log from v0.14.0 publish confirms the failure:

> Running wasm-opt index_bg.wasm --output wasm_opt.wasm --strip-dwarf -O3 ...
Fatal: error parsing wasm
(!) [plugin rust] wasm-opt failed: Command `wasm-opt` failed with error code: 1

Test plan

  • CI WASM build job passes (confirms npm binaryen is found and works)
  • Check the built WASM size in CI logs to confirm ~14MB output

@WiktorStarczewski
fix(web-client): ship optimized WASM by adding binaryen as devDependency
0350f08 
The 0.14.0 npm package shipped a 68MB unoptimized WASM instead of ~14MB
because wasm-opt was silently failing on CI. The rollup plugin lists
binaryen as a peer dependency (not auto-installed by yarn), so the only
wasm-opt available was the v108 from apt which can't parse the WASM.
The plugin swallows wasm-opt failures, so the build continued with the
unoptimized, DWARF-laden binary.

Fix: add binaryen as a devDependency so yarn installs it, and remove
the apt-get binaryen steps from CI workflows (v108 is too old and
would shadow the npm version in non-yarn contexts).
@WiktorStarczewski WiktorStarczewski changed the base branch from next to main April 10, 2026 14:26
@WiktorStarczewski WiktorStarczewski added the no changelog This PR does not require an entry in the `CHANGELOG.md` file label Apr 10, 2026
@WiktorStarczewski
ci: add WASM size gate to publish workflows
827d5c9 
Fail the publish if the WASM exceeds 25MB. This catches silent wasm-opt
failures (the rollup plugin swallows errors) before the bloated binary
reaches npm.
igamigo
igamigo approved these changes Apr 10, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@igamigo igamigo left a comment

Choose a reason for hiding this comment

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

LGTM! In the end maybe this was unrelated to the CI changes I had made on the VM migration.
BTW I checked the devnet faucet and the wasm package seems to be at 16 MB:

Image

Though this is probably just because the server is sending it compressed? Otherwise not sure why it's not 68 MB.

Also, would it be better to add these changes on the split repo? I wonder if it will let us test the new CI flows. But also this is probably urgent so let's merge whenever you want

@TomasArrachea
Copy link
Copy Markdown
Collaborator

TomasArrachea commented Apr 10, 2026 •
edited
Loading

Though this is probably just because the server is sending it compressed? Otherwise not sure why it's not 68 MB.

Yes, the faucet sends the wasm compressed. But looking at this on the testnet faucet is just 4 MB, so there is actually a visible ~x4 increase in size on the current devnet (16 MB).

Edit: corrected KB to MB

@igamigo
Copy link
Copy Markdown
Collaborator

igamigo commented Apr 10, 2026

Though this is probably just because the server is sending it compressed? Otherwise not sure why it's not 68 MB.

Yes, the faucet sends the wasm compressed. But looking at this on the testnet faucet is just 4 KB, so there is actually a visible ~x4 increase in size on the current devnet (16 KB).

Yeah, devnet doesn't have the optimizations that this PR adds, so makes sense (I think you meant MB btw)

SantiagoPittella
SantiagoPittella approved these changes Apr 10, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@SantiagoPittella SantiagoPittella left a comment

Choose a reason for hiding this comment

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

Looks good.

@WiktorStarczewski WiktorStarczewski merged commit 237a3f2 into main Apr 12, 2026
35 checks passed
@WiktorStarczewski WiktorStarczewski deleted the fix-wasm-size branch April 12, 2026 13:23
TomasArrachea added a commit that referenced this pull request Apr 15, 2026
@TomasArrachea
Squashed commit of the following:
5015be3 
commit bcf8557
Author: tomasarrachea <tomas.arrachea@lambdaclass.com>
Date:   Wed Apr 15 18:28:39 2026 -0300

    chore: use node 0.14

commit d6bcdf0
Author: tomasarrachea <tomas.arrachea@lambdaclass.com>
Date:   Wed Apr 15 18:25:25 2026 -0300

    chore: bump to v0.14.2

commit f9cb4bc
Author: tomasarrachea <tomas.arrachea@lambdaclass.com>
Date:   Wed Apr 15 18:24:51 2026 -0300

    chore: changelog

commit 03605ac
Author: tomasarrachea <tomas.arrachea@lambdaclass.com>
Date:   Wed Apr 15 18:21:49 2026 -0300

    review: address comments

commit 8d3b30b
Author: tomasarrachea <tomas.arrachea@lambdaclass.com>
Date:   Wed Apr 15 17:34:08 2026 -0300

    feat: add test

commit a018afa
Author: tomasarrachea <tomas.arrachea@lambdaclass.com>
Date:   Wed Apr 15 17:22:33 2026 -0300

    feat: detect erased note from sync transactions proofs

commit 920a787
Author: Wiktor Starczewski <poszerny@gmail.com>
Date:   Wed Apr 15 18:27:27 2026 +0200

    fix(web-client): correct createP2IDNote/P2IDENote return type (#2043)

    * fix(web-client): correct createP2IDNote/P2IDENote return type to Note

    The hand-written TS declarations in api-types.d.ts said `OutputNote`, but
    the implementation forwards to `wasm.Note.createP2IDNote(...)` which returns
    `Note`. Consumers feeding the result into `NoteArray` or any `Note[]`-typed
    API hit a TS error and had to cast.

    Adds a compile-only typecheck (`yarn check:api-types`) wired into CI to
    catch this class of drift in the standalone wrappers going forward.

    * chore: retrigger CI

    * replace per-function typecheck with generic standalone-wrapper lint

    The earlier `api-types.check.ts` only caught drift for the three functions
    listed there; a new wrapper added tomorrow would be unchecked.

    Replaces that with `scripts/check-standalone-types.js`, which parses
    `js/standalone.js` to find every forwarder-style wrapper (functions whose
    return is `wasm.<Class>.<method>(...)` or `_WebClient.<method>(...)`) and
    compares each wrapper's declared return type in `dist/api-types.d.ts`
    against the underlying wasm-bindgen method's return type via the TS
    compiler API. Any new forwarder is checked automatically, no hardcoding.

    Also switches the three existing declarations to
    `ReturnType<WasmModule["Class"]["method"]>` so the return types source
    from the bindgen output directly and cannot drift by construction.

    Wires into the same `wasm-bindgen-types` CI job as the other check:* scripts.

    * fix(web-client): apply prettier formatting to check-standalone-types.js

    * docs(web-client): regenerate typedoc for P2ID Note return type

commit 9e30153
Author: Wiktor Starczewski <poszerny@gmail.com>
Date:   Wed Apr 15 18:21:33 2026 +0200

    fix(web): stop `insertBlockHeader` from overwriting historical peaks (#2039)

commit 30b0ee2
Author: juan518munoz <62400508+juan518munoz@users.noreply.github.com>
Date:   Tue Apr 14 16:06:24 2026 -0300

    refactor: retrieve deltas from large public accounts (#1916)

commit 50d19ef
Author: Wiktor Starczewski <poszerny@gmail.com>
Date:   Tue Apr 14 20:46:23 2026 +0200

    fix(web-client): use NoteArray in send({ returnNote: true }) + bump to 0.14.1 (#2012)

commit 8e51059
Author: igamigo <ignacio.amigo@lambdaclass.com>
Date:   Tue Apr 14 14:46:53 2026 -0300

    fix(keystore): write index atomically via tempfile (#2009)

    * refactor: use tempfile for for write lock

    * chore: comment

    * chore: changelog

    * chore: mve out of workspace deps

    * refactor: add source manager getter

commit 50f0048
Author: Wiktor Starczewski <poszerny@gmail.com>
Date:   Tue Apr 14 16:04:41 2026 +0200

    fix(web-client): classic Worker + TLA-free WASM glue for Safari/WKWebView (#2010)

    * fix(web-client): classic worker + remove WASM TLA for Safari/WKWebView

    Two changes to fix mobile (Safari/WKWebView) compatibility:

    1. Remove WASM TLA from Cargo glue: The top-level `await __wbg_init()`
       blocks WKWebView ESM module evaluation. Converted to a non-blocking
       call. The Worker's loadWasm() calls __wbg_init explicitly with the
       correct WASM URL.

    2. Classic Worker instead of module Worker: Safari/WKWebView is
       extremely slow with module workers ({type: "module"}) + dynamic
       import() — 10+ minutes on iOS Simulator vs 2 seconds with classic
       workers. The worker is now built as a self-contained async-IIFE
       with inlineDynamicImports, matching webpack's behavior.

       - rollup.config.js: Added inlineDynamicImports + wrap-worker-classic
         plugin that replaces import.meta.url, strips exports, wraps in IIFE
       - js/index.js: Removed {type: "module"} from Worker constructor
       - js/wasm.js: Calls __wbg_init({module_or_path: __wasm_url}) explicitly

    * docs(web-client): note Vite's import.meta rewrite ordering in wrap-worker-classic

    The plugin runs during rollup generateBundle, which is BEFORE downstream
    bundlers (Vite/webpack) see the output. Vite's own import.meta.url
    rewrite into hashed asset URLs happens later, so the sequence is:
      rollup emits import.meta.url → Vite hashes it → our plugin NEVER sees
      import.meta.url (it was replaced upstream). This is fine — the hashed
      URL already encodes the right path. Comment added so the next reader
      doesn't 'fix' this by moving the replacement earlier.

    * fix(web-client): satisfy prettier and eslint in worker wrap bits

    - Reformat rollup.config.js per prettier rules
    - Disable camelcase lint on wasm-bindgen's module_or_path init arg

commit 73dec84
Author: igamigo <ignacio.amigo@lambdaclass.com>
Date:   Mon Apr 13 19:48:57 2026 -0300

    fix: make source manager handling more consistent (#2006)

commit 558ac6b
Author: Wiktor Starczewski <poszerny@gmail.com>
Date:   Mon Apr 13 19:56:23 2026 +0200

    fix(sync): store MMR forest value and always persist auth nodes for tracked blocks (#1997)

    * fix(sync): store MMR forest value and always persist auth nodes for tracked blocks

    Two related bugs in the IndexedDB store caused `syncState` and
    `executeTransaction` to fail with MMR errors on web clients that
    perform large catch-up syncs (e.g. fresh wallet syncing from genesis
    to a chain tip thousands of blocks ahead).

    **Bug 1 — Forest/peaks mismatch (idxdb-store)**

    `get_partial_blockchain_peaks_by_block_num` reconstructed `MmrPeaks`
    using `Forest::new(block_num)`, but the peaks stored alongside a block
    header were captured *before* that block's leaf was added to the MMR
    (see `state_sync.rs:410`). When the block number's binary one-bit
    count differed from `(block_num - 1)`'s, `MmrPeaks::new` failed with
    "number of one bits in leaves does not equal peak length".

    Fix: store the actual `forest` value (number of MMR leaves at
    peak-capture time) alongside the peaks in both the individual
    `insert_block_header` path and the batch `apply_state_sync` path.
    On read, use the stored forest; fall back to `block_num` for records
    written before this field existed.

    **Bug 2 — Missing auth nodes for already-tracked blocks (state_sync)**

    During `process_state_sync_data`, phase 1 (`apply(mmr_delta)`) may
    implicitly mark a block as tracked in the in-memory `PartialMmr`,
    but on a fresh wallet with no prior tracked leaves, `apply()` returns
    zero authentication nodes. Phase 2 (note block screening) then finds
    `is_tracked(block_pos) == true` and skips collecting auth nodes
    entirely (`vec![]`). The block is inserted into `BlockUpdates` with
    `has_client_notes=true` but no authentication nodes.

    On the next sync, `get_current_partial_mmr` calls
    `PartialMmr::from_parts(peaks, tracked_nodes, tracked_leaves)` which
    validates that every tracked leaf has its leaf node value in the nodes
    map — failing with "tracked leaf at position N has no value in nodes".

    Fix: always snapshot nodes before/after the tracking step and collect
    any new nodes, regardless of whether `is_tracked` was already true.
    This ensures the leaf node and its authentication path siblings are
    always persisted.

    **Why existing tests didn't catch this:**

    The integration tests (`test:clean`) exercise small incremental syncs
    where a note is minted, synced, and consumed in a single session. The
    MMR delta is small and note blocks are at or near the chain tip, so
    they are never already-tracked when the note screening phase runs.
    The bugs only manifest during large catch-up syncs where the delta
    spans thousands of blocks that include note-relevant blocks.

    * docs: add changelog entries for MMR fixes in 0.14.1

    * test(sync): verify auth nodes are persisted for tracked note blocks

    * chore: formatting and safe integer casts

    * fix(sync): format state_sync test code with nightly rustfmt

    * fix(sync): replace redundant closure with method reference in test

    * test(sync): remove unit test that cannot exercise the store-layer bug

    The forest/peaks mismatch lives in the IndexedDB store read path
    (get_partial_blockchain_peaks_by_block_num), which is unreachable
    from pure Rust unit tests. The test was passing on both main and
    the fix branch because it builds the PartialMmr in-memory without
    going through the store. A proper regression test would require a
    Playwright integration test against a chain at a specific block
    number (power-of-2 boundary), which is not deterministic on a
    live network.

    * fix(sync): remove trailing blank line in state_sync tests

    * fix(sync): keep block MMR forest implicit (#1998)

    ---------

    Co-authored-by: igamigo <ignacio.amigo@lambdaclass.com>

commit 237a3f2
Author: Wiktor Starczewski <poszerny@gmail.com>
Date:   Sun Apr 12 15:23:42 2026 +0200

    fix(web-client): ship optimized WASM by adding binaryen as devDependency (#1994)

    * fix(web-client): ship optimized WASM by adding binaryen as devDependency

    The 0.14.0 npm package shipped a 68MB unoptimized WASM instead of ~14MB
    because wasm-opt was silently failing on CI. The rollup plugin lists
    binaryen as a peer dependency (not auto-installed by yarn), so the only
    wasm-opt available was the v108 from apt which can't parse the WASM.
    The plugin swallows wasm-opt failures, so the build continued with the
    unoptimized, DWARF-laden binary.

    Fix: add binaryen as a devDependency so yarn installs it, and remove
    the apt-get binaryen steps from CI workflows (v108 is too old and
    would shadow the npm version in non-yarn contexts).

    * ci: add WASM size gate to publish workflows

    Fail the publish if the WASM exceeds 25MB. This catches silent wasm-opt
    failures (the rollup plugin swallows errors) before the bloated binary
    reaches npm.
@juan518munoz juan518munoz mentioned this pull request Apr 16, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@igamigo igamigo igamigo approved these changes

@SantiagoPittella SantiagoPittella SantiagoPittella approved these changes

Assignees

No one assigned

Labels

no changelog This PR does not require an entry in the `CHANGELOG.md` file

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@WiktorStarczewski @TomasArrachea @igamigo @SantiagoPittella