evm: store reverted tx as not failed (#228)

* add tx when evm revert

* add comment

* use cache ctx

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

docs/basics/json_rpc.md

@@ -565,6 +565,12 @@ curl localhost:8545 -H "Content-Type:application/json" -X POST --data '{"jsonrpc
 
 Returns the receipt of a transaction by transaction hash.
 
+Note: Tx Code from Tendermint and the Ethereum receipt status are switched:
+|         | Tendermint | Ethereum | 
+| ------- | ---------- | -------- |
+| Success | 0          | 1        |
+| Fail    | 1          | 0        |
+
 #### Parameters
 
 - Hash of a transaction

ethereum/rpc/namespaces/eth/api.go

@@ -532,7 +532,7 @@ func (e *PublicAPI) Call(args rpctypes.CallArgs, blockNr rpctypes.BlockNumber, _
 	}
 
 	if data.Reverted {
-		return []byte{}, rpctypes.ErrRevertedWith(data.Ret)
+		return []byte{}, evmtypes.NewExecErrorWithReason(data.Ret)
 	}
 
 	return (hexutil.Bytes)(data.Ret), nil
@@ -683,7 +683,7 @@ func (e *PublicAPI) EstimateGas(args rpctypes.CallArgs) (hexutil.Uint64, error)
 	}
 
 	if data.Reverted {
-		return 0, rpctypes.ErrRevertedWith(data.Ret)
+		return 0, evmtypes.NewExecErrorWithReason(data.Ret)
 	}
 
 	return hexutil.Uint64(data.GasUsed), nil
@@ -920,18 +920,12 @@ func (e *PublicAPI) GetTransactionReceipt(hash common.Hash) (map[string]interfac
 		cumulativeGasUsed += uint64(blockRes.TxsResults[i].GasUsed)
 	}
 
+	// Get the transaction result from the log
 	var status hexutil.Uint
-
-	// NOTE: Response{Deliver/Check}Tx Code from Tendermint and the Ethereum receipt status are switched:
-	// |         | Tendermint | Ethereum |
-	// | ------- | ---------- | -------- |
-	// | Success | 0          | 1        |
-	// | Fail    | 1          | 0        |
-
-	if res.TxResult.Code == 0 {
-		status = hexutil.Uint(ethtypes.ReceiptStatusSuccessful)
-	} else {
+	if strings.Contains(res.TxResult.GetLog(), evmtypes.AttributeKeyEthereumTxReverted) {
 		status = hexutil.Uint(ethtypes.ReceiptStatusFailed)
+	} else {
+		status = hexutil.Uint(ethtypes.ReceiptStatusSuccessful)
 	}
 
 	from, err := msg.GetSender(e.chainIDEpoch)

x/evm/keeper/msg_server.go

@@ -75,6 +75,10 @@ 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"))
+	}
+
 	// emit events
 	ctx.EventManager().EmitEvents(sdk.Events{
 		sdk.NewEvent(

x/evm/keeper/state_transition.go

@@ -126,12 +126,16 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT
 		return nil, stacktrace.Propagate(err, "failed to return ethereum transaction as core message")
 	}
 
+	// create an ethereum StateTransition instance and run TransitionDb
+	// we use a ctx context to avoid modifying to state in case EVM msg is reverted
+	originalCtx := k.ctx
+	cacheCtx, commit := k.ctx.CacheContext()
+	k.ctx = cacheCtx
+
 	evm := k.NewEVM(msg, ethCfg)
 
 	k.SetTxHashTransient(tx.Hash())
 	k.IncreaseTxIndexTransient()
-
-	// create an ethereum StateTransition instance and run TransitionDb
 	res, err := k.ApplyMessage(evm, msg, ethCfg)
 	if err != nil {
 		return nil, stacktrace.Propagate(err, "failed to apply ethereum core message")
@@ -140,14 +144,27 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT
 	txHash := tx.Hash()
 	res.Hash = txHash.Hex()
 
-	logs := k.GetTxLogs(txHash)
-	res.Logs = types.NewLogsFromEth(logs)
+	// Set the bloom filter and commit only if transaction is NOT reverted
+	if !res.Reverted {
+		logs := k.GetTxLogs(txHash)
+		res.Logs = types.NewLogsFromEth(logs)
+		// update block bloom filter
+		bloom := k.GetBlockBloomTransient()
+		bloom.Or(bloom, big.NewInt(0).SetBytes(ethtypes.LogsBloom(logs)))
+		k.SetBlockBloomTransient(bloom)
 
-	// update block bloom filter
-	bloom := k.GetBlockBloomTransient()
-	bloom.Or(bloom, big.NewInt(0).SetBytes(ethtypes.LogsBloom(logs)))
-	k.SetBlockBloomTransient(bloom)
+		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
 }
@@ -215,26 +232,19 @@ 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())
 	}
 
-	// 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())
-	}
-
+	var reverted bool
 	if vmErr != nil {
-		if errors.Is(vmErr, vm.ErrExecutionReverted) {
-			// unpack the return data bytes from the err if the execution has been "reverted" on the VM
-			return nil, stacktrace.Propagate(types.NewExecErrorWithReson(ret), "transaction reverted")
+		if !errors.Is(vmErr, vm.ErrExecutionReverted) {
+			// wrap the VM error
+			return nil, stacktrace.Propagate(sdkerrors.Wrap(types.ErrVMExecution, vmErr.Error()), "vm execution failed")
 		}
-
-		// wrap the VM error
-		return nil, stacktrace.Propagate(sdkerrors.Wrap(types.ErrVMExecution, vmErr.Error()), "vm execution failed")
+		reverted = true
 	}
 
 	gasUsed := msg.Gas() - leftoverGas
 	return &types.MsgEthereumTxResponse{
 		Ret:      ret,
-		Reverted: false,
+		Reverted: reverted,
 		GasUsed:  gasUsed,
 	}, nil
 }

x/evm/types/errors.go

@@ -73,9 +73,9 @@ var (
 	ErrInconsistentGas = sdkerrors.Register(ModuleName, codeErrInconsistentGas, "inconsistent gas")
 )
 
-// NewExecErrorWithReson unpacks the revert return bytes and returns a wrapped error
+// NewExecErrorWithReason unpacks the revert return bytes and returns a wrapped error
 // with the return reason.
-func NewExecErrorWithReson(revertReason []byte) error {
+func NewExecErrorWithReason(revertReason []byte) error {
 	hexValue := hexutil.Encode(revertReason)
 	reason, errUnpack := abi.UnpackRevert(revertReason)
 	if errUnpack == nil {

x/evm/types/events.go

@@ -4,11 +4,12 @@ package types
 const (
 	EventTypeEthereumTx = TypeMsgEthereumTx
 
-	AttributeKeyContractAddress = "contract"
-	AttributeKeyRecipient       = "recipient"
-	AttributeKeyTxHash          = "txHash"
-	AttributeKeyEthereumTxHash  = "ethereumTxHash"
-	AttributeValueCategory      = ModuleName
+	AttributeKeyContractAddress    = "contract"
+	AttributeKeyRecipient          = "recipient"
+	AttributeKeyTxHash             = "txHash"
+	AttributeKeyEthereumTxHash     = "ethereumTxHash"
+	AttributeKeyEthereumTxReverted = "ethereumTxReverted"
+	AttributeValueCategory         = ModuleName
 
 	MetricKeyTransitionDB = "transition_db"
 	MetricKeyStaticCall   = "static_call"