Skip to content

Conversation

@jwasinger
Copy link
Contributor

@jwasinger jwasinger commented Mar 22, 2025

Currently we only return a revertError from BlockchainAPI.(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 from DoCall/DoEstimateGas upon revert regardless of whether there was a reason supplied, we consistently return the same revert error code whenever a revert occurs from eth_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.

./build/bin/geth --datadir ./data init genesis-empty-revert.json
./build/bin/geth --dev --datadir ./data --http --http.api eth

In a separate terminal:

> curl http://localhost:8545 \
  -X POST \
  -H "Content-Type: application/json" \
  --data '{"jsonrpc":"2.0","method":"eth_call","params":[{"to": "0x1000000000000000000000000000000000000001"}],"id":1}'

which on master outputs:

{"jsonrpc":"2.0","id":1,"error":{"code":-32000,"message":"execution reverted"}}

with this PR:

{"jsonrpc":"2.0","id":1,"error":{"code":3,"message":"execution reverted","data":"0x"}}

Now repeat the process again, except this time instantiating the datadir with genesis-nonempty-revert.json: The 0x1000000000000000000000000000000000000001 account now contains code that reverts with a single byte of revert data.

On master, and this PR:

{"jsonrpc":"2.0","id":1,"error":{"code":3,"message":"execution reverted","data":"0x00"}}

The same instructions can be repeated except with eth_call instead of eth_estimateGas and produce the same results as above.

@jwasinger
Copy link
Contributor Author

jwasinger commented Mar 22, 2025

I will add some additional test coverage with this PR if we agree that the changes here are ok.

@DenisNysheta
Copy link

l

@gballet gballet requested a review from Copilot March 23, 2025 14:02
Copy link
Contributor

Copilot AI left a 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.

Copy link
Member

@gballet gballet left a 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.

@jwasinger
Copy link
Contributor Author

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 -32000.

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.

@fjl fjl changed the title internal/ethapi: return ethapi.revertError from DoCall/DoEstimateGas even if a revert reason was not supplied internal/ethapi: return code 3 from call/estimateGas even if a revert reason was not returned Mar 23, 2025
@fjl fjl merged commit b0b2b76 into ethereum:master Mar 23, 2025
3 of 4 checks passed
@fjl fjl added this to the 1.15.6 milestone Mar 23, 2025
sidhujag pushed a commit to syscoin/go-ethereum that referenced this pull request Mar 26, 2025
sivaratrisrinivas pushed a commit to sivaratrisrinivas/go-ethereum that referenced this pull request Apr 21, 2025
sduchesneau pushed a commit to streamingfast/go-ethereum that referenced this pull request May 22, 2025
jakub-freebit pushed a commit to fblch/go-ethereum that referenced this pull request Jul 3, 2025
howjmay pushed a commit to iotaledger/go-ethereum that referenced this pull request Aug 27, 2025
gballet pushed a commit to gballet/go-ethereum that referenced this pull request Sep 11, 2025
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.

4 participants