moonbeam-foundation / moonbeam

An Ethereum-compatible smart contract parachain on Polkadot
https://moonbeam.network
GNU General Public License v3.0
920 stars 335 forks source link

Gas estimation not working in complex call with internal X-Tokens call #2937

Closed TorstenStueber closed 1 month ago

TorstenStueber commented 1 month ago

It seems that with the runtime upgrade of Moonbeam to version 3100 two days ago gas estimation for contract calls that have an internal call to the X-Tokens precompiled contract is not working correctly anymore.

Here is a call to the Axelar SquidRouter contract that has a post hook call that is supposed to eventually send the incoming tokens via XCM to another parachain using the X-Tokens precompiled.

The transaction has many internal calls and will eventually call the precompiled at address 0x00...804. However, this is when the call reverts (more below). The outer call does not revert as it wraps the post hook calls into a try catch block.

Estimating the gas (see this gist) for this very call yields that the call would consume 578867 gas. The gas limit used in this call was actually 40% higher to give plenty of room for the call to execute.

However, the call to X-Tokens precompiled reverted because of an insufficient gas limit or weight usage. The weight usage for the execution is tracked in a SubstrateStackState object. The failure happened exactly at this point . Here the weight_info has the value

WeightInfo { ref_time_limit: Some(20010350000), proof_size_limit: Some(101301), ref_time_usage: Some(0), proof_size_usage: Some(60772) }

and the value of amount is 50000, so that 60772 + 50000 exceeds the proof_size_limit 101301.

Conclusion

It is clear that with a higher gas limit this call would have succeeded because the proof_size_limit in the SubstrateStackState is just 1/8 of the transaction gas limit. This indicates that the gas estimation did not work properly.

It seems that there is some confusion of tracking gas usage in the proof_size component of the SubstrateStackState in regards to the xtokens precompiled when it comes to gas estimation for the outer transaction in this example.

When estimating gas usage for a direct call to the X-Tokens precompiled, then the usage is (correctly) estimated to be 495342.

This problem did not seem to occur in the previous Moonbeam runtime version 3001.

librelois commented 1 month ago

thank @TorstenStueber for your highly detailed report.

I think the behavior didn't change, but it was unlikely to be triggered before because GasLimitPovSizeRatio was equal to 4.

To trigger this situation you need to have 60772+ XCM msg weight < (gas_limit / GasLimitPovSizeRatio), you need a gas limit high enough to reach the post hook, but low enough to fail.

The scenario you describe seem's not a bug to me, it's the expected behavior because the outer call succeed.

Like geth, our gas estimation algorithm is a binary search, that find the lowest gas limit where the outer call Succeed.

It's the Axelar SquidRouter contract implementation that is wrong, as it should revert or return an error that the estimation gas algorithm can understand here: https://github.com/moonbeam-foundation/frontier/blob/moonbeam-polkadot-v1.11.0/client/rpc/src/eth/execute.rs#L983-L985

TorstenStueber commented 1 month ago

Thanks @librelois, this makes sense to me.

Looking at the contract the outer contract doesn’t revert when the inner contract (post hook) call fails, such as when it runs out of gas. Your gas estimation algorithm would then determine a gas estimate that leaves only a minimum amount of gas to the inner call so that it immediately reverts due to being out of gas.

Previously, it worked because the caller added a 40% margin to the gas estimate, but with your new gas metering (which in addition to the standard EVM gas metering uses a Substrate based weight metering as invoked here), that’s no longer enough.

I initially thought this was a bug, expecting your gas estimation and metering to be fully EVM-compatible. However, I now understand that it’s intentionally not (Section "Transaction Fees").

Is my understanding correct?

librelois commented 1 month ago

I initially thought this was a bug, expecting your gas estimation and metering to be fully EVM-compatible. However, I now understand that it’s intentionally not (Section "Transaction Fees").

Technically, it's impossible to be fully EVM-compatible as a parachain because of the proof size hard limit from the relay chain. All storage read/write imply to fill a merkle proof, and the combiend merkle proof that contains all keys that are read inside the block should not exceed 5Mo.

So, we have to account for the storage proof size impact of any storage read/write operation, taht's why we have a multidimensional gasometer that account both gas and proof size.

The GasLimitPovSizeRatio is equal to BlockGasLimit / PovLimit, since we have 2 times more gas per block but the same PovLimit, the ratio has doubled.

TorstenStueber commented 1 month ago

Thanks for the explanation. I will close this ticket now.