Fix for: Invalid transaction envelope type: specified type "0x0" but including...requires type: "0x2" (#14823)

* we should not call normalize if we're failing the transaction
refactor out update history
we should fail if un update we get an error and the warning message is
    error submitting.

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* refactor _setTransactionStatus

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

app/scripts/controllers/transactions/pending-tx-tracker.js

@@ -2,6 +2,7 @@ import EventEmitter from 'safe-event-emitter';
 import log from 'loglevel';
 import EthQuery from 'ethjs-query';
 import { TRANSACTION_STATUSES } from '../../../../shared/constants/transaction';
+import { ERROR_SUBMITTING } from './tx-state-manager';
 
 /**
  * Event emitter utility class for tracking the transactions as they
@@ -106,7 +107,7 @@ export default class PendingTransactionTracker extends EventEmitter {
         // encountered real error - transition to error state
         txMeta.warning = {
           error: errorMessage,
-          message: 'There was an error when resubmitting this transaction.',
+          message: ERROR_SUBMITTING,
         };
         this.emit('tx:warning', txMeta, err);
       }

app/scripts/controllers/transactions/tx-state-manager.js

@@ -18,6 +18,8 @@ import {
   validateConfirmedExternalTransaction,
 } from './lib/util';
 
+export const ERROR_SUBMITTING =
+  'There was an error when resubmitting this transaction.';
 /**
  * TransactionStatuses reimported from the shared transaction constants file
  *
@@ -304,9 +306,23 @@ export default class TransactionStateManager extends EventEmitter {
   updateTransaction(txMeta, note) {
     // normalize and validate txParams if present
     if (txMeta.txParams) {
-      txMeta.txParams = normalizeAndValidateTxParams(txMeta.txParams, false);
+      try {
+        txMeta.txParams = normalizeAndValidateTxParams(txMeta.txParams, false);
+      } catch (error) {
+        if (txMeta.warning.message === ERROR_SUBMITTING) {
+          this.setTxStatusFailed(txMeta.id, error);
+        } else {
+          throw error;
+        }
+
+        return;
+      }
     }
 
+    this._updateTransactionHistory(txMeta, note);
+  }
+
+  _updateTransactionHistory(txMeta, note) {
     // create txMeta snapshot for history
     const currentState = snapshotFromTxMeta(txMeta);
     // recover previous tx state obj
@@ -551,10 +567,11 @@ export default class TransactionStateManager extends EventEmitter {
       rpc: error.value,
       stack: error.stack,
     };
-    this.updateTransaction(
+    this._updateTransactionHistory(
       txMeta,
       'transactions:tx-state-manager#fail - add error',
     );
+
     this._setTransactionStatus(txId, TRANSACTION_STATUSES.FAILED);
   }
 
@@ -638,7 +655,7 @@ export default class TransactionStateManager extends EventEmitter {
 
     txMeta.status = status;
     try {
-      this.updateTransaction(
+      this._updateTransactionHistory(
         txMeta,
         `txStateManager: setting status to ${status}`,
       );

app/scripts/controllers/transactions/tx-state-manager.test.js

@@ -12,7 +12,7 @@ import {
 } from '../../../../shared/constants/network';
 import { GAS_LIMITS } from '../../../../shared/constants/gas';
 import { ORIGIN_METAMASK } from '../../../../shared/constants/app';
-import TxStateManager from './tx-state-manager';
+import TxStateManager, { ERROR_SUBMITTING } from './tx-state-manager';
 import { snapshotFromTxMeta } from './lib/tx-state-history-helpers';
 
 const VALID_ADDRESS = '0x0000000000000000000000000000000000000000';
@@ -918,6 +918,113 @@ describe('TransactionStateManager', function () {
       const { history } = txStateManager.getTransaction('1');
       assert.equal(history.length, 1, 'two history items (initial + diff)');
     });
+
+    it('should set tx status to failed if updating after error submitting', function () {
+      const txMeta = {
+        id: '1',
+        status: TRANSACTION_STATUSES.UNAPPROVED,
+        metamaskNetworkId: currentNetworkId,
+        txParams: {
+          from: VALID_ADDRESS_TWO,
+          to: VALID_ADDRESS,
+          gasLimit: '0x5028',
+          maxFeePerGas: '0x2540be400',
+          maxPriorityFeePerGas: '0x3b9aca00',
+        },
+      };
+
+      txStateManager.addTransaction(txMeta);
+      const { history } = txStateManager.getTransaction('1');
+      assert.equal(history.length, 1);
+
+      txMeta.txParams.type = '0x0';
+      txMeta.warning = {
+        message: ERROR_SUBMITTING,
+        error: 'Testing tx status failed with arbitrary error',
+      };
+
+      // should result in additional 2 history entries
+      txStateManager.updateTransaction(txMeta);
+
+      const result = txStateManager.getTransaction('1');
+
+      assert.equal(result.history.length, 3);
+
+      // history[1] should contain 3 entries
+      assert.equal(result.history[1].length, 3);
+      assert.equal(
+        result.history[1][0].note,
+        'transactions:tx-state-manager#fail - add error',
+      );
+      assert.equal(result.history[1][0].op, 'add');
+      assert.equal(result.history[1][0].path, '/txParams/type');
+      assert.equal(result.history[1][0].value, '0x0');
+
+      assert.equal(result.history[1][1].op, 'add');
+      assert.equal(result.history[1][1].path, '/warning');
+      assert.equal(result.history[1][1].value.message, ERROR_SUBMITTING);
+
+      assert.equal(result.history[1][2].op, 'add');
+      assert.equal(result.history[1][2].path, '/err');
+      assert.equal(
+        result.history[1][2].value.message,
+        'Invalid transaction envelope type: specified type "0x0" but including maxFeePerGas and maxPriorityFeePerGas requires type: "0x2"',
+      );
+
+      assert.equal(result.history[2].length, 1);
+      assert.equal(
+        result.history[2][0].note,
+        'txStateManager: setting status to failed',
+      );
+      assert.equal(result.history[2][0].op, 'replace');
+      assert.equal(result.history[2][0].path, '/status');
+      assert.equal(result.history[2][0].value, 'failed');
+    });
+
+    it('should set transaction status to failed', function () {
+      const txMeta = {
+        id: '1',
+        status: TRANSACTION_STATUSES.UNAPPROVED,
+        metamaskNetworkId: currentNetworkId,
+        txParams: {
+          from: VALID_ADDRESS_TWO,
+          to: VALID_ADDRESS,
+          gasPrice: '0x01',
+        },
+      };
+
+      txStateManager.addTransaction(txMeta);
+      const { history } = txStateManager.getTransaction('1');
+      assert.equal(history.length, 1);
+
+      // should result in additional 2 history entries
+      txStateManager.setTxStatusFailed(
+        txMeta.id,
+        new Error('Testing tx status failed with arbitrary error'),
+      );
+
+      const result = txStateManager.getTransaction('1');
+      assert.equal(result.history.length, 3);
+
+      assert.equal(
+        result.history[1][0].note,
+        'transactions:tx-state-manager#fail - add error',
+      );
+      assert.equal(result.history[1][0].op, 'add');
+      assert.equal(result.history[1][0].path, '/err');
+      assert.equal(
+        result.history[1][0].value.message,
+        'Testing tx status failed with arbitrary error',
+      );
+      assert.equal(result.history[2].length, 1);
+      assert.equal(
+        result.history[2][0].note,
+        'txStateManager: setting status to failed',
+      );
+      assert.equal(result.history[2][0].op, 'replace');
+      assert.equal(result.history[2][0].path, '/status');
+      assert.equal(result.history[2][0].value, 'failed');
+    });
   });
 
   describe('#getUnapprovedTxList', function () {