fix(web): drop inspectable from JsAccountUpdate / JsStorageMapEntry / JsStorageSlot / JsVaultAsset#150
Open
WiktorStarczewski wants to merge 3 commits into
Open
fix(web): drop inspectable from JsAccountUpdate / JsStorageMapEntry / JsStorageSlot / JsVaultAsset#150WiktorStarczewski wants to merge 3 commits into
inspectable from JsAccountUpdate / JsStorageMapEntry / JsStorageSlot / JsVaultAsset#150WiktorStarczewski wants to merge 3 commits into
Conversation
… JsStorageSlot / JsVaultAsset
Resolves miden-client#2183: under Next.js 16.2 dev-mode the patched
console.* path (clientFileLogger.log) runs every non-primitive
argument through safe-stable-stringify, which invokes toJSON()
automatically. These four wasm-bindgen structs had
#[wasm_bindgen(getter_with_clone, inspectable)], which caused
wasm-bindgen to auto-emit a toJSON() reading every public field via
wasm.__wbg_get_<class>_<field>(this.__wbg_ptr) — i.e. each
console.log(update) fired 11 WASM getter calls. If the underlying
pointer had been freed or another WASM call was in flight, the
resulting 'null pointer passed to rust' trap propagated out of the
user's console.log call site and crashed the caller.
Other inspectable usages in the codebase (Address, NoteFile,
CodeBuilder, miden_array) wrap inner objects as tuple structs with no
public fields, so wasm-bindgen's auto-toJSON for them is a no-op —
they were never affected and stay as-is.
Dropping the attribute makes the JS class behave like the 15 other
wasm-bindgen exports the issue references: no toJSON() method, so
JSON.stringify / safe-stable-stringify see an empty wrapper and
return {}. Field access via the named getters is unchanged — only
the auto-stringification path is muted.
CHANGELOG entry added under a new 0.14.10 (TBA) section (0.14.9
was just shipped — per the project rule against adding to released
sections).
Locks in the contract enforced by the previous commit: the
wasm-bindgen wrapper classes for JsAccountUpdate, JsStorageMapEntry,
JsStorageSlot, and JsVaultAsset must NOT expose a toJSON() method.
Without toJSON(), safe-stable-stringify (used by Next.js 16.2 dev-mode
console.* patching) falls back to {} on these wrappers and the
__wbg_get_<class>_<field> re-entry path can never fire.
The test runs in the existing crates/web-client Playwright harness;
it loads the built SDK on window and asserts both
(a) prototype.toJSON is undefined for all four classes, and
(b) JSON.stringify of a fabricated JsAccountUpdate instance returns
'{}'. If anyone re-adds inspectable to these structs (or otherwise
emits an auto-toJSON), the test fails immediately.
The original idxdb_store_no_tojson.test.ts only checked the four
classes named in miden-client#2183 (JsAccountUpdate,
JsStorageMapEntry, JsStorageSlot, JsVaultAsset). The actual failure
mode — a wasm-bindgen-auto-generated toJSON that reads fields via
`__wbg_get_<class>_<field>(this.__wbg_ptr)` — can happen to ANY
struct declared `#[wasm_bindgen(inspectable)]` with `pub` fields,
not just those four.
Rename to no_wasm_reentry_via_tojson.test.ts and widen the contract:
enumerate every export on `window` that looks like a wasm-bindgen
wrapper (static `__wrap`, prototype `__destroy_into_raw`), fabricate
an instance with `__wbg_ptr = 0`, and assert JSON.stringify returns
'{}'. That property holds for:
- classes with no toJSON (default JSON.stringify behavior),
- classes with wasm-bindgen's safe `toJSON() { return {}; }` shape
(emitted for `inspectable` structs that have no `pub` fields —
Address, NoteFile, CodeBuilder, the array wrappers, etc.),
- any class with an explicit toJSON that doesn't read WASM fields.
It fails only for the dangerous shape: toJSON reading fields through
WASM getters. Anyone who later adds `inspectable` to a struct with
`pub` fields will see this assertion fire with the offending class
name in the failure message, regardless of whether the new struct is
in idxdb-store or anywhere else.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves
miden-client#2183.Why
Under Next.js 16.2 dev-mode the patched `console.*` path (`clientFileLogger.log`) runs every non-primitive argument through `safe-stable-stringify`, which invokes `toJSON()` automatically. These four wasm-bindgen structs in `crates/idxdb-store` were declared with `#[wasm_bindgen(getter_with_clone, inspectable)]`, which caused wasm-bindgen to auto-emit a `toJSON()` reading every public field via `wasm._wbg_get_(this.__wbg_ptr)` — i.e. each `console.log(update)` fired 11 WASM getter calls.
If the underlying pointer had been freed or another WASM call was in flight, the resulting `"null pointer passed to rust"` trap propagated out of the user's `console.log` call site and crashed the caller. `next build && next start` (production) avoided the symptom because the dev-only console patch isn't installed.
What this PR does
Drop `inspectable` on the four affected structs:
Without `inspectable`, wasm-bindgen does not emit `toJSON()`; `JSON.stringify` and `safe-stable-stringify` fall back to `{}` (the wasm-bindgen wrapper has no own enumerable data — it's all behind the `__wbg_ptr`). Field access via the named getters is unchanged — only the auto-stringification path is muted.
Other `inspectable` usages in the codebase (`Address`, `NoteFile`, `CodeBuilder`, the `miden_array` macro) wrap inner objects as tuple structs with no public fields, so wasm-bindgen's auto-toJSON for them is a no-op — they were never affected and stay as-is.
A long-form doc-comment above each struct records the rationale so the attribute isn't reintroduced by mistake.
CHANGELOG
Added under a new `0.14.10 (TBA)` section. `0.14.9` was just shipped to crates.io / npm — per the project rule (CLAUDE.md) against adding to released sections.
Test plan