Enhance transaction validation with standardized RPC errors (#1690)

## Explanation This PR aims to update the `TransactionController` to throw standardised `rpcErrors.invalidParams` errors using the `@metamask/rpc-errors` package that will ensure they are standardised and more easily parsed and recognised by the dApps. **Context** The extension `TransactionController` uses the `eth-rpc-errors` package to throw standardised errors when validation fails. This update is a part of the ongoing TransactionController unification effort. Note: As the colleagues pointed out in this PR `eth-rpc-errors` is deprecated so for equivalence the `@metamask/rpc-errors` package was added in the `TransactionController` and usages in the code was replaced accordingly. Bump `@metamask/rpc-errors` to v6 in `transaction-controller` and `assets-controller`. ### `@metamask/transaction-controller` - **CHANGED**: update `validateTxParams` to throw standardised errors using the `@metamask/rpc-errors` package. - add `@metamask/rpc-errors` package. - remove `eth-rpc-errors` and replace usage with the new package.
MetaMask · Sep 27, 2023 · 762882d · 762882d
1 parent ff9e2b3
commit 762882d
Show file tree

Hide file tree

Showing 9 changed files with 73 additions and 47 deletions.
diff --git a/packages/assets-controllers/package.json b/packages/assets-controllers/package.json
@@ -41,7 +41,7 @@
     "@metamask/metamask-eth-abis": "3.0.0",
     "@metamask/network-controller": "^13.0.0",
     "@metamask/preferences-controller": "^4.4.1",
-    "@metamask/rpc-errors": "^5.1.1",
+    "@metamask/rpc-errors": "^6.0.0",
     "@metamask/utils": "^6.2.0",
     "@types/uuid": "^8.3.0",
     "async-mutex": "^0.2.6",

diff --git a/packages/transaction-controller/package.json b/packages/transaction-controller/package.json
@@ -37,10 +37,10 @@
     "@metamask/eth-query": "^3.0.1",
     "@metamask/metamask-eth-abis": "^3.0.0",
     "@metamask/network-controller": "^13.0.0",
+    "@metamask/rpc-errors": "^6.0.0",
     "@metamask/utils": "^6.2.0",
     "async-mutex": "^0.2.6",
     "eth-method-registry": "1.1.0",
-    "eth-rpc-errors": "^4.0.2",
     "ethereumjs-util": "^7.0.10",
     "fast-json-patch": "^3.1.1",
     "lodash": "^4.17.21",

diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts
@@ -13,7 +13,7 @@ import type {
   Provider,
 } from '@metamask/network-controller';
 import { NetworkClientType, NetworkStatus } from '@metamask/network-controller';
-import { errorCodes } from 'eth-rpc-errors';
+import { errorCodes } from '@metamask/rpc-errors';
 import HttpProvider from 'ethjs-provider-http';
 import NonceTracker from 'nonce-tracker';
 

diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts
@@ -30,10 +30,10 @@ import type {
   NetworkState,
   Provider,
 } from '@metamask/network-controller';
+import { errorCodes, rpcErrors, providerErrors } from '@metamask/rpc-errors';
 import type { Hex } from '@metamask/utils';
 import { Mutex } from 'async-mutex';
 import MethodRegistry from 'eth-method-registry';
-import { errorCodes, ethErrors } from 'eth-rpc-errors';
 import { addHexPrefix, bufferToHex } from 'ethereumjs-util';
 import { EventEmitter } from 'events';
 import { merge, pickBy } from 'lodash';
@@ -1161,7 +1161,7 @@ export class TransactionController extends BaseController<
           if (error.code === errorCodes.provider.userRejectedRequest) {
             this.cancelTransaction(transactionId);
 
-            throw ethErrors.provider.userRejectedRequest(
+            throw providerErrors.userRejectedRequest(
               'User rejected the transaction',
             );
           } else {
@@ -1176,10 +1176,10 @@ export class TransactionController extends BaseController<
     switch (finalMeta?.status) {
       case TransactionStatus.failed:
         resultCallbacks?.error(finalMeta.error);
-        throw ethErrors.rpc.internal(finalMeta.error.message);
+        throw rpcErrors.internal(finalMeta.error.message);
 
       case TransactionStatus.cancelled:
-        const cancelError = ethErrors.rpc.internal(
+        const cancelError = rpcErrors.internal(
           'User cancelled the transaction',
         );
 
@@ -1191,7 +1191,7 @@ export class TransactionController extends BaseController<
         return finalMeta.hash as string;
 
       default:
-        const internalError = ethErrors.rpc.internal(
+        const internalError = rpcErrors.internal(
           `MetaMask Tx Signature: Unknown problem: ${JSON.stringify(
             finalMeta || transactionId,
           )}`,

diff --git a/packages/transaction-controller/src/external-transactions.test.ts b/packages/transaction-controller/src/external-transactions.test.ts
@@ -1,4 +1,4 @@
-import { ethErrors } from 'eth-rpc-errors';
+import { rpcErrors } from '@metamask/rpc-errors';
 
 import { validateConfirmedExternalTransaction } from './external-transactions';
 import type { TransactionMeta } from './types';
@@ -17,15 +17,15 @@ describe('validateConfirmedExternalTransaction', () => {
     expect(() =>
       validateConfirmedExternalTransaction(undefined, [], []),
     ).toThrow(
-      ethErrors.rpc.invalidParams(
+      rpcErrors.invalidParams(
         '"transactionMeta" or "transactionMeta.txParams" is missing',
       ),
     );
 
     expect(() =>
       validateConfirmedExternalTransaction({} as TransactionMeta, [], []),
     ).toThrow(
-      ethErrors.rpc.invalidParams(
+      rpcErrors.invalidParams(
         '"transactionMeta" or "transactionMeta.txParams" is missing',
       ),
     );
@@ -37,7 +37,7 @@ describe('validateConfirmedExternalTransaction', () => {
       '123',
     );
     expect(() => validateConfirmedExternalTransaction(transactionMeta)).toThrow(
-      ethErrors.rpc.invalidParams(
+      rpcErrors.invalidParams(
         'External transaction status should be "confirmed"',
       ),
     );
@@ -56,7 +56,7 @@ describe('validateConfirmedExternalTransaction', () => {
     expect(() =>
       validateConfirmedExternalTransaction(transactionMeta, [], pendingTxs),
     ).toThrow(
-      ethErrors.rpc.invalidParams(
+      rpcErrors.invalidParams(
         'External transaction nonce should not be in pending txs',
       ),
     );
@@ -75,7 +75,7 @@ describe('validateConfirmedExternalTransaction', () => {
     expect(() =>
       validateConfirmedExternalTransaction(transactionMeta, confirmedTxs, []),
     ).toThrow(
-      ethErrors.rpc.invalidParams(
+      rpcErrors.invalidParams(
         'External transaction nonce should not be in confirmed txs',
       ),
     );

diff --git a/packages/transaction-controller/src/external-transactions.ts b/packages/transaction-controller/src/external-transactions.ts
@@ -1,5 +1,5 @@
 // These utility functions are exclusively used by `confirmExternalTransaction` method in controller
-import { ethErrors } from 'eth-rpc-errors';
+import { rpcErrors } from '@metamask/rpc-errors';
 
 import { TransactionStatus } from './types';
 import type { TransactionMeta } from './types';
@@ -17,13 +17,13 @@ export function validateConfirmedExternalTransaction(
   pendingTxs?: TransactionMeta[],
 ) {
   if (!transactionMeta || !transactionMeta.txParams) {
-    throw ethErrors.rpc.invalidParams(
+    throw rpcErrors.invalidParams(
       '"transactionMeta" or "transactionMeta.txParams" is missing',
     );
   }
 
   if (transactionMeta.status !== TransactionStatus.confirmed) {
-    throw ethErrors.rpc.invalidParams(
+    throw rpcErrors.invalidParams(
       'External transaction status should be "confirmed"',
     );
   }
@@ -34,7 +34,7 @@ export function validateConfirmedExternalTransaction(
       (tx) => tx.txParams?.nonce === externalTxNonce,
     );
     if (foundPendingTxByNonce) {
-      throw ethErrors.rpc.invalidParams(
+      throw rpcErrors.invalidParams(
         'External transaction nonce should not be in pending txs',
       );
     }
@@ -45,7 +45,7 @@ export function validateConfirmedExternalTransaction(
       (tx) => tx.txParams?.nonce === externalTxNonce,
     );
     if (foundConfirmedTxByNonce) {
-      throw ethErrors.rpc.invalidParams(
+      throw rpcErrors.invalidParams(
         'External transaction nonce should not be in confirmed txs',
       );
     }

diff --git a/packages/transaction-controller/src/utils.test.ts b/packages/transaction-controller/src/utils.test.ts
@@ -1,3 +1,4 @@
+import { rpcErrors } from '@metamask/rpc-errors';
 import type { Transaction as NonceTrackerTransaction } from 'nonce-tracker/dist/NonceTracker';
 
 import type {
@@ -74,19 +75,25 @@ describe('utils', () => {
   describe('validateTxParams', () => {
     it('should throw if no from address', () => {
       expect(() => util.validateTxParams({} as any)).toThrow(
-        'Invalid "from" address: undefined must be a valid string.',
+        rpcErrors.invalidParams(
+          'Invalid "from" address: undefined must be a valid string.',
+        ),
       );
     });
 
     it('should throw if non-string from address', () => {
       expect(() => util.validateTxParams({ from: 1337 } as any)).toThrow(
-        'Invalid "from" address: 1337 must be a valid string.',
+        rpcErrors.invalidParams(
+          'Invalid "from" address: 1337 must be a valid string.',
+        ),
       );
     });
 
     it('should throw if invalid from address', () => {
       expect(() => util.validateTxParams({ from: '1337' } as any)).toThrow(
-        'Invalid "from" address: 1337 must be a valid string.',
+        rpcErrors.invalidParams(
+          'Invalid "from" address: 1337 must be a valid string.',
+        ),
       );
     });
 
@@ -96,13 +103,21 @@ describe('utils', () => {
           from: '0x3244e191f1b4903970224322180f1fbbc415696b',
           to: '0x',
         } as any),
-      ).toThrow('Invalid "to" address: 0x must be a valid string.');
+      ).toThrow(
+        rpcErrors.invalidParams(
+          'Invalid "to" address: 0x must be a valid string.',
+        ),
+      );
 
       expect(() =>
         util.validateTxParams({
           from: '0x3244e191f1b4903970224322180f1fbbc415696b',
         } as any),
-      ).toThrow('Invalid "to" address: undefined must be a valid string.');
+      ).toThrow(
+        rpcErrors.invalidParams(
+          'Invalid "to" address: undefined must be a valid string.',
+        ),
+      );
     });
 
     it('should delete data', () => {
@@ -121,7 +136,11 @@ describe('utils', () => {
           from: '0x3244e191f1b4903970224322180f1fbbc415696b',
           to: '1337',
         } as any),
-      ).toThrow('Invalid "to" address: 1337 must be a valid string.');
+      ).toThrow(
+        rpcErrors.invalidParams(
+          'Invalid "to" address: 1337 must be a valid string.',
+        ),
+      );
     });
 
     it('should throw if value is invalid', () => {
@@ -131,23 +150,35 @@ describe('utils', () => {
           to: '0x3244e191f1b4903970224322180f1fbbc415696b',
           value: '133-7',
         } as any),
-      ).toThrow('Invalid "value": 133-7 is not a positive number.');
+      ).toThrow(
+        rpcErrors.invalidParams(
+          'Invalid "value": 133-7 is not a positive number.',
+        ),
+      );
 
       expect(() =>
         util.validateTxParams({
           from: '0x3244e191f1b4903970224322180f1fbbc415696b',
           to: '0x3244e191f1b4903970224322180f1fbbc415696b',
           value: '133.7',
         } as any),
-      ).toThrow('Invalid "value": 133.7 number must be denominated in wei.');
+      ).toThrow(
+        rpcErrors.invalidParams(
+          'Invalid "value": 133.7 number must be denominated in wei.',
+        ),
+      );
 
       expect(() =>
         util.validateTxParams({
           from: '0x3244e191f1b4903970224322180f1fbbc415696b',
           to: '0x3244e191f1b4903970224322180f1fbbc415696b',
           value: 'hello',
         } as any),
-      ).toThrow('Invalid "value": hello number must be a valid number.');
+      ).toThrow(
+        rpcErrors.invalidParams(
+          'Invalid "value": hello number must be a valid number.',
+        ),
+      );
 
       expect(() =>
         util.validateTxParams({
@@ -156,7 +187,9 @@ describe('utils', () => {
           value: 'one million dollar$',
         } as any),
       ).toThrow(
-        'Invalid "value": one million dollar$ number must be a valid number.',
+        rpcErrors.invalidParams(
+          'Invalid "value": one million dollar$ number must be a valid number.',
+        ),
       );
 
       expect(() =>

diff --git a/packages/transaction-controller/src/utils.ts b/packages/transaction-controller/src/utils.ts
@@ -2,6 +2,7 @@ import {
   convertHexToDecimal,
   isValidHexAddress,
 } from '@metamask/controller-utils';
+import { rpcErrors } from '@metamask/rpc-errors';
 import { addHexPrefix, isHexString } from 'ethereumjs-util';
 import type { Transaction as NonceTrackerTransaction } from 'nonce-tracker/dist/NonceTracker';
 
@@ -60,7 +61,7 @@ export function validateTxParams(txParams: TransactionParams) {
     typeof txParams.from !== 'string' ||
     !isValidHexAddress(txParams.from)
   ) {
-    throw new Error(
+    throw rpcErrors.invalidParams(
       `Invalid "from" address: ${txParams.from} must be a valid string.`,
     );
   }
@@ -69,24 +70,26 @@ export function validateTxParams(txParams: TransactionParams) {
     if (txParams.data) {
       delete txParams.to;
     } else {
-      throw new Error(
+      throw rpcErrors.invalidParams(
         `Invalid "to" address: ${txParams.to} must be a valid string.`,
       );
     }
   } else if (txParams.to !== undefined && !isValidHexAddress(txParams.to)) {
-    throw new Error(
+    throw rpcErrors.invalidParams(
       `Invalid "to" address: ${txParams.to} must be a valid string.`,
     );
   }
 
   if (txParams.value !== undefined) {
     const value = txParams.value.toString();
     if (value.includes('-')) {
-      throw new Error(`Invalid "value": ${value} is not a positive number.`);
+      throw rpcErrors.invalidParams(
+        `Invalid "value": ${value} is not a positive number.`,
+      );
     }
 
     if (value.includes('.')) {
-      throw new Error(
+      throw rpcErrors.invalidParams(
         `Invalid "value": ${value} number must be denominated in wei.`,
       );
     }
@@ -97,7 +100,7 @@ export function validateTxParams(txParams: TransactionParams) {
       !isNaN(Number(value)) &&
       Number.isSafeInteger(intValue);
     if (!isValid) {
-      throw new Error(
+      throw rpcErrors.invalidParams(
         `Invalid "value": ${value} number must be a valid number.`,
       );
     }

diff --git a/yarn.lock b/yarn.lock
@@ -1402,7 +1402,7 @@ __metadata:
     "@metamask/metamask-eth-abis": 3.0.0
     "@metamask/network-controller": ^13.0.0
     "@metamask/preferences-controller": ^4.4.1
-    "@metamask/rpc-errors": ^5.1.1
+    "@metamask/rpc-errors": ^6.0.0
     "@metamask/utils": ^6.2.0
     "@types/jest": ^27.4.1
     "@types/node": ^16.18.24
@@ -2247,16 +2247,6 @@ __metadata:
   languageName: unknown
   linkType: soft
 
-"@metamask/rpc-errors@npm:^5.1.1":
-  version: 5.1.1
-  resolution: "@metamask/rpc-errors@npm:5.1.1"
-  dependencies:
-    "@metamask/utils": ^5.0.0
-    fast-safe-stringify: ^2.0.6
-  checksum: ccd1b24da66af3ae63960b79c04b86efb8b96acb89ca6f7e0bbfe636d23ba5cddeba533c0692eafb87c44ec6f840085372d0f21b39e05df9a80700ff61538a30
-  languageName: node
-  linkType: hard
-
 "@metamask/rpc-errors@npm:^6.0.0":
   version: 6.0.0
   resolution: "@metamask/rpc-errors@npm:6.0.0"
@@ -2587,14 +2577,14 @@ __metadata:
     "@metamask/eth-query": ^3.0.1
     "@metamask/metamask-eth-abis": ^3.0.0
     "@metamask/network-controller": ^13.0.0
+    "@metamask/rpc-errors": ^6.0.0
     "@metamask/utils": ^6.2.0
     "@types/jest": ^27.4.1
     "@types/node": ^16.18.24
     async-mutex: ^0.2.6
     babel-runtime: ^6.26.0
     deepmerge: ^4.2.2
     eth-method-registry: 1.1.0
-    eth-rpc-errors: ^4.0.2
     ethereumjs-util: ^7.0.10
     ethjs-provider-http: ^0.1.6
     fast-json-patch: ^3.1.1