From fd70cfe87ad5dfeb1b0b75d49bba5e7ada694f80 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Thu, 14 May 2020 13:32:45 +0200 Subject: [PATCH 01/26] internal/ethapi: return revert reason for eth_call --- internal/ethapi/api.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index a572e4081c91..15a06e541c48 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -861,6 +861,7 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo if evm.Cancelled() { return nil, fmt.Errorf("execution aborted (timeout = %v)", timeout) } + return result, err } @@ -879,6 +880,13 @@ func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNrOr if err != nil { return nil, err } + if result.Err != nil { + reason, err := abi.UnpackRevert(result.Revert()) + if err != nil { + return result.Return(), err + } + return result.Return(), errors.New(reason) + } return result.Return(), nil } From 6d13d42e713f2bf6558cecd04284d119b19f4e74 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 15 May 2020 09:30:54 +0200 Subject: [PATCH 02/26] internal/ethapi: moved revert reason logic to doCall --- internal/ethapi/api.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 15a06e541c48..03e4fb4cfd8f 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -861,8 +861,15 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo if evm.Cancelled() { return nil, fmt.Errorf("execution aborted (timeout = %v)", timeout) } - - return result, err + // If the result contains a revert reason, unpack and return it. + if result.Err != nil { + reason, err := abi.UnpackRevert(result.Revert()) + if err != nil { + return nil, err + } + return nil, fmt.Errorf("execution reverted: %v", reason) + } + return result, nil } // Call executes the given transaction on the state for the given block number. @@ -880,13 +887,6 @@ func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNrOr if err != nil { return nil, err } - if result.Err != nil { - reason, err := abi.UnpackRevert(result.Revert()) - if err != nil { - return result.Return(), err - } - return result.Return(), errors.New(reason) - } return result.Return(), nil } From 23312e071673a6248fcc78721778ad0a08f5c951 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 15 May 2020 09:31:32 +0200 Subject: [PATCH 03/26] accounts/abi/bind/backends: added revert reason logic to simulated backend --- accounts/abi/bind/backends/simulated.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/accounts/abi/bind/backends/simulated.go b/accounts/abi/bind/backends/simulated.go index 3571595662ec..90c74ce7a65b 100644 --- a/accounts/abi/bind/backends/simulated.go +++ b/accounts/abi/bind/backends/simulated.go @@ -360,6 +360,14 @@ func (b *SimulatedBackend) CallContract(ctx context.Context, call ethereum.CallM if err != nil { return nil, err } + // If the result contains a revert reason, unpack and return it. + if res.Err != nil { + reason, err := abi.UnpackRevert(res.Revert()) + if err != nil { + return nil, err + } + return nil, fmt.Errorf("execution reverted: %v", reason) + } return res.Return(), nil } @@ -373,6 +381,14 @@ func (b *SimulatedBackend) PendingCallContract(ctx context.Context, call ethereu if err != nil { return nil, err } + // If the result contains a revert reason, unpack and return it. + if res.Err != nil { + reason, err := abi.UnpackRevert(res.Revert()) + if err != nil { + return nil, err + } + return nil, fmt.Errorf("execution reverted: %v", reason) + } return res.Return(), nil } From 05eb3d5f0f44852ca0f3e129081820f892bca199 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 15 May 2020 09:47:43 +0200 Subject: [PATCH 04/26] internal/ethapi: fixed linting error --- internal/ethapi/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 03e4fb4cfd8f..2034f75eeae6 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -869,7 +869,7 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo } return nil, fmt.Errorf("execution reverted: %v", reason) } - return result, nil + return result, err } // Call executes the given transaction on the state for the given block number. From 92bee6b03756c6edc07d59194333218dec08d918 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 15 May 2020 10:06:23 +0200 Subject: [PATCH 05/26] internal/ethapi: check if require reason can be unpacked --- accounts/abi/bind/backends/simulated.go | 4 ++-- internal/ethapi/api.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/accounts/abi/bind/backends/simulated.go b/accounts/abi/bind/backends/simulated.go index 90c74ce7a65b..a74dad67edcd 100644 --- a/accounts/abi/bind/backends/simulated.go +++ b/accounts/abi/bind/backends/simulated.go @@ -361,7 +361,7 @@ func (b *SimulatedBackend) CallContract(ctx context.Context, call ethereum.CallM return nil, err } // If the result contains a revert reason, unpack and return it. - if res.Err != nil { + if res.Err != nil && len(res.Revert()) > 0 { reason, err := abi.UnpackRevert(res.Revert()) if err != nil { return nil, err @@ -382,7 +382,7 @@ func (b *SimulatedBackend) PendingCallContract(ctx context.Context, call ethereu return nil, err } // If the result contains a revert reason, unpack and return it. - if res.Err != nil { + if res.Err != nil && len(res.Revert()) > 0 { reason, err := abi.UnpackRevert(res.Revert()) if err != nil { return nil, err diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 2034f75eeae6..e49e2e136a86 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -862,7 +862,7 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo return nil, fmt.Errorf("execution aborted (timeout = %v)", timeout) } // If the result contains a revert reason, unpack and return it. - if result.Err != nil { + if result.Err != nil && len(result.Revert()) > 0 { reason, err := abi.UnpackRevert(result.Revert()) if err != nil { return nil, err From 024e5a74f20642d5a3f0e2140c1137cd26d8cd58 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Tue, 19 May 2020 09:28:18 +0200 Subject: [PATCH 06/26] internal/ethapi: better error logic --- accounts/abi/bind/backends/simulated.go | 18 ++++++++---------- internal/ethapi/api.go | 13 ++++++------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/accounts/abi/bind/backends/simulated.go b/accounts/abi/bind/backends/simulated.go index a74dad67edcd..c55b4944851b 100644 --- a/accounts/abi/bind/backends/simulated.go +++ b/accounts/abi/bind/backends/simulated.go @@ -360,15 +360,14 @@ func (b *SimulatedBackend) CallContract(ctx context.Context, call ethereum.CallM if err != nil { return nil, err } - // If the result contains a revert reason, unpack and return it. + // If the result contains a revert reason, try to unpack and return it. if res.Err != nil && len(res.Revert()) > 0 { reason, err := abi.UnpackRevert(res.Revert()) - if err != nil { - return nil, err + if err == nil { + return nil, fmt.Errorf("execution reverted: %v", reason) } - return nil, fmt.Errorf("execution reverted: %v", reason) } - return res.Return(), nil + return res.Return(), res.Err } // PendingCallContract executes a contract call on the pending state. @@ -381,15 +380,14 @@ func (b *SimulatedBackend) PendingCallContract(ctx context.Context, call ethereu if err != nil { return nil, err } - // If the result contains a revert reason, unpack and return it. + // If the result contains a revert reason, try to unpack and return it. if res.Err != nil && len(res.Revert()) > 0 { reason, err := abi.UnpackRevert(res.Revert()) - if err != nil { - return nil, err + if err == nil { + return nil, fmt.Errorf("execution reverted: %v", reason) } - return nil, fmt.Errorf("execution reverted: %v", reason) } - return res.Return(), nil + return res.Return(), res.Err } // PendingNonceAt implements PendingStateReader.PendingNonceAt, retrieving diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index e49e2e136a86..563250ea1ee2 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -861,15 +861,14 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo if evm.Cancelled() { return nil, fmt.Errorf("execution aborted (timeout = %v)", timeout) } - // If the result contains a revert reason, unpack and return it. - if result.Err != nil && len(result.Revert()) > 0 { - reason, err := abi.UnpackRevert(result.Revert()) - if err != nil { - return nil, err + // If the result contains a revert reason, try to unpack and return it. + if res.Err != nil && len(res.Revert()) > 0 { + reason, err := abi.UnpackRevert(res.Revert()) + if err == nil { + return nil, fmt.Errorf("execution reverted: %v", reason) } - return nil, fmt.Errorf("execution reverted: %v", reason) } - return result, err + return result, res.Err } // Call executes the given transaction on the state for the given block number. From fbdc6bba6608a197521f9563bd05a6363329fdb2 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Thu, 21 May 2020 15:33:27 +0200 Subject: [PATCH 07/26] internal/ethapi: simplify logic --- accounts/abi/bind/backends/simulated.go | 4 ++-- internal/ethapi/api.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/accounts/abi/bind/backends/simulated.go b/accounts/abi/bind/backends/simulated.go index c55b4944851b..78e14cc09daf 100644 --- a/accounts/abi/bind/backends/simulated.go +++ b/accounts/abi/bind/backends/simulated.go @@ -361,7 +361,7 @@ func (b *SimulatedBackend) CallContract(ctx context.Context, call ethereum.CallM return nil, err } // If the result contains a revert reason, try to unpack and return it. - if res.Err != nil && len(res.Revert()) > 0 { + if len(res.Revert()) > 0 { reason, err := abi.UnpackRevert(res.Revert()) if err == nil { return nil, fmt.Errorf("execution reverted: %v", reason) @@ -381,7 +381,7 @@ func (b *SimulatedBackend) PendingCallContract(ctx context.Context, call ethereu return nil, err } // If the result contains a revert reason, try to unpack and return it. - if res.Err != nil && len(res.Revert()) > 0 { + if len(res.Revert()) > 0 { reason, err := abi.UnpackRevert(res.Revert()) if err == nil { return nil, fmt.Errorf("execution reverted: %v", reason) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 563250ea1ee2..6611da650cdc 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -862,13 +862,13 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo return nil, fmt.Errorf("execution aborted (timeout = %v)", timeout) } // If the result contains a revert reason, try to unpack and return it. - if res.Err != nil && len(res.Revert()) > 0 { - reason, err := abi.UnpackRevert(res.Revert()) + if len(result.Revert()) > 0 { + reason, err := abi.UnpackRevert(result.Revert()) if err == nil { return nil, fmt.Errorf("execution reverted: %v", reason) } } - return result, res.Err + return result, result.Err } // Call executes the given transaction on the state for the given block number. From 9f8a0b597e537d4164d61e0d7cef33d6cfc84122 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Thu, 21 May 2020 15:45:50 +0200 Subject: [PATCH 08/26] internal/ethapi: return vmError() --- internal/ethapi/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 6611da650cdc..d6c0fa4c5077 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -868,7 +868,7 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo return nil, fmt.Errorf("execution reverted: %v", reason) } } - return result, result.Err + return result, err } // Call executes the given transaction on the state for the given block number. From 7b6fbcadc311be2b535ea5b6edc27e92d39286f7 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 22 May 2020 10:59:19 +0200 Subject: [PATCH 09/26] internal/ethapi: move handling of revert out of docall --- graphql/graphql.go | 21 +++++++++++++++++++-- internal/ethapi/api.go | 14 +++++++------- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/graphql/graphql.go b/graphql/graphql.go index 9555d8cce254..548000271c3a 100644 --- a/graphql/graphql.go +++ b/graphql/graphql.go @@ -23,6 +23,7 @@ import ( "time" "github.com/ethereum/go-ethereum" + "github.com/ethereum/go-ethereum/accounts/abi" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/core/rawdb" @@ -811,8 +812,16 @@ func (b *Block) Call(ctx context.Context, args struct { if result.Failed() { status = 0 } + data := result.Return() + // If the result contains a revert reason, try to unpack and return it. + if len(result.Revert()) > 0 { + reason, err := abi.UnpackRevert(result.Revert()) + if err == nil { + data = []byte(reason) + } + } return &CallResult{ - data: result.Return(), + data: data, gasUsed: hexutil.Uint64(result.UsedGas), status: status, }, nil @@ -880,8 +889,16 @@ func (p *Pending) Call(ctx context.Context, args struct { if result.Failed() { status = 0 } + data := result.Return() + // If the result contains a revert reason, try to unpack and return it. + if len(result.Revert()) > 0 { + reason, err := abi.UnpackRevert(result.Revert()) + if err == nil { + data = []byte(reason) + } + } return &CallResult{ - data: result.Return(), + data: data, gasUsed: hexutil.Uint64(result.UsedGas), status: status, }, nil diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index d6c0fa4c5077..0a25cdc70c0c 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -861,13 +861,6 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo if evm.Cancelled() { return nil, fmt.Errorf("execution aborted (timeout = %v)", timeout) } - // If the result contains a revert reason, try to unpack and return it. - if len(result.Revert()) > 0 { - reason, err := abi.UnpackRevert(result.Revert()) - if err == nil { - return nil, fmt.Errorf("execution reverted: %v", reason) - } - } return result, err } @@ -886,6 +879,13 @@ func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNrOr if err != nil { return nil, err } + // If the result contains a revert reason, try to unpack and return it. + if len(result.Revert()) > 0 { + reason, err := abi.UnpackRevert(result.Revert()) + if err == nil { + return nil, fmt.Errorf("execution reverted: %v", reason) + } + } return result.Return(), nil } From ca35628cc2bc752e6fbef722799e1704ac73d250 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Tue, 26 May 2020 10:20:24 +0200 Subject: [PATCH 10/26] graphql: removed revert logic until spec change --- graphql/graphql.go | 23 ++++------------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/graphql/graphql.go b/graphql/graphql.go index 548000271c3a..446242aa67d6 100644 --- a/graphql/graphql.go +++ b/graphql/graphql.go @@ -23,7 +23,6 @@ import ( "time" "github.com/ethereum/go-ethereum" - "github.com/ethereum/go-ethereum/accounts/abi" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/core/rawdb" @@ -812,16 +811,9 @@ func (b *Block) Call(ctx context.Context, args struct { if result.Failed() { status = 0 } - data := result.Return() - // If the result contains a revert reason, try to unpack and return it. - if len(result.Revert()) > 0 { - reason, err := abi.UnpackRevert(result.Revert()) - if err == nil { - data = []byte(reason) - } - } + //TODO(rjl493456442, MariusVanDerWijden) return revert reason here once the spec supports an error reason. return &CallResult{ - data: data, + data: result.Return(), gasUsed: hexutil.Uint64(result.UsedGas), status: status, }, nil @@ -889,16 +881,9 @@ func (p *Pending) Call(ctx context.Context, args struct { if result.Failed() { status = 0 } - data := result.Return() - // If the result contains a revert reason, try to unpack and return it. - if len(result.Revert()) > 0 { - reason, err := abi.UnpackRevert(result.Revert()) - if err == nil { - data = []byte(reason) - } - } + //TODO(rjl493456442, MariusVanDerWijden) return revert reason here once the spec supports an error reason. return &CallResult{ - data: data, + data: result.Return(), gasUsed: hexutil.Uint64(result.UsedGas), status: status, }, nil From d77770d96ea72a04ca4621e0516938648a3bac0d Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Tue, 26 May 2020 13:02:33 +0200 Subject: [PATCH 11/26] rpc: internal/ethapi: added custom error types --- console/bridge.go | 2 ++ internal/ethapi/api.go | 28 ++++++++++++++++++++++++++-- rpc/json.go | 4 ++++ rpc/types.go | 6 ++++++ 4 files changed, 38 insertions(+), 2 deletions(-) diff --git a/console/bridge.go b/console/bridge.go index ace8aeeba071..436db6ca5b95 100644 --- a/console/bridge.go +++ b/console/bridge.go @@ -435,6 +435,8 @@ func (b *bridge) Send(call jsre.Call) (goja.Value, error) { } case rpc.Error: setError(resp, err.ErrorCode(), err.Error()) + case rpc.DataError: + resp.Set("error", map[string]interface{}{"code": -32603, "message": err.Error(), "data": err.ErrorData()}) default: setError(resp, -32603, err.Error()) } diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 0a25cdc70c0c..28aec5228c06 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -864,6 +864,21 @@ 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 + errData interface{} // additional data +} + +func (e revertError) Error() string { + return e.err +} + +func (e revertError) ErrorData() interface{} { + return e.errData +} + // Call executes the given transaction on the state for the given block number. // // Additionally, the caller can specify a batch of contract for fields overriding. @@ -883,12 +898,17 @@ func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNrOr if len(result.Revert()) > 0 { reason, err := abi.UnpackRevert(result.Revert()) if err == nil { - return nil, fmt.Errorf("execution reverted: %v", reason) + return nil, &revertError{ + err: "execution reverted", + errData: reason, + } } } - return result.Return(), nil + 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 @@ -906,6 +926,10 @@ func (e estimateGasError) Error() string { 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 ( diff --git a/rpc/json.go b/rpc/json.go index 61631a3d7660..5b48e01e6705 100644 --- a/rpc/json.go +++ b/rpc/json.go @@ -115,6 +115,10 @@ func errorMessage(err error) *jsonrpcMessage { if ok { msg.Error.Code = ec.ErrorCode() } + de, ok := err.(DataError) + if ok { + msg.Error.Data = de.ErrorData() + } return msg } diff --git a/rpc/types.go b/rpc/types.go index dc9248d0feb8..bab1b3957b90 100644 --- a/rpc/types.go +++ b/rpc/types.go @@ -41,6 +41,12 @@ type Error interface { ErrorCode() int // returns the code } +// A DataError contains some data in addition to the error message. +type DataError interface { + Error() string // returns the message + ErrorData() interface{} // returns the error data +} + // ServerCodec implements reading, parsing and writing RPC messages for the server side of // a RPC session. Implementations must be go-routine safe since the codec can be called in // multiple go-routines concurrently. From 9f13c66fbd66013ea1ecacdeb195ba48401672c2 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Wed, 27 May 2020 10:14:40 +0200 Subject: [PATCH 12/26] 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 --- graphql/graphql.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/graphql/graphql.go b/graphql/graphql.go index 446242aa67d6..17a1868a1e13 100644 --- a/graphql/graphql.go +++ b/graphql/graphql.go @@ -813,7 +813,7 @@ func (b *Block) Call(ctx context.Context, args struct { } //TODO(rjl493456442, MariusVanDerWijden) return revert reason here once the spec supports an error reason. return &CallResult{ - data: result.Return(), + data: result.ReturnData, gasUsed: hexutil.Uint64(result.UsedGas), status: status, }, nil @@ -883,7 +883,7 @@ func (p *Pending) Call(ctx context.Context, args struct { } //TODO(rjl493456442, MariusVanDerWijden) return revert reason here once the spec supports an error reason. return &CallResult{ - data: result.Return(), + data: result.ReturnData, gasUsed: hexutil.Uint64(result.UsedGas), status: status, }, nil From cc37ce796b5b9268a585fc7558072a68c9a5108f Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Thu, 28 May 2020 10:36:52 +0200 Subject: [PATCH 13/26] accounts/abi/bind/backends: added tests for revert reason --- accounts/abi/bind/backends/simulated_test.go | 173 +++++++++++-------- 1 file changed, 104 insertions(+), 69 deletions(-) diff --git a/accounts/abi/bind/backends/simulated_test.go b/accounts/abi/bind/backends/simulated_test.go index 25b36d497d6b..190ce005b996 100644 --- a/accounts/abi/bind/backends/simulated_test.go +++ b/accounts/abi/bind/backends/simulated_test.go @@ -20,6 +20,7 @@ import ( "bytes" "context" "errors" + "fmt" "math/big" "strings" "testing" @@ -106,14 +107,18 @@ const deployedCode = `60806040526004361061003b576000357c010000000000000000000000 // expected return value contains "hello world" var expectedReturn = []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 32, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 11, 104, 101, 108, 108, 111, 32, 119, 111, 114, 108, 100, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} -func TestNewSimulatedBackend(t *testing.T) { - testAddr := crypto.PubkeyToAddress(testKey.PublicKey) - expectedBal := big.NewInt(10000000000) - sim := NewSimulatedBackend( +func simTestBackend(testAddr common.Address) *SimulatedBackend { + return NewSimulatedBackend( core.GenesisAlloc{ - testAddr: {Balance: expectedBal}, + testAddr: {Balance: big.NewInt(10000000000)}, }, 10000000, ) +} + +func TestNewSimulatedBackend(t *testing.T) { + testAddr := crypto.PubkeyToAddress(testKey.PublicKey) + expectedBal := big.NewInt(10000000000) + sim := simTestBackend(testAddr) defer sim.Close() if sim.config != params.AllEthashProtocolChanges { @@ -152,11 +157,7 @@ func TestSimulatedBackend_AdjustTime(t *testing.T) { func TestSimulatedBackend_BalanceAt(t *testing.T) { testAddr := crypto.PubkeyToAddress(testKey.PublicKey) expectedBal := big.NewInt(10000000000) - sim := NewSimulatedBackend( - core.GenesisAlloc{ - testAddr: {Balance: expectedBal}, - }, 10000000, - ) + sim := simTestBackend(testAddr) defer sim.Close() bgCtx := context.Background() @@ -229,11 +230,7 @@ func TestSimulatedBackend_BlockByNumber(t *testing.T) { func TestSimulatedBackend_NonceAt(t *testing.T) { testAddr := crypto.PubkeyToAddress(testKey.PublicKey) - sim := NewSimulatedBackend( - core.GenesisAlloc{ - testAddr: {Balance: big.NewInt(10000000000)}, - }, 10000000, - ) + sim := simTestBackend(testAddr) defer sim.Close() bgCtx := context.Background() @@ -283,11 +280,7 @@ func TestSimulatedBackend_NonceAt(t *testing.T) { func TestSimulatedBackend_SendTransaction(t *testing.T) { testAddr := crypto.PubkeyToAddress(testKey.PublicKey) - sim := NewSimulatedBackend( - core.GenesisAlloc{ - testAddr: {Balance: big.NewInt(10000000000)}, - }, 10000000, - ) + sim := simTestBackend(testAddr) defer sim.Close() bgCtx := context.Background() @@ -546,11 +539,7 @@ func TestSimulatedBackend_EstimateGasWithPrice(t *testing.T) { func TestSimulatedBackend_HeaderByHash(t *testing.T) { testAddr := crypto.PubkeyToAddress(testKey.PublicKey) - sim := NewSimulatedBackend( - core.GenesisAlloc{ - testAddr: {Balance: big.NewInt(10000000000)}, - }, 10000000, - ) + sim := simTestBackend(testAddr) defer sim.Close() bgCtx := context.Background() @@ -571,11 +560,7 @@ func TestSimulatedBackend_HeaderByHash(t *testing.T) { func TestSimulatedBackend_HeaderByNumber(t *testing.T) { testAddr := crypto.PubkeyToAddress(testKey.PublicKey) - sim := NewSimulatedBackend( - core.GenesisAlloc{ - testAddr: {Balance: big.NewInt(10000000000)}, - }, 10000000, - ) + sim := simTestBackend(testAddr) defer sim.Close() bgCtx := context.Background() @@ -622,11 +607,7 @@ func TestSimulatedBackend_HeaderByNumber(t *testing.T) { func TestSimulatedBackend_TransactionCount(t *testing.T) { testAddr := crypto.PubkeyToAddress(testKey.PublicKey) - sim := NewSimulatedBackend( - core.GenesisAlloc{ - testAddr: {Balance: big.NewInt(10000000000)}, - }, 10000000, - ) + sim := simTestBackend(testAddr) defer sim.Close() bgCtx := context.Background() currentBlock, err := sim.BlockByNumber(bgCtx, nil) @@ -676,11 +657,7 @@ func TestSimulatedBackend_TransactionCount(t *testing.T) { func TestSimulatedBackend_TransactionInBlock(t *testing.T) { testAddr := crypto.PubkeyToAddress(testKey.PublicKey) - sim := NewSimulatedBackend( - core.GenesisAlloc{ - testAddr: {Balance: big.NewInt(10000000000)}, - }, 10000000, - ) + sim := simTestBackend(testAddr) defer sim.Close() bgCtx := context.Background() @@ -743,11 +720,7 @@ func TestSimulatedBackend_TransactionInBlock(t *testing.T) { func TestSimulatedBackend_PendingNonceAt(t *testing.T) { testAddr := crypto.PubkeyToAddress(testKey.PublicKey) - sim := NewSimulatedBackend( - core.GenesisAlloc{ - testAddr: {Balance: big.NewInt(10000000000)}, - }, 10000000, - ) + sim := simTestBackend(testAddr) defer sim.Close() bgCtx := context.Background() @@ -809,11 +782,7 @@ func TestSimulatedBackend_PendingNonceAt(t *testing.T) { func TestSimulatedBackend_TransactionReceipt(t *testing.T) { testAddr := crypto.PubkeyToAddress(testKey.PublicKey) - sim := NewSimulatedBackend( - core.GenesisAlloc{ - testAddr: {Balance: big.NewInt(10000000000)}, - }, 10000000, - ) + sim := simTestBackend(testAddr) defer sim.Close() bgCtx := context.Background() @@ -859,12 +828,7 @@ func TestSimulatedBackend_SuggestGasPrice(t *testing.T) { func TestSimulatedBackend_PendingCodeAt(t *testing.T) { testAddr := crypto.PubkeyToAddress(testKey.PublicKey) - sim := NewSimulatedBackend( - core.GenesisAlloc{ - testAddr: {Balance: big.NewInt(10000000000)}, - }, - 10000000, - ) + sim := simTestBackend(testAddr) defer sim.Close() bgCtx := context.Background() code, err := sim.CodeAt(bgCtx, testAddr, nil) @@ -900,12 +864,7 @@ func TestSimulatedBackend_PendingCodeAt(t *testing.T) { func TestSimulatedBackend_CodeAt(t *testing.T) { testAddr := crypto.PubkeyToAddress(testKey.PublicKey) - sim := NewSimulatedBackend( - core.GenesisAlloc{ - testAddr: {Balance: big.NewInt(10000000000)}, - }, - 10000000, - ) + sim := simTestBackend(testAddr) defer sim.Close() bgCtx := context.Background() code, err := sim.CodeAt(bgCtx, testAddr, nil) @@ -944,12 +903,7 @@ func TestSimulatedBackend_CodeAt(t *testing.T) { // receipt{status=1 cgas=23949 bloom=00000000004000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000040200000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 logs=[log: b6818c8064f645cd82d99b59a1a267d6d61117ef [75fd880d39c1daf53b6547ab6cb59451fc6452d27caa90e5b6649dd8293b9eed] 000000000000000000000000376c47978271565f56deb45495afa69e59c16ab200000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000000158 9ae378b6d4409eada347a5dc0c180f186cb62dc68fcc0f043425eb917335aa28 0 95d429d309bb9d753954195fe2d69bd140b4ae731b9b5b605c34323de162cf00 0]} func TestSimulatedBackend_PendingAndCallContract(t *testing.T) { testAddr := crypto.PubkeyToAddress(testKey.PublicKey) - sim := NewSimulatedBackend( - core.GenesisAlloc{ - testAddr: {Balance: big.NewInt(10000000000)}, - }, - 10000000, - ) + sim := simTestBackend(testAddr) defer sim.Close() bgCtx := context.Background() @@ -965,7 +919,7 @@ func TestSimulatedBackend_PendingAndCallContract(t *testing.T) { input, err := parsed.Pack("receive", []byte("X")) if err != nil { - t.Errorf("could pack receive function on contract: %v", err) + t.Errorf("could not pack receive function on contract: %v", err) } // make sure you can call the contract in pending state @@ -1005,3 +959,84 @@ func TestSimulatedBackend_PendingAndCallContract(t *testing.T) { t.Errorf("response from calling contract was expected to be 'hello world' instead received %v", string(res)) } } + +func TestSimulatedBackend_CallContractRevert(t *testing.T) { + testAddr := crypto.PubkeyToAddress(testKey.PublicKey) + sim := simTestBackend(testAddr) + defer sim.Close() + bgCtx := context.Background() + + reverterABI := `[{"inputs": [],"name": "noRevert","outputs": [],"stateMutability": "pure","type": "function"},{"inputs": [],"name": "revertASM","outputs": [],"stateMutability": "pure","type": "function"},{"inputs": [],"name": "revertNoString","outputs": [],"stateMutability": "pure","type": "function"},{"inputs": [],"name": "revertString","outputs": [],"stateMutability": "pure","type": "function"}]` + reverterBin := "608060405234801561001057600080fd5b506101d3806100206000396000f3fe608060405234801561001057600080fd5b506004361061004c5760003560e01c80634b409e01146100515780639b340e361461005b5780639bd6103714610065578063b7246fc11461006f575b600080fd5b610059610079565b005b6100636100ca565b005b61006d6100cf565b005b610077610145565b005b60006100c8576040517f08c379a0000000000000000000000000000000000000000000000000000000008152600401808060200182810382526000815260200160200191505060405180910390fd5b565b600080fd5b6000610143576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040180806020018281038252600a8152602001807f736f6d65206572726f720000000000000000000000000000000000000000000081525060200191505060405180910390fd5b565b7f08c379a0000000000000000000000000000000000000000000000000000000006000526020600452600a6024527f736f6d65206572726f720000000000000000000000000000000000000000000060445260646000f3fea2646970667358221220cdd8af0609ec4996b7360c7c780bad5c735740c64b1fffc3445aa12d37f07cb164736f6c63430006070033" + + parsed, err := abi.JSON(strings.NewReader(reverterABI)) + if err != nil { + t.Errorf("could not get code at test addr: %v", err) + } + contractAuth := bind.NewKeyedTransactor(testKey) + addr, _, _, err := bind.DeployContract(contractAuth, parsed, common.FromHex(reverterBin), sim) + if err != nil { + t.Errorf("could not deploy contract: %v", err) + } + + inputs := make(map[string]interface{}, 3) + inputs["revertASM"] = nil + inputs["revertNoString"] = "" + inputs["revertString"] = "some error" + + call := make([]func([]byte) ([]byte, error), 2) + call[0] = func(input []byte) ([]byte, error) { + return sim.PendingCallContract(bgCtx, ethereum.CallMsg{ + From: testAddr, + To: &addr, + Data: input, + }) + } + call[1] = func(input []byte) ([]byte, error) { + return sim.CallContract(bgCtx, ethereum.CallMsg{ + From: testAddr, + To: &addr, + Data: input, + }, nil) + } + + // Run pending calls then commit + for _, cl := range call { + for key, val := range inputs { + input, err := parsed.Pack(key) + if err != nil { + t.Errorf("could not pack %v function on contract: %v", key, err) + } + + res, err := cl(input) + if err == nil { + t.Errorf("call to %v was not reverted", key) + } + if res != nil { + t.Errorf("result from %v was not nil: %v", key, res) + } + if val != nil { + if err.Error() != fmt.Sprintf("execution reverted: %v", val) { + t.Errorf("error was malformed: got %v want %v", err, fmt.Errorf("execution reverted: %v", val)) + } + } else { + // revert(0x0,0x0) + if err.Error() != "execution reverted" { + t.Errorf("error was malformed: got %v want %v", err, "execution reverted") + } + } + } + input, err := parsed.Pack("noRevert") + if err != nil { + t.Errorf("could not pack noRevert function on contract: %v", err) + } + res, err := cl(input) + if err != nil { + t.Error("call to noRevert was reverted") + } + if res == nil { + t.Errorf("result from noRevert was nil") + } + sim.Commit() + } +} From d9b4bc76d4cee1c6d2240faededfc0b7c513ea53 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Thu, 28 May 2020 11:27:20 +0200 Subject: [PATCH 14/26] internal/ethapi: add errorCode to revert error --- console/bridge.go | 8 +++++--- internal/ethapi/api.go | 5 +++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/console/bridge.go b/console/bridge.go index 436db6ca5b95..7249435b0212 100644 --- a/console/bridge.go +++ b/console/bridge.go @@ -434,9 +434,11 @@ func (b *bridge) Send(call jsre.Call) (goja.Value, error) { } } case rpc.Error: - setError(resp, err.ErrorCode(), err.Error()) - case rpc.DataError: - resp.Set("error", map[string]interface{}{"code": -32603, "message": err.Error(), "data": err.ErrorData()}) + errMap := map[string]interface{}{"code": err.ErrorCode(), "message": err.Error()} + if dataErr, ok := err.(rpc.DataError); ok { + errMap["data"] = dataErr.ErrorData() + } + resp.Set("error", errMap) default: setError(resp, -32603, err.Error()) } diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 28aec5228c06..693fcc9f8da3 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -868,6 +868,7 @@ var _ rpc.DataError = (*revertError)(nil) type revertError struct { err string // The error string + code int // optional error code errData interface{} // additional data } @@ -875,6 +876,10 @@ func (e revertError) Error() string { return e.err } +func (e revertError) ErrorCode() int { + return e.code +} + func (e revertError) ErrorData() interface{} { return e.errData } From 304a63c298825400b4664321078b7bb6cb6300d0 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Thu, 28 May 2020 12:41:56 +0200 Subject: [PATCH 15/26] internal/ethapi: add errorCode of 3 to revertError --- internal/ethapi/api.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 693fcc9f8da3..b94611fe41db 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -868,7 +868,6 @@ var _ rpc.DataError = (*revertError)(nil) type revertError struct { err string // The error string - code int // optional error code errData interface{} // additional data } @@ -877,7 +876,9 @@ func (e revertError) Error() string { } func (e revertError) ErrorCode() int { - return e.code + // revert errors are execution errors. + // See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal + return 3 } func (e revertError) ErrorData() interface{} { 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 16/26] 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 190ce005b996..9631bbb6e8c7 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 7249435b0212..c3cdcfaafcad 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 17a1868a1e13..6e29ccc6eb48 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 b94611fe41db..4a412650f49c 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 From 5048fd10280b4556bceb22a849057b925f968452 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Tue, 2 Jun 2020 14:05:16 +0200 Subject: [PATCH 17/26] internal/ethapi: unified handling of errors in DoEstimateGas --- internal/ethapi/api.go | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 4a412650f49c..1cb165adc608 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -999,19 +999,16 @@ func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash } if failed { if result != nil && result.Err != vm.ErrOutOfGas { - var revert string if len(result.Revert()) > 0 { - ret, err := abi.UnpackRevert(result.Revert()) - if err != nil { - revert = hexutil.Encode(result.Revert()) - } else { - revert = ret + reason, err := abi.UnpackRevert(result.Revert()) + if err == nil { + return 0, &revertError{ + error: errors.New("execution reverted"), + errData: reason, + } } } - return 0, revertError{ - error: errors.New("always failing transaction"), - errData: revert, - } + return 0, result.Err } // Otherwise, the specified gas cap is too low return 0, fmt.Errorf("gas required exceeds allowance (%d)", cap) From c7765c0abb4d493b628234ff59819f0fd358dee4 Mon Sep 17 00:00:00 2001 From: rjl493456442 Date: Wed, 3 Jun 2020 13:50:48 +0800 Subject: [PATCH 18/26] rpc: print error data field --- rpc/handler.go | 10 ++++++++-- rpc/json.go | 4 ++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/rpc/handler.go b/rpc/handler.go index c8571ad79539..23023eaca1f2 100644 --- a/rpc/handler.go +++ b/rpc/handler.go @@ -296,10 +296,16 @@ func (h *handler) handleCallMsg(ctx *callProc, msg *jsonrpcMessage) *jsonrpcMess return nil case msg.isCall(): resp := h.handleCall(ctx, msg) + var ctx []interface{} + ctx = append(ctx, "reqid", idForLog{msg.ID}, "t", time.Since(start)) if resp.Error != nil { - h.log.Warn("Served "+msg.Method, "reqid", idForLog{msg.ID}, "t", time.Since(start), "err", resp.Error.Message) + ctx = append(ctx, "err", resp.Error.Message) + if resp.Error.Data != nil { + ctx = append(ctx, "errdata", resp.Error.Data) + } + h.log.Warn("Served "+msg.Method, ctx...) } else { - h.log.Debug("Served "+msg.Method, "reqid", idForLog{msg.ID}, "t", time.Since(start)) + h.log.Debug("Served "+msg.Method, ctx...) } return resp case msg.hasValidID(): diff --git a/rpc/json.go b/rpc/json.go index 5b48e01e6705..3be5d55f48e5 100644 --- a/rpc/json.go +++ b/rpc/json.go @@ -139,6 +139,10 @@ func (err *jsonError) ErrorCode() int { return err.Code } +func (err *jsonError) ErrorData() interface{} { + return err.Data +} + // Conn is a subset of the methods of net.Conn which are sufficient for ServerCodec. type Conn interface { io.ReadWriteCloser From 4515f84958874c44cd826a0a141b70867758cb0d Mon Sep 17 00:00:00 2001 From: rjl493456442 Date: Wed, 3 Jun 2020 14:10:41 +0800 Subject: [PATCH 19/26] accounts/abi/bind/backends: unify simulatedBackend and RPC --- accounts/abi/bind/backends/simulated.go | 39 +++++++++++++++----- accounts/abi/bind/backends/simulated_test.go | 32 +++++++++++----- 2 files changed, 52 insertions(+), 19 deletions(-) diff --git a/accounts/abi/bind/backends/simulated.go b/accounts/abi/bind/backends/simulated.go index 78e14cc09daf..cadf095efe36 100644 --- a/accounts/abi/bind/backends/simulated.go +++ b/accounts/abi/bind/backends/simulated.go @@ -344,6 +344,21 @@ func (b *SimulatedBackend) PendingCodeAt(ctx context.Context, contract common.Ad return b.pendingState.GetCode(contract), nil } +type revertError struct { + error + errData interface{} // additional data +} + +func (e revertError) ErrorCode() int { + // revert errors are execution errors. + // See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal + return 3 +} + +func (e revertError) ErrorData() interface{} { + return e.errData +} + // CallContract executes a contract call. func (b *SimulatedBackend) CallContract(ctx context.Context, call ethereum.CallMsg, blockNumber *big.Int) ([]byte, error) { b.mu.Lock() @@ -364,7 +379,10 @@ func (b *SimulatedBackend) CallContract(ctx context.Context, call ethereum.CallM if len(res.Revert()) > 0 { reason, err := abi.UnpackRevert(res.Revert()) if err == nil { - return nil, fmt.Errorf("execution reverted: %v", reason) + return nil, &revertError{ + error: errors.New("execution reverted"), + errData: reason, + } } } return res.Return(), res.Err @@ -384,7 +402,10 @@ func (b *SimulatedBackend) PendingCallContract(ctx context.Context, call ethereu if len(res.Revert()) > 0 { reason, err := abi.UnpackRevert(res.Revert()) if err == nil { - return nil, fmt.Errorf("execution reverted: %v", reason) + return nil, &revertError{ + error: errors.New("execution reverted"), + errData: reason, + } } } return res.Return(), res.Err @@ -486,16 +507,16 @@ func (b *SimulatedBackend) EstimateGas(ctx context.Context, call ethereum.CallMs } if failed { if result != nil && result.Err != vm.ErrOutOfGas { - errMsg := fmt.Sprintf("always failing transaction (%v)", result.Err) if len(result.Revert()) > 0 { - ret, err := abi.UnpackRevert(result.Revert()) - if err != nil { - errMsg += fmt.Sprintf(" (%#x)", result.Revert()) - } else { - errMsg += fmt.Sprintf(" (%s)", ret) + reason, err := abi.UnpackRevert(result.Revert()) + if err == nil { + return 0, &revertError{ + error: errors.New("execution reverted"), + errData: reason, + } } } - return 0, errors.New(errMsg) + return 0, result.Err } // Otherwise, the specified gas cap is too low return 0, fmt.Errorf("gas required exceeds allowance (%d)", cap) diff --git a/accounts/abi/bind/backends/simulated_test.go b/accounts/abi/bind/backends/simulated_test.go index 9631bbb6e8c7..2d246d353e00 100644 --- a/accounts/abi/bind/backends/simulated_test.go +++ b/accounts/abi/bind/backends/simulated_test.go @@ -20,8 +20,8 @@ import ( "bytes" "context" "errors" - "fmt" "math/big" + "reflect" "strings" "testing" "time" @@ -388,6 +388,7 @@ func TestSimulatedBackend_EstimateGas(t *testing.T) { message ethereum.CallMsg expect uint64 expectError error + expectData interface{} }{ {"plain transfer(valid)", ethereum.CallMsg{ From: addr, @@ -396,7 +397,7 @@ func TestSimulatedBackend_EstimateGas(t *testing.T) { GasPrice: big.NewInt(0), Value: big.NewInt(1), Data: nil, - }, params.TxGas, nil}, + }, params.TxGas, nil, nil}, {"plain transfer(invalid)", ethereum.CallMsg{ From: addr, @@ -405,7 +406,7 @@ func TestSimulatedBackend_EstimateGas(t *testing.T) { GasPrice: big.NewInt(0), Value: big.NewInt(1), Data: nil, - }, 0, errors.New("always failing transaction (execution reverted)")}, + }, 0, errors.New("execution reverted"), nil}, {"Revert", ethereum.CallMsg{ From: addr, @@ -414,7 +415,7 @@ func TestSimulatedBackend_EstimateGas(t *testing.T) { GasPrice: big.NewInt(0), Value: nil, Data: common.Hex2Bytes("d8b98391"), - }, 0, errors.New("always failing transaction (execution reverted) (revert reason)")}, + }, 0, errors.New("execution reverted"), "revert reason"}, {"PureRevert", ethereum.CallMsg{ From: addr, @@ -423,7 +424,7 @@ func TestSimulatedBackend_EstimateGas(t *testing.T) { GasPrice: big.NewInt(0), Value: nil, Data: common.Hex2Bytes("aa8b1d30"), - }, 0, errors.New("always failing transaction (execution reverted)")}, + }, 0, errors.New("execution reverted"), nil}, {"OOG", ethereum.CallMsg{ From: addr, @@ -432,7 +433,7 @@ func TestSimulatedBackend_EstimateGas(t *testing.T) { GasPrice: big.NewInt(0), Value: nil, Data: common.Hex2Bytes("50f6fe34"), - }, 0, errors.New("gas required exceeds allowance (100000)")}, + }, 0, errors.New("gas required exceeds allowance (100000)"), nil}, {"Assert", ethereum.CallMsg{ From: addr, @@ -441,7 +442,7 @@ func TestSimulatedBackend_EstimateGas(t *testing.T) { GasPrice: big.NewInt(0), Value: nil, Data: common.Hex2Bytes("b9b046f9"), - }, 0, errors.New("always failing transaction (invalid opcode: opcode 0xfe not defined)")}, + }, 0, errors.New("invalid opcode: opcode 0xfe not defined"), nil}, {"Valid", ethereum.CallMsg{ From: addr, @@ -450,7 +451,7 @@ func TestSimulatedBackend_EstimateGas(t *testing.T) { GasPrice: big.NewInt(0), Value: nil, Data: common.Hex2Bytes("e09fface"), - }, 21275, nil}, + }, 21275, nil, nil}, } for _, c := range cases { got, err := sim.EstimateGas(context.Background(), c.message) @@ -461,6 +462,13 @@ func TestSimulatedBackend_EstimateGas(t *testing.T) { if c.expectError.Error() != err.Error() { t.Fatalf("Expect error, want %v, got %v", c.expectError, err) } + if c.expectData != nil { + if rerr, ok := err.(*revertError); !ok { + t.Fatalf("Expect revert error, got %T", err) + } else if !reflect.DeepEqual(rerr.ErrorData(), c.expectData) { + t.Fatalf("Error data mismatch, want %v, got %v", c.expectData, rerr.ErrorData()) + } + } continue } if got != c.expect { @@ -1041,8 +1049,12 @@ func TestSimulatedBackend_CallContractRevert(t *testing.T) { t.Errorf("result from %v was not nil: %v", key, res) } if val != nil { - if err.Error() != fmt.Sprintf("execution reverted: %v", val) { - t.Errorf("error was malformed: got %v want %v", err, fmt.Errorf("execution reverted: %v", val)) + rerr, ok := err.(*revertError) + if !ok { + t.Errorf("expect revert error") + } + if !reflect.DeepEqual(rerr.ErrorData(), val) { + t.Errorf("error was malformed: got %v want %v", rerr.ErrorData(), val) } } else { // revert(0x0,0x0) From c9deb8a227aeb9a6407cf3b5d1d8a86d75d4f609 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Thu, 4 Jun 2020 10:40:23 +0200 Subject: [PATCH 20/26] internal/ethapi: added binary data to revertError data --- accounts/abi/bind/backends/simulated.go | 8 ++++---- accounts/abi/bind/backends/simulated_test.go | 4 ++-- internal/ethapi/api.go | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/accounts/abi/bind/backends/simulated.go b/accounts/abi/bind/backends/simulated.go index cadf095efe36..24a6e909e7e4 100644 --- a/accounts/abi/bind/backends/simulated.go +++ b/accounts/abi/bind/backends/simulated.go @@ -380,8 +380,8 @@ func (b *SimulatedBackend) CallContract(ctx context.Context, call ethereum.CallM reason, err := abi.UnpackRevert(res.Revert()) if err == nil { return nil, &revertError{ - error: errors.New("execution reverted"), - errData: reason, + error: fmt.Errorf("execution reverted: %v", reason), + errData: res.Revert(), } } } @@ -403,8 +403,8 @@ func (b *SimulatedBackend) PendingCallContract(ctx context.Context, call ethereu reason, err := abi.UnpackRevert(res.Revert()) if err == nil { return nil, &revertError{ - error: errors.New("execution reverted"), - errData: reason, + error: fmt.Errorf("execution reverted: %v", reason), + errData: res.Revert(), } } } diff --git a/accounts/abi/bind/backends/simulated_test.go b/accounts/abi/bind/backends/simulated_test.go index 2d246d353e00..61ee6b8bae03 100644 --- a/accounts/abi/bind/backends/simulated_test.go +++ b/accounts/abi/bind/backends/simulated_test.go @@ -1053,8 +1053,8 @@ func TestSimulatedBackend_CallContractRevert(t *testing.T) { if !ok { t.Errorf("expect revert error") } - if !reflect.DeepEqual(rerr.ErrorData(), val) { - t.Errorf("error was malformed: got %v want %v", rerr.ErrorData(), val) + if rerr.Error() != "execution reverted: "+val.(string) { + t.Errorf("error was malformed: got %v want %v", rerr.Error(), val) } } else { // revert(0x0,0x0) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 1cb165adc608..5687a12968b1 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -899,8 +899,8 @@ func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNrOr reason, err := abi.UnpackRevert(result.Revert()) if err == nil { return nil, &revertError{ - error: errors.New("execution reverted"), - errData: reason, + error: fmt.Errorf("execution reverted: %v", reason), + errData: result.Revert(), } } } @@ -1003,8 +1003,8 @@ func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash reason, err := abi.UnpackRevert(result.Revert()) if err == nil { return 0, &revertError{ - error: errors.New("execution reverted"), - errData: reason, + error: fmt.Errorf("execution reverted: %v", reason), + errData: result.Revert(), } } } From 7cdf4eea7e0c79a79e4051298aa37a6044d55b46 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Thu, 4 Jun 2020 11:26:00 +0200 Subject: [PATCH 21/26] internal/ethapi: refactored unpacking logic into newRevertError --- accounts/abi/bind/backends/simulated.go | 29 +++++++++++++------------ internal/ethapi/api.go | 28 ++++++++++++------------ 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/accounts/abi/bind/backends/simulated.go b/accounts/abi/bind/backends/simulated.go index 24a6e909e7e4..5d621a24f371 100644 --- a/accounts/abi/bind/backends/simulated.go +++ b/accounts/abi/bind/backends/simulated.go @@ -28,6 +28,7 @@ import ( "github.com/ethereum/go-ethereum/accounts/abi" "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/consensus/ethash" "github.com/ethereum/go-ethereum/core" @@ -344,6 +345,18 @@ func (b *SimulatedBackend) PendingCodeAt(ctx context.Context, contract common.Ad return b.pendingState.GetCode(contract), nil } +func newRevertError(result *core.ExecutionResult) *revertError { + reason, errUnpack := abi.UnpackRevert(result.Revert()) + err := errors.New("execution reverted") + if errUnpack == nil { + err = fmt.Errorf("execution reverted: %v", reason) + } + return &revertError{ + error: err, + errData: hexutil.Encode(result.Revert()), + } +} + type revertError struct { error errData interface{} // additional data @@ -377,13 +390,7 @@ func (b *SimulatedBackend) CallContract(ctx context.Context, call ethereum.CallM } // If the result contains a revert reason, try to unpack and return it. if len(res.Revert()) > 0 { - reason, err := abi.UnpackRevert(res.Revert()) - if err == nil { - return nil, &revertError{ - error: fmt.Errorf("execution reverted: %v", reason), - errData: res.Revert(), - } - } + return nil, newRevertError(res) } return res.Return(), res.Err } @@ -400,13 +407,7 @@ func (b *SimulatedBackend) PendingCallContract(ctx context.Context, call ethereu } // If the result contains a revert reason, try to unpack and return it. if len(res.Revert()) > 0 { - reason, err := abi.UnpackRevert(res.Revert()) - if err == nil { - return nil, &revertError{ - error: fmt.Errorf("execution reverted: %v", reason), - errData: res.Revert(), - } - } + return nil, newRevertError(res) } return res.Return(), res.Err } diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 5687a12968b1..2933fd7a77ce 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -864,6 +864,18 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo return result, err } +func newRevertError(result *core.ExecutionResult) *revertError { + reason, errUnpack := abi.UnpackRevert(result.Revert()) + err := errors.New("execution reverted") + if errUnpack == nil { + err = fmt.Errorf("execution reverted: %v", reason) + } + return &revertError{ + error: err, + errData: hexutil.Encode(result.Revert()), + } +} + type revertError struct { error errData interface{} // additional data @@ -896,13 +908,7 @@ func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNrOr } // If the result contains a revert reason, try to unpack and return it. if len(result.Revert()) > 0 { - reason, err := abi.UnpackRevert(result.Revert()) - if err == nil { - return nil, &revertError{ - error: fmt.Errorf("execution reverted: %v", reason), - errData: result.Revert(), - } - } + return nil, newRevertError(result) } return result.Return(), result.Err } @@ -1000,13 +1006,7 @@ func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash if failed { if result != nil && result.Err != vm.ErrOutOfGas { if len(result.Revert()) > 0 { - reason, err := abi.UnpackRevert(result.Revert()) - if err == nil { - return 0, &revertError{ - error: fmt.Errorf("execution reverted: %v", reason), - errData: result.Revert(), - } - } + return 0, newRevertError(result) } return 0, result.Err } From 53508c5d462792d43339b2976d1086cf26a0b93d Mon Sep 17 00:00:00 2001 From: rjl493456442 Date: Thu, 4 Jun 2020 19:10:24 +0800 Subject: [PATCH 22/26] accounts/abi/bind/backends: fix EstimateGas --- accounts/abi/bind/backends/simulated.go | 8 +------- accounts/abi/bind/backends/simulated_test.go | 2 +- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/accounts/abi/bind/backends/simulated.go b/accounts/abi/bind/backends/simulated.go index 5d621a24f371..2cf7a3810f64 100644 --- a/accounts/abi/bind/backends/simulated.go +++ b/accounts/abi/bind/backends/simulated.go @@ -509,13 +509,7 @@ func (b *SimulatedBackend) EstimateGas(ctx context.Context, call ethereum.CallMs if failed { if result != nil && result.Err != vm.ErrOutOfGas { if len(result.Revert()) > 0 { - reason, err := abi.UnpackRevert(result.Revert()) - if err == nil { - return 0, &revertError{ - error: errors.New("execution reverted"), - errData: reason, - } - } + return 0, newRevertError(result) } return 0, result.Err } diff --git a/accounts/abi/bind/backends/simulated_test.go b/accounts/abi/bind/backends/simulated_test.go index 61ee6b8bae03..6dfd071b86d8 100644 --- a/accounts/abi/bind/backends/simulated_test.go +++ b/accounts/abi/bind/backends/simulated_test.go @@ -415,7 +415,7 @@ func TestSimulatedBackend_EstimateGas(t *testing.T) { GasPrice: big.NewInt(0), Value: nil, Data: common.Hex2Bytes("d8b98391"), - }, 0, errors.New("execution reverted"), "revert reason"}, + }, 0, errors.New("execution reverted: revert reason"), "0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000d72657665727420726561736f6e00000000000000000000000000000000000000"}, {"PureRevert", ethereum.CallMsg{ From: addr, From 2d3ef53c5304e429a04983210a417c1f4e0dafb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Fri, 5 Jun 2020 12:10:03 +0300 Subject: [PATCH 23/26] accounts, console, internal, rpc: minor error interface cleanups --- accounts/abi/bind/backends/simulated.go | 19 ++++++++------ accounts/abi/bind/backends/simulated_test.go | 6 ++--- console/bridge.go | 23 +++++++++-------- internal/ethapi/api.go | 19 ++++++++------ rpc/errors.go | 27 +++++++++++--------- rpc/handler.go | 2 +- rpc/json.go | 16 +++--------- rpc/types.go | 14 +++++----- 8 files changed, 63 insertions(+), 63 deletions(-) diff --git a/accounts/abi/bind/backends/simulated.go b/accounts/abi/bind/backends/simulated.go index 2cf7a3810f64..73e3434acad7 100644 --- a/accounts/abi/bind/backends/simulated.go +++ b/accounts/abi/bind/backends/simulated.go @@ -352,24 +352,27 @@ func newRevertError(result *core.ExecutionResult) *revertError { err = fmt.Errorf("execution reverted: %v", reason) } return &revertError{ - error: err, - errData: hexutil.Encode(result.Revert()), + error: err, + reason: hexutil.Encode(result.Revert()), } } +// revertError is an API error that encompassas an EVM revertal with JSON error +// code and a binary data blob. type revertError struct { error - errData interface{} // additional data + reason string // revert reason hex encoded } -func (e revertError) ErrorCode() int { - // revert errors are execution errors. - // See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal +// Core returns the JSON error code for a revertal. +// See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal +func (e *revertError) Code() int { return 3 } -func (e revertError) ErrorData() interface{} { - return e.errData +// Data returns the hex encoded revert reason. +func (e *revertError) Data() interface{} { + return e.reason } // CallContract executes a contract call. diff --git a/accounts/abi/bind/backends/simulated_test.go b/accounts/abi/bind/backends/simulated_test.go index 6dfd071b86d8..cc425a5f6164 100644 --- a/accounts/abi/bind/backends/simulated_test.go +++ b/accounts/abi/bind/backends/simulated_test.go @@ -463,10 +463,10 @@ func TestSimulatedBackend_EstimateGas(t *testing.T) { t.Fatalf("Expect error, want %v, got %v", c.expectError, err) } if c.expectData != nil { - if rerr, ok := err.(*revertError); !ok { + if err, ok := err.(*revertError); !ok { t.Fatalf("Expect revert error, got %T", err) - } else if !reflect.DeepEqual(rerr.ErrorData(), c.expectData) { - t.Fatalf("Error data mismatch, want %v, got %v", c.expectData, rerr.ErrorData()) + } else if !reflect.DeepEqual(err.Data(), c.expectData) { + t.Fatalf("Error data mismatch, want %v, got %v", c.expectData, err.Data()) } } continue diff --git a/console/bridge.go b/console/bridge.go index c3cdcfaafcad..e3002c79d84b 100644 --- a/console/bridge.go +++ b/console/bridge.go @@ -413,9 +413,7 @@ func (b *bridge) Send(call jsre.Call) (goja.Value, error) { resp.Set("id", req.ID) var result json.RawMessage - err = b.client.Call(&result, req.Method, req.Params...) - switch err := err.(type) { - case nil: + if err = b.client.Call(&result, req.Method, req.Params...); err == nil { if result == nil { // Special case null because it is decoded as an empty // raw message for some reason. @@ -433,18 +431,21 @@ func (b *bridge) Send(call jsre.Call) (goja.Value, error) { resp.Set("result", resultVal) } } - case rpc.Error: - if dataErr, ok := err.(rpc.DataError); ok { - setError(resp, err.ErrorCode(), err.Error(), dataErr.ErrorData()) - } else { - setError(resp, err.ErrorCode(), err.Error(), nil) + } else { + var ( + code int = -32603 + data interface{} = nil + ) + if err, ok := err.(rpc.ErrorWithCode); ok { + code = err.Code() + } + if err, ok := err.(rpc.ErrorWithData); ok { + data = err.Data() } - default: - setError(resp, -32603, err.Error(), nil) + setError(resp, code, err.Error(), data) } resps = append(resps, resp) } - // Return the responses either to the callback (if supplied) // or directly as the return value. var result goja.Value diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 2933fd7a77ce..5b4bb600f3a2 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -871,24 +871,27 @@ func newRevertError(result *core.ExecutionResult) *revertError { err = fmt.Errorf("execution reverted: %v", reason) } return &revertError{ - error: err, - errData: hexutil.Encode(result.Revert()), + error: err, + reason: hexutil.Encode(result.Revert()), } } +// revertError is an API error that encompassas an EVM revertal with JSON error +// code and a binary data blob. type revertError struct { error - errData interface{} // additional data + reason string // revert reason hex encoded } -func (e revertError) ErrorCode() int { - // revert errors are execution errors. - // See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal +// Core returns the JSON error code for a revertal. +// See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal +func (e *revertError) Code() int { return 3 } -func (e revertError) ErrorData() interface{} { - return e.errData +// Data returns the hex encoded revert reason. +func (e *revertError) Data() interface{} { + return e.reason } // Call executes the given transaction on the state for the given block number. diff --git a/rpc/errors.go b/rpc/errors.go index c3aa826cc88a..30b718646282 100644 --- a/rpc/errors.go +++ b/rpc/errors.go @@ -18,20 +18,27 @@ package rpc import "fmt" +var ( + _ ErrorWithCode = new(methodNotFoundError) + _ ErrorWithCode = new(subscriptionNotFoundError) + _ ErrorWithCode = new(parseError) + _ ErrorWithCode = new(invalidRequestError) + _ ErrorWithCode = new(invalidMessageError) + _ ErrorWithCode = new(invalidParamsError) +) + const defaultErrorCode = -32000 type methodNotFoundError struct{ method string } -func (e *methodNotFoundError) ErrorCode() int { return -32601 } - +func (e *methodNotFoundError) Code() int { return -32601 } func (e *methodNotFoundError) Error() string { return fmt.Sprintf("the method %s does not exist/is not available", e.method) } type subscriptionNotFoundError struct{ namespace, subscription string } -func (e *subscriptionNotFoundError) ErrorCode() int { return -32601 } - +func (e *subscriptionNotFoundError) Code() int { return -32601 } func (e *subscriptionNotFoundError) Error() string { return fmt.Sprintf("no %q subscription in %s namespace", e.subscription, e.namespace) } @@ -39,27 +46,23 @@ func (e *subscriptionNotFoundError) Error() string { // Invalid JSON was received by the server. type parseError struct{ message string } -func (e *parseError) ErrorCode() int { return -32700 } - +func (e *parseError) Code() int { return -32700 } func (e *parseError) Error() string { return e.message } // received message isn't a valid request type invalidRequestError struct{ message string } -func (e *invalidRequestError) ErrorCode() int { return -32600 } - +func (e *invalidRequestError) Code() int { return -32600 } func (e *invalidRequestError) Error() string { return e.message } // received message is invalid type invalidMessageError struct{ message string } -func (e *invalidMessageError) ErrorCode() int { return -32700 } - +func (e *invalidMessageError) Code() int { return -32700 } func (e *invalidMessageError) Error() string { return e.message } // unable to decode supplied params, or an invalid number of parameters type invalidParamsError struct{ message string } -func (e *invalidParamsError) ErrorCode() int { return -32602 } - +func (e *invalidParamsError) Code() int { return -32602 } func (e *invalidParamsError) Error() string { return e.message } diff --git a/rpc/handler.go b/rpc/handler.go index 23023eaca1f2..2999c44c6eb7 100644 --- a/rpc/handler.go +++ b/rpc/handler.go @@ -301,7 +301,7 @@ func (h *handler) handleCallMsg(ctx *callProc, msg *jsonrpcMessage) *jsonrpcMess if resp.Error != nil { ctx = append(ctx, "err", resp.Error.Message) if resp.Error.Data != nil { - ctx = append(ctx, "errdata", resp.Error.Data) + ctx = append(ctx, "data", resp.Error.Data) } h.log.Warn("Served "+msg.Method, ctx...) } else { diff --git a/rpc/json.go b/rpc/json.go index 3be5d55f48e5..3f2026301bf9 100644 --- a/rpc/json.go +++ b/rpc/json.go @@ -111,13 +111,13 @@ func errorMessage(err error) *jsonrpcMessage { Code: defaultErrorCode, Message: err.Error(), }} - ec, ok := err.(Error) + ec, ok := err.(ErrorWithCode) if ok { - msg.Error.Code = ec.ErrorCode() + msg.Error.Code = ec.Code() } - de, ok := err.(DataError) + de, ok := err.(ErrorWithData) if ok { - msg.Error.Data = de.ErrorData() + msg.Error.Data = de.Data() } return msg } @@ -135,14 +135,6 @@ func (err *jsonError) Error() string { return err.Message } -func (err *jsonError) ErrorCode() int { - return err.Code -} - -func (err *jsonError) ErrorData() interface{} { - return err.Data -} - // Conn is a subset of the methods of net.Conn which are sufficient for ServerCodec. type Conn interface { io.ReadWriteCloser diff --git a/rpc/types.go b/rpc/types.go index bab1b3957b90..23c59cd56c33 100644 --- a/rpc/types.go +++ b/rpc/types.go @@ -35,16 +35,14 @@ type API struct { Public bool // indication if the methods must be considered safe for public use } -// Error wraps RPC errors, which contain an error code in addition to the message. -type Error interface { - Error() string // returns the message - ErrorCode() int // returns the code +// ErrorWithCode defines an error code in addition to the message. +type ErrorWithCode interface { + Code() int // returns the code } -// A DataError contains some data in addition to the error message. -type DataError interface { - Error() string // returns the message - ErrorData() interface{} // returns the error data +// ErrorWithData defines a data item in addition to the message. +type ErrorWithData interface { + Data() interface{} // returns the error data } // ServerCodec implements reading, parsing and writing RPC messages for the server side of From 5a625323345785b31bb76f9614340939665acf98 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 5 Jun 2020 17:11:09 +0200 Subject: [PATCH 24/26] Revert "accounts, console, internal, rpc: minor error interface cleanups" This reverts commit 2d3ef53c5304e429a04983210a417c1f4e0dafb7. --- accounts/abi/bind/backends/simulated.go | 19 ++++++-------- accounts/abi/bind/backends/simulated_test.go | 6 ++--- console/bridge.go | 23 ++++++++--------- internal/ethapi/api.go | 19 ++++++-------- rpc/errors.go | 27 +++++++++----------- rpc/handler.go | 2 +- rpc/json.go | 16 +++++++++--- rpc/types.go | 14 +++++----- 8 files changed, 63 insertions(+), 63 deletions(-) diff --git a/accounts/abi/bind/backends/simulated.go b/accounts/abi/bind/backends/simulated.go index 73e3434acad7..2cf7a3810f64 100644 --- a/accounts/abi/bind/backends/simulated.go +++ b/accounts/abi/bind/backends/simulated.go @@ -352,27 +352,24 @@ func newRevertError(result *core.ExecutionResult) *revertError { err = fmt.Errorf("execution reverted: %v", reason) } return &revertError{ - error: err, - reason: hexutil.Encode(result.Revert()), + error: err, + errData: hexutil.Encode(result.Revert()), } } -// revertError is an API error that encompassas an EVM revertal with JSON error -// code and a binary data blob. type revertError struct { error - reason string // revert reason hex encoded + errData interface{} // additional data } -// Core returns the JSON error code for a revertal. -// See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal -func (e *revertError) Code() int { +func (e revertError) ErrorCode() int { + // revert errors are execution errors. + // See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal return 3 } -// Data returns the hex encoded revert reason. -func (e *revertError) Data() interface{} { - return e.reason +func (e revertError) ErrorData() interface{} { + return e.errData } // CallContract executes a contract call. diff --git a/accounts/abi/bind/backends/simulated_test.go b/accounts/abi/bind/backends/simulated_test.go index cc425a5f6164..6dfd071b86d8 100644 --- a/accounts/abi/bind/backends/simulated_test.go +++ b/accounts/abi/bind/backends/simulated_test.go @@ -463,10 +463,10 @@ func TestSimulatedBackend_EstimateGas(t *testing.T) { t.Fatalf("Expect error, want %v, got %v", c.expectError, err) } if c.expectData != nil { - if err, ok := err.(*revertError); !ok { + if rerr, ok := err.(*revertError); !ok { t.Fatalf("Expect revert error, got %T", err) - } else if !reflect.DeepEqual(err.Data(), c.expectData) { - t.Fatalf("Error data mismatch, want %v, got %v", c.expectData, err.Data()) + } else if !reflect.DeepEqual(rerr.ErrorData(), c.expectData) { + t.Fatalf("Error data mismatch, want %v, got %v", c.expectData, rerr.ErrorData()) } } continue diff --git a/console/bridge.go b/console/bridge.go index e3002c79d84b..c3cdcfaafcad 100644 --- a/console/bridge.go +++ b/console/bridge.go @@ -413,7 +413,9 @@ func (b *bridge) Send(call jsre.Call) (goja.Value, error) { resp.Set("id", req.ID) var result json.RawMessage - if err = b.client.Call(&result, req.Method, req.Params...); err == nil { + err = b.client.Call(&result, req.Method, req.Params...) + switch err := err.(type) { + case nil: if result == nil { // Special case null because it is decoded as an empty // raw message for some reason. @@ -431,21 +433,18 @@ func (b *bridge) Send(call jsre.Call) (goja.Value, error) { resp.Set("result", resultVal) } } - } else { - var ( - code int = -32603 - data interface{} = nil - ) - if err, ok := err.(rpc.ErrorWithCode); ok { - code = err.Code() - } - if err, ok := err.(rpc.ErrorWithData); ok { - data = err.Data() + case rpc.Error: + if dataErr, ok := err.(rpc.DataError); ok { + setError(resp, err.ErrorCode(), err.Error(), dataErr.ErrorData()) + } else { + setError(resp, err.ErrorCode(), err.Error(), nil) } - setError(resp, code, err.Error(), data) + default: + setError(resp, -32603, err.Error(), nil) } resps = append(resps, resp) } + // Return the responses either to the callback (if supplied) // or directly as the return value. var result goja.Value diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 5b4bb600f3a2..2933fd7a77ce 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -871,27 +871,24 @@ func newRevertError(result *core.ExecutionResult) *revertError { err = fmt.Errorf("execution reverted: %v", reason) } return &revertError{ - error: err, - reason: hexutil.Encode(result.Revert()), + error: err, + errData: hexutil.Encode(result.Revert()), } } -// revertError is an API error that encompassas an EVM revertal with JSON error -// code and a binary data blob. type revertError struct { error - reason string // revert reason hex encoded + errData interface{} // additional data } -// Core returns the JSON error code for a revertal. -// See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal -func (e *revertError) Code() int { +func (e revertError) ErrorCode() int { + // revert errors are execution errors. + // See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal return 3 } -// Data returns the hex encoded revert reason. -func (e *revertError) Data() interface{} { - return e.reason +func (e revertError) ErrorData() interface{} { + return e.errData } // Call executes the given transaction on the state for the given block number. diff --git a/rpc/errors.go b/rpc/errors.go index 30b718646282..c3aa826cc88a 100644 --- a/rpc/errors.go +++ b/rpc/errors.go @@ -18,27 +18,20 @@ package rpc import "fmt" -var ( - _ ErrorWithCode = new(methodNotFoundError) - _ ErrorWithCode = new(subscriptionNotFoundError) - _ ErrorWithCode = new(parseError) - _ ErrorWithCode = new(invalidRequestError) - _ ErrorWithCode = new(invalidMessageError) - _ ErrorWithCode = new(invalidParamsError) -) - const defaultErrorCode = -32000 type methodNotFoundError struct{ method string } -func (e *methodNotFoundError) Code() int { return -32601 } +func (e *methodNotFoundError) ErrorCode() int { return -32601 } + func (e *methodNotFoundError) Error() string { return fmt.Sprintf("the method %s does not exist/is not available", e.method) } type subscriptionNotFoundError struct{ namespace, subscription string } -func (e *subscriptionNotFoundError) Code() int { return -32601 } +func (e *subscriptionNotFoundError) ErrorCode() int { return -32601 } + func (e *subscriptionNotFoundError) Error() string { return fmt.Sprintf("no %q subscription in %s namespace", e.subscription, e.namespace) } @@ -46,23 +39,27 @@ func (e *subscriptionNotFoundError) Error() string { // Invalid JSON was received by the server. type parseError struct{ message string } -func (e *parseError) Code() int { return -32700 } +func (e *parseError) ErrorCode() int { return -32700 } + func (e *parseError) Error() string { return e.message } // received message isn't a valid request type invalidRequestError struct{ message string } -func (e *invalidRequestError) Code() int { return -32600 } +func (e *invalidRequestError) ErrorCode() int { return -32600 } + func (e *invalidRequestError) Error() string { return e.message } // received message is invalid type invalidMessageError struct{ message string } -func (e *invalidMessageError) Code() int { return -32700 } +func (e *invalidMessageError) ErrorCode() int { return -32700 } + func (e *invalidMessageError) Error() string { return e.message } // unable to decode supplied params, or an invalid number of parameters type invalidParamsError struct{ message string } -func (e *invalidParamsError) Code() int { return -32602 } +func (e *invalidParamsError) ErrorCode() int { return -32602 } + func (e *invalidParamsError) Error() string { return e.message } diff --git a/rpc/handler.go b/rpc/handler.go index 2999c44c6eb7..23023eaca1f2 100644 --- a/rpc/handler.go +++ b/rpc/handler.go @@ -301,7 +301,7 @@ func (h *handler) handleCallMsg(ctx *callProc, msg *jsonrpcMessage) *jsonrpcMess if resp.Error != nil { ctx = append(ctx, "err", resp.Error.Message) if resp.Error.Data != nil { - ctx = append(ctx, "data", resp.Error.Data) + ctx = append(ctx, "errdata", resp.Error.Data) } h.log.Warn("Served "+msg.Method, ctx...) } else { diff --git a/rpc/json.go b/rpc/json.go index 3f2026301bf9..3be5d55f48e5 100644 --- a/rpc/json.go +++ b/rpc/json.go @@ -111,13 +111,13 @@ func errorMessage(err error) *jsonrpcMessage { Code: defaultErrorCode, Message: err.Error(), }} - ec, ok := err.(ErrorWithCode) + ec, ok := err.(Error) if ok { - msg.Error.Code = ec.Code() + msg.Error.Code = ec.ErrorCode() } - de, ok := err.(ErrorWithData) + de, ok := err.(DataError) if ok { - msg.Error.Data = de.Data() + msg.Error.Data = de.ErrorData() } return msg } @@ -135,6 +135,14 @@ func (err *jsonError) Error() string { return err.Message } +func (err *jsonError) ErrorCode() int { + return err.Code +} + +func (err *jsonError) ErrorData() interface{} { + return err.Data +} + // Conn is a subset of the methods of net.Conn which are sufficient for ServerCodec. type Conn interface { io.ReadWriteCloser diff --git a/rpc/types.go b/rpc/types.go index 23c59cd56c33..bab1b3957b90 100644 --- a/rpc/types.go +++ b/rpc/types.go @@ -35,14 +35,16 @@ type API struct { Public bool // indication if the methods must be considered safe for public use } -// ErrorWithCode defines an error code in addition to the message. -type ErrorWithCode interface { - Code() int // returns the code +// Error wraps RPC errors, which contain an error code in addition to the message. +type Error interface { + Error() string // returns the message + ErrorCode() int // returns the code } -// ErrorWithData defines a data item in addition to the message. -type ErrorWithData interface { - Data() interface{} // returns the error data +// A DataError contains some data in addition to the error message. +type DataError interface { + Error() string // returns the message + ErrorData() interface{} // returns the error data } // ServerCodec implements reading, parsing and writing RPC messages for the server side of From e5adaec178f083d150fb78453a2e55f89f22accb Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 5 Jun 2020 17:30:13 +0200 Subject: [PATCH 25/26] re-apply the good parts of 2d3ef53c53 --- accounts/abi/bind/backends/simulated.go | 19 ++++++++++-------- accounts/abi/bind/backends/simulated_test.go | 6 +++--- console/bridge.go | 21 ++++++++++---------- internal/ethapi/api.go | 19 ++++++++++-------- 4 files changed, 35 insertions(+), 30 deletions(-) diff --git a/accounts/abi/bind/backends/simulated.go b/accounts/abi/bind/backends/simulated.go index 2cf7a3810f64..0783b586e105 100644 --- a/accounts/abi/bind/backends/simulated.go +++ b/accounts/abi/bind/backends/simulated.go @@ -352,24 +352,27 @@ func newRevertError(result *core.ExecutionResult) *revertError { err = fmt.Errorf("execution reverted: %v", reason) } return &revertError{ - error: err, - errData: hexutil.Encode(result.Revert()), + error: err, + reason: hexutil.Encode(result.Revert()), } } +// revertError is an API error that encompassas an EVM revertal with JSON error +// code and a binary data blob. type revertError struct { error - errData interface{} // additional data + reason string // revert reason hex encoded } -func (e revertError) ErrorCode() int { - // revert errors are execution errors. - // See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal +// ErrorCode returns the JSON error code for a revertal. +// See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal +func (e *revertError) ErrorCode() int { return 3 } -func (e revertError) ErrorData() interface{} { - return e.errData +// ErrorData returns the hex encoded revert reason. +func (e *revertError) ErrorData() interface{} { + return e.reason } // CallContract executes a contract call. diff --git a/accounts/abi/bind/backends/simulated_test.go b/accounts/abi/bind/backends/simulated_test.go index 6dfd071b86d8..d14a88e8bb8a 100644 --- a/accounts/abi/bind/backends/simulated_test.go +++ b/accounts/abi/bind/backends/simulated_test.go @@ -463,10 +463,10 @@ func TestSimulatedBackend_EstimateGas(t *testing.T) { t.Fatalf("Expect error, want %v, got %v", c.expectError, err) } if c.expectData != nil { - if rerr, ok := err.(*revertError); !ok { + if err, ok := err.(*revertError); !ok { t.Fatalf("Expect revert error, got %T", err) - } else if !reflect.DeepEqual(rerr.ErrorData(), c.expectData) { - t.Fatalf("Error data mismatch, want %v, got %v", c.expectData, rerr.ErrorData()) + } else if !reflect.DeepEqual(err.ErrorData(), c.expectData) { + t.Fatalf("Error data mismatch, want %v, got %v", c.expectData, err.ErrorData()) } } continue diff --git a/console/bridge.go b/console/bridge.go index c3cdcfaafcad..995448afb375 100644 --- a/console/bridge.go +++ b/console/bridge.go @@ -413,9 +413,7 @@ func (b *bridge) Send(call jsre.Call) (goja.Value, error) { resp.Set("id", req.ID) var result json.RawMessage - err = b.client.Call(&result, req.Method, req.Params...) - switch err := err.(type) { - case nil: + if err = b.client.Call(&result, req.Method, req.Params...); err == nil { if result == nil { // Special case null because it is decoded as an empty // raw message for some reason. @@ -433,18 +431,19 @@ func (b *bridge) Send(call jsre.Call) (goja.Value, error) { resp.Set("result", resultVal) } } - case rpc.Error: - if dataErr, ok := err.(rpc.DataError); ok { - setError(resp, err.ErrorCode(), err.Error(), dataErr.ErrorData()) - } else { - setError(resp, err.ErrorCode(), err.Error(), nil) + } else { + code := -32603 + var data interface{} + if err, ok := err.(rpc.Error); ok { + code = err.ErrorCode() + } + if err, ok := err.(rpc.DataError); ok { + data = err.ErrorData() } - default: - setError(resp, -32603, err.Error(), nil) + setError(resp, code, err.Error(), data) } resps = append(resps, resp) } - // Return the responses either to the callback (if supplied) // or directly as the return value. var result goja.Value diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 2933fd7a77ce..8e2fe4b3deac 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -871,24 +871,27 @@ func newRevertError(result *core.ExecutionResult) *revertError { err = fmt.Errorf("execution reverted: %v", reason) } return &revertError{ - error: err, - errData: hexutil.Encode(result.Revert()), + error: err, + reason: hexutil.Encode(result.Revert()), } } +// revertError is an API error that encompassas an EVM revertal with JSON error +// code and a binary data blob. type revertError struct { error - errData interface{} // additional data + reason string // revert reason hex encoded } -func (e revertError) ErrorCode() int { - // revert errors are execution errors. - // See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal +// ErrorCode returns the JSON error code for a revertal. +// See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal +func (e *revertError) ErrorCode() int { return 3 } -func (e revertError) ErrorData() interface{} { - return e.errData +// ErrorData returns the hex encoded revert reason. +func (e *revertError) ErrorData() interface{} { + return e.reason } // Call executes the given transaction on the state for the given block number. From 440d60320603970012d73a68473907384e2ffac2 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 5 Jun 2020 17:31:11 +0200 Subject: [PATCH 26/26] rpc: add test for returning server error data from client --- rpc/client_test.go | 27 +++++++++++++++++++++++++++ rpc/errors.go | 9 +++++++++ rpc/server_test.go | 2 +- rpc/testservice_test.go | 10 ++++++++++ 4 files changed, 47 insertions(+), 1 deletion(-) diff --git a/rpc/client_test.go b/rpc/client_test.go index 8a996aeda51e..19c2facb5561 100644 --- a/rpc/client_test.go +++ b/rpc/client_test.go @@ -66,6 +66,33 @@ func TestClientResponseType(t *testing.T) { } } +// This test checks that server-returned errors with code and data come out of Client.Call. +func TestClientErrorData(t *testing.T) { + server := newTestServer() + defer server.Stop() + client := DialInProc(server) + defer client.Close() + + var resp interface{} + err := client.Call(&resp, "test_returnError") + if err == nil { + t.Fatal("expected error") + } + + // Check code. + if e, ok := err.(Error); !ok { + t.Fatalf("client did not return rpc.Error, got %#v", e) + } else if e.ErrorCode() != (testError{}.ErrorCode()) { + t.Fatalf("wrong error code %d, want %d", e.ErrorCode(), testError{}.ErrorCode()) + } + // Check data. + if e, ok := err.(DataError); !ok { + t.Fatalf("client did not return rpc.DataError, got %#v", e) + } else if e.ErrorData() != (testError{}.ErrorData()) { + t.Fatalf("wrong error data %#v, want %#v", e.ErrorData(), testError{}.ErrorData()) + } +} + func TestClientBatchRequest(t *testing.T) { server := newTestServer() defer server.Stop() diff --git a/rpc/errors.go b/rpc/errors.go index c3aa826cc88a..dbfde8b19652 100644 --- a/rpc/errors.go +++ b/rpc/errors.go @@ -18,6 +18,15 @@ package rpc import "fmt" +var ( + _ Error = new(methodNotFoundError) + _ Error = new(subscriptionNotFoundError) + _ Error = new(parseError) + _ Error = new(invalidRequestError) + _ Error = new(invalidMessageError) + _ Error = new(invalidParamsError) +) + const defaultErrorCode = -32000 type methodNotFoundError struct{ method string } diff --git a/rpc/server_test.go b/rpc/server_test.go index 99cca26ddffc..6a2b09e44940 100644 --- a/rpc/server_test.go +++ b/rpc/server_test.go @@ -45,7 +45,7 @@ func TestServerRegisterName(t *testing.T) { t.Fatalf("Expected service calc to be registered") } - wantCallbacks := 8 + wantCallbacks := 9 if len(svc.callbacks) != wantCallbacks { t.Errorf("Expected %d callbacks for service 'service', got %d", wantCallbacks, len(svc.callbacks)) } diff --git a/rpc/testservice_test.go b/rpc/testservice_test.go index 3094c639e61e..6f948a1bac17 100644 --- a/rpc/testservice_test.go +++ b/rpc/testservice_test.go @@ -63,6 +63,12 @@ type echoResult struct { Args *echoArgs } +type testError struct{} + +func (testError) Error() string { return "testError" } +func (testError) ErrorCode() int { return 444 } +func (testError) ErrorData() interface{} { return "testError data" } + func (s *testService) NoArgsRets() {} func (s *testService) Echo(str string, i int, args *echoArgs) echoResult { @@ -99,6 +105,10 @@ func (s *testService) InvalidRets3() (string, string, error) { return "", "", nil } +func (s *testService) ReturnError() error { + return testError{} +} + func (s *testService) CallMeBack(ctx context.Context, method string, args []interface{}) (interface{}, error) { c, ok := ClientFromContext(ctx) if !ok {