-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Adapt network manager handlers to copy rather than mutate requests #8090
Description
The hook handlers for onRequest previously ensured that a passed in request would not be modified using a deepClone. We have remove this clone as a performance improvement - but this technically alters the ownership commitments of the onRequest hook.
So at a high level:
const myRequest = {...}
const response = await next(myRequest)
doSomething(myRequest)Next previously commited to not mutating myRequest - so doSomething(myRequest) was safe. With the removal of clone we are mutating myRequest within next. In practice this is not currently an issue.
However, we can update the internal handlers applied in onRequest so that instead of updating in place, they return a new object.
See
hardhat/v-next/hardhat/src/internal/builtin-plugins/network-manager/hook-handlers/network.ts
Lines 45 to 78 in 3268b33
| const requestHandlers = await initializationMutex.exclusiveRun( | |
| async () => { | |
| let handlersPerConnection = | |
| requestHandlersPerConnection.get(networkConnection); | |
| if (handlersPerConnection === undefined) { | |
| handlersPerConnection = | |
| await createHandlersArray(networkConnection); | |
| requestHandlersPerConnection.set( | |
| networkConnection, | |
| handlersPerConnection, | |
| ); | |
| } | |
| return handlersPerConnection; | |
| }, | |
| ); | |
| // We clone the request to avoid interfering with other hook handlers that | |
| // might be using the original request. | |
| let request = await deepClone(jsonRpcRequest); | |
| for (const handler of requestHandlers) { | |
| const newRequestOrResponse = await handler.handle(request); | |
| if (isJsonRpcResponse(newRequestOrResponse)) { | |
| return newRequestOrResponse; | |
| } | |
| request = newRequestOrResponse; | |
| } | |
| return next(context, networkConnection, request); |
Metadata
Metadata
Assignees
Labels
Type
Projects
Status