Add helpers to create client interfaces from a raw Rpc - #1898
Conversation
🦋 Changeset detectedLatest commit: 44d8f96 The changes in this PR will be included in the next version bump. This PR includes changesets to release 48 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
BundleMonFiles updated (4)
Unchanged files (146)
Total files change +925B +0.16% Final result: ✅ View report in BundleMon website ➡️ |
trevor-cortex
left a comment
There was a problem hiding this comment.
Summary
This PR adds three convenience helpers to @solana/kit — createClientWithGetMinimumBalanceFromRpc, createClientWithFetchAccountsFromRpc, and createClientWithInterfacesFromRpc — that wrap a raw Rpc object into the ClientWithGetMinimumBalance and/or ClientWithFetchAccounts plugin interfaces without assembling a full Kit client. It ships with unit tests, typetests, README docs, and a minor changeset (appropriate for new features).
The API shape, type-level narrowing in ClientInterfacesFromRpc<TRpc>, typetests, and docs are all well done. Unfortunately there's a fundamental runtime problem that the unit tests don't catch. (I would have marked this "request changes", but I don't have that permission on this repo — treat this as such.)
Critical: in rpc capability checks never pass for real Kit Rpc objects
Every Rpc produced by createRpc / createSolanaRpc / createSolanaRpcFromTransport is a Proxy (see makeProxy in packages/rpc-spec/src/rpc.ts). That proxy defines only get, defineProperty, and deleteProperty traps, and its target is the createJsonRpcApi proxy — which itself has no has trap and wraps a plain {} target. The in operator therefore falls through both proxies to Reflect.has({}, ...) and always returns false for every method name.
Consequences:
createClientWithInterfacesFromRpc(createSolanaRpc(url))returns an empty object at runtime, while its type claims both interfaces —client.getMinimumBalance is not a functionfor the primary real-world use case.createClientWithFetchAccountsFromRpcon a real RPC never takes thegetMultipleAccountspath and silently degrades to NgetAccountInfocalls, contradicting the documented behaviour.
The unit tests pass only because they use plain-object mocks, where in works. Note that typeof rpc.getAccountInfo === 'function' is no better as a detection mechanism — the get trap returns a function for any property name — so runtime feature detection on these proxies is fundamentally unreliable. Some options to consider:
- Since
ClientInterfacesFromRpc<TRpc>already narrows the return type statically, always build both interfaces at runtime and let the cast hide the untyped ones (the currentnot.toHavePropertytest expectations would need to change). - Accept an explicit config telling the helper which interfaces/methods to back.
- Add a
hastrap to the RPC proxies in@solana/rpc-spec— though that's a larger change andcreateJsonRpcApiis intentionally a catch-all, so there may be no method list to consult.
Other issues
- The
withoutHeadercomputation derives a per-byte rate via integer division, which can silently underestimate rent (see inline comment). An exact subtraction-based approach is available. - Tests: please add at least one test that exercises these helpers against a proxy-backed RPC (e.g. built with
createRpc+createJsonRpcApiand a mock transport, orcreateSolanaRpcFromTransport). That test would have caught theinissue immediately, and it guards against future regressions in how the proxies interact with these helpers.
Notes for subsequent reviewers
- Verify the intended semantics of
withoutHeaderagainst the runtime's rent formula ((BASE_ACCOUNT_SIZE + data_len) × rate) — the current implementation and the interface docs in@solana/plugin-interfacesagree on intent, but the arithmetic deserves a close look. - The changeset (
minor) and README additions look consistent with repo conventions; no concerns there. - The typetests are thorough and the
ClientInterfacesFromRpc<TRpc>conditional type handles theRpc<SolanaRpcApi>superset case correctly.
|
Documentation Preview: https://kit-docs-hljd2fvuh-anza-tech.vercel.app |
Add `createClientWithGetMinimumBalanceFromRpc`, `createClientWithFetchAccountsFromRpc` and `createClientWithInterfacesFromRpc` to `@solana/kit`. These convenience helpers let consumers that only have a raw `Rpc` object construct the corresponding client interfaces (`ClientWithGetMinimumBalance` and `ClientWithFetchAccounts`) without assembling a full Kit client. `createClientWithInterfacesFromRpc` fills in whichever interfaces the RPC supports and narrows its return type accordingly.
5209565 to
44d8f96
Compare
trevor-cortex
left a comment
There was a problem hiding this comment.
Re-review
Verdict: approve. (I don't have permission to submit a formal APPROVE review on this repo, so this is a comment review — please treat it as an approval.)
All points from my previous review have been addressed — nicely done:
in rpccapability checks (critical): Resolved by building both interfaces unconditionally and letting theClientInterfacesFromRpc<TRpc>return type do the narrowing (option 1 from my review). The docs, README, and JSDoc now clearly state that a rawRpc's capabilities cannot be detected at runtime and that calling an unsupported method fails at the underlying RPC call. That's an honest, well-documented contract.- Proxy-backed RPC test: The new
with a proxy-backed RPCdescribe block exercises the helpers against a realcreateRpc+createJsonRpcApiproxy with a mock transport, coveringgetMinimumBalance, the single-accountgetAccountInfopath, and the multi-accountgetMultipleAccountspath. This is exactly the regression guard that would have caught the original issue. withoutHeaderarithmetic: The per-byte rate is now derived fromgetMinimumBalanceForRentExemption(0n)with a documented argument for why the division byBASE_ACCOUNT_SIZEis exact (rate * 128 / 128 === rateunder the runtime's linear rent formula). Single round-trip, exact under the current formula — good resolution.- Empty address list:
fetchAccounts([])now short-circuits to[]without issuing an RPC call, with test coverage.
Notes for subsequent reviewers
- The exactness of the
withoutHeaderderivation relies on the header-only balance being an exact integer multiple ofBASE_ACCOUNT_SIZE, which holds under the current rent parameters (lamports_per_byte_year × exemption_thresholdis integral). If a cluster ever ran with rent parameters where that product is non-integral, a small truncation error could reappear — theoretical today, and the code comment documents the assumption, so nothing to change here. - Typetests cover all the interesting
ClientInterfacesFromRpc<TRpc>branches, including rejection of single-account-method RPCs and theRpc<SolanaRpcApi>superset case.
Changeset (minor), README, and export wiring all look consistent with repo conventions. Ship it.
|
🔎💬 Inkeep AI search and chat service is syncing content for source 'Solana Kit Docs' |

Add
createClientWithGetMinimumBalanceFromRpc,createClientWithFetchAccountsFromRpcandcreateClientWithInterfacesFromRpcto@solana/kit. These convenience helpers let consumers that only have a rawRpcobject construct the corresponding client interfaces (ClientWithGetMinimumBalanceandClientWithFetchAccounts) without assembling a full Kit client.createClientWithInterfacesFromRpcfills in whichever interfaces the RPC supports and narrows its return type accordingly.