Skip to content

Commit

Permalink
chore: Validate RPC inputs (#9672)
Browse files Browse the repository at this point in the history
**Adds schemas for every API.** Every API exposed via JSON RPC now
requires a zod schema (see #9656 for more context on the rationale for
this change). All schemas are in `circuit-types/interfaces`, and look
like:


https://github.com/AztecProtocol/aztec-packages/blob/3e78ec721285fcd533cff61329a8e156958e2d65/yarn-project/circuit-types/src/interfaces/prover-node.ts#L33-L45

These schemas are type-checked against the interface via the
`ApiSchemaFor` utility type, so if the interface changes, schemas are
required by the compiler to change as well. Schemas are now used in the
JSON RPC server to 1) identify which methods are exposed (so we no
longer need the method disallowlist) and 2) parse their arguments. The
JSON RPC server, once it has identified the method to be called, grabs
the arguments schema and funnels the result of a vanilla JSON parse
through it.

Every type or struct that is exposed via an interface now has an
associated schema, which is referenced in the API for parsing. Schemas
both validate input and hydrate instances. This means that we no longer
set a `type` property to identify how to hydrate each object in a
request during deserialization, which was a security risk.


https://github.com/AztecProtocol/aztec-packages/blob/3e78ec721285fcd533cff61329a8e156958e2d65/yarn-project/circuit-types/src/l2_block.ts#L24-L32

Schemas are also used in the JSON RPC client for deserializing the
result types. Again, this lets us remove the `type` parameter from all
serialized entities, though this is still present in since it is
required by the `TypeRegistry` (still to be removed) which is only used
in the snapshot manager.

All schemas are tested via mini integration tests. These tests define a
mock implementation for each service, use it for setting up a JSON RPC
server, starting it in a free port, and test calling every method
through JSON RPC.


https://github.com/AztecProtocol/aztec-packages/blob/3e78ec721285fcd533cff61329a8e156958e2d65/yarn-project/circuit-types/src/interfaces/prover-node.test.ts#L12-L31

These changes prompted other changes. For instance, we introduced the
following changes to APIs:

- `ProvingJobSource.rejectProvingJob` now accepts a reason `string`
instead of an `Error` type
- `PXE.getEvents(type)` is removed in favor of `PXE.getEncryptedEvents`
and `PXE.getUnencryptedEvents` since both methods required different
arguments

We also removed service-management methods (ie `stop`) from interfaces.
We were inadvertently calling `stop` on remote instances over http when
we shouldn't have. We also typed some previously untyped interfaces,
such as the TXE's.

Fixes #9455
  • Loading branch information
spalladino authored Nov 10, 2024
1 parent 5246251 commit 6554122
Show file tree
Hide file tree
Showing 268 changed files with 5,990 additions and 3,007 deletions.
1 change: 1 addition & 0 deletions yarn-project/.earthlyignore
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ aztec-faucet/data*
aztec-node/data*
aztec/log
circuits.js/fixtures/*.json
circuit-types/src/test/artifacts
docs/dist
end-to-end/addresses.json
end-to-end/log
Expand Down
1 change: 1 addition & 0 deletions yarn-project/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ aztec-faucet/data*
aztec-node/data*
aztec/log
circuits.js/fixtures/*.json
circuit-types/src/test/artifacts
docs/dist
end-to-end/addresses.json
end-to-end/flame_graph
Expand Down
6 changes: 4 additions & 2 deletions yarn-project/archiver/src/factory.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { type ArchiverApi, type Service } from '@aztec/circuit-types';
import { type ContractClassPublic } from '@aztec/circuits.js';
import { createDebugLogger } from '@aztec/foundation/log';
import { type Maybe } from '@aztec/foundation/types';
import { createStore } from '@aztec/kv-store/utils';
import { getCanonicalProtocolContract, protocolContractNames } from '@aztec/protocol-contracts';
import { type TelemetryClient } from '@aztec/telemetry-client';
Expand All @@ -8,13 +10,13 @@ import { NoopTelemetryClient } from '@aztec/telemetry-client/noop';
import { Archiver } from './archiver/archiver.js';
import { type ArchiverConfig } from './archiver/config.js';
import { KVArchiverDataStore } from './archiver/index.js';
import { createArchiverClient } from './rpc/archiver_client.js';
import { createArchiverClient } from './rpc/index.js';

export async function createArchiver(
config: ArchiverConfig,
telemetry: TelemetryClient = new NoopTelemetryClient(),
opts: { blockUntilSync: boolean } = { blockUntilSync: true },
) {
): Promise<ArchiverApi & Maybe<Service>> {
if (!config.archiverUrl) {
const store = await createStore('archiver', config, createDebugLogger('aztec:archiver:lmdb'));
const archiverStore = new KVArchiverDataStore(store, config.maxLogs);
Expand Down
29 changes: 0 additions & 29 deletions yarn-project/archiver/src/rpc/archiver_client.ts

This file was deleted.

35 changes: 0 additions & 35 deletions yarn-project/archiver/src/rpc/archiver_server.ts

This file was deleted.

13 changes: 11 additions & 2 deletions yarn-project/archiver/src/rpc/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,11 @@
export * from './archiver_client.js';
export * from './archiver_server.js';
import { type ArchiverApi, ArchiverApiSchema } from '@aztec/circuit-types';
import { createSafeJsonRpcClient, makeFetch } from '@aztec/foundation/json-rpc/client';
import { createSafeJsonRpcServer } from '@aztec/foundation/json-rpc/server';

export function createArchiverClient(url: string, fetch = makeFetch([1, 2, 3], true)): ArchiverApi {
return createSafeJsonRpcClient<ArchiverApi>(url, ArchiverApiSchema, false, 'archiver', fetch);
}

export function createArchiverRpcServer(handler: ArchiverApi) {
return createSafeJsonRpcServer(handler, ArchiverApiSchema);
}
61 changes: 3 additions & 58 deletions yarn-project/aztec-node/src/aztec-node/http_rpc_server.ts
Original file line number Diff line number Diff line change
@@ -1,66 +1,11 @@
import {
type AztecNode,
EncryptedL2NoteLog,
EncryptedNoteL2BlockL2Logs,
EpochProofQuote,
ExtendedUnencryptedL2Log,
L2Block,
LogId,
NullifierMembershipWitness,
PublicDataWitness,
PublicSimulationOutput,
SiblingPath,
Tx,
TxEffect,
TxHash,
TxReceipt,
UnencryptedL2BlockL2Logs,
} from '@aztec/circuit-types';
import { FunctionSelector, Header, PublicKeys } from '@aztec/circuits.js';
import { NoteSelector } from '@aztec/foundation/abi';
import { AztecAddress } from '@aztec/foundation/aztec-address';
import { Buffer32 } from '@aztec/foundation/buffer';
import { EthAddress } from '@aztec/foundation/eth-address';
import { Fr } from '@aztec/foundation/fields';
import { JsonRpcServer } from '@aztec/foundation/json-rpc/server';
import { type AztecNode, AztecNodeApiSchema } from '@aztec/circuit-types';
import { createSafeJsonRpcServer } from '@aztec/foundation/json-rpc/server';

/**
* Wrap an AztecNode instance with a JSON RPC HTTP server.
* @param node - The AztecNode
* @returns An JSON-RPC HTTP server
*/
export function createAztecNodeRpcServer(node: AztecNode) {
const rpc = new JsonRpcServer(
node,
{
AztecAddress,
EthAddress,
ExtendedUnencryptedL2Log,
Fr,
FunctionSelector,
Header,
L2Block,
TxEffect,
LogId,
TxHash,
Buffer32,
PublicDataWitness,
PublicKeys,
SiblingPath,
},
{
EncryptedNoteL2BlockL2Logs,
EncryptedL2NoteLog,
NoteSelector,
NullifierMembershipWitness,
PublicSimulationOutput,
Tx,
TxReceipt,
UnencryptedL2BlockL2Logs,
EpochProofQuote,
},
// disable methods not part of the AztecNode interface
['start', 'stop'],
);
return rpc;
return createSafeJsonRpcServer(node, AztecNodeApiSchema);
}
6 changes: 4 additions & 2 deletions yarn-project/aztec-node/src/aztec-node/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
PublicDataWitness,
PublicSimulationOutput,
type SequencerConfig,
type Service,
SiblingPath,
type Tx,
type TxEffect,
Expand All @@ -30,6 +31,7 @@ import {
TxStatus,
type TxValidator,
type WorldStateSynchronizer,
tryStop,
} from '@aztec/circuit-types';
import {
type ARCHIVE_HEIGHT,
Expand Down Expand Up @@ -89,7 +91,7 @@ export class AztecNodeService implements AztecNode {
constructor(
protected config: AztecNodeConfig,
protected readonly p2pClient: P2P,
protected readonly blockSource: L2BlockSource,
protected readonly blockSource: L2BlockSource & Partial<Service>,
protected readonly encryptedLogsSource: L2LogsSource,
protected readonly unencryptedLogsSource: L2LogsSource,
protected readonly contractDataSource: ContractDataSource,
Expand Down Expand Up @@ -373,7 +375,7 @@ export class AztecNodeService implements AztecNode {
await this.sequencer?.stop();
await this.p2pClient.stop();
await this.worldStateSynchronizer.stop();
await this.blockSource.stop();
await tryStop(this.blockSource);
await this.telemetry.stop();
this.log.info(`Stopped`);
}
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/aztec.js/src/contract/batch_call.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ export class BatchCall extends BaseContractInteraction {

const unconstrainedCalls = unconstrained.map(async indexedCall => {
const call = indexedCall[0];
return [await this.wallet.simulateUnconstrained(call.name, call.args, call.to, options?.from), indexedCall[1]];
return [
await this.wallet.simulateUnconstrained(call.name, call.args, call.to, options?.from),
indexedCall[1],
] as const;
});

const [unconstrainedResults, simulatedTx] = await Promise.all([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,12 @@ export class FeeJuicePaymentMethodWithClaim extends FeeJuicePaymentMethod {
name: 'claim',
selector,
isStatic: false,
args: [this.sender, this.claim.claimAmount, this.claim.claimSecret, new Fr(this.claim.messageLeafIndex)],
args: [
this.sender.toField(),
this.claim.claimAmount,
this.claim.claimSecret,
new Fr(this.claim.messageLeafIndex),
],
returnTypes: [],
type: FunctionType.PRIVATE,
},
Expand Down
4 changes: 2 additions & 2 deletions yarn-project/aztec.js/src/fee/private_fee_payment_method.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export class PrivateFeePaymentMethod implements FeePaymentMethod {
caller: this.paymentContract,
action: {
name: 'setup_refund',
args: [this.feeRecipient, this.wallet.getAddress(), maxFee, nonce],
args: [this.feeRecipient.toField(), this.wallet.getAddress().toField(), maxFee, nonce],
selector: FunctionSelector.fromSignature('setup_refund((Field),(Field),Field,Field)'),
type: FunctionType.PRIVATE,
isStatic: false,
Expand All @@ -81,7 +81,7 @@ export class PrivateFeePaymentMethod implements FeePaymentMethod {
selector: FunctionSelector.fromSignature('fee_entrypoint_private(Field,(Field),Field)'),
type: FunctionType.PRIVATE,
isStatic: false,
args: [maxFee, this.asset, nonce],
args: [maxFee, this.asset.toField(), nonce],
returnTypes: [],
},
];
Expand Down
4 changes: 2 additions & 2 deletions yarn-project/aztec.js/src/fee/public_fee_payment_method.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export class PublicFeePaymentMethod implements FeePaymentMethod {
caller: this.paymentContract,
action: {
name: 'transfer_public',
args: [this.wallet.getAddress(), this.paymentContract, maxFee, nonce],
args: [this.wallet.getAddress().toField(), this.paymentContract.toField(), maxFee, nonce],
selector: FunctionSelector.fromSignature('transfer_public((Field),(Field),Field,Field)'),
type: FunctionType.PUBLIC,
isStatic: false,
Expand All @@ -71,7 +71,7 @@ export class PublicFeePaymentMethod implements FeePaymentMethod {
selector: FunctionSelector.fromSignature('fee_entrypoint_public(Field,(Field),Field)'),
type: FunctionType.PRIVATE,
isStatic: false,
args: [maxFee, this.asset, nonce],
args: [maxFee, this.asset.toField(), nonce],
returnTypes: [],
},
]);
Expand Down
95 changes: 5 additions & 90 deletions yarn-project/aztec.js/src/rpc_clients/pxe_client.ts
Original file line number Diff line number Diff line change
@@ -1,97 +1,12 @@
import {
AuthWitness,
CountedNoteLog,
CountedPublicExecutionRequest,
EncryptedL2Log,
EncryptedL2NoteLog,
EncryptedNoteL2BlockL2Logs,
EventMetadata,
ExtendedNote,
ExtendedUnencryptedL2Log,
L2Block,
LogId,
Note,
NullifierMembershipWitness,
type PXE,
PrivateExecutionResult,
SiblingPath,
Tx,
TxEffect,
TxExecutionRequest,
TxHash,
TxProvingResult,
TxReceipt,
TxSimulationResult,
UnencryptedL2BlockL2Logs,
UnencryptedL2Log,
UniqueNote,
} from '@aztec/circuit-types';
import {
AztecAddress,
CompleteAddress,
EthAddress,
Fr,
FunctionSelector,
GrumpkinScalar,
Point,
PrivateCircuitPublicInputs,
PublicKeys,
} from '@aztec/circuits.js';
import { EventSelector, NoteSelector } from '@aztec/foundation/abi';
import { Buffer32 } from '@aztec/foundation/buffer';
import { createJsonRpcClient, makeFetch } from '@aztec/foundation/json-rpc/client';
import { type PXE, PXESchema } from '@aztec/circuit-types';
import { createSafeJsonRpcClient, makeFetch } from '@aztec/foundation/json-rpc/client';

/**
* Creates a JSON-RPC client to remotely talk to PXE.
* @param url - The URL of the PXE.
* @param fetch - The fetch implementation to use.
* @returns A JSON-RPC client of PXE.
*/
export const createPXEClient = (url: string, fetch = makeFetch([1, 2, 3], false)): PXE =>
createJsonRpcClient<PXE>(
url,
{
AuthWitness,
AztecAddress,
CompleteAddress,
FunctionSelector,
EthAddress,
EventSelector,
ExtendedNote,
UniqueNote,
ExtendedUnencryptedL2Log,
Fr,
GrumpkinScalar,
L2Block,
TxEffect,
LogId,
Note,
Point,
PublicKeys,
TxExecutionRequest,
TxHash,
Buffer32,
SiblingPath,
},
{
EncryptedNoteL2BlockL2Logs,
EncryptedL2NoteLog,
EncryptedL2Log,
EventMetadata,
UnencryptedL2Log,
NoteSelector,
NullifierMembershipWitness,
TxSimulationResult,
TxProvingResult,
PrivateCircuitPublicInputs,
PrivateExecutionResult,
CountedPublicExecutionRequest,
CountedNoteLog,
Tx,
TxReceipt,
UnencryptedL2BlockL2Logs,
},
false,
'pxe',
fetch,
) as PXE;
export function createPXEClient(url: string, fetch = makeFetch([1, 2, 3], false)): PXE {
return createSafeJsonRpcClient<PXE>(url, PXESchema, false, 'pxe', fetch);
}
2 changes: 1 addition & 1 deletion yarn-project/aztec.js/src/utils/abi_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export type FieldLike = Fr | Buffer | bigint | number | { /** Converts to field
export type EthAddressLike = { /** Wrapped address */ address: FieldLike } | EthAddress;

/** Any type that can be converted into an AztecAddress Aztec.nr struct. */
export type AztecAddressLike = { /** Wrapped address */ address: FieldLike } | AztecAddress;
export type AztecAddressLike = { /** Wrapped address */ address: FieldLike } | AztecAddress | Fr;

/** Any type that can be converted into a FunctionSelector Aztec.nr struct. */
export type FunctionSelectorLike = FieldLike | FunctionSelector;
Expand Down
Loading

0 comments on commit 6554122

Please sign in to comment.