Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

internal/ethapi: return revert reason for eth_call #21083

Merged
merged 26 commits into from
Jun 8, 2020
Merged

internal/ethapi: return revert reason for eth_call #21083

Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
fd70cfe
internal/ethapi: return revert reason for eth_call
MariusVanDerWijden May 14, 2020
6d13d42
internal/ethapi: moved revert reason logic to doCall
MariusVanDerWijden May 15, 2020
23312e0
accounts/abi/bind/backends: added revert reason logic to simulated ba…
MariusVanDerWijden May 15, 2020
05eb3d5
internal/ethapi: fixed linting error
MariusVanDerWijden May 15, 2020
92bee6b
internal/ethapi: check if require reason can be unpacked
MariusVanDerWijden May 15, 2020
024e5a7
internal/ethapi: better error logic
MariusVanDerWijden May 19, 2020
fbdc6bb
internal/ethapi: simplify logic
MariusVanDerWijden May 21, 2020
9f8a0b5
internal/ethapi: return vmError()
MariusVanDerWijden May 21, 2020
7b6fbca
internal/ethapi: move handling of revert out of docall
MariusVanDerWijden May 22, 2020
ca35628
graphql: removed revert logic until spec change
MariusVanDerWijden May 26, 2020
d77770d
rpc: internal/ethapi: added custom error types
MariusVanDerWijden May 26, 2020
9f13c66
graphql: use returndata instead of return
MariusVanDerWijden May 27, 2020
cc37ce7
accounts/abi/bind/backends: added tests for revert reason
MariusVanDerWijden May 28, 2020
d9b4bc7
internal/ethapi: add errorCode to revert error
MariusVanDerWijden May 28, 2020
304a63c
internal/ethapi: add errorCode of 3 to revertError
MariusVanDerWijden May 28, 2020
693db4d
internal/ethapi: unified estimateGasErrors, simplified logic
MariusVanDerWijden Jun 2, 2020
5048fd1
internal/ethapi: unified handling of errors in DoEstimateGas
MariusVanDerWijden Jun 2, 2020
c7765c0
rpc: print error data field
rjl493456442 Jun 3, 2020
4515f84
accounts/abi/bind/backends: unify simulatedBackend and RPC
rjl493456442 Jun 3, 2020
c9deb8a
internal/ethapi: added binary data to revertError data
MariusVanDerWijden Jun 4, 2020
7cdf4ee
internal/ethapi: refactored unpacking logic into newRevertError
MariusVanDerWijden Jun 4, 2020
53508c5
accounts/abi/bind/backends: fix EstimateGas
rjl493456442 Jun 4, 2020
2d3ef53
accounts, console, internal, rpc: minor error interface cleanups
karalabe Jun 5, 2020
5a62532
Revert "accounts, console, internal, rpc: minor error interface clean…
fjl Jun 5, 2020
e5adaec
re-apply the good parts of 2d3ef53c53
fjl Jun 5, 2020
440d603
rpc: add test for returning server error data from client
fjl Jun 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 43 additions & 10 deletions accounts/abi/bind/backends/simulated.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

// 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
}

// Data returns the hex encoded revert reason.
func (e *revertError) Data() 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()
Expand All @@ -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.
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the execution is reverted with e.g. revert(0,0), so that the execution did revert, but len(res.Revert()) is 0?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the same, I guess, if the revert-data is non-abi-encoded

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If revert(0,0) or the revert data is non-abi encoded, the error from res.Err is returned.
This is "execution reverted" for revert(0,0). I've added a test for this

}

// PendingNonceAt implements PendingStateReader.PendingNonceAt, retrieving
Expand Down Expand Up @@ -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)
Expand Down
Loading