RPC errors formatting mismatch

[drortirosh]

hardhat node formats its error output little different than geth. specifically, the actual data bytes are at .error.data.data instead of .error.data

eth_call error returned from geth:

{
  "jsonrpc": "2.0",
  "id": 1234,
  "error": {
    "code": 3,
    "data": "0x08c379a000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000015756e7374616b652064656c617920746f6f206c6f770000000000000000000000",
    "message": "execution reverted: unstake delay too low"
  }
}

eth_call error returned from hardhat node (for web3_clientVersion = HardhatNetwork/2.11.0/@nomicfoundation/ethereumjs-vm/6.0.0-rc.3")

{
  "jsonrpc": "2.0",
  "id": 1234,
  "error": {
    "code": -32603,
    "message": "Error: VM Exception while processing transaction: reverted with reason string 'unstake delay too low'",
    "data": {
      "message": "Error: VM Exception while processing transaction: reverted with reason string 'unstake delay too low'",
      "data": "0x08c379a000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000015756e7374616b652064656c617920746f6f206c6f770000000000000000000000"
    }
  }
}

[github-actions[bot]]

This issue is also being tracked on Linear.

We use Linear to manage our development process, but we keep the conversations on Github.

LINEAR-ID: b7b7169f-1d9a-4581-8776-96b3f8fb6e85

[alcuadrado]

We are actually considering adding more data to the data field, which is the only json-rpc compliant field that we can control.

Maybe the confusion is that error.data.data could have been error.data.returndata

[fvictorio]

Maybe the confusion is that error.data.data could have been error.data.returndata

IIRC, this was necessary to make it work with ethers.js, but agree that is very confusing.

[fvictorio]

I'm not sure if there's much we can do here. The object we are returning is JSON-RPC compliant:

data A Primitive or Structured value that contains additional information about the error.

(emphasis mine) And even if we wanted to match geth's behavior it would be a big breaking change, and some things in ethers would stop working.

@drortirosh if you are curious, I can try digging in my notes and explain better why exactly we need to do this because I don't remember the exact details. But at the very least, returning an object instead of a string is much more forward compatible, since we can add new fields to that object as needed.

[drortirosh]

Please try to adhere to the de-facto standard (e.g. geth), and add whatever extra data on top of that. reasoning: with the addition custom errors, protocols add more elaborate reverts than the standard Error(string) This requires the client side to perform this parsing, not the provider (which is ok. hardhat provider does its best to populate the "message" field, but it can only do so if it can find the error in the specific contract's abi. a client code might have more information, e.g. as to the inner contract that caused the revert) What I think we should have is a standardized way to find the revert data. currently, it's error.data ?? error.data.data, in order to support both geth and hardhat

-- dror.

On Fri, Sep 9, 2022 at 6:22 PM Patricio Palladino @.***> wrote:

We are actually considering adding more data to the data field, which is the only json-rpc compliant field that we can control.

Maybe the confusion is that error.data.data could have been error.data.returndata

— Reply to this email directly, view it on GitHub https://github.com/NomicFoundation/hardhat/issues/3157#issuecomment-1242116880, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJTY4D7DD43NENGADY7XK4DV5NI2ZANCNFSM6AAAAAAQIEUAKQ . You are receiving this because you authored the thread.Message ID: @.***>

[fvictorio]

Maybe it was a mistake to not do the same thing as geth here (I'm not really sure, I think some other nodes have a different format for example), but changing it now would be a significant breaking change. I don't want to be rude and close this as a wontfix but if I have to be honest it's unlikely we'll change it.