Improve JSON-RPC validation

[Mrtenz]

TypeScript does not validate types at runtime, so to make sure the data passed from an external source, we need to validate it. The current JSON-RPC validation just checks if the value contains a single field, like id or result. If a client sends a message like this...

{
  "id": {},
  "invalid": "param"
}

...the message would be considered valid by isJsonRpcRequest. This can easily cause errors if the implementing code assumes that the JSON-RPC request contains all the standard JSON-RPC properties after that.

I propose we improve the JSON-RPC validation with some schema validation, like Superstruct. This lets us write the validation schema and TypeScript type without code duplication. For example:

const JsonRpcRequestStruct = object({
  id: union([string(), number()]),
  method: string(),
  params: optional(JsonRpcParamsStruct)
});

type JsonRpcRequest = Infer<typeof JsonRpcRequestStruct>;
// Inferred as:
// {
//   id: string | number;
//   method: string;
//   params?: JsonRpcParams;
// }

const isValidJsonRpcRequest = (request: unknown): request is JsonRpcRequest =>
  is(request, JsonRpcRequestStruct);

This guarantees that the JSON-RPC request has the right shape and types, and has full support for TypeScript types as well.

[shanejonas]

JSON Schema seems like a good fit here. snaps skunkworks utils uses it as well and gets nice typescript types out of JSON Schema https://github.com/MetaMask/snaps-skunkworks/blob/main/packages/utils/scripts/transpileSchemaTypes.js

[Mrtenz]

@shanejonas JSON schemas have, in my opinion, some problems compared to Superstruct:

1. It adds a lot of boilerplate to use them, like the transpileSchemaTypes script and auto-generated files in the json-schemas folder. This also means extra build steps are needed to generate those files and keep them up-to-date.

2. JSON schemas are hard to read and maintain. Superstruct uses a syntax which I think is quite familiar to TypeScript developers, making it much easier to read and maintain. For comparison, this is the schema for the JsonRpcRequest above (using a basic array type for the params):

   {
    "type": "object",
    "properties": {
      "id": {
        "type": "integer"
      },
      "method": {
        "type": "string"
      },
      "params": {
        "type": "array",
        "items": {}
      }
    },
    "required": [
      "id",
      "method"
    ]
   }

   It's four times bigger than the Superstruct schema.

3. The generated types are difficult to read. The output has types like ObjectOfStringDj4X5WuPStringDj4X5WuPHQwLk7Md, and errors like "... is not assignable to ObjectOfString..." are quite useless.

4. The generated validation files are relatively large. The validateSnapManifest file is 23 KB or 3.7 KB gzipped. Superstruct in its entirety is only 3.5 KB.

I was going to propose replacing the JSON schemas in snaps-skunkworks too at some point, for the reasons mentioned above.

[mcmire]

I read through Superstruct's README and I dig it. It looks like it's very well thought out. I appreciate how detailed the error messages are (and how they can be customized), and the TypeScript integration is really neat.

One question I have involves the example you gave, @Mrtenz:

const JsonRpcRequestStruct = object({
  id: union([string(), number()]),
  method: string(),
  params: optional(JsonRpcParamsStruct)
});

type JsonRpcRequest = Infer<typeof JsonRpcRequestStruct>;

You said this is inferred as:

{
  id: string | number;
  method: string;
  params: JsonRpcParams;
}

However, if params is optional, shouldn't this be the following?

{
  id: string | number;
  method: string;
  params?: JsonRpcParams;
}

[Mrtenz]

@mcmire You're right, that's a typo. Fixed it now, thanks for the heads up!

[Mrtenz]

@mcmire @shanejonas I did a PR implementing Superstruct in this library: #19. Would appreciate feedback on that as well!