evm: various fixes

CHANGELOG.md

@@ -56,6 +56,11 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (`x/evm`) [\#255](https://github.com/ChainSafe/ethermint/pull/255) Add missing `GenesisState` fields and support `ExportGenesis` functionality.
 * [\#272](https://github.com/ChainSafe/ethermint/pull/272) Add `Logger` for evm module.
 * [\#317](https://github.com/ChainSafe/ethermint/pull/317) `GenesisAccount` validation.
+* (`x/evm`) [\#319](https://github.com/ChainSafe/ethermint/pull/319) Verious evm improvements:
+  * Add transaction `[]*ethtypes.Logs` to evm's `GenesisState` to persist logs after an upgrade.
+  * Remove evm `CodeKey` and `BlockKey`in favor of a prefix `Store`.
+  * Set `BlockBloom` during `EndBlock` instead of `BeginBlock`.
+  * `Commit` state object and `Finalize` storage after `InitGenesis` setup.
 
 ### Features
 
@@ -73,5 +78,6 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Bug Fixes
 
+* (`x/evm`) [\#319](https://github.com/ChainSafe/ethermint/pull/319) Fix `SetBlockHash` that was setting the incorrect height during `BeginBlock`.
 * (x/evm) [\#176](https://github.com/ChainSafe/ethermint/issues/176) Updated Web3 transaction hash from using RLP hash. Now all transaction hashes exposed are amino hashes.
   * Removes `Hash()` (RLP) function from `MsgEthereumTx` to avoid confusion or misuse in future.

app/ethermint.go

@@ -147,10 +147,9 @@ func NewEthermintApp(
 	keys := sdk.NewKVStoreKeys(
 		bam.MainStoreKey, auth.StoreKey, bank.StoreKey, staking.StoreKey,
 		supply.StoreKey, mint.StoreKey, distr.StoreKey, slashing.StoreKey,
-		gov.StoreKey, params.StoreKey, evidence.StoreKey, evm.CodeKey, evm.StoreKey,
+		gov.StoreKey, params.StoreKey, evidence.StoreKey, evm.StoreKey,
 		faucet.StoreKey,
 	)
-	blockKey := sdk.NewKVStoreKey(evm.BlockKey)
 
 	tkeys := sdk.NewTransientStoreKeys(params.TStoreKey)
 
@@ -203,7 +202,7 @@ func NewEthermintApp(
 		app.subspaces[crisis.ModuleName], invCheckPeriod, app.SupplyKeeper, auth.FeeCollectorName,
 	)
 	app.EvmKeeper = evm.NewKeeper(
-		app.cdc, blockKey, keys[evm.CodeKey], keys[evm.StoreKey], app.AccountKeeper,
+		app.cdc, keys[evm.StoreKey], app.AccountKeeper,
 		app.BankKeeper,
 	)
 	// TODO: use protobuf
@@ -299,10 +298,6 @@ func NewEthermintApp(
 	app.MountKVStores(keys)
 	app.MountTransientStores(tkeys)
 
-	// Mount block hash mapping key as DB (no need for historical queries)
-	// TODO: why does this need to be always StoreTypeDB?
-	app.MountStore(blockKey, sdk.StoreTypeDB)
-
 	// initialize BaseApp
 	app.SetInitChainer(app.InitChainer)
 	app.SetBeginBlocker(app.BeginBlocker)

x/evm/abci.go

@@ -10,23 +10,23 @@ import (
 	ethtypes "github.com/ethereum/go-ethereum/core/types"
 )
 
-// BeginBlock sets the Bloom and Hash mappings and resets the Bloom filter and
+// BeginBlock sets the block hash -> block height map and resets the Bloom filter and
 // the transaction count to 0.
 func BeginBlock(k Keeper, ctx sdk.Context, req abci.RequestBeginBlock) {
 	if req.Header.LastBlockId.GetHash() == nil || req.Header.GetHeight() < 1 {
 		return
 	}
 
-	// Consider removing this when using evm as module without web3 API
-	bloom := ethtypes.BytesToBloom(k.Bloom.Bytes())
-	k.SetBlockBloomMapping(ctx, bloom, req.Header.GetHeight())
-	k.SetBlockHashMapping(ctx, req.Header.LastBlockId.GetHash(), req.Header.GetHeight())
+	k.SetBlockHash(ctx, req.Header.LastBlockId.GetHash(), req.Header.GetHeight()-1)
+
+	// reset counters
 	k.Bloom = big.NewInt(0)
 	k.TxCount = 0
 }
 
-// EndBlock updates the accounts and commits states objects to the KV Store
-func EndBlock(k Keeper, ctx sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate {
+// EndBlock updates the accounts and commits states objects to the KV Store.
+//
+func EndBlock(k Keeper, ctx sdk.Context, req abci.RequestEndBlock) []abci.ValidatorUpdate {
 	// Gas costs are handled within msg handler so costs should be ignored
 	ctx = ctx.WithBlockGasMeter(sdk.NewInfiniteGasMeter())
 
@@ -42,5 +42,8 @@ func EndBlock(k Keeper, ctx sdk.Context, _ abci.RequestEndBlock) []abci.Validato
 	// Clear accounts cache after account data has been committed
 	k.CommitStateDB.ClearStateObjects()
 
+	bloom := ethtypes.BytesToBloom(k.Bloom.Bytes())
+	k.SetBlockBloom(ctx, ctx.BlockHeight(), bloom)
+
 	return []abci.ValidatorUpdate{}
 }

x/evm/alias.go

@@ -9,8 +9,6 @@ import (
 const (
 	ModuleName           = types.ModuleName
 	StoreKey             = types.StoreKey
-	CodeKey              = types.StoreKey
-	BlockKey             = types.BlockKey
 	RouterKey            = types.RouterKey
 	QueryProtocolVersion = types.QueryProtocolVersion
 	QueryBalance         = types.QueryBalance
@@ -20,7 +18,7 @@ const (
 	QueryNonce           = types.QueryNonce
 	QueryHashToHeight    = types.QueryHashToHeight
 	QueryTransactionLogs = types.QueryTransactionLogs
-	QueryLogsBloom       = types.QueryLogsBloom
+	QueryLogsBloom       = types.QueryBloom
 	QueryLogs            = types.QueryLogs
 	QueryAccount         = types.QueryAccount
 )

x/evm/genesis.go

@@ -13,15 +13,29 @@ import (
 // InitGenesis initializes genesis state based on exported genesis
 func InitGenesis(ctx sdk.Context, k Keeper, data GenesisState) []abci.ValidatorUpdate {
 	for _, account := range data.Accounts {
-		csdb := k.CommitStateDB.WithContext(ctx)
 		// FIXME: this will override bank InitGenesis balance!
-		csdb.SetBalance(account.Address, account.Balance)
-		csdb.SetCode(account.Address, account.Code)
+		k.SetBalance(ctx, account.Address, account.Balance)
+		k.SetCode(ctx, account.Address, account.Code)
 		for _, storage := range account.Storage {
-			csdb.SetState(account.Address, storage.Key, storage.Value)
+			k.SetState(ctx, account.Address, storage.Key, storage.Value)
 		}
 	}
-	// TODO: Commit?
+
+	for _, txLog := range data.TxsLogs {
+		k.SetLogs(ctx, txLog.Hash, txLog.Logs)
+	}
+
+	// set state objects and code to store
+	_, err := k.Commit(ctx, false)
+	if err != nil {
+		panic(err)
+	}
+
+	// set storage to store
+	err = k.Finalise(ctx, true)
+	if err != nil {
+		panic(err)
+	}
 	return []abci.ValidatorUpdate{}
 }
 
@@ -59,5 +73,8 @@ func ExportGenesis(ctx sdk.Context, k Keeper, ak types.AccountKeeper) GenesisSta
 		ethGenAccounts = append(ethGenAccounts, genAccount)
 	}
 
-	return GenesisState{Accounts: ethGenAccounts}
+	return GenesisState{
+		Accounts: ethGenAccounts,
+		TxsLogs:  k.GetAllTxLogs(ctx),
+	}
 }

x/evm/handler.go

@@ -74,7 +74,7 @@ func handleMsgEthereumTx(ctx sdk.Context, k Keeper, msg types.MsgEthereumTx) (*s
 	k.Bloom.Or(k.Bloom, executionResult.Bloom)
 
 	// update transaction logs in KVStore
-	k.SetTransactionLogs(ctx, txHash, executionResult.Logs)
+	k.SetLogs(ctx, common.BytesToHash(txHash), executionResult.Logs)
 
 	// log successful execution
 	k.Logger(ctx).Info(executionResult.Result.Log)
@@ -147,7 +147,7 @@ func handleMsgEthermint(ctx sdk.Context, k Keeper, msg types.MsgEthermint) (*sdk
 	k.Bloom.Or(k.Bloom, executionResult.Bloom)
 
 	// update transaction logs in KVStore
-	k.SetTransactionLogs(ctx, txHash, executionResult.Logs)
+	k.SetLogs(ctx, common.BytesToHash(txHash), executionResult.Logs)
 
 	// log successful execution
 	k.Logger(ctx).Info(executionResult.Result.Log)

x/evm/handler_test.go

@@ -236,9 +236,9 @@ func (suite *EvmTestSuite) TestHandlerLogs() {
 	suite.Require().Equal(len(resultData.Logs[0].Topics), 2)
 
 	hash := []byte{1}
-	suite.app.EvmKeeper.SetTransactionLogs(suite.ctx, hash, resultData.Logs)
+	suite.app.EvmKeeper.SetLogs(suite.ctx, ethcmn.BytesToHash(hash), resultData.Logs)
 
-	logs, err := suite.app.EvmKeeper.GetTransactionLogs(suite.ctx, hash)
+	logs, err := suite.app.EvmKeeper.GetLogs(suite.ctx, ethcmn.BytesToHash(hash))
 	suite.Require().NoError(err, "failed to get logs")
 
 	suite.Require().Equal(logs, resultData.Logs)
@@ -273,7 +273,7 @@ func (suite *EvmTestSuite) TestQueryTxLogs() {
 	// get logs by tx hash
 	hash := resultData.TxHash.Bytes()
 
-	logs, err := suite.app.EvmKeeper.GetTransactionLogs(suite.ctx, hash)
+	logs, err := suite.app.EvmKeeper.GetLogs(suite.ctx, ethcmn.BytesToHash(hash))
 	suite.Require().NoError(err, "failed to get logs")
 
 	suite.Require().Equal(logs, resultData.Logs)

x/evm/keeper/keeper.go

@@ -2,30 +2,33 @@ package keeper
 
 import (
 	"encoding/binary"
-	"errors"
 	"fmt"
+	"math/big"
 
 	"github.com/tendermint/tendermint/libs/log"
 
 	"github.com/cosmos/cosmos-sdk/codec"
+	"github.com/cosmos/cosmos-sdk/store/prefix"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	"github.com/cosmos/ethermint/x/evm/types"
 
 	ethcmn "github.com/ethereum/go-ethereum/common"
 	ethtypes "github.com/ethereum/go-ethereum/core/types"
-
-	"math/big"
 )
 
 // Keeper wraps the CommitStateDB, allowing us to pass in SDK context while adhering
 // to the StateDB interface.
 type Keeper struct {
 	// Amino codec
 	cdc *codec.Codec
-	// Store key required to update the block bloom filter mappings needed for the
-	// Web3 API
-	blockKey      sdk.StoreKey
+	// Store key required for the EVM Prefix KVStore. It is required by:
+	// - storing Account's Storage State
+	// - storing Account's Code
+	// - storing transaction Logs
+	// - storing block height -> bloom filter map. Needed for the Web3 API.
+	// - storing block hash -> block height map. Needed for the Web3 API.
+	storeKey      sdk.StoreKey
 	CommitStateDB *types.CommitStateDB
 	// Transaction counter in a block. Used on StateSB's Prepare function.
 	// It is reset to 0 every block on BeginBlock so there's no point in storing the counter
@@ -36,13 +39,12 @@ type Keeper struct {
 
 // NewKeeper generates new evm module keeper
 func NewKeeper(
-	cdc *codec.Codec, blockKey, codeKey, storeKey sdk.StoreKey,
-	ak types.AccountKeeper, bk types.BankKeeper,
+	cdc *codec.Codec, storeKey sdk.StoreKey, ak types.AccountKeeper, bk types.BankKeeper,
 ) Keeper {
 	return Keeper{
 		cdc:           cdc,
-		blockKey:      blockKey,
-		CommitStateDB: types.NewCommitStateDB(sdk.Context{}, codeKey, storeKey, ak, bk),
+		storeKey:      storeKey,
+		CommitStateDB: types.NewCommitStateDB(sdk.Context{}, storeKey, ak, bk),
 		TxCount:       0,
 		Bloom:         big.NewInt(0),
 	}
@@ -55,68 +57,66 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger {
 
 // ----------------------------------------------------------------------------
 // Block hash mapping functions
-// May be removed when using only as module (only required by rpc api)
+// Required by Web3 API.
+//  TODO: remove once tendermint support block queries by hash.
 // ----------------------------------------------------------------------------
 
-// GetBlockHashMapping gets block height from block consensus hash
-func (k Keeper) GetBlockHashMapping(ctx sdk.Context, hash []byte) (int64, error) {
-	store := ctx.KVStore(k.blockKey)
+// GetBlockHash gets block height from block consensus hash
+func (k Keeper) GetBlockHash(ctx sdk.Context, hash []byte) (int64, bool) {
+	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixBlockHash)
 	bz := store.Get(hash)
 	if len(bz) == 0 {
-		return 0, fmt.Errorf("block with hash '%s' not found", ethcmn.BytesToHash(hash).Hex())
+		return 0, false
 	}
 
 	height := binary.BigEndian.Uint64(bz)
-	return int64(height), nil
+	return int64(height), true
 }
 
-// SetBlockHashMapping sets the mapping from block consensus hash to block height
-func (k Keeper) SetBlockHashMapping(ctx sdk.Context, hash []byte, height int64) {
-	store := ctx.KVStore(k.blockKey)
+// SetBlockHash sets the mapping from block consensus hash to block height
+func (k Keeper) SetBlockHash(ctx sdk.Context, hash []byte, height int64) {
+	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixBlockHash)
 	bz := sdk.Uint64ToBigEndian(uint64(height))
 	store.Set(hash, bz)
 }
 
 // ----------------------------------------------------------------------------
 // Block bloom bits mapping functions
-// May be removed when using only as module (only required by rpc api)
+// Required by Web3 API.
 // ----------------------------------------------------------------------------
 
-// GetBlockBloomMapping gets bloombits from block height
-func (k Keeper) GetBlockBloomMapping(ctx sdk.Context, height int64) (ethtypes.Bloom, error) {
-	store := ctx.KVStore(k.blockKey)
-	heightBz := sdk.Uint64ToBigEndian(uint64(height))
-	bz := store.Get(types.BloomKey(heightBz))
+// GetBlockBloom gets bloombits from block height
+func (k Keeper) GetBlockBloom(ctx sdk.Context, height int64) (ethtypes.Bloom, bool) {
+	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixBloom)
+	bz := store.Get(types.BloomKey(height))
 	if len(bz) == 0 {
-		return ethtypes.Bloom{}, fmt.Errorf("block at height %d not found", height)
+		return ethtypes.Bloom{}, false
 	}
 
-	return ethtypes.BytesToBloom(bz), nil
+	return ethtypes.BytesToBloom(bz), true
 }
 
-// SetBlockBloomMapping sets the mapping from block height to bloom bits
-func (k Keeper) SetBlockBloomMapping(ctx sdk.Context, bloom ethtypes.Bloom, height int64) {
-	store := ctx.KVStore(k.blockKey)
-	heightBz := sdk.Uint64ToBigEndian(uint64(height))
-	store.Set(types.BloomKey(heightBz), bloom.Bytes())
+// SetBlockBloom sets the mapping from block height to bloom bits
+func (k Keeper) SetBlockBloom(ctx sdk.Context, height int64, bloom ethtypes.Bloom) {
+	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixBloom)
+	store.Set(types.BloomKey(height), bloom.Bytes())
 }
 
-// SetTransactionLogs sets the transaction's logs in the KVStore
-func (k *Keeper) SetTransactionLogs(ctx sdk.Context, hash []byte, logs []*ethtypes.Log) {
-	store := ctx.KVStore(k.blockKey)
-	bz := k.cdc.MustMarshalBinaryLengthPrefixed(logs)
-	store.Set(types.LogsKey(hash), bz)
-}
-
-// GetTransactionLogs gets the logs for a transaction from the KVStore
-func (k *Keeper) GetTransactionLogs(ctx sdk.Context, hash []byte) ([]*ethtypes.Log, error) {
-	store := ctx.KVStore(k.blockKey)
-	bz := store.Get(types.LogsKey(hash))
-	if len(bz) == 0 {
-		return nil, errors.New("cannot get transaction logs")
+// GetAllTxLogs return all the transaction logs from the store.
+func (k Keeper) GetAllTxLogs(ctx sdk.Context) []types.TransactionLogs {
+	store := ctx.KVStore(k.storeKey)
+	iterator := sdk.KVStorePrefixIterator(store, types.KeyPrefixLogs)
+	defer iterator.Close()
+
+	txsLogs := []types.TransactionLogs{}
+	for ; iterator.Valid(); iterator.Next() {
+		hash := ethcmn.BytesToHash(iterator.Key())
+		var logs []*ethtypes.Log
+		k.cdc.UnmarshalBinaryLengthPrefixed(iterator.Value(), &logs)
+
+		// add a new entry
+		txLog := types.NewTransactionLogs(hash, logs)
+		txsLogs = append(txsLogs, txLog)
 	}
-
-	var logs []*ethtypes.Log
-	k.cdc.MustUnmarshalBinaryLengthPrefixed(bz, &logs)
-	return logs, nil
+	return txsLogs
 }