From 75fb3ccb8935db7f51245b33c8c5954baab7c886 Mon Sep 17 00:00:00 2001 From: Ray Pierre <974741468@qq.com> Date: Fri, 19 Mar 2021 11:04:29 +0800 Subject: [PATCH] fix incorrect blockHash in evm (#780) * Fix wrong blockHash value in evm * fix ut * store current block hash in BeginBlock * update ut * update comment * add changelog --- CHANGELOG.md | 1 + importer/importer_test.go | 3 ++- x/evm/handler.go | 3 +-- x/evm/keeper/abci.go | 9 ++++--- x/evm/keeper/abci_test.go | 7 +++--- x/evm/keeper/msg_server.go | 3 +-- x/evm/keeper/statedb.go | 4 +-- x/evm/keeper/statedb_test.go | 5 ++-- x/evm/types/state_transition.go | 26 +++---------------- x/evm/types/state_transition_test.go | 4 ++- x/evm/types/statedb.go | 7 ++++-- x/evm/types/statedb_test.go | 5 ++-- x/evm/types/utils.go | 37 ---------------------------- 13 files changed, 34 insertions(+), 80 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bbb697fc..43730ea3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (api) [\#687](https://github.com/cosmos/ethermint/issues/687) Returns error for a transaction with an incorrect nonce. * (evm) [\#674](https://github.com/cosmos/ethermint/issues/674) Reset all cache after account data has been committed in `EndBlock` to make sure every node state consistent. * (evm) [\#672](https://github.com/cosmos/ethermint/issues/672) Fix panic of `wrong Block.Header.AppHash` when restart a node with snapshot. +* (evm) [\#775](https://github.com/cosmos/ethermint/issues/775) MisUse of headHash as blockHash when create EVM context. ### Features * (api) [\#821](https://github.com/cosmos/ethermint/pull/821) Individually enable the api modules. Will be implemented in the latest version of ethermint with the upcoming stargate upgrade. diff --git a/importer/importer_test.go b/importer/importer_test.go index 005f893d..57565373 100644 --- a/importer/importer_test.go +++ b/importer/importer_test.go @@ -254,7 +254,8 @@ func TestImportBlocks(t *testing.T) { } for i, tx := range block.Transactions() { - evmKeeper.Prepare(ctx, tx.Hash(), block.Hash(), i) + evmKeeper.Prepare(ctx, tx.Hash(), i) + evmKeeper.CommitStateDB.SetBlockHash(block.Hash()) receipt, gas, err := applyTransaction( chainConfig, chainContext, nil, gp, evmKeeper, header, tx, usedGas, vmConfig, diff --git a/x/evm/handler.go b/x/evm/handler.go index e4e25f70..d2d92ab1 100644 --- a/x/evm/handler.go +++ b/x/evm/handler.go @@ -106,8 +106,7 @@ func handleMsgEthermint(ctx sdk.Context, k *Keeper, msg types.MsgEthermint) (*sd if !st.Simulate { // Prepare db for logs - blockHash := types.HashFromContext(ctx) - k.CommitStateDB.Prepare(ethHash, blockHash, k.TxCount) + k.CommitStateDB.Prepare(ethHash, k.TxCount) k.TxCount++ } diff --git a/x/evm/keeper/abci.go b/x/evm/keeper/abci.go index 6a8baf52..920e28c6 100644 --- a/x/evm/keeper/abci.go +++ b/x/evm/keeper/abci.go @@ -22,11 +22,12 @@ func (k *Keeper) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) // Set the hash -> height and height -> hash mapping. - hash := req.Header.LastBlockId.GetHash() - height := req.Header.GetHeight() - 1 + currentHash := req.Hash + height := req.Header.GetHeight() - k.SetHeightHash(ctx, uint64(height), common.BytesToHash(hash)) - k.SetBlockHash(ctx, hash, height) + k.SetHeightHash(ctx, uint64(height), common.BytesToHash(currentHash)) + k.SetBlockHash(ctx, currentHash, height) + k.CommitStateDB.SetBlockHash(common.BytesToHash(currentHash)) // reset counters that are used on CommitStateDB.Prepare k.Bloom = big.NewInt(0) diff --git a/x/evm/keeper/abci_test.go b/x/evm/keeper/abci_test.go index c86fc7bb..95564b05 100644 --- a/x/evm/keeper/abci_test.go +++ b/x/evm/keeper/abci_test.go @@ -8,10 +8,11 @@ func (suite *KeeperTestSuite) TestBeginBlock() { req := abci.RequestBeginBlock{ Header: abci.Header{ LastBlockId: abci.BlockID{ - Hash: []byte("hash"), + Hash: []byte("last hash"), }, Height: 10, }, + Hash: []byte("hash"), } // get the initial consumption @@ -33,9 +34,9 @@ func (suite *KeeperTestSuite) TestBeginBlock() { suite.Require().Equal(int64(initialConsumed), int64(suite.ctx.GasMeter().GasConsumed())) - lastHeight, found := suite.app.EvmKeeper.GetBlockHash(suite.ctx, req.Header.LastBlockId.Hash) + lastHeight, found := suite.app.EvmKeeper.GetBlockHash(suite.ctx, req.Hash) suite.Require().True(found) - suite.Require().Equal(int64(9), lastHeight) + suite.Require().Equal(int64(10), lastHeight) } func (suite *KeeperTestSuite) TestEndBlock() { diff --git a/x/evm/keeper/msg_server.go b/x/evm/keeper/msg_server.go index 94273202..b2b1bd25 100644 --- a/x/evm/keeper/msg_server.go +++ b/x/evm/keeper/msg_server.go @@ -52,8 +52,7 @@ func (k Keeper) EthereumTx(ctx sdk.Context, msg types.MsgEthereumTx) (*sdk.Resul // other nodes, causing a consensus error if !st.Simulate { // Prepare db for logs - blockHash := types.HashFromContext(ctx) - k.CommitStateDB.Prepare(ethHash, blockHash, k.TxCount) + k.CommitStateDB.Prepare(ethHash, k.TxCount) k.TxCount++ } diff --git a/x/evm/keeper/statedb.go b/x/evm/keeper/statedb.go index 272404b6..1deb6455 100644 --- a/x/evm/keeper/statedb.go +++ b/x/evm/keeper/statedb.go @@ -225,8 +225,8 @@ func (k *Keeper) Reset(ctx sdk.Context, root ethcmn.Hash) error { } // Prepare calls CommitStateDB.Prepare using the passed in context -func (k *Keeper) Prepare(ctx sdk.Context, thash, bhash ethcmn.Hash, txi int) { - k.CommitStateDB.WithContext(ctx).Prepare(thash, bhash, txi) +func (k *Keeper) Prepare(ctx sdk.Context, thash ethcmn.Hash, txi int) { + k.CommitStateDB.WithContext(ctx).Prepare(thash, txi) } // CreateAccount calls CommitStateDB.CreateAccount using the passed in context diff --git a/x/evm/keeper/statedb_test.go b/x/evm/keeper/statedb_test.go index d67d71ad..0196f672 100644 --- a/x/evm/keeper/statedb_test.go +++ b/x/evm/keeper/statedb_test.go @@ -18,7 +18,7 @@ import ( func (suite *KeeperTestSuite) TestBloomFilter() { // Prepare db for logs tHash := ethcmn.BytesToHash([]byte{0x1}) - suite.app.EvmKeeper.Prepare(suite.ctx, tHash, ethcmn.Hash{}, 0) + suite.app.EvmKeeper.Prepare(suite.ctx, tHash, 0) contractAddress := ethcmn.BigToAddress(big.NewInt(1)) log := ethtypes.Log{Address: contractAddress} @@ -359,7 +359,8 @@ func (suite *KeeperTestSuite) TestSuiteDB_Prepare() { bhash := ethcmn.BytesToHash([]byte("bhash")) txi := 1 - suite.app.EvmKeeper.Prepare(suite.ctx, thash, bhash, txi) + suite.app.EvmKeeper.Prepare(suite.ctx, thash, txi) + suite.app.EvmKeeper.CommitStateDB.SetBlockHash(bhash) suite.Require().Equal(txi, suite.app.EvmKeeper.TxIndex(suite.ctx)) suite.Require().Equal(bhash, suite.app.EvmKeeper.BlockHash(suite.ctx)) diff --git a/x/evm/types/state_transition.go b/x/evm/types/state_transition.go index f9a5ffd8..1c07b2c6 100644 --- a/x/evm/types/state_transition.go +++ b/x/evm/types/state_transition.go @@ -48,16 +48,16 @@ type ExecutionResult struct { } // GetHashFn implements vm.GetHashFunc for Ethermint. It handles 3 cases: -// 1. The requested height matches the current height from context (and thus same epoch number) +// 1. The requested height matches the current height (and thus same epoch number) // 2. The requested height is from an previous height from the same chain epoch // 3. The requested height is from a height greater than the latest one func GetHashFn(ctx sdk.Context, csdb *CommitStateDB) vm.GetHashFunc { return func(height uint64) common.Hash { switch { case ctx.BlockHeight() == int64(height): - // Case 1: The requested height matches the one from the context so we can retrieve the header - // hash directly from the context. - return HashFromContext(ctx) + // Case 1: The requested height matches the one from the CommitStateDB so we can retrieve the block + // hash directly from the CommitStateDB. + return csdb.bhash case ctx.BlockHeight() > int64(height): // Case 2: if the chain is not the current height we need to retrieve the hash from the store for the @@ -268,21 +268,3 @@ func (st StateTransition) TransitionDb(ctx sdk.Context, config ChainConfig) (*Ex return executionResult, nil } - -// HashFromContext returns the Ethereum Header hash from the context's Tendermint -// block header. -func HashFromContext(ctx sdk.Context) common.Hash { - // cast the ABCI header to tendermint Header type - tmHeader := AbciHeaderToTendermint(ctx.BlockHeader()) - - // get the Tendermint block hash from the current header - tmBlockHash := tmHeader.Hash() - - // NOTE: if the validator set hash is missing the hash will be returned as nil, - // so we need to check for this case to prevent a panic when calling Bytes() - if tmBlockHash == nil { - return common.Hash{} - } - - return common.BytesToHash(tmBlockHash.Bytes()) -} diff --git a/x/evm/types/state_transition_test.go b/x/evm/types/state_transition_test.go index b9e36463..841d23e1 100644 --- a/x/evm/types/state_transition_test.go +++ b/x/evm/types/state_transition_test.go @@ -34,6 +34,8 @@ func (suite *StateDBTestSuite) TestGetHashFn() { ValidatorsHash: []byte("val_hash"), }, ) + hash := ethcmn.BytesToHash([]byte("test hash")) + suite.stateDB.SetBlockHash(hash) }, false, }, @@ -54,7 +56,7 @@ func (suite *StateDBTestSuite) TestGetHashFn() { ValidatorsHash: []byte("val_hash"), }, ) - hash := types.HashFromContext(suite.ctx) + hash := ethcmn.BytesToHash([]byte("test hash")) suite.stateDB.WithContext(suite.ctx).SetHeightHash(1, hash) }, false, diff --git a/x/evm/types/statedb.go b/x/evm/types/statedb.go index 498d73f9..72d46af4 100644 --- a/x/evm/types/statedb.go +++ b/x/evm/types/statedb.go @@ -342,6 +342,10 @@ func (csdb *CommitStateDB) BlockHash() ethcmn.Hash { return csdb.bhash } +func (csdb *CommitStateDB) SetBlockHash(hash ethcmn.Hash) { + csdb.bhash = hash +} + // GetCode returns the code for a given account. func (csdb *CommitStateDB) GetCode(addr ethcmn.Address) []byte { so := csdb.getStateObject(addr) @@ -733,9 +737,8 @@ func (csdb *CommitStateDB) clearJournalAndRefund() { // Prepare sets the current transaction hash and index and block hash which is // used when the EVM emits new state logs. -func (csdb *CommitStateDB) Prepare(thash, bhash ethcmn.Hash, txi int) { +func (csdb *CommitStateDB) Prepare(thash ethcmn.Hash, txi int) { csdb.thash = thash - csdb.bhash = bhash csdb.txIndex = txi } diff --git a/x/evm/types/statedb_test.go b/x/evm/types/statedb_test.go index cce16082..80d863bc 100644 --- a/x/evm/types/statedb_test.go +++ b/x/evm/types/statedb_test.go @@ -81,7 +81,7 @@ func (suite *StateDBTestSuite) TestGetHeightHash() { func (suite *StateDBTestSuite) TestBloomFilter() { // Prepare db for logs tHash := ethcmn.BytesToHash([]byte{0x1}) - suite.stateDB.Prepare(tHash, ethcmn.Hash{}, 0) + suite.stateDB.Prepare(tHash, 0) contractAddress := ethcmn.BigToAddress(big.NewInt(1)) log := ethtypes.Log{Address: contractAddress} @@ -419,7 +419,8 @@ func (suite *StateDBTestSuite) TestSuiteDB_Prepare() { bhash := ethcmn.BytesToHash([]byte("bhash")) txi := 1 - suite.stateDB.Prepare(thash, bhash, txi) + suite.stateDB.Prepare(thash, txi) + suite.stateDB.SetBlockHash(bhash) suite.Require().Equal(txi, suite.stateDB.TxIndex()) suite.Require().Equal(bhash, suite.stateDB.BlockHash()) diff --git a/x/evm/types/utils.go b/x/evm/types/utils.go index 051b7dae..529abc01 100644 --- a/x/evm/types/utils.go +++ b/x/evm/types/utils.go @@ -8,10 +8,6 @@ import ( "github.com/pkg/errors" "golang.org/x/crypto/sha3" - abci "github.com/tendermint/tendermint/abci/types" - tmtypes "github.com/tendermint/tendermint/types" - "github.com/tendermint/tendermint/version" - "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -162,36 +158,3 @@ func recoverEthSig(R, S, Vb *big.Int, sigHash ethcmn.Hash) (ethcmn.Address, erro return addr, nil } - -// AbciHeaderToTendermint is a util function to parse a tendermint ABCI Header to -// tendermint types Header. -func AbciHeaderToTendermint(header abci.Header) tmtypes.Header { - return tmtypes.Header{ - Version: version.Consensus{ - Block: version.Protocol(header.Version.Block), - App: version.Protocol(header.Version.App), - }, - ChainID: header.ChainID, - Height: header.Height, - Time: header.Time, - - LastBlockID: tmtypes.BlockID{ - Hash: header.LastBlockId.Hash, - PartsHeader: tmtypes.PartSetHeader{ - Total: int(header.LastBlockId.PartsHeader.Total), - Hash: header.LastBlockId.PartsHeader.Hash, - }, - }, - LastCommitHash: header.LastCommitHash, - DataHash: header.DataHash, - - ValidatorsHash: header.ValidatorsHash, - NextValidatorsHash: header.NextValidatorsHash, - ConsensusHash: header.ConsensusHash, - AppHash: header.AppHash, - LastResultsHash: header.LastResultsHash, - - EvidenceHash: header.EvidenceHash, - ProposerAddress: header.ProposerAddress, - } -}