Skip to content

[json-rpc-engine] Rewrite JsonRpcEngine for safety, ergonomics, and debuggability #6088

@rekmarks

Description

@rekmarks

The venerable JsonRpcEngine is the backbone MetaMask's JSON-RPC stack. Being venerable, it reliably works, although it has a number of issues that we can't comprehensively fix without a full rewrite. Chief among these problems are:

  1. Middleware errors are automatically serialized
    • The engine serializes errors before they are exposed to callers of engine.handle().
    • This is because the engine is responsible both for middleware processing and the server-like responsibilities of our JSON-RPC stack, in this case ensuring that response objects are JSON-serializable.
      • This mixing of the responsibilities of middleware processing and JSON-RPC server functionality also complicates the engine implementation.
  2. Request and response objects are ambiently mutable
    • All middleware can freely mutate request and response objects at any point in time via their request or return handlers.
    • This is a Principle of Least Authority (POLA) violation, and can make debugging middleware pipelines difficult.
  3. It uses mixed callback and Promise / async signatures
    • The engine was originally implemented using the conventions of the Before Times, when async function calls looked like this:
      • asyncFn(arg, function (err, result) { ... })
    • After Promises and async / await were introduced, we slapped those capabilities on top of the original callback implementation.
    • This is confusing, and vastly complicates the implementation of the engine.
  4. It forces consumers to handle JSON-RPC notifications separately
    • JSON-RPC notifications, i.e. requests without the id property, should never be responded to.
    • Support for notifications was not included in the original implementation, and was tacked-on some years ago.
    • They complicate the engine's implementation, and are processed separately from requests, in an un-ergonomic fashion.

To address these problems, we will fully rewrite JsonRpcEngine into a new set of composable modules. The rewrite is motivated by the complexities surfaced in previous work referenced in #2024.

The goals of the rewrite include:

  • Provide an incremental migration path from the existing JsonRpcEngine to the new modules
    • It being the backbone of our JSON-RPC stack, migrating even one of our primary JSON-RPC pipelines at once is probably untenable
  • Separate "middleware processing" and "JSON-RPC server" functionality into two distinct modules
    • This will allow the middleware processor to throw native errors, and the server to serialize them
  • Handle JSON-RPC notifications "natively" (not separately from requests)
  • Use Promises and async / await, without any callbacks
  • Reduce ambient authority in middleware processing
    • Make request objects immutable
    • Conceal response objects; middleware can return a result or throw an error
    • Expose a mutable "context" object for middleware-to-middleware communication
      • We're not gonna bend over backwards to make this type safe
  • Generally, simplify the implementation and make the interface(s) more ergonomic

Metadata

Metadata

Assignees

Labels

enhancementNew feature or requestteam-wallet-frameworkDeprecated: Please use `team-core-platform` instead.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions