From e6543a83effce673c22397f828f410dff74bace9 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Fri, 6 Aug 2021 19:48:53 -0230 Subject: [PATCH] Ensure transaction activity log supports EIP-1559 (#11794) * Ensure transaction activity log supports EIP-1559 * unit test fix * Add estimated base fee to swaps txMeta * fix lint and tests * Improve activity log copy --- app/_locales/en/messages.json | 6 +-- app/scripts/controllers/transactions/index.js | 20 +++++++++- app/scripts/metamask-controller.js | 14 ++++++- .../edit-gas-popover.component.js | 14 ++++++- .../transaction-activity-log.util.js | 40 +++++++++++++++---- ui/ducks/swaps/swaps.js | 6 ++- ui/hooks/useGasFeeInputs.js | 1 + .../confirm-transaction-base.component.js | 6 +++ .../confirm-transaction-base.container.js | 1 + ui/store/actions.js | 14 ++++++- 10 files changed, 103 insertions(+), 19 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 4966e16b9..799c3236f 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -2499,7 +2499,7 @@ "message": "transaction" }, "transactionCancelAttempted": { - "message": "Transaction cancel attempted with gas fee of $1 at $2" + "message": "Transaction cancel attempted with estimated gas fee of $1 at $2" }, "transactionCancelSuccess": { "message": "Transaction successfully cancelled at $2" @@ -2557,10 +2557,10 @@ "message": "Total Gas Fee" }, "transactionResubmitted": { - "message": "Transaction resubmitted with gas fee increased to $1 at $2" + "message": "Transaction resubmitted with estimated gas fee increased to $1 at $2" }, "transactionSubmitted": { - "message": "Transaction submitted with gas fee of $1 at $2." + "message": "Transaction submitted with estimated gas fee of $1 at $2." }, "transactionUpdated": { "message": "Transaction updated at $2." diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index 9afa217e1..5a1d72611 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -693,7 +693,11 @@ export default class TransactionController extends EventEmitter { * params instead of allowing this method to generate them * @returns {txMeta} */ - async createCancelTransaction(originalTxId, customGasSettings) { + async createCancelTransaction( + originalTxId, + customGasSettings, + { estimatedBaseFee } = {}, + ) { const originalTxMeta = this.txStateManager.getTransaction(originalTxId); const { txParams } = originalTxMeta; const { from, nonce } = txParams; @@ -723,6 +727,10 @@ export default class TransactionController extends EventEmitter { type: TRANSACTION_TYPES.CANCEL, }); + if (estimatedBaseFee) { + newTxMeta.estimatedBaseFee = estimatedBaseFee; + } + this.addTransaction(newTxMeta); await this.approveTransaction(newTxMeta.id); return newTxMeta; @@ -738,7 +746,11 @@ export default class TransactionController extends EventEmitter { * params instead of allowing this method to generate them * @returns {txMeta} */ - async createSpeedUpTransaction(originalTxId, customGasSettings) { + async createSpeedUpTransaction( + originalTxId, + customGasSettings, + { estimatedBaseFee } = {}, + ) { const originalTxMeta = this.txStateManager.getTransaction(originalTxId); const { txParams } = originalTxMeta; @@ -758,6 +770,10 @@ export default class TransactionController extends EventEmitter { type: TRANSACTION_TYPES.RETRY, }); + if (estimatedBaseFee) { + newTxMeta.estimatedBaseFee = estimatedBaseFee; + } + this.addTransaction(newTxMeta); await this.approveTransaction(newTxMeta.id); return newTxMeta; diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index adef54dbb..05bfad876 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -2099,10 +2099,15 @@ export default class MetamaskController extends EventEmitter { * instead of allowing this method to generate them * @returns {Object} MetaMask state */ - async createCancelTransaction(originalTxId, customGasSettings) { + async createCancelTransaction( + originalTxId, + customGasSettings, + newTxMetaProps, + ) { await this.txController.createCancelTransaction( originalTxId, customGasSettings, + newTxMetaProps, ); const state = await this.getState(); return state; @@ -2119,10 +2124,15 @@ export default class MetamaskController extends EventEmitter { * instead of allowing this method to generate them * @returns {Object} MetaMask state */ - async createSpeedUpTransaction(originalTxId, customGasSettings) { + async createSpeedUpTransaction( + originalTxId, + customGasSettings, + newTxMetaProps, + ) { await this.txController.createSpeedUpTransaction( originalTxId, customGasSettings, + newTxMetaProps, ); const state = await this.getState(); return state; diff --git a/ui/components/app/edit-gas-popover/edit-gas-popover.component.js b/ui/components/app/edit-gas-popover/edit-gas-popover.component.js index 4c152c411..c3768b6ea 100644 --- a/ui/components/app/edit-gas-popover/edit-gas-popover.component.js +++ b/ui/components/app/edit-gas-popover/edit-gas-popover.component.js @@ -90,6 +90,7 @@ export default function EditGasPopover({ onManualChange, balanceError, estimatesUnavailableWarning, + estimatedBaseFee, } = useGasFeeInputs(defaultEstimateToUse, transaction, minimumGasLimit, mode); /** @@ -139,10 +140,18 @@ export default function EditGasPopover({ switch (mode) { case EDIT_GAS_MODES.CANCEL: - dispatch(createCancelTransaction(transaction.id, newGasSettings)); + dispatch( + createCancelTransaction(transaction.id, newGasSettings, { + estimatedBaseFee, + }), + ); break; case EDIT_GAS_MODES.SPEED_UP: - dispatch(createSpeedUpTransaction(transaction.id, newGasSettings)); + dispatch( + createSpeedUpTransaction(transaction.id, newGasSettings, { + estimatedBaseFee, + }), + ); break; case EDIT_GAS_MODES.MODIFY_IN_PLACE: dispatch(updateTransaction(updatedTxMeta)); @@ -170,6 +179,7 @@ export default function EditGasPopover({ maxPriorityFeePerGas, networkAndAccountSupport1559, estimateToUse, + estimatedBaseFee, ]); let title = t('editGasTitle'); diff --git a/ui/components/app/transaction-activity-log/transaction-activity-log.util.js b/ui/components/app/transaction-activity-log/transaction-activity-log.util.js index ee3d7ee0a..f33421280 100644 --- a/ui/components/app/transaction-activity-log/transaction-activity-log.util.js +++ b/ui/components/app/transaction-activity-log/transaction-activity-log.util.js @@ -1,5 +1,6 @@ import { TRANSACTION_TYPES } from '../../../../shared/constants/transaction'; import { getHexGasTotal } from '../../../helpers/utils/confirm-tx.util'; +import { sumHexes } from '../../../helpers/utils/transactions.util'; import { // event constants @@ -22,6 +23,7 @@ import { const STATUS_PATH = '/status'; const GAS_PRICE_PATH = '/txParams/gasPrice'; const GAS_LIMIT_PATH = '/txParams/gas'; +const ESTIMATE_BASE_FEE_PATH = '/estimatedBaseFee'; // op constants const REPLACE_OP = 'replace'; @@ -53,11 +55,20 @@ export function getActivities(transaction, isFirstTransaction = false) { metamaskNetworkId, hash, history = [], - txParams: { gas: paramsGasLimit, gasPrice: paramsGasPrice }, - xReceipt: { status } = {}, + txParams: { + gas: paramsGasLimit, + gasPrice: paramsGasPrice, + maxPriorityFeePerGas: paramsMaxPriorityFeePerGas, + }, + txReceipt: { status } = {}, type, + estimatedBaseFee: paramsEstimatedBaseFee, } = transaction; + const paramsEip1559Price = + paramsEstimatedBaseFee && + paramsMaxPriorityFeePerGas && + sumHexes(paramsEstimatedBaseFee, paramsMaxPriorityFeePerGas); let cachedGasLimit = '0x0'; let cachedGasPrice = '0x0'; @@ -66,13 +77,19 @@ export function getActivities(transaction, isFirstTransaction = false) { if (index === 0 && !Array.isArray(base) && base.txParams) { const { time: timestamp, - txParams: { value, gas = '0x0', gasPrice = '0x0' } = {}, + estimatedBaseFee, + txParams: { value, gas = '0x0', gasPrice, maxPriorityFeePerGas } = {}, } = base; + + const eip1559Price = + estimatedBaseFee && + maxPriorityFeePerGas && + sumHexes(estimatedBaseFee, maxPriorityFeePerGas); // The cached gas limit and gas price are used to display the gas fee in the activity log. We // need to cache these values because the status update history events don't provide us with // the latest gas limit and gas price. cachedGasLimit = gas; - cachedGasPrice = gasPrice; + cachedGasPrice = eip1559Price || gasPrice || '0x0'; if (isFirstTransaction) { return acc.concat({ @@ -95,14 +112,16 @@ export function getActivities(transaction, isFirstTransaction = false) { // timestamp, the first sub-entry in a history entry should. const timestamp = entryTimestamp || (base[0] && base[0].timestamp); - if (path in eventPathsHash && op === REPLACE_OP) { + const isAddBaseFee = path === ESTIMATE_BASE_FEE_PATH && op === 'add'; + + if ((path in eventPathsHash && op === REPLACE_OP) || isAddBaseFee) { switch (path) { case STATUS_PATH: { const gasFee = cachedGasLimit === '0x0' && cachedGasPrice === '0x0' ? getHexGasTotal({ gasLimit: paramsGasLimit, - gasPrice: paramsGasPrice, + gasPrice: paramsEip1559Price || paramsGasPrice, }) : getHexGasTotal({ gasLimit: cachedGasLimit, @@ -144,7 +163,8 @@ export function getActivities(transaction, isFirstTransaction = false) { // previously submitted event. These events happen when the gas limit and gas price is // changed at the confirm screen. case GAS_PRICE_PATH: - case GAS_LIMIT_PATH: { + case GAS_LIMIT_PATH: + case ESTIMATE_BASE_FEE_PATH: { const lastEvent = events[events.length - 1] || {}; const { lastEventKey } = lastEvent; @@ -152,6 +172,12 @@ export function getActivities(transaction, isFirstTransaction = false) { cachedGasLimit = value; } else if (path === GAS_PRICE_PATH) { cachedGasPrice = value; + } else if (path === ESTIMATE_BASE_FEE_PATH) { + cachedGasPrice = paramsEip1559Price || base?.txParams?.gasPrice; + lastEvent.value = getHexGasTotal({ + gasLimit: paramsGasLimit, + gasPrice: cachedGasPrice, + }); } if ( diff --git a/ui/ducks/swaps/swaps.js b/ui/ducks/swaps/swaps.js index 22eaed69c..d12d3c4ff 100644 --- a/ui/ducks/swaps/swaps.js +++ b/ui/ducks/swaps/swaps.js @@ -674,19 +674,21 @@ export const signAndSendTransactions = (history, metaMetricsEvent) => { let maxFeePerGas; let maxPriorityFeePerGas; let baseAndPriorityFeePerGas; + let decEstimatedBaseFee; if (networkAndAccountSupports1559) { const { high: { suggestedMaxFeePerGas, suggestedMaxPriorityFeePerGas }, estimatedBaseFee = '0', } = getGasFeeEstimates(state); + decEstimatedBaseFee = decGWEIToHexWEI(estimatedBaseFee); maxFeePerGas = customMaxFeePerGas || decGWEIToHexWEI(suggestedMaxFeePerGas); maxPriorityFeePerGas = customMaxPriorityFeePerGas || decGWEIToHexWEI(suggestedMaxPriorityFeePerGas); baseAndPriorityFeePerGas = addHexes( - decGWEIToHexWEI(estimatedBaseFee), + decEstimatedBaseFee, maxPriorityFeePerGas, ); } @@ -816,6 +818,7 @@ export const signAndSendTransactions = (history, metaMetricsEvent) => { updateTransaction( { ...approveTxMeta, + estimatedBaseFee: decEstimatedBaseFee, type: TRANSACTION_TYPES.SWAP_APPROVAL, sourceTokenSymbol: sourceTokenInfo.symbol, }, @@ -855,6 +858,7 @@ export const signAndSendTransactions = (history, metaMetricsEvent) => { updateTransaction( { ...tradeTxMeta, + estimatedBaseFee: decEstimatedBaseFee, sourceTokenSymbol: sourceTokenInfo.symbol, destinationTokenSymbol: destinationTokenInfo.symbol, type: TRANSACTION_TYPES.SWAP, diff --git a/ui/hooks/useGasFeeInputs.js b/ui/hooks/useGasFeeInputs.js index 16dc9b263..b153e3fff 100644 --- a/ui/hooks/useGasFeeInputs.js +++ b/ui/hooks/useGasFeeInputs.js @@ -544,5 +544,6 @@ export function useGasFeeInputs( }, balanceError, estimatesUnavailableWarning, + estimatedBaseFee: gasSettings.baseFeePerGas, }; } diff --git a/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js b/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js index e15b80126..3a3364466 100644 --- a/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js +++ b/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js @@ -111,6 +111,7 @@ export default class ConfirmTransactionBase extends Component { useNativeCurrencyAsPrimaryCurrency: PropTypes.bool, maxFeePerGas: PropTypes.string, maxPriorityFeePerGas: PropTypes.string, + baseFeePerGas: PropTypes.string, }; state = { @@ -612,6 +613,7 @@ export default class ConfirmTransactionBase extends Component { updateCustomNonce, maxFeePerGas, maxPriorityFeePerGas, + baseFeePerGas, } = this.props; const { submitting } = this.state; @@ -619,6 +621,10 @@ export default class ConfirmTransactionBase extends Component { return; } + if (baseFeePerGas) { + txData.estimatedBaseFee = baseFeePerGas; + } + if (maxFeePerGas) { txData.txParams = { ...txData.txParams, diff --git a/ui/pages/confirm-transaction-base/confirm-transaction-base.container.js b/ui/pages/confirm-transaction-base/confirm-transaction-base.container.js index 6ccaf561d..f10330399 100644 --- a/ui/pages/confirm-transaction-base/confirm-transaction-base.container.js +++ b/ui/pages/confirm-transaction-base/confirm-transaction-base.container.js @@ -200,6 +200,7 @@ const mapStateToProps = (state, ownProps) => { useNativeCurrencyAsPrimaryCurrency, maxFeePerGas: gasEstimationObject.maxFeePerGas, maxPriorityFeePerGas: gasEstimationObject.maxPriorityFeePerGas, + baseFeePerGas: gasEstimationObject.baseFeePerGas, }; }; diff --git a/ui/store/actions.js b/ui/store/actions.js index 594c10595..e265c923d 100644 --- a/ui/store/actions.js +++ b/ui/store/actions.js @@ -1340,7 +1340,11 @@ export function clearPendingTokens() { }; } -export function createCancelTransaction(txId, customGasSettings) { +export function createCancelTransaction( + txId, + customGasSettings, + newTxMetaProps, +) { log.debug('background.cancelTransaction'); let newTxId; @@ -1349,6 +1353,7 @@ export function createCancelTransaction(txId, customGasSettings) { background.createCancelTransaction( txId, customGasSettings, + newTxMetaProps, (err, newState) => { if (err) { dispatch(displayWarning(err.message)); @@ -1368,7 +1373,11 @@ export function createCancelTransaction(txId, customGasSettings) { }; } -export function createSpeedUpTransaction(txId, customGasSettings) { +export function createSpeedUpTransaction( + txId, + customGasSettings, + newTxMetaProps, +) { log.debug('background.createSpeedUpTransaction'); let newTx; @@ -1377,6 +1386,7 @@ export function createSpeedUpTransaction(txId, customGasSettings) { background.createSpeedUpTransaction( txId, customGasSettings, + newTxMetaProps, (err, newState) => { if (err) { dispatch(displayWarning(err.message));