From 336703cfc9f98b96ae36a9fae370c0851dace3fa Mon Sep 17 00:00:00 2001 From: yihuang Date: Tue, 29 Jun 2021 18:54:29 +0800 Subject: [PATCH] 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 | 5 ++-- x/evm/keeper/keeper.go | 7 +++-- x/evm/keeper/keeper_test.go | 13 +++++++- x/evm/keeper/statedb.go | 32 +++++++++----------- x/evm/keeper/statedb_test.go | 58 +++++++++++++++++++++++------------- 5 files changed, 72 insertions(+), 43 deletions(-) diff --git a/app/app.go b/app/app.go index 62af7cde..39f8a135 100644 --- a/app/app.go +++ b/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, ) diff --git a/x/evm/keeper/keeper.go b/x/evm/keeper/keeper.go index e415d547..ebec98bd 100644 --- a/x/evm/keeper/keeper.go +++ b/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, diff --git a/x/evm/keeper/keeper_test.go b/x/evm/keeper/keeper_test.go index c4ac2f0d..ae9e5d2d 100644 --- a/x/evm/keeper/keeper_test.go +++ b/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) { diff --git a/x/evm/keeper/statedb.go b/x/evm/keeper/statedb.go index ae021c1e..92660ad0 100644 --- a/x/evm/keeper/statedb.go +++ b/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 := ðtypes.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()) diff --git a/x/evm/keeper/statedb_test.go b/x/evm/keeper/statedb_test.go index 4553c212..ce49315c 100644 --- a/x/evm/keeper/statedb_test.go +++ b/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{}, - ðtypes.Log{ - Address: addr, - }, - ðtypes.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() {}, }, }