Remove JSON Params hack

packages/cli/package.json

@@ -45,7 +45,7 @@
     "@babel/preset-typescript": "^7.16.7",
     "@metamask/snap-utils": "^0.23.0",
     "@metamask/snaps-browserify-plugin": "^0.23.0",
-    "@metamask/utils": "^3.3.0",
+    "@metamask/utils": "^3.3.1",
     "babelify": "^10.0.0",
     "browserify": "^17.0.0",
     "chokidar": "^3.5.2",

packages/controllers/package.json

@@ -39,7 +39,7 @@
     "@metamask/rpc-methods": "^0.23.0",
     "@metamask/snap-types": "^0.23.0",
     "@metamask/snap-utils": "^0.23.0",
-    "@metamask/utils": "^3.3.0",
+    "@metamask/utils": "^3.3.1",
     "@xstate/fsm": "^2.0.0",
     "concat-stream": "^2.0.0",
     "cron-parser": "^4.5.0",

packages/controllers/src/services/AbstractExecutionService.ts

@@ -1,15 +1,18 @@
 import { Duplex } from 'stream';
+
 import ObjectMultiplex from '@metamask/object-multiplex';
 import {
   SnapRpcHook,
   SnapRpcHookArgs,
   SNAP_STREAM_NAMES,
 } from '@metamask/snap-utils';
 
+import { BasePostMessageStream } from '@metamask/post-message-stream';
 import {
   Duration,
   isJsonRpcNotification,
   isObject,
+  Json,
   JsonRpcNotification,
 } from '@metamask/utils';
 import {
@@ -18,10 +21,9 @@ import {
   JsonRpcRequest,
   PendingJsonRpcResponse,
 } from 'json-rpc-engine';
+import { createStreamMiddleware } from 'json-rpc-middleware-stream';
 import { nanoid } from 'nanoid';
 import pump from 'pump';
-import { createStreamMiddleware } from 'json-rpc-middleware-stream';
-import { BasePostMessageStream } from '@metamask/post-message-stream';
 import { hasTimedOut, withTimeout } from '../utils';
 import {
   ExecutionService,
@@ -226,7 +228,7 @@ export abstract class AbstractExecutionService<WorkerType>
     const notificationHandler = (
       message:
         | JsonRpcRequest<unknown>
-        | JsonRpcNotification<unknown[] | Record<string, unknown>>,
+        | JsonRpcNotification<Json[] | Record<string, Json>>,
     ) => {
       if (!isJsonRpcNotification(message)) {
         return;

packages/examples/examples/insights/package.json

@@ -27,7 +27,7 @@
   },
   "dependencies": {
     "@metamask/abi-utils": "^1.0.0",
-    "@metamask/utils": "^3.3.0"
+    "@metamask/utils": "^3.3.1"
   },
   "devDependencies": {
     "@lavamoat/allow-scripts": "^2.0.3",

packages/examples/examples/insights/snap.manifest.json

@@ -7,7 +7,7 @@
     "url": "https://github.com/MetaMask/snaps-monorepo.git"
   },
   "source": {
-    "shasum": "rWNQVlpSAlUXVByhpVyXxvNRTwEfabK4vS2MU6ANNwc=",
+    "shasum": "NQC19hFQU5NAlR2idYt0bETu0XU2K723AmguhnUtJok=",
     "location": {
       "npm": {
         "filePath": "dist/bundle.js",

packages/execution-environments/package.json

@@ -35,7 +35,7 @@
     "@metamask/providers": "^10.2.0",
     "@metamask/snap-types": "^0.23.0",
     "@metamask/snap-utils": "^0.23.0",
-    "@metamask/utils": "^3.3.0",
+    "@metamask/utils": "^3.3.1",
     "eth-rpc-errors": "^4.0.3",
     "pump": "^3.0.0",
     "ses": "^0.17.0",

packages/execution-environments/src/common/BaseSnapExecutor.ts

@@ -1,37 +1,38 @@
 // eslint-disable-next-line @typescript-eslint/triple-slash-reference, spaced-comment
 /// <reference path="../../../../node_modules/ses/index.d.ts" />
 import { Duplex } from 'stream';
+
 import { MetaMaskInpageProvider } from '@metamask/providers';
-import { SnapProvider, SnapExports } from '@metamask/snap-types';
-import { errorCodes, ethErrors, serializeError } from 'eth-rpc-errors';
+import { SnapExports, SnapProvider } from '@metamask/snap-types';
+import {
+  HandlerType,
+  SnapExportsParameters,
+  SNAP_EXPORT_NAMES,
+} from '@metamask/snap-utils';
 import {
-  isObject,
-  isValidJson,
-  JsonRpcNotification,
   assert,
+  hasProperty,
   isJsonRpcRequest,
+  isObject,
+  isValidJson,
+  Json,
   JsonRpcId,
-  JsonRpcRequest,
+  JsonRpcNotification,
   JsonRpcParams,
-  Json,
-  hasProperty,
+  JsonRpcRequest,
 } from '@metamask/utils';
-import {
-  HandlerType,
-  SNAP_EXPORT_NAMES,
-  SnapExportsParameters,
-} from '@metamask/snap-utils';
+import { errorCodes, ethErrors, serializeError } from 'eth-rpc-errors';
 import { validate } from 'superstruct';
 import EEOpenRPCDocument from '../openrpc.json';
-import { createEndowments } from './endowments';
 import {
-  getCommandMethodImplementations,
   CommandMethodsMapping,
+  getCommandMethodImplementations,
 } from './commands';
-import { removeEventListener, addEventListener } from './globalEvents';
+import { createEndowments } from './endowments';
+import { addEventListener, removeEventListener } from './globalEvents';
+import { wrapKeyring } from './keyring';
 import { sortParamKeys } from './sortParams';
 import { constructError, withTeardown } from './utils';
-import { wrapKeyring } from './keyring';
 import {
   ExecuteSnapRequestArgumentsStruct,
   PingRequestArgumentsStruct,
@@ -112,7 +113,7 @@ export class BaseSnapExecutor {
 
     this.methods = getCommandMethodImplementations(
       this.startSnap.bind(this),
-      (target, handlerName, args) => {
+      async (target, handlerName, args) => {
         const data = this.snapData.get(target);
         // We're capturing the handler in case someone modifies the data object before the call
         const handler =
@@ -123,28 +124,39 @@ export class BaseSnapExecutor {
           handler !== undefined,
           `No ${handlerName} handler exported for snap "${target}`,
         );
-        // TODO: fix type
-        return this.executeInSnapContext(target, () => handler(args as any));
+        // TODO: fix handler args type cast
+        const result = await this.executeInSnapContext(target, () =>
+          handler(args as any),
+        );
+        assert(
+          isValidJson(result),
+          new TypeError('Received non JSON serializable value'),
+        );
+        return result;
       },
       this.onTerminate.bind(this),
     );
   }
 
-  private errorHandler(error: unknown, data: Record<string, unknown>) {
+  private errorHandler(error: unknown, data: Record<string, Json>) {
     const constructedError = constructError(error);
     const serializedError = serializeError(constructedError, {
       fallbackError,
       shouldIncludeStack: false,
     });
+
+    // We're setting it this way to avoid sentData.stack = undefined
+    const sentData: Json = { ...data };
+    if (constructedError?.stack) {
+      sentData.stack = constructedError.stack;
+    }
+
     this.notify({
       method: 'UnhandledError',
       params: {
         error: {
           ...serializedError,
-          data: {
-            ...data,
-            stack: constructedError?.stack,
-          },
+          data: sentData,
         },
       },
     });
@@ -214,7 +226,7 @@ export class BaseSnapExecutor {
 
   protected notify(
     requestObject: Omit<
-      JsonRpcNotification<Record<string, unknown> | unknown[] | undefined>,
+      JsonRpcNotification<Record<string, Json> | Json[] | undefined>,
       'jsonrpc'
     >,
   ) {

packages/execution-environments/src/common/keyring.ts

@@ -1,5 +1,11 @@
 import { SnapKeyring } from '@metamask/snap-types';
-import { JsonRpcNotification, JsonRpcRequest } from '@metamask/utils';
+import {
+  assert,
+  isValidJson,
+  Json,
+  JsonRpcNotification,
+  JsonRpcRequest,
+} from '@metamask/utils';
 
 /**
  * Wraps a SnapKeyring class, returning a handler that can route requests to the exposed functions by the class.
@@ -11,7 +17,7 @@ import { JsonRpcNotification, JsonRpcRequest } from '@metamask/utils';
 export function wrapKeyring(
   notify: (
     requestObject: Omit<
-      JsonRpcNotification<Record<string, unknown> | unknown[]>,
+      JsonRpcNotification<Record<string, Json> | Json[]>,
       'jsonrpc'
     >,
   ) => void,
@@ -21,29 +27,33 @@ export function wrapKeyring(
     throw new Error('Keyring not exported');
   }
 
-  const keyringHandler = ({
-    request,
-  }: {
-    request: JsonRpcRequest<unknown[]>;
-  }) => {
+  const keyringHandler = ({ request }: { request: JsonRpcRequest<Json[]> }) => {
     const { method, params } = request;
     if (!(method in keyring)) {
       throw new Error(`Keyring does not expose ${method}`);
     }
-    let args = params ?? [];
-    // @ts-expect-error TODO: Figure out how to type this better
-    const func = keyring[method].bind(keyring);
+    let args = (params ?? []) as unknown[];
+    const keyringMethod = keyring[method as keyof SnapKeyring];
+    assert(keyringMethod !== undefined);
+    const func = keyringMethod.bind(keyring);
     // Special case for registering events
     if (method === 'on') {
-      const data = args[0];
-      const listener = (listenerArgs: unknown) =>
-        notify({
+      const data = args[0] as Json;
+      const listener = (...listenerArgs: unknown[]) => {
+        assert(
+          isValidJson(listenerArgs),
+          new TypeError(
+            'Keyrings .on listener received non Json serializable value',
+          ),
+        );
+        return notify({
           method: 'SnapKeyringEvent',
           params: { data, args: listenerArgs },
         });
+      };
       args = [data, listener];
     }
-    return func(...args);
+    return (func as (..._: unknown[]) => unknown)(...args);
   };
   return keyringHandler;
 }

packages/execution-environments/src/common/validation.ts

@@ -1,5 +1,14 @@
-import { ChainIdStruct, HandlerType } from '@metamask/snap-utils';
 import { SnapKeyring } from '@metamask/snap-types';
+import { ChainIdStruct, HandlerType } from '@metamask/snap-utils';
+import {
+  assertStruct,
+  Json,
+  JsonRpcIdStruct,
+  JsonRpcRequestStruct,
+  JsonRpcSuccess,
+  JsonRpcSuccessStruct,
+  JsonStruct,
+} from '@metamask/utils';
 import {
   array,
   assign,
@@ -14,17 +23,7 @@ import {
   string,
   tuple,
   union,
-  unknown,
 } from 'superstruct';
-import {
-  assertStruct,
-  Json,
-  JsonRpcIdStruct,
-  JsonRpcRequestStruct,
-  JsonRpcSuccess,
-  JsonRpcSuccessStruct,
-  JsonStruct,
-} from '@metamask/utils';
 
 const VALIDATION_FUNCTIONS = {
   [HandlerType.OnRpcRequest]: validateFunctionExport,
@@ -126,7 +125,7 @@ export const SnapRpcRequestArgumentsStruct = tuple([
   assign(
     JsonRpcRequestWithoutIdStruct,
     object({
-      params: optional(record(string(), unknown())),
+      params: optional(record(string(), JsonStruct)),
     }),
   ),
 ]);

packages/plugin-webpack/package.json

@@ -30,7 +30,7 @@
   },
   "dependencies": {
     "@metamask/snap-utils": "^0.23.0",
-    "@metamask/utils": "^3.3.0",
+    "@metamask/utils": "^3.3.1",
     "webpack-sources": "^3.2.3"
   },
   "devDependencies": {

packages/provider/package.json

@@ -27,7 +27,7 @@
   "dependencies": {
     "@metamask/safe-event-emitter": "^2.0.0",
     "@metamask/snap-types": "^0.23.0",
-    "@metamask/utils": "^3.3.0",
+    "@metamask/utils": "^3.3.1",
     "nanoid": "^3.1.31"
   },
   "devDependencies": {

packages/provider/src/MultiChainProvider.ts

@@ -1,5 +1,5 @@
 import SafeEventEmitter from '@metamask/safe-event-emitter';
-import { nanoid } from 'nanoid';
+import type { SnapProvider } from '@metamask/snap-types';
 import {
   assertIsConnectArguments,
   assertIsMetaMaskNotification,
@@ -12,8 +12,8 @@ import {
   RequestNamespace,
   Session,
 } from '@metamask/snap-utils';
-import { JsonRpcRequest } from '@metamask/utils';
-import type { SnapProvider } from '@metamask/snap-types';
+import { Json, JsonRpcRequest } from '@metamask/utils';
+import { nanoid } from 'nanoid';
 import { Provider } from './Provider';
 
 declare global {
@@ -126,11 +126,17 @@ export class MultiChainProvider extends SafeEventEmitter implements Provider {
 
     assertIsMultiChainRequest(args);
 
+    // We're doing it this way to avoid sentRequest.params = undefined.
+    const sentRequest: Json = { method: args.request.method };
+    if (args.request.params !== undefined) {
+      sentRequest.params = args.request.params;
+    }
+
     return this.#rpcRequest({
       method: 'caip_request',
       params: {
         chainId: args.chainId,
-        request: { method: args.request.method, params: args.request.params },
+        request: sentRequest,
       },
     });
   }
@@ -152,7 +158,7 @@ export class MultiChainProvider extends SafeEventEmitter implements Provider {
    */
   async #rpcRequest(
     payload: { method: string } & Partial<
-      JsonRpcRequest<unknown[] | Record<string, unknown>>
+      JsonRpcRequest<Record<string, Json> | Json[] | undefined>
     >,
   ) {
     return await this.#getProvider().request({

packages/rpc-methods/package.json

@@ -30,7 +30,7 @@
     "@metamask/key-tree": "^6.0.0",
     "@metamask/snap-utils": "^0.23.0",
     "@metamask/types": "^1.1.0",
-    "@metamask/utils": "^3.3.0",
+    "@metamask/utils": "^3.3.1",
     "eth-rpc-errors": "^4.0.2",
     "nanoid": "^3.1.31",
     "superstruct": "^0.16.7"

packages/types/src/types.d.ts

@@ -48,7 +48,7 @@ export interface SnapKeyring {
       request: RequestArguments;
     },
   ): Promise<Json>;
-  on(data: KeyringEvent, listener: (...args: unknown[]) => void): void;
+  on(data: KeyringEvent, listener: (...args: Json[]) => void): void;
   off(data: KeyringEvent): void;
 
   addAccount?(chainId: ChainId): Promise<AccountId>;