update txParams normalization and validation. (#11406)

app/scripts/controllers/transactions/lib/util.js

@@ -1,17 +1,23 @@
 import { ethErrors } from 'eth-rpc-errors';
 import { addHexPrefix } from '../../../lib/util';
-import { TRANSACTION_STATUSES } from '../../../../../shared/constants/transaction';
+import {
+  TRANSACTION_ENVELOPE_TYPES,
+  TRANSACTION_STATUSES,
+} from '../../../../../shared/constants/transaction';
 import { isValidHexAddress } from '../../../../../shared/modules/hexstring-utils';
 
 const normalizers = {
-  from: (from) => addHexPrefix(from),
+  from: addHexPrefix,
   to: (to, lowerCase) =>
     lowerCase ? addHexPrefix(to).toLowerCase() : addHexPrefix(to),
-  nonce: (nonce) => addHexPrefix(nonce),
+  nonce: addHexPrefix,
-  value: (value) => addHexPrefix(value),
+  value: addHexPrefix,
-  data: (data) => addHexPrefix(data),
+  data: addHexPrefix,
-  gas: (gas) => addHexPrefix(gas),
+  gas: addHexPrefix,
-  gasPrice: (gasPrice) => addHexPrefix(gasPrice),
+  gasPrice: addHexPrefix,
+  maxFeePerGas: addHexPrefix,
+  maxPriorityFeePerGas: addHexPrefix,
+  type: addHexPrefix,
 };
 
 export function normalizeAndValidateTxParams(txParams, lowerCase = true) {
@@ -38,6 +44,78 @@ export function normalizeTxParams(txParams, lowerCase = true) {
   return normalizedTxParams;
 }
 
+/**
+ * Given two fields, ensure that the second field is not included in txParams,
+ * and if it is throw an invalidParams error.
+ * @param {Object} txParams - the transaction parameters object
+ * @param {string} fieldBeingValidated - the current field being validated
+ * @param {string} mutuallyExclusiveField - the field to ensure is not provided
+ * @throws {ethErrors.rpc.invalidParams} - throws if mutuallyExclusiveField is
+ *  present in txParams.
+ */
+function ensureMutuallyExclusiveFieldsNotProvided(
+  txParams,
+  fieldBeingValidated,
+  mutuallyExclusiveField,
+) {
+  if (typeof txParams[mutuallyExclusiveField] !== 'undefined') {
+    throw ethErrors.rpc.invalidParams(
+      `Invalid transaction params: specified ${fieldBeingValidated} but also included ${mutuallyExclusiveField}, these cannot be mixed`,
+    );
+  }
+}
+
+/**
+ * Ensures that the provided value for field is a string, throws an
+ * invalidParams error if field is not a string.
+ * @param {Object} txParams - the transaction parameters object
+ * @param {string} field - the current field being validated
+ * @throws {ethErrors.rpc.invalidParams} - throws if field is not a string
+ */
+function ensureFieldIsString(txParams, field) {
+  if (typeof txParams[field] !== 'string') {
+    throw ethErrors.rpc.invalidParams(
+      `Invalid transaction params: ${field} is not a string. got: (${txParams[field]})`,
+    );
+  }
+}
+
+/**
+ * Ensures that the provided txParams has the proper 'type' specified for the
+ * given field, if it is provided. If types do not match throws an
+ * invalidParams error.
+ * @param {Object} txParams - the transaction parameters object
+ * @param {'gasPrice' | 'maxFeePerGas' | 'maxPriorityFeePerGas'} field - the
+ *  current field being validated
+ * @throws {ethErrors.rpc.invalidParams} - throws if type does not match the
+ *  expectations for provided field.
+ */
+function ensureProperTransactionEnvelopeTypeProvided(txParams, field) {
+  switch (field) {
+    case 'maxFeePerGas':
+    case 'maxPriorityFeePerGas':
+      if (
+        txParams.type &&
+        txParams.type !== TRANSACTION_ENVELOPE_TYPES.FEE_MARKET
+      ) {
+        throw ethErrors.rpc.invalidParams(
+          `Invalid transaction envelope type: specified type "${txParams.type}" but including maxFeePerGas and maxPriorityFeePerGas requires type: "${TRANSACTION_ENVELOPE_TYPES.FEE_MARKET}"`,
+        );
+      }
+      break;
+    case 'gasPrice':
+    default:
+      if (
+        txParams.type &&
+        txParams.type === TRANSACTION_ENVELOPE_TYPES.FEE_MARKET
+      ) {
+        throw ethErrors.rpc.invalidParams(
+          `Invalid transaction envelope type: specified type "${txParams.type}" but included a gasPrice instead of maxFeePerGas and maxPriorityFeePerGas`,
+        );
+      }
+  }
+}
+
 /**
  * Validates the given tx parameters
  * @param {Object} txParams - the tx params
@@ -64,12 +142,43 @@ export function validateTxParams(txParams) {
       case 'to':
         validateRecipient(txParams);
         break;
+      case 'gasPrice':
+        ensureProperTransactionEnvelopeTypeProvided(txParams, 'gasPrice');
+        ensureMutuallyExclusiveFieldsNotProvided(
+          txParams,
+          'gasPrice',
+          'maxFeePerGas',
+        );
+        ensureMutuallyExclusiveFieldsNotProvided(
+          txParams,
+          'gasPrice',
+          'maxPriorityFeePerGas',
+        );
+        ensureFieldIsString(txParams, 'gasPrice');
+        break;
+      case 'maxFeePerGas':
+        ensureProperTransactionEnvelopeTypeProvided(txParams, 'maxFeePerGas');
+        ensureMutuallyExclusiveFieldsNotProvided(
+          txParams,
+          'maxFeePerGas',
+          'gasPrice',
+        );
+        ensureFieldIsString(txParams, 'maxFeePerGas');
+        break;
+      case 'maxPriorityFeePerGas':
+        ensureProperTransactionEnvelopeTypeProvided(
+          txParams,
+          'maxPriorityFeePerGas',
+        );
+        ensureMutuallyExclusiveFieldsNotProvided(
+          txParams,
+          'maxPriorityFeePerGas',
+          'gasPrice',
+        );
+        ensureFieldIsString(txParams, 'maxPriorityFeePerGas');
+        break;
       case 'value':
-        if (typeof value !== 'string') {
+        ensureFieldIsString(txParams, 'value');
-          throw ethErrors.rpc.invalidParams(
-            `Invalid transaction params: ${key} is not a string. got: (${value})`,
-          );
-        }
         if (value.toString().includes('-')) {
           throw ethErrors.rpc.invalidParams(
             `Invalid transaction value "${value}": not a positive number.`,
@@ -90,11 +199,7 @@ export function validateTxParams(txParams) {
         }
         break;
       default:
-        if (typeof value !== 'string') {
+        ensureFieldIsString(txParams, key);
-          throw ethErrors.rpc.invalidParams(
-            `Invalid transaction params: ${key} is not a string. got: (${value})`,
-          );
-        }
     }
   });
 }

app/scripts/controllers/transactions/lib/util.test.js

@@ -1,4 +1,6 @@
 import { strict as assert } from 'assert';
+import { TRANSACTION_ENVELOPE_TYPES } from '../../../../../shared/constants/transaction';
+import { BURN_ADDRESS } from '../../../../../shared/modules/hexstring-utils';
 import * as txUtils from './util';
 
 describe('txUtils', function () {
@@ -48,6 +50,239 @@ describe('txUtils', function () {
         message: 'Invalid transaction value "-0x01": not a positive number.',
       });
     });
+
+    describe('when validating gasPrice', function () {
+      it('should error when specifying incorrect type', function () {
+        const txParams = {
+          gasPrice: '0x1',
+          type: TRANSACTION_ENVELOPE_TYPES.FEE_MARKET,
+          to: BURN_ADDRESS,
+        };
+
+        assert.throws(
+          () => {
+            txUtils.validateTxParams(txParams);
+          },
+          {
+            message: `Invalid transaction envelope type: specified type "0x2" but included a gasPrice instead of maxFeePerGas and maxPriorityFeePerGas`,
+          },
+        );
+      });
+
+      it('should error when gasPrice is not a string', function () {
+        const txParams = {
+          gasPrice: 1,
+          to: BURN_ADDRESS,
+        };
+
+        assert.throws(
+          () => {
+            txUtils.validateTxParams(txParams);
+          },
+          {
+            message:
+              'Invalid transaction params: gasPrice is not a string. got: (1)',
+          },
+        );
+      });
+
+      it('should error when specifying maxFeePerGas', function () {
+        const txParams = {
+          gasPrice: '0x1',
+          maxFeePerGas: '0x1',
+          to: BURN_ADDRESS,
+        };
+
+        assert.throws(
+          () => {
+            txUtils.validateTxParams(txParams);
+          },
+          {
+            message:
+              'Invalid transaction params: specified gasPrice but also included maxFeePerGas, these cannot be mixed',
+          },
+        );
+      });
+
+      it('should error when specifying maxPriorityFeePerGas', function () {
+        const txParams = {
+          gasPrice: '0x1',
+          maxPriorityFeePerGas: '0x1',
+          to: BURN_ADDRESS,
+        };
+
+        assert.throws(
+          () => {
+            txUtils.validateTxParams(txParams);
+          },
+          {
+            message:
+              'Invalid transaction params: specified gasPrice but also included maxPriorityFeePerGas, these cannot be mixed',
+          },
+        );
+      });
+
+      it('should validate if gasPrice is set with no type or EIP-1559 gas fields', function () {
+        const txParams = {
+          gasPrice: '0x1',
+          to: BURN_ADDRESS,
+        };
+        assert.doesNotThrow(() => txUtils.validateTxParams(txParams));
+      });
+
+      it('should validate if gasPrice is set with a type of "0x0"', function () {
+        const txParams = {
+          gasPrice: '0x1',
+          type: TRANSACTION_ENVELOPE_TYPES.LEGACY,
+          to: BURN_ADDRESS,
+        };
+        assert.doesNotThrow(() => txUtils.validateTxParams(txParams));
+      });
+    });
+
+    describe('when validating maxFeePerGas', function () {
+      it('should error when specifying incorrect type', function () {
+        const txParams = {
+          maxFeePerGas: '0x1',
+          type: TRANSACTION_ENVELOPE_TYPES.LEGACY,
+          to: BURN_ADDRESS,
+        };
+
+        assert.throws(
+          () => {
+            txUtils.validateTxParams(txParams);
+          },
+          {
+            message:
+              'Invalid transaction envelope type: specified type "0x0" but including maxFeePerGas and maxPriorityFeePerGas requires type: "0x2"',
+          },
+        );
+      });
+
+      it('should error when maxFeePerGas is not a string', function () {
+        const txParams = {
+          maxFeePerGas: 1,
+          to: BURN_ADDRESS,
+        };
+
+        assert.throws(
+          () => {
+            txUtils.validateTxParams(txParams);
+          },
+          {
+            message:
+              'Invalid transaction params: maxFeePerGas is not a string. got: (1)',
+          },
+        );
+      });
+
+      it('should error when specifying gasPrice', function () {
+        const txParams = {
+          gasPrice: '0x1',
+          maxFeePerGas: '0x1',
+          to: BURN_ADDRESS,
+        };
+
+        assert.throws(
+          () => {
+            txUtils.validateTxParams(txParams);
+          },
+          {
+            message:
+              'Invalid transaction params: specified gasPrice but also included maxFeePerGas, these cannot be mixed',
+          },
+        );
+      });
+
+      it('should validate if maxFeePerGas is set with no type or gasPrice field', function () {
+        const txParams = {
+          maxFeePerGas: '0x1',
+          to: BURN_ADDRESS,
+        };
+        assert.doesNotThrow(() => txUtils.validateTxParams(txParams));
+      });
+
+      it('should validate if maxFeePerGas is set with a type of "0x2"', function () {
+        const txParams = {
+          maxFeePerGas: '0x1',
+          type: TRANSACTION_ENVELOPE_TYPES.FEE_MARKET,
+          to: BURN_ADDRESS,
+        };
+        assert.doesNotThrow(() => txUtils.validateTxParams(txParams));
+      });
+    });
+
+    describe('when validating maxPriorityFeePerGas', function () {
+      it('should error when specifying incorrect type', function () {
+        const txParams = {
+          maxPriorityFeePerGas: '0x1',
+          type: TRANSACTION_ENVELOPE_TYPES.LEGACY,
+          to: BURN_ADDRESS,
+        };
+
+        assert.throws(
+          () => {
+            txUtils.validateTxParams(txParams);
+          },
+          {
+            message:
+              'Invalid transaction envelope type: specified type "0x0" but including maxFeePerGas and maxPriorityFeePerGas requires type: "0x2"',
+          },
+        );
+      });
+
+      it('should error when maxFeePerGas is not a string', function () {
+        const txParams = {
+          maxPriorityFeePerGas: 1,
+          to: BURN_ADDRESS,
+        };
+
+        assert.throws(
+          () => {
+            txUtils.validateTxParams(txParams);
+          },
+          {
+            message:
+              'Invalid transaction params: maxPriorityFeePerGas is not a string. got: (1)',
+          },
+        );
+      });
+
+      it('should error when specifying gasPrice', function () {
+        const txParams = {
+          gasPrice: '0x1',
+          maxPriorityFeePerGas: '0x1',
+          to: BURN_ADDRESS,
+        };
+
+        assert.throws(
+          () => {
+            txUtils.validateTxParams(txParams);
+          },
+          {
+            message:
+              'Invalid transaction params: specified gasPrice but also included maxPriorityFeePerGas, these cannot be mixed',
+          },
+        );
+      });
+
+      it('should validate if maxPriorityFeePerGas is set with no type or gasPrice field', function () {
+        const txParams = {
+          maxPriorityFeePerGas: '0x1',
+          to: BURN_ADDRESS,
+        };
+        assert.doesNotThrow(() => txUtils.validateTxParams(txParams));
+      });
+
+      it('should validate if maxPriorityFeePerGas is set with a type of "0x2"', function () {
+        const txParams = {
+          maxPriorityFeePerGas: '0x1',
+          type: TRANSACTION_ENVELOPE_TYPES.FEE_MARKET,
+          to: BURN_ADDRESS,
+        };
+        assert.doesNotThrow(() => txUtils.validateTxParams(txParams));
+      });
+    });
   });
 
   describe('#normalizeTxParams', function () {
@@ -58,6 +293,10 @@ describe('txUtils', function () {
         to: null,
         data: '68656c6c6f20776f726c64',
         random: 'hello world',
+        gasPrice: '1',
+        maxFeePerGas: '1',
+        maxPriorityFeePerGas: '1',
+        type: '1',
       };
 
       let normalizedTxParams = txUtils.normalizeTxParams(txParams);
@@ -89,6 +328,28 @@ describe('txUtils', function () {
         '0x',
         'to should be hex-prefixed',
       );
+
+      assert.equal(
+        normalizedTxParams.gasPrice,
+        '0x1',
+        'gasPrice should be hex-prefixed',
+      );
+
+      assert.equal(
+        normalizedTxParams.maxFeePerGas,
+        '0x1',
+        'maxFeePerGas should be hex-prefixed',
+      );
+      assert.equal(
+        normalizedTxParams.maxPriorityFeePerGas,
+        '0x1',
+        'maxPriorityFeePerGas should be hex-prefixed',
+      );
+      assert.equal(
+        normalizedTxParams.type,
+        '0x1',
+        'type should be hex-prefixed',
+      );
     });
   });
 

shared/constants/transaction.js

@@ -60,6 +60,33 @@ export const TRANSACTION_TYPES = {
   ETH_GET_ENCRYPTION_PUBLIC_KEY: MESSAGE_TYPE.ETH_GET_ENCRYPTION_PUBLIC_KEY,
 };
 
+/**
+ * In EIP-2718 typed transaction envelopes were specified, with the very first
+ * typed envelope being 'legacy' and describing the shape of the base
+ * transaction params that were hitherto the only transaction type sent on
+ * Ethereum.
+ * @typedef {Object} TransactionEnvelopeTypes
+ * @property {'0x0'} LEGACY - A legacy transaction, the very first type.
+ * @property {'0x1'} ACCESS_LIST - EIP-2930 defined the access list transaction
+ *  type that allowed for specifying the state that a transaction would act
+ *  upon in advance and theoretically save on gas fees.
+ * @property {'0x2'} FEE_MARKET - The type introduced comes from EIP-1559,
+ *  Fee Market describes the addition of a baseFee to blocks that will be
+ *  burned instead of distributed to miners. Transactions of this type have
+ *  both a maxFeePerGas (maximum total amount in gwei per gas to spend on the
+ *  transaction) which is inclusive of the maxPriorityFeePerGas (maximum amount
+ *  of gwei per gas from the transaction fee to distribute to miner).
+ */
+
+/**
+ * @type {TransactionEnvelopeTypes}
+ */
+export const TRANSACTION_ENVELOPE_TYPES = {
+  LEGACY: '0x0',
+  ACCESS_LIST: '0x1',
+  FEE_MARKET: '0x2',
+};
+
 /**
  * Transaction Status is a mix of Ethereum and MetaMask terminology, used internally
  * for transaction processing.