feat(rate-limit): port read bucket, usage headers and path-based API keys to v0.123 - #1743
Merged
Merged
Conversation
…rs (#1741) Rate limiting was method-agnostic: the middleware checked the quota before the JSON-RPC body was parsed, so `gen_getContractCode` — a single indexed read — cost exactly what a write fanning out to LLM validators cost. That forces the limits to be sized for the expensive case, which starves the cheap read traffic that batch tooling generates. Reads that never enter the GenVM now meter into their own bucket at RATE_LIMIT_READ_MULTIPLIER (default 10x) of the tier limits. The standard bucket keeps its key shape and numbers, so limits in flight at deploy time carry over rather than resetting. The allowlist is maintained by hand and deliberately conservative. Two methods that read like lookups are excluded because they build a Node backed by a GenVMManager (gen_getContractSchema, gen_getContractSchemaForCode), and eth_call is excluded for the same reason despite appearing in the DISABLE_INFO_LOGS_ENDPOINTS env list that otherwise looks like the natural source for this. Anything ambiguous — unparseable, oversized, or a batch with one expensive member — is charged to the stricter bucket. Responses now carry X-RateLimit-Limit/Remaining/Reset/Window/Bucket for the window closest to exhaustion, so clients can pace themselves instead of discovering the ceiling by hitting it. Reset is the time until the oldest entry ages out, which is the honest answer for a sliding window. These are listed in the CORS expose_headers, without which browsers hide them from JS. Finally, gen_getContractCode no longer loads the whole contract state to read one slot out of it. ContractSnapshot pulls the entire `data` JSONB — every storage slot the contract owns — which for a contract holding a large vector store is a substantial fetch and deserialize on a call that batch tooling polls hard. The slot is now extracted in SQL. Postgres still detoasts the JSONB server-side, so this narrows transfer and parse cost rather than eliminating the read; legacy and undeployed row shapes defer to the original path to keep their error semantics intact. The Lua script gains a test that actually executes it under a stubbed redis.call. It was previously covered only through mocked evalsha, so an error in the script body would have surfaced first in production, on every /api request.
Keys could only be supplied as an X-API-Key header, which makes Studio
unusable from most of the EVM toolchain. MetaMask's "Add network" takes a
URL and has no header field at all; the same is true of foundry.toml,
--rpc-url, and most viem/ethers setups. Every major provider puts the key in
the path for this reason (Alchemy /v2/<key>, Infura /v3/<key>).
This is not hypothetical: an integrator on a shared Studio key told us they
would have to stand up a backend proxy "for them to be able to use it
properly as most of the use comes from testing deployments through
metamask". That proxy's whole job would be turning a URL into a header, and
a shared proxy is also what pushes teams onto one shared key instead of one
key per person.
POST /api/{api_key} now routes to the same JSON-RPC handler, with the header
still accepted for server-side callers. The path wins when both are present,
since it is what the caller typed rather than something an intermediary may
have injected. Only `glk_`-prefixed segments count as keys: any other
segment falls through to anonymous rather than erroring, while a mistyped
real key is still treated as a key so it fails loudly as invalid instead of
silently dropping to anonymous limits.
The middleware's path gate now covers every path the route can match, not
just key-shaped ones. A path served by the route but missed by the gate
would be an unlimited, unauthenticated RPC endpoint, so a test pins that
correspondence.
Keys in URLs get written down in places headers are not, and two of those
were ours: uvicorn access logging is on, and Sentry runs with
send_default_pii and full trace sampling, so request URLs leave the process
on every transaction, not only on errors. api_key_redaction scrubs keys from
both — a logging filter covering uvicorn's args-based access line, and
before_send/before_send_transaction hooks that walk the whole event rather
than known fields. Without this, path keys would be a downgrade on
header-only rather than an improvement.
The ported allowlist predates three methods that exist only on this branch. sim_getFeeConfig and gen_getTransactionStatusDetails are cheap reads — a config lookup and a transactions_processor read respectively, neither touching the GenVM. sim_estimateTransactionFees is not. It takes validators_manager and genvm_manager and runs the contract through sim_call to measure fees, so it costs a full execution. Worth flagging because eth_estimateGas *is* allowlisted — that one returns a hardcoded constant. The names are nearly identical and the costs are not, which is the same shape of trap as eth_call and the two schema methods already documented here.
Member
Author
|
/run-e2e |
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Member
Author
|
/run-e2e studio |
Member
Author
|
/run-e2e |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports #1741 and #1742 from
v0.121, plus the classification work those PRs could not do because the methods do not exist on that branch.What
RATE_LIMIT_READ_MULTIPLIER(default 10x);X-RateLimit-*headers on every response;gen_getContractCodeextracts its slot in SQL instead of loading the whole state JSONBPOST /api/{api_key}accepts the key in the URL, with log and Sentry scrubbing so keys do not leak into CloudWatch or a third-party error trackerv0.123-devWhy
Both were merged to
v0.121and are covered in detail on those PRs. This is the port, so review can reasonably focus on the merge resolutions and the new classification rather than re-reviewing the designs.Decisions made
sim_estimateTransactionFeesis not a cheap read. It takesvalidators_managerandgenvm_managerand runs the contract throughsim_callto measure fees, so it costs a full execution. This matters becauseeth_estimateGasis allowlisted — that one returns a hardcoded constant. Near-identical names, wildly different costs. It is the third instance of this trap on this allowlist, aftereth_calland the two schema methods.sim_getFeeConfig(config lookup) andgen_getTransactionStatusDetails(transactions_processor read) are cheap and allowlisted.Merge resolutions
Three places where
v0.123-devhad diverged:RateLimitMiddlewarebefore CORS so that CORS decorates short-circuit responses such as 429s, and setsallow_credentials=False. Theexpose_headerslist from feat(rate-limit): meter cheap reads separately and expose usage headers #1741 auto-merged into the relocated CORS block; verified at runtime that CORS is still outermost, so that fix is preserved.docker-compose.ymlconflicted between this branch'sGENLAYER_STUDIO_*fee-config vars and feat(rate-limit): meter cheap reads separately and expose usage headers #1741'sRATE_LIMIT_READ_MULTIPLIER. Both kept.contract_snapshot.pygainedgenvm_executor_selectorhere; the newfetch_deployed_code_b64is additive and merged cleanly alongside it.One stale comment inherited from this branch:
# This public RPC uses header-based API keys and no cookie authentication.sits above the CORS block and is now only half true, since keys can ride in the path. Left alone to keep the diff to the port, but worth a follow-up.Testing done
appobject that both/apiand/api/{api_key}register, that middleware order isCORSMiddlewarethenRateLimitMiddleware(CORS outermost, as this branch intends), and that a path-keyed POST returns 200Checks
Reviewing tips
sim_estimateTransactionFeesbeing excluded — worth confirming independently, since getting it wrong hands out free contract executions._is_rpc_pathin the middleware must keep covering every path the/api/{api_key}route matches; a test pins this.User facing release notes
X-RateLimit-*headers so clients can pace themselves rather than discovering the limit by hitting it./api/<key>) as well as theX-API-Keyheader, making keyed access work from MetaMask, Foundry and other tools that accept only an RPC URL.