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: contract call estimate gas bug #79

Merged
merged 1 commit into from
Apr 23, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion internal/ethapi/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -1323,6 +1323,17 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr
}
return result.Failed(), result, nil
}

// When calling across contracts, an opCall call error out of gas will occur. However, this error is not execution reverted,
// causing the error and cannot be captured by the call stack. Therefore, when subsequent opcode is executed, opRevert is
// triggered because the return value is not the expected value. But if this error is triggered during the bisection process,
// it can be tolerated because smaller gas is used in the estimation.
//
// Therefore, the function of this flag is that as long as estimateGas is successful once during the bisection process, it will
// not terminate. The process of bisection and throwing an evm error (drawing on the idea of ​​Ethereum, using the largest balance
// to estimate once, if successful, no subsequent judgment errors will be made, if failed, an exception will be thrown directly)
var success bool

// Execute the binary search and hone in on an executable gas limit
for lo+1 < hi {
mid := (hi + lo) / 2

Choose a reason for hiding this comment

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

if do call error with first mid gas, this PR don't fix the problem
it is better to do call with hi gas value before into binary search

Expand All @@ -1339,7 +1350,9 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr
// returning normally, but the result.Err will be nil, which will cause the
// estimated gas to be 0(return 0, result.Err), thus causing txpool to
// report an error: intrinsic gas too low
if result != nil && result.Failed() &&
//
// It is only necessary to determine the evm error when the estimate is unsuccessful.
if !success && result != nil && result.Failed() &&
!errors.Is(result.Err, vm.ErrOutOfGas) &&
!errors.Is(result.Err, vm.ErrCodeStoreOutOfGas) {
if len(result.Revert()) > 0 {
Expand All @@ -1351,6 +1364,7 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr
if failed {
lo = mid
} else {
success = true
hi = mid
}
}
Expand Down