Cherry pick upstream PR for revert reason in eth_call (upstream #21083)

[oneeman]

Description

This cherry-picks a PR from upstream that adds the revert reason to the error returned from eth_call if there was a revert.

The eth_call behavior in case of a revert was as follows:

• go-ethereum v1.9.13 and earlier: the revert reason is in the return data. web3 checks the return data and generates an error with the revert reason
• go-ethereum v1.9.14: the return data and error are both nil (which is a bug introduced in v1.9.14). web3 thinks the call succeeded
• go-etherem v1.9.15: the return value is nil, and the error includes a data property with the revert reason. web3 checks the error's data and generates and error with the revert reason

On celo-blockchain, v1.2.x and earlier have the pre-1.9.14 behavior, master currently has the v1.9.14, and this PR pulls in the v1.9.15 behavior

Tested

Manually generated an eth_call request which reverts, using contractkit, and logged both the RPC response and contractkit's handling

With this branch:

  rpc:response { jsonrpc: '2.0',
  rpc:response   id: 3,
  rpc:response   error:
  rpc:response    { code: 3,
  rpc:response      message:
  rpc:response       'execution reverted: UptimeLookbackWindow must be within safe range',
  rpc:response      data:
  rpc:response       '0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000002e557074696d654c6f6f6b6261636b57696e646f77206d7573742062652077697468696e20736166652072616e6765000000000000000000000000000000000000' } } +10ms
Error from web3: Error: execution reverted: UptimeLookbackWindow must be within safe range

With master:

  rpc:response { jsonrpc: '2.0', id: 3, result: '0x' } +9ms
No error from web3

With v1.2.3:

  rpc:response { jsonrpc: '2.0',
  rpc:response   id: 3,
  rpc:response   result:
  rpc:response    '0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000002e557074696d654c6f6f6b6261636b57696e646f77206d7573742062652077697468696e20736166652072616e6765000000000000000000000000000000000000' } +10ms
Error from web3: { Error: Your request got reverted with the following reason string: UptimeLookbackWindow must be within safe range

The difference in errors between this PR and v1.2.x (Error: Your request got reverted with the following reason string: vs Error: execution reverted: ) is due to the following code in contractkit: https://github.com/celo-org/celo-monorepo/blob/oneeman/cip35-e2e-tests/packages/sdk/connect/src/utils/rpc-caller.ts#L104-L112

If the error has a message, this code generates a new error with that message, discarding the original error object, which has the data property (which is what web3 then uses if it's available, see https://github.com/ChainSafe/web3.js/blob/2c5a8ec7bbb678b8e718a28fd5fec7709bacde25/packages/web3-core-method/src/index.js#L624-L648).

As a reference, this is the error object before this code in rpc-caller.ts:

{
  "code": 3,
  "message": "execution reverted: Ownable: caller is not the owner",
  "data": "0x08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000204f776e61626c653a2063616c6c6572206973206e6f7420746865206f776e6572"
}

Backwards compatibility

There was both a breaking change and a bug on master when merging in part of v1.9.14. Relative to v1.2.3, this PR results in a breaking change (eth_call now returns an error in case of revert rather than reporting a success with the revert reason in the return data). But this breaking change is already handled in web3 (and, specifically, the version of web3 used in contractkit), so both the old and new will result in an error when it should (see above). However, the message will be different (see above).

[oneeman]

@nambrot @gastonponti I investigated the difference in the resulting error between v1.2.x and master and wrote down my findings in the PR description. If we want it to be the same as it was with v1.2.x we would have to make a change to the contractkit code mentioned there. Please have a look and weigh in on whether such a difference could be problematic.

[mcortesi]

@nambrot @gastonponti I investigated the difference in the resulting error between v1.2.x and master and wrote down my findings in the PR description. If we want it to be the same as it was with v1.2.x we would have to make a change to the contractkit code mentioned there. Please have a look and weigh in on whether such a difference could be problematic.

I think it's fine to leave it as it is... i don't expect for software to rely on error string pattern matching.