Migrate JSON-RPC stack to `JsonRpcEngineV2`
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 413
- Forks
- 308
- Avg merge
- 1d 4h
- Merged PRs (30d)
- 253
Description
Following #6088, all existing uses of `JsonRpcEngine` and its middleware pattern must be migrated to `JsonRpcEngineV2`. This is the tracking issue for this process, which will have to be incremental. This issue does not yet provide a full account of every affected package, but at least tries to record some of the non-obvious requirements of the migration.
#6176, the implementation for #6088, provides backwards- and forwards-compatibility adapters in the form of `asLegacyMiddleware()` and `asV2Middleware()`, respectively. A small but possibly negligible performance hit should be expected whenever these adapters are used. The upshot is that a complex middleware pipeline, e.g. the MetaMask clients' "provider engines", can be migrated incrementally in arbitrary order. That being said, it's probably best to identify the distinct components of these middleware pipelines and migrate them one at a time. It may also make the most sense to start at the end of the engine / middleware stack and work back up to the "root" / initial engine. This would imply e.g. migration `@metamask/eth-json-rpc-middleware` first, then the method middleware, and so and and so forth until the initial engine is finally migrated. However, due to `asV2Middleware()`, the initial engine could be migrated at any time, which would begin to address one of the problems motivating the migration in the first place, namely mangled errors from the legacy engine module.
### TODO
This list is unordered and potentially inexhaustive.
- [ ] Migrate `'notification'` event notifications to some new pattern
- `JsonRpcEngine` is also a `SafeEventEmitter`. To wit, it emits a single event, `'notification'`, which we use exclusively(?) to forward JSON-RPC notifications to streams. The engine does nothing with these notifications except include them as the payload of the event.
- In other words, this is just a convenient if idiosyncratic way of sending notifications to different remotes, including dapps. Rather than (ab)using the engine in this manner, we could just write these notifications directly to their intended dispatch mechanisms (again, usually a stream).
- If we want to leverage `json-rpc-engine` to emit notifications, the proper place to do it would be on `JsonRpcServer`. However, it would need to receive a function or stream to dispatch the notification, and that is probably the wrong solution for our stack.
- [ ] Migrate method middlewares
- See `/methodmiddleware/giu` in extension and mobile.
- The method middleware pattern expects a legacy middleware function as the method implementation, so either all method implementations per method middleware are migrated in one go, or they are migrated piecemeal.
- [ ] Migrate `@metamask/providers`
- Our providers use `JsonRpcEngine` internally.
- [ ] Migrate `@metamask/json-rpc-middleware-stream`
- Will close: #2025
- [ ] Migrate `@metamask/multichain-api-middleware`
- [ ] Migrate `@metamask/eip1193-permission-middleware`
- [ ] Migrate `@metamask/eth-block-tracker`
- [ ] Migrate `@metamask/eth-json-rpc-infura`
- [ ] Migrate `@metamask/permission-controller`
- [ ] Migrate `@metamask/permission-log-controller`
- [ ] Migrate `@metamask/selected-network-controller`
- [x] Migrate `@metamask/eth-json-rpc-middleware` (#7065)
- [x] Migrate `@metamask/eth-json-rpc-provider` (#7001)
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading the compatibility adapters from #6176 and the unchecked migration items in this issue. Identify one affected package, such as @metamask/providers or @metamask/permission-controller, and trace its JsonRpcEngine usage and tests. Done means that package no longer depends on the legacy middleware pattern and its tests still pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- backend-api-design
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 20/100