refactor: conditionally avoid network handlers#8103
Conversation
🦋 Changeset detectedLatest commit: 14b335a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
kanej
added
no docs needed
kanej
force-pushed
the
feat/conditionally-avoid-network-handlers
branch
2 times, most recently
from
6e169c3 to
04edee8
Compare
| readonly #methods: ReadonlySet<string> = new Set([ | ||
| "eth_accounts", | ||
| "eth_requestAccounts", | ||
| "eth_sign", | ||
| "personal_sign", | ||
| "eth_signTypedData_v4", | ||
| "eth_sendTransaction", | ||
| ]); | ||
|
|
There was a problem hiding this comment.
This list is based on #resolveRequest method - which returns null if no change is required. It is covered by tests, hence this list should be safe.
| public async handle( | ||
| jsonRpcRequest: JsonRpcRequest, | ||
| ): Promise<JsonRpcRequest | JsonRpcResponse> { | ||
| if (!this.isSupportedMethod(jsonRpcRequest)) { | ||
| return jsonRpcRequest; | ||
| } | ||
|
|
||
| const response = await this.#resolveRequest(jsonRpcRequest); | ||
| if (response !== null) { | ||
| return response; | ||
| } | ||
|
|
||
| await this.#modifyRequest(jsonRpcRequest); | ||
|
|
||
| return jsonRpcRequest; | ||
| } | ||
|
|
There was a problem hiding this comment.
My apologies on this one, I moved a private method below it hence the appearence of total change.
The only actual change is the addition of an explicit isSupportedMethod check at the top, which is intended to replicate the implicit check wihtin #resolveRequest.
There was a problem hiding this comment.
Pull request overview
This PR refactors the network request-handling pipeline to synchronously skip handlers that don’t apply to a given JSON-RPC method, reducing unnecessary async/await overhead in the network stack.
Changes:
- Extend the
RequestHandlerinterface with anisSupportedMethodguard for method-based applicability checks. - Update built-in gas, sender, accounts, and chain-id handlers to declare supported methods and (in most cases) re-check support inside
handle. - Update the
onRequestpipeline to skip handlers viaisSupportedMethodbefore awaitinghandle, and add a changeset documenting the performance improvement.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/hardhat/src/internal/builtin-plugins/network-manager/request-handlers/types.ts | Adds isSupportedMethod to the handler contract to enable early skipping. |
| packages/hardhat/src/internal/builtin-plugins/network-manager/request-handlers/handlers/gas/fixed-gas-price-handler.ts | Declares supported methods and reuses the guard in handle. |
| packages/hardhat/src/internal/builtin-plugins/network-manager/request-handlers/handlers/gas/fixed-gas-handler.ts | Declares supported methods and reuses the guard in handle. |
| packages/hardhat/src/internal/builtin-plugins/network-manager/request-handlers/handlers/gas/automatic-gas-price-handler.ts | Declares supported methods and reuses the guard in handle. |
| packages/hardhat/src/internal/builtin-plugins/network-manager/request-handlers/handlers/gas/automatic-gas-handler.ts | Declares supported methods and reuses the guard in handle. |
| packages/hardhat/src/internal/builtin-plugins/network-manager/request-handlers/handlers/chain-id/chain-id-handler.ts | Adds isSupportedMethod implementation for the chain-id validator handler. |
| packages/hardhat/src/internal/builtin-plugins/network-manager/request-handlers/handlers/accounts/sender.ts | Centralizes method applicability for sender population and early-returns when unsupported. |
| packages/hardhat/src/internal/builtin-plugins/network-manager/request-handlers/handlers/accounts/local-accounts.ts | Adds method applicability gating to avoid running local-accounts logic for unrelated RPC methods. |
| packages/hardhat/src/internal/builtin-plugins/network-manager/request-handlers/handlers/accounts/hd-wallet-handler.ts | Minor formatting change (blank line). |
| packages/hardhat/src/internal/builtin-plugins/network-manager/hook-handlers/network.ts | Skips handlers synchronously via isSupportedMethod before awaiting handle. |
| .changeset/cuddly-insects-invent.md | Documents the performance improvement as a patch changeset. |
| public isSupportedMethod(_jsonRpcRequest: JsonRpcRequest): boolean { | ||
| // This handler should be executed for every request, | ||
| // as it needs to validate the chain ID before any other | ||
| // handler processes the request. |
There was a problem hiding this comment.
isSupportedMethod currently always returns true, which means onRequest will still await this handler for every request even when it will immediately no-op (e.g. eth_chainId / net_version, and after #alreadyValidated becomes true). To fully realize the PR’s goal of skipping async handlers, consider returning false for methods this handler short-circuits and when #alreadyValidated is already true (the handler already treats that as a no-op in handle).
| public isSupportedMethod(_jsonRpcRequest: JsonRpcRequest): boolean { | |
| // This handler should be executed for every request, | |
| // as it needs to validate the chain ID before any other | |
| // handler processes the request. | |
| public isSupportedMethod(jsonRpcRequest: JsonRpcRequest): boolean { | |
| if (this.#alreadyValidated) { | |
| return false; | |
| } | |
| if ( | |
| jsonRpcRequest.method === "eth_chainId" || | |
| jsonRpcRequest.method === "net_version" | |
| ) { | |
| return false; | |
| } |
There was a problem hiding this comment.
I have pulled the conditional logic up into isSupportedMethod, and now use that to determine whether to short-circuit on in handle.
Add a `isSupportedMethod` to each network handler to allow skipping without have to delegate into the async `handle` method. The same supported check has been wired into the handle request as a guard and to ensure the same logic is in play. Resolves #8047
kanej
force-pushed
the
feat/conditionally-avoid-network-handlers
branch
from
04edee8 to
14b335a
Compare
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Add metadata to each subhandler in
onRequestspecifying which which RPC Methods are used. The handler will be skipped if the request's method does not match.The goal here is avoid a cycle of async/awaits, instead we do sync checks.
The check is rerun within the handler as a guard and to force an engineer modifying the handler to consider the supported list. This justifies the no additional tests.
Resolves #8047