Geth reverts no longer being decoded.

[nlordell]

Due to, recent updates, Geth revert messages aren't getting correctly decoded. This causes previously benign errors to no longer be considered benign and causes a bunch of alerts.

This is because Geth now distinguishes between success and reverts for eth_calls when it previously didn't.

[vkgnosis]

Taking a look. This is also a good opportunity to refactor the revert message detection into a shared crate I guess. Edit: I thought this was about not detecting transactions not being maded because of replaced with higher gas price. This isn't the case. Will still look into the problem.

[nlordell]

Thanks! This helps a lot so I can look into the other Rinkeby issues we are seeing.

[nlordell]

Here is the JSON response if it helps:

{"jsonrpc":"2.0","id":294,"error":{"code":3,"message":"execution reverted: SafeMath: subtraction overflow","data":"0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001e536166654d6174683a207375627472616374696f6e206f766572666c6f770000"}}

[vkgnosis]

the response we used to get from openethereum in this case:

  {
    "jsonrpc": "2.0",
    "id": 1,
    "error": {
          "code": -32015,
          "message": "VM execution error.",
          "data": "Reverted 0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001e536166654d6174683a207375627472616374696f6e206f766572666c6f770000"
    },
  }

the response we get now

  {
      "jsonrpc": "2.0",
      "id": 1,
      "error": {
          "code": 3,
          "message": "execution reverted: SafeMath: subtraction overflow",
          "data": "0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001e536166654d6174683a207375627472616374696f6e206f766572666c6f770000"
      }
  }

I believe they are different because they come from different nodes as our staging environment used to only use openethereum and now also includes infura (which uses geth? https://github.com/ethereum/go-ethereum/blob/fba5a63afe0993da70896c763b4fdfa953e066ff/internal/ethapi/api.go#L893). In ethcontract we have node specific code for getting the revert reason and this code does not handle the new case. To fix this we could change the code here to inspect the web3 rpc error data directly or change ethcontract to also detect reverts from geth which feels like a better solution.

[nlordell]

I think we have new OpenEthereum node versions since today, which is why it changed (just guessing, don't know for sure).

change ethcontract to also detect reverts from geth which feels like a better solution.

We already had code to handle Geth reverts in a hacky way (since the nodes behave differently when encountering reverts in an eth_call): https://github.com/gnosis/ethcontract-rs/blob/40dbdf802f82c2a897d644a59d54533746b42a0b/ethcontract/src/contract/method.rs#L311-L339

I imagine that the way Geth responds to eth_call reverts changed.

But I completely agree that this should probably be fixed in ethcontract and not here.

[nlordell]

Indeed this appears to be the case: https://github.com/ethereum/go-ethereum/pull/21083

It looks like Geth now distinguishes between success and failure for eth_call.

edit: Updated the issue title and description accordingly.