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.

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

fix(evm): include revert reason in jsonrpc return #2855

Merged
merged 1 commit into from
Sep 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
51 changes: 51 additions & 0 deletions packages/evm/evmerrors/reverterror.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package evmerrors

import (
"bytes"
"encoding/hex"
"errors"

"github.com/ethereum/go-ethereum/accounts/abi"

"github.com/iotaledger/wasp/packages/isc"
"github.com/iotaledger/wasp/packages/vm"
)

func IsExecutionReverted(err error) bool {
if err == nil {
return false
}
return isc.VMErrorIs(err, vm.ErrEVMExecutionReverted)
}

func ExtractRevertData(err error) ([]byte, error) {
if err == nil {
return nil, errors.New("expected err != nil")
}
if !IsExecutionReverted(err) {
return nil, nil
}
var customError *isc.VMError
ok := errors.As(err, &customError)
if !ok {
return nil, errors.New("could not extract VMError")
}
if len(customError.Params()) != 1 {
return nil, errors.New("expected len(params) == 1")
}
revertDataHex, ok := customError.Params()[0].(string)
if !ok {
return nil, errors.New("expected params[0] to be string")
}
return hex.DecodeString(revertDataHex)
}

func UnpackCustomError(data []byte, abiError abi.Error) ([]any, error) {
if len(data) < 4 {
return nil, errors.New("invalid data for unpacking")
}
if !bytes.Equal(data[:4], abiError.ID[:4]) {
return nil, errors.New("invalid error selector")
}
return abiError.Inputs.Unpack(data[4:])
}
2 changes: 1 addition & 1 deletion packages/evm/evmtest/ISCTest.abi
Original file line number Diff line number Diff line change
@@ -1 +1 @@
[{"anonymous":false,"inputs":[{"indexed":false,"internalType":"bytes32","name":"entropy","type":"bytes32"}],"name":"EntropyEvent","type":"event"},{"anonymous":false,"inputs":[],"name":"LoopEvent","type":"event"},{"anonymous":false,"inputs":[{"components":[{"internalType":"bytes","name":"data","type":"bytes"}],"indexed":false,"internalType":"struct ISCRequestID","name":"reqID","type":"tuple"}],"name":"RequestIDEvent","type":"event"},{"anonymous":false,"inputs":[{"components":[{"internalType":"bytes","name":"data","type":"bytes"}],"indexed":false,"internalType":"struct ISCAgentID","name":"sender","type":"tuple"}],"name":"SenderAccountEvent","type":"event"},{"inputs":[],"name":"TokensForGas","outputs":[{"internalType":"uint64","name":"","type":"uint64"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"callInccounter","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[],"name":"emitEntropy","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[],"name":"emitRequestID","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[],"name":"emitSenderAccount","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[],"name":"getChainID","outputs":[{"internalType":"ISCChainID","name":"","type":"bytes32"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"loopWithGasLeft","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[],"name":"makeISCPanic","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"internalType":"uint32","name":"foundrySN","type":"uint32"},{"internalType":"uint256","name":"amount","type":"uint256"},{"internalType":"uint64","name":"storageDeposit","type":"uint64"}],"name":"mint","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"components":[{"internalType":"bytes","name":"data","type":"bytes"}],"internalType":"struct ISCAgentID","name":"targetAgentID","type":"tuple"},{"components":[{"internalType":"uint64","name":"baseTokens","type":"uint64"},{"components":[{"components":[{"internalType":"bytes","name":"data","type":"bytes"}],"internalType":"struct NativeTokenID","name":"ID","type":"tuple"},{"internalType":"uint256","name":"amount","type":"uint256"}],"internalType":"struct NativeToken[]","name":"nativeTokens","type":"tuple[]"},{"internalType":"NFTID[]","name":"nfts","type":"bytes32[]"}],"internalType":"struct ISCAssets","name":"allowance","type":"tuple"}],"name":"moveToAccount","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"components":[{"internalType":"bytes","name":"data","type":"bytes"}],"internalType":"struct L1Address","name":"receiver","type":"tuple"},{"internalType":"uint64","name":"baseTokens","type":"uint64"}],"name":"sendBaseTokens","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"components":[{"internalType":"bytes","name":"data","type":"bytes"}],"internalType":"struct L1Address","name":"receiver","type":"tuple"},{"internalType":"NFTID","name":"id","type":"bytes32"},{"internalType":"uint64","name":"storageDeposit","type":"uint64"}],"name":"sendNFT","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"internalType":"address payable","name":"to","type":"address"},{"internalType":"uint256","name":"amount","type":"uint256"}],"name":"sendTo","outputs":[],"stateMutability":"payable","type":"function"},{"inputs":[],"name":"testCallViewCaller","outputs":[{"internalType":"bytes","name":"","type":"bytes"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"testRevertReason","outputs":[],"stateMutability":"pure","type":"function"},{"inputs":[{"internalType":"address payable","name":"beneficiary","type":"address"}],"name":"testSelfDestruct","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[],"name":"testStackOverflow","outputs":[],"stateMutability":"view","type":"function"},{"inputs":[],"name":"testStaticCall","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"internalType":"string","name":"s","type":"string"}],"name":"triggerEvent","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"internalType":"string","name":"s","type":"string"}],"name":"triggerEventFail","outputs":[],"stateMutability":"nonpayable","type":"function"}]
[{"inputs":[{"internalType":"uint8","name":"","type":"uint8"}],"name":"CustomError","type":"error"},{"anonymous":false,"inputs":[{"indexed":false,"internalType":"bytes32","name":"entropy","type":"bytes32"}],"name":"EntropyEvent","type":"event"},{"anonymous":false,"inputs":[],"name":"LoopEvent","type":"event"},{"anonymous":false,"inputs":[{"components":[{"internalType":"bytes","name":"data","type":"bytes"}],"indexed":false,"internalType":"struct ISCRequestID","name":"reqID","type":"tuple"}],"name":"RequestIDEvent","type":"event"},{"anonymous":false,"inputs":[{"components":[{"internalType":"bytes","name":"data","type":"bytes"}],"indexed":false,"internalType":"struct ISCAgentID","name":"sender","type":"tuple"}],"name":"SenderAccountEvent","type":"event"},{"inputs":[],"name":"TokensForGas","outputs":[{"internalType":"uint64","name":"","type":"uint64"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"callInccounter","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[],"name":"emitEntropy","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[],"name":"emitRequestID","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[],"name":"emitSenderAccount","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[],"name":"getChainID","outputs":[{"internalType":"ISCChainID","name":"","type":"bytes32"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"loopWithGasLeft","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[],"name":"makeISCPanic","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"internalType":"uint32","name":"foundrySN","type":"uint32"},{"internalType":"uint256","name":"amount","type":"uint256"},{"internalType":"uint64","name":"storageDeposit","type":"uint64"}],"name":"mint","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"components":[{"internalType":"bytes","name":"data","type":"bytes"}],"internalType":"struct ISCAgentID","name":"targetAgentID","type":"tuple"},{"components":[{"internalType":"uint64","name":"baseTokens","type":"uint64"},{"components":[{"components":[{"internalType":"bytes","name":"data","type":"bytes"}],"internalType":"struct NativeTokenID","name":"ID","type":"tuple"},{"internalType":"uint256","name":"amount","type":"uint256"}],"internalType":"struct NativeToken[]","name":"nativeTokens","type":"tuple[]"},{"internalType":"NFTID[]","name":"nfts","type":"bytes32[]"}],"internalType":"struct ISCAssets","name":"allowance","type":"tuple"}],"name":"moveToAccount","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[],"name":"revertWithCustomError","outputs":[],"stateMutability":"pure","type":"function"},{"inputs":[{"components":[{"internalType":"bytes","name":"data","type":"bytes"}],"internalType":"struct L1Address","name":"receiver","type":"tuple"},{"internalType":"uint64","name":"baseTokens","type":"uint64"}],"name":"sendBaseTokens","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"components":[{"internalType":"bytes","name":"data","type":"bytes"}],"internalType":"struct L1Address","name":"receiver","type":"tuple"},{"internalType":"NFTID","name":"id","type":"bytes32"},{"internalType":"uint64","name":"storageDeposit","type":"uint64"}],"name":"sendNFT","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"internalType":"address payable","name":"to","type":"address"},{"internalType":"uint256","name":"amount","type":"uint256"}],"name":"sendTo","outputs":[],"stateMutability":"payable","type":"function"},{"inputs":[],"name":"testCallViewCaller","outputs":[{"internalType":"bytes","name":"","type":"bytes"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"testRevertReason","outputs":[],"stateMutability":"pure","type":"function"},{"inputs":[{"internalType":"address payable","name":"beneficiary","type":"address"}],"name":"testSelfDestruct","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[],"name":"testStackOverflow","outputs":[],"stateMutability":"view","type":"function"},{"inputs":[],"name":"testStaticCall","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"internalType":"string","name":"s","type":"string"}],"name":"triggerEvent","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"internalType":"string","name":"s","type":"string"}],"name":"triggerEventFail","outputs":[],"stateMutability":"nonpayable","type":"function"}]
2 changes: 1 addition & 1 deletion packages/evm/evmtest/ISCTest.bin

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions packages/evm/evmtest/ISCTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -190,4 +190,10 @@ contract ISCTest {
}
revert();
}

error CustomError(uint8);

function revertWithCustomError() public pure {
revert CustomError(42);
}
}
31 changes: 31 additions & 0 deletions packages/evm/jsonrpc/jsonrpctest/jsonrpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/stretchr/testify/require"

"github.com/iotaledger/hive.go/logger"
"github.com/iotaledger/wasp/packages/evm/evmerrors"
"github.com/iotaledger/wasp/packages/evm/evmtest"
"github.com/iotaledger/wasp/packages/evm/evmtypes"
"github.com/iotaledger/wasp/packages/evm/evmutil"
Expand Down Expand Up @@ -456,6 +457,36 @@ func TestRPCTxRejectedIfNotEnoughFunds(t *testing.T) {
require.Contains(t, err.Error(), "sender doesn't have enough L2 funds to cover tx gas budget")
}

func TestRPCCustomError(t *testing.T) {
env := newSoloTestEnv(t)
creator, creatorAddress := env.soloChain.NewEthereumAccountWithL2Funds()
contractABI, err := abi.JSON(strings.NewReader(evmtest.ISCTestContractABI))
require.NoError(t, err)
_, _, contractAddress := env.DeployEVMContract(creator, contractABI, evmtest.ISCTestContractBytecode)

callArguments, err := contractABI.Pack("revertWithCustomError")
require.NoError(t, err)

_, err = env.Client.CallContract(context.Background(), ethereum.CallMsg{
From: creatorAddress,
To: &contractAddress,
Data: callArguments,
}, nil)
require.ErrorContains(t, err, "execution reverted")

dataErr, ok := err.(rpc.DataError)
require.True(t, ok)

revertData, err := hexutil.Decode(dataErr.ErrorData().(string))
require.NoError(t, err)

args, err := evmerrors.UnpackCustomError(revertData, contractABI.Errors["CustomError"])
require.NoError(t, err)

require.Len(t, args, 1)
require.EqualValues(t, 42, args[0])
}

func BenchmarkRPCEstimateGas(b *testing.B) {
env := newSoloTestEnv(b)
_, addr := env.soloChain.NewEthereumAccountWithL2Funds()
Expand Down
28 changes: 23 additions & 5 deletions packages/evm/jsonrpc/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/ethereum/go-ethereum/rpc"
"golang.org/x/crypto/sha3"

"github.com/iotaledger/wasp/packages/evm/evmerrors"
"github.com/iotaledger/wasp/packages/isc"
"github.com/iotaledger/wasp/packages/metrics"
vmerrors "github.com/iotaledger/wasp/packages/vm/core/errors"
Expand Down Expand Up @@ -54,14 +55,31 @@ func (e *EthService) resolveError(err error) error {
if err == nil {
return nil
}
if vmError, ok := err.(*isc.UnresolvedVMError); ok {
resolvedErr, resolveErr := vmerrors.Resolve(vmError, e.evmChain.ViewCaller(e.evmChain.backend.ISCLatestState()))

var resolvedErr *isc.VMError

ok := errors.As(err, &resolvedErr)
if !ok {
var vmError *isc.UnresolvedVMError
ok := errors.As(err, &vmError)
if !ok {
return err
}
var resolveErr error
resolvedErr, resolveErr = vmerrors.Resolve(vmError, e.evmChain.ViewCaller(e.evmChain.backend.ISCLatestState()))
if resolveErr != nil {
return fmt.Errorf("could not resolve VMError %w: %v", vmError, resolveErr)
return fmt.Errorf("could not resolve VMError: %w: %v", err, resolveErr)
}
return resolvedErr.AsGoError()
}
return err

revertData, extractErr := evmerrors.ExtractRevertData(resolvedErr)
if extractErr != nil {
return fmt.Errorf("could not extract revert data: %w: %v", err, extractErr)
}
if len(revertData) > 0 {
return newRevertError(revertData)
}
return resolvedErr.AsGoError()
}

func (e *EthService) getTransactionCount(address common.Address, blockNumberOrHash *rpc.BlockNumberOrHash) (hexutil.Uint64, error) {
Expand Down
29 changes: 29 additions & 0 deletions packages/evm/jsonrpc/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"math/big"

"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/types"
Expand Down Expand Up @@ -403,3 +404,31 @@ func decodeTopic(s string) (common.Hash, error) {
}
return common.BytesToHash(b), err
}

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
}

func newRevertError(revertData []byte) *revertError {
reason, errUnpack := abi.UnpackRevert(revertData)
err := errors.New("execution reverted")
if errUnpack == nil {
err = fmt.Errorf("execution reverted: %v", reason)
}
return &revertError{
error: err,
reason: hexutil.Encode(revertData),
}
}
2 changes: 1 addition & 1 deletion packages/isc/vmerror.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ func validateParams(params []any) {
switch t.Kind() {
case reflect.String:
s := v.String()
if len(s) > 255 {
if len(s) > 1024 {
panic("string param too long")
}

Expand Down
11 changes: 4 additions & 7 deletions packages/vm/core/evm/evmimpl/impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@
package evmimpl

import (
"fmt"
"encoding/hex"

"github.com/ethereum/go-ethereum/accounts/abi"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core"
"github.com/ethereum/go-ethereum/core/types"
Expand All @@ -22,6 +21,7 @@ import (
"github.com/iotaledger/wasp/packages/kv/dict"
"github.com/iotaledger/wasp/packages/util"
"github.com/iotaledger/wasp/packages/util/panicutil"
"github.com/iotaledger/wasp/packages/vm"
"github.com/iotaledger/wasp/packages/vm/core/accounts"
"github.com/iotaledger/wasp/packages/vm/core/errors/coreerrors"
"github.com/iotaledger/wasp/packages/vm/core/evm"
Expand Down Expand Up @@ -383,16 +383,13 @@ func getChainID(ctx isc.SandboxView) dict.Dict {
return result(codec.EncodeUint16(chainID))
}

// include the revert reason in the error
func tryGetRevertError(res *core.ExecutionResult) error {
// try to include the revert reason in the error
if res.Err == nil {
return nil
}
if len(res.Revert()) > 0 {
reason, errUnpack := abi.UnpackRevert(res.Revert())
if errUnpack == nil {
return fmt.Errorf("%s: %v", res.Err.Error(), reason)
}
return vm.ErrEVMExecutionReverted.Create(hex.EncodeToString(res.Revert()))
}
return res.Err
}
Expand Down
33 changes: 30 additions & 3 deletions packages/vm/core/evm/evmtest/evm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,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/core/types"
"github.com/ethereum/go-ethereum/crypto"
Expand All @@ -25,6 +26,7 @@ import (
iotago "github.com/iotaledger/iota.go/v3"
"github.com/iotaledger/iota.go/v3/tpkg"
"github.com/iotaledger/wasp/contracts/native/inccounter"
"github.com/iotaledger/wasp/packages/evm/evmerrors"
"github.com/iotaledger/wasp/packages/evm/evmtest"
"github.com/iotaledger/wasp/packages/evm/evmutil"
"github.com/iotaledger/wasp/packages/evm/jsonrpc"
Expand Down Expand Up @@ -1643,14 +1645,20 @@ func TestSolidityRevertMessage(t *testing.T) {
Gas: 100_000,
Data: callData,
}, nil)
require.Error(t, err)
require.EqualValues(t, "execution reverted: foobar", err.Error())
require.ErrorContains(t, err, "execution reverted")

revertData, err := evmerrors.ExtractRevertData(err)
require.NoError(t, err)
revertString, err := abi.UnpackRevert(revertData)
require.NoError(t, err)

require.Equal(t, "foobar", revertString)

res, err := iscTest.callFn([]ethCallOptions{{
gasLimit: 100_000, // needed because gas estimation would fail
}}, "testRevertReason")
require.Error(t, err)
require.EqualValues(t, "execution reverted: foobar", res.iscReceipt.ResolvedError)
require.Regexp(t, `execution reverted: \w+`, res.iscReceipt.ResolvedError)
}

func TestCallContractCannotCauseStackOverflow(t *testing.T) {
Expand Down Expand Up @@ -1947,3 +1955,22 @@ func TestCaller(t *testing.T) {
require.NoError(t, err)
require.EqualValues(t, 42, big.NewInt(0).SetBytes(r).Uint64())
}

func TestCustomError(t *testing.T) {
env := initEVM(t)
ethKey, _ := env.soloChain.NewEthereumAccountWithL2Funds()
iscTest := env.deployISCTestContract(ethKey)
_, err := iscTest.callFn([]ethCallOptions{{
gasLimit: 100000,
}}, "revertWithCustomError")
require.ErrorContains(t, err, "execution reverted")

revertData, err := evmerrors.ExtractRevertData(err)
require.NoError(t, err)

args, err := evmerrors.UnpackCustomError(revertData, iscTest.abi.Errors["CustomError"])
require.NoError(t, err)

require.Len(t, args, 1)
require.EqualValues(t, 42, args[0])
}
1 change: 1 addition & 0 deletions packages/vm/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,5 @@ var (
ErrUnauthorized = coreerrors.Register("unauthorized access").Create()
ErrIllegalCall = coreerrors.Register("illegal call - entrypoint cannot be called from contracts")
ErrSendMultipleNFTs = coreerrors.Register("cannot send more than 1 NFT").Create()
ErrEVMExecutionReverted = coreerrors.Register("execution reverted: %s") // hex-encoded revert data
)