From 81b3e6e925b9becc3efcfb560ef74d9eead24282 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Thu, 24 Mar 2022 11:16:51 -0230 Subject: [PATCH] Transaction updates methods return whole tx meta (#14147) * Transaction updates methods return whole tx meta * Fix unit test Co-authored-by: Dan Miller --- app/scripts/controllers/transactions/index.js | 37 ++++++++++++++++--- .../controllers/transactions/index.test.js | 18 ++++----- ui/store/actions.js | 29 +++++++++++---- 3 files changed, 61 insertions(+), 23 deletions(-) diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index 3310b38d2..44d8fac06 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -375,6 +375,7 @@ export default class TransactionController extends EventEmitter { } /** + * updates the params that are editible in the send edit flow * * @param {string} txId - transaction id * @param {object} editableParams - holds the editable parameters @@ -382,10 +383,13 @@ export default class TransactionController extends EventEmitter { * @param {string} editableParams.from * @param {string} editableParams.to * @param {string} editableParams.value + * @returns {TransactionMeta} the txMeta of the updated transaction */ updateEditableParams(txId, { data, from, to, value }) { if (!this._checkIfTxStatusIsUnapproved(txId)) { - return; + throw new Error( + 'Cannot call updateEditableParams on a transaction that is not in an unapproved state', + ); } const editableParams = { @@ -401,6 +405,7 @@ export default class TransactionController extends EventEmitter { editableParams.txParams = pickBy(editableParams.txParams); const note = `Update Editable Params for ${txId}`; this._updateTransaction(txId, editableParams, note); + return this._getTransaction(txId); } /** @@ -417,6 +422,7 @@ export default class TransactionController extends EventEmitter { * @param {string} txGasFees.defaultGasEstimates * @param {string} txGasFees.gas * @param {string} txGasFees.originalGasEstimate + * @returns {TransactionMeta} the txMeta of the updated transaction */ updateTransactionGasFees( txId, @@ -433,7 +439,9 @@ export default class TransactionController extends EventEmitter { }, ) { if (!this._checkIfTxStatusIsUnapproved(txId)) { - return; + throw new Error( + 'Cannot call updateTransactionGasFees on a transaction that is not in an unapproved state', + ); } let txGasFees = { @@ -455,6 +463,7 @@ export default class TransactionController extends EventEmitter { txGasFees = pickBy(txGasFees); const note = `Update Transaction Gas Fees for ${txId}`; this._updateTransaction(txId, txGasFees, note); + return this._getTransaction(txId); } /** @@ -464,13 +473,16 @@ export default class TransactionController extends EventEmitter { * @param {object} txEstimateBaseFees - holds the estimate base fees parameters * @param {string} txEstimateBaseFees.estimatedBaseFee * @param {string} txEstimateBaseFees.decEstimatedBaseFee + * @returns {TransactionMeta} the txMeta of the updated transaction */ updateTransactionEstimatedBaseFee( txId, { estimatedBaseFee, decEstimatedBaseFee }, ) { if (!this._checkIfTxStatusIsUnapproved(txId)) { - return; + throw new Error( + 'Cannot call updateTransactionEstimatedBaseFee on a transaction that is not in an unapproved state', + ); } let txEstimateBaseFees = { estimatedBaseFee, decEstimatedBaseFee }; @@ -479,6 +491,7 @@ export default class TransactionController extends EventEmitter { const note = `Update Transaction Estimated Base Fees for ${txId}`; this._updateTransaction(txId, txEstimateBaseFees, note); + return this._getTransaction(txId); } /** @@ -489,10 +502,13 @@ export default class TransactionController extends EventEmitter { * @param {object} swapApprovalTransaction - holds the metadata and token symbol * @param {string} swapApprovalTransaction.type * @param {string} swapApprovalTransaction.sourceTokenSymbol + * @returns {TransactionMeta} the txMeta of the updated transaction */ updateSwapApprovalTransaction(txId, { type, sourceTokenSymbol }) { if (!this._checkIfTxStatusIsUnapproved(txId)) { - return; + throw new Error( + 'Cannot call updateSwapApprovalTransaction on a transaction that is not in an unapproved state', + ); } let swapApprovalTransaction = { type, sourceTokenSymbol }; @@ -501,6 +517,7 @@ export default class TransactionController extends EventEmitter { const note = `Update Swap Approval Transaction for ${txId}`; this._updateTransaction(txId, swapApprovalTransaction, note); + return this._getTransaction(txId); } /** @@ -518,6 +535,7 @@ export default class TransactionController extends EventEmitter { * @param {string} swapTransaction.swapTokenValue * @param {string} swapTransaction.estimatedBaseFee * @param {string} swapTransaction.approvalTxId + * @returns {TransactionMeta} the txMeta of the updated transaction */ updateSwapTransaction( txId, @@ -534,7 +552,9 @@ export default class TransactionController extends EventEmitter { }, ) { if (!this._checkIfTxStatusIsUnapproved(txId)) { - return; + throw new Error( + 'Cannot call updateSwapTransaction on a transaction that is not in an unapproved state', + ); } let swapTransaction = { sourceTokenSymbol, @@ -553,6 +573,7 @@ export default class TransactionController extends EventEmitter { const note = `Update Swap Transaction for ${txId}`; this._updateTransaction(txId, swapTransaction, note); + return this._getTransaction(txId); } /** @@ -562,10 +583,13 @@ export default class TransactionController extends EventEmitter { * @param {object} userSettings - holds the metadata * @param {string} userSettings.userEditedGasLimit * @param {string} userSettings.userFeeLevel + * @returns {TransactionMeta} the txMeta of the updated transaction */ updateTransactionUserSettings(txId, { userEditedGasLimit, userFeeLevel }) { if (!this._checkIfTxStatusIsUnapproved(txId)) { - return; + throw new Error( + 'Cannot call updateTransactionUserSettings on a transaction that is not in an unapproved state', + ); } let userSettings = { userEditedGasLimit, userFeeLevel }; @@ -574,6 +598,7 @@ export default class TransactionController extends EventEmitter { const note = `Update User Settings for ${txId}`; this._updateTransaction(txId, userSettings, note); + return this._getTransaction(txId); } // ==================================================================================================================================================== diff --git a/app/scripts/controllers/transactions/index.test.js b/app/scripts/controllers/transactions/index.test.js index def07386e..0d99759bf 100644 --- a/app/scripts/controllers/transactions/index.test.js +++ b/app/scripts/controllers/transactions/index.test.js @@ -2186,7 +2186,7 @@ describe('Transaction Controller', function () { assert.equal(result.userFeeLevel, 'high'); }); - it('does not update if status is not unapproved', function () { + it('throws error if status is not unapproved', function () { txStateManager.addTransaction({ id: '4', status: TRANSACTION_STATUSES.APPROVED, @@ -2200,14 +2200,14 @@ describe('Transaction Controller', function () { estimateUsed: '0x009', }); - txController.updateTransactionGasFees('4', { maxFeePerGas: '0x0088' }); - let result = txStateManager.getTransaction('4'); - assert.equal(result.txParams.maxFeePerGas, '0x008'); - - // test update estimate used - txController.updateTransactionGasFees('4', { estimateUsed: '0x0099' }); - result = txStateManager.getTransaction('4'); - assert.equal(result.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', + ); + } }); it('does not update unknown parameters in update method', function () { diff --git a/ui/store/actions.js b/ui/store/actions.js index a0f8b0e26..1cae0cfe9 100644 --- a/ui/store/actions.js +++ b/ui/store/actions.js @@ -673,8 +673,9 @@ const updateMetamaskStateFromBackground = () => { export function updateSwapApprovalTransaction(txId, txSwapApproval) { return async (dispatch) => { + let updatedTransaction; try { - await promisifiedBackground.updateSwapApprovalTransaction( + updatedTransaction = await promisifiedBackground.updateSwapApprovalTransaction( txId, txSwapApproval, ); @@ -685,14 +686,18 @@ export function updateSwapApprovalTransaction(txId, txSwapApproval) { throw error; } - return txSwapApproval; + return updatedTransaction; }; } export function updateEditableParams(txId, editableParams) { return async (dispatch) => { + let updatedTransaction; try { - await promisifiedBackground.updateEditableParams(txId, editableParams); + updatedTransaction = await promisifiedBackground.updateEditableParams( + txId, + editableParams, + ); } catch (error) { dispatch(txError(error)); dispatch(goHome()); @@ -700,14 +705,18 @@ export function updateEditableParams(txId, editableParams) { throw error; } - return editableParams; + return updatedTransaction; }; } export function updateTransactionGasFees(txId, txGasFees) { return async (dispatch) => { + let updatedTransaction; try { - await promisifiedBackground.updateTransactionGasFees(txId, txGasFees); + updatedTransaction = await promisifiedBackground.updateTransactionGasFees( + txId, + txGasFees, + ); } catch (error) { dispatch(txError(error)); dispatch(goHome()); @@ -715,14 +724,18 @@ export function updateTransactionGasFees(txId, txGasFees) { throw error; } - return txGasFees; + return updatedTransaction; }; } export function updateSwapTransaction(txId, txSwap) { return async (dispatch) => { + let updatedTransaction; try { - await promisifiedBackground.updateSwapTransaction(txId, txSwap); + updatedTransaction = await promisifiedBackground.updateSwapTransaction( + txId, + txSwap, + ); } catch (error) { dispatch(txError(error)); dispatch(goHome()); @@ -730,7 +743,7 @@ export function updateSwapTransaction(txId, txSwap) { throw error; } - return txSwap; + return updatedTransaction; }; }