internal/ethapi: expose raw error data

[rjl493456442]

Alternative approach for #30653

[rjl493456442]

@fjl @MariusVanDerWijden Please take a look

[Confucian-e]

Please check PR #30653 first

[MariusVanDerWijden]

WDYT about this? @rjl493456442 @Confucian-e

diff --git a/internal/ethapi/errors.go b/internal/ethapi/errors.go
index ae38061234..272d427c1f 100644
--- a/internal/ethapi/errors.go
+++ b/internal/ethapi/errors.go
@@ -29,10 +29,16 @@ import (
 // revertError is an API error that encompasses an EVM revert with JSON error
 // code and a binary data blob.
 type revertError struct {
-       error
        reason string // revert reason hex encoded
 }
 
+func (e *revertError) Error() string {
+       if e.reason != "" {
+               return fmt.Sprintf("%s: %v", vm.ErrExecutionReverted, e.reason)
+       }
+       return vm.ErrExecutionReverted.Error()
+}
+
 // ErrorCode returns the JSON error code for a revert.
 // See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal
 func (e *revertError) ErrorCode() int {
@@ -46,15 +52,12 @@ func (e *revertError) ErrorData() interface{} {
 
 // newRevertError creates a revertError instance with the provided revert data.
 func newRevertError(revert []byte) *revertError {
-       err := vm.ErrExecutionReverted
-
-       reason, errUnpack := abi.UnpackRevert(revert)
-       if errUnpack == nil {
-               err = fmt.Errorf("%w: %v", vm.ErrExecutionReverted, reason)
+       reason := hexutil.Encode(revert)
+       if r, errUnpack := abi.UnpackRevert(revert); errUnpack == nil {
+               reason = r
        }
        return &revertError{
-               error:  err,
-               reason: hexutil.Encode(revert),
+               reason: reason,
        }
 }

[rjl493456442]

@MariusVanDerWijden sgtm and feel free to push the changes!

[Confucian-e]

Maybe you could read this: #30653 (comment)

[rjl493456442]

@Confucian-e I do see the point you raised.

However, rpc is a very general package and can even be used as a library for other projects.
Specializing the revert error in rpc is quite hacky and I don't think we should go with this direction.

I think we have to wait until the community gradually upgrade their infra to new version.

[Confucian-e]

@Confucian-e I do see the point you raised.

However, rpc is a very general package and can even be used as a library for other projects. Specializing the revert error in rpc is quite hacky and I don't think we should go with this direction.

I think we have to wait until the community gradually upgrade their infra to new version.

Okay, I respect your opinion.

[fjl]

I think we should not expose the revert data in error messages. Doing so will only increase the incentive to resort to error message parsing. You can see what will happen down that path in the example of this spec issue: ethereum/execution-apis#463

Since the revert reason is returned by Geth with a dedicated error code, we should just provide a better way of extracting it on the client side. Here is a PR to do that: #30669