-
Notifications
You must be signed in to change notification settings - Fork 20.9k
internal/ethapi: return code 3 from call/estimateGas even if a revert reason was not returned #31456
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
Conversation
…even if a revert reason was not supplied
I will add some additional test coverage with this PR if we agree that the changes here are ok. |
l |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the error handling in the ethapi package so that both DoCall and DoEstimateGas consistently return a revertError when a revert occurs, regardless of whether revert data is provided.
- Updates condition checks in Call and DoEstimateGas to use errors.Is(err, vm.ErrExecutionReverted) instead of checking the length of the revert data.
- Ensures uniform error reporting across eth_call and eth_estimateGas methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but as you said, tests are needed. Also, refer to the PR that prompted you to work on this.
This PR was prompted by #31444 . I think there was some confusion with the other PR because currently we define the default rpc error code and a constant for the revert error code to have the same value of So right now, when receiving an rpc error (of a generic type) from a revert w/o revert data, where the error contains said error code and the message "execution reverted", it could lead to the assumption that the error is a revert error type. |
… reason was not returned (ethereum#31456)
… reason was not returned (ethereum#31456)
… reason was not returned (ethereum#31456)
… reason was not returned (ethereum#31456)
Currently we only return a
revertError
fromBlockchainAPI.(DoCall,EstimateGas)
when execution reverts with non-empty revert reason.If the revert data is empty, the RPC response is constructed with a generic
errCodeDefault
code here.By altering the behavior to return a
revertError
fromDoCall
/DoEstimateGas
upon revert regardless of whether there was a reason supplied, we consistently return the same revert error code whenever a revert occurs frometh_estimateGas
/eth_call
.Steps to Verify this PR is Correct
Using the genesis files I have attached in this gist:
Spin up a dev-mode instance of Geth. The genesis contains an account
0x1000000000000000000000000000000000000001
which when called will revert with no data.In a separate terminal:
which on master outputs:
with this PR:
Now repeat the process again, except this time instantiating the datadir with
genesis-nonempty-revert.json
: The0x1000000000000000000000000000000000000001
account now contains code that reverts with a single byte of revert data.On master, and this PR:
The same instructions can be repeated except with
eth_call
instead ofeth_estimateGas
and produce the same results as above.