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

Enhance error message for reverting eth_call #2829

Closed
victor-yanev opened this issue Aug 15, 2024 · 2 comments
Closed

Enhance error message for reverting eth_call #2829

victor-yanev opened this issue Aug 15, 2024 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@victor-yanev
Copy link
Contributor

victor-yanev commented Aug 15, 2024

Description

Calling eth_call on a function which reverts with a custom error will return the following:

"error": {
  "code": 3,
  "message": `execution reverted: CONTRACT_REVERT_EXECUTED`,
  "data": "0x858d70bd" // ABI-encoded custom error
}

which:

  1. is too generic and does not give any meaning to the user
  2. is returning an invalid error code (3 is not in the JSON-RPC 2.0 specs https://www.jsonrpc.org/specification#error_object and usually -32000 is used for contract reverts)
  3. is inconsistent with what other ETH clients would return:
    • executing the same against a fork of ETH mainnet through Remix returns an error in the following format:
      "error": {
          "code": -32000,
          "message": "Execution reverted",
          "data": "0x858d70bd"
      }

Steps to reproduce

  1. send a transaction which calls a reverting contract function through eth_sendRawTransaction
  2. send an eth_call request with the unsigned transaction body and the block number from the receipt of the executed transaction

Expected:

  • should return the error code for contract revert and the ABI-encoded revert reason from the contract call in the "data" field of the error.

Example with custom error thrown in solidity:

{
    "jsonrpc": "2.0",
    "id": 1,
    "error": {
        "code": -32000,
        "message": "Execution reverted",
        "data": "0x858d70bd"
    }
}

If the returned reason is encoded as a generic Error(string)(prefixed with 0x08c379a) and not a custom error, we can decode it and append it to the message to be consistent with hyperledger/besu/pull/5706:

{
    "jsonrpc": "2.0",
    "id": 1,
    "error": {
        "code": -32000,
        "message": "Execution reverted: ERC20: transfer amount exceeds balance",
        "data": "0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000002645524332303a207472616e7366657220616d6f756e7420657863656564732062616c616e63650000000000000000000000000000000000000000000000000000"
    }
}

Actual:

  • returns an error with message execution reverted: CONTRACT_REVERT_EXECUTED and the wrong error code 3 instead of -32000 which is the default for contract reverts

Additional context

DEBUG (relay-eth/85 on 5f4099310571): [Request ID: 140ad105-54ff-484c-86fc-2b6958721759] Making eth_call on contract 0xa1fc597dba310b38413afade462d917f0f9ee403 with gas 400000 and call data "0xf8a8fd6d" from "0x17b2b8c63fa35402088640e426c6709a254c7ffb" at blockBlockNumberOrTag: "0x405" using mirror-node.
DEBUG (mirror-node/85 on 5f4099310571): [Request ID: 140ad105-54ff-484c-86fc-2b6958721759] [POST] contracts/call Contract Revert: ( StatusCode: '400', StatusText: '', Detail: 'undefined',Data: '{"_status":{"messages":[{"message":"CONTRACT_REVERT_EXECUTED","detail":"","data":"0x858d70bd"}]}}')
TRACE (relay-eth/85 on 5f4099310571): [Request ID: 140ad105-54ff-484c-86fc-2b6958721759] mirror node eth_call request encountered contract revert. message: CONTRACT_REVERT_EXECUTED, details: , data: 0x858d70bd
DEBUG (relay-eth/85 on 5f4099310571): [Request ID: 140ad105-54ff-484c-86fc-2b6958721759] eth_call response: {"code":3,"message":"execution reverted: CONTRACT_REVERT_EXECUTED","data":"0x858d70bd"}
DEBUG (rpc-server/85 on 5f4099310571): [Request ID: 140ad105-54ff-484c-86fc-2b6958721759] execution reverted: CONTRACT_REVERT_EXECUTED
INFO (rpc-server/85 on 5f4099310571): [Request ID: 140ad105-54ff-484c-86fc-2b6958721759] [POST]: eth_call 200 20 ms

Hedera network

No response

Version

v0.54.0

Operating system

None

@victor-yanev victor-yanev added the bug Something isn't working label Aug 15, 2024
@victor-yanev victor-yanev self-assigned this Aug 15, 2024
@victor-yanev victor-yanev changed the title Calling eth_call throws an error when a contract revert is executed Calling eth_call throws the wrong error code when a contract revert is executed Aug 15, 2024
@victor-yanev victor-yanev moved this from Backlog to Tasks In Progress in Smart Contract Sprint Board Aug 15, 2024
@victor-yanev victor-yanev moved this from Tasks In Progress to Sprint Backlog in Smart Contract Sprint Board Aug 15, 2024
@victor-yanev victor-yanev moved this from Sprint Backlog to Tasks In Progress in Smart Contract Sprint Board Aug 15, 2024
@acuarica
Copy link
Contributor

acuarica commented Aug 15, 2024

Hi @victor-yanev, thanks for sending this. A couple of points

1. In addition to Error(string), Solidity defines another error type[1], Panic(uint256). If we are going to decode standard errors, we could include Panic(uint256) as well. Maybe as part of this issue, on another one, but I believe we should decode both error types.

2. Regarding the error code

is returning an invalid error code (3 is not in the JSON-RPC 2.0 specs https://www.jsonrpc.org/specification#error_object and usually -32000 is used for contract reverts)

AFAIU, regarding error codes, the JSON-RPC says, "The remainder of the space is available for application defined errors.". Meaning 3 is a valid error code. Moreover, both Infura and Alchemy return 3 for eth_call reverts. I believe we should keep that.

3. I think we can downgrade the priority here. Medium or even Low seem good choices. The reason is that this is not a blocker for other issues, for example #2821. Hardhat or Foundry already perform error decoding.


[1] https://docs.soliditylang.org/en/latest/control-structures.html#panic-via-assert-and-error-via-require

@victor-yanev victor-yanev changed the title Calling eth_call throws the wrong error code when a contract revert is executed Enhance error message for reverting eth_call Aug 16, 2024
@victor-yanev
Copy link
Contributor Author

Hi @victor-yanev, thanks for sending this. A couple of points

1. In addition to Error(string), Solidity defines another error type[1], Panic(uint256). If we are going to decode standard errors, we could include Panic(uint256) as well. Maybe as part of this issue, on another one, but I believe we should decode both error types.

2. Regarding the error code

is returning an invalid error code (3 is not in the JSON-RPC 2.0 specs https://www.jsonrpc.org/specification#error_object and usually -32000 is used for contract reverts)

AFAIU, regarding error codes, the JSON-RPC says, "The remainder of the space is available for application defined errors.". Meaning 3 is a valid error code. Moreover, both Infura and Alchemy return 3 for eth_call reverts. I believe we should keep that.

3. I think we can downgrade the priority here. Medium or even Low seem good choices. The reason is that this is not a blocker for other issues, for example #2821. Hardhat or Foundry already perform error decoding.

[1] https://docs.soliditylang.org/en/latest/control-structures.html#panic-via-assert-and-error-via-require

Hmmmm, I think we should close this issue then, as it seems that it's not a real problem

@github-project-automation github-project-automation bot moved this from Tasks In Progress to Done in Smart Contract Sprint Board Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

2 participants