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

update eth_call error message #8129

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

matkt
Copy link
Contributor

@matkt matkt commented Jan 16, 2025

PR description

This allows changing the error message during an eth_call by displaying only the error message itself if available, instead of showing the name in the enum. It also makes it possible to display the invalid opcode in case of an invalid operation and more information in case of error (see tests modifications).

Before :
{"jsonrpc":"2.0","id":1,"error":{"code":-32000,"message":"Transaction processing could not be completed due to an exception"}}

After :
{"jsonrpc":"2.0","id":1,"error":{"code":-32000,"message":"Transaction processing could not be completed due to an exception (invalid opcode: 0xaa)"}}

you will be able to see more info after an eth_call:

  • Intrinsic gas exceeds gas limit (intrinsic gas cost 21000 exceeds gas limit 0)
  • Total blob gas too high (total blob gas 917504 exceeds max blob gas per block 786432)“}
  • Upfront cost exceeds account balance (transaction up-front cost 0x2 exceeds transaction sender account balance 0x1)
  • etc

Fixed Issue(s)

Thanks for sending a pull request! Have you done the following?

  • Checked out our contribution guidelines?
  • Considered documentation and added the doc-change-required label to this PR if updates are required.
  • Considered the changelog and included an update if required.
  • For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

  • unit tests: ./gradlew build
  • acceptance tests: ./gradlew acceptanceTest
  • integration tests: ./gradlew integrationTest
  • reference tests: ./gradlew ethereum:referenceTests:referenceTests

matkt added 2 commits January 16, 2025 14:49
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
@matkt matkt requested review from shemnon and matthew1001 January 16, 2025 14:32
@matkt matkt marked this pull request as ready for review January 16, 2025 14:33
@shemnon
Copy link
Contributor

shemnon commented Jan 16, 2025

The changes in the evm module seem orthogonal to this PR. Can they be put in a separate PR? Segregation like this helps immensity when doing "code spelunking" as it removes false-positive rat-holes when bug hunting.

@matkt
Copy link
Contributor Author

matkt commented Jan 17, 2025

The changes in the evm module seem orthogonal to this PR. Can they be put in a separate PR? Segregation like this helps immensity when doing "code spelunking" as it removes false-positive rat-holes when bug hunting.

The changes to the EVM are necessary for the PR, as they enable displaying the invalid opcode in the eth_call response, which was the initial goal but I can create two PRs if you really think it's better @shemnon ?

matkt added 4 commits January 17, 2025 10:59
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
@shemnon
Copy link
Contributor

shemnon commented Jan 17, 2025

RPC and EVM are uncoupled systems. I would prefer two PRs. Perhaps the RPC changes and then the EVM changes on top of them.

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.

2 participants