evm: implement ADR-002 EVM Hooks (#417)

* 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

@@ -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
 

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
 

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
+}

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(&ethtypes.Log{
+			Topics:  []common.Hash{},
+			Address: suite.address,
+		})
+		logs := k.GetTxLogs(txHash)
+		result := k.PostTxProcessing(txHash, logs)
+
+		tc.expFunc(hook, result)
+	}
+}

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)
+}

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

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")

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
+}