Skip to content

Client: make JSON-RPC payload limits configurable for eth and engine servers #3993

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Client: make JSON-RPC payload limits configurable for eth and engine servers #3993

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0eba2ce
Client: make JSON-RPC payload limits configurable for eth and engine …
ether-wan Apr 18, 2025
dd46b41
Client: add tests and config references for JSON-RPC payload limits
ether-wan Apr 19, 2025
5e47e75
Merge branch 'master' into client-rpc-payload-limit
am1r021 Apr 20, 2025
91a2c93
Merge branch 'master' into client-rpc-payload-limit
jochem-brouwer Apr 24, 2025
dcfa7fa
Merge branch 'master' into client-rpc-payload-limit
acolytec3 May 21, 2025
c0af203
spellcheck
acolytec3 May 21, 2025
c5defc6
fix types
acolytec3 May 21, 2025
95aae7c
Update packages/client/src/config.ts
jochem-brouwer May 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/client/bin/repl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ const setupClient = async (
jwtSecret: '',
rpcEngineAuth: false,
rpcCors: '',
rpcEthMaxPayload: args.rpcEthMaxPayload ?? '5mb',
rpcEngineMaxPayload: args.rpcEngineMaxPayload ?? '15mb',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with these limits, but could you motivate this choice?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also point these to RPC_ENGINE_MAXPAYLOAD_DEFAULT (also for eth)?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with these limits, but could you motivate this choice?

For the eth RPC size limit, I based it on the average block sizes observed on Etherscan. The highest daily average recorded was ~0.25 MB, so I multiplied it by 20 in anticipation of more intensive use of block space.

For the engine RPC size limit, according to EIP-4844 the total maximum blob data per block is ~0.75 MB, and I also multiplied it by 20.

This provides a safety margin while still avoiding excessive resource usage. Maybe that's too much, what do you think? 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the eth RPC engine is intended to be used by "third parties" and it is OK to expose this port to others. For instance providers like Infura/Quicknode also expose those. Also note that this is the limit of incoming messages, so actual requests. Not the reports. So for eth one could request eth_getBlock and this request itself would be very small (just a block hash or number). But the response will be bigger (block itself).

For engine, this is a trusted RPC endpoint and one can only reach it with authentication. This is also the endpoint from which the consensus layer (CL) will send new payloads (new block-like requests). So we can up that limit.

I'm fine with these limits but just wanted to check your motivation, thanks 😄 👍

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification!
I’ll make sure to better explain my motivations in future PRs 🫡

})

return { client, executionRPC: servers[0], engineRPC: servers[1] }
Expand Down
7 changes: 7 additions & 0 deletions packages/client/bin/startRPC.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ export type RPCArgs = {
jwtSecret?: string
rpcEngineAuth: boolean
rpcCors: string
rpcEthMaxPayload: string
rpcEngineMaxPayload: string
}

/**
Expand Down Expand Up @@ -97,7 +99,10 @@ export function startRPCServers(client: EthereumClient, args: RPCArgs) {
rpcCors,
rpcDebug,
rpcDebugVerbose,
rpcEthMaxPayload,
rpcEngineMaxPayload,
} = args

const manager = new RPCManager(client, config)
const { logger } = config
const jwtSecret =
Expand Down Expand Up @@ -136,6 +141,7 @@ export function startRPCServers(client: EthereumClient, args: RPCArgs) {
: req.body.method.includes('engine_') === false,
}
: undefined,
maxPayload: rpcEthMaxPayload,
})
rpcHttpServer.listen(rpcPort, rpcAddr)
logger.info(
Expand Down Expand Up @@ -191,6 +197,7 @@ export function startRPCServers(client: EthereumClient, args: RPCArgs) {
jwtSecret,
}
: undefined,
maxPayload: rpcEngineMaxPayload,
})
rpcHttpServer.listen(rpcEnginePort, rpcEngineAddr)
logger.info(
Expand Down
10 changes: 10 additions & 0 deletions packages/client/bin/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,16 @@ export function getArgs(): ClientOpts {
boolean: true,
default: true,
})
.option('rpcEthMaxPayload', {
describe: 'Define max JSON payload size for eth/debug RPC requests',
string: true,
default: '5mb',
})
.option('rpcEngineMaxPayload', {
describe: 'Define max JSON payload size for engine RPC requests',
string: true,
default: '15mb',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you point this to RPC_ENGINE_MAXPAYLOAD_DEFAULT? Also for rpcEthMaxPayload 😄 👍

})
.option('jwtSecret', {
describe: 'Provide a file containing a hex encoded jwt secret for Engine RPC server',
coerce: (arg: string) => (arg ? path.resolve(arg) : undefined),
Expand Down
20 changes: 19 additions & 1 deletion packages/client/src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,16 @@ export interface ConfigOptions {
* Enables Prometheus Metrics that can be collected for monitoring client health
*/
prometheusMetrics?: PrometheusMetrics

/**
* Max JSON payload size for eth/eth RPC requests (untrusted)
*/
rpcEthMaxPayload?: string

/*
* Max JSON payload size for engine RPC requests (trusted)
*/
rpcEngineMaxPayload?: string
}

export class Config {
Expand All @@ -356,7 +366,7 @@ export class Config {
public readonly events: EventEmitter<EventParams>

public static readonly CHAIN_DEFAULT = Mainnet
public static readonly SYNCMODE_DEFAULT = SyncMode.Full
public static readonly SYNCMODE_DEFAULT = SyncMode.None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this changed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry changed it while testing, fixing it now

public static readonly DATADIR_DEFAULT = `./datadir`
public static readonly PORT_DEFAULT = 30303
public static readonly MAXPERREQUEST_DEFAULT = 100
Expand All @@ -365,6 +375,8 @@ export class Config {
public static readonly MINPEERS_DEFAULT = 1
public static readonly MAXPEERS_DEFAULT = 25
public static readonly DNSADDR_DEFAULT = '8.8.8.8'
public static readonly RPC_ETH_MAXPAYLOAD_DEFAULT = '5mb'
public static readonly RPC_ENGINE_MAXPAYLOAD_DEFAULT = '15mb'
public static readonly EXECUTION = true
public static readonly NUM_BLOCKS_PER_ITERATION = 100
public static readonly ACCOUNT_CACHE = 400000
Expand Down Expand Up @@ -458,6 +470,9 @@ export class Config {

public readonly blobsAndProofsCacheBlocks: number

public readonly rpcEthMaxPayload: string
public readonly rpcEngineMaxPayload: string

public synchronized: boolean
public lastSynchronized?: boolean
/** lastSyncDate in ms */
Expand Down Expand Up @@ -563,6 +578,9 @@ export class Config {
this.blobsAndProofsCacheBlocks =
options.blobsAndProofsCacheBlocks ?? Config.BLOBS_AND_PROOFS_CACHE_BLOCKS

this.rpcEthMaxPayload = options.rpcEthMaxPayload ?? Config.RPC_ETH_MAXPAYLOAD_DEFAULT
this.rpcEngineMaxPayload = options.rpcEngineMaxPayload ?? Config.RPC_ENGINE_MAXPAYLOAD_DEFAULT

this.discDns = this.getDnsDiscovery(options.discDns)
this.discV4 = options.discV4 ?? true

Expand Down
2 changes: 2 additions & 0 deletions packages/client/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,8 @@ export interface ClientOpts {
skipEngineExec?: boolean
ignoreStatelessInvalidExecs?: boolean
useJsCrypto?: boolean
rpcEthMaxPayload?: string
rpcEngineMaxPayload?: string
}

export type PrometheusMetrics = {
Expand Down
7 changes: 4 additions & 3 deletions packages/client/src/util/rpc.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type CreateRPCServerListenerOpts = {
withEngineMiddleware?: WithEngineMiddleware
}
type CreateWSServerOpts = CreateRPCServerListenerOpts & { httpServer?: jayson.HttpServer }
type CreateHTTPServerOpts = CreateRPCServerListenerOpts & { maxPayload: string }
type WithEngineMiddleware = { jwtSecret: Uint8Array; unlessFn?: (req: IncomingMessage) => boolean }

export type MethodConfig = (typeof MethodConfig)[keyof typeof MethodConfig]
Expand Down Expand Up @@ -182,13 +183,13 @@ function checkHeaderAuth(req: any, jwtSecret: Uint8Array): void {
}
}

export function createRPCServerListener(opts: CreateRPCServerListenerOpts): jayson.HttpServer {
const { server, withEngineMiddleware, RPCCors } = opts
export function createRPCServerListener(opts: CreateHTTPServerOpts): jayson.HttpServer {
const { server, withEngineMiddleware, RPCCors, maxPayload } = opts

const app = Connect()
if (typeof RPCCors === 'string') app.use(cors({ origin: RPCCors }))
// GOSSIP_MAX_SIZE_BELLATRIX is proposed to be 10MiB
app.use(JSONParser({ limit: '11mb' }))
app.use(JSONParser({ limit: maxPayload }))

if (withEngineMiddleware) {
const { jwtSecret, unlessFn } = withEngineMiddleware
Expand Down
2 changes: 2 additions & 0 deletions packages/client/test/util/rpc.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ describe('[Util/RPC]', () => {
const httpServer = createRPCServerListener({
server,
withEngineMiddleware: { jwtSecret: new Uint8Array(32) },
maxPayload: '15mb',
})
const wsServer = createWsRPCServerListener({
server,
Expand Down Expand Up @@ -74,6 +75,7 @@ describe('[Util/RPC]', () => {
const httpServer = createRPCServerListener({
server,
withEngineMiddleware: { jwtSecret: new Uint8Array(32) },
maxPayload: '15mb',
})
const wsServer = createWsRPCServerListener({
server,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a test here to verify that for both engine/eth the config is handled correctly? You can likely just set it to some low limit and then try to push more data over the RPC than the limit to verify that it fails.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done ✅

Expand Down
Loading