treat all vm errors the same as reverted

Closes: #274

client/docs/swagger-ui/swagger.yaml

@@ -1792,10 +1792,9 @@ paths:
                 title: >-
                   returned data from evm function (result or data supplied with
                   revert opcode)
-              reverted:
-                type: boolean
-                format: boolean
-                title: reverted flag is set to true when the call has been reverted
+              vm_error:
+                type: string
+                title: vm error is the error returned by vm execution
               gas_used:
                 type: string
                 format: uint64
@@ -13211,10 +13210,9 @@ definitions:
         title: >-
           returned data from evm function (result or data supplied with revert
           opcode)
-      reverted:
-        type: boolean
-        format: boolean
-        title: reverted flag is set to true when the call has been reverted
+      vm_error:
+        type: string
+        title: vm error is the error returned by vm execution
       gas_used:
         type: string
         format: uint64

docs/core/proto-docs.md

@@ -444,7 +444,7 @@ MsgEthereumTxResponse defines the Msg/EthereumTx response type.
 | `hash` | [string](#string) |  | ethereum transaction hash in hex format. This hash differs from the Tendermint sha256 hash of the transaction bytes. See https://github.com/tendermint/tendermint/issues/6539 for reference |
 | `logs` | [Log](#ethermint.evm.v1alpha1.Log) | repeated | logs contains the transaction hash and the proto-compatible ethereum logs. |
 | `ret` | [bytes](#bytes) |  | returned data from evm function (result or data supplied with revert opcode) |
-| `reverted` | [bool](#bool) |  | reverted flag is set to true when the call has been reverted |
+| `vm_error` | [string](#string) |  | vm error is the error returned by vm execution |
 | `gas_used` | [uint64](#uint64) |  | gas consumed by the transaction |
 
 

ethereum/rpc/namespaces/eth/api.go

@@ -26,6 +26,7 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	ethtypes "github.com/ethereum/go-ethereum/core/types"
+	"github.com/ethereum/go-ethereum/core/vm"
 	"github.com/ethereum/go-ethereum/crypto"
 
 	"github.com/tharsis/ethermint/crypto/hd"
@@ -525,10 +526,6 @@ func (e *PublicAPI) Call(args evmtypes.CallArgs, blockNr rpctypes.BlockNumber, _
 		return []byte{}, err
 	}
 
-	if data.Reverted {
-		return []byte{}, evmtypes.NewExecErrorWithReason(data.Ret)
-	}
-
 	return (hexutil.Bytes)(data.Ret), nil
 }
 
@@ -547,6 +544,13 @@ func (e *PublicAPI) doCall(
 		return nil, err
 	}
 
+	if len(res.VmError) > 0 {
+		if res.VmError == vm.ErrExecutionReverted.Error() {
+			return nil, evmtypes.NewExecErrorWithReason(res.Ret)
+		}
+		return nil, errors.New(res.VmError)
+	}
+
 	return res, nil
 }
 
@@ -562,10 +566,6 @@ func (e *PublicAPI) EstimateGas(args evmtypes.CallArgs) (hexutil.Uint64, error)
 		return 0, err
 	}
 
-	if data.Reverted {
-		return 0, evmtypes.NewExecErrorWithReason(data.Ret)
-	}
-
 	return hexutil.Uint64(data.GasUsed), nil
 }
 
@@ -781,7 +781,7 @@ func (e *PublicAPI) GetTransactionReceipt(hash common.Hash) (map[string]interfac
 
 	// Get the transaction result from the log
 	var status hexutil.Uint
-	if strings.Contains(res.TxResult.GetLog(), evmtypes.AttributeKeyEthereumTxReverted) {
+	if strings.Contains(res.TxResult.GetLog(), evmtypes.AttributeKeyEthereumTxFailed) {
 		status = hexutil.Uint(ethtypes.ReceiptStatusFailed)
 	} else {
 		status = hexutil.Uint(ethtypes.ReceiptStatusSuccessful)

proto/ethermint/evm/v1alpha1/tx.proto

@@ -121,8 +121,8 @@ message MsgEthereumTxResponse {
   repeated Log logs = 2;
   // returned data from evm function (result or data supplied with revert opcode)
   bytes ret = 3;
-  // reverted flag is set to true when the call has been reverted
-  bool reverted = 4;
+  // vm error is the error returned by vm execution
+  string vm_error = 4;
   // gas consumed by the transaction
   uint64 gas_used = 5;
-}
+}

x/evm/keeper/grpc_query.go

@@ -395,14 +395,10 @@ func (k Keeper) EthCall(c context.Context, req *types.EthCallRequest) (*types.Ms
 	}
 	ethCfg := cfg.EthereumConfig(k.eip155ChainID)
 	evm := k.NewEVM(msg, ethCfg)
-	res, err := k.ApplyMessage(evm, msg, ethCfg)
+	res, err := k.ApplyMessage(evm, msg, ethCfg, true)
 	if err != nil {
 		return nil, status.Error(codes.Internal, err.Error())
 	}
 
-	// ApplyMessage don't handle gas refund, let's do it here
-	refund := k.GasToRefund(res.GasUsed)
-	res.GasUsed -= refund
-
-	return res, nil
+	return types.TxResponseFromResult(res), nil
 }

x/evm/keeper/msg_server.go

@@ -75,8 +75,8 @@ func (k *Keeper) EthereumTx(goCtx context.Context, msg *types.MsgEthereumTx) (*t
 		attrs = append(attrs, sdk.NewAttribute(types.AttributeKeyRecipient, tx.To().Hex()))
 	}
 
-	if response.Reverted {
-		attrs = append(attrs, sdk.NewAttribute(types.AttributeKeyEthereumTxReverted, "true"))
+	if len(response.VmError) > 0 {
+		attrs = append(attrs, sdk.NewAttribute(types.AttributeKeyEthereumTxFailed, response.VmError))
 	}
 
 	// emit events

x/evm/keeper/state_transition.go

@@ -1,7 +1,6 @@
 package keeper
 
 import (
-	"errors"
 	"math/big"
 	"os"
 	"time"
@@ -136,18 +135,20 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT
 
 	k.SetTxHashTransient(tx.Hash())
 	k.IncreaseTxIndexTransient()
-	res, err := k.ApplyMessage(evm, msg, ethCfg)
+	res, err := k.ApplyMessage(evm, msg, ethCfg, false)
 	if err != nil {
 		return nil, stacktrace.Propagate(err, "failed to apply ethereum core message")
 	}
 
+	rsp := types.TxResponseFromResult(res)
+
 	txHash := tx.Hash()
-	res.Hash = txHash.Hex()
+	rsp.Hash = txHash.Hex()
 
-	// Set the bloom filter and commit only if transaction is NOT reverted
-	if !res.Reverted {
+	// Set the bloom filter and commit only if transaction is NOT failed
+	if !res.Failed() {
 		logs := k.GetTxLogs(txHash)
-		res.Logs = types.NewLogsFromEth(logs)
+		rsp.Logs = types.NewLogsFromEth(logs)
 		// update block bloom filter
 		bloom := k.GetBlockBloomTransient()
 		bloom.Or(bloom, big.NewInt(0).SetBytes(ethtypes.LogsBloom(logs)))
@@ -156,17 +157,11 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT
 		commit()
 	}
 
-	// refund gas prior to handling the vm error in order to set the updated gas meter
-	k.ctx = originalCtx
-	leftoverGas := msg.Gas() - res.GasUsed
-	leftoverGas, err = k.RefundGas(msg, leftoverGas)
-	if err != nil {
-		return nil, stacktrace.Propagate(err, "failed to refund gas leftover gas to sender %s", msg.From())
-	}
 	// update the gas used after refund
-	res.GasUsed = msg.Gas() - leftoverGas
-	k.resetGasMeterAndConsumeGas(res.GasUsed)
-	return res, nil
+	k.ctx = originalCtx
+	k.resetGasMeterAndConsumeGas(res.UsedGas)
+
+	return rsp, nil
 }
 
 // Gas consumption notes (write doc from this)
@@ -209,7 +204,9 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT
 // The preprocessing steps performed by the AnteHandler are:
 //
 // 1. set up the initial access list (iff fork > Berlin)
-func (k *Keeper) ApplyMessage(evm *vm.EVM, msg core.Message, cfg *params.ChainConfig) (*types.MsgEthereumTxResponse, error) {
+
+// NOTE: eth_call/eth_estimateGas don't do ante handler, so shouldn't do gas refund either.
+func (k *Keeper) ApplyMessage(evm *vm.EVM, msg core.Message, cfg *params.ChainConfig, simulate bool) (*core.ExecutionResult, error) {
 	var (
 		ret   []byte // return bytes from evm execution
 		vmErr error  // vm errors do not effect consensus and are therefore not assigned to err
@@ -223,7 +220,10 @@ func (k *Keeper) ApplyMessage(evm *vm.EVM, msg core.Message, cfg *params.ChainCo
 		// should have already been checked on Ante Handler
 		return nil, stacktrace.Propagate(err, "intrinsic gas failed")
 	}
-	// should be > 0 as it is checked on Ante Handler
+	// Should check again even if it is checked on Ante Handler, because eth_call don't go through Ante Handler.
+	if msg.Gas() < intrinsicGas {
+		return nil, core.ErrIntrinsicGas
+	}
 	leftoverGas := msg.Gas() - intrinsicGas
 
 	if contractCreation {
@@ -232,20 +232,22 @@ func (k *Keeper) ApplyMessage(evm *vm.EVM, msg core.Message, cfg *params.ChainCo
 		ret, leftoverGas, vmErr = evm.Call(sender, *msg.To(), msg.Data(), leftoverGas, msg.Value())
 	}
 
-	var reverted bool
-	if vmErr != nil {
-		if !errors.Is(vmErr, vm.ErrExecutionReverted) {
-			// wrap the VM error
-			return nil, stacktrace.Propagate(sdkerrors.Wrap(types.ErrVMExecution, vmErr.Error()), "vm execution failed")
+	if simulate {
+		// eth_call/eth_estimateGas don't call ante handler to deduct gas fee, so don't do actual refund.
+		leftoverGas += k.GasToRefund(msg.Gas() - leftoverGas)
+	} else {
+		// refund gas prior to handling the vm error in order to set the updated gas meter
+		leftoverGas, err = k.RefundGas(msg, leftoverGas)
+		if err != nil {
+			return nil, stacktrace.Propagate(err, "failed to refund gas leftover gas to sender %s", msg.From())
 		}
-		reverted = true
 	}
 
 	gasUsed := msg.Gas() - leftoverGas
-	return &types.MsgEthereumTxResponse{
-		Ret:      ret,
-		Reverted: reverted,
-		GasUsed:  gasUsed,
+	return &core.ExecutionResult{
+		UsedGas:    gasUsed,
+		Err:        vmErr,
+		ReturnData: ret,
 	}, nil
 }
 

x/evm/types/events.go

@@ -4,12 +4,13 @@ package types
 const (
 	EventTypeEthereumTx = TypeMsgEthereumTx
 
-	AttributeKeyContractAddress    = "contract"
-	AttributeKeyRecipient          = "recipient"
-	AttributeKeyTxHash             = "txHash"
-	AttributeKeyEthereumTxHash     = "ethereumTxHash"
-	AttributeKeyEthereumTxReverted = "ethereumTxReverted"
-	AttributeValueCategory         = ModuleName
+	AttributeKeyContractAddress = "contract"
+	AttributeKeyRecipient       = "recipient"
+	AttributeKeyTxHash          = "txHash"
+	AttributeKeyEthereumTxHash  = "ethereumTxHash"
+	// tx failed in eth vm execution
+	AttributeKeyEthereumTxFailed = "ethereumTxFailed"
+	AttributeValueCategory       = ModuleName
 
 	MetricKeyTransitionDB = "transition_db"
 	MetricKeyStaticCall   = "static_call"