-
-
Notifications
You must be signed in to change notification settings - Fork 252
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
- Migrate
'notification'event notifications to some new patternJsonRpcEngineis also aSafeEventEmitter. To wit, it emits a single event,'notification', which we use almost 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-engineto emit notifications, the proper place to do it would be onJsonRpcServer. 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/giuin 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.
- See
- Migrate
@metamask/eth-json-rpc-middleware- This is a major "leaf node" in our JSON-RPC pipelines, and could be migrated at any time.
- Be advised that it uses
createScaffoldMiddleware()from the legacy engine, which has no V2 equivalent. If this utility seems generally useful, it should probably be reimplemented for V2 with improved type safety. On the other hand, a locally implemented equivalent in@metamask/eth-json-rpc-middlewarewould also suffice.
- Migrate
@metamask/eth-json-rpc-provider- The provider uses the legacy engine under the hood. It must be migrated as well.