evm: fix begin and endblock (#583)

* evm: fix begin and endblock

* fix tests and changelog

* fix gas

* update GetBlockBloom

CHANGELOG.md

@@ -45,6 +45,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Bug Fixes
 
+* (evm) [\#583](https://github.com/cosmos/ethermint/pull/583) Fixes incorrect resetting of tx count and block bloom during `BeginBlock`, as well as gas consumption.
 * (crypto) [\#577](https://github.com/cosmos/ethermint/pull/577) Fix `BIP44HDPath` that did not prepend `m/` to the path. This now uses the `DefaultBaseDerivationPath` variable from go-ethereum to ensure addresses are consistent.
 
 ## [v0.2.1] - 2020-09-30

x/evm/abci.go

@@ -1,49 +0,0 @@
-package evm
-
-import (
-	"math/big"
-
-	abci "github.com/tendermint/tendermint/abci/types"
-
-	sdk "github.com/cosmos/cosmos-sdk/types"
-
-	ethtypes "github.com/ethereum/go-ethereum/core/types"
-)
-
-// 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
-	}
-
-	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 {
-	// Gas costs are handled within msg handler so costs should be ignored
-	ctx = ctx.WithBlockGasMeter(sdk.NewInfiniteGasMeter())
-
-	// Update account balances before committing other parts of state
-	k.UpdateAccounts(ctx)
-
-	// Commit state objects to KV store
-	_, err := k.Commit(ctx, true)
-	if err != nil {
-		panic(err)
-	}
-
-	// Clear accounts cache after account data has been committed
-	k.ClearStateObjects(ctx)
-
-	bloom := ethtypes.BytesToBloom(k.Bloom.Bytes())
-	k.SetBlockBloom(ctx, ctx.BlockHeight(), bloom)
-
-	return []abci.ValidatorUpdate{}
-}

x/evm/keeper/abci.go

@@ -0,0 +1,55 @@
+package keeper
+
+import (
+	"math/big"
+
+	abci "github.com/tendermint/tendermint/abci/types"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	ethtypes "github.com/ethereum/go-ethereum/core/types"
+)
+
+// BeginBlock sets the block hash -> block height map for the previous block height
+// and resets the Bloom filter and the transaction count to 0.
+func (k *Keeper) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {
+	if req.Header.LastBlockId.GetHash() == nil || req.Header.GetHeight() < 1 {
+		return
+	}
+
+	// Gas costs are handled within msg handler so costs should be ignored
+	ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
+
+	k.SetBlockHash(ctx, req.Header.LastBlockId.GetHash(), req.Header.GetHeight()-1)
+
+	// reset counters that are used on CommitStateDB.Prepare
+	k.Bloom = big.NewInt(0)
+	k.TxCount = 0
+}
+
+// EndBlock updates the accounts and commits state objects to the KV Store, while
+// deleting the empty ones. It also sets the bloom filers for the request block to
+// the store. The EVM end block loginc doesn't update the validator set, thus it returns
+// an empty slice.
+func (k Keeper) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) []abci.ValidatorUpdate {
+	// Gas costs are handled within msg handler so costs should be ignored
+	ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
+
+	// Update account balances before committing other parts of state
+	k.UpdateAccounts(ctx)
+
+	// Commit state objects to KV store
+	_, err := k.Commit(ctx, true)
+	if err != nil {
+		panic(err)
+	}
+
+	// Clear accounts cache after account data has been committed
+	k.ClearStateObjects(ctx)
+
+	// set the block bloom filter bytes to store
+	bloom := ethtypes.BytesToBloom(k.Bloom.Bytes())
+	k.SetBlockBloom(ctx, req.Height, bloom)
+
+	return []abci.ValidatorUpdate{}
+}

x/evm/keeper/abci_test.go

@@ -0,0 +1,56 @@
+package keeper_test
+
+import (
+	abci "github.com/tendermint/tendermint/abci/types"
+)
+
+func (suite *KeeperTestSuite) TestBeginBlock() {
+	req := abci.RequestBeginBlock{
+		Header: abci.Header{
+			LastBlockId: abci.BlockID{
+				Hash: []byte("hash"),
+			},
+			Height: 10,
+		},
+	}
+
+	// get the initial consumption
+	initialConsumed := suite.ctx.GasMeter().GasConsumed()
+
+	// update the counters
+	suite.app.EvmKeeper.Bloom.SetInt64(10)
+	suite.app.EvmKeeper.TxCount = 10
+
+	suite.app.EvmKeeper.BeginBlock(suite.ctx, abci.RequestBeginBlock{})
+	suite.Require().NotZero(suite.app.EvmKeeper.Bloom.Int64())
+	suite.Require().NotZero(suite.app.EvmKeeper.TxCount)
+
+	suite.Require().Equal(int64(initialConsumed), int64(suite.ctx.GasMeter().GasConsumed()))
+
+	suite.app.EvmKeeper.BeginBlock(suite.ctx, req)
+	suite.Require().Zero(suite.app.EvmKeeper.Bloom.Int64())
+	suite.Require().Zero(suite.app.EvmKeeper.TxCount)
+
+	suite.Require().Equal(int64(initialConsumed), int64(suite.ctx.GasMeter().GasConsumed()))
+
+	lastHeight, found := suite.app.EvmKeeper.GetBlockHash(suite.ctx, req.Header.LastBlockId.Hash)
+	suite.Require().True(found)
+	suite.Require().Equal(int64(9), lastHeight)
+}
+
+func (suite *KeeperTestSuite) TestEndBlock() {
+	// update the counters
+	suite.app.EvmKeeper.Bloom.SetInt64(10)
+
+	// set gas limit to 1 to ensure no gas is consumed during the operation
+	initialConsumed := suite.ctx.GasMeter().GasConsumed()
+
+	_ = suite.app.EvmKeeper.EndBlock(suite.ctx, abci.RequestEndBlock{Height: 100})
+
+	suite.Require().Equal(int64(initialConsumed), int64(suite.ctx.GasMeter().GasConsumed()))
+
+	bloom, found := suite.app.EvmKeeper.GetBlockBloom(suite.ctx, 100)
+	suite.Require().True(found)
+	suite.Require().Equal(int64(10), bloom.Big().Int64())
+
+}

x/evm/keeper/keeper.go

@@ -98,7 +98,7 @@ func (k Keeper) GetBlockBloom(ctx sdk.Context, height int64) (ethtypes.Bloom, bo
 	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixBloom)
 	has := store.Has(types.BloomKey(height))
 	if !has {
-		return ethtypes.Bloom{}, true // TODO: sometimes bloom cannot be found, fix this
+		return ethtypes.Bloom{}, false
 	}
 
 	bz := store.Get(types.BloomKey(height))

x/evm/module.go

@@ -113,12 +113,12 @@ func (am AppModule) NewQuerierHandler() sdk.Querier {
 
 // BeginBlock function for module at start of each block
 func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {
-	BeginBlock(am.keeper, ctx, req)
+	am.keeper.BeginBlock(ctx, req)
 }
 
 // EndBlock function for module at end of block
 func (am AppModule) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) []abci.ValidatorUpdate {
-	return EndBlock(am.keeper, ctx, req)
+	return am.keeper.EndBlock(ctx, req)
 }
 
 // InitGenesis instantiates the genesis state