iotaledger / wasp

Node for IOTA Smart Contracts
Apache License 2.0
296 stars 147 forks source link

Unable to estimate gas on few ApeDAO Smart Contract functions #2740

Closed eGarciaR closed 1 year ago

eGarciaR commented 1 year ago

Describe the bug

ShimmerEVM is not able to estimate gas of two functions of our smart contract (mint() and redeem()). The rest of the functions work well in terms of gas estimation. This bug is forcing us to estimate gas manually via frontend, which is not ideal as sometimes manual estimation is wrong and provides a bad UX for our users. This bug is not unique to this ShimmerEVM version, and this happened on previous versions as well.

The contract that we are using can be found at address: "0x4D5E906e570B2ef2bbbE2121Ffd024108ceeb211". Block explorer can be used to test this, and you can see how MM throws an error as it is unable to estimate fees. You can try for example to run redeem() function with args: redeem(1,0,[162],[])

To Reproduce

Therefore, to reproduce the error: 1: Go the the explorer at address "0x4D5E906e570B2ef2bbbE2121Ffd024108ceeb211": https://explorer.evm.testnet.shimmer.network/address/0x4D5E906e570B2ef2bbbE2121Ffd024108ceeb211

  1. Go to "Write Proxy" tab (as this is under a Transparent Proxy
  2. Fill redeem() function with values 1,0,[162],[]. -> redeem(1,0,[162],[])
  3. Press "Write" button
  4. Metamask will open and you'll be able to see the estimation error:

image

Probably you'll always get the error, since you don't own the required tokens to redeem the underlying NFT. But we've tried with enough tokens and we still get the same error. We are happy to provide test tokens for you to reproduce the error.

If you want to check the code, this is the implementation address: "0xA074C8932207b07Be5d1Ca6D35ed64be2D4DA67E". As we are using TransparentProxy under address "0x4D5E906e570B2ef2bbbE2121Ffd024108ceeb211".

Regarding the mint() function, the network is able to estimate correctly until you use more than 5 items in the array as parameters. When you do this, estimation fails.

Additional context

This works well on Fantom EVM, so we believe it is an issue with ShimmerEVM. We are more than happy to provide you with details of Fantom Testnet so that you can test there as well if needed to see how it behaves.

Please let us know if you would need more information or if you want us to test anything.

dessaya commented 1 year ago

From what I can see in the devtools, eth_estimateGas returns execution reverted: ERC20: burn amount exceeds balance. Is this an error message produced by the contract?

eGarciaR commented 1 year ago

Yes, this is what I was referring to when I said "Probably you'll always get the error, since you don't own the required tokens to redeem the underlying NFT". It would be interesting to see what's the error that you see if you have enough tokens. I can send you the tokens for you to test, or I can test this myself if you kindly explain to me how to use those devtools to see the underlying error :D

Thanks in advance

dessaya commented 1 year ago

In order to get to the error message, in chrome I do the following:

  1. Open MM in a tab (menu -> "expand view")
  2. In the URL Replace home.html with background.html
  3. Open developer tools for this URL, go to Network tab to start recording
  4. Use MM from the popup
eGarciaR commented 1 year ago

That's good to know, thanks :)

So, I've checked that, and the error I get from eth_estimateGas is execution reverted: Low-level call failed.

Any idea from where this is coming from? Doesn't seem to be a Smart Contract issue, since in Fantom EVM it works, and also, if I proceed with the Metmask warning of "unable to estimate gas fees", the transaction succeeds.

dessaya commented 1 year ago

Never seen that error before. Searching github, it seems that it is produced by many contracts. Maybe you are using one of those?: https://github.com/search?q=%22%5C%22Low-level+call+failed%5C%22%22&type=code

eGarciaR commented 1 year ago

Not using one of those, but at least you made find from where in our Smart Contracts this is coming from. We are using low-level calls to call an ERC721 function. For an unknown reason that call reverts with no data in it, and therefore that error is thrown.

I'm not sure if this might fix the gas estimation problem, but instead of making a low-level call I'll try to use an ERC721 interface instead.

I'll be re-deploying contracts to test this. I'll come back with the results as soon as I can. Thanks Diego for your time

eGarciaR commented 1 year ago

Hi Diego, just an update before I close this issue:

I updated the smart contracts to use interfaces instead of low-level calls. I can confirm that the network is able to properly estimate gas fees now. Knowing this might be helpful for anyone else facing the same issues.

Thanks for your time and help!