Conversation
rekmarks
force-pushed
the
rekm/json-rpc-engine-rewrite-next
branch
from
f74dff2 to
c393838
Compare
FrederikBolding
previously approved these changes
Oct 15, 2025
Using a generic Result forced each middleware function to have the same return type. This makes the generic useless. Supporting this, almost all of our uses of the equivalent generic on the legacy JsonRpcMiddlware type are Json, any, or unknown. In consequence, we remove the Result generic. In addition, unfreezes the result returned from asLegacyMiddleware, a problem that was surfaced by the type refactor. Adds a test to verify this behavior.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
FrederikBolding
previously approved these changes
Oct 20, 2025
|
|
||
| constructor({ middleware }: Options<Request>) { | ||
| // See .create() for why this is private. | ||
| private constructor({ middleware }: ConstructorOptions<Request, Context>) { |
Member
There was a problem hiding this comment.
If we want to entirely protect this from usage we could consider a constructor guard: https://github.com/MetaMask/key-tree/blob/main/src/SLIP10Node.ts#L517-L535
Member
Author
There was a problem hiding this comment.
Ah, right! I don't think we need that level of protection. It's not that the constructor is dangerous but rather that type inference won't work as well.
FrederikBolding
approved these changes
Oct 24, 2025
Gudahtt
added a commit
that referenced
this pull request
Oct 24, 2025
…r/multichain-transactions-controller * origin/main: (35 commits) feat: `JsonRpcEngineV2` (#6176) Release 641.0.0 (#6940) feat: Add transaction emulation actions (#6935) Release/640.0.0 (#6934) fix(core-backend): control randomness to fix flaky test (#6936) chore: Add `@metamask-previews/*` to NPM age gate exceptions (#6937) Release/639.0.0 (#6931) feat: make getCryptoApproveTransactionParams synchronous (#6930) feat: add new actions to `KeyringController` (#6928) feat: add `getAccounts` to `AccountsController` (#6927) chore: remove `Monad Mainnet` single call balance contract and add into account v4 (#6929) Release/638.0.0 (#6923) fix: Downgrade `multiformats` to `^9.9.0` to avoid ESM-only dependency (#6920) Release/637.0.0 (#6919) feat(account-tree-controller): add callbacks for hidden and pinned data (#6910) Release 636.0.0 (#6918) fix(core-backend): reconnection logic (#6861) fix: Tx state listener and signature coverage (#6906) Release/635.0.0 (#6917) fix(base-controller): add TypeScript declaration file for legacy module resolution (#6915) ...
This was referenced Nov 9, 2025
Closed
7 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
5 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
Introduces
JsonRpcEngineV2andJsonRpcServer, intended to replace all existing usage of the existingJsonRpcEngineimplementation. For the motivation behind this change, see #6088 and/or this ADR.Implementation
In order to resolve the problems listed in the motivation, V2 engine is split in two:
JsonRpcEngineV2JsonRpcServererrorHandlerconstructor parameter for capturing engine / middleware errorsSee the updated package
README.mdfor details.Migration
While this PR is substantial, migrating our existing JSON-RPC pipelines will be a significant project involving multiple teams over many releases cycles. To facilitate this, the PR introduces a forwards-compatibility adapter,
asV2Middleware(), and backwards-compatibility adapter,asLegacyMiddleware(), for the legacy and V2 engines, respectively. In addition, all V2 exports are exposed under the/v2export path, making this update completely non-breaking (although all legacy exports are deprecated).Note to reviewers
I recommend proceeding as follows:
JsonRpcEngineV2JsonRpcServerReferences
JsonRpcEnginefor safety, ergonomics, and debuggability #6088JsonRpcEngineChecklist
I've prepared draft pull requests for clients and consumer packages to resolve any breaking changesNote
Introduces a new v2 JSON-RPC engine and server, adds two-way adapters with the legacy engine, updates exports/docs, and adds supporting utilities, tests, and dependencies.
JsonRpcEngineV2for middleware orchestration andJsonRpcServerfor spec-compliant request/response handling.MiddlewareContext, utility types/helpers (isRequest,isNotification,JsonRpcEngineError,stringify), and composition APIs (asMiddleware,handle).asLegacyMiddleware(use V2 in legacy engine) andasV2Middleware(use legacy engine in V2).JsonRpcEngineand related types as deprecated; uses sharedstringifyutil; minor test/ESLint adjustments.exportsentry"./v2"and Browserify shimv2.js.indexand newv2/indexexports; addssrc/README.mdfor legacy andv2/README.md.README.mdwith V2 usage and migration notes;CHANGELOG.mdnotes Added (V2) and Deprecated (legacy).deep-freeze-strict,klona, and@types/deep-freeze-strict.Written by Cursor Bugbot for commit 04fdf3b. This will update automatically on new commits. Configure here.