Allow omitting jsonrpc and id in handleRequest (#1556)

* Slightly refactor handleRequest * Fix tests and coverage * Update shasums
MetaMask · Jun 30, 2023 · 6cbd7c4 · 6cbd7c4
1 parent 2fce138
commit 6cbd7c4
Show file tree

Hide file tree

Showing 13 changed files with 28 additions and 63 deletions.
diff --git a/packages/examples/packages/bip32/snap.manifest.json b/packages/examples/packages/bip32/snap.manifest.json
@@ -7,7 +7,7 @@
     "url": "https://github.com/MetaMask/snaps.git"
   },
   "source": {
-    "shasum": "cOJYxa17Mn160j/YH8887WOpGFVl0N0c2TK4WBfpdh0=",
+    "shasum": "eZsL+yuk4ZRUs5mKlFUgQ/cOSxBR3LgjnuYV4pnu/mY=",
     "location": {
       "npm": {
         "filePath": "dist/bundle.js",

diff --git a/packages/examples/packages/bip44/snap.manifest.json b/packages/examples/packages/bip44/snap.manifest.json
@@ -7,7 +7,7 @@
     "url": "https://github.com/MetaMask/snaps.git"
   },
   "source": {
-    "shasum": "/tbydO725MRM49ZgIJ/B9mKSysfVRxDL8JulEYLcvz4=",
+    "shasum": "uX4m5soXlVN7UYHzLL/Ushqc8HdD3HDi0feLkl7vOOo=",
     "location": {
       "npm": {
         "filePath": "dist/bundle.js",

diff --git a/packages/examples/packages/dialogs/snap.manifest.json b/packages/examples/packages/dialogs/snap.manifest.json
@@ -7,7 +7,7 @@
     "url": "https://github.com/MetaMask/snaps.git"
   },
   "source": {
-    "shasum": "VGoLdBnY9aOihxKIC243QpxbSd7N54D2JtSUwCwj/Go=",
+    "shasum": "x7nzQYgkmWOAQ46WfBl/RE7R6QbLdg+3Cem52I/AgrI=",
     "location": {
       "npm": {
         "filePath": "dist/bundle.js",

diff --git a/packages/examples/packages/get-entropy/snap.manifest.json b/packages/examples/packages/get-entropy/snap.manifest.json
@@ -7,7 +7,7 @@
     "url": "https://github.com/MetaMask/snaps.git"
   },
   "source": {
-    "shasum": "8LTCy3FTdNiSavFDKD/vtBC2dIDHF1KnO+oWGp/frdc=",
+    "shasum": "HgZvE/n41ozPudgvCEmFdGc30cW50H/PZkrzCp4gFNU=",
     "location": {
       "npm": {
         "filePath": "dist/bundle.js",

diff --git a/packages/examples/packages/invoke-snap/packages/core-signer/snap.manifest.json b/packages/examples/packages/invoke-snap/packages/core-signer/snap.manifest.json
@@ -7,7 +7,7 @@
     "url": "https://github.com/MetaMask/snaps.git"
   },
   "source": {
-    "shasum": "FiljNwk76RZIi5odcK4IzZ6CJh9qA6OZo4iZTSFh40A=",
+    "shasum": "Nr7S8uNAt9uLcvQCeyRiwfa0YLjaeJkDIuccHUiVv4k=",
     "location": {
       "npm": {
         "filePath": "dist/bundle.js",

diff --git a/packages/examples/packages/manage-state/snap.manifest.json b/packages/examples/packages/manage-state/snap.manifest.json
@@ -7,7 +7,7 @@
     "url": "https://github.com/MetaMask/snaps.git"
   },
   "source": {
-    "shasum": "G8MTnTgczLaqj0/5pSeyW9lqakzHfBd5MW8ltGwLm/4=",
+    "shasum": "fBYptDM+etAMdZ0RLcTrDxp5utOdqVnNA9ct3sT369c=",
     "location": {
       "npm": {
         "filePath": "dist/bundle.js",

diff --git a/packages/examples/packages/notifications/snap.manifest.json b/packages/examples/packages/notifications/snap.manifest.json
@@ -7,7 +7,7 @@
     "url": "https://github.com/MetaMask/snaps.git"
   },
   "source": {
-    "shasum": "8ip4xc7sMbpic8lNDVYRUEhPK0QHR2SLPyDXCuf62m4=",
+    "shasum": "/lLHUZ/SWHARUnhQ3KaAcyg7dEY2JLpr0SX+rWJBbHU=",
     "location": {
       "npm": {
         "filePath": "dist/bundle.js",

diff --git a/packages/rpc-methods/jest.config.js b/packages/rpc-methods/jest.config.js
@@ -10,10 +10,10 @@ module.exports = deepmerge(baseConfig, {
   ],
   coverageThreshold: {
     global: {
-      branches: 86.08,
+      branches: 85.96,
       functions: 100,
-      lines: 97.75,
-      statements: 96.39,
+      lines: 97.95,
+      statements: 96.57,
     },
   },
 });
diff --git a/packages/rpc-methods/src/restricted/invokeSnap.test.ts b/packages/rpc-methods/src/restricted/invokeSnap.test.ts
@@ -110,8 +110,6 @@ describe('implementation', () => {
       handler: 'onRpcRequest',
       origin: MOCK_ORIGIN,
       request: {
-        jsonrpc: '2.0',
-        id: expect.any(String),
         method: 'hello',
         params: {},
       },
@@ -139,24 +137,6 @@ describe('implementation', () => {
 
     expect(hooks.handleSnapRpcRequest).not.toHaveBeenCalled();
   });
-
-  it('throws if request is not valid', async () => {
-    const hooks = getMockHooks();
-    hooks.getSnap.mockImplementation(getTruncatedSnap);
-    const implementation = getInvokeSnapImplementation(hooks);
-    await expect(
-      implementation({
-        context: { origin: MOCK_ORIGIN },
-        method: WALLET_SNAP_PERMISSION_KEY,
-        params: {},
-      }),
-    ).rejects.toThrow(
-      'Must specify a valid JSON-RPC request object as single parameter.',
-    );
-
-    expect(hooks.getSnap).toHaveBeenCalledTimes(0);
-    expect(hooks.handleSnapRpcRequest).not.toHaveBeenCalled();
-  });
 });
 
 describe('handleSnapInstall', () => {

diff --git a/packages/rpc-methods/src/restricted/invokeSnap.ts b/packages/rpc-methods/src/restricted/invokeSnap.ts
@@ -15,9 +15,8 @@ import {
   RequestedSnapPermissions,
   InstallSnapsResult,
 } from '@metamask/snaps-utils';
-import { isJsonRpcRequest, Json, NonEmptyArray } from '@metamask/utils';
+import { Json, NonEmptyArray } from '@metamask/utils';
 import { ethErrors } from 'eth-rpc-errors';
-import { nanoid } from 'nanoid';
 
 import { MethodHooksObject } from '../utils';
 
@@ -171,19 +170,6 @@ export function getInvokeSnapImplementation({
 
     const { snapId, request } = params as InvokeSnapParams;
 
-    const rpcRequest = {
-      jsonrpc: '2.0',
-      id: nanoid(),
-      ...request,
-    };
-
-    if (!isJsonRpcRequest(rpcRequest)) {
-      throw ethErrors.rpc.invalidParams({
-        message:
-          'Must specify a valid JSON-RPC request object as single parameter.',
-      });
-    }
-
     if (!getSnap(snapId)) {
       throw ethErrors.rpc.invalidRequest({
         message: `The snap "${snapId}" is not installed. Please install it first, before invoking the snap.`,
@@ -195,7 +181,7 @@ export function getInvokeSnapImplementation({
     return (await handleSnapRpcRequest({
       snapId,
       origin,
-      request: rpcRequest,
+      request,
       handler: HandlerType.OnRpcRequest,
     })) as Json;
   };

diff --git a/packages/snaps-controllers/coverage.json b/packages/snaps-controllers/coverage.json
@@ -1,6 +1,6 @@
 {
-  "branches": 88.55,
+  "branches": 88.73,
   "functions": 95.97,
-  "lines": 96.74,
-  "statements": 96.39
+  "lines": 96.8,
+  "statements": 96.45
 }
diff --git a/packages/snaps-controllers/src/snaps/SnapController.test.ts b/packages/snaps-controllers/src/snaps/SnapController.test.ts
@@ -1659,8 +1659,8 @@ describe('SnapController', () => {
         }),
       ).rejects.toThrow(
         ethErrors.rpc.invalidRequest({
-          message: 'Invalid "jsonrpc" property. Must be "2.0" if provided.',
-          data: 'kaplar',
+          message:
+            'Invalid JSON-RPC request: At path: jsonrpc -- Expected the literal `"2.0"`, but received: "kaplar".',
         }),
       );
 

diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts
@@ -60,6 +60,7 @@ import {
 } from '@metamask/snaps-utils';
 import {
   assert,
+  assertIsJsonRpcRequest,
   Duration,
   gtRange,
   gtVersion,
@@ -2420,8 +2421,16 @@ export class SnapController extends BaseController<
     snapId,
     origin,
     handler: handlerType,
-    request,
+    request: rawRequest,
   }: SnapRpcHookArgs & { snapId: ValidatedSnapId }): Promise<unknown> {
+    const request = {
+      jsonrpc: '2.0',
+      id: nanoid(),
+      ...rawRequest,
+    };
+
+    assertIsJsonRpcRequest(request);
+
     const permissionName = handlerEndowments[handlerType];
     const hasPermission = this.messagingSystem.call(
       'PermissionController:hasPermission',
@@ -2526,23 +2535,13 @@ export class SnapController extends BaseController<
         }
       }
 
-      let _request = request;
-      if (!hasProperty(request, 'jsonrpc')) {
-        _request = { ...(request as Record<string, unknown>), jsonrpc: '2.0' };
-      } else if (request.jsonrpc !== '2.0') {
-        throw ethErrors.rpc.invalidRequest({
-          message: 'Invalid "jsonrpc" property. Must be "2.0" if provided.',
-          data: request.jsonrpc,
-        });
-      }
-
       const timer = new Timer(this.maxRequestTime);
       this.#recordSnapRpcRequestStart(snapId, request.id, timer);
 
       const handleRpcRequestPromise = this.messagingSystem.call(
         'ExecutionService:handleRpcRequest',
         snapId,
-        { origin, handler: handlerType, request: _request },
+        { origin, handler: handlerType, request },
       );
 
       // This will either get the result or reject due to the timeout.