From 089afe41a8e56e0eaff0d7bbea2f6668ab104f12 Mon Sep 17 00:00:00 2001 From: yihuang Date: Thu, 2 Sep 2021 20:36:33 +0800 Subject: [PATCH] evm: implement ADR-002 EVM Hooks (#417) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Allow evm to call native modules through logs Closes #416 comment add txHash parameter review suggestions add hooks test * Update x/evm/keeper/hooks.go Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> * Update x/evm/keeper/hooks_test.go Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> * Update x/evm/keeper/keeper.go Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> * Update x/evm/keeper/keeper.go Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> * Update x/evm/keeper/keeper.go Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> * use table tests * update adr comment * update adr * changelog * Update CHANGELOG.md Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> --- CHANGELOG.md | 1 + docs/architecture/adr-002-evm-hooks.md | 6 +- x/evm/keeper/hooks.go | 31 +++++++++++ x/evm/keeper/hooks_test.go | 76 ++++++++++++++++++++++++++ x/evm/keeper/keeper.go | 21 +++++++ x/evm/keeper/state_transition.go | 14 +++++ x/evm/types/errors.go | 4 ++ x/evm/types/interfaces.go | 11 ++++ 8 files changed, 161 insertions(+), 3 deletions(-) create mode 100644 x/evm/keeper/hooks.go create mode 100644 x/evm/keeper/hooks_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 3949982e..1aa0271e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Features * (evm) [tharsis#469](https://github.com/tharsis/ethermint/pull/469) Support [EIP-1559](https://eips.ethereum.org/EIPS/eip-1559) +* (evm) [tharsis#417](https://github.com/tharsis/ethermint/pull/417) Add `EvmHooks` for tx post-processing ### Bug Fixes diff --git a/docs/architecture/adr-002-evm-hooks.md b/docs/architecture/adr-002-evm-hooks.md index 8b2ba53d..9f2adad2 100644 --- a/docs/architecture/adr-002-evm-hooks.md +++ b/docs/architecture/adr-002-evm-hooks.md @@ -49,7 +49,7 @@ func (k *EvmKeeper) SetHooks(eh types.EvmHooks) *Keeper; The EVM state transition method `ApplyTransaction` should be changed like this: ```golang -// Create cached context which convers both the tx processing and post processing +// Need to create a snapshot explicitly to cover both tx processing and post processing logic revision := k.Snapshot() res, err := k.ApplyMessage(evm, msg, ethCfg, false) @@ -186,12 +186,12 @@ The proposed ADR is backward compatible. ### Negative -- It's possible that some contracts accidentally define a log with the same signature and cause an unintentional result. +- On the use case of native call: It's possible that some contracts accidentally define a log with the same signature and cause an unintentional result. To mitigate this, the implementor could whitelist contracts that are allowed to invoke native calls. ### Neutral -- The contract can only call native modules asynchronously, which means it can neither get the result nor handle the error. +- On the use case of native call: The contract can only call native modules asynchronously, which means it can neither get the result nor handle the error. ## Further Discussions diff --git a/x/evm/keeper/hooks.go b/x/evm/keeper/hooks.go new file mode 100644 index 00000000..66057f4c --- /dev/null +++ b/x/evm/keeper/hooks.go @@ -0,0 +1,31 @@ +package keeper + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + ethcmn "github.com/ethereum/go-ethereum/common" + ethtypes "github.com/ethereum/go-ethereum/core/types" + "github.com/tharsis/ethermint/x/evm/types" +) + +var ( + _ types.EvmHooks = MultiEvmHooks{} +) + +// MultiEvmHooks combine multiple evm hooks, all hook functions are run in array sequence +type MultiEvmHooks []types.EvmHooks + +// NewMultiEvmHooks combine multiple evm hooks +func NewMultiEvmHooks(hooks ...types.EvmHooks) MultiEvmHooks { + return hooks +} + +// PostTxProcessing delegate the call to underlying hooks +func (mh MultiEvmHooks) PostTxProcessing(ctx sdk.Context, txHash ethcmn.Hash, logs []*ethtypes.Log) error { + for i := range mh { + if err := mh[i].PostTxProcessing(ctx, txHash, logs); err != nil { + return sdkerrors.Wrapf(err, "EVM hook %T failed", mh[i]) + } + } + return nil +} diff --git a/x/evm/keeper/hooks_test.go b/x/evm/keeper/hooks_test.go new file mode 100644 index 00000000..4035d640 --- /dev/null +++ b/x/evm/keeper/hooks_test.go @@ -0,0 +1,76 @@ +package keeper_test + +import ( + "errors" + "math/big" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/ethereum/go-ethereum/common" + ethtypes "github.com/ethereum/go-ethereum/core/types" + + "github.com/tharsis/ethermint/x/evm/keeper" + "github.com/tharsis/ethermint/x/evm/types" +) + +// LogRecordHook records all the logs +type LogRecordHook struct { + Logs []*ethtypes.Log +} + +func (dh *LogRecordHook) PostTxProcessing(ctx sdk.Context, txHash common.Hash, logs []*ethtypes.Log) error { + dh.Logs = logs + return nil +} + +// FailureHook always fail +type FailureHook struct{} + +func (dh FailureHook) PostTxProcessing(ctx sdk.Context, txHash common.Hash, logs []*ethtypes.Log) error { + return errors.New("post tx processing failed") +} + +func (suite *KeeperTestSuite) TestEvmHooks() { + testCases := []struct { + msg string + setupHook func() types.EvmHooks + expFunc func(hook types.EvmHooks, result error) + }{ + { + "log collect hook", + func() types.EvmHooks { + return &LogRecordHook{} + }, + func(hook types.EvmHooks, result error) { + suite.Require().NoError(result) + suite.Require().Equal(1, len((hook.(*LogRecordHook).Logs))) + }, + }, + { + "always fail hook", + func() types.EvmHooks { + return &FailureHook{} + }, + func(hook types.EvmHooks, result error) { + suite.Require().Error(result) + }, + }, + } + + for _, tc := range testCases { + suite.SetupTest() + hook := tc.setupHook() + suite.app.EvmKeeper.SetHooks(keeper.NewMultiEvmHooks(hook)) + + k := suite.app.EvmKeeper + txHash := common.BigToHash(big.NewInt(1)) + k.SetTxHashTransient(txHash) + k.AddLog(ðtypes.Log{ + Topics: []common.Hash{}, + Address: suite.address, + }) + logs := k.GetTxLogs(txHash) + result := k.PostTxProcessing(txHash, logs) + + tc.expFunc(hook, result) + } +} diff --git a/x/evm/keeper/keeper.go b/x/evm/keeper/keeper.go index 6af67b44..4ac2dbb8 100644 --- a/x/evm/keeper/keeper.go +++ b/x/evm/keeper/keeper.go @@ -54,6 +54,9 @@ type Keeper struct { // trace EVM state transition execution. This value is obtained from the `--trace` flag. // For more info check https://geth.ethereum.org/docs/dapp/tracing debug bool + + // EVM Hooks for tx post-processing + hooks types.EvmHooks } // NewKeeper generates new evm module keeper @@ -416,3 +419,21 @@ func (k Keeper) ResetAccount(addr common.Address) { k.DeleteCode(addr) k.DeleteAccountStorage(addr) } + +// SetHooks sets the hooks for the EVM module +func (k *Keeper) SetHooks(eh types.EvmHooks) *Keeper { + if k.hooks != nil { + panic("cannot set evm hooks twice") + } + + k.hooks = eh + return k +} + +// PostTxProcessing delegate the call to the hooks. If no hook has been registered, this function returns with a `nil` error +func (k *Keeper) PostTxProcessing(txHash common.Hash, logs []*ethtypes.Log) error { + if k.hooks == nil { + return nil + } + return k.hooks.PostTxProcessing(k.Ctx(), txHash, logs) +} diff --git a/x/evm/keeper/state_transition.go b/x/evm/keeper/state_transition.go index 077b0526..77d08c2f 100644 --- a/x/evm/keeper/state_transition.go +++ b/x/evm/keeper/state_transition.go @@ -175,6 +175,9 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT panic("context stack shouldn't be dirty before apply message") } + // Contains the tx processing and post processing in same scope + revision := k.Snapshot() + // pass false to execute in real mode, which do actual gas refunding res, err := k.ApplyMessage(evm, msg, ethCfg, false) if err != nil { @@ -183,6 +186,17 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT res.Hash = txHash.Hex() logs := k.GetTxLogs(txHash) + + if !res.Failed() { + // Only call hooks if tx executed successfully. + if err = k.PostTxProcessing(txHash, logs); err != nil { + // If hooks return error, revert the whole tx. + k.RevertToSnapshot(revision) + res.VmError = types.ErrPostTxProcessing.Error() + k.Logger(ctx).Error("tx post processing failed", "error", err) + } + } + if len(logs) > 0 { res.Logs = types.NewLogsFromEth(logs) // Update transient block bloom filter diff --git a/x/evm/types/errors.go b/x/evm/types/errors.go index 09cefc22..6c56c7ef 100644 --- a/x/evm/types/errors.go +++ b/x/evm/types/errors.go @@ -32,6 +32,10 @@ const ( codeErrInvalidBaseFee ) +var ( + ErrPostTxProcessing = errors.New("failed to execute post processing") +) + var ( // ErrInvalidState returns an error resulting from an invalid Storage State. ErrInvalidState = sdkerrors.Register(ModuleName, codeErrInvalidState, "invalid storage state") diff --git a/x/evm/types/interfaces.go b/x/evm/types/interfaces.go index b45089e7..4b60f69a 100644 --- a/x/evm/types/interfaces.go +++ b/x/evm/types/interfaces.go @@ -4,6 +4,8 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + ethcmn "github.com/ethereum/go-ethereum/common" + ethtypes "github.com/ethereum/go-ethereum/core/types" ) // AccountKeeper defines the expected account keeper interface @@ -33,3 +35,12 @@ type StakingKeeper interface { GetHistoricalInfo(ctx sdk.Context, height int64) (stakingtypes.HistoricalInfo, bool) GetValidatorByConsAddr(ctx sdk.Context, consAddr sdk.ConsAddress) (validator stakingtypes.Validator, found bool) } + +// Event Hooks +// These can be utilized to customize evm transaction processing. + +// EvmHooks event hooks for evm tx processing +type EvmHooks interface { + // Must be called after tx is processed successfully, if return an error, the whole transaction is reverted. + PostTxProcessing(ctx sdk.Context, txHash ethcmn.Hash, logs []*ethtypes.Log) error +}