BREAKING: Refactor RPC method params and add tests

packages/rpc-methods/jest.config.js

@@ -5,10 +5,10 @@ module.exports = deepmerge(baseConfig, {
   coveragePathIgnorePatterns: ['./src/index.ts'],
   coverageThreshold: {
     global: {
-      branches: 82.85,
-      functions: 80.85,
-      lines: 59.77,
-      statements: 59.77,
+      branches: 87.01,
+      functions: 87.03,
+      lines: 78.96,
+      statements: 78.96,
     },
   },
   testTimeout: 2500,

packages/rpc-methods/src/permitted/invokeSnapSugar.test.ts

@@ -0,0 +1,90 @@
+import {
+  JsonRpcEngineEndCallback,
+  JsonRpcEngineNextCallback,
+  JsonRpcRequest,
+} from '@metamask/types';
+import { ethErrors } from 'eth-rpc-errors';
+import { getValidatedParams, invokeSnapSugar } from './invokeSnapSugar';
+
+describe('wallet_invokeSnap', () => {
+  describe('invokeSnapSugar', () => {
+    it('invokes snap with next()', () => {
+      const req: JsonRpcRequest<unknown> = {
+        id: 'some-id',
+        jsonrpc: '2.0',
+        method: 'wallet_invokeSnap',
+        params: {
+          snapId: 'npm:@metamask/example-snap',
+          request: { method: 'hello' },
+        },
+      };
+      const _res: unknown = {};
+      const next: JsonRpcEngineNextCallback = jest
+        .fn()
+        .mockResolvedValueOnce(true);
+      const end: JsonRpcEngineEndCallback = jest.fn();
+
+      invokeSnapSugar(req, _res, next, end);
+
+      expect(next).toHaveBeenCalledTimes(1);
+    });
+
+    it('ends with an error if params are invalid', () => {
+      const req: JsonRpcRequest<unknown> = {
+        id: 'some-id',
+        jsonrpc: '2.0',
+        method: 'wallet_invokeSnap',
+        params: {
+          snapId: undefined,
+          request: [],
+        },
+      };
+      const _res: unknown = {};
+      const next: JsonRpcEngineNextCallback = jest
+        .fn()
+        .mockResolvedValueOnce(true);
+      const end: JsonRpcEngineEndCallback = jest.fn();
+
+      invokeSnapSugar(req, _res, next, end);
+
+      expect(end).toHaveBeenCalledWith(
+        ethErrors.rpc.invalidParams({
+          message: 'Must specify a valid snap ID.',
+        }),
+      );
+      expect(next).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('getValidatedParams', () => {
+    it('throws an error if the params is not an object', () => {
+      expect(() => getValidatedParams([])).toThrow(
+        'Expected params to be a single object.',
+      );
+    });
+
+    it('throws an error if the snap ID is missing from params object', () => {
+      expect(() =>
+        getValidatedParams({ snapId: undefined, request: {} }),
+      ).toThrow('Must specify a valid snap ID.');
+    });
+
+    it('throws an error if the request is not a plain object', () => {
+      expect(() =>
+        getValidatedParams({ snapId: 'snap-id', request: [] }),
+      ).toThrow('Expected request to be a single object.');
+    });
+
+    it('returns valid parameters', () => {
+      expect(
+        getValidatedParams({
+          snapId: 'npm:@metamask/example-snap',
+          request: { method: 'hello' },
+        }),
+      ).toStrictEqual({
+        snapId: 'npm:@metamask/example-snap',
+        request: { method: 'hello' },
+      });
+    });
+  });
+});

packages/rpc-methods/src/permitted/invokeSnapSugar.ts

@@ -8,6 +8,11 @@ import {
 import { isObject } from '@metamask/utils';
 import { SNAP_PREFIX } from '@metamask/snaps-utils';
 
+export type InvokeSnapSugarArgs = {
+  snapId: string;
+  request: JsonRpcRequest<unknown>;
+};
+
 /**
  * `wallet_invokeSnap` attempts to invoke an RPC method of the specified Snap.
  */
@@ -33,25 +38,51 @@ export const invokeSnapSugarHandler: PermittedHandlerExport<
  * @returns Nothing.
  * @throws If the params are invalid.
  */
-function invokeSnapSugar(
+export function invokeSnapSugar(
   req: JsonRpcRequest<unknown>,
   _res: unknown,
   next: JsonRpcEngineNextCallback,
   end: JsonRpcEngineEndCallback,
 ): void {
-  if (
-    !Array.isArray(req.params) ||
-    typeof req.params[0] !== 'string' ||
-    !isObject(req.params[1])
-  ) {
-    return end(
-      ethErrors.rpc.invalidParams({
-        message: 'Must specify a string snap ID and a plain request object.',
-      }),
-    );
+  let params: InvokeSnapSugarArgs;
+  try {
+    params = getValidatedParams(req.params);
+  } catch (error) {
+    return end(error);
   }
 
-  req.method = SNAP_PREFIX + req.params[0];
-  req.params = [req.params[1]];
+  req.method = SNAP_PREFIX + params.snapId;
+  req.params = params.request;
   return next();
 }
+
+/**
+ * Validates the wallet_invokeSnap method `params` and returns them cast to the correct
+ * type. Throws if validation fails.
+ *
+ * @param params - The unvalidated params object from the method request.
+ * @returns The validated method parameter object.
+ */
+export function getValidatedParams(params: unknown): InvokeSnapSugarArgs {
+  if (!isObject(params)) {
+    throw ethErrors.rpc.invalidParams({
+      message: 'Expected params to be a single object.',
+    });
+  }
+
+  const { snapId, request } = params;
+
+  if (!snapId || typeof snapId !== 'string' || snapId === '') {
+    throw ethErrors.rpc.invalidParams({
+      message: 'Must specify a valid snap ID.',
+    });
+  }
+
+  if (!isObject(request)) {
+    throw ethErrors.rpc.invalidParams({
+      message: 'Expected request to be a single object.',
+    });
+  }
+
+  return params as InvokeSnapSugarArgs;
+}

packages/rpc-methods/src/restricted/invokeSnap.test.ts

@@ -48,7 +48,7 @@ describe('implementation', () => {
     await implementation({
       context: { origin: MOCK_ORIGIN },
       method: `wallet_snap_${MOCK_SNAP_ID}`,
-      params: [{ method: 'foo', params: {} }],
+      params: { method: 'foo', params: {} },
     });
 
     expect(hooks.getSnap).toHaveBeenCalledTimes(1);
@@ -73,7 +73,7 @@ describe('implementation', () => {
       implementation({
         context: { origin: MOCK_ORIGIN },
         method: `wallet_snap_${MOCK_SNAP_ID}`,
-        params: [{ method: 'foo', params: {} }],
+        params: { method: 'foo', params: {} },
       }),
     ).rejects.toThrow(
       `The snap "${MOCK_SNAP_ID}" is not installed. This is a bug, please report it.`,
@@ -92,7 +92,7 @@ describe('implementation', () => {
       implementation({
         context: { origin: MOCK_ORIGIN },
         method: `wallet_snap_${MOCK_SNAP_ID}`,
-        params: [{}],
+        params: {},
       }),
     ).rejects.toThrow(
       'Must specify a valid JSON-RPC request object as single parameter.',

packages/rpc-methods/src/restricted/invokeSnap.ts

@@ -92,12 +92,11 @@ export function getInvokeSnapImplementation({
   handleSnapRpcRequest,
 }: InvokeSnapMethodHooks) {
   return async function invokeSnap(
-    options: RestrictedMethodOptions<[Record<string, Json>]>,
+    options: RestrictedMethodOptions<Record<string, Json>>,
   ): Promise<Json> {
-    const { params = [], method, context } = options;
-    const rawRequest = params[0];
+    const { params = {}, method, context } = options;
 
-    const request = { jsonrpc: '2.0', id: nanoid(), ...rawRequest };
+    const request = { jsonrpc: '2.0', id: nanoid(), ...params };
 
     if (!isJsonRpcRequest(request)) {
       throw ethErrors.rpc.invalidParams({