refactor!(eth-json-rpc-provider): Migrate to JsonRpcEngineV2#7001
Merged
refactor!(eth-json-rpc-provider): Migrate to JsonRpcEngineV2#7001
JsonRpcEngineV2#7001Conversation
JsonRpcEngineV2JsonRpcEngineV2
6cb53fa to
88118a9
Compare
34175ce to
bb89c25
Compare
JsonRpcEngineV2JsonRpcEngineV2
fbc467a to
c0feb25
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
9c8482f to
3d5a68a
Compare
Member
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
JsonRpcEngineV2JsonRpcEngineV2
…hten v2 types
- InternalProvider
- replace rpcHandler with server
- wrap legacy engine via asV2Middleware + JsonRpcServer
- update tests
- asV2Middleware: fix middleware signature
- providerFromMiddleware
- Use providerFromMiddlewareV2 and asV2Middleware to wrap legacy
middleware
- json-rpc-engine:
- add MiddlewareConstraint
- update JsonRpcServer/asV2Middleware generics
4 tasks
mcmire
reviewed
Nov 4, 2025
Contributor
mcmire
left a comment
There was a problem hiding this comment.
Some suggestions but overall makes sense / looks good.
mcmire
reviewed
Nov 5, 2025
Contributor
mcmire
left a comment
There was a problem hiding this comment.
This looks good, but I am going to do another pass tomorrow just to be sure.
mcmire
reviewed
Nov 5, 2025
| return provider; | ||
| >(middleware: LegacyJsonRpcMiddleware<Params, Result>): InternalProvider { | ||
| return providerFromMiddlewareV2( | ||
| asV2Middleware(middleware) as JsonRpcMiddleware<JsonRpcRequest>, |
Contributor
There was a problem hiding this comment.
I see that if we remove the type assertion we get a type error around params. Do we need to do some kind of transformation of params to remove undefined or something?
Member
Author
There was a problem hiding this comment.
It actually fails due to the context generics, specifically by tripping up the InvalidEngine type. I do not know why.
Member
Author
There was a problem hiding this comment.
We put on our thinking caps and figured out why: 4a63eea
Member
Author
|
For posterity: we labeled this |
Member
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
13 tasks
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
2 participants
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.
Explanation
Per #6327, migrates
@metamask/eth-json-rpc-providertoJsonRpcEngineV2following its recent introduction. Intended to be closely followed by #6976 and subsequently released.The
InternalProvideris updated to useJsonRpcServerunder the hood. It can be constructed with aJsonRpcServeror legacy engine, but the latter will be wrapped by aJsonRpcServerin order to ensure consistent behavior across all internal providers. Meanwhile,providerFromEngine()is removed, it being a useless wrapper over theInternalProviderconstructor.Elsewhere in the monorepo, these changes revealed a discrepancy in behavior between the error handling of the legacy engine and
asV2Middleware(), which the latter is amended to resolve.In addition, the changes to the
InternalProviderrevealed that the legacy engine tolerates responses with{ result: undefined }. This is impossible to express using the V2 engine, which caused someNetworkControllertests reliant on the legacy behavior to fail. Further investigation proved that theseundefinedresults would error elsewhere in our JSON-RPC pipelines, so we simply remove these test cases. The upshot is that we no longer retryundefinedresults for "child requests" in theretryOnEmptymiddleware. It is unclear if this was occurring in practice, and it ought to be treated as a breach of contract by the RPC endpoint if it does.References
JsonRpcEngineV2#6327Checklist
JsonRpcEngineV2#6976, but it appears we could if we wanted to:Note
Migrates InternalProvider to JsonRpcEngineV2, adds providerFromMiddlewareV2, removes providerFromEngine, aligns error handling and retry semantics across packages, and updates consumers/tests and config.
InternalProvidernow wrapsJsonRpcEngineV2(accepts legacy engine or V2 server).providerFromMiddlewareV2; deprecateproviderFromMiddleware; removeproviderFromEngine.uuidtonanoidfor request ids.asV2Middlewareto usedeserializeErrorand ignoreundefinederrors from legacy middleware; add tests.JsonRpcServerandJsonRpcEngineV2.retryOnEmptyno longer treatsundefinedas an empty value; now errors on advancing block tag withundefinedresults.providerFromEnginewith directInternalProviderconstruction increate-network-client.@metamask/json-rpc-engine/v2in Jest/TS paths.InternalProviderdirectly.Written by Cursor Bugbot for commit 684a40c. This will update automatically on new commits. Configure here.