-
Notifications
You must be signed in to change notification settings - Fork 21.5k
all: improve EstimateGas API #20830
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
all: improve EstimateGas API #20830
Changes from all commits
c1e706a
38cbee9
e390d4f
af58d13
662dab7
19882fd
bc1e1a1
b0821c0
0015a42
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ import ( | |
| "time" | ||
|
|
||
| "github.com/ethereum/go-ethereum" | ||
| "github.com/ethereum/go-ethereum/accounts/abi" | ||
| "github.com/ethereum/go-ethereum/accounts/abi/bind" | ||
| "github.com/ethereum/go-ethereum/common" | ||
| "github.com/ethereum/go-ethereum/common/math" | ||
|
|
@@ -49,7 +50,6 @@ var ( | |
| errBlockNumberUnsupported = errors.New("simulatedBackend cannot access blocks other than the latest block") | ||
| errBlockDoesNotExist = errors.New("block does not exist in blockchain") | ||
| errTransactionDoesNotExist = errors.New("transaction does not exist") | ||
| errGasEstimationFailed = errors.New("gas required exceeds allowance or always failing transaction") | ||
| ) | ||
|
|
||
| // SimulatedBackend implements bind.ContractBackend, simulating a blockchain in | ||
|
|
@@ -349,8 +349,11 @@ func (b *SimulatedBackend) CallContract(ctx context.Context, call ethereum.CallM | |
| if err != nil { | ||
| return nil, err | ||
| } | ||
| rval, _, _, err := b.callContract(ctx, call, b.blockchain.CurrentBlock(), state) | ||
| return rval, err | ||
| res, err := b.callContract(ctx, call, b.blockchain.CurrentBlock(), state) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return res.Return(), nil | ||
| } | ||
|
|
||
| // PendingCallContract executes a contract call on the pending state. | ||
|
|
@@ -359,8 +362,11 @@ func (b *SimulatedBackend) PendingCallContract(ctx context.Context, call ethereu | |
| defer b.mu.Unlock() | ||
| defer b.pendingState.RevertToSnapshot(b.pendingState.Snapshot()) | ||
|
|
||
| rval, _, _, err := b.callContract(ctx, call, b.pendingBlock, b.pendingState) | ||
| return rval, err | ||
| res, err := b.callContract(ctx, call, b.pendingBlock, b.pendingState) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return res.Return(), nil | ||
| } | ||
|
|
||
| // PendingNonceAt implements PendingStateReader.PendingNonceAt, retrieving | ||
|
|
@@ -398,39 +404,67 @@ func (b *SimulatedBackend) EstimateGas(ctx context.Context, call ethereum.CallMs | |
| cap = hi | ||
|
|
||
| // Create a helper to check if a gas allowance results in an executable transaction | ||
| executable := func(gas uint64) bool { | ||
| executable := func(gas uint64) (bool, *core.ExecutionResult, error) { | ||
| call.Gas = gas | ||
|
|
||
| snapshot := b.pendingState.Snapshot() | ||
| _, _, failed, err := b.callContract(ctx, call, b.pendingBlock, b.pendingState) | ||
| res, err := b.callContract(ctx, call, b.pendingBlock, b.pendingState) | ||
| b.pendingState.RevertToSnapshot(snapshot) | ||
|
|
||
| if err != nil || failed { | ||
| return false | ||
| if err != nil { | ||
| if err == core.ErrIntrinsicGas { | ||
| return true, nil, nil // Special case, raise gas limit | ||
| } | ||
| return true, nil, err // Bail out | ||
| } | ||
| return true | ||
| return res.Failed(), res, nil | ||
| } | ||
| // Execute the binary search and hone in on an executable gas limit | ||
| for lo+1 < hi { | ||
| mid := (hi + lo) / 2 | ||
| if !executable(mid) { | ||
| failed, _, err := executable(mid) | ||
|
|
||
| // 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 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't strictly true. A transaction may have a line like
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @MicahZoltu It might help if we could collect a small pool of solidity code to add to tests and the desired behavior in those cases. Code that depends on the gas counter is a bit hard to handle correctly. e.g. ^ cannot be binary searched, so the estimator cannot ever properly pick a correct number. It might be a valid corner case though to support estimating these kinds of txs as long as they can be binary searched.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feel free to open a new issue with a code that fails estimation and we can add some tweaks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah sorry, I don't think I actually want support for these edge cases, it feels not worth the effort. I was merely stating that the comment in the code isn't strictly correct and for clarity to future readers it may be valuable to mention something like "we don't care about these edge cases at the moment" in the comment.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @MicahZoltu Actually the scenario you described is handled. Since all |
||
| if err != nil { | ||
| return 0, err | ||
| } | ||
| if failed { | ||
| lo = mid | ||
| } else { | ||
| hi = mid | ||
| } | ||
| } | ||
| // Reject the transaction as invalid if it still fails at the highest allowance | ||
| if hi == cap { | ||
| if !executable(hi) { | ||
| return 0, errGasEstimationFailed | ||
| failed, result, err := executable(hi) | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
| if failed { | ||
| if result != nil && result.Err != vm.ErrOutOfGas { | ||
| errMsg := fmt.Sprintf("always failing transaction (%v)", result.Err) | ||
rjl493456442 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| if len(result.Revert()) > 0 { | ||
| ret, err := abi.UnpackRevert(result.Revert()) | ||
| if err != nil { | ||
| errMsg += fmt.Sprintf(" (%#x)", result.Revert()) | ||
| } else { | ||
| errMsg += fmt.Sprintf(" (%s)", ret) | ||
| } | ||
| } | ||
| return 0, errors.New(errMsg) | ||
| } | ||
| // Otherwise, the specified gas cap is too low | ||
| return 0, fmt.Errorf("gas required exceeds allowance (%d)", cap) | ||
| } | ||
| } | ||
| return hi, nil | ||
| } | ||
|
|
||
| // callContract implements common code between normal and pending contract calls. | ||
| // state is modified during execution, make sure to copy it if necessary. | ||
| func (b *SimulatedBackend) callContract(ctx context.Context, call ethereum.CallMsg, block *types.Block, statedb *state.StateDB) ([]byte, uint64, bool, error) { | ||
| func (b *SimulatedBackend) callContract(ctx context.Context, call ethereum.CallMsg, block *types.Block, statedb *state.StateDB) (*core.ExecutionResult, error) { | ||
| // Ensure message is initialized properly. | ||
| if call.GasPrice == nil { | ||
| call.GasPrice = big.NewInt(1) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.