From e2c4e93ab08d6df4d81103fc7877fad08b1c5634 Mon Sep 17 00:00:00 2001 From: Olusegun Akintayo Date: Thu, 8 Jun 2023 13:26:18 +0300 Subject: [PATCH] When gas fees suggested by dapp is too high, show warning color and icon (#19088) * When gas fees suggested by dapp is too high, show warning color and icon Signed-off-by: Olusegun Akintayo tests Signed-off-by: Olusegun Akintayo Fix tests Signed-off-by: Olusegun Akintayo set a default for high gas fees Signed-off-by: Olusegun Akintayo Fix test cases where transaction is undefined. Signed-off-by: Olusegun Akintayo Fix locale error Signed-off-by: Olusegun Akintayo Fix error where dappSuggestedGasFees is null Signed-off-by: Olusegun Akintayo Fix icon for site suggested Signed-off-by: Olusegun Akintayo Fix unit tests snapshot Signed-off-by: Olusegun Akintayo * Fix QA Comments Signed-off-by: Olusegun Akintayo * lint:fix Signed-off-by: Olusegun Akintayo * Fix unit tests Signed-off-by: Olusegun Akintayo * Fix PR comments Signed-off-by: Olusegun Akintayo * Lint fix Signed-off-by: Olusegun Akintayo * Fix PR comment. - call setEstimateUsed only once. Signed-off-by: Olusegun Akintayo * use constants for Priority levels. Signed-off-by: Olusegun Akintayo --------- Signed-off-by: Olusegun Akintayo --- app/_locales/en/messages.json | 6 +++++ shared/constants/gas.ts | 1 + .../edit-gas-item/edit-gas-item.js | 2 ++ .../app/gas-details-item/gas-details-item.js | 9 +++++-- .../gas-details-item/gas-details-item.test.js | 27 +++++++++++++++++++ ui/helpers/constants/gas.js | 1 + ui/hooks/gasFeeInput/useGasFeeInputs.js | 21 ++++++++++++++- 7 files changed, 64 insertions(+), 3 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index e515c1b61..be79b0fc8 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -1024,6 +1024,12 @@ "message": "$1 has suggested this price.", "description": "$1 is url for the dapp that has suggested gas settings" }, + "dappSuggestedHigh": { + "message": "Site suggested" + }, + "dappSuggestedHighShortLabel": { + "message": "Site (high)" + }, "dappSuggestedShortLabel": { "message": "Site" }, diff --git a/shared/constants/gas.ts b/shared/constants/gas.ts index cf2f911bc..4753f8613 100644 --- a/shared/constants/gas.ts +++ b/shared/constants/gas.ts @@ -55,6 +55,7 @@ export enum PriorityLevels { high = 'high', custom = 'custom', dAppSuggested = 'dappSuggested', + dappSuggestedHigh = 'dappSuggestedHigh', } /** diff --git a/ui/components/app/edit-gas-fee-popover/edit-gas-item/edit-gas-item.js b/ui/components/app/edit-gas-fee-popover/edit-gas-item/edit-gas-item.js index 7856db4b7..63893dadd 100644 --- a/ui/components/app/edit-gas-fee-popover/edit-gas-item/edit-gas-item.js +++ b/ui/components/app/edit-gas-fee-popover/edit-gas-item/edit-gas-item.js @@ -25,6 +25,8 @@ const getTitleAndIcon = (priorityLevel, editGasMode) => { let title = priorityLevel; if (priorityLevel === PriorityLevels.dAppSuggested) { title = 'dappSuggestedShortLabel'; + } else if (priorityLevel === PriorityLevels.dappSuggestedHigh) { + title = 'dappSuggestedHighShortLabel'; } else if (priorityLevel === PriorityLevels.tenPercentIncreased) { icon = null; title = 'tenPercentIncreased'; diff --git a/ui/components/app/gas-details-item/gas-details-item.js b/ui/components/app/gas-details-item/gas-details-item.js index cc716900d..10e8ca459 100644 --- a/ui/components/app/gas-details-item/gas-details-item.js +++ b/ui/components/app/gas-details-item/gas-details-item.js @@ -21,6 +21,7 @@ import TransactionDetailItem from '../transaction-detail-item/transaction-detail import UserPreferencedCurrencyDisplay from '../user-preferenced-currency-display'; import { hexWEIToDecGWEI } from '../../../../shared/modules/conversion.utils'; import { useDraftTransactionWithTxParams } from '../../../hooks/useDraftTransactionWithTxParams'; +import { PriorityLevels } from '../../../../shared/constants/gas'; import GasDetailsItemTitle from './gas-details-item-title'; const GasDetailsItem = ({ userAcknowledgedGasMissing = false }) => { @@ -94,13 +95,17 @@ const GasDetailsItem = ({ userAcknowledgedGasMissing = false }) => { key="editGasSubTextFeeLabel" display="inline-flex" className={classNames('gas-details-item__gasfee-label', { - 'gas-details-item__gas-fee-warning': estimateUsed === 'high', + 'gas-details-item__gas-fee-warning': + estimateUsed === PriorityLevels.high || + estimateUsed === PriorityLevels.dappSuggestedHigh, })} > - {estimateUsed === 'high' && '⚠ '} + {(estimateUsed === PriorityLevels.high || + estimateUsed === PriorityLevels.dappSuggestedHigh) && + '⚠ '} {t('editGasSubTextFeeLabel')} diff --git a/ui/components/app/gas-details-item/gas-details-item.test.js b/ui/components/app/gas-details-item/gas-details-item.test.js index 9b61c28ce..87511133a 100644 --- a/ui/components/app/gas-details-item/gas-details-item.test.js +++ b/ui/components/app/gas-details-item/gas-details-item.test.js @@ -33,6 +33,7 @@ const render = ({ contextProps } = {}) => { useNativeCurrencyAsPrimaryCurrency: true, }, gasFeeEstimates: mockEstimates[GasEstimateTypes.feeMarket], + ...contextProps, }, }); @@ -74,6 +75,32 @@ describe('GasDetailsItem', () => { }); }); + it('should show warning icon if dapp estimates are high', async () => { + render({ + contextProps: { + gasFeeEstimates: { + high: { + suggestedMaxPriorityFeePerGas: '1', + }, + }, + transaction: { + txParams: { + gas: '0x52081', + maxFeePerGas: '0x38D7EA4C68000', + }, + userFeeLevel: 'medium', + dappSuggestedGasFees: { + maxPriorityFeePerGas: '0x38D7EA4C68000', + maxFeePerGas: '0x38D7EA4C68000', + }, + }, + }, + }); + await waitFor(() => { + expect(screen.queryByText('⚠ Max fee:')).toBeInTheDocument(); + }); + }); + it('should not show warning icon if estimates are not high', async () => { render({ contextProps: { transaction: { txParams: {}, userFeeLevel: 'low' } }, diff --git a/ui/helpers/constants/gas.js b/ui/helpers/constants/gas.js index d0f30408c..373bcbc15 100644 --- a/ui/helpers/constants/gas.js +++ b/ui/helpers/constants/gas.js @@ -37,6 +37,7 @@ export const PRIORITY_LEVEL_ICON_MAP = { medium: '🦊', high: '🦍', dappSuggested: '🌐', + dappSuggestedHigh: '🌐', swapSuggested: '🔄', custom: '⚙️', }; diff --git a/ui/hooks/gasFeeInput/useGasFeeInputs.js b/ui/hooks/gasFeeInput/useGasFeeInputs.js index 9b8d4a4df..275ebac99 100644 --- a/ui/hooks/gasFeeInput/useGasFeeInputs.js +++ b/ui/hooks/gasFeeInput/useGasFeeInputs.js @@ -17,6 +17,8 @@ import { useGasFeeEstimates } from '../useGasFeeEstimates'; import { editGasModeIsSpeedUpOrCancel } from '../../helpers/utils/gas'; import { hexToDecimal } from '../../../shared/modules/conversion.utils'; +import { Numeric } from '../../../shared/modules/Numeric'; +import { EtherDenomination } from '../../../shared/constants/common'; import { useGasFeeErrors } from './useGasFeeErrors'; import { useGasPriceInput } from './useGasPriceInput'; import { useMaxFeePerGasInput } from './useMaxFeePerGasInput'; @@ -96,6 +98,8 @@ export function useGasFeeInputs( minimumGasLimit = '0x5208', editGasMode = EditGasModes.modifyInPlace, ) { + const GAS_LIMIT_TOO_HIGH_IN_ETH = '1'; + const initialRetryTxMeta = { txParams: _transaction?.txParams, id: _transaction?.id, @@ -165,9 +169,24 @@ export function useGasFeeInputs( useEffect(() => { if (supportsEIP1559) { if (transaction?.userFeeLevel) { - setEstimateUsed(transaction?.userFeeLevel); setInternalEstimateToUse(transaction?.userFeeLevel); } + + const maximumGas = new Numeric(transaction?.txParams?.gas ?? '0x0', 16) + .times(new Numeric(transaction?.txParams?.maxFeePerGas ?? '0x0', 16)) + .toPrefixedHexString(); + + const fee = new Numeric(maximumGas, 16, EtherDenomination.WEI) + .toDenomination(EtherDenomination.ETH) + .toBase(10) + .toString(); + + if (Number(fee) > Number(GAS_LIMIT_TOO_HIGH_IN_ETH)) { + setEstimateUsed(PriorityLevels.dappSuggestedHigh); + } else if (transaction?.userFeeLevel) { + setEstimateUsed(transaction?.userFeeLevel); + } + setGasLimit(Number(hexToDecimal(transaction?.txParams?.gas ?? '0x0'))); } }, [