From 735f00d4a3f876508fb7d9a634c9104b72bf3078 Mon Sep 17 00:00:00 2001 From: Thomas Nguy <81727899+thomas-nguy@users.noreply.github.com> Date: Thu, 8 Jul 2021 17:14:11 +0900 Subject: [PATCH] evm: store reverted tx as not failed (#228) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 | 6 ++++ ethereum/rpc/namespaces/eth/api.go | 18 ++++------- x/evm/keeper/msg_server.go | 4 +++ x/evm/keeper/state_transition.go | 52 ++++++++++++++++++------------ x/evm/types/errors.go | 4 +-- x/evm/types/events.go | 11 ++++--- 6 files changed, 55 insertions(+), 40 deletions(-) diff --git a/docs/basics/json_rpc.md b/docs/basics/json_rpc.md index 7dc2ac02..37632770 100644 --- a/docs/basics/json_rpc.md +++ b/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 diff --git a/ethereum/rpc/namespaces/eth/api.go b/ethereum/rpc/namespaces/eth/api.go index a9b20cf7..c3b57287 100644 --- a/ethereum/rpc/namespaces/eth/api.go +++ b/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) diff --git a/x/evm/keeper/msg_server.go b/x/evm/keeper/msg_server.go index 5ce986f1..26dc242b 100644 --- a/x/evm/keeper/msg_server.go +++ b/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( diff --git a/x/evm/keeper/state_transition.go b/x/evm/keeper/state_transition.go index 4fbcc344..797a2f94 100644 --- a/x/evm/keeper/state_transition.go +++ b/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 } diff --git a/x/evm/types/errors.go b/x/evm/types/errors.go index eae5727b..17574b4d 100644 --- a/x/evm/types/errors.go +++ b/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 { diff --git a/x/evm/types/events.go b/x/evm/types/events.go index 8cfe3601..f5836571 100644 --- a/x/evm/types/events.go +++ b/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"