diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index 964be3bdb..27cf59277 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -361,13 +361,30 @@ export default class TransactionController extends EventEmitter { return transactions[txId]; } - _checkIfTxStatusIsUnapproved(txId) { + /** + * @param {number} txId + * @returns {boolean} + */ + _isUnapprovedTransaction(txId) { return ( this.txStateManager.getTransaction(txId).status === TRANSACTION_STATUSES.UNAPPROVED ); } + /** + * @param {number} txId + * @param {string} fnName + */ + _throwErrorIfNotUnapprovedTx(txId, fnName) { + if (!this._isUnapprovedTransaction(txId)) { + throw new Error( + `TransactionsController: Can only call ${fnName} on an unapproved transaction. + Current tx status: ${this.txStateManager.getTransaction(txId).status}`, + ); + } + } + _updateTransaction(txId, proposedUpdate, note) { const txMeta = this.txStateManager.getTransaction(txId); const updated = merge(txMeta, proposedUpdate); @@ -416,11 +433,7 @@ export default class TransactionController extends EventEmitter { * @returns {TransactionMeta} the txMeta of the updated transaction */ updateEditableParams(txId, { data, from, to, value, gas, gasPrice }) { - if (!this._checkIfTxStatusIsUnapproved(txId)) { - throw new Error( - 'Cannot call updateEditableParams on a transaction that is not in an unapproved state', - ); - } + this._throwErrorIfNotUnapprovedTx(txId, 'updateEditableParams'); const editableParams = { txParams: { @@ -474,11 +487,7 @@ export default class TransactionController extends EventEmitter { userFeeLevel, }, ) { - if (!this._checkIfTxStatusIsUnapproved(txId)) { - throw new Error( - 'Cannot call updateTransactionGasFees on a transaction that is not in an unapproved state', - ); - } + this._throwErrorIfNotUnapprovedTx(txId, 'updateTransactionGasFees'); let txGasFees = { txParams: { @@ -517,11 +526,10 @@ export default class TransactionController extends EventEmitter { txId, { estimatedBaseFee, decEstimatedBaseFee }, ) { - if (!this._checkIfTxStatusIsUnapproved(txId)) { - throw new Error( - 'Cannot call updateTransactionEstimatedBaseFee on a transaction that is not in an unapproved state', - ); - } + this._throwErrorIfNotUnapprovedTx( + txId, + 'updateTransactionEstimatedBaseFee', + ); let txEstimateBaseFees = { estimatedBaseFee, decEstimatedBaseFee }; // only update what is defined @@ -543,11 +551,7 @@ export default class TransactionController extends EventEmitter { * @returns {TransactionMeta} the txMeta of the updated transaction */ updateSwapApprovalTransaction(txId, { type, sourceTokenSymbol }) { - if (!this._checkIfTxStatusIsUnapproved(txId)) { - throw new Error( - 'Cannot call updateSwapApprovalTransaction on a transaction that is not in an unapproved state', - ); - } + this._throwErrorIfNotUnapprovedTx(txId, 'updateSwapApprovalTransaction'); let swapApprovalTransaction = { type, sourceTokenSymbol }; // only update what is defined @@ -589,11 +593,7 @@ export default class TransactionController extends EventEmitter { approvalTxId, }, ) { - if (!this._checkIfTxStatusIsUnapproved(txId)) { - throw new Error( - 'Cannot call updateSwapTransaction on a transaction that is not in an unapproved state', - ); - } + this._throwErrorIfNotUnapprovedTx(txId, 'updateSwapTransaction'); let swapTransaction = { sourceTokenSymbol, @@ -625,11 +625,7 @@ export default class TransactionController extends EventEmitter { * @returns {TransactionMeta} the txMeta of the updated transaction */ updateTransactionUserSettings(txId, { userEditedGasLimit, userFeeLevel }) { - if (!this._checkIfTxStatusIsUnapproved(txId)) { - throw new Error( - 'Cannot call updateTransactionUserSettings on a transaction that is not in an unapproved state', - ); - } + this._throwErrorIfNotUnapprovedTx(txId, 'updateTransactionUserSettings'); let userSettings = { userEditedGasLimit, userFeeLevel }; // only update what is defined diff --git a/app/scripts/controllers/transactions/index.test.js b/app/scripts/controllers/transactions/index.test.js index 0d99759bf..ce0a3339f 100644 --- a/app/scripts/controllers/transactions/index.test.js +++ b/app/scripts/controllers/transactions/index.test.js @@ -2186,10 +2186,10 @@ describe('Transaction Controller', function () { assert.equal(result.userFeeLevel, 'high'); }); - it('throws error if status is not unapproved', function () { + it('should not update and should throw error if status is not type "unapproved"', function () { txStateManager.addTransaction({ id: '4', - status: TRANSACTION_STATUSES.APPROVED, + status: TRANSACTION_STATUSES.DROPPED, metamaskNetworkId: currentNetworkId, txParams: { maxPriorityFeePerGas: '0x007', @@ -2200,14 +2200,18 @@ describe('Transaction Controller', function () { estimateUsed: '0x009', }); - try { - txController.updateTransactionGasFees('4', { maxFeePerGas: '0x0088' }); - } catch (e) { - assert.equal( - e.message, - 'Cannot call updateTransactionGasFees on a transaction that is not in an unapproved state', - ); - } + assert.throws( + () => + txController.updateTransactionGasFees('4', { + maxFeePerGas: '0x0088', + }), + Error, + `TransactionsController: Can only call updateTransactionGasFees on an unapproved transaction. + Current tx status: ${TRANSACTION_STATUSES.DROPPED}`, + ); + + const transaction = txStateManager.getTransaction('4'); + assert.equal(transaction.txParams.maxFeePerGas, '0x008'); }); it('does not update unknown parameters in update method', function () {