Fixing EIP-1559 V2 advance gas fee inputs validations (#13013)

app/_locales/en/messages.json

@@ -845,7 +845,7 @@
     "message": "Max priority fee must be greater than 0 GWEI"
   },
   "editGasMaxPriorityFeeBelowMinimumV2": {
-    "message": "Priority fee must be at least 1 GWEI"
+    "message": "Priority fee must be greater than 0."
   },
   "editGasMaxPriorityFeeHigh": {
     "message": "Max priority fee is higher than necessary. You may pay more than needed."

ui/components/app/advanced-gas-fee-popover/advanced-gas-fee-gas-limit/advanced-gas-fee-gas-limit.js

@@ -22,9 +22,7 @@ const validateGasLimit = (gasLimit, minimumGasLimitDec) => {
 const AdvancedGasFeeGasLimit = () => {
   const t = useI18nContext();
   const {
-    setDirty,
     setGasLimit: setGasLimitInContext,
-    setHasError,
   } = useAdvancedGasFeePopoverContext();
   const {
     gasLimit: gasLimitInTransaction,
@@ -36,15 +34,13 @@ const AdvancedGasFeeGasLimit = () => {
 
   const updateGasLimit = (value) => {
     setGasLimit(value);
-    setDirty(true);
   };
 
   useEffect(() => {
     setGasLimitInContext(gasLimit);
     const error = validateGasLimit(gasLimit, minimumGasLimitDec);
     setGasLimitError(error);
-    setHasError(Boolean(error));
-  }, [gasLimit, minimumGasLimitDec, setGasLimitInContext, setHasError]);
+  }, [gasLimit, minimumGasLimitDec, setGasLimitInContext]);
 
   if (isEditing) {
     return (

ui/components/app/advanced-gas-fee-popover/advanced-gas-fee-inputs/base-fee-input/base-fee-input.js

@@ -67,10 +67,9 @@ const BaseFeeInput = () => {
   const t = useI18nContext();
   const { gasFeeEstimates, estimateUsed, maxFeePerGas } = useGasFeeContext();
   const {
-    setDirty,
-    setHasError,
-    setMaxFeePerGas,
     maxPriorityFeePerGas,
+    setErrorValue,
+    setMaxFeePerGas,
   } = useAdvancedGasFeePopoverContext();
 
   const { estimatedBaseFee } = gasFeeEstimates;
@@ -133,7 +132,6 @@ const BaseFeeInput = () => {
       }
       setMaxBaseFeeGWEI(baseFeeInGWEI);
       setMaxBaseFeeMultiplier(baseFeeMultiplierValue);
-      setDirty(true);
     },
     [
       editingInGwei,
@@ -141,7 +139,6 @@ const BaseFeeInput = () => {
       numberOfDecimalsPrimary,
       setMaxBaseFeeGWEI,
       setMaxBaseFeeMultiplier,
-      setDirty,
     ],
   );
 
@@ -152,14 +149,15 @@ const BaseFeeInput = () => {
       gasFeeEstimates,
       maxPriorityFeePerGas,
     );
+
     setBaseFeeError(error);
-    setHasError(Boolean(error));
+    setErrorValue('maxFeePerGas', error === 'editGasMaxBaseFeeImbalance');
   }, [
     gasFeeEstimates,
     maxBaseFeeGWEI,
     maxPriorityFeePerGas,
-    setHasError,
     setBaseFeeError,
+    setErrorValue,
     setMaxFeePerGas,
   ]);
 

ui/components/app/advanced-gas-fee-popover/advanced-gas-fee-inputs/priority-fee-input/priority-fee-input.js

@@ -17,7 +17,7 @@ import { useAdvancedGasFeePopoverContext } from '../../context';
 import AdvancedGasFeeInputSubtext from '../../advanced-gas-fee-input-subtext';
 
 const validatePriorityFee = (value, gasFeeEstimates) => {
-  if (value < 1) {
+  if (value <= 0) {
     return 'editGasMaxPriorityFeeBelowMinimumV2';
   }
   if (
@@ -43,8 +43,7 @@ const PriorityFeeInput = () => {
   const t = useI18nContext();
   const advancedGasFeeValues = useSelector(getAdvancedGasFeeValues);
   const {
-    setDirty,
-    setHasError,
+    setErrorValue,
     setMaxPriorityFeePerGas,
   } = useAdvancedGasFeePopoverContext();
   const {
@@ -72,18 +71,20 @@ const PriorityFeeInput = () => {
 
   const updatePriorityFee = (value) => {
     setPriorityFee(value);
-    setDirty(true);
   };
 
   useEffect(() => {
     setMaxPriorityFeePerGas(priorityFee);
     const error = validatePriorityFee(priorityFee, gasFeeEstimates);
+    setErrorValue(
+      'maxPriorityFeePerGas',
+      error === 'editGasMaxPriorityFeeBelowMinimumV2',
+    );
     setPriorityFeeError(error);
-    setHasError(Boolean(error));
   }, [
     gasFeeEstimates,
     priorityFee,
-    setHasError,
+    setErrorValue,
     setMaxPriorityFeePerGas,
     setPriorityFeeError,
   ]);

ui/components/app/advanced-gas-fee-popover/advanced-gas-fee-inputs/priority-fee-input/priority-fee-input.test.js

@@ -1,4 +1,5 @@
 import React from 'react';
+import { fireEvent, screen } from '@testing-library/react';
 
 import { GAS_ESTIMATE_TYPES } from '../../../../../../shared/constants/gas';
 import { renderWithProvider } from '../../../../../../test/lib/render-helpers';
@@ -67,4 +68,21 @@ describe('PriorityfeeInput', () => {
     });
     expect(document.getElementsByTagName('input')[0]).toHaveValue(2);
   });
+
+  it('should show error if value entered is 0', () => {
+    render({
+      txParams: {
+        maxPriorityFeePerGas: '0x174876E800',
+      },
+    });
+    expect(
+      screen.queryByText('Priority fee must be greater than 0.'),
+    ).not.toBeInTheDocument();
+    fireEvent.change(document.getElementsByTagName('input')[0], {
+      target: { value: 0 },
+    });
+    expect(
+      screen.queryByText('Priority fee must be greater than 0.'),
+    ).toBeInTheDocument();
+  });
 });

ui/components/app/advanced-gas-fee-popover/advanced-gas-fee-popover.test.js

@@ -36,7 +36,6 @@ const render = () => {
           balance: '0x1F4',
         },
       },
-      advancedGasFee: { priorityFee: 100 },
       featureFlags: { advancedInlineGas: true },
       gasFeeEstimates:
         mockEstimates[GAS_ESTIMATE_TYPES.FEE_MARKET].gasFeeEstimates,
@@ -46,7 +45,7 @@ const render = () => {
   return renderWithProvider(
     <GasFeeContextProvider
       transaction={{
-        userFeeLevel: 'custom',
+        userFeeLevel: 'high',
       }}
     >
       <AdvancedGasFeePopover />
@@ -56,16 +55,24 @@ const render = () => {
 };
 
 describe('AdvancedGasFeePopover', () => {
-  it('should renders save button disabled by default', () => {
+  it('should renders save button enabled by default', () => {
     render();
+    expect(screen.queryByRole('button', { name: 'Save' })).not.toBeDisabled();
+  });
+
+  it('should disable save button if priority fee 0 is entered', () => {
+    render();
+    fireEvent.change(document.getElementsByTagName('input')[1], {
+      target: { value: 0 },
+    });
     expect(screen.queryByRole('button', { name: 'Save' })).toBeDisabled();
   });
 
-  it('should enable save button as input value is changed', () => {
+  it('should disable save button if priority fee entered is greater than base fee', () => {
     render();
-    fireEvent.change(document.getElementsByTagName('input')[0], {
-      target: { value: 3 },
+    fireEvent.change(document.getElementsByTagName('input')[1], {
+      target: { value: 100000 },
     });
-    expect(screen.queryByRole('button', { name: 'Save' })).not.toBeDisabled();
+    expect(screen.queryByRole('button', { name: 'Save' })).toBeDisabled();
   });
 });

ui/components/app/advanced-gas-fee-popover/advanced-gas-fee-save/advanced-gas-fee-save.js

@@ -13,9 +13,8 @@ const AdvancedGasFeeSaveButton = () => {
   const { closeModal } = useTransactionModalContext();
   const { updateTransaction } = useGasFeeContext();
   const {
-    isDirty,
     gasLimit,
-    hasError,
+    hasErrors,
     maxFeePerGas,
     maxPriorityFeePerGas,
   } = useAdvancedGasFeePopoverContext();
@@ -31,7 +30,7 @@ const AdvancedGasFeeSaveButton = () => {
   };
 
   return (
-    <Button type="primary" disabled={!isDirty || hasError} onClick={onSave}>
+    <Button type="primary" disabled={hasErrors} onClick={onSave}>
       <I18nValue messageKey="save" />
     </Button>
   );

ui/components/app/advanced-gas-fee-popover/context/advancedGasFeePopover.js

@@ -1,4 +1,4 @@
-import React, { createContext, useContext, useState } from 'react';
+import React, { createContext, useCallback, useContext, useState } from 'react';
 import PropTypes from 'prop-types';
 
 export const AdvancedGasFeePopoverContext = createContext({});
@@ -7,20 +7,29 @@ export const AdvancedGasFeePopoverContextProvider = ({ children }) => {
   const [gasLimit, setGasLimit] = useState();
   const [maxFeePerGas, setMaxFeePerGas] = useState();
   const [maxPriorityFeePerGas, setMaxPriorityFeePerGas] = useState();
-  const [isDirty, setDirty] = useState();
-  const [hasError, setHasError] = useState(false);
+  const [errors, setErrors] = useState({
+    maxFeePerGas: false,
+    maxPriorityFeePerGas: false,
+  });
+
+  const setErrorValue = useCallback(
+    (field, value) => {
+      if (errors[field] !== value) {
+        setErrors({ ...errors, [field]: value });
+      }
+    },
+    [errors, setErrors],
+  );
 
   return (
     <AdvancedGasFeePopoverContext.Provider
       value={{
         gasLimit,
-        hasError,
-        isDirty,
+        hasErrors: errors.maxFeePerGas || errors.maxPriorityFeePerGas,
         maxFeePerGas,
         maxPriorityFeePerGas,
-        setDirty,
+        setErrorValue,
         setGasLimit,
-        setHasError,
         setMaxPriorityFeePerGas,
         setMaxFeePerGas,
       }}