Skip to content

fix(react): keep page-side asset-metadata & bech32 on the configured network - #189

Open
WiktorStarczewski wants to merge 3 commits into
nextfrom
wiktor/fix-asset-metadata-testnet
Open

fix(react): keep page-side asset-metadata & bech32 on the configured network#189
WiktorStarczewski wants to merge 3 commits into
nextfrom
wiktor/fix-asset-metadata-testnet

Conversation

@WiktorStarczewski

Copy link
Copy Markdown
Collaborator

Fixes #188.

Problem

On a devnet-configured run, page-side code issued testnet RPC calls and could tag addresses for testnet, while the service-worker sync and persisted storage correctly stayed on devnet.

MidenProvider resolves the endpoint synchronously but writes it to the store (config.rpcUrl) only inside its async init (isReady flips at the end). Page-side consumers that read config.rpcUrl from the store see undefined during the init window and fall back to a hardcoded testnet default:

  • useAssetMetadata builds its own page-side RpcClient; with config.rpcUrl unset it used Endpoint.testnet() and fired getAccountDetails against testnet, independent of the configured WebClient/worker.
  • accountBech32's inferNetworkId defaulted to NetworkId.testnet() whenever the endpoint was unset or unrecognized (incl. localhost/custom URLs), tagging addresses mtst.

Fix

  • useAssetMetadata: gate RpcClient construction on isReady (the same pattern the default prover already uses). Once init completes it uses the resolved endpoint; the testnet default applies only when no endpoint is configured at all. Also avoids constructing WASM Endpoint/RpcClient objects before the module is ready.
  • accountBech32: derive the network from the resolved endpoint, treat local nodes as devnet, and return the raw account id (rather than a wrong-network bech32 string) when the network can't be confirmed.

A top-level testnet default (MidenProvider / WebClient when nothing is passed) is intentionally preserved — this only fixes the inconsistency where a configured network leaked testnet on the page side.

Tests

  • New jsdom regression tests for both paths:
    • useAssetMetadata: defers (no RPC, no Endpoint.testnet) before ready; uses the configured endpoint and never testnet once ready; still uses the testnet default when nothing is configured.
    • accountBech32: devnet/testnet/localhost selection, testnet default when unconfigured-but-ready, and raw-id fallback before ready / for custom endpoints.
  • Existing useAssetMetadata fetch tests now configure an endpoint, mirroring real usage through the readiness gate.
  • Full react-sdk suite green (57 files / 794 tests); coverage gate holds. accountBech32 real-WASM bech32 output remains covered by the Playwright test/accountBech32.test.ts.

…d network

useAssetMetadata builds its own page-side RpcClient and accountBech32 picks a
NetworkId from config.rpcUrl in the store, which MidenProvider only populates
after its async init. During that window both reached the hardcoded testnet
default even when the consumer configured devnet — the WebClient/worker
correctly used devnet while page-side asset-metadata calls hit testnet RPC and
bech32 tagged addresses mtst.

- useAssetMetadata: gate RpcClient construction on `isReady` (the same pattern
  the default prover already uses), so it uses the resolved endpoint once init
  completes. The testnet default applies only when no endpoint is configured.
- accountBech32: derive the network from the resolved endpoint, treat local
  nodes as devnet, and return the raw account id rather than a wrong-network
  bech32 string when the network can't be confirmed.

Adds jsdom regression tests for both (the existing fetch tests now configure an
endpoint, mirroring real usage through the readiness gate).
… fix

The unrecognized-custom-endpoint case now returns the raw account id rather than
a testnet bech32 string, so update that assertion (and its name/comment). Rename
the stale inferNetworkId references in the integration test and test-app to
resolveNetworkName.
Comment thread CHANGELOG.md

### Fixes

* [FIX][react] Page-side code no longer reaches the testnet default under a non-testnet (e.g. devnet) configuration. `useAssetMetadata` builds its own page-side `RpcClient` from the resolved `config.rpcUrl`, but ran during the window before `MidenProvider` populated the store — falling back to `Endpoint.testnet()` and firing `getAccountDetails` against testnet even when the consumer configured devnet (the WebClient/worker correctly used devnet while page-side asset-metadata calls hit testnet). It now gates `RpcClient` construction on `isReady` (the same pattern the default prover uses). `accountBech32`'s network resolution had the same shape: it tagged addresses for testnet whenever the network couldn't be confirmed (before the provider was ready, or for `localhost`/custom endpoints); it now derives the network from the resolved endpoint, treats local nodes as devnet, and returns the raw account id rather than a wrong-network bech32 string when the network is undetermined. In both cases the testnet default still applies only when no endpoint was configured at all. ([#189](https://github.com/0xMiden/web-sdk/pull/189))

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 think the fix might be fine (waiting for the client to be initialized sounds like something you want to do anyway), but honestly falling back to Endpoint.testnet() when nothing is provided sounds like it should probably be an error if it allows for these kinds of race conditions. Also, using the store to persist the RPC config feels like an antipattern as well. Can't we derive the RPC from the client itself?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yeah so the only place with default is the initialisation of the client - if you just say , it willl go to testnet (mainnet in 2 months). I think this is a worthwhile shortcut. The main issue is other places should never fall back, and this is what I'm doing in this PR.

Re, using store to persist the RPC - that's a good one. A little bit more involved, but I think its worth it. I'll add.

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.

When I mentioned

Can't we derive the RPC from the client itself?

I mostly meant deriving the RPC from the WebClient struct. Because WebClient::create_client() already takes the endpoint as a parameter, we could store that and use that instead?

WiktorStarczewski added a commit that referenced this pull request Jun 29, 2026
Add `WebClient.endpoint()` and `RpcClient.endpoint()` (backed by miden-client's
new `NodeRpcClient::endpoint` / `Client::rpc_endpoint`), returning the node URL
the client is configured to talk to. `WebClient` caches it at creation so it can
be read synchronously without locking the async `inner` cell.

The React SDK now derives the RPC endpoint from the live client instead of a
separately-stored `config.rpcUrl` copy: `useAssetMetadata` and `accountBech32`
read `client.endpoint()`. `client` exists only once the provider has finished
initializing, so a configured (e.g. devnet) app no longer reads an unset URL and
falls back to testnet during the init window; addresses are tagged for the right
network, falling back to the raw id when the network can't be confirmed.

Supersedes the gating approach in #189 (addresses the reviewer's "derive the RPC
from the client" feedback). Requires miden-client 0xMiden/rust-sdk#2291.
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