From 693db4dc1786266a1f21dcc22026e9a2fa300e55 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Tue, 2 Jun 2020 10:00:17 +0200 Subject: [PATCH] internal/ethapi: unified estimateGasErrors, simplified logic --- accounts/abi/bind/backends/simulated_test.go | 25 ++++++++++++ console/bridge.go | 20 ++++++---- graphql/graphql.go | 4 +- internal/ethapi/api.go | 42 +++----------------- 4 files changed, 46 insertions(+), 45 deletions(-) diff --git a/accounts/abi/bind/backends/simulated_test.go b/accounts/abi/bind/backends/simulated_test.go index 190ce005b9..9631bbb6e8 100644 --- a/accounts/abi/bind/backends/simulated_test.go +++ b/accounts/abi/bind/backends/simulated_test.go @@ -960,6 +960,31 @@ func TestSimulatedBackend_PendingAndCallContract(t *testing.T) { } } +// This test is based on the following contract: +/* +contract Reverter { + function revertString() public pure{ + require(false, "some error"); + } + function revertNoString() public pure { + require(false, ""); + } + function revertASM() public pure { + assembly { + revert(0x0, 0x0) + } + } + function noRevert() public pure { + assembly { + // Assembles something that looks like require(false, "some error") but is not reverted + mstore(0x0, 0x08c379a000000000000000000000000000000000000000000000000000000000) + mstore(0x4, 0x0000000000000000000000000000000000000000000000000000000000000020) + mstore(0x24, 0x000000000000000000000000000000000000000000000000000000000000000a) + mstore(0x44, 0x736f6d65206572726f7200000000000000000000000000000000000000000000) + return(0x0, 0x64) + } + } +}*/ func TestSimulatedBackend_CallContractRevert(t *testing.T) { testAddr := crypto.PubkeyToAddress(testKey.PublicKey) sim := simTestBackend(testAddr) diff --git a/console/bridge.go b/console/bridge.go index 7249435b02..c3cdcfaafc 100644 --- a/console/bridge.go +++ b/console/bridge.go @@ -428,19 +428,19 @@ func (b *bridge) Send(call jsre.Call) (goja.Value, error) { } resultVal, err := parse(goja.Null(), call.VM.ToValue(string(result))) if err != nil { - setError(resp, -32603, err.Error()) + setError(resp, -32603, err.Error(), nil) } else { resp.Set("result", resultVal) } } case rpc.Error: - errMap := map[string]interface{}{"code": err.ErrorCode(), "message": err.Error()} if dataErr, ok := err.(rpc.DataError); ok { - errMap["data"] = dataErr.ErrorData() + setError(resp, err.ErrorCode(), err.Error(), dataErr.ErrorData()) + } else { + setError(resp, err.ErrorCode(), err.Error(), nil) } - resp.Set("error", errMap) default: - setError(resp, -32603, err.Error()) + setError(resp, -32603, err.Error(), nil) } resps = append(resps, resp) } @@ -460,8 +460,14 @@ func (b *bridge) Send(call jsre.Call) (goja.Value, error) { return result, nil } -func setError(resp *goja.Object, code int, msg string) { - resp.Set("error", map[string]interface{}{"code": code, "message": msg}) +func setError(resp *goja.Object, code int, msg string, data interface{}) { + err := make(map[string]interface{}) + err["code"] = code + err["message"] = msg + if data != nil { + err["data"] = data + } + resp.Set("error", err) } // isNumber returns true if input value is a JS number. diff --git a/graphql/graphql.go b/graphql/graphql.go index 17a1868a1e..6e29ccc6eb 100644 --- a/graphql/graphql.go +++ b/graphql/graphql.go @@ -811,7 +811,7 @@ func (b *Block) Call(ctx context.Context, args struct { if result.Failed() { status = 0 } - //TODO(rjl493456442, MariusVanDerWijden) return revert reason here once the spec supports an error reason. + return &CallResult{ data: result.ReturnData, gasUsed: hexutil.Uint64(result.UsedGas), @@ -881,7 +881,7 @@ func (p *Pending) Call(ctx context.Context, args struct { if result.Failed() { status = 0 } - //TODO(rjl493456442, MariusVanDerWijden) return revert reason here once the spec supports an error reason. + return &CallResult{ data: result.ReturnData, gasUsed: hexutil.Uint64(result.UsedGas), diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index b94611fe41..4a412650f4 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -864,17 +864,11 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo return result, err } -var _ rpc.DataError = (*revertError)(nil) - type revertError struct { - err string // The error string + error errData interface{} // additional data } -func (e revertError) Error() string { - return e.err -} - func (e revertError) ErrorCode() int { // revert errors are execution errors. // See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal @@ -905,7 +899,7 @@ func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNrOr reason, err := abi.UnpackRevert(result.Revert()) if err == nil { return nil, &revertError{ - err: "execution reverted", + error: errors.New("execution reverted"), errData: reason, } } @@ -913,29 +907,6 @@ func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNrOr return result.Return(), result.Err } -var _ rpc.DataError = (*estimateGasError)(nil) - -type estimateGasError struct { - error string // Concrete error type if it's failed to estimate gas usage - vmerr error // Additional field, it's non-nil if the given transaction is invalid - revert string // Additional field, it's non-empty if the transaction is reverted and reason is provided -} - -func (e estimateGasError) Error() string { - errMsg := e.error - if e.vmerr != nil { - errMsg += fmt.Sprintf(" (%v)", e.vmerr) - } - if e.revert != "" { - errMsg += fmt.Sprintf(" (%s)", e.revert) - } - return errMsg -} - -func (e estimateGasError) ErrorData() interface{} { - return e.revert -} - func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.BlockNumberOrHash, gasCap *big.Int) (hexutil.Uint64, error) { // Binary search the gas requirement, as it may be higher than the amount used var ( @@ -1037,14 +1008,13 @@ func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash revert = ret } } - return 0, estimateGasError{ - error: "always failing transaction", - vmerr: result.Err, - revert: revert, + return 0, revertError{ + error: errors.New("always failing transaction"), + errData: revert, } } // Otherwise, the specified gas cap is too low - return 0, estimateGasError{error: fmt.Sprintf("gas required exceeds allowance (%d)", cap)} + return 0, fmt.Errorf("gas required exceeds allowance (%d)", cap) } } return hexutil.Uint64(hi), nil