eth_call underspecified return error message

[emlautarom1]

The eth_call method can result in a execution error when, for example, a contract uses a revert operation. The current spec does not define how should the return error look like, making it hard for users to parse the execution failure cause.

The following screenshot perfectly describes the current situation for users:

Source: https://twitter.com/smpalladino/status/1410686918296313869/photo/1.

It would be great if all clients could agree in a single, standardized format for error messages.

[MicahZoltu]

Historically, JSON-RPC errors were reserved for protocol errors and not applied to EVM execution errors. If we want to maintain this pattern, then the JSON-RPC response would be a success with something in the result field. I generally think we should continue following this pattern as upcoming API feature additions like eth_multicall may have a subset of calls that revert and some that are a success so from the protocol level, a success should be returned.

As to what to put in the result field of the JSON-RPC response, I don't think the Ethereum client (JSON-RPC server) should try to do any sort of revert byte decoding. The JSON-RPC server doesn't have enough context to do this correctly so at best it would just be making educated guesses. The client (e.g., web app) has far more context and is in a much better position to properly decode the bytes.

It does seem like there needs to be some way to indicate that the result was erroneous, and prepending some ASCII text to a hex encoded byte array feels like just about the worst way to achieve this. An ideal solution, if we were building something from the ground up, would be to have the JSON-RPC result be an object like { success: false, reason: 'Execution Reverted', data: '0x...' }.

This would allow clients to discriminate on the success and if we want to add other failure modes in the future we can include additional strings to a union of possible values for the reason field which can also be discriminated on.

[emlautarom1]

It does seem like there needs to be some way to indicate that the result was erroneous, and prepending some ASCII text to a hex encoded byte array feels like just about the worst way to achieve this.

100% agree since it forces clients to perform prefix/regex checks.

I don't think the Ethereum client (JSON-RPC server) should try to do any sort of revert byte decoding. The JSON-RPC server doesn't have enough context to do this correctly so at best it would just be making educated guesses.

Servers can decode information from these error messages using well defined formats (see: https://docs.soliditylang.org/en/v0.8.22/control-structures.html#revert). Still, these formats are subject to change, so I think it should be considered an optional feature, maybe by including a message field in case of failure.

For a failure case, the response would look something like:

{
    "success": false,
    "reason": "Execution Reverted",
    "message": "Contract check failed", // optional on a per-client basis (not enforced by spec)
    "data": "0x..."
}

while on success, it would look like:

{
    "success": true,
    "gas": "0x123456"
}

[mfw78]

This is a great initiative to standardise this part of the JSON-RPC spec, and has been the source of many headaches.

while on success, it would look like:

```js
{
    "success": true,
    "gas": "0x123456"
}

Just confirming on the success case that there would also be data, such that:

{
    "success": true,
    "gas": "0x123456",
    "data": "0x..."
}

[s1na]

I agree it'd be good to standardize this between clients, and not only this. The way I'd approach this is to add an error code for "execution reverted" and let clients flexible on the actual wording. But agree with adding a separate field for the decoded revert message.

We can take this further by defining an error code for all failures that can happen as part of EVM execution, e.g. stack over/underflow. All clients necessarily will share the same failures because this is consensus-critical code. Only their error messages could be different. Adding an error code would allow for different messages (e.g. some clients might wish to add context like opcode information).

For context, this is roughly the list of all these faults:

const (
 	VMErrorCodeOutOfGas = 1 + iota
 	VMErrorCodeCodeStoreOutOfGas
 	VMErrorCodeDepth
 	VMErrorCodeInsufficientBalance
 	VMErrorCodeContractAddressCollision
 	VMErrorCodeExecutionReverted
 	VMErrorCodeMaxInitCodeSizeExceeded
 	VMErrorCodeMaxCodeSizeExceeded
 	VMErrorCodeInvalidJump
 	VMErrorCodeWriteProtection
 	VMErrorCodeReturnDataOutOfBounds
 	VMErrorCodeGasUintOverflow
 	VMErrorCodeInvalidCode
 	VMErrorCodeNonceUintOverflow
 	VMErrorCodeStackUnderflow
 	VMErrorCodeStackOverflow
 	VMErrorCodeInvalidOpCode
 )

[MicahZoltu]

We potentially have the opportunity to get this "right" with eth_multicall (#383), if we decide we want to delay release of that feature until we settle on how EVM errors are handled. In the case of eth_multicall, it would not be a JSON-RPC error because a subset of the calls may fail while others succeed and this would be a successful JSON-RPC request/response just as a valid block can have some reverting transactions.

If we decide to implement this in eth_multicall, then I think the right way to do it would be to have the call result (for each call in the multicall) be an object like:

{
	success: true,
	data: `0x${string}`,
} | {
	success: false,
	error_code: Omit<Errors, RevertedError>,
	message: string,
} | {
	success: false,
	error_code: RevertedError,
	message: string,
	data?: `0x${string}`,
}

In the success case, we would just include whatever the call returndata was.

In the failure case, we would have an error_code that contains the specific VM error that occurred (from the list provided by Sina) and if the error was a revert then we would put the revertdata in the data field. The message field would be a freeform string where the client can put any additional information they like such as stack traces, etc.

My preference would be to have the Errors type be a string union, so that it is easy for a developer to read, rather than having numeric codes that then have to be looked up in a table on the client side of the connection.

---

Note: The list provided by @s1na above doesn't include all possible per-transaction errors, like invalid signature and contract-caller. For eth_multicall, I believe we are ignoring both of these so it doesn't matter, but we may want to make sure there are no other ways an individual transaction can fail other than the list provided.

[ryanschneider]

I'm 100% on board with this, but would ask that we "objectize" success and revert rather than keying off the boolean success value to decide what fields are valid.

So a successful call would have a success object value:

{
  // standard JSONRPC id etc elided
  "result": {
     "success":  {
        "data": "0x..",
        "gas": "0x.."
     }
   }
}

While a failed call would contain a revert like:

{ 
  // standard JSONRPC id etc elided
  "result" { 
    "revert": {
        "code": ...
        "reason": ...
    }
}

(I'm not advocating for any specific keys in the revert above since I'm not a domain expert on what data people need when a call reverts, just the overall format).

This allows much cleaner schemas (both in the OpenRPC spec and in client and user code) since success and revert objects can be modeled separately instead of their fields intermingling.

edit: and in the case of multicall result would be an array of [{ "success": ... }, { "revert": ... }] objects.

[fjl]

Recently, this problem has come up in go-ethereum issues several times. I would like to propose that we extend the spec to make error code 3 mandatory for reverts, along with returning the raw revert data as the error data field.

We've been handling reverts like this in Geth for a good while, and it gives users all the information they could want.

[fjl]

We discussed this topic on ACD today, and there was weakly positive feedback to PR #600, which adds error code 3 into the spec.