Skip to content

fix(foundation): always include a result key in json-rpc responses - #24840

Merged
spalladino merged 3 commits into
merge-train/spartan-v5from
as/jsonrpc-null-result
Jul 21, 2026
Merged

fix(foundation): always include a result key in json-rpc responses#24840
spalladino merged 3 commits into
merge-train/spartan-v5from
as/jsonrpc-null-result

Conversation

@aminsammara

@aminsammara aminsammara commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Problem

The safe JSON-RPC server built success responses as { jsonrpc, id, result }. When a handler returns undefined, JSON.stringify drops the result key, producing {"jsonrpc":"2.0","id":N} — a response with neither result nor error, which violates JSON-RPC 2.0 and leaves raw/external callers unable to tell "not found" from a malformed reply.

Surfaced via node_getContract on an undeployed (but valid) address: the response had no result field at all.

Fix

One line in the shared server (safe_json_rpc_server.ts): coerce an undefined return to null (result: result ?? null, so 0/false/'' are preserved). The result key is now always present.

Blast radius

Framework-level, so it fixes every method that can return undefined — ~20 on the node interface (getContract, getContractClass, getTxEffect, getTxByHash, getBlock, the witness getters, synced slot/epoch, validator stats, …) plus every other server on this framework (PXE, prover-node, archiver). Void methods now also return result: null.

Why it's safe for existing clients

The TS client short-circuits null/undefined/'null'/'undefined' before schema validation, so it never runs null through an .optional() schema — omitted vs result: null both resolve to undefined client-side. Only raw/external callers change, and they now get a spec-compliant response.

Testing

  • Added a test: a handler returning undefined now yields { jsonrpc, result: null } (red before, green after).
  • Updated the existing void-method (clear) assertions, single and batch, to the new shape.
  • Added integration tests (test/integration.test.ts) pinning that optional result schemas tolerate null responses:
    • end-to-end: the wire response for an undefined return carries an explicit result: null, and the safe client resolves it to undefined;
    • client-isolated: a stubbed fetch returning result: null verbatim resolves to undefined against an .optional() schema instead of throwing a ZodError. Both go red if the client's null short-circuit is removed (verified: Invalid input: expected object, received null).
  • Full json-rpc suite: 102 passed; foundation lint clean.

The safe json-rpc server built success responses as { jsonrpc, id, result }; when a handler returned undefined, JSON.stringify dropped the key, yielding a response with neither result nor error — a JSON-RPC 2.0 violation that left raw callers unable to distinguish 'not found' from a malformed reply. Coerce undefined to null so result is always present. Affects every method that can return undefined (e.g. node getContract/getTxEffect/getBlock/witness getters) across all servers on this framework. TS clients are unaffected: the client already short-circuits null/undefined identically before schema validation.
@aminsammara
aminsammara force-pushed the as/jsonrpc-null-result branch from 79a84cc to a960ca8 Compare July 21, 2026 14:34
@aminsammara
aminsammara changed the base branch from merge-train/spartan to merge-train/spartan-v5 July 21, 2026 14:34
aminsammara and others added 2 commits July 21, 2026 15:47
…al schemas

The safe client validates results against zod schemas where optional returns use .optional(), which rejects null. A result: null (now always sent by the server for undefined returns) is only safe because the client short-circuits null before validation. Pin that behavior: assert the wire response carries an explicit result: null end-to-end, and that a client with an optional result schema resolves undefined instead of throwing when any server responds with null.
@spalladino
spalladino enabled auto-merge (squash) July 21, 2026 16:06
@spalladino
spalladino merged commit f403d88 into merge-train/spartan-v5 Jul 21, 2026
12 checks passed
@spalladino
spalladino deleted the as/jsonrpc-null-result branch July 21, 2026 16:21
aminsammara added a commit that referenced this pull request Jul 22, 2026
Promotes `v5-next` onto `v5` for the **v5.1.0** release.

- Merges `v5-next` into `v5`; the resulting tree is **identical to
`v5-next`** and `.release-please-manifest.json` is set to **5.1.0** (the
only merge conflict was the manifest — v5 held 5.0.1, resolved to
v5-next's 5.1.0).
- Merge via **merge commit** (v5 ruleset allows `merge` only).
- After this merges, `v5` HEAD is tagged `v5.1.0`.

Does not include #24840 (json-rpc result key), which is still on
`merge-train/spartan-v5` and not in `v5-next`.
@PhilWindle PhilWindle added the port-to-next Forward-port this merged PR into next label Jul 24, 2026
@AztecBot

Copy link
Copy Markdown
Collaborator

✅ Successfully ported to port-to-next-staging #24964.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

port-to-next Forward-port this merged PR into next

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants