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

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

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,

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, k.TxCount)
-		k.CommitStateDB.Prepare(ethHash, blockHash, k.TxCount)
 		k.TxCount++
 	}
 

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()
+	currentHash := req.Hash
-	height := req.Header.GetHeight() - 1
+	height := req.Header.GetHeight()
 
-	k.SetHeightHash(ctx, uint64(height), common.BytesToHash(hash))
+	k.SetHeightHash(ctx, uint64(height), common.BytesToHash(currentHash))
-	k.SetBlockHash(ctx, hash, height)
+	k.SetBlockHash(ctx, currentHash, height)
+	k.CommitStateDB.SetBlockHash(common.BytesToHash(currentHash))
 
 	// reset counters that are used on CommitStateDB.Prepare
 	k.Bloom = big.NewInt(0)

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() {

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, k.TxCount)
-		k.CommitStateDB.Prepare(ethHash, blockHash, k.TxCount)
 		k.TxCount++
 	}
 

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) {
+func (k *Keeper) Prepare(ctx sdk.Context, thash ethcmn.Hash, txi int) {
-	k.CommitStateDB.WithContext(ctx).Prepare(thash, bhash, txi)
+	k.CommitStateDB.WithContext(ctx).Prepare(thash, txi)
 }
 
 // CreateAccount calls CommitStateDB.CreateAccount using the passed in context

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

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
+			// Case 1: The requested height matches the one from the CommitStateDB so we can retrieve the block
-			// hash directly from the context.
+			// hash directly from the CommitStateDB.
-			return HashFromContext(ctx)
+			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())
-}

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,

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
 }
 

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

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