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

Standardise error response with error codes instead of strings for eth_sendTransaction, eth_sendRawTransaction and eth_estimateGas #523

Open
smartprogrammer93 opened this issue Feb 12, 2024 · 2 comments

Comments

@smartprogrammer93
Copy link
Contributor

smartprogrammer93 commented Feb 12, 2024

Like seen in this issue, there is no standard error code response for such errors.
My opinion is that depending on strings is futile. and we should possibly try to depend on error codes for tx failure errors instead.
Geth implementation:
https://github.com/ethereum/go-ethereum/blob/master/core/error.go
and here
https://github.com/ethereum/go-ethereum/blob/master/core/types/transaction.go
and here
https://github.com/ethereum/go-ethereum/blob/master/core/txpool/errors.go
Neth implementation:
https://github.com/NethermindEth/nethermind/blob/master/src/Nethermind/Nethermind.TxPool/AcceptTxResult.cs
and possibly other places as well.

@smartprogrammer93
Copy link
Contributor Author

smartprogrammer93 commented Feb 12, 2024

One thing to note is what @shemnon said on discord:
"This list should be prioritized, in case there are multiple things wrong with a single call."

@smartprogrammer93
Copy link
Contributor Author

from @fjl

At the moment, we only have a single custom error code: when eth_call or eth_estimateGas hits a revert, Geth returns error code 3
I have added a hive test to check this behavior and while other clients including Nethermind catch the revert and return an error, we do not all agree on the code in this situation
As a first step, I propose we come up with a list of error situations
Then we can assign the codes and add rpc-compat tests

fjl added a commit to ethereum/go-ethereum that referenced this issue Nov 7, 2024
Here I'm adding a new helper function that extracts the revert reason of
a contract call. Unfortunately, this aspect of the API is underspecified.
See these spec issues for more detail:

- ethereum/execution-apis#232
- ethereum/execution-apis#463
- ethereum/execution-apis#523

The function added here only works with Geth-like servers that return
error code `3`. We will not be able to support all possible servers.
However, if there is a specific server implementation that makes it
possible to extract the same info, we could add it in the same function
as well.

---------

Co-authored-by: Marius van der Wijden <[email protected]>
holiman pushed a commit to ethereum/go-ethereum that referenced this issue Nov 19, 2024
Here I'm adding a new helper function that extracts the revert reason of
a contract call. Unfortunately, this aspect of the API is underspecified.
See these spec issues for more detail:

- ethereum/execution-apis#232
- ethereum/execution-apis#463
- ethereum/execution-apis#523

The function added here only works with Geth-like servers that return
error code `3`. We will not be able to support all possible servers.
However, if there is a specific server implementation that makes it
possible to extract the same info, we could add it in the same function
as well.

---------

Co-authored-by: Marius van der Wijden <[email protected]>
zfy0701 pushed a commit to sentioxyz/go-ethereum that referenced this issue Dec 3, 2024
Here I'm adding a new helper function that extracts the revert reason of
a contract call. Unfortunately, this aspect of the API is underspecified.
See these spec issues for more detail:

- ethereum/execution-apis#232
- ethereum/execution-apis#463
- ethereum/execution-apis#523

The function added here only works with Geth-like servers that return
error code `3`. We will not be able to support all possible servers.
However, if there is a specific server implementation that makes it
possible to extract the same info, we could add it in the same function
as well.

---------

Co-authored-by: Marius van der Wijden <[email protected]>
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

No branches or pull requests

1 participant