Fix speed-up/cancel: don't update existing transaction data (#14355)

* Fix speed-up/cancel: don't update existing transaction data

* Move retryTxMeta state management to useGasFeeInputs.js

* Handle initial retryTxMeta set if no transaction is passed to useGasFeeInputs

* Ensure previousGas is use on retry transaction if it is available in useGasFeeInputs

* Remove update transaction mock and correctly test gas fee increase scenarios now that updateTransaction used in cancel-speedup is defined on the front end

ui/components/app/cancel-speedup-popover/cancel-speedup-popover.test.js

@@ -1,5 +1,6 @@
 import React from 'react';
 import { act, screen } from '@testing-library/react';
+import BigNumber from 'bignumber.js';
 
 import {
   EDIT_GAS_MODES,
@@ -10,9 +11,47 @@ import mockEstimates from '../../../../test/data/mock-estimates.json';
 import mockState from '../../../../test/data/mock-state.json';
 import { GasFeeContextProvider } from '../../../contexts/gasFee';
 import configureStore from '../../../store/store';
+import {
+  hexWEIToDecETH,
+  decGWEIToHexWEI,
+} from '../../../helpers/utils/conversions.util';
 
 import CancelSpeedupPopover from './cancel-speedup-popover';
 
+const MAXFEEPERGAS_ABOVE_MOCK_MEDIUM_HEX = '0x174876e800';
+const MAXGASCOST_ABOVE_MOCK_MEDIUM_BN = new BigNumber(
+  MAXFEEPERGAS_ABOVE_MOCK_MEDIUM_HEX,
+  16,
+).times(21000, 10);
+const MAXGASCOST_ABOVE_MOCK_MEDIUM_BN_PLUS_TEN_PCT_HEX = MAXGASCOST_ABOVE_MOCK_MEDIUM_BN.times(
+  1.1,
+  10,
+).toString(16);
+
+const EXPECTED_ETH_FEE_1 = hexWEIToDecETH(
+  MAXGASCOST_ABOVE_MOCK_MEDIUM_BN_PLUS_TEN_PCT_HEX,
+);
+
+const MOCK_SUGGESTED_MEDIUM_MAXFEEPERGAS_DEC_GWEI =
+  mockEstimates[GAS_ESTIMATE_TYPES.FEE_MARKET].gasFeeEstimates.medium
+    .suggestedMaxFeePerGas;
+const MOCK_SUGGESTED_MEDIUM_MAXFEEPERGAS_BN_WEI = new BigNumber(
+  decGWEIToHexWEI(MOCK_SUGGESTED_MEDIUM_MAXFEEPERGAS_DEC_GWEI),
+  16,
+);
+const MAXFEEPERGAS_BELOW_MOCK_MEDIUM_HEX = MOCK_SUGGESTED_MEDIUM_MAXFEEPERGAS_BN_WEI.div(
+  10,
+  10,
+).toString(16);
+
+const EXPECTED_ETH_FEE_2 = hexWEIToDecETH(
+  MOCK_SUGGESTED_MEDIUM_MAXFEEPERGAS_BN_WEI.times(21000, 10).toString(16),
+);
+
+const MOCK_SUGGESTED_MEDIUM_MAXFEEPERGAS_HEX_WEI = MOCK_SUGGESTED_MEDIUM_MAXFEEPERGAS_BN_WEI.toString(
+  16,
+);
+
 jest.mock('../../../store/actions', () => ({
   disconnectGasFeeEstimatePoller: jest.fn(),
   getGasFeeTimeEstimate: jest.fn().mockImplementation(() => Promise.resolve()),
@@ -21,7 +60,6 @@ jest.mock('../../../store/actions', () => ({
     .mockImplementation(() => Promise.resolve()),
   addPollingTokenToAppState: jest.fn(),
   removePollingTokenFromAppState: jest.fn(),
-  updateTransaction: () => ({ type: 'UPDATE_TRANSACTION_PARAMS' }),
   updateTransactionGasFees: () => ({ type: 'UPDATE_TRANSACTION_PARAMS' }),
   updatePreviousGasParams: () => ({ type: 'UPDATE_TRANSACTION_PARAMS' }),
   createTransactionEventFragment: jest.fn(),
@@ -34,7 +72,10 @@ jest.mock('../../../contexts/transaction-modal', () => ({
   }),
 }));
 
-const render = (props) => {
+const render = (
+  props,
+  maxFeePerGas = MOCK_SUGGESTED_MEDIUM_MAXFEEPERGAS_HEX_WEI,
+) => {
   const store = configureStore({
     metamask: {
       ...mockState.metamask,
@@ -56,7 +97,7 @@ const render = (props) => {
         userFeeLevel: 'tenPercentIncreased',
         txParams: {
           gas: '0x5208',
-          maxFeePerGas: '0x59682f10',
+          maxFeePerGas,
           maxPriorityFeePerGas: '0x59682f00',
         },
       }}
@@ -80,12 +121,32 @@ describe('CancelSpeedupPopover', () => {
     expect(screen.queryByText('🚀Speed Up')).toBeInTheDocument();
   });
 
-  it('should show correct gas values', async () => {
+  it('should show correct gas values, increased by 10%, when initial initial gas value is above estimated medium', async () => {
     await act(async () =>
-      render({
+      render(
+        {
           editGasMode: EDIT_GAS_MODES.SPEED_UP,
-      }),
+        },
+        MAXFEEPERGAS_ABOVE_MOCK_MEDIUM_HEX,
+      ),
     );
-    expect(screen.queryAllByTitle('0.0000315 ETH').length).toBeGreaterThan(0);
+    expect(
+      screen.queryAllByTitle(`${EXPECTED_ETH_FEE_1} ETH`).length,
+    ).toBeGreaterThan(0);
+  });
+
+  it('should show correct gas values, set to the estimated medium, when initial initial gas value is below estimated medium', async () => {
+    await act(async () =>
+      render(
+        {
+          editGasMode: EDIT_GAS_MODES.SPEED_UP,
+        },
+        `0x${MAXFEEPERGAS_BELOW_MOCK_MEDIUM_HEX}`,
+      ),
+    );
+
+    expect(
+      screen.queryAllByTitle(`${EXPECTED_ETH_FEE_2} ETH`).length,
+    ).toBeGreaterThan(0);
   });
 });

ui/helpers/utils/gas.js

@@ -1,7 +1,10 @@
 import { constant, times, uniq, zip } from 'lodash';
 import BigNumber from 'bignumber.js';
 import { addHexPrefix } from 'ethereumjs-util';
-import { GAS_RECOMMENDATIONS } from '../../../shared/constants/gas';
+import {
+  GAS_RECOMMENDATIONS,
+  EDIT_GAS_MODES,
+} from '../../../shared/constants/gas';
 import { multiplyCurrencies } from '../../../shared/modules/conversion.utils';
 import {
   bnGreaterThan,
@@ -106,3 +109,16 @@ export function formatGasFeeOrFeeRange(
 
   return `${formattedRange} GWEI`;
 }
+
+/**
+ * Helper method for determining whether an edit gas mode is either a speed up or cancel transaction
+ *
+ * @param {string | undefined} editGasMode - One of 'speed-up', 'cancel', 'modify-in-place', or 'swaps'
+ * @returns boolean
+ */
+export function editGasModeIsSpeedUpOrCancel(editGasMode) {
+  return (
+    editGasMode === EDIT_GAS_MODES.CANCEL ||
+    editGasMode === EDIT_GAS_MODES.SPEED_UP
+  );
+}

ui/hooks/gasFeeInput/useGasFeeInputs.js

@@ -17,6 +17,7 @@ import { hexToDecimal } from '../../helpers/utils/conversions.util';
 import { isLegacyTransaction } from '../../helpers/utils/transactions.util';
 import { useGasFeeEstimates } from '../useGasFeeEstimates';
 
+import { editGasModeIsSpeedUpOrCancel } from '../../helpers/utils/gas';
 import { useGasFeeErrors } from './useGasFeeErrors';
 import { useGasPriceInput } from './useGasPriceInput';
 import { useMaxFeePerGasInput } from './useMaxFeePerGasInput';
@@ -81,7 +82,7 @@ import { useTransactionFunctions } from './useTransactionFunctions';
  *
  * @param {EstimateLevel} [defaultEstimateToUse] - which estimate
  *  level to default the 'estimateToUse' state variable to.
- * @param {object} [transaction]
+ * @param {object} [_transaction]
  * @param {string} [minimumGasLimit]
  * @param {EDIT_GAS_MODES[keyof EDIT_GAS_MODES]} editGasMode
  * @returns {GasFeeInputReturnType & import(
@@ -90,10 +91,28 @@ import { useTransactionFunctions } from './useTransactionFunctions';
  */
 export function useGasFeeInputs(
   defaultEstimateToUse = GAS_RECOMMENDATIONS.MEDIUM,
-  transaction,
+  _transaction,
   minimumGasLimit = '0x5208',
   editGasMode = EDIT_GAS_MODES.MODIFY_IN_PLACE,
 ) {
+  const initialRetryTxMeta = {
+    txParams: _transaction?.txParams,
+    id: _transaction?.id,
+    userFeeLevel: _transaction?.userFeeLevel,
+    originalGasEstimate: _transaction?.originalGasEstimate,
+    userEditedGasLimit: _transaction?.userEditedGasLimit,
+  };
+
+  if (_transaction?.previousGas) {
+    initialRetryTxMeta.previousGas = _transaction?.previousGas;
+  }
+
+  const [retryTxMeta, setRetryTxMeta] = useState(initialRetryTxMeta);
+
+  const transaction = editGasModeIsSpeedUpOrCancel(editGasMode)
+    ? retryTxMeta
+    : _transaction;
+
   const eip1559V2Enabled = useSelector(getEIP1559V2Enabled);
 
   const supportsEIP1559 =
@@ -272,6 +291,7 @@ export function useGasFeeInputs(
     maxPriorityFeePerGas,
     minimumGasLimit,
     transaction,
+    setRetryTxMeta,
   });
 
   // When a user selects an estimate level, it will wipe out what they have

ui/hooks/gasFeeInput/useTransactionFunctions.js

@@ -6,7 +6,10 @@ import {
   decimalToHex,
   decGWEIToHexWEI,
 } from '../../helpers/utils/conversions.util';
-import { addTenPercentAndRound } from '../../helpers/utils/gas';
+import {
+  addTenPercentAndRound,
+  editGasModeIsSpeedUpOrCancel,
+} from '../../helpers/utils/gas';
 import {
   createCancelTransaction,
   createSpeedUpTransaction,
@@ -24,6 +27,7 @@ export const useTransactionFunctions = ({
   gasLimit: gasLimitValue,
   maxPriorityFeePerGas: maxPriorityFeePerGasValue,
   transaction,
+  setRetryTxMeta,
 }) => {
   const dispatch = useDispatch();
 
@@ -87,6 +91,8 @@ export const useTransactionFunctions = ({
           updateSwapsUserFeeLevel(estimateUsed || PRIORITY_LEVELS.CUSTOM),
         );
         dispatch(updateCustomSwapsEIP1559GasParams(newGasSettings));
+      } else if (editGasModeIsSpeedUpOrCancel(editGasMode)) {
+        setRetryTxMeta(updatedTxMeta);
       } else {
         newGasSettings.userEditedGasLimit = updatedTxMeta.userEditedGasLimit;
         newGasSettings.userFeeLevel = updatedTxMeta.userFeeLevel;
@@ -110,6 +116,7 @@ export const useTransactionFunctions = ({
       getTxMeta,
       maxPriorityFeePerGasValue,
       transaction,
+      setRetryTxMeta,
     ],
   );