From 442edc5a914ec728cf4ff338f94b706771ec76de Mon Sep 17 00:00:00 2001 From: Olusegun Akintayo Date: Mon, 7 Mar 2022 23:14:48 +0400 Subject: [PATCH] Update EIP1559 params. (#13652) * Draft methods to brak updateTransaction into smaller more targeted methods. Signed-off-by: Akintayo A. Olusegun * normalize and validate tx params. Signed-off-by: Akintayo A. Olusegun * Method to normalize tx and check if it's unapproved. Signed-off-by: Akintayo A. Olusegun * Move the methods to controllers/transactions/index.js Signed-off-by: Akintayo A. Olusegun * Flesh out the methods to update transaction with custom notes. Signed-off-by: Akintayo A. Olusegun * Test update gas fees Signed-off-by: Akintayo A. Olusegun * Update swap approval transaction Signed-off-by: Akintayo A. Olusegun * use lodash to remove undefined properties update swap transaction tests Signed-off-by: Akintayo A. Olusegun * Updates transaction user settings. Signed-off-by: Akintayo A. Olusegun * Lint fixes. Signed-off-by: Akintayo A. Olusegun * Add Update Transaction Metrics Signed-off-by: Akintayo A. Olusegun * Update transaction gas fees actions.js Signed-off-by: Akintayo A. Olusegun * Lint fixes. Signed-off-by: Akintayo A. Olusegun * Update EIP 1559 Params. Signed-off-by: Akintayo A. Olusegun * Lint Fixes. Signed-off-by: Akintayo A. Olusegun * Documentations. Signed-off-by: Akintayo A. Olusegun * Remove metrics from this PR Signed-off-by: Akintayo A. Olusegun * Lint fixes: Removed unused variables Signed-off-by: Akintayo A. Olusegun * Add more params to updateTransactionGasFees. Signed-off-by: Akintayo A. Olusegun * Update eip1559 method to editableParams. Signed-off-by: Akintayo A. Olusegun * lint fixes. Signed-off-by: Akintayo A. Olusegun * Fix Mocha tests Signed-off-by: Akintayo A. Olusegun * add gasPrice to updateEditableParams Signed-off-by: Akintayo A. Olusegun * Remove duplicated Params in notes. Signed-off-by: Akintayo A. Olusegun * A few more tests to cover if transaction status is not unapproved transaction is passed more parameters than it requires. Signed-off-by: Akintayo A. Olusegun * Update EIP1559 params. Signed-off-by: Akintayo A. Olusegun * Remove metrics Signed-off-by: Akintayo A. Olusegun * Add updateTransactionParams to actions.js updateXXX and return txData at the end of updateXXX method Signed-off-by: Akintayo A. Olusegun * Lint fixes. Signed-off-by: Akintayo A. Olusegun * change method name to update editable params. Signed-off-by: Akintayo A. Olusegun * lint fixes. Signed-off-by: Akintayo A. Olusegun * Editable params should pass in the txParams and not the full tx object Signed-off-by: Akintayo A. Olusegun * Fix jest tests Signed-off-by: Akintayo A. Olusegun * lint fixes. Signed-off-by: Akintayo A. Olusegun * Do not hideLoading since we're not showing it. Signed-off-by: Akintayo A. Olusegun * Proper JSDOCs comments. Signed-off-by: Akintayo A. Olusegun * Remove gas settings from updateEditableParams since we already have a dedicated gas update method. Signed-off-by: Akintayo A. Olusegun * Fix duplicate codes from rebasing. Signed-off-by: Akintayo A. Olusegun * Lint fixes. Signed-off-by: Akintayo A. Olusegun --- app/scripts/controllers/transactions/index.js | 93 ++++++------------- app/scripts/metamask-controller.js | 6 ++ ui/ducks/send/send.js | 6 +- ui/ducks/send/send.test.js | 17 +++- ui/store/actions.js | 40 ++++++++ 5 files changed, 93 insertions(+), 69 deletions(-) diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index f68b88e65..24fb0cf78 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -375,15 +375,13 @@ export default class TransactionController extends EventEmitter { /** * * @param {string} txId - transaction id - * @param {object} editableParams - holds the eip1559 fees parameters - * @param editableParams.data - * @param editableParams.from - * @param editableParams.to - * @param editableParams.value - * @param editableParams.gas - * @param editableParams.gasPrice + * @param {object} editableParams - holds the editable parameters + * @param {object} editableParams.data + * @param {string} editableParams.from + * @param {string} editableParams.to + * @param {string} editableParams.value */ - updateEditableParams(txId, { data, from, to, value, gas, gasPrice }) { + updateEditableParams(txId, { data, from, to, value }) { if (!this._checkIfTxStatusIsUnapproved(txId)) { return; } @@ -394,8 +392,6 @@ export default class TransactionController extends EventEmitter { from, to, value, - gas, - gasPrice, }, }; @@ -410,23 +406,15 @@ export default class TransactionController extends EventEmitter { * * @param {string} txId - transaction id * @param {object} txGasFees - holds the gas fees parameters - * { - * gasLimit, - * gasPrice, - * maxPriorityFeePerGas, - * maxFeePerGas, - * estimateUsed, - * estimateSuggested - * } - * @param txGasFees.gasLimit - * @param txGasFees.gasPrice - * @param txGasFees.maxPriorityFeePerGas - * @param txGasFees.maxFeePerGas - * @param txGasFees.estimateUsed - * @param txGasFees.estimateSuggested - * @param txGasFees.defaultGasEstimates - * @param txGasFees.gas - * @param txGasFees.originalGasEstimate + * @param {string} txGasFees.gasLimit + * @param {string} txGasFees.gasPrice + * @param {string} txGasFees.maxPriorityFeePerGas + * @param {string} txGasFees.maxFeePerGas + * @param {string} txGasFees.estimateUsed + * @param {string} txGasFees.estimateSuggested + * @param {string} txGasFees.defaultGasEstimates + * @param {string} txGasFees.gas + * @param {string} txGasFees.originalGasEstimate */ updateTransactionGasFees( txId, @@ -472,12 +460,8 @@ export default class TransactionController extends EventEmitter { * * @param {string} txId - transaction id * @param {object} txEstimateBaseFees - holds the estimate base fees parameters - * { - * estimatedBaseFee, - * decEstimatedBaseFee - * } - * @param txEstimateBaseFees.estimatedBaseFee - * @param txEstimateBaseFees.decEstimatedBaseFee + * @param {string} txEstimateBaseFees.estimatedBaseFee + * @param {string} txEstimateBaseFees.decEstimatedBaseFee */ updateTransactionEstimatedBaseFee( txId, @@ -501,12 +485,8 @@ export default class TransactionController extends EventEmitter { * * @param {string} txId * @param {object} swapApprovalTransaction - holds the metadata and token symbol - * { - * type, - * sourceTokenSymbol - * } - * @param swapApprovalTransaction.type - * @param swapApprovalTransaction.sourceTokenSymbol + * @param {string} swapApprovalTransaction.type + * @param {string} swapApprovalTransaction.sourceTokenSymbol */ updateSwapApprovalTransaction(txId, { type, sourceTokenSymbol }) { if (!this._checkIfTxStatusIsUnapproved(txId)) { @@ -527,26 +507,15 @@ export default class TransactionController extends EventEmitter { * * @param {string} txId * @param {object} swapTransaction - holds the metadata - * { - * sourceTokenSymbol, - * destinationTokenSymbol, - * type, - * destinationTokenDecimals, - * destinationTokenAddress, - * swapMetaData, - * swapTokenValue, - * estimatedBaseFee, - * approvalTxId - *} - * @param swapTransaction.sourceTokenSymbol - * @param swapTransaction.destinationTokenSymbol - * @param swapTransaction.type - * @param swapTransaction.destinationTokenDecimals - * @param swapTransaction.destinationTokenAddress - * @param swapTransaction.swapMetaData - * @param swapTransaction.swapTokenValue - * @param swapTransaction.estimatedBaseFee - * @param swapTransaction.approvalTxId + * @param {string} swapTransaction.sourceTokenSymbol + * @param {string} swapTransaction.destinationTokenSymbol + * @param {string} swapTransaction.type + * @param {string} swapTransaction.destinationTokenDecimals + * @param {string} swapTransaction.destinationTokenAddress + * @param {string} swapTransaction.swapMetaData + * @param {string} swapTransaction.swapTokenValue + * @param {string} swapTransaction.estimatedBaseFee + * @param {string} swapTransaction.approvalTxId */ updateSwapTransaction( txId, @@ -565,7 +534,6 @@ export default class TransactionController extends EventEmitter { if (!this._checkIfTxStatusIsUnapproved(txId)) { return; } - let swapTransaction = { sourceTokenSymbol, destinationTokenSymbol, @@ -590,9 +558,8 @@ export default class TransactionController extends EventEmitter { * * @param {string} txId * @param {object} userSettings - holds the metadata - * { userEditedGasLimit, userFeeLevel } - * @param userSettings.userEditedGasLimit - * @param userSettings.userFeeLevel + * @param {string} userSettings.userEditedGasLimit + * @param {string} userSettings.userFeeLevel */ updateTransactionUserSettings(txId, { userEditedGasLimit, userFeeLevel }) { if (!this._checkIfTxStatusIsUnapproved(txId)) { diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index cec719181..9b2fc8783 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -1526,6 +1526,12 @@ export default class MetamaskController extends EventEmitter { ), getTransactions: txController.getTransactions.bind(txController), + updateEditableParams: txController.updateEditableParams.bind( + txController, + ), + updateTransactionGasFees: txController.updateTransactionGasFees.bind( + txController, + ), // messageManager signMessage: this.signMessage.bind(this), cancelMessage: this.cancelMessage.bind(this), diff --git a/ui/ducks/send/send.js b/ui/ducks/send/send.js index d01e954fc..d4b4c1084 100644 --- a/ui/ducks/send/send.js +++ b/ui/ducks/send/send.js @@ -53,7 +53,8 @@ import { hideLoadingIndication, showConfTxPage, showLoadingIndication, - updateTransaction, + updateEditableParams, + updateTransactionGasFees, addPollingTokenToAppState, removePollingTokenFromAppState, isCollectibleOwner, @@ -1700,7 +1701,8 @@ export function signTransaction() { eip1559support ? eip1559OnlyTxParamsToUpdate : txParams, ), }; - dispatch(updateTransaction(editingTx)); + dispatch(updateEditableParams(id, editingTx.txParams)); + dispatch(updateTransactionGasFees(id, editingTx.txParams)); } else if (asset.type === ASSET_TYPES.TOKEN) { // When sending a token transaction we have to the token.transfer method // on the token contract to construct the transaction. This results in diff --git a/ui/ducks/send/send.test.js b/ui/ducks/send/send.test.js index 741b1370c..8e901b9dc 100644 --- a/ui/ducks/send/send.test.js +++ b/ui/ducks/send/send.test.js @@ -95,6 +95,12 @@ describe('Send Slice', () => { jest .spyOn(Actions, 'isCollectibleOwner') .mockImplementation(() => Promise.resolve(true)); + jest.spyOn(Actions, 'updateEditableParams').mockImplementation(() => ({ + type: 'UPDATE_TRANSACTION_EDITABLE_PARAMS', + })); + jest + .spyOn(Actions, 'updateTransactionGasFees') + .mockImplementation(() => ({ type: 'UPDATE_TRANSACTION_GAS_FEES' })); }); describe('Reducers', () => { @@ -2070,10 +2076,13 @@ describe('Send Slice', () => { const actionResult = store.getActions(); - expect(actionResult).toHaveLength(5); - expect(actionResult[0].type).toStrictEqual('SHOW_LOADING_INDICATION'); - expect(actionResult[1].type).toStrictEqual('UPDATE_TRANSACTION_PARAMS'); - expect(actionResult[2].type).toStrictEqual('HIDE_LOADING_INDICATION'); + expect(actionResult).toHaveLength(2); + expect(actionResult[0].type).toStrictEqual( + 'UPDATE_TRANSACTION_EDITABLE_PARAMS', + ); + expect(actionResult[1].type).toStrictEqual( + 'UPDATE_TRANSACTION_GAS_FEES', + ); }); }); diff --git a/ui/store/actions.js b/ui/store/actions.js index 84643cc34..6d504bca6 100644 --- a/ui/store/actions.js +++ b/ui/store/actions.js @@ -673,6 +673,46 @@ const updateMetamaskStateFromBackground = () => { }); }; +export function updateEditableParams(txId, editableParams) { + return async (dispatch) => { + try { + await promisifiedBackground.updateEditableParams(txId, editableParams); + } catch (error) { + dispatch(txError(error)); + dispatch(goHome()); + log.error(error.message); + throw error; + } + + dispatch( + updateTransactionParams(editableParams.id, editableParams.txParams), + ); + const newState = await updateMetamaskStateFromBackground(); + dispatch(updateMetamaskState(newState)); + dispatch(showConfTxPage({ id: editableParams.id })); + return editableParams; + }; +} + +export function updateTransactionGasFees(txId, txGasFees) { + return async (dispatch) => { + try { + await promisifiedBackground.updateTransactionGasFees(txId, txGasFees); + } catch (error) { + dispatch(txError(error)); + dispatch(goHome()); + log.error(error.message); + throw error; + } + + dispatch(updateTransactionParams(txGasFees.id, txGasFees.txParams)); + const newState = await updateMetamaskStateFromBackground(); + dispatch(updateMetamaskState(newState)); + dispatch(showConfTxPage({ id: txGasFees.id })); + return txGasFees; + }; +} + export function updateTransaction(txData, dontShowLoadingIndicator) { return async (dispatch) => { !dontShowLoadingIndicator && dispatch(showLoadingIndication());