From d274c76ac556506e5e803586f50cfa0d3b583445 Mon Sep 17 00:00:00 2001 From: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Date: Tue, 20 Oct 2020 19:53:13 +0200 Subject: [PATCH] evm: fix begin and endblock (#583) * evm: fix begin and endblock * fix tests and changelog * fix gas * update GetBlockBloom --- CHANGELOG.md | 1 + x/evm/abci.go | 49 ---------------------------------- x/evm/keeper/abci.go | 55 ++++++++++++++++++++++++++++++++++++++ x/evm/keeper/abci_test.go | 56 +++++++++++++++++++++++++++++++++++++++ x/evm/keeper/keeper.go | 2 +- x/evm/module.go | 4 +-- 6 files changed, 115 insertions(+), 52 deletions(-) delete mode 100644 x/evm/abci.go create mode 100644 x/evm/keeper/abci.go create mode 100644 x/evm/keeper/abci_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index f0a50c33..53f55c2c 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/x/evm/abci.go b/x/evm/abci.go deleted file mode 100644 index 61c40c7b..00000000 --- a/x/evm/abci.go +++ /dev/null @@ -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{} -} diff --git a/x/evm/keeper/abci.go b/x/evm/keeper/abci.go new file mode 100644 index 00000000..21136fee --- /dev/null +++ b/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{} +} diff --git a/x/evm/keeper/abci_test.go b/x/evm/keeper/abci_test.go new file mode 100644 index 00000000..c86fc7bb --- /dev/null +++ b/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()) + +} diff --git a/x/evm/keeper/keeper.go b/x/evm/keeper/keeper.go index 18bfd945..1b49ec3a 100644 --- a/x/evm/keeper/keeper.go +++ b/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)) diff --git a/x/evm/module.go b/x/evm/module.go index e1108e9e..6b5de639 100644 --- a/x/evm/module.go +++ b/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