evm: fix AddLog unmarshaling tx (#192)

* fix `AddLog` unmarshaling tx

Closes #187

* use cosmos tx in AddLog unit test

* Apply suggestions from code review

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

app/app.go

@@ -229,13 +229,14 @@ func NewEthermintApp(
 	appCodec := encodingConfig.Marshaler
 	cdc := encodingConfig.Amino
 	interfaceRegistry := encodingConfig.InterfaceRegistry
+	txDecoder := encodingConfig.TxConfig.TxDecoder()
 
 	// NOTE we use custom transaction decoder that supports the sdk.Tx interface instead of sdk.StdTx
 	bApp := baseapp.NewBaseApp(
 		appName,
 		logger,
 		db,
-		encodingConfig.TxConfig.TxDecoder(),
+		txDecoder,
 		baseAppOptions...,
 	)
 	bApp.SetCommitMultiStoreTracer(traceStore)
@@ -311,7 +312,7 @@ func NewEthermintApp(
 
 	// Create Ethermint keepers
 	app.EvmKeeper = evmkeeper.NewKeeper(
-		appCodec, keys[evmtypes.StoreKey], tkeys[evmtypes.TransientKey], app.GetSubspace(evmtypes.ModuleName),
+		appCodec, txDecoder, keys[evmtypes.StoreKey], tkeys[evmtypes.TransientKey], app.GetSubspace(evmtypes.ModuleName),
 		app.AccountKeeper, app.BankKeeper, app.StakingKeeper,
 	)
 

x/evm/keeper/keeper.go

@@ -20,7 +20,8 @@ import (
 // to the StateDB interface.
 type Keeper struct {
 	// Protobuf codec
-	cdc codec.BinaryMarshaler
+	cdc       codec.BinaryMarshaler
+	txDecoder sdk.TxDecoder
 	// Store key required for the EVM Prefix KVStore. It is required by:
 	// - storing Account's Storage State
 	// - storing Account's Code
@@ -37,6 +38,7 @@ type Keeper struct {
 	stakingKeeper types.StakingKeeper
 
 	ctx sdk.Context
+
 	// chain ID number obtained from the context's chain id
 	eip155ChainID *big.Int
 	debug         bool
@@ -44,7 +46,7 @@ type Keeper struct {
 
 // NewKeeper generates new evm module keeper
 func NewKeeper(
-	cdc codec.BinaryMarshaler, storeKey, transientKey sdk.StoreKey, paramSpace paramtypes.Subspace,
+	cdc codec.BinaryMarshaler, txDecoder sdk.TxDecoder, storeKey, transientKey sdk.StoreKey, paramSpace paramtypes.Subspace,
 	ak types.AccountKeeper, bankKeeper types.BankKeeper, sk types.StakingKeeper,
 ) *Keeper {
 	// set KeyTable if it has not already been set
@@ -55,6 +57,7 @@ func NewKeeper(
 	// NOTE: we pass in the parameter space to the CommitStateDB in order to use custom denominations for the EVM operations
 	return &Keeper{
 		cdc:           cdc,
+		txDecoder:     txDecoder,
 		paramSpace:    paramSpace,
 		accountKeeper: ak,
 		bankKeeper:    bankKeeper,

x/evm/keeper/keeper_test.go

@@ -1,22 +1,25 @@
 package keeper_test
 
 import (
-	"github.com/tharsis/ethermint/crypto/ethsecp256k1"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/suite"
 
 	"github.com/cosmos/cosmos-sdk/baseapp"
+	"github.com/cosmos/cosmos-sdk/client"
 	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"
 
 	"github.com/tharsis/ethermint/app"
+	"github.com/tharsis/ethermint/crypto/ethsecp256k1"
+	"github.com/tharsis/ethermint/encoding"
 	ethermint "github.com/tharsis/ethermint/types"
 	"github.com/tharsis/ethermint/x/evm/types"
 
 	ethcmn "github.com/ethereum/go-ethereum/common"
+	ethtypes "github.com/ethereum/go-ethereum/core/types"
 	ethcrypto "github.com/ethereum/go-ethereum/crypto"
 
 	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
@@ -37,6 +40,10 @@ type KeeperTestSuite struct {
 	queryClient types.QueryClient
 	address     ethcmn.Address
 	consAddress sdk.ConsAddress
+
+	// for generate test tx
+	clientCtx client.Context
+	ethSigner ethtypes.Signer
 }
 
 func (suite *KeeperTestSuite) SetupTest() {
@@ -68,6 +75,10 @@ func (suite *KeeperTestSuite) SetupTest() {
 	suite.app.StakingKeeper.SetValidatorByConsAddr(suite.ctx, validator)
 	suite.app.StakingKeeper.SetValidator(suite.ctx, validator)
 	suite.consAddress = sdk.ConsAddress(priv.PubKey().Address())
+
+	encodingConfig := encoding.MakeConfig(app.ModuleBasics)
+	suite.clientCtx = client.Context{}.WithTxConfig(encodingConfig.TxConfig)
+	suite.ethSigner = ethtypes.LatestSignerForChainID(suite.app.EvmKeeper.ChainID())
 }
 
 func TestKeeperTestSuite(t *testing.T) {

x/evm/keeper/statedb.go

@@ -568,25 +568,21 @@ func (k *Keeper) RevertToSnapshot(_ int) {}
 // context. This function also fills in the tx hash, block hash, tx index and log index fields before setting the log
 // to store.
 func (k *Keeper) AddLog(log *ethtypes.Log) {
-
-	key := log.TxHash
-
-	if len(k.ctx.TxBytes()) > 0 {
-		tx := &ethtypes.Transaction{}
-		if err := tx.UnmarshalBinary(k.ctx.TxBytes()); err != nil {
-			k.Logger(k.ctx).Error(
-				"ethereum tx unmarshaling failed",
-				"error", err,
-			)
-			return
-		}
-
-		// NOTE: we set up the transaction hash from tendermint as it is the format expected by the application:
-		// Remove once hashing is fixed on Tendermint. See https://github.com/tendermint/tendermint/issues/6539
-		key = common.BytesToHash(tmtypes.Tx(k.ctx.TxBytes()).Hash())
-
-		log.TxHash = tx.Hash()
+	tx, err := k.txDecoder(k.ctx.TxBytes())
+	if err != nil {
+		// safety check, should be checked when processing the tx
+		panic(err)
 	}
+	// NOTE: tx length checked on AnteHandler
+	ethTx, ok := tx.GetMsgs()[0].(*types.MsgEthereumTx)
+	if !ok {
+		panic("invalid ethereum tx")
+	}
+
+	// NOTE: we set up the transaction hash from tendermint as it is the format expected by the application:
+	// Remove once hashing is fixed on Tendermint. See https://github.com/tendermint/tendermint/issues/6539
+	key := common.BytesToHash(tmtypes.Tx(k.ctx.TxBytes()).Hash())
+	log.TxHash = common.HexToHash(ethTx.Hash)
 
 	log.BlockHash = common.BytesToHash(k.ctx.HeaderHash())
 	log.TxIndex = uint(k.GetTxIndexTransient())

x/evm/keeper/statedb_test.go

@@ -4,7 +4,11 @@ import (
 	"fmt"
 	"math/big"
 
+	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
+	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
+	authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	tmtypes "github.com/tendermint/tendermint/types"
 
@@ -13,6 +17,7 @@ import (
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/tharsis/ethermint/tests"
 	"github.com/tharsis/ethermint/x/evm/types"
+	evmtypes "github.com/tharsis/ethermint/x/evm/types"
 )
 
 func (suite *KeeperTestSuite) TestCreateAccount() {
@@ -453,32 +458,47 @@ func (suite *KeeperTestSuite) TestSnapshot() {
 	suite.app.EvmKeeper.RevertToSnapshot(revision) // no-op
 }
 
-func (suite *KeeperTestSuite) TestAddLog() {
-	addr := tests.GenerateAddress()
-	msg := types.NewMsgEthereumTx(big.NewInt(1), 0, &suite.address, big.NewInt(1), 100000, big.NewInt(1), []byte("test"), nil)
-	tx := msg.AsTransaction()
-	txBz, err := tx.MarshalBinary()
+func (suite *KeeperTestSuite) CreateTestTx(msg *evmtypes.MsgEthereumTx, priv cryptotypes.PrivKey) authsigning.Tx {
+	option, err := codectypes.NewAnyWithValue(&evmtypes.ExtensionOptionsEthereumTx{})
+	suite.Require().NoError(err)
+
+	txBuilder := suite.clientCtx.TxConfig.NewTxBuilder()
+	builder, ok := txBuilder.(authtx.ExtensionOptionsTxBuilder)
+	suite.Require().True(ok)
+
+	builder.SetExtensionOptions(option)
+
+	err = msg.Sign(suite.ethSigner, tests.NewSigner(priv))
+	suite.Require().NoError(err)
+
+	err = txBuilder.SetMsgs(msg)
+	suite.Require().NoError(err)
+
+	return txBuilder.GetTx()
+}
+
+func (suite *KeeperTestSuite) TestAddLog() {
+	addr, privKey := tests.NewAddrKey()
+	msg := types.NewMsgEthereumTx(big.NewInt(1), 0, &suite.address, big.NewInt(1), 100000, big.NewInt(1), []byte("test"), nil)
+	msg.From = addr.Hex()
+
+	tx := suite.CreateTestTx(msg, privKey)
+	txBz, err := suite.clientCtx.TxConfig.TxEncoder()(tx)
 	suite.Require().NoError(err)
-	txHash := tx.Hash()
 	tmHash := common.BytesToHash(tmtypes.Tx(txBz).Hash())
 
+	msg, _ = tx.GetMsgs()[0].(*evmtypes.MsgEthereumTx)
+	ethTx := msg.AsTransaction()
+	txHash := ethTx.Hash()
+
+	suite.app.EvmKeeper.WithContext(suite.ctx.WithTxBytes(txBz))
+
 	testCases := []struct {
 		name        string
 		hash        common.Hash
 		log, expLog *ethtypes.Log // pre and post populating log fields
 		malleate    func()
 	}{
-		{
-			"block hash not found",
-			common.Hash{},
-			&ethtypes.Log{
-				Address: addr,
-			},
-			&ethtypes.Log{
-				Address: addr,
-			},
-			func() {},
-		},
 		{
 			"tx hash from message",
 			tmHash,
@@ -489,9 +509,7 @@ func (suite *KeeperTestSuite) TestAddLog() {
 				Address: addr,
 				TxHash:  txHash,
 			},
-			func() {
-				suite.app.EvmKeeper.WithContext(suite.ctx.WithTxBytes(txBz))
-			},
+			func() {},
 		},
 	}