feat(rpc): expose the configured node endpoint URL#2291
feat(rpc): expose the configured node endpoint URL#2291WiktorStarczewski wants to merge 2 commits into
Conversation
Add `NodeRpcClient::endpoint` (default `None`, overridden by `GrpcClient` to return its configured URL) and `Client::rpc_endpoint`, so consumers can read the endpoint a client is talking to without a network round-trip or threading the URL alongside the client. Motivating consumer: the web-sdk needs the React layer to derive the RPC endpoint from the WebClient itself rather than a separately-stored copy.
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.
igamigo
left a comment
igamigo
left a comment
There was a problem hiding this comment.
Looks good. I expanded a bit on my original comment on 0xMiden/web-sdk#189 (comment). In short, I think we could resolve the original problem without modifying the RPC trait. I left a couple of comments in case we want to merge this anyway.
| /// | ||
| /// Defaults to `None` for implementations that don't track an endpoint (e.g. test mocks); real | ||
| /// transports return their configured URL. | ||
| fn endpoint(&self) -> Option<&str> { |
There was a problem hiding this comment.
If we go this route, I think we should make this return a more strict type than &str, to ensure validity by construction. For example, it could be Endpoint.
| /// Defaults to `None` for implementations that don't track an endpoint (e.g. test mocks); real | ||
| /// transports return their configured URL. |
There was a problem hiding this comment.
I think we could avoid returning Option<> (mock implementations can provide mocked endpoints). Also I think I would avoid mentioning mock implementations specifically on the docs.
SantiagoPittella
left a comment
SantiagoPittella
left a comment
There was a problem hiding this comment.
Considering the deeply coupled motive of this change to the React web-client, should this be a method of the web client itself? Just a getter of the inner value stored in the object
3 participants
What
Adds
NodeRpcClient::endpoint(&self) -> Option<&str>(defaultNone, overridden byGrpcClientto return its configured URL) andClient::rpc_endpoint(&self) -> Option<&str>.Why
Lets consumers read the endpoint a client is configured to talk to without a network round-trip or threading the URL alongside the client. The trait method has a default (
None), so it's non-breaking for externalNodeRpcClientimplementors; mocks inherit it.Motivating consumer: web-sdk's React layer derives the RPC endpoint from the
WebClientitself (single source of truth) instead of a separately-stored copy in its store — see the linked web-sdk PR.