diff --git a/accounts/abi/bind/backends/simulated.go b/accounts/abi/bind/backends/simulated.go index 357159566..0783b586e 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,36 @@ 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, + 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 + reason string // revert reason hex encoded +} + +// 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 +} + +// ErrorData returns the hex encoded revert reason. +func (e *revertError) ErrorData() interface{} { + return e.reason +} + // CallContract executes a contract call. func (b *SimulatedBackend) CallContract(ctx context.Context, call ethereum.CallMsg, blockNumber *big.Int) ([]byte, error) { b.mu.Lock() @@ -360,7 +391,11 @@ func (b *SimulatedBackend) CallContract(ctx context.Context, call ethereum.CallM if err != nil { return nil, err } - return res.Return(), nil + // If the result contains a revert reason, try to unpack and return it. + if len(res.Revert()) > 0 { + return nil, newRevertError(res) + } + return res.Return(), res.Err } // PendingCallContract executes a contract call on the pending state. @@ -373,7 +408,11 @@ func (b *SimulatedBackend) PendingCallContract(ctx context.Context, call ethereu if err != nil { return nil, err } - return res.Return(), nil + // If the result contains a revert reason, try to unpack and return it. + if len(res.Revert()) > 0 { + return nil, newRevertError(res) + } + return res.Return(), res.Err } // PendingNonceAt implements PendingStateReader.PendingNonceAt, retrieving @@ -472,16 +511,10 @@ 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) - } + return 0, newRevertError(result) } - 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 25b36d497..d14a88e8b 100644 --- a/accounts/abi/bind/backends/simulated_test.go +++ b/accounts/abi/bind/backends/simulated_test.go @@ -21,6 +21,7 @@ import ( "context" "errors" "math/big" + "reflect" "strings" "testing" "time" @@ -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 simTestBackend(testAddr common.Address) *SimulatedBackend { + return NewSimulatedBackend( + core.GenesisAlloc{ + testAddr: {Balance: big.NewInt(10000000000)}, + }, 10000000, + ) +} + func TestNewSimulatedBackend(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() 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() @@ -395,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, @@ -403,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, @@ -412,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, @@ -421,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"), "0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000d72657665727420726561736f6e00000000000000000000000000000000000000"}, {"PureRevert", ethereum.CallMsg{ From: addr, @@ -430,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, @@ -439,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, @@ -448,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, @@ -457,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) @@ -468,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 err, ok := err.(*revertError); !ok { + t.Fatalf("Expect revert error, got %T", err) + } else if !reflect.DeepEqual(err.ErrorData(), c.expectData) { + t.Fatalf("Error data mismatch, want %v, got %v", c.expectData, err.ErrorData()) + } + } continue } if got != c.expect { @@ -546,11 +547,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 +568,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 +615,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 +665,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 +728,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 +790,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 +836,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 +872,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 +911,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 +927,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 +967,113 @@ func TestSimulatedBackend_PendingAndCallContract(t *testing.T) { t.Errorf("response from calling contract was expected to be 'hello world' instead received %v", string(res)) } } + +// 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) + 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 { + rerr, ok := err.(*revertError) + if !ok { + t.Errorf("expect revert error") + } + if rerr.Error() != "execution reverted: "+val.(string) { + t.Errorf("error was malformed: got %v want %v", rerr.Error(), 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() + } +} diff --git a/console/bridge.go b/console/bridge.go index ace8aeeba..995448afb 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. @@ -428,19 +426,24 @@ 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: - setError(resp, err.ErrorCode(), err.Error()) - default: - setError(resp, -32603, err.Error()) + } 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() + } + 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 @@ -456,8 +459,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 9555d8cce..6e29ccc6e 100644 --- a/graphql/graphql.go +++ b/graphql/graphql.go @@ -811,8 +811,9 @@ func (b *Block) Call(ctx context.Context, args struct { if result.Failed() { status = 0 } + return &CallResult{ - data: result.Return(), + data: result.ReturnData, gasUsed: hexutil.Uint64(result.UsedGas), status: status, }, nil @@ -880,8 +881,9 @@ func (p *Pending) Call(ctx context.Context, args struct { if result.Failed() { status = 0 } + return &CallResult{ - data: result.Return(), + data: result.ReturnData, gasUsed: hexutil.Uint64(result.UsedGas), status: status, }, nil diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index a572e4081..8e2fe4b3d 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -864,6 +864,36 @@ 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, + 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 + reason string // revert reason hex encoded +} + +// 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 +} + +// 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. // // Additionally, the caller can specify a batch of contract for fields overriding. @@ -879,24 +909,11 @@ func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNrOr if err != nil { return nil, err } - return result.Return(), 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 the result contains a revert reason, try to unpack and return it. + if len(result.Revert()) > 0 { + return nil, newRevertError(result) } - if e.revert != "" { - errMsg += fmt.Sprintf(" (%s)", e.revert) - } - return errMsg + return result.Return(), result.Err } func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.BlockNumberOrHash, gasCap *big.Int) (hexutil.Uint64, error) { @@ -991,23 +1008,13 @@ 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 - } - } - return 0, estimateGasError{ - error: "always failing transaction", - vmerr: result.Err, - revert: revert, + return 0, newRevertError(result) } + return 0, result.Err } // 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 diff --git a/rpc/client_test.go b/rpc/client_test.go index 8a996aeda..19c2facb5 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 c3aa826cc..dbfde8b19 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/handler.go b/rpc/handler.go index c8571ad79..23023eaca 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 61631a3d7..3be5d55f4 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 } @@ -135,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 diff --git a/rpc/server_test.go b/rpc/server_test.go index 99cca26dd..6a2b09e44 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 3094c639e..6f948a1ba 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 { diff --git a/rpc/types.go b/rpc/types.go index dc9248d0f..bab1b3957 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.