ethereum/eth-tester

Have error formatting comply with the JSON-RPC error format

kclowes opened this issue · 3 comments

What was wrong?

eth-tester (or possibly py-evm) returns errors as strings instead of dicts, which makes downstream libraries have to account for a case in testing that isn't quite what they'd see if they were connected to a node.

How can it be fixed?

A JSON-RPC error format can be found here. Convert errors coming from eth-tester to be compliant.

I'm not sure of how the evm behaves in the real world. Is there any need to format errors from py-evm (or the default backend)? Would the evm have its own spec for errors it throws?

I'm considering diving into this one.

Looking at eth_getBalance as an example. The response contains the result encoded data in some formatted json.

In eth-tester, t.get_balance simply returns the actual balance.

Is the intent to mimic the response/error that the Ethereum JSON-RPC spec uses? Should the test methods like t.get_balance return a valid JSON response object?

Looking at this a little bit deeper, I think this actually may be a change that is better in web3 somewhere in here: https://github.com/ethereum/web3.py/blob/main/web3/providers/eth_tester/main.py#L154-L176. It may as straightforward as catching the KeyError on line 158 in web3 and changing the response format from {'error': f"Unknown RPC Endpoint"} to something like:

{
        "jsonrpc": "2.0",
        "error": {
            "code": -32016, # or something, there is a specific code used for this IIRC
            "message": "Unknown RPC Endpoint",
            "data": "...",
        },
        "id": 2987,
    }

and then have something similar in line 160 where we're catching the NotImplementedError. The change that may be beneficial to make in eth-tester is returning the tx data if it's straightforward to do so. It may be on the computation that comes back, but I'm not sure. I believe this is the method that gets called when eth-tester is used from web3: https://github.com/ethereum/eth-tester/blob/master/eth_tester/backends/pyevm/main.py#L779C11-L779C11. Let me know if that brings up more questions!