Metamask sets gas limit to block gas limit

[tschubotz]

Title/Description

Sometimes, Metamask sets the gas limit to the block gas limit for Safe transactions as well as Safe creations

Environment

• Browser: ?
• Safe: ?
• Wallet: Metamask
• App Version: ?
• Environment:
  • production (mainnet)

Steps to reproduce

??

Expected result

Gas limit is properly set

Obtained result

Gas limit is set to 11M

Screenshots

Misc

• Metamask sets the gas limit to the block gas limit on estimation failure.
• Yes, this seems like a Metamask issue, still it happens regularly.
• This ticket is to collect steps to perhaps make this reproducible.

[tschubotz]

Report:

Create a Spending limit for 10GNO (monthly, recurring) on Chrome, incl. enable module.

[tschubotz]

Add owner, (approveHash): https://gnosisinc.slack.com/archives/C63LQTGNB/p1606327619373700

Safe creation: https://gnosisinc.slack.com/archives/C63LQTGNB/p1606383472379200?thread_ts=1606383316.378900&cid=C63LQTGNB

[Agupane]

I'm putting the on hold label as @pablofullana suggested until we find a way to replicate/implement the fix

[lukasschor]

Just had a user complain about this issue during a Safe creation, but this user was using the Desktop App with Portis. So it seems this is not even necessarily related to the web app, a specific browser or Metamask.

[tschubotz]

@Agupane the problem is, we cannot reproduce this 😬

Is there anything else we can do?

Are we setting the gas limit which is then used in the owner wallet (e.g. Metamask)? Or is it Metamask that is doing the estimation and then setting the gas limit?

[Agupane]

As fas I investigated we are not setting manually a limit for the transactions, seems that MM (or the wallet) it's doing the estimation automatically, we can set the gasLimit manually, but I'm not sure if that would be expected, can you confirm this?

CC: @rmeissner

[tschubotz]

If there is a reliable way for us to estimate the transactions, I'm in favor of setting the gasLimit that is being used in the owner wallet (e.g. Metamask) manually. Seems to me like that would be more reliable than what Metamask does. Don't we already do that for e.g. hardware wallets? Or how does it work there?

For technical judgement etc I would follow whatever @rmeissner or @dasanra think here though.

[Agupane]

We are not setting any gasLimit, seems that every wallet do his own estimation, same for hardware wallet

[rmeissner]

Seems to me like that would be more reliable than what Metamask does

Yes there have always been cases where MM messed it up but the recent cases (that triggered this issue) kind of make it required that we start providing an estimate. I am not sure how these estimates are handled by other smart contract wallets. I know that the legacy mobile app just ignored them. In any case it should not harm to provide a gas limit estimate (e.g. eth_estimateGas * 1.2 -> this is what MM does). Also we could do more advanced logic like trying to check at what gas limit execTransaction is successfull (aka returns true)