Skip to content

Commit 0b3f3be

Browse files
MariusVanDerWijdenrjl493456442karalabefjl
authored
internal/ethapi: return revert reason for eth_call (#21083)
* internal/ethapi: return revert reason for eth_call * internal/ethapi: moved revert reason logic to doCall * accounts/abi/bind/backends: added revert reason logic to simulated backend * internal/ethapi: fixed linting error * internal/ethapi: check if require reason can be unpacked * internal/ethapi: better error logic * internal/ethapi: simplify logic * internal/ethapi: return vmError() * internal/ethapi: move handling of revert out of docall * graphql: removed revert logic until spec change * rpc: internal/ethapi: added custom error types * graphql: use returndata instead of return Return() checks if there is an error. If an error is found, we return nil. For most use cases it can be beneficial to return the output even if there was an error. This code should be changed anyway once the spec supports error reasons in graphql responses * accounts/abi/bind/backends: added tests for revert reason * internal/ethapi: add errorCode to revert error * internal/ethapi: add errorCode of 3 to revertError * internal/ethapi: unified estimateGasErrors, simplified logic * internal/ethapi: unified handling of errors in DoEstimateGas * rpc: print error data field * accounts/abi/bind/backends: unify simulatedBackend and RPC * internal/ethapi: added binary data to revertError data * internal/ethapi: refactored unpacking logic into newRevertError * accounts/abi/bind/backends: fix EstimateGas * accounts, console, internal, rpc: minor error interface cleanups * Revert "accounts, console, internal, rpc: minor error interface cleanups" This reverts commit 2d3ef53. * re-apply the good parts of 2d3ef53 * rpc: add test for returning server error data from client Co-authored-by: rjl493456442 <[email protected]> Co-authored-by: Péter Szilágyi <[email protected]> Co-authored-by: Felix Lange <[email protected]>
1 parent 88125d8 commit 0b3f3be

File tree

12 files changed

+321
-132
lines changed

12 files changed

+321
-132
lines changed

accounts/abi/bind/backends/simulated.go

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/ethereum/go-ethereum/accounts/abi"
2929
"github.com/ethereum/go-ethereum/accounts/abi/bind"
3030
"github.com/ethereum/go-ethereum/common"
31+
"github.com/ethereum/go-ethereum/common/hexutil"
3132
"github.com/ethereum/go-ethereum/common/math"
3233
"github.com/ethereum/go-ethereum/consensus/ethash"
3334
"github.com/ethereum/go-ethereum/core"
@@ -344,6 +345,36 @@ func (b *SimulatedBackend) PendingCodeAt(ctx context.Context, contract common.Ad
344345
return b.pendingState.GetCode(contract), nil
345346
}
346347

348+
func newRevertError(result *core.ExecutionResult) *revertError {
349+
reason, errUnpack := abi.UnpackRevert(result.Revert())
350+
err := errors.New("execution reverted")
351+
if errUnpack == nil {
352+
err = fmt.Errorf("execution reverted: %v", reason)
353+
}
354+
return &revertError{
355+
error: err,
356+
reason: hexutil.Encode(result.Revert()),
357+
}
358+
}
359+
360+
// revertError is an API error that encompassas an EVM revertal with JSON error
361+
// code and a binary data blob.
362+
type revertError struct {
363+
error
364+
reason string // revert reason hex encoded
365+
}
366+
367+
// ErrorCode returns the JSON error code for a revertal.
368+
// See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal
369+
func (e *revertError) ErrorCode() int {
370+
return 3
371+
}
372+
373+
// ErrorData returns the hex encoded revert reason.
374+
func (e *revertError) ErrorData() interface{} {
375+
return e.reason
376+
}
377+
347378
// CallContract executes a contract call.
348379
func (b *SimulatedBackend) CallContract(ctx context.Context, call ethereum.CallMsg, blockNumber *big.Int) ([]byte, error) {
349380
b.mu.Lock()
@@ -360,7 +391,11 @@ func (b *SimulatedBackend) CallContract(ctx context.Context, call ethereum.CallM
360391
if err != nil {
361392
return nil, err
362393
}
363-
return res.Return(), nil
394+
// If the result contains a revert reason, try to unpack and return it.
395+
if len(res.Revert()) > 0 {
396+
return nil, newRevertError(res)
397+
}
398+
return res.Return(), res.Err
364399
}
365400

366401
// PendingCallContract executes a contract call on the pending state.
@@ -373,7 +408,11 @@ func (b *SimulatedBackend) PendingCallContract(ctx context.Context, call ethereu
373408
if err != nil {
374409
return nil, err
375410
}
376-
return res.Return(), nil
411+
// If the result contains a revert reason, try to unpack and return it.
412+
if len(res.Revert()) > 0 {
413+
return nil, newRevertError(res)
414+
}
415+
return res.Return(), res.Err
377416
}
378417

379418
// PendingNonceAt implements PendingStateReader.PendingNonceAt, retrieving
@@ -472,16 +511,10 @@ func (b *SimulatedBackend) EstimateGas(ctx context.Context, call ethereum.CallMs
472511
}
473512
if failed {
474513
if result != nil && result.Err != vm.ErrOutOfGas {
475-
errMsg := fmt.Sprintf("always failing transaction (%v)", result.Err)
476514
if len(result.Revert()) > 0 {
477-
ret, err := abi.UnpackRevert(result.Revert())
478-
if err != nil {
479-
errMsg += fmt.Sprintf(" (%#x)", result.Revert())
480-
} else {
481-
errMsg += fmt.Sprintf(" (%s)", ret)
482-
}
515+
return 0, newRevertError(result)
483516
}
484-
return 0, errors.New(errMsg)
517+
return 0, result.Err
485518
}
486519
// Otherwise, the specified gas cap is too low
487520
return 0, fmt.Errorf("gas required exceeds allowance (%d)", cap)

0 commit comments

Comments
 (0)