From 6d92759853d49f281204e980d63426d95c7f78d8 Mon Sep 17 00:00:00 2001 From: David Walsh Date: Tue, 20 Jul 2021 14:34:32 -0500 Subject: [PATCH] EIP-1559 - Implement form validation for EIP-1559 (#11540) * Restructure advanced gas form errors * Use shared constant for gas errors * Add validation for fields too low * Add warnings for high max fee and max priority fee * Fix lint * Fix priority fee high warning string --- app/_locales/en/messages.json | 15 +++++ .../advanced-gas-controls.component.js | 24 ++++++-- .../app/advanced-gas-controls/index.scss | 15 ++--- .../edit-gas-display.component.js | 24 ++++---- ui/components/app/edit-gas-display/index.scss | 8 +++ .../edit-gas-popover.component.js | 17 ++---- ui/helpers/constants/gas.js | 27 ++++++++ ui/hooks/useGasFeeInputs.js | 61 ++++++++++++++++--- 8 files changed, 144 insertions(+), 47 deletions(-) create mode 100644 ui/helpers/constants/gas.js diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 68306c809..967d54247 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -653,24 +653,36 @@ "editGasHigh": { "message": "High" }, + "editGasLimitOutOfBounds": { + "message": "Gas limit must be greater than 20999 and less than 7920027" + }, "editGasLimitTooltip": { "message": "Gas limit is the maximum units of gas you are willing to use. Units of gas are a multiplier to “Max priority fee” and “Max fee”." }, "editGasLow": { "message": "Low" }, + "editGasMaxFeeHigh": { + "message": "Max fee is higher than necessary" + }, "editGasMaxFeeLow": { "message": "Max fee too low for network conditions" }, "editGasMaxFeeTooltip": { "message": "The max fee is the most you’ll pay (base fee + priority fee)." }, + "editGasMaxPriorityFeeHigh": { + "message": "Max priority fee is higher than necessary. You may pay more than needed." + }, "editGasMaxPriorityFeeLow": { "message": "Max priority fee too low for network conditions" }, "editGasMaxPriorityFeeTooltip": { "message": "Max priority fee (aka “miner tip”) goes directly to miners and incentivizes them to prioritize your transaction. You’ll most often pay your max setting" }, + "editGasMaxPriorityFeeZeroError": { + "message": "Max priority fee must be at least 1 GWEI" + }, "editGasMedium": { "message": "Medium" }, @@ -683,6 +695,9 @@ "editGasTooLow": { "message": "Unknown processing time" }, + "editGasTooLowTooltip": { + "message": "Your max fee is low for current market conditions. We don’t know when (or if) your transaction will be processed." + }, "editGasTotalBannerSubtitle": { "message": "Up to $1 ($2)", "display": "$1 represents a fiat value" diff --git a/ui/components/app/advanced-gas-controls/advanced-gas-controls.component.js b/ui/components/app/advanced-gas-controls/advanced-gas-controls.component.js index b97af9ee8..f9040329d 100644 --- a/ui/components/app/advanced-gas-controls/advanced-gas-controls.component.js +++ b/ui/components/app/advanced-gas-controls/advanced-gas-controls.component.js @@ -13,6 +13,7 @@ import { GAS_ESTIMATE_TYPES, GAS_RECOMMENDATIONS, } from '../../../../shared/constants/gas'; +import { getGasFormErrorText } from '../../../helpers/constants/gas'; const DEFAULT_ESTIMATES_LEVEL = 'medium'; @@ -31,8 +32,7 @@ export default function AdvancedGasControls({ setGasPrice, maxPriorityFeeFiat, maxFeeFiat, - maxPriorityFeeError, - maxFeeError, + gasErrors, }) { const t = useContext(I18nContext); @@ -63,6 +63,11 @@ export default function AdvancedGasControls({
) } - error={maxPriorityFeeError} + error={ + gasErrors?.maxPriorityFee + ? getGasFormErrorText(gasErrors.maxPriorityFee, t) + : null + } /> ) } - error={maxFeeError} + error={ + gasErrors?.maxFee + ? getGasFormErrorText(gasErrors.maxFee, t) + : null + } /> ) : ( @@ -216,6 +229,5 @@ AdvancedGasControls.propTypes = { setGasPrice: PropTypes.func, maxPriorityFeeFiat: PropTypes.string, maxFeeFiat: PropTypes.string, - maxPriorityFeeError: PropTypes.string, - maxFeeError: PropTypes.string, + gasErrors: PropTypes.object, }; diff --git a/ui/components/app/advanced-gas-controls/index.scss b/ui/components/app/advanced-gas-controls/index.scss index 64888d83f..7fdf9174f 100644 --- a/ui/components/app/advanced-gas-controls/index.scss +++ b/ui/components/app/advanced-gas-controls/index.scss @@ -16,10 +16,12 @@ align-self: center; } - &__row-error, - &__row--error h6 { - color: $error-1 !important; - padding-top: 6px; + .form-field__row--error .form-field__heading-title h6 { + color: $error-1; + + & path { + fill: $error-1; + } } h6 { @@ -27,8 +29,7 @@ margin-inline-end: 6px; } - i { - color: #dadada; - font-size: $font-size-h7; + path { + fill: #dadada; } } diff --git a/ui/components/app/edit-gas-display/edit-gas-display.component.js b/ui/components/app/edit-gas-display/edit-gas-display.component.js index 4e41d565c..65fe594c3 100644 --- a/ui/components/app/edit-gas-display/edit-gas-display.component.js +++ b/ui/components/app/edit-gas-display/edit-gas-display.component.js @@ -48,14 +48,13 @@ export default function EditGasDisplay({ setEstimateToUse, estimatedMinimumFiat, estimatedMaximumFiat, - isMaxFeeError, - isMaxPriorityFeeError, - isGasTooLow, + hasGasErrors, dappSuggestedGasFeeAcknowledged, setDappSuggestedGasFeeAcknowledged, showAdvancedForm, setShowAdvancedForm, warning, + gasErrors, }) { const t = useContext(I18nContext); @@ -126,14 +125,19 @@ export default function EditGasDisplay({ {t('gasDisplayAcknowledgeDappButtonText')} )} - {isGasTooLow && ( + {hasGasErrors && (
- {t('editGasTooLow')} + {t('editGasTooLow')}{' '} +
)} @@ -194,10 +198,7 @@ export default function EditGasDisplay({ setGasPrice={setGasPrice} maxPriorityFeeFiat={maxPriorityFeePerGasFiat} maxFeeFiat={maxFeePerGasFiat} - maxPriorityFeeError={ - isMaxPriorityFeeError ? t('editGasMaxPriorityFeeLow') : null - } - maxFeeError={isMaxFeeError ? t('editGasMaxFeeLow') : null} + gasErrors={gasErrors} /> )}
@@ -236,13 +237,12 @@ EditGasDisplay.propTypes = { setEstimateToUse: PropTypes.func, estimatedMinimumFiat: PropTypes.string, estimatedMaximumFiat: PropTypes.string, - isMaxFeeError: PropTypes.boolean, - isMaxPriorityFeeError: PropTypes.boolean, - isGasTooLow: PropTypes.boolean, + hasGasErrors: PropTypes.boolean, dappSuggestedGasFeeAcknowledged: PropTypes.boolean, setDappSuggestedGasFeeAcknowledged: PropTypes.func, showAdvancedForm: PropTypes.bool, setShowAdvancedForm: PropTypes.func, warning: PropTypes.string, transaction: PropTypes.object, + gasErrors: PropTypes.object, }; diff --git a/ui/components/app/edit-gas-display/index.scss b/ui/components/app/edit-gas-display/index.scss index ae5f96fa0..e3ccea500 100644 --- a/ui/components/app/edit-gas-display/index.scss +++ b/ui/components/app/edit-gas-display/index.scss @@ -21,6 +21,14 @@ } } + &__error .info-tooltip { + display: inline-block; + + path { + fill: $error-1; + } + } + &__dapp-acknowledgement-warning { margin-bottom: 20px; } 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 5f48b16d0..67cff9e6e 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 @@ -72,9 +72,8 @@ export default function EditGasPopover({ setEstimateToUse, estimatedMinimumFiat, estimatedMaximumFiat, - isMaxFeeError, - isMaxPriorityFeeError, - isGasTooLow, + hasGasErrors, + gasErrors, } = useGasFeeInputs(defaultEstimateToUse); /** @@ -173,12 +172,7 @@ export default function EditGasPopover({ @@ -217,12 +211,11 @@ export default function EditGasPopover({ setEstimateToUse={setEstimateToUse} estimatedMinimumFiat={estimatedMinimumFiat} estimatedMaximumFiat={estimatedMaximumFiat} - isMaxFeeError={isMaxFeeError} - isMaxPriorityFeeError={isMaxPriorityFeeError} - isGasTooLow={isGasTooLow} + hasGasErrors={hasGasErrors} onEducationClick={() => setShowEducationContent(true)} mode={mode} transaction={transaction} + gasErrors={gasErrors} {...editGasDisplayProps} /> )} diff --git a/ui/helpers/constants/gas.js b/ui/helpers/constants/gas.js new file mode 100644 index 000000000..d9bb1fbd5 --- /dev/null +++ b/ui/helpers/constants/gas.js @@ -0,0 +1,27 @@ +export const GAS_FORM_ERRORS = { + GAS_LIMIT_OUT_OF_BOUNDS: 'editGasLimitOutOfBounds', + MAX_PRIORITY_FEE_TOO_LOW: 'editGasMaxPriorityFeeLow', + MAX_FEE_TOO_LOW: 'editGasMaxFeeLow', + MAX_PRIORITY_FEE_ZERO: 'editGasMaxPriorityFeeZeroError', + MAX_PRIORITY_FEE_HIGH_WARNING: 'editGasMaxPriorityFeeHigh', + MAX_FEE_HIGH_WARNING: 'editGasMaxFeeHigh', +}; + +export function getGasFormErrorText(type, t) { + switch (type) { + case GAS_FORM_ERRORS.GAS_LIMIT_OUT_OF_BOUNDS: + return t('editGasLimitOutOfBounds'); + case GAS_FORM_ERRORS.MAX_PRIORITY_FEE_TOO_LOW: + return t('editGasMaxPriorityFeeLow'); + case GAS_FORM_ERRORS.MAX_FEE_TOO_LOW: + return t('editGasMaxFeeLow'); + case GAS_FORM_ERRORS.MAX_PRIORITY_FEE_ZERO: + return t('editGasMaxPriorityFeeZeroError'); + case GAS_FORM_ERRORS.MAX_PRIORITY_FEE_HIGH_WARNING: + return t('editGasMaxPriorityFeeHigh'); + case GAS_FORM_ERRORS.MAX_FEE_HIGH_WARNING: + return t('editGasMaxFeeHigh'); + default: + return ''; + } +} diff --git a/ui/hooks/useGasFeeInputs.js b/ui/hooks/useGasFeeInputs.js index 5b6ac4df5..c04f2b416 100644 --- a/ui/hooks/useGasFeeInputs.js +++ b/ui/hooks/useGasFeeInputs.js @@ -13,10 +13,13 @@ import { decimalToHex, } from '../helpers/utils/conversions.util'; import { getShouldShowFiat } from '../selectors'; +import { GAS_FORM_ERRORS } from '../helpers/constants/gas'; import { useCurrencyDisplay } from './useCurrencyDisplay'; import { useGasFeeEstimates } from './useGasFeeEstimates'; import { useUserPreferencedCurrency } from './useUserPreferencedCurrency'; +const HIGH_FEE_WARNING_MULTIPLIER = 1.5; + /** * Opaque string type representing a decimal (base 10) number in GWEI * @typedef {`${number}`} DecGweiString @@ -271,39 +274,75 @@ export function useGasFeeInputs(defaultEstimateToUse = 'medium') { }, ); - let isMaxPriorityFeeError = false; - let isMaxFeeError = false; + // Separating errors from warnings so we can know which value problems + // are blocking or simply useful information for the users + const gasErrors = {}; + const gasWarnings = {}; + + if (gasLimit < 21000 || gasLimit > 7920027) { + gasErrors.gasLimit = GAS_FORM_ERRORS.GAS_LIMIT_OUT_OF_BOUNDS; + } switch (gasEstimateType) { case GAS_ESTIMATE_TYPES.FEE_MARKET: - isMaxPriorityFeeError = + if (maxPriorityFeePerGasToUse < 1) { + gasErrors.maxPriorityFee = GAS_FORM_ERRORS.MAX_PRIORITY_FEE_ZERO; + } else if ( !isGasEstimatesLoading && maxPriorityFeePerGasToUse < - gasFeeEstimates?.low?.suggestedMaxPriorityFeePerGas; - isMaxFeeError = + gasFeeEstimates?.low?.suggestedMaxPriorityFeePerGas + ) { + gasErrors.maxPriorityFee = GAS_FORM_ERRORS.MAX_PRIORITY_FEE_TOO_LOW; + } else if ( + gasFeeEstimates?.high && + maxPriorityFeePerGasToUse > + gasFeeEstimates.high.suggestedMaxPriorityFeePerGas * + HIGH_FEE_WARNING_MULTIPLIER + ) { + gasWarnings.maxPriorityFee = + GAS_FORM_ERRORS.MAX_PRIORITY_FEE_HIGH_WARNING; + } + + if ( !isGasEstimatesLoading && - maxFeePerGasToUse < gasFeeEstimates?.low?.suggestedMaxFeePerGas; + maxFeePerGasToUse < gasFeeEstimates?.low?.suggestedMaxFeePerGas + ) { + gasErrors.maxFee = GAS_FORM_ERRORS.MAX_FEE_TOO_LOW; + } else if ( + gasFeeEstimates?.high && + maxFeePerGasToUse > + gasFeeEstimates.high.suggestedMaxFeePerGas * + HIGH_FEE_WARNING_MULTIPLIER + ) { + gasWarnings.maxFee = GAS_FORM_ERRORS.MAX_FEE_HIGH_WARNING; + } break; default: break; } - const isGasTooLow = Boolean(isMaxPriorityFeeError || isMaxFeeError); + // Determine if we have any errors which should block submission + const hasBlockingGasErrors = Boolean(Object.keys(gasErrors).length); + + // Now that we've determined errors that block submission, we can pool the warnings + // and errors into one object for easier use within the UI. This object should have + // no effect on whether or not the user can submit the form + const errorsAndWarnings = { + ...gasErrors, + ...gasWarnings, + }; return { maxFeePerGas: maxFeePerGasToUse, maxFeePerGasFiat: showFiat ? maxFeePerGasFiat : '', setMaxFeePerGas, - isMaxFeeError, maxPriorityFeePerGas: maxPriorityFeePerGasToUse, maxPriorityFeePerGasFiat: showFiat ? maxPriorityFeePerGasFiat : '', setMaxPriorityFeePerGas, - isMaxPriorityFeeError, gasPrice: gasPriceToUse, setGasPrice, gasLimit, setGasLimit, - isGasTooLow, estimateToUse, setEstimateToUse, estimatedMinimumFiat: showFiat ? estimatedMinimumFiat : '', @@ -313,5 +352,7 @@ export function useGasFeeInputs(defaultEstimateToUse = 'medium') { gasFeeEstimates, gasEstimateType, estimatedGasFeeTimeBounds, + gasErrors: errorsAndWarnings, + hasGasErrors: hasBlockingGasErrors, }; }