Proper calculation of the gas limit (#12784)

app/_locales/en/messages.json

@@ -1210,6 +1210,9 @@
   "gasLimitInfoTooltipContent": {
     "message": "Gas limit is the maximum amount of units of gas you are willing to spend."
   },
+  "gasLimitRecommended": {
+    "message": "Recommended gas limit is $1. If the gas limit is less than that, it may fail."
+  },
   "gasLimitTooLow": {
     "message": "Gas limit must be at least 21000"
   },

app/scripts/controllers/transactions/index.js

@@ -541,6 +541,7 @@ export default class TransactionController extends EventEmitter {
 
     if (defaultGasLimit && !txMeta.txParams.gas) {
       txMeta.txParams.gas = defaultGasLimit;
+      txMeta.originalGasEstimate = defaultGasLimit;
     }
     return txMeta;
   }

app/scripts/controllers/transactions/tx-state-manager.js

@@ -117,6 +117,8 @@ export default class TransactionStateManager extends EventEmitter {
       time: new Date().getTime(),
       status: TRANSACTION_STATUSES.UNAPPROVED,
       metamaskNetworkId: netId,
+      originalGasEstimate: opts.txParams?.gas,
+      userEditedGasLimit: false,
       chainId,
       loadingDefaults: true,
       dappSuggestedGasFees,

shared/constants/transaction.js

@@ -226,6 +226,10 @@ export const TRANSACTION_GROUP_CATEGORIES = {
  *  TransactionMeta object.
  * @property {string} origin - A string representing the interface that
  *  suggested the transaction.
+ * @property {string} originalGasEstimate - A string representing the original
+ * gas estimation on the transaction metadata.
+ * @property {boolean} userEditedGasLimit - A boolean representing when the
+ * user manually edited the gas limit.
  * @property {Object} nonceDetails - A metadata object containing information
  *  used to derive the suggested nonce, useful for debugging nonce issues.
  * @property {string} rawTx - A hex string of the final signed transaction,

ui/components/app/edit-gas-display/edit-gas-display.component.js

@@ -2,6 +2,7 @@ import React, { useContext, useLayoutEffect, useRef, useState } from 'react';
 import { useSelector } from 'react-redux';
 import PropTypes from 'prop-types';
 
+import BigNumber from 'bignumber.js';
 import {
   GAS_RECOMMENDATIONS,
   EDIT_GAS_MODES,
@@ -57,6 +58,7 @@ export default function EditGasDisplay({
   setGasPrice,
   gasLimit,
   setGasLimit,
+  properGasLimit,
   estimateToUse,
   setEstimateToUse,
   estimatedMinimumFiat,
@@ -107,6 +109,11 @@ export default function EditGasDisplay({
       dappSuggestedAndTxParamGasFeesAreTheSame,
   );
 
+  let warningMessage;
+  if (new BigNumber(gasLimit).lessThan(new BigNumber(properGasLimit))) {
+    warningMessage = t('gasLimitRecommended', [properGasLimit]);
+  }
+
   const showTopError =
     (balanceError || estimatesUnavailableWarning) &&
     (!isGasEstimatesLoading || txParamsHaveBeenCustomized);
@@ -138,6 +145,16 @@ export default function EditGasDisplay({
             <ErrorMessage errorKey={errorKey} />
           </div>
         )}
+        {warningMessage && (
+          <div className="edit-gas-display__warning">
+            <ActionableMessage
+              className="actionable-message--warning"
+              message={warningMessage}
+              iconFillColor="#f8c000"
+              useIcon
+            />
+          </div>
+        )}
         {requireDappAcknowledgement && !isGasEstimatesLoading && (
           <div className="edit-gas-display__dapp-acknowledgement-warning">
             <ActionableMessage
@@ -332,6 +349,7 @@ EditGasDisplay.propTypes = {
   setGasPrice: PropTypes.func,
   gasLimit: PropTypes.number,
   setGasLimit: PropTypes.func,
+  properGasLimit: PropTypes.number,
   estimateToUse: PropTypes.string,
   setEstimateToUse: PropTypes.func,
   estimatedMinimumFiat: PropTypes.string,

ui/components/app/edit-gas-popover/edit-gas-popover.component.js

@@ -96,6 +96,7 @@ export default function EditGasPopover({
     setGasPrice,
     gasLimit,
     setGasLimit,
+    properGasLimit,
     estimateToUse,
     setEstimateToUse,
     estimatedMinimumFiat,
@@ -162,6 +163,7 @@ export default function EditGasPopover({
 
     const updatedTxMeta = {
       ...updatedTransaction,
+      userEditedGasLimit: gasLimit !== Number(transaction.originalGasEstimate),
       userFeeLevel: estimateToUse || CUSTOM_GAS_ESTIMATE,
       txParams: {
         ...cleanTransactionParams,
@@ -208,6 +210,7 @@ export default function EditGasPopover({
     closePopover,
     gasLimit,
     gasPrice,
+    transaction.originalGasEstimate,
     maxFeePerGas,
     maxPriorityFeePerGas,
     supportsEIP1559,
@@ -281,6 +284,7 @@ export default function EditGasPopover({
               setGasPrice={setGasPrice}
               gasLimit={gasLimit}
               setGasLimit={setGasLimit}
+              properGasLimit={properGasLimit}
               estimateToUse={estimateToUse}
               setEstimateToUse={setEstimateToUse}
               estimatedMinimumFiat={estimatedMinimumFiat}

ui/ducks/send/send.js

@@ -369,9 +369,15 @@ export const computeEstimatedGasLimit = createAsyncThunk(
   async (_, thunkApi) => {
     const state = thunkApi.getState();
     const { send, metamask } = state;
+    const unapprovedTxs = getUnapprovedTxs(state);
+    const transaction = unapprovedTxs[send.draftTransaction.id];
     const isNonStandardEthChain = getIsNonStandardEthChain(state);
     const chainId = getCurrentChainId(state);
-    if (send.stage !== SEND_STAGES.EDIT) {
+    if (
+      send.stage !== SEND_STAGES.EDIT ||
+      !transaction.dappSuggestedGasFees?.gas ||
+      !transaction.userEditedGasLimit
+    ) {
       const gasLimit = await estimateGasLimitForSend({
         gasPrice: send.gas.gasPrice,
         blockGasLimit: metamask.currentBlockGasLimit,
@@ -1662,15 +1668,18 @@ export function signTransaction() {
       // merge in the modified txParams. Once the transaction has been modified
       // we can send that to the background to update the transaction in state.
       const unapprovedTxs = getUnapprovedTxs(state);
+      const unapprovedTx = unapprovedTxs[id];
       // We only update the tx params that can be changed via the edit flow UX
       const eip1559OnlyTxParamsToUpdate = {
         data: txParams.data,
         from: txParams.from,
         to: txParams.to,
         value: txParams.value,
-        gas: txParams.gas,
+        gas: unapprovedTx.userEditedGasLimit
+          ? unapprovedTx.txParams.gas
+          : txParams.gas,
       };
-      const unapprovedTx = unapprovedTxs[id];
+      unapprovedTx.originalGasEstimate = eip1559OnlyTxParamsToUpdate.gas;
       const editingTx = {
         ...unapprovedTx,
         txParams: Object.assign(

ui/ducks/send/send.test.js

@@ -1303,10 +1303,7 @@ describe('Send Slice', () => {
           'send/computeEstimatedGasLimit/pending',
         );
         expect(actionResult[2].type).toStrictEqual(
-          'metamask/gas/SET_CUSTOM_GAS_LIMIT',
-        );
-        expect(actionResult[3].type).toStrictEqual(
-          'send/computeEstimatedGasLimit/fulfilled',
+          'send/computeEstimatedGasLimit/rejected',
         );
       });
 
@@ -1355,10 +1352,7 @@ describe('Send Slice', () => {
           'send/computeEstimatedGasLimit/pending',
         );
         expect(actionResult[2].type).toStrictEqual(
-          'metamask/gas/SET_CUSTOM_GAS_LIMIT',
-        );
-        expect(actionResult[3].type).toStrictEqual(
-          'send/computeEstimatedGasLimit/fulfilled',
+          'send/computeEstimatedGasLimit/rejected',
         );
       });
 
@@ -1398,16 +1392,13 @@ describe('Send Slice', () => {
 
         const actionResult = store.getActions();
 
-        expect(actionResult).toHaveLength(4);
+        expect(actionResult).toHaveLength(3);
         expect(actionResult[0].type).toStrictEqual('send/updateSendAmount');
         expect(actionResult[1].type).toStrictEqual(
           'send/computeEstimatedGasLimit/pending',
         );
         expect(actionResult[2].type).toStrictEqual(
-          'metamask/gas/SET_CUSTOM_GAS_LIMIT',
-        );
-        expect(actionResult[3].type).toStrictEqual(
-          'send/computeEstimatedGasLimit/fulfilled',
+          'send/computeEstimatedGasLimit/rejected',
         );
       });
     });
@@ -1460,7 +1451,7 @@ describe('Send Slice', () => {
 
         const actionResult = store.getActions();
 
-        expect(actionResult).toHaveLength(4);
+        expect(actionResult).toHaveLength(3);
 
         expect(actionResult[0].type).toStrictEqual('send/updateAsset');
         expect(actionResult[0].payload).toStrictEqual({
@@ -1473,10 +1464,7 @@ describe('Send Slice', () => {
           'send/computeEstimatedGasLimit/pending',
         );
         expect(actionResult[2].type).toStrictEqual(
-          'metamask/gas/SET_CUSTOM_GAS_LIMIT',
-        );
-        expect(actionResult[3].type).toStrictEqual(
-          'send/computeEstimatedGasLimit/fulfilled',
+          'send/computeEstimatedGasLimit/rejected',
         );
       });
 
@@ -1503,7 +1491,7 @@ describe('Send Slice', () => {
 
         const actionResult = store.getActions();
 
-        expect(actionResult).toHaveLength(6);
+        expect(actionResult).toHaveLength(5);
         expect(actionResult[0].type).toStrictEqual('SHOW_LOADING_INDICATION');
         expect(actionResult[1].type).toStrictEqual('HIDE_LOADING_INDICATION');
         expect(actionResult[2].payload).toStrictEqual({
@@ -1516,10 +1504,7 @@ describe('Send Slice', () => {
           'send/computeEstimatedGasLimit/pending',
         );
         expect(actionResult[4].type).toStrictEqual(
-          'metamask/gas/SET_CUSTOM_GAS_LIMIT',
-        );
-        expect(actionResult[5].type).toStrictEqual(
-          'send/computeEstimatedGasLimit/fulfilled',
+          'send/computeEstimatedGasLimit/rejected',
         );
       });
 
@@ -1704,16 +1689,13 @@ describe('Send Slice', () => {
 
         const actionResult = store.getActions();
 
-        expect(actionResult).toHaveLength(4);
+        expect(actionResult).toHaveLength(3);
         expect(actionResult[0].type).toStrictEqual('send/updateRecipient');
         expect(actionResult[1].type).toStrictEqual(
           'send/computeEstimatedGasLimit/pending',
         );
         expect(actionResult[2].type).toStrictEqual(
-          'metamask/gas/SET_CUSTOM_GAS_LIMIT',
-        );
-        expect(actionResult[3].type).toStrictEqual(
-          'send/computeEstimatedGasLimit/fulfilled',
+          'send/computeEstimatedGasLimit/rejected',
         );
       });
 
@@ -1772,7 +1754,7 @@ describe('Send Slice', () => {
         );
 
         const actionResult = store.getActions();
-        expect(actionResult).toHaveLength(4);
+        expect(actionResult).toHaveLength(3);
         expect(actionResult[0].type).toStrictEqual('send/updateRecipient');
         expect(actionResult[0].payload.address).toStrictEqual(
           TEST_RECIPIENT_ADDRESS,
@@ -1822,16 +1804,13 @@ describe('Send Slice', () => {
 
         const actionResult = store.getActions();
 
-        expect(actionResult).toHaveLength(4);
+        expect(actionResult).toHaveLength(3);
         expect(actionResult[0].type).toStrictEqual('send/updateRecipient');
         expect(actionResult[1].type).toStrictEqual(
           'send/computeEstimatedGasLimit/pending',
         );
         expect(actionResult[2].type).toStrictEqual(
-          'metamask/gas/SET_CUSTOM_GAS_LIMIT',
-        );
-        expect(actionResult[3].type).toStrictEqual(
-          'send/computeEstimatedGasLimit/fulfilled',
+          'send/computeEstimatedGasLimit/rejected',
         );
       });
     });
@@ -1896,7 +1875,7 @@ describe('Send Slice', () => {
         await store.dispatch(resetRecipientInput());
         const actionResult = store.getActions();
 
-        expect(actionResult).toHaveLength(7);
+        expect(actionResult).toHaveLength(6);
         expect(actionResult[0].type).toStrictEqual(
           'send/updateRecipientUserInput',
         );
@@ -1906,13 +1885,10 @@ describe('Send Slice', () => {
           'send/computeEstimatedGasLimit/pending',
         );
         expect(actionResult[3].type).toStrictEqual(
-          'metamask/gas/SET_CUSTOM_GAS_LIMIT',
+          'send/computeEstimatedGasLimit/rejected',
         );
-        expect(actionResult[4].type).toStrictEqual(
-          'send/computeEstimatedGasLimit/fulfilled',
-        );
-        expect(actionResult[5].type).toStrictEqual('ENS/resetEnsResolution');
-        expect(actionResult[6].type).toStrictEqual(
+        expect(actionResult[4].type).toStrictEqual('ENS/resetEnsResolution');
+        expect(actionResult[5].type).toStrictEqual(
           'send/validateRecipientUserInput',
         );
       });
@@ -1979,17 +1955,14 @@ describe('Send Slice', () => {
 
         const actionResult = store.getActions();
 
-        expect(actionResult).toHaveLength(5);
+        expect(actionResult).toHaveLength(4);
         expect(actionResult[0].type).toStrictEqual('send/updateAmountMode');
         expect(actionResult[1].type).toStrictEqual('send/updateAmountToMax');
         expect(actionResult[2].type).toStrictEqual(
           'send/computeEstimatedGasLimit/pending',
         );
         expect(actionResult[3].type).toStrictEqual(
-          'metamask/gas/SET_CUSTOM_GAS_LIMIT',
-        );
-        expect(actionResult[4].type).toStrictEqual(
-          'send/computeEstimatedGasLimit/fulfilled',
+          'send/computeEstimatedGasLimit/rejected',
         );
       });
 
@@ -2026,17 +1999,14 @@ describe('Send Slice', () => {
 
         const actionResult = store.getActions();
 
-        expect(actionResult).toHaveLength(5);
+        expect(actionResult).toHaveLength(4);
         expect(actionResult[0].type).toStrictEqual('send/updateAmountMode');
         expect(actionResult[1].type).toStrictEqual('send/updateSendAmount');
         expect(actionResult[2].type).toStrictEqual(
           'send/computeEstimatedGasLimit/pending',
         );
         expect(actionResult[3].type).toStrictEqual(
-          'metamask/gas/SET_CUSTOM_GAS_LIMIT',
-        );
-        expect(actionResult[4].type).toStrictEqual(
-          'send/computeEstimatedGasLimit/fulfilled',
+          'send/computeEstimatedGasLimit/rejected',
         );
       });
     });

ui/hooks/gasFeeInput/useGasFeeInputs.js

@@ -140,6 +140,12 @@ export function useGasFeeInputs(
     Number(hexToDecimal(transaction?.txParams?.gas ?? '0x0')),
   );
 
+  const properGasLimit = Number(hexToDecimal(transaction?.originalGasEstimate));
+
+  const [userEditedGasLimit, setUserEditedGasLimit] = useState(() =>
+    Boolean(transaction?.userEditedGasLimit),
+  );
+
   /**
    * In EIP-1559 V2 designs change to gas estimate is always updated to transaction
    * Thus callback setEstimateToUse can be deprecate in favour of this useEffect
@@ -298,6 +304,7 @@ export function useGasFeeInputs(
     // Restore existing values
     setGasPrice(gasPrice);
     setGasLimit(gasLimit);
+    setUserEditedGasLimit(true);
     setMaxFeePerGas(maxFeePerGas);
     setMaxPriorityFeePerGas(maxPriorityFeePerGas);
     setGasPriceHasBeenManuallySet(true);
@@ -309,6 +316,7 @@ export function useGasFeeInputs(
     gasPrice,
     setGasLimit,
     gasLimit,
+    setUserEditedGasLimit,
     setMaxFeePerGas,
     maxFeePerGas,
     setMaxPriorityFeePerGas,
@@ -328,6 +336,8 @@ export function useGasFeeInputs(
     setGasPrice,
     gasLimit,
     setGasLimit,
+    properGasLimit,
+    userEditedGasLimit,
     editGasMode,
     estimateToUse,
     setEstimateToUse,