diff --git a/packages/snaps-controllers/src/snaps/SnapController.test.tsx b/packages/snaps-controllers/src/snaps/SnapController.test.tsx index 14b0e74372..a5ea4a82ce 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.test.tsx +++ b/packages/snaps-controllers/src/snaps/SnapController.test.tsx @@ -2192,6 +2192,27 @@ describe('SnapController', () => { }); describe('handleRequest', () => { + it('throws if the Snap is not installed', async () => { + const snapController = getSnapController(); + + await expect( + snapController.handleRequest({ + snapId: 'npm:foo' as SnapId, + origin: METAMASK_ORIGIN, + handler: HandlerType.OnUserInput, + request: { + jsonrpc: '2.0', + method: 'test', + params: { id: MOCK_INTERFACE_ID }, + }, + }), + ).rejects.toThrow( + 'The Snap "npm:foo" is not installed. Please install it before invoking it.', + ); + + snapController.destroy(); + }); + it.each( Object.keys(handlerEndowments).filter( (handler) => handlerEndowments[handler as HandlerType], diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts index 61b2af80f4..a1cee38882 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.ts @@ -3588,6 +3588,13 @@ export class SnapController extends BaseController< }: SnapRpcHookArgs & { snapId: SnapId }): Promise { this.#assertCanUsePlatform(); + const snap = this.get(snapId); + + assert( + snap, + `The Snap "${snapId}" is not installed. Please install it before invoking it.`, + ); + assert( origin === METAMASK_ORIGIN || isValidUrl(origin), "'origin' must be a valid URL or 'metamask'.", @@ -3666,11 +3673,11 @@ export class SnapController extends BaseController< throw new Error(`"${handlerType}" can only be invoked by MetaMask.`); } - if (!this.state.snaps[snapId].enabled) { + if (!snap.enabled) { throw new Error(`Snap "${snapId}" is disabled.`); } - if (this.state.snaps[snapId].status === SnapStatus.Installing) { + if (snap.status === SnapStatus.Installing) { throw new Error( `Snap "${snapId}" is currently being installed. Please try again later.`, ); diff --git a/packages/snaps-rpc-methods/jest.config.js b/packages/snaps-rpc-methods/jest.config.js index 38828b642d..bda41c9c0b 100644 --- a/packages/snaps-rpc-methods/jest.config.js +++ b/packages/snaps-rpc-methods/jest.config.js @@ -10,9 +10,9 @@ module.exports = deepmerge(baseConfig, { ], coverageThreshold: { global: { - branches: 95.7, + branches: 95.68, functions: 98.75, - lines: 98.99, + lines: 98.98, statements: 98.69, }, }, diff --git a/packages/snaps-rpc-methods/src/permitted/invokeKeyring.test.ts b/packages/snaps-rpc-methods/src/permitted/invokeKeyring.test.ts index 45b6b89a71..70b266195b 100644 --- a/packages/snaps-rpc-methods/src/permitted/invokeKeyring.test.ts +++ b/packages/snaps-rpc-methods/src/permitted/invokeKeyring.test.ts @@ -303,7 +303,7 @@ describe('wallet_invokeKeyring', () => { expect(response.error).toStrictEqual({ ...rpcErrors .invalidRequest({ - message: `The snap "${MOCK_SNAP_ID}" is not installed. Please install it first, before invoking the snap.`, + message: `The Snap "${MOCK_SNAP_ID}" is not installed. Please install it before invoking it.`, }) .serialize(), stack: expect.any(String), diff --git a/packages/snaps-rpc-methods/src/permitted/invokeKeyring.ts b/packages/snaps-rpc-methods/src/permitted/invokeKeyring.ts index d916636262..8a97178098 100644 --- a/packages/snaps-rpc-methods/src/permitted/invokeKeyring.ts +++ b/packages/snaps-rpc-methods/src/permitted/invokeKeyring.ts @@ -107,8 +107,9 @@ async function invokeKeyringImplementation( if (!getSnap(snapId)) { return end( + // Mirror error message from SnapController. rpcErrors.invalidRequest({ - message: `The snap "${snapId}" is not installed. Please install it first, before invoking the snap.`, + message: `The Snap "${snapId}" is not installed. Please install it before invoking it.`, }), ); } diff --git a/packages/snaps-rpc-methods/src/restricted/invokeSnap.test.ts b/packages/snaps-rpc-methods/src/restricted/invokeSnap.test.ts index 9df38a2f37..2e404008da 100644 --- a/packages/snaps-rpc-methods/src/restricted/invokeSnap.test.ts +++ b/packages/snaps-rpc-methods/src/restricted/invokeSnap.test.ts @@ -23,7 +23,6 @@ describe('builder', () => { targetName: WALLET_SNAP_PERMISSION_KEY, specificationBuilder: expect.any(Function), methodHooks: { - getSnap: true, handleSnapRpcRequest: true, }, }); @@ -33,7 +32,6 @@ describe('builder', () => { expect( invokeSnapBuilder.specificationBuilder({ methodHooks: { - getSnap: jest.fn(), handleSnapRpcRequest: jest.fn(), }, }), @@ -53,7 +51,6 @@ describe('builder', () => { describe('specificationBuilder', () => { const specification = invokeSnapBuilder.specificationBuilder({ methodHooks: { - getSnap: jest.fn(), handleSnapRpcRequest: jest.fn(), }, }); @@ -90,7 +87,6 @@ describe('implementation', () => { }) as any; it('calls handleSnapRpcRequest', async () => { const hooks = getMockHooks(); - hooks.getSnap.mockImplementation(getTruncatedSnap); const implementation = getInvokeSnapImplementation(hooks); await implementation({ context: { origin: MOCK_ORIGIN }, @@ -101,8 +97,6 @@ describe('implementation', () => { }, }); - expect(hooks.getSnap).toHaveBeenCalledTimes(1); - expect(hooks.getSnap).toHaveBeenCalledWith(MOCK_SNAP_ID); expect(hooks.handleSnapRpcRequest).toHaveBeenCalledWith({ handler: 'onRpcRequest', origin: MOCK_ORIGIN, @@ -113,27 +107,6 @@ describe('implementation', () => { snapId: MOCK_SNAP_ID, }); }); - - it('throws if snap is not installed', async () => { - const hooks = getMockHooks(); - const implementation = getInvokeSnapImplementation(hooks); - await expect( - implementation({ - context: { origin: MOCK_ORIGIN }, - method: WALLET_SNAP_PERMISSION_KEY, - params: { - snapId: MOCK_SNAP_ID, - request: { method: 'hello', params: {} }, - }, - }), - ).rejects.toThrow( - `The snap "${MOCK_SNAP_ID}" is not installed. Please install it first, before invoking the snap.`, - ); - expect(hooks.getSnap).toHaveBeenCalledTimes(1); - expect(hooks.getSnap).toHaveBeenCalledWith(MOCK_SNAP_ID); - - expect(hooks.handleSnapRpcRequest).not.toHaveBeenCalled(); - }); }); describe('handleSnapInstall', () => { diff --git a/packages/snaps-rpc-methods/src/restricted/invokeSnap.ts b/packages/snaps-rpc-methods/src/restricted/invokeSnap.ts index 166dd0520f..bffc491cd9 100644 --- a/packages/snaps-rpc-methods/src/restricted/invokeSnap.ts +++ b/packages/snaps-rpc-methods/src/restricted/invokeSnap.ts @@ -12,7 +12,7 @@ import type { RequestSnapsParams, RequestSnapsResult, } from '@metamask/snaps-sdk'; -import type { Snap, SnapRpcHookArgs } from '@metamask/snaps-utils'; +import type { SnapRpcHookArgs } from '@metamask/snaps-utils'; import { HandlerType, SnapCaveatType } from '@metamask/snaps-utils'; import type { Json, NonEmptyArray } from '@metamask/utils'; @@ -37,7 +37,6 @@ export type GetPermittedSnaps = { type AllowedActions = InstallSnaps | GetPermittedSnaps; export type InvokeSnapMethodHooks = { - getSnap: (snapId: string) => Snap | undefined; handleSnapRpcRequest: ({ snapId, origin, @@ -139,7 +138,6 @@ const specificationBuilder: PermissionSpecificationBuilder< }; const methodHooks: MethodHooksObject = { - getSnap: true, handleSnapRpcRequest: true, }; @@ -153,13 +151,11 @@ export const invokeSnapBuilder = Object.freeze({ * Builds the method implementation for `wallet_snap_*`. * * @param hooks - The RPC method hooks. - * @param hooks.getSnap - A function that retrieves all information stored about a snap. * @param hooks.handleSnapRpcRequest - A function that sends an RPC request to a snap's RPC handler or throws if that fails. * @returns The method implementation which returns the result of `handleSnapRpcRequest`. * @throws If the params are invalid. */ export function getInvokeSnapImplementation({ - getSnap, handleSnapRpcRequest, }: InvokeSnapMethodHooks) { return async function invokeSnap( @@ -169,12 +165,6 @@ export function getInvokeSnapImplementation({ const { snapId, request } = params as InvokeSnapParams; - if (!getSnap(snapId)) { - throw rpcErrors.invalidRequest({ - message: `The snap "${snapId}" is not installed. Please install it first, before invoking the snap.`, - }); - } - const { origin } = context; return (await handleSnapRpcRequest({