Skip to content

internal/ethapi: return code 3 in eth_simulate for reverts#31444

Closed
mattsse wants to merge 1 commit intoethereum:masterfrom
mattsse:matt/use-revert-error-code-for-reverterr
Closed

internal/ethapi: return code 3 in eth_simulate for reverts#31444
mattsse wants to merge 1 commit intoethereum:masterfrom
mattsse:matt/use-revert-error-code-for-reverterr

Conversation

@mattsse
Copy link
Copy Markdown

@mattsse mattsse commented Mar 20, 2025

Revert error has a special error code:

https://github.com/ethereum/go-ethereum/blob/main/internal/ethapi/errors.go#L36-L40

This changes the errCodeReverted to 3 and uses the ErrorCode function for the callError

the referenced wiki no longer exists, but is archived here: https://github.com/vapory-legacy/wiki/blob/master/JSON-RPC-Error-Codes-Improvement-Proposal.md

ref paradigmxyz/reth#15126

@jwasinger jwasinger force-pushed the matt/use-revert-error-code-for-reverterr branch from 67a7ec2 to 87fdcf3 Compare March 22, 2025 10:16
@jwasinger jwasinger self-assigned this Mar 22, 2025
Copy link
Copy Markdown
Contributor

@jwasinger jwasinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The underlying issue is that Geth doesn't return a revert error code if a call/estimateGas failed without a revert reason. I've submitted a separate PR to address that. But I think we should merge this PR as well. CC @s1na as this does alter the behavior of the simulate API when reverts happen (but it appears it was faulty before).

@jwasinger jwasinger changed the title chore: use revert error code for execution reverted internal/ethapi: set errCodeReverted to the proper code Mar 22, 2025
@jwasinger
Copy link
Copy Markdown
Contributor

I guess one other piece of context here. We currently define errCodeReverted to have the same value as the generic rpc error code errcodeDefault. Currently Geth will return a generic error code for a revert that occurs without revert data. It will return the correct code (3) for a revert containing data.

The PR I linked before fixes this issue. But this PR should go in as well.

@lightclient lightclient changed the title internal/ethapi: set errCodeReverted to the proper code internal/ethapi: return code 3 in eth_simulate for reverts May 1, 2025
@lightclient lightclient added this to the 1.15.11 milestone May 1, 2025
Copy link
Copy Markdown
Member

@lightclient lightclient left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was about to merge, but realized that eth_simulate does not use 3 for the return code during reverts. This is also unrelated to the original issue between reth <> geth, which was resolved in #31456. See spec here

A nice thing to do might be to mention in a comment that these error codes are specifically for eth_simulate to avoid future confusion.

@lightclient lightclient removed this from the 1.15.11 milestone May 1, 2025
@lightclient lightclient closed this May 1, 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