From 88286664b51f590ad152c70fcd30d77f86040a35 Mon Sep 17 00:00:00 2001 From: Olusegun Akintayo Date: Mon, 13 Jun 2022 20:49:18 +0400 Subject: [PATCH] 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 * refactor _setTransactionStatus Signed-off-by: Akintayo A. Olusegun --- .../transactions/pending-tx-tracker.js | 3 +- .../transactions/tx-state-manager.js | 23 +++- .../transactions/tx-state-manager.test.js | 109 +++++++++++++++++- 3 files changed, 130 insertions(+), 5 deletions(-) diff --git a/app/scripts/controllers/transactions/pending-tx-tracker.js b/app/scripts/controllers/transactions/pending-tx-tracker.js index 02b4cbeb4..2bd547189 100644 --- a/app/scripts/controllers/transactions/pending-tx-tracker.js +++ b/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); } diff --git a/app/scripts/controllers/transactions/tx-state-manager.js b/app/scripts/controllers/transactions/tx-state-manager.js index 33e23bbf8..c924c9b66 100644 --- a/app/scripts/controllers/transactions/tx-state-manager.js +++ b/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}`, ); diff --git a/app/scripts/controllers/transactions/tx-state-manager.test.js b/app/scripts/controllers/transactions/tx-state-manager.test.js index ba3dd6be2..7fbe6ec1e 100644 --- a/app/scripts/controllers/transactions/tx-state-manager.test.js +++ b/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 () {