internal, accounts, eth: utilize vm failed flag to help gas estimation #15030
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fjl
approved these changes
Sep 7, 2017
|
@karalabe PTAL |
karalabe
force-pushed
the
expose_vm_failed
branch
from
ab71fa4 to
94903d5
Compare
|
I've pushed a minor polish for this PR so it can also estimate 21000 exactly (it would only do 21001 until now). One thing we need to check though is if this PR breaks pre-byzantium chains estimation or not. I don't remember exactly when we return |
|
Seems to work for both plain value transfers as well as a funky gas contract on both pre- and post Byzantium. Will merge as is and hope we don't break anything (given that post Byz is currently borked). |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for this PR:
Now the gas is estimated through the gas is consumed all. But this method does not apply to all scenes. e.g. in the frontier contract deploys can fail without consuming all gas due to have not enough gas to store contract code. And in metropolish will have a revert opcode that can also revert without using all gas.
So a failed flag returned by vm is useful to detect whether an error occurred during the execution of the transaction. It can help to estimate gas accurately.
Besides, I raise the lower limit of the gas estimate to 21000, since the minimum gas cost is 21000.
Though there is another situation we can not handle it. If the call itself has a logical problem, no matter how much gas assigned, it always failed.