polkadot-evm / frontier

Ethereum compatibility layer for Substrate.
Apache License 2.0
574 stars 489 forks source link

Incorrect gas estimation causing `OutOfGas` error #1500

Closed magecnion closed 2 months ago

magecnion commented 2 months ago

Overview

When sending transactions for the precompile pallet-parachain-staking, gas estimation is not working for some calls. We also realized that gas estimation is incorrect for other pallets as well, as described here.

Details

When sending the delegate action using Remix + Metamask, the pallet-evm raises an OutOfGas error, as shown in the PolkadotJS UI: image

However, when performing the same action using web3 libraries, the issue does not occur. Setting a fixed amount of gas allows the process to work fine. The problem is that Metamask sets the gas amount by retrieving the value from a previous call to eth_estimateGas. The estimation returned for the delegation call is 101433, but the actual gas consumption for delegation is 220440. As a result, when using Metamask, the gas value is lower than the actual gas used, causing the call to run out of gas and resulting in the OutOfGas error from pallet-evm.

The workaround is to set a sufficiently high fixed gas amount to ensure the transaction does not run out of gas during execution.

A brief investigation revealed that Moonbeam forked all EVM-related projects and introduced the concept of storage growth related to gas, as described here. Further investigation is needed to determine whether the discrepancies in estimation are due to this.

Other calls that ran out of gas:

Environment

Expected Behavior

As a Metamask (or any web3 tool) user, I want to perform all EVM actions using the gas value obtained from estimation, without needing to set a fixed value to prevent running out of gas. Standard online tools should work out of the box.

u-hubar commented 2 months ago

This has also become an issue for pallet-evm on the Neuroweb parachain after updating Frontier from branch polkadot-v0.9.40 to branch polkadot-v1.11.0.

On older version it was working just fine, so this bug was introduced with one of the reworks for gas estimation that I see happened.

zjb0807 commented 2 months ago

This PR fixes some issues with eth_estimateGas, you can try to see if it solves the problem. It is in polkadot-v1.12.0

u-hubar commented 2 months ago

@zjb0807 but this PR is included on the polkadot-v1.11.0 branch?

branarakic commented 2 months ago

This issue seems to have a major impact. Basic user interactions (e.g. sending ERC20 tokens through Metamask) are mostly non-operational. From a user perspective, the tokens will seem locked and unusable, and this definitely requires an escalation.

Apart from an (urgent) fix to this, I suggest an e2e test be created with at least one major wallet which is added into the CI/CD pipeline.

magecnion commented 2 months ago

This PR fixes some issues with eth_estimateGas, you can try to see if it solves the problem. It is in polkadot-v1.12.0

@zjb0807 I have upgraded to stable2407 as polkadot-v1.12.0 brings some errors. The estimation problem still persists, as you can see in this test. It can be seen that the estimated gas value is still lower than the 220440 gas needed.

Any idea what is going on here?

cc @boundless-forest might have some hint, as he was involved in the previously mentioned fixes.

boundless-forest commented 2 months ago

Sorry for the delayed response. I've been a bit busy lately. I believe the issue you're encountering is due to missing something in the runtime during the upgrade. I took a quick look at your repo, https://github.com/freeverseio/laos/blob/main/runtime/laos/src/apis.rs, and noticed that the EVM call and create differ from the Frontier template node. You can check it here: https://github.com/polkadot-evm/frontier/blob/stable2407/template/runtime/src/lib.rs#L786-L864. You can revise them and then give it another attempt.

Tips: While upgrading the polkadot-sdk, you need to review all the changes that occur instead of relying solely on the compile results.

magecnion commented 2 months ago

@boundless-forest that was exactly the problem. It is working now, even with the polkadot-v1.11.0 version. Thank you so much!