MetaMask incorrectly estimates 100,000 gas for ERC20 transfer. Why?

Question

MetaMask is incorrectly limiting the gas limit estimate to 100,000. This is actually a rather complex "transfer" that will need about 145,000.

Why does MetaMask limit ERC20 .transfer() to 100,000 as appears to be the case? We get the expected behaviour if we manually adjust that, but that extra step is too much to expect of end users.

https://etherscan.io/tx/0x31548bf3b8180d02f6884b9c5a9d7170173237fefc3b72c170928da66e8a14b6#statechange

Answer 1

Since you are interested in knowing why does Metamask estimate wrong the gas limit, I'll share with you some information that is good to know.

Metamask makes use of the function eth_estimateGas to estimate the maximum amount of gas used by that piece of code. However, when performing some specific type of operations with mappings and arrays such as cleaning the data stored inside or just deleting the entries, you get paid back some gas. Why is this a 'problem'? Well, Metamask uses eth_estimateGas and simulates an execution of the function. Since the function is clearing space that was previously on-chain, it gets rewarded with some gas back.

E.g. Your function consumes 125.000 gas, but you clear some space or delete some entries from your mappings and get 25.000 back. Those 25.000 back are given to you AFTER executing your function, but the 125.000 are still necessary to execute the function because you have to pay the computational power. On the other hand, the simulation made by Metamask returns 100.000 because it computes GAS USED - GAS RECOVERED.

There are also another variables that may alter the Gas Limit. Those are the ones that change from one execution to the other (simulation and real transaction) such as timestamp, hash and blockNumber, but in my opinion, you are probably experiencing the issue involving mappings and arrays rather than the variables.

Answer 2

As a disclaimer Rob and I work together so I have an unfair advantage here but thought it would be helpful to give a very detailed breakdown of what is going on...

TLDR;

Likely an unhandled error in metamask gas estimation (uses  eth_estimateGas) - you'll need to log metamask js console to see if this is true. See the following link about getting error messages from eth_estimateGas.

Analysis

The GAS_LIMITS.BASE_TOKEN_ESTIMATE is hardcoded at 100000 here:

• https://github.com/MetaMask/metamask-extension/blob/e02f5972be33c2ff56827c25dc2d0e55cae0f695/shared/constants/gas.ts#L12

This is then used by the send function and as per comment uses BASE_TOKEN_ESTIMATE as a fallback

    // Set a basic gasLimit in the event that other estimation fails
...
      gasLimit =
        draftTransaction.asset.type === AssetType.token ||
        draftTransaction.asset.type === AssetType.NFT
          ? GAS_LIMITS.BASE_TOKEN_ESTIMATE
          : GAS_LIMITS.SIMPLE;
      // Run our estimateGasLimit logic to get a more accurate estimation of
      // required gas. If this value isn't nullish, set it as the new gasLimit
      const estimatedGasLimit = await estimateGasLimitForSend({
...
      });
      gasLimit = estimatedGasLimit || gasLimit;
    }

This leads to the main culprit estimateGasLimitForSend

There are some potential places that can cause estimateGasLimitForSend to return with BASE_TOKEN_ESTIMATE

addGasBuffer

Why I ruled out this function...

• ansatz - failed gas simulation causes this to return null or BASE_TOKEN_ESTIMATE

    if (simulationFailed) {
      const estimateWithBuffer = addGasBuffer(
        paramsForGasEstimate?.gas ?? gasLimit,
        blockGasLimit,
        bufferMultiplier,
      );
      return addHexPrefix(estimateWithBuffer);
    }

This function always returns a value so it must return BASE_TOKEN_ESTIMATE if it is the source of error.

• We can't exit under the first: paramsForGasEstimate.gas > blockGasLimit (Ruled Out) and paramsForGasEstimate.gas ?? gasLimit == BASE_TOKEN_ESTIMATE as the input arg to estimateGasLimitForSend sets blockGasLimit=metamask.currentBlockGasLimit which is defined as getBlockByNumber(blockNumber, false).gasLimit and was 30,000,000 when I checked block # 17891191
• We can't exit under the final return because we would get 30M as above
• The middle return seems impossible as bufferMultiplier=1.5 for mainnet so we would have to have exactly 66667 and it seems that for this case paramsForGasEstimate.gas = 30,000,000 * 0.95 anyway

Finally, I don't consider the simulation passing (the estimate is passed into addGasBuffer before return) as otherwise we would not see a consistent behaviour - as per above it would return the estimatedGasLimit * 1.5.

!to == true

This seems really bizarre and wasn't the first place I'd look as to=draftTransaction.recipient.address.toLowerCase() in the input args to estimateGasLimitForSend but by process of elimination it's another place that can cause a return value of BASE_TOKEN_ESTIMATE since

  if (sendToken) {
    if (!to) {
      // If no to address is provided, we cannot generate the token transfer
      // hexData. hexData in a transaction largely dictates how much gas will
      // be consumed by a transaction. We must use our best guess, which is
      // represented in the gas shared constants.
      return GAS_LIMITS.BASE_TOKEN_ESTIMATE;
    }

and I'm assuming they put this in for a reason...

Unhandled Error

This seems the most likely area - I'm not expert in js but I assume an error would return null from estimateGasLimitForSend and only some errors are handled

• 'Transaction execution error.'
• 'gas required exceeds allowance or always failing transaction'

  } catch (error) {
    const simulationFailed =
      error.message.includes('Transaction execution error.') ||
      error.message.includes(
        'gas required exceeds allowance or always failing transaction',
      ) ||
      (CHAIN_ID_TO_GAS_LIMIT_BUFFER_MAP[chainId] &&
        error.message.includes('gas required exceeds allowance'));
    if (simulationFailed) {
...
    }
    throw error;
  }

note that last error isn't handled for mainnet

The error message received in many cases, when calling eth_estimateGas is just "execution reverted" - this is notably missing from metamask's error handling.

This would therefore trigger the fallback onto gasLimit when estimateGasLimitForSend returned null

gasLimit = estimatedGasLimit || gasLimit;

where gasLimit is set as BASE_TOKEN_ESTIMATE

Answer 3

This is a complex contract set and would require a lot of testing to determine the exact cause.

uint256 policyDegradationPeriod = _mitigationParameters[policyId].degradationPeriod;

I was not able to find this library but this is where the tx is running out of gas.

The mapping is using the type MitigationParameters. I suspect this might be the cause. Check if the library was correctly linked on deployment. If it is not link correctly it's possible that the gas estimate is not getting accurate data that rely on the actual transaction.

Possible cause would be that a previous transaction that change the state of the contract, did not get executed fast enough for the estimate of the subsequent transaction to read the accurate data. The first transaction get executed, affecting the state, the second transaction now fails because the estimate was wrong.