Refactor eth_sendTransaction method handler

The method handler for `eth_sendTransaction` has been moved to a
separate module to simplify unit testing. Unit tests have been added
as well.

This module was written in TypeScript, which asked for some additional
validation of the method parameters. We now throw a more explicit
message when the params are missing, and when the transaction
parameters are not an object. Previously the method would still throw
an error in these scenarios, just with a less helpful message. The
change in error message should be the only functional change here.

This refactor is intended to make testing PR #5619 easier.

app/core/RPCMethods/RPCMethodMiddleware.ts

@@ -324,13 +324,25 @@ export const getRpcMethodMiddleware = ({
       eth_coinbase: getEthAccounts,
       eth_sendTransaction: async () => {
         checkTabActive();
-        await checkActiveAccountAndChainId({
-          hostname,
-          address: req.params[0].from,
-          chainId: req.params[0].chainId,
-          checkSelectedAddress: isMMSDK || isWalletConnect,
+        return RPCMethods.eth_sendTransaction({
+          next,
+          req,
+          res,
+          validateAccountAndChainId: async ({
+            from,
+            chainId,
+          }: {
+            from?: string;
+            chainId?: number;
+          }) => {
+            await checkActiveAccountAndChainId({
+              hostname,
+              address: from,
+              chainId,
+              checkSelectedAddress: isMMSDK || isWalletConnect,
+            });
+          },
         });
-        next();
       },
       eth_signTransaction: async () => {
         // This is implemented later in our middleware stack – specifically, in

app/core/RPCMethods/eth_sendTransaction.test.ts

@@ -0,0 +1,110 @@
+// eslint-disable-next-line import/no-nodejs-modules
+import { inspect } from 'util';
+import type { JsonRpcRequest, PendingJsonRpcResponse } from 'json-rpc-engine';
+import eth_sendTransaction from './eth_sendTransaction';
+
+/**
+ * Construct a `eth_sendTransaction` JSON-RPC request.
+ *
+ * @param params - The request parameters.
+ * @returns The JSON-RPC request.
+ */
+function constructSendTransactionRequest(
+  params: unknown,
+): JsonRpcRequest<unknown> & { method: 'eth_sendTransaction' } {
+  return {
+    jsonrpc: '2.0',
+    id: 1,
+    method: 'eth_sendTransaction',
+    params,
+  };
+}
+
+/**
+ * Construct a pending JSON-RPC response.
+ *
+ * @returns A pending JSON-RPC response.
+ */
+function constructPendingJsonRpcResponse(): PendingJsonRpcResponse<unknown> {
+  return {
+    jsonrpc: '2.0',
+    id: 1,
+  };
+}
+
+describe('eth_sendTransaction', () => {
+  it('invokes next middleware for a valid request', async () => {
+    const nextMock = jest.fn();
+    const minimalValidParams = [{}];
+
+    await eth_sendTransaction({
+      next: nextMock,
+      req: constructSendTransactionRequest(minimalValidParams),
+      res: constructPendingJsonRpcResponse(),
+      validateAccountAndChainId: jest.fn(),
+    });
+
+    expect(nextMock).toHaveBeenCalledTimes(1);
+  });
+
+  const invalidParameters = [null, undefined, '', {}];
+  for (const invalidParameter of invalidParameters) {
+    it(`throws a JSON-RPC invalid parameters error if given "${inspect(
+      invalidParameter,
+    )}"`, async () => {
+      const nextMock = jest.fn();
+
+      await expect(
+        async () =>
+          await eth_sendTransaction({
+            next: nextMock,
+            req: constructSendTransactionRequest(invalidParameter),
+            res: constructPendingJsonRpcResponse(),
+            validateAccountAndChainId: jest.fn(),
+          }),
+      ).rejects.toThrow('Invalid parameters: expected an array');
+      expect(nextMock).not.toHaveBeenCalled();
+    });
+  }
+
+  const invalidTransactionParameters = [null, undefined, '', []];
+  for (const invalidTransactionParameter of invalidTransactionParameters) {
+    it(`throws a JSON-RPC invalid parameters error if given "${inspect(
+      invalidTransactionParameter,
+    )}" transaction parameters`, async () => {
+      const nextMock = jest.fn();
+      const invalidParameter = [invalidTransactionParameter];
+
+      await expect(
+        async () =>
+          await eth_sendTransaction({
+            next: nextMock,
+            req: constructSendTransactionRequest(invalidParameter),
+            res: constructPendingJsonRpcResponse(),
+            validateAccountAndChainId: jest.fn(),
+          }),
+      ).rejects.toThrow(
+        'Invalid parameters: expected the first parameter to be an object',
+      );
+      expect(nextMock).not.toHaveBeenCalled();
+    });
+  }
+
+  it('throws any validation errors', async () => {
+    const nextMock = jest.fn();
+    const minimalValidParams = [{}];
+
+    await expect(
+      async () =>
+        await eth_sendTransaction({
+          next: nextMock,
+          req: constructSendTransactionRequest(minimalValidParams),
+          res: constructPendingJsonRpcResponse(),
+          validateAccountAndChainId: jest.fn().mockImplementation(async () => {
+            throw new Error('test validation error');
+          }),
+        }),
+    ).rejects.toThrow('test validation error');
+    expect(nextMock).not.toHaveBeenCalled();
+  });
+});

app/core/RPCMethods/eth_sendTransaction.ts

@@ -0,0 +1,55 @@
+import type {
+  AsyncJsonRpcEngineNextCallback,
+  JsonRpcRequest,
+  PendingJsonRpcResponse,
+} from 'json-rpc-engine';
+import { isObject, hasProperty } from '@metamask/utils';
+import { ethErrors } from 'eth-json-rpc-errors';
+
+/**
+ * Handle a `eth_sendTransaction` request.
+ *
+ * @param args - Named arguments.
+ * @param args.checkActiveAccountAndChainId - A function that validates the account and chain ID
+ * used in the transaction.
+ * @param args.req - The JSON-RPC request.
+ * @param args.res - The JSON-RPC response.
+ */
+async function eth_sendTransaction({
+  next,
+  req,
+  res: _res,
+  validateAccountAndChainId,
+}: {
+  validateAccountAndChainId: (args: {
+    from: string;
+    chainId?: number;
+  }) => Promise<void>;
+  next: AsyncJsonRpcEngineNextCallback;
+  req: JsonRpcRequest<unknown> & { method: 'eth_sendTransaction' };
+  res: PendingJsonRpcResponse<unknown>;
+}) {
+  if (
+    !Array.isArray(req.params) &&
+    !(isObject(req.params) && hasProperty(req.params, 0))
+  ) {
+    throw ethErrors.rpc.invalidParams({
+      message: `Invalid parameters: expected an array`,
+    });
+  }
+  const transactionParameters = req.params[0];
+  if (!isObject(transactionParameters)) {
+    throw ethErrors.rpc.invalidParams({
+      message: `Invalid parameters: expected the first parameter to be an object`,
+    });
+  }
+  await validateAccountAndChainId({
+    from: req.params[0].from,
+    chainId: req.params[0].chainId,
+  });
+
+  // This is handled later in the network middleware
+  next();
+}
+
+export default eth_sendTransaction;

app/core/RPCMethods/index.js

@@ -1,7 +1,9 @@
+import eth_sendTransaction from './eth_sendTransaction';
 import wallet_addEthereumChain from './wallet_addEthereumChain.js';
 import wallet_switchEthereumChain from './wallet_switchEthereumChain.js';
 
 const RPCMethods = {
+  eth_sendTransaction,
   wallet_addEthereumChain,
   wallet_switchEthereumChain,
 };

package.json

@@ -152,8 +152,8 @@
     "@metamask/message-manager": "^1.0.1",
     "@metamask/network-controller": "^2.0.0",
     "@metamask/permission-controller": "^1.0.2",
-    "@metamask/preferences-controller": "^2.1.0",
     "@metamask/phishing-controller": "^2.0.0",
+    "@metamask/preferences-controller": "^2.1.0",
     "@metamask/sdk-communication-layer": "^0.1.0",
     "@metamask/swaps-controller": "^6.8.0",
     "@metamask/transaction-controller": "^2.0.0",
@@ -324,6 +324,7 @@
     "@metamask/eslint-config": "^7.0.0",
     "@metamask/eslint-config-typescript": "^7.0.0",
     "@metamask/mobile-provider": "^2.1.0",
+    "@metamask/utils": "^5.0.0",
     "@react-native-community/eslint-config": "^2.0.0",
     "@rpii/wdio-html-reporter": "^7.7.1",
     "@storybook/addon-actions": "^5.3",