Merge branch 'develop' into feat/refactor-nft-experience

README.md

@@ -123,11 +123,11 @@ Before running e2e tests, ensure you've run `yarn install` to download dependenc
    - `yarn build:test` for main build
    - `yarn build:test:flask` for flask build
    - `yarn build:test:mmi` for mmi build
-   - `yarn build:test:mv3` for mv3 build
+   - `yarn build:test:mv2` for mv2 build
 3. Start a test build with live changes: `yarn start:test` is particularly useful for development. It starts a test build that automatically recompiles application code upon changes. This option is ideal for iterative testing and development. This command also allows you to generate test builds for various types, including:
    - `yarn start:test` for main build
    - `yarn start:test:flask` for flask build
-   - `yarn start:test:mv3` for mv3 build
+   - `yarn start:test:mv2` for mv2 build
 
 Note: The `yarn start:test` command (which initiates the testDev build type) has LavaMoat disabled for both the build system and the application, offering a streamlined testing experience during development. On the other hand, `yarn build:test` enables LavaMoat for enhanced security in both the build system and application, mirroring production environments more closely.
 
@@ -183,7 +183,7 @@ Different build types have different e2e tests sets. In order to run them look i
 ```console
     "test:e2e:chrome:mmi": "SELENIUM_BROWSER=chrome node test/e2e/run-all.js --mmi",
     "test:e2e:chrome:snaps": "SELENIUM_BROWSER=chrome node test/e2e/run-all.js --snaps",
-    "test:e2e:chrome:mv3": "ENABLE_MV3=true SELENIUM_BROWSER=chrome node test/e2e/run-all.js",
+    "test:e2e:firefox": "ENABLE_MV3=false SELENIUM_BROWSER=firefox node test/e2e/run-all.js",
 ```
 
 #### Note: Running MMI e2e tests

app/scripts/controllers/permissions/specifications.js

@@ -339,6 +339,7 @@ export const unrestrictedMethods = Object.freeze([
   'eth_newFilter',
   'eth_newPendingTransactionFilter',
   'eth_protocolVersion',
+  'eth_requestAccounts',
   'eth_sendRawTransaction',
   'eth_sendTransaction',
   'eth_sign',
@@ -348,17 +349,49 @@ export const unrestrictedMethods = Object.freeze([
   'eth_signTypedData_v4',
   'eth_submitHashrate',
   'eth_submitWork',
+  'eth_subscribe',
   'eth_syncing',
   'eth_uninstallFilter',
+  'eth_unsubscribe',
   'metamask_getProviderState',
+  'metamask_logWeb3ShimUsage',
+  'metamask_sendDomainMetadata',
   'metamask_watchAsset',
   'net_listening',
   'net_peerCount',
   'net_version',
   'personal_ecRecover',
   'personal_sign',
+  'wallet_addEthereumChain',
+  'wallet_getPermissions',
+  'wallet_requestPermissions',
+  'wallet_revokePermissions',
+  'wallet_registerOnboarding',
   'wallet_switchEthereumChain',
   'wallet_watchAsset',
   'web3_clientVersion',
   'web3_sha3',
+  ///: BEGIN:ONLY_INCLUDE_IF(snaps)
+  'wallet_getAllSnaps',
+  'wallet_getSnaps',
+  'wallet_requestSnaps',
+  'wallet_invokeSnap',
+  'wallet_invokeKeyring',
+  'snap_getClientStatus',
+  'snap_getFile',
+  'snap_createInterface',
+  'snap_updateInterface',
+  'snap_getInterfaceState',
+  ///: END:ONLY_INCLUDE_IF
+  ///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
+  'metamaskinstitutional_authenticate',
+  'metamaskinstitutional_reauthenticate',
+  'metamaskinstitutional_refresh_token',
+  'metamaskinstitutional_supported',
+  'metamaskinstitutional_portfolio',
+  'metamaskinstitutional_open_swaps',
+  'metamaskinstitutional_checkIfTokenIsPresent',
+  'metamaskinstitutional_setAccountAndNetwork',
+  'metamaskinstitutional_openAddHardwareWallet',
+  ///: END:ONLY_INCLUDE_IF
 ]);

app/scripts/lib/WeakRefObjectMap.ts

@@ -41,7 +41,7 @@ export class WeakRefObjectMap<RecordType extends Record<string, object>>
   set(key: string, value: RecordType): this {
     const weakRefValue: Partial<WeakRefObject<RecordType>> = {};
     for (const keyValue in value) {
-      if (!Object.hasOwn(value, keyValue)) {
+      if (!Object.prototype.hasOwnProperty.call(value, keyValue)) {
         continue;
       }
       const item: RecordType[typeof keyValue] = value[keyValue];
@@ -74,7 +74,7 @@ export class WeakRefObjectMap<RecordType extends Record<string, object>>
 
     const deRefValue: Partial<RecordType> = {};
     for (const keyValue in weakRefValue) {
-      if (!Object.hasOwn(weakRefValue, keyValue)) {
+      if (!Object.prototype.hasOwnProperty.call(weakRefValue, keyValue)) {
         continue;
       }
       const deref = weakRefValue[keyValue].deref();

app/scripts/lib/rpc-method-middleware/createMethodMiddleware.js

@@ -2,75 +2,92 @@ import { permissionRpcMethods } from '@metamask/permission-controller';
 import { selectHooks } from '@metamask/snaps-rpc-methods';
 import { hasProperty } from '@metamask/utils';
 import { ethErrors } from 'eth-rpc-errors';
-import { UNSUPPORTED_RPC_METHODS } from '../../../../shared/constants/network';
-import localHandlers from './handlers';
+import { handlers as localHandlers, legacyHandlers } from './handlers';
 
 const allHandlers = [...localHandlers, ...permissionRpcMethods.handlers];
 
-const handlerMap = allHandlers.reduce((map, handler) => {
-  for (const methodName of handler.methodNames) {
-    map[methodName] = handler;
-  }
-  return map;
-}, {});
+// The primary home of RPC method implementations in MetaMask. MUST be subsequent
+// to our permissioning logic in the JSON-RPC middleware pipeline.
+export const createMethodMiddleware = makeMethodMiddlewareMaker(allHandlers);
 
-const expectedHookNames = new Set(
-  allHandlers.flatMap(({ hookNames }) => Object.getOwnPropertyNames(hookNames)),
-);
+// A collection of RPC method implementations that, for legacy reasons, MAY precede
+// our permissioning logic in the JSON-RPC middleware pipeline.
+export const createLegacyMethodMiddleware =
+  makeMethodMiddlewareMaker(legacyHandlers);
 
 /**
- * Creates a json-rpc-engine middleware of RPC method implementations.
- *
- * Handlers consume functions that hook into the background, and only depend
- * on their signatures, not e.g. controller internals.
+ * Creates a method middleware factory function given a set of method handlers.
  *
- * @param  {Record<string, (...args: unknown[]) => unknown | Promise<unknown>>} hooks - Required "hooks" into our
- * controllers.
- * @returns {import('json-rpc-engine').JsonRpcMiddleware<unknown, unknown>} The method middleware function.
+ * @param {Record<string, import('@metamask/permission-controller').PermittedHandlerExport>} handlers - The RPC method
+ * handler implementations.
+ * @returns The method middleware factory function.
  */
-export function createMethodMiddleware(hooks) {
-  assertExpectedHook(hooks);
-
-  return async function methodMiddleware(req, res, next, end) {
-    // Reject unsupported methods.
-    if (UNSUPPORTED_RPC_METHODS.has(req.method)) {
-      return end(ethErrors.rpc.methodNotSupported());
+function makeMethodMiddlewareMaker(handlers) {
+  const handlerMap = handlers.reduce((map, handler) => {
+    for (const methodName of handler.methodNames) {
+      map[methodName] = handler;
     }
+    return map;
+  }, {});
+
+  const expectedHookNames = new Set(
+    handlers.flatMap(({ hookNames }) => Object.getOwnPropertyNames(hookNames)),
+  );
 
-    const handler = handlerMap[req.method];
-    if (handler) {
-      const { implementation, hookNames } = handler;
-      try {
-        // Implementations may or may not be async, so we must await them.
-        return await implementation(
-          req,
-          res,
-          next,
-          end,
-          selectHooks(hooks, hookNames),
-        );
-      } catch (error) {
-        if (process.env.METAMASK_DEBUG) {
-          console.error(error);
+  /**
+   * Creates a json-rpc-engine middleware of RPC method implementations.
+   *
+   * Handlers consume functions that hook into the background, and only depend
+   * on their signatures, not e.g. controller internals.
+   *
+   * @param  {Record<string, (...args: unknown[]) => unknown | Promise<unknown>>} hooks - Required "hooks" into our
+   * controllers.
+   * @returns {import('json-rpc-engine').JsonRpcMiddleware<unknown, unknown>} The method middleware function.
+   */
+  const makeMethodMiddleware = (hooks) => {
+    assertExpectedHook(hooks, expectedHookNames);
+
+    const methodMiddleware = async (req, res, next, end) => {
+      const handler = handlerMap[req.method];
+      if (handler) {
+        const { implementation, hookNames } = handler;
+        try {
+          // Implementations may or may not be async, so we must await them.
+          return await implementation(
+            req,
+            res,
+            next,
+            end,
+            selectHooks(hooks, hookNames),
+          );
+        } catch (error) {
+          if (process.env.METAMASK_DEBUG) {
+            console.error(error);
+          }
+          return end(
+            error instanceof Error
+              ? error
+              : ethErrors.rpc.internal({ data: error }),
+          );
         }
-        return end(
-          error instanceof Error
-            ? error
-            : ethErrors.rpc.internal({ data: error }),
-        );
       }
-    }
 
-    return next();
+      return next();
+    };
+
+    return methodMiddleware;
   };
+
+  return makeMethodMiddleware;
 }
 
 /**
- * Asserts that the hooks object only has all expected hooks and no extraneous ones.
+ * Asserts that the specified hooks object only has all expected hooks and no extraneous ones.
  *
  * @param {Record<string, unknown>} hooks - Required "hooks" into our controllers.
+ * @param {string[]} expectedHookNames - The expected hook names.
  */
-function assertExpectedHook(hooks) {
+function assertExpectedHook(hooks, expectedHookNames) {
   const missingHookNames = [];
   expectedHookNames.forEach((hookName) => {
     if (!hasProperty(hooks, hookName)) {

app/scripts/lib/rpc-method-middleware/createMethodMiddleware.test.js

@@ -3,14 +3,14 @@ import {
   assertIsJsonRpcFailure,
   assertIsJsonRpcSuccess,
 } from '@metamask/utils';
-import { createMethodMiddleware } from '.';
+import { createMethodMiddleware, createLegacyMethodMiddleware } from '.';
 
 jest.mock('@metamask/permission-controller', () => ({
   permissionRpcMethods: { handlers: [] },
 }));
 
-jest.mock('./handlers', () => [
-  {
+jest.mock('./handlers', () => {
+  const getHandler = () => ({
     implementation: (req, res, _next, end, hooks) => {
       if (Array.isArray(req.params)) {
         switch (req.params[0]) {
@@ -35,10 +35,18 @@ jest.mock('./handlers', () => [
     },
     hookNames: { hook1: true, hook2: true },
     methodNames: ['method1', 'method2'],
-  },
-]);
+  });
+
+  return {
+    handlers: [getHandler()],
+    legacyHandlers: [getHandler()],
+  };
+});
 
-describe('createMethodMiddleware', () => {
+describe.each([
+  ['createMethodMiddleware', createMethodMiddleware],
+  ['createLegacyMethodMiddleware', createLegacyMethodMiddleware],
+])('%s', (_name, createMiddleware) => {
   const method1 = 'method1';
 
   const getDefaultHooks = () => ({
@@ -47,17 +55,15 @@ describe('createMethodMiddleware', () => {
   });
 
   it('should return a function', () => {
-    const middleware = createMethodMiddleware(getDefaultHooks());
+    const middleware = createMiddleware(getDefaultHooks());
     expect(typeof middleware).toBe('function');
   });
 
   it('should throw an error if a required hook is missing', () => {
     const hooks = { hook1: () => 42 };
 
     // @ts-expect-error Intentional destructive testing
-    expect(() => createMethodMiddleware(hooks)).toThrow(
-      'Missing expected hooks',
-    );
+    expect(() => createMiddleware(hooks)).toThrow('Missing expected hooks');
   });
 
   it('should throw an error if an extraneous hook is provided', () => {
@@ -66,13 +72,11 @@ describe('createMethodMiddleware', () => {
       extraneousHook: () => 100,
     };
 
-    expect(() => createMethodMiddleware(hooks)).toThrow(
-      'Received unexpected hooks',
-    );
+    expect(() => createMiddleware(hooks)).toThrow('Received unexpected hooks');
   });
 
   it('should call the handler for the matching method (uses hook1)', async () => {
-    const middleware = createMethodMiddleware(getDefaultHooks());
+    const middleware = createMiddleware(getDefaultHooks());
     const engine = new JsonRpcEngine();
     engine.push(middleware);
 
@@ -88,7 +92,7 @@ describe('createMethodMiddleware', () => {
   });
 
   it('should call the handler for the matching method (uses hook2)', async () => {
-    const middleware = createMethodMiddleware(getDefaultHooks());
+    const middleware = createMiddleware(getDefaultHooks());
     const engine = new JsonRpcEngine();
     engine.push(middleware);
 
@@ -104,7 +108,7 @@ describe('createMethodMiddleware', () => {
   });
 
   it('should not call the handler for a non-matching method', async () => {
-    const middleware = createMethodMiddleware(getDefaultHooks());
+    const middleware = createMiddleware(getDefaultHooks());
     const engine = new JsonRpcEngine();
     engine.push(middleware);
 
@@ -122,23 +126,8 @@ describe('createMethodMiddleware', () => {
     });
   });
 
-  it('should reject unsupported methods', async () => {
-    const middleware = createMethodMiddleware(getDefaultHooks());
-    const engine = new JsonRpcEngine();
-    engine.push(middleware);
-
-    const response = await engine.handle({
-      jsonrpc: '2.0',
-      id: 1,
-      method: 'eth_signTransaction',
-    });
-    assertIsJsonRpcFailure(response);
-
-    expect(response.error.message).toBe('Method not supported.');
-  });
-
   it('should handle errors returned by the implementation', async () => {
-    const middleware = createMethodMiddleware(getDefaultHooks());
+    const middleware = createMiddleware(getDefaultHooks());
     const engine = new JsonRpcEngine();
     engine.push(middleware);
 
@@ -154,7 +143,7 @@ describe('createMethodMiddleware', () => {
   });
 
   it('should handle errors thrown by the implementation', async () => {
-    const middleware = createMethodMiddleware(getDefaultHooks());
+    const middleware = createMiddleware(getDefaultHooks());
     const engine = new JsonRpcEngine();
     engine.push(middleware);
 
@@ -170,7 +159,7 @@ describe('createMethodMiddleware', () => {
   });
 
   it('should handle non-errors thrown by the implementation', async () => {
-    const middleware = createMethodMiddleware(getDefaultHooks());
+    const middleware = createMiddleware(getDefaultHooks());
     const engine = new JsonRpcEngine();
     engine.push(middleware);