Skip to content

feat(web-client): link dependency modules in compile.component; add notes.listConsumable - #170

Open
WiktorStarczewski wants to merge 5 commits into
nextfrom
wiktor/midenclient-compile-libraries-listconsumable
Open

feat(web-client): link dependency modules in compile.component; add notes.listConsumable#170
WiktorStarczewski wants to merge 5 commits into
nextfrom
wiktor/midenclient-compile-libraries-listconsumable

Conversation

@WiktorStarczewski

Copy link
Copy Markdown
Collaborator

Addresses #169.

Closes the two MidenClient surface gaps that force consumers (e.g. @openzeppelin/miden-multisig-client) to drop down to a second raw WasmWebClient over the same store — the dual-instance pattern behind the "synced public notes not found" report.

Changes

compile.component can now link dependency modules. CompileComponentOptions gains libraries, and component() static-links each entry as a source module via builder.linkModule(namespace, code) before compilation — matching what consumers do by hand off a raw code builder today.

Correctness note: a component's compiled MAST determines the account's code commitment (baked into the account ID), so the link path must produce a byte-identical component. linkModule (static source linking) does exactly that; buildLibrary + linkStaticLibrary is a different assembler path and was deliberately avoided.

notes.listConsumable({ account? }) returns ConsumableNoteRecord[] with consumability metadata intact. notes.listAvailable maps every record to inputNoteRecord(), discarding noteConsumability() — so it silently includes block-locked notes (NoteConsumptionStatus::ConsumableAfter) and can't express a consumable-now filter. listConsumable preserves noteConsumability() so callers can distinguish now vs. consumableAfterBlock.

After these, consumers can run entirely on a single MidenClient (or one shared WasmWebClient) — one instance per store — which removes the dual-instance bug at the source.

Verification

  • Built the napi module and confirmed compile.component({ libraries }) yields a component digest-identical to the raw createCodeBuilder().linkModule() path (EQUAL: true) — existing accounts are unaffected.
  • Unit tests (compiler + notes): 48 passing.
  • Node integration compile.component group (incl. a new equivalence test): 4 passing. Mirror added to the browser suite.

Out of scope / follow-up

…otes.listConsumable

Closes the two MidenClient surface gaps that force consumers (e.g. the
multisig client) onto a second raw WasmWebClient over the same store,
which causes synced notes to be unreadable through the reader client.

- compile.component now accepts `libraries` and static-links each as a
  source module via `linkModule` before compilation. Verified the
  resulting AccountComponent is byte-identical (same procedure digests)
  to the raw createCodeBuilder().linkModule() path, so account code
  commitments are unchanged.
- notes.listConsumable({ account? }) returns ConsumableNoteRecord[] with
  consumability preserved, unlike listAvailable which maps it away — so
  callers can distinguish consumable-now from block-locked notes.

Unit tests (compiler + notes) and node integration coverage added.
…e null, changelog

Addresses review feedback on #170:
- api-types: give compile.component its own `ComponentLibrary` ({namespace, code})
  type instead of reusing `CompileTxScriptLibrary`, which exposed an inert
  `linking` field — a silent, MAST-changing footgun.
- compiler.js: validate each `libraries` entry and throw a descriptive error
  for malformed input (parity with the txScript/noteScript guard); document
  that duplicate namespaces error.
- notes.js: `listConsumable` treats `{ account: null }` as "all accounts"
  (loose null check), matching the underlying `getConsumableNotes(account?)`.
- CHANGELOG: add entries for both new APIs (fixes the changelog gate).
- tests: cover empty libraries, malformed entry, linkModule throwing, and
  null account.
… equivalence test

wasm-bindgen moves/frees StorageSlot handles when AccountComponent.compile
consumes them, so reusing one slots array for both the resource and raw
compilation passes freed handles ('array contains a value of the wrong type')
in the browser. napi clones args so it passed there. Build fresh slots per
path. Procedure digests are independent of slot values, so the equivalence
assertion is unaffected.

@igamigo igamigo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I agree these are useful additions to the MidenClient surface, especially compile.component({ libraries }), but I don’t think they solve the reported bug at the right layer.

The issue in #169 is not just that compile.component is missing library support, or that listAvailable drops consumability metadata. The bigger problem is that consumers can still end up creating a second WasmWebClient over the same store because MidenClient does not give them a supported way to reuse or borrow its underlying client.

I’d rather see #169 fixed at the client-ownership level: for example, by adding a supported raw-client borrow/escape hatch, a per-store singleton or registry, or guardrails that prevent two live clients from being created over the same store. That would let the multisig client reuse the same underlying WasmWebClient, regardless of whether the high-level surface happens to expose every operation it needs.

So I’m fine with keeping compile.component({ libraries }) as useful API expansion, but I would not consider this enough to close the dual-instance store bug.

Comment thread crates/web-client/js/resources/notes.js Outdated
Comment on lines +46 to +58
// Like `listAvailable`, but keeps each note's consumability metadata
// (`noteConsumability()`) instead of mapping it away. Callers that must
// distinguish consumable-now from block-locked notes (status
// `consumableAfterBlock`) need this; `listAvailable` cannot express it.
// Omit `account` (or pass null) to list notes consumable by any tracked
// account — matching the underlying `getConsumableNotes(account?)`.
async listConsumable(opts) {
this.#client.assertNotTerminated();
const wasm = await this.#getWasm();
const accountId =
opts?.account == null ? undefined : resolveAccountRef(opts.account, wasm);
return await this.#inner.getConsumableNotes(accountId);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This exposes the metadata, but I think it leaves the confusing part of the API unchanged.

listAvailable() still calls getConsumableNotes() and then drops the result of noteConsumability(). Since getConsumableNotes() can return notes that are only ConsumableAfter, “available” can still mean “not actually consumable yet”. listConsumable() gives callers a way around that, but existing users of listAvailable() can still get block-locked notes from an API name that suggests they are usable now.

Should we fix the existing surface instead? For example, listAvailable() could return only notes that are consumable now, and we could add a separate metadata-preserving method with a more precise name. Alternatively, we could deprecate or rename the current behavior so the semantics are explicit.

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.

2 participants