From b0821c0fcc240275f53a11e6a10cf76ae416df13 Mon Sep 17 00:00:00 2001 From: rjl493456442 Date: Mon, 6 Apr 2020 10:44:52 +0800 Subject: [PATCH] accounts, internal: resolve revert reason if possible --- accounts/abi/abi.go | 23 +++++++++++++ accounts/abi/abi_test.go | 32 +++++++++++++++++ accounts/abi/bind/backends/simulated.go | 8 ++++- accounts/abi/bind/backends/simulated_test.go | 2 +- internal/ethapi/api.go | 36 +++++++++++++++++--- 5 files changed, 94 insertions(+), 7 deletions(-) diff --git a/accounts/abi/abi.go b/accounts/abi/abi.go index b9a34a77a0..6bbc01800d 100644 --- a/accounts/abi/abi.go +++ b/accounts/abi/abi.go @@ -24,6 +24,7 @@ import ( "io" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/crypto" ) // The ABI holds information about a contract's context and available @@ -234,3 +235,25 @@ func (abi *ABI) HasFallback() bool { func (abi *ABI) HasReceive() bool { return abi.Receive.Type == Receive } + +// revertSelector is a special function selector for revert reason unpacking. +var revertSelector = crypto.Keccak256([]byte("Error(string)"))[:4] + +// UnpackRevert resolves the abi-encoded revert reason. According to the solidity +// spec https://solidity.readthedocs.io/en/latest/control-structures.html#revert, +// the provided revert reason is abi-encoded as if it were a call to a function +// `Error(string)`. So it's a special tool for it. +func UnpackRevert(data []byte) (string, error) { + if len(data) < 4 { + return "", errors.New("invalid data for unpacking") + } + if !bytes.Equal(data[:4], revertSelector) { + return "", errors.New("invalid data for unpacking") + } + var reason string + typ, _ := NewType("string", "", nil) + if err := (Arguments{{Type: typ}}).Unpack(&reason, data[4:]); err != nil { + return "", err + } + return reason, nil +} diff --git a/accounts/abi/abi_test.go b/accounts/abi/abi_test.go index 509040e5d5..7701168369 100644 --- a/accounts/abi/abi_test.go +++ b/accounts/abi/abi_test.go @@ -19,6 +19,7 @@ package abi import ( "bytes" "encoding/hex" + "errors" "fmt" "math/big" "reflect" @@ -1070,3 +1071,34 @@ func TestUnnamedEventParam(t *testing.T) { t.Fatalf("Could not find input") } } + +func TestUnpackRevert(t *testing.T) { + t.Parallel() + + var cases = []struct { + input string + expect string + expectErr error + }{ + {"", "", errors.New("invalid data for unpacking")}, + {"08c379a1", "", errors.New("invalid data for unpacking")}, + {"08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000d72657665727420726561736f6e00000000000000000000000000000000000000", "revert reason", nil}, + } + for index, c := range cases { + t.Run(fmt.Sprintf("case %d", index), func(t *testing.T) { + got, err := UnpackRevert(common.Hex2Bytes(c.input)) + if c.expectErr != nil { + if err == nil { + t.Fatalf("Expected non-nil error") + } + if err.Error() != c.expectErr.Error() { + t.Fatalf("Expected error mismatch, want %v, got %v", c.expectErr, err) + } + return + } + if c.expect != got { + t.Fatalf("Output mismatch, want %v, got %v", c.expect, got) + } + }) + } +} diff --git a/accounts/abi/bind/backends/simulated.go b/accounts/abi/bind/backends/simulated.go index 2d72ff9e46..aaa9293958 100644 --- a/accounts/abi/bind/backends/simulated.go +++ b/accounts/abi/bind/backends/simulated.go @@ -25,6 +25,7 @@ import ( "time" "github.com/ethereum/go-ethereum" + "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/math" @@ -445,7 +446,12 @@ func (b *SimulatedBackend) EstimateGas(ctx context.Context, call ethereum.CallMs if result != nil && result.Err != vm.ErrOutOfGas { errMsg := fmt.Sprintf("always failing transaction (%v)", result.Err) if len(result.Revert()) > 0 { - errMsg += fmt.Sprintf(" (0x%x)", result.Revert()) + ret, err := abi.UnpackRevert(result.Revert()) + if err != nil { + errMsg += fmt.Sprintf(" (0x%x)", result.Revert()) + } else { + errMsg += fmt.Sprintf(" (%s)", ret) + } } return 0, errors.New(errMsg) } diff --git a/accounts/abi/bind/backends/simulated_test.go b/accounts/abi/bind/backends/simulated_test.go index 3b1c6aaaef..a28f99aeaa 100644 --- a/accounts/abi/bind/backends/simulated_test.go +++ b/accounts/abi/bind/backends/simulated_test.go @@ -411,7 +411,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) (0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000d72657665727420726561736f6e00000000000000000000000000000000000000)")}, + }, 0, errors.New("always failing transaction (execution reverted) (revert reason)")}, {"PureRevert", ethereum.CallMsg{ From: addr, diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index a7c61b267a..c494a8294d 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -27,6 +27,7 @@ import ( "github.com/davecgh/go-spew/spew" "github.com/ethereum/go-ethereum/accounts" + "github.com/ethereum/go-ethereum/accounts/abi" "github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/accounts/scwallet" "github.com/ethereum/go-ethereum/common" @@ -796,7 +797,6 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo if state == nil || err != nil { return nil, err } - // Override the fields of specified contracts before execution. for addr, account := range overrides { // Override account nonce. @@ -882,6 +882,23 @@ func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNrOr 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 e.revert != "" { + errMsg += fmt.Sprintf(" (%s)", e.revert) + } + return errMsg +} + 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 ( @@ -947,14 +964,23 @@ func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash } if failed { if result != nil && result.Err != vm.ErrOutOfGas { - errMsg := fmt.Sprintf("always failing transaction (%v)", result.Err) + var revert string if len(result.Revert()) > 0 { - errMsg += fmt.Sprintf(" (0x%x)", result.Revert()) + ret, err := abi.UnpackRevert(result.Revert()) + if err != nil { + revert = "0x" + common.Bytes2Hex(result.Revert()) + } else { + revert = ret + } + } + return 0, estimateGasError{ + error: "always failing transaction", + vmerr: result.Err, + revert: revert, } - return 0, errors.New(errMsg) } // Otherwise, the specified gas cap is too low - return 0, fmt.Errorf("gas required exceeds allowance (%d)", cap) + return 0, estimateGasError{error: fmt.Sprintf("gas required exceeds allowance (%d)", cap)} } } return hexutil.Uint64(hi), nil