Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[R4R]-fix: estimate gas evm error #77

Merged
merged 2 commits into from
Apr 15, 2024

Conversation

asdv23
Copy link

@asdv23 asdv23 commented Apr 12, 2024

When calling eth_estimateGas, if the EVM throws an error, it will result in the program throwing an "insufficient funds for gas * price + value" error instead of the correct EVM error.

Cause: When estimating gas in the layer 2, it includes gas cost from the layer 1. This causes a slight shortage when estimating gas using the user's entire balance. Additionally, during the re-estimation process, if an EVM error occurs but is not an out-of-gas error, it will be ignored. This ultimately leads to the entire estimation process failing to correctly throw an EVM error. Therefore, each estimation should include a check for an EVM result. If the result is not empty and not an out-of-gas error, the estimation should be terminated, and an exception should be thrown.

Reproduction steps: Deploy an ERC20 contract and use the transfer function. Test with three different accounts to verify that the ERC20InsufficientBalance error is correctly thrown. The balances of the three accounts are 0 MNT, 0.1 MNT, and 1000 MNT, respectively.


// If the error is not nil(consensus error), it means the provided message
// call or transaction will never be accepted no matter how much gas it is
// assigned. Return the error directly, don't struggle any more.
if err != nil {
return 0, err
}

if result != nil && result.Err != vm.ErrOutOfGas {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if result != nil && !errors.Is(result.Err, vm.ErrOutOfGas) may be better

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@Tri-stone Tri-stone merged commit 36177af into release/v1.0.0 Apr 15, 2024
2 checks passed
Tri-stone pushed a commit that referenced this pull request Apr 28, 2024
* fix estimateGas eat evm error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants