feat(sdp-api): forward user_id to Kora on sign calls (PRO-1348)#548
feat(sdp-api): forward user_id to Kora on sign calls (PRO-1348)#548niks3089 wants to merge 1 commit into
Conversation
Greptile SummaryThis PR fixes the mainnet Kora rejection ("user_id is required when usage tracking is enabled and pricing is free") by threading a stable per-user identifier through the per-request adapter construction path. A new
Confidence Score: 5/5Safe to merge for the per-request sign paths; the three deferred background callers are a known gap acknowledged in the PR description and won't break devnet. The logic is clean and isolated: userId is optional everywhere so the change is backward-compatible, existing callers compile without modification, and the new unit tests cover all four sign × presence combinations. The one noteworthy item is that the PR description's claim of a package bump to 0.3.0-beta.0 is contradicted by both the lockfile (still 0.2.1) and the code comment itself — the actual approach relies on 0.2.1 forwarding extra params at runtime rather than on typed SDK support. This is documented in the code comment and is consistent with how JSON-RPC clients work, but it does mean mainnet is the first real end-to-end test of the user_id delivery. apps/sdp-api/src/services/adapters/fee-payment/kora/kora.adapter.ts — the buildSignRequest cast and its relationship to the installed SDK version. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Req as HTTP Request
participant RKU as resolveKoraUserId(c)
participant CTX as payments/context.ts or signer-check.ts
participant FAC as createFeePaymentAdapter(env, userId)
participant KA as KoraAdapter
participant BSR as buildSignRequest(base64Tx)
participant SDK as KoraClient (@solana/kora 0.2.1)
participant KORA as Kora RPC (mainnet)
Req->>CTX: inbound sign request
CTX->>RKU: resolve userId
RKU-->>CTX: session.userId ?? clerk.userId ?? apiKey.id
CTX->>FAC: createFeePaymentAdapter(env, userId)
FAC->>KA: "new KoraAdapter({ ..., userId })"
KA-->>FAC: adapter instance
FAC-->>CTX: FeePaymentPort
CTX->>KA: signAndSend(txBytes) or signAsFeePayer(txBytes)
KA->>BSR: buildSignRequest(base64Tx)
BSR-->>KA: "{ transaction, user_id? } cast to SDK param type"
KA->>SDK: signAndSendTransaction(params) or signTransaction(params)
SDK->>KORA: "JSON-RPC { method, params: { transaction, user_id } }"
KORA-->>SDK: "{ signature, signed_transaction }"
SDK-->>KA: response
KA-->>CTX: Signature or signed bytes
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Req as HTTP Request
participant RKU as resolveKoraUserId(c)
participant CTX as payments/context.ts or signer-check.ts
participant FAC as createFeePaymentAdapter(env, userId)
participant KA as KoraAdapter
participant BSR as buildSignRequest(base64Tx)
participant SDK as KoraClient (@solana/kora 0.2.1)
participant KORA as Kora RPC (mainnet)
Req->>CTX: inbound sign request
CTX->>RKU: resolve userId
RKU-->>CTX: session.userId ?? clerk.userId ?? apiKey.id
CTX->>FAC: createFeePaymentAdapter(env, userId)
FAC->>KA: "new KoraAdapter({ ..., userId })"
KA-->>FAC: adapter instance
FAC-->>CTX: FeePaymentPort
CTX->>KA: signAndSend(txBytes) or signAsFeePayer(txBytes)
KA->>BSR: buildSignRequest(base64Tx)
BSR-->>KA: "{ transaction, user_id? } cast to SDK param type"
KA->>SDK: signAndSendTransaction(params) or signTransaction(params)
SDK->>KORA: "JSON-RPC { method, params: { transaction, user_id } }"
KORA-->>SDK: "{ signature, signed_transaction }"
SDK-->>KA: response
KA-->>CTX: Signature or signed bytes
Reviews (2): Last reviewed commit: "feat(sdp-api): forward user_id to Kora o..." | Re-trigger Greptile |
| * Returns undefined when unauthenticated; the adapter then omits `user_id`. | ||
| */ | ||
| export function resolveKoraUserId(c: AppContext): string | undefined { | ||
| return c.get("session")?.userId ?? c.get("clerk")?.userId ?? c.get("apiKey")?.id ?? undefined; |
There was a problem hiding this comment.
Redundant trailing
?? undefined
All three nullable-access expressions (session?.userId, clerk?.userId, apiKey?.id) produce string | undefined — none of the underlying properties are typed as string | null. The final ?? undefined therefore never fires and just adds noise to the chain. Removing it keeps the intent clear and the return type still satisfies string | undefined.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| it("omits user_id when not configured (e.g. unauthenticated / devnet)", async () => { | ||
| const adapter = new KoraAdapter({ rpcUrl: "http://kora" }); | ||
| await adapter.signAndSend(TX); | ||
| const arg = mocks.signAndSendTransaction.mock.calls[0]?.[0] ?? {}; | ||
| expect(arg).not.toHaveProperty("user_id"); | ||
| expect(arg).toHaveProperty("transaction"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Missing omission test for
signAsFeePayer
The "omits user_id when not configured" case only exercises signAndSend → signAndSendTransaction. The symmetrical signAsFeePayer → signTransaction path uses identical spread logic but isn't covered. If the spread were accidentally dropped from one of the two methods, the existing tests wouldn't catch it.
Kora rejects signTransaction/signAndSendTransaction with "user_id is required" under its mainnet config (free pricing + usage tracking). Forward a stable per-end-user user_id on both sign calls. Stays on @solana/kora 0.2.1: its client forwards every request field to the JSON-RPC params, so user_id reaches the server via a typed cast (0.2.1's request type doesn't declare it). Deliberately did NOT bump to 0.3.0-beta.0 — that beta ships an empty `exports` map + unbundleable peer kit-plugins that break the esbuild Docker bundle (and destabilized @solana/pay's resolution). Swap to the typed user_id when @solana/kora 0.3.0 stable ships. resolveKoraUserId(c) sources the id (SDP session/clerk user id, else API key id), threaded through the per-request adapter (payments getFeePayment, custody signer-check) so every signAndSend carries it. Devnet has usage tracking off, so this is a no-op there. Adapter test covers forwarded + omitted for both methods. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
9310286 to
54c9cd8
Compare
|
Addressed both nits — and a bigger CI fix:
|
Closes the Kora
user_idrequirement (PRO-1348): Kora rejectssignTransaction/signAndSendTransactionwith "user_id is required when usage tracking is enabled and pricing is free" — its mainnet config. Forwards a stable per-end-useruser_id.Approach: stay on
@solana/kora0.2.1 (no SDK bump)0.2.1's client forwards every request field to the JSON-RPC
params(signTransaction(request) → rpcRequest('signTransaction', request)), souser_idreaches the server via a small typed cast (0.2.1's request type doesn't declare it yet).I first bumped to
0.3.0-beta.0(which typesuser_id) but it broke CI: that beta ships an emptyexportsmap + peer kit-plugins (@solana/kit-plugin-payer) that esbuild can't resolve in the Docker bundle, and the bump destabilized@solana/pay's resolution of the same plugin. Reverted — net dependency change is now zero (package.json+ lockfile == main). Swap to the typeduser_idwhen@solana/kora0.3.0 stable ships.Changes (6 files, source-only)
KoraAdapter:buildSignRequest()attachesuser_id; forwarded on both sign calls.resolveKoraUserId(c): SDP session/clerk user id, else API key id.getFeePayment(c), custodysigner-check) — no per-call-site changes.Devnet impact: none
kora-devnet has usage tracking off, so
user_idis ignored there. Mainnet-only effect.Follow-up (not in this PR)
Background/service paths (
mosaic,solana/factory,recurring-payments) don't threaduserIdyet — fine for devnet, needed before they hit mainnet Kora.