-
Notifications
You must be signed in to change notification settings - Fork 322
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
chore: Safe JSON RPC server and client (#9656)
## Current situation
What was wrong with our way of exposing our APIs over JSON RPC? First of
all, the JsonRpcServer defaulted to exporting every method on the
underlying handler, including those flagged as private in ts (though
js-private like #method were safe). While we had a blacklist, it's safer
to be explicit on what to expose.
Then, argument deserialization was handled based on client inputs
exclusively, allowing to target any class registered in the RPC
interface. This means a method expecting an instance of class A as an
argument, could just be fed an instance of B by its caller.
Deserialization itself was also unchecked. Most fromJSON methods did not
validate any of their inputs, just assuming the any input object matched
the expected shape. Primitive arguments to functions were also
unchecked, so a caller could eg pass a number where a string was
expected.
Other unrelated issues included often forgetting to register a class for
(de)serialization in the rpc server, or forgetting to type all API
return types as async (if they are over an RPC interface, calling any
method should be async!).
These issues all affected the client as well. Though security is not so
much of an issue on the client, lack of validation could indeed cause
bugs by querying a server with a mismatching version.
## Proposed fix
We introduce a pair of "safe" JSON rpc client and server abstractions,
that consume a **schema** along with a handler. The schema leverages
`zod` for validating all inputs (in the case of the server) and outputs
(in the case of the client) and defining the exact set of methods that
are exposed. Schemas are type-checked against the interface, so a change
in the interface will trigger tsc to alert us to change the schemas as
well.
As a side effect of this change, since each method now knows _what_ it
should be deserializing, we can kill much of the custom logic for
(de)serialization, such as the string and class converters, and just
rely on vanilla json serialization. Each class that we intend to pass
through the wire should expose a static zod schema used for both
validation and hydration, and a `toJSON` method that is used for
serialization:
```typescript
export class TestNote {
constructor(private data: string) {}
static get schema() {
return z.object({ data: z.string() }).transform(({ data }) => new TestNote(data));
}
toJSON() {
return { data: this.data };
}
}
```
Then the API is defined as a plain interface:
```typescript
export interface TestStateApi {
getNote: (index: number) => Promise<TestNote>;
getNotes: () => Promise<TestNote[]>;
clear: () => Promise<void>;
addNotes: (notes: TestNote[]) => Promise<TestNote[]>;
fail: () => Promise<void>;
count: () => Promise<number>;
getStatus: () => Promise<{ status: string; count: bigint }>;
}
```
With its corresponding schema:
```typescript
export const TestStateSchema: ApiSchemaFor<TestStateApi> = {
getNote: z.function().args(z.number()).returns(TestNote.schema),
getNotes: z.function().returns(z.array(TestNote.schema)),
clear: z.function().returns(z.void()),
addNotes: z.function().args(z.array(TestNote.schema)).returns(z.array(TestNote.schema)),
fail: z.function().returns(z.void()),
count: z.function().returns(z.number()),
getStatus: z.function().returns(z.object({ status: z.string(), count: schemas.BigInt })),
};
```
## Scope of this PR
This PR introduces the new safe json rpc client and server abstractions,
but does **not** yet enable them. All `start` methods still use the old
flavors. Upcoming PRs will add schemas for all our public interfaces and
make the switch.- Loading branch information
1 parent
b70b6f1
commit e63e219
Showing
33 changed files
with
916 additions
and
66 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| import { type Signature } from '@aztec/foundation/eth-signature'; | ||
| import { type ApiSchemaFor } from '@aztec/foundation/schemas'; | ||
|
|
||
| import { z } from 'zod'; | ||
|
|
||
| import { EpochProofQuote } from '../prover_coordination/epoch_proof_quote.js'; | ||
|
|
||
| // Required by ts to export the schema of EpochProofQuote | ||
| export { type Signature }; | ||
|
|
||
| const EpochProvingJobState = [ | ||
| 'initialized', | ||
| 'processing', | ||
| 'awaiting-prover', | ||
| 'publishing-proof', | ||
| 'completed', | ||
| 'failed', | ||
| ] as const; | ||
|
|
||
| export type EpochProvingJobState = (typeof EpochProvingJobState)[number]; | ||
|
|
||
| /** JSON RPC public interface to a prover node. */ | ||
| export interface ProverNodeApi { | ||
| getJobs(): Promise<{ uuid: string; status: EpochProvingJobState }[]>; | ||
|
|
||
| startProof(epochNumber: number): Promise<void>; | ||
|
|
||
| prove(epochNumber: number): Promise<void>; | ||
|
|
||
| sendEpochProofQuote(quote: EpochProofQuote): Promise<void>; | ||
| } | ||
|
|
||
| /** Schemas for prover node API functions. */ | ||
| export class ProverNodeApiSchema implements ApiSchemaFor<ProverNodeApi> { | ||
| getJobs = z | ||
| .function() | ||
| .args() | ||
| .returns(z.array(z.object({ uuid: z.string().uuid(), status: z.enum(EpochProvingJobState) }))); | ||
|
|
||
| startProof = z.function().args(z.number()).returns(z.void()); | ||
|
|
||
| prove = z.function().args(z.number()).returns(z.void()); | ||
|
|
||
| sendEpochProofQuote = z.function().args(EpochProofQuote.schema).returns(z.void()); | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,2 @@ | ||
| export { createJsonRpcClient, defaultFetch, makeFetch } from './json_rpc_client.js'; | ||
| export * from './safe_json_rpc_client.js'; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
64 changes: 64 additions & 0 deletions
64
yarn-project/foundation/src/json-rpc/client/safe_json_rpc_client.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| import { format } from 'util'; | ||
|
|
||
| import { createDebugLogger } from '../../log/logger.js'; | ||
| import { type ApiSchema, type ApiSchemaFor, schemaHasMethod } from '../../schemas/api.js'; | ||
| import { jsonStringify2 } from '../convert.js'; | ||
| import { defaultFetch } from './json_rpc_client.js'; | ||
|
|
||
| export { jsonStringify } from '../convert.js'; | ||
|
|
||
| /** | ||
| * Creates a Proxy object that delegates over RPC and validates outputs against a given schema. | ||
| * The server is expected to be a JsonRpcServer. | ||
| * @param host - The host URL. | ||
| * @param schema - The api schema to validate returned data against. | ||
| * @param useApiEndpoints - Whether to use the API endpoints or the default RPC endpoint. | ||
| * @param namespaceMethods - String value (or false/empty) to namespace all methods sent to the server. e.g. 'getInfo' -\> 'pxe_getInfo' | ||
| * @param fetch - The fetch implementation to use. | ||
| */ | ||
| export function createSafeJsonRpcClient<T extends object>( | ||
| host: string, | ||
| schema: ApiSchemaFor<T>, | ||
| useApiEndpoints: boolean = false, | ||
| namespaceMethods?: string | false, | ||
| fetch = defaultFetch, | ||
| log = createDebugLogger('json-rpc:client'), | ||
| ): T { | ||
| let id = 0; | ||
| const request = async (methodName: string, params: any[]): Promise<any> => { | ||
| if (!schemaHasMethod(schema, methodName)) { | ||
| throw new Error(`Unspecified method ${methodName} in client schema`); | ||
| } | ||
| const method = namespaceMethods ? `${namespaceMethods}_${methodName}` : methodName; | ||
| const body = { jsonrpc: '2.0', id: id++, method, params }; | ||
|
|
||
| log.debug(format(`request`, method, params)); | ||
| const res = await fetch(host, method, body, useApiEndpoints, undefined, jsonStringify2); | ||
| log.debug(format(`result`, method, res)); | ||
|
|
||
| if (res.error) { | ||
| throw res.error; | ||
| } | ||
| // TODO: Why check for string null and undefined? | ||
| if ([null, undefined, 'null', 'undefined'].includes(res.result)) { | ||
| return; | ||
| } | ||
|
|
||
| return (schema as ApiSchema)[methodName].returnType().parse(res.result); | ||
| }; | ||
|
|
||
| // Intercept any RPC methods with a proxy | ||
| const proxy = new Proxy( | ||
| {}, | ||
| { | ||
| get: (target, method: string) => { | ||
| if (['then', 'catch'].includes(method)) { | ||
| return Reflect.get(target, method); | ||
| } | ||
| return (...params: any[]) => request(method, params); | ||
| }, | ||
| }, | ||
| ) as T; | ||
|
|
||
| return proxy; | ||
| } |
Oops, something went wrong.