x/evm: unit tests and fixes (#223)

* evm: move Keeper and Querier to /keeper package

* keeper: update keeper_test.go

* fix format

* evm: use aliased types

* bump SDK version to v0.38.1

* app: updates from new version

* errors: switch sdk.Error -> error

* errors: switch sdk.Error -> error. Continuation

* more fixes

* update app/

* update keys and client pkgs

* build

* fix tests

* lint

* minor changes

* changelog

* address @austinbell comments

* Fix keyring usage in rpc API and CLI

* fix keyring

* break line

* Misc cleanup (#188)

* evm: move Begin and EndBlock to abci.go

* evm: use expected keeper interfaces

* app: use EthermintApp for integration and unit test setup

* evm: remove count type; update codec

* go mod verify

* evm: rename msgs for consistency

* evm: events

* minor cleanup

* lint

* ante: update tests

* changelog

* nolint

* evm: update statedb to create ethermint Account instead of BaseAccount

* fix importer test

* address @austinabell comments

* update README

* changelog

* evm: update codec

* rename GasLimit->Gas and Price ->GasPrice

* msg cleanup and tests

* cleanup TxData

* fix marshaling

* revert rename

* move types

* evm/keeper: querier tests

* switch MarshalLengthPrefixed -> BinaryBare; remove panics

* fix event sender

* fix panic

* try fix txDecoder error

* evm: handler tests

* evm: handler MsgEthermint test

* fix tests

* store logs in keeper after transition (#210)

* add some comments

* begin log handler test

* update TransitionCSDB to return ReturnData

* use rlp for result data encode/decode

* update tests

* implement SetBlockLogs

* implement GetBlockLogs

* test log set/get

* update keeper get/set logs to use hash as key

* fix test

* move logsKey to csdb

* attempt to fix test

* attempt to fix test

* attempt to fix test

* lint

* lint

* lint

* save logs after handling msg

* update k.Logs

* cleanup

* remove unused

* fix issues

* comment out handler test

* address comments

* lint

* fix handler test

* address comments

* use amino

* lint

* address comments

* merge

* fix encoding bug

* minor fix

* rpc: error handling

* rpc: simulate only returns gasConsumed

* rpc: error ineffassign

* evm: handler test

* go: bump version to 1.14 and SDK version to latest master

* rpc: fix simulation return value

* breaking changes from SDK

* sdk: breaking changes; build

* tests: fixes

* minor fix

* proto: ethermint types attempt

* proto: define EthAccount proto type and extend sdk std.Codec

* evm: fix panic on handler test

* evm: minor state object changes

* cleanup

* tests: update test-importer

* fix evm test

* fix pubkey registration

* lint

* cleanup

* more test checks for importer

* minor change

* codec fixes

* rm init func

* fix importer test build

* fixes

* test fixes

* fix bloom key

* rm unnecesary func

* remove comment

Co-authored-by: austinabell <austinabell8@gmail.com>
Co-authored-by: noot <36753753+noot@users.noreply.github.com>

x/evm/keeper/keeper.go

@@ -1,7 +1,6 @@
 package keeper
 
 import (
-	"bytes"
 	"encoding/binary"
 	"errors"
 	"fmt"
@@ -50,25 +49,23 @@ func NewKeeper(
 // May be removed when using only as module (only required by rpc api)
 // ----------------------------------------------------------------------------
 
-// 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)
-	if !bytes.Equal(hash, []byte{}) {
-		bz := sdk.Uint64ToBigEndian(uint64(height))
-		store.Set(hash, bz)
-	}
-}
-
 // GetBlockHashMapping gets block height from block consensus hash
-func (k *Keeper) GetBlockHashMapping(ctx sdk.Context, hash []byte) (height int64) {
+func (k Keeper) GetBlockHashMapping(ctx sdk.Context, hash []byte) (int64, error) {
 	store := ctx.KVStore(k.blockKey)
 	bz := store.Get(hash)
-	if bytes.Equal(bz, []byte{}) {
-		panic(fmt.Errorf("block with hash %s not found", ethcmn.BytesToHash(hash)))
+	if len(bz) == 0 {
+		return 0, fmt.Errorf("block with hash '%s' not found", ethcmn.BytesToHash(hash))
 	}
 
-	height = int64(binary.BigEndian.Uint64(bz))
-	return height
+	height := binary.BigEndian.Uint64(bz)
+	return int64(height), nil
+}
+
+// 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)
+	bz := sdk.Uint64ToBigEndian(uint64(height))
+	store.Set(hash, bz)
 }
 
 // ----------------------------------------------------------------------------
@@ -76,28 +73,23 @@ func (k *Keeper) GetBlockHashMapping(ctx sdk.Context, hash []byte) (height int64
 // May be removed when using only as module (only required by rpc api)
 // ----------------------------------------------------------------------------
 
-// SetBlockBloomMapping sets the mapping from block height to bloom bits
-func (k *Keeper) SetBlockBloomMapping(ctx sdk.Context, bloom ethtypes.Bloom, height int64) error {
+// GetBlockBloomMapping gets bloombits from block height
+func (k Keeper) GetBlockBloomMapping(ctx sdk.Context, height int64) (ethtypes.Bloom, error) {
 	store := ctx.KVStore(k.blockKey)
-	bz := sdk.Uint64ToBigEndian(uint64(height))
+	heightBz := sdk.Uint64ToBigEndian(uint64(height))
+	bz := store.Get(types.BloomKey(heightBz))
 	if len(bz) == 0 {
-		return fmt.Errorf("block with bloombits %v not found", bloom)
+		return ethtypes.Bloom{}, fmt.Errorf("block at height %d not found", height)
 	}
 
-	store.Set(types.BloomKey(bz), bloom.Bytes())
-	return nil
+	return ethtypes.BytesToBloom(bz), nil
 }
 
-// GetBlockBloomMapping gets bloombits from block height
-func (k *Keeper) GetBlockBloomMapping(ctx sdk.Context, height int64) (ethtypes.Bloom, error) {
+// 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)
-	bz := sdk.Uint64ToBigEndian(uint64(height))
-	if len(bz) == 0 {
-		return ethtypes.BytesToBloom([]byte{}), fmt.Errorf("block with height %d not found", height)
-	}
-
-	bloom := store.Get(types.BloomKey(bz))
-	return ethtypes.BytesToBloom(bloom), nil
+	heightBz := sdk.Uint64ToBigEndian(uint64(height))
+	store.Set(types.BloomKey(heightBz), bloom.Bytes())
 }
 
 // SetTransactionLogs sets the transaction's logs in the KVStore
@@ -107,8 +99,8 @@ func (k *Keeper) SetTransactionLogs(ctx sdk.Context, logs []*ethtypes.Log, hash
 	if err != nil {
 		return err
 	}
-	store.Set(types.LogsKey(hash), encLogs)
 
+	store.Set(types.LogsKey(hash), encLogs)
 	return nil
 }
 
@@ -135,7 +127,6 @@ func (k *Keeper) CreateGenesisAccount(ctx sdk.Context, account types.GenesisAcco
 	for _, key := range account.Storage {
 		csdb.SetState(account.Address, key, account.Storage[key])
 	}
-
 }
 
 // ----------------------------------------------------------------------------

x/evm/keeper/keeper_test.go

@@ -18,7 +18,9 @@ import (
 	abci "github.com/tendermint/tendermint/abci/types"
 )
 
-var address = ethcmn.HexToAddress("0x756F45E3FA69347A9A973A725E3C98bC4db0b4c1")
+const addrHex = "0x756F45E3FA69347A9A973A725E3C98bC4db0b4c1"
+
+var address = ethcmn.HexToAddress(addrHex)
 
 type KeeperTestSuite struct {
 	suite.Suite
@@ -50,12 +52,15 @@ func (suite *KeeperTestSuite) TestDBStorage() {
 
 	// Test block hash mapping functionality
 	suite.app.EvmKeeper.SetBlockHashMapping(suite.ctx, ethcmn.FromHex("0x0d87a3a5f73140f46aac1bf419263e4e94e87c292f25007700ab7f2060e2af68"), 7)
+	height, err := suite.app.EvmKeeper.GetBlockHashMapping(suite.ctx, ethcmn.FromHex("0x0d87a3a5f73140f46aac1bf419263e4e94e87c292f25007700ab7f2060e2af68"))
+	suite.Require().NoError(err)
+	suite.Require().Equal(int64(7), height)
+
 	suite.app.EvmKeeper.SetBlockHashMapping(suite.ctx, []byte{0x43, 0x32}, 8)
 
 	// Test block height mapping functionality
 	testBloom := ethtypes.BytesToBloom([]byte{0x1, 0x3})
-	err := suite.app.EvmKeeper.SetBlockBloomMapping(suite.ctx, testBloom, 4)
-	suite.Require().NoError(err, "failed to set block bloom mapping")
+	suite.app.EvmKeeper.SetBlockBloomMapping(suite.ctx, testBloom, 4)
 
 	// Get those state transitions
 	suite.Require().Equal(suite.app.EvmKeeper.GetBalance(suite.ctx, address).Cmp(big.NewInt(5)), 0)
@@ -63,8 +68,13 @@ func (suite *KeeperTestSuite) TestDBStorage() {
 	suite.Require().Equal(suite.app.EvmKeeper.GetState(suite.ctx, address, ethcmn.HexToHash("0x2")), ethcmn.HexToHash("0x3"))
 	suite.Require().Equal(suite.app.EvmKeeper.GetCode(suite.ctx, address), []byte{0x1})
 
-	suite.Require().Equal(suite.app.EvmKeeper.GetBlockHashMapping(suite.ctx, ethcmn.FromHex("0x0d87a3a5f73140f46aac1bf419263e4e94e87c292f25007700ab7f2060e2af68")), int64(7))
-	suite.Require().Equal(suite.app.EvmKeeper.GetBlockHashMapping(suite.ctx, []byte{0x43, 0x32}), int64(8))
+	height, err = suite.app.EvmKeeper.GetBlockHashMapping(suite.ctx, ethcmn.FromHex("0x0d87a3a5f73140f46aac1bf419263e4e94e87c292f25007700ab7f2060e2af68"))
+	suite.Require().NoError(err)
+	suite.Require().Equal(height, int64(7))
+	height, err = suite.app.EvmKeeper.GetBlockHashMapping(suite.ctx, []byte{0x43, 0x32})
+	suite.Require().NoError(err)
+	suite.Require().Equal(height, int64(8))
+
 	bloom, err := suite.app.EvmKeeper.GetBlockBloomMapping(suite.ctx, 4)
 	suite.Require().NoError(err)
 	suite.Require().Equal(bloom, testBloom)

x/evm/keeper/querier.go

@@ -7,11 +7,14 @@ import (
 	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+
 	"github.com/cosmos/ethermint/utils"
 	"github.com/cosmos/ethermint/version"
 	"github.com/cosmos/ethermint/x/evm/types"
+
 	ethcmn "github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
+
 	abci "github.com/tendermint/tendermint/abci/types"
 )
 
@@ -61,8 +64,12 @@ func queryProtocolVersion(keeper Keeper) ([]byte, error) {
 func queryBalance(ctx sdk.Context, path []string, keeper Keeper) ([]byte, error) {
 	addr := ethcmn.HexToAddress(path[1])
 	balance := keeper.GetBalance(ctx, addr)
+	balanceStr, err := utils.MarshalBigInt(balance)
+	if err != nil {
+		return nil, err
+	}
 
-	res := types.QueryResBalance{Balance: utils.MarshalBigInt(balance)}
+	res := types.QueryResBalance{Balance: balanceStr}
 	bz, err := codec.MarshalJSONIndent(keeper.cdc, res)
 	if err != nil {
 		return nil, sdkerrors.Wrap(sdkerrors.ErrJSONMarshal, err.Error())
@@ -120,7 +127,10 @@ func queryNonce(ctx sdk.Context, path []string, keeper Keeper) ([]byte, error) {
 
 func queryHashToHeight(ctx sdk.Context, path []string, keeper Keeper) ([]byte, error) {
 	blockHash := ethcmn.FromHex(path[1])
-	blockNumber := keeper.GetBlockHashMapping(ctx, blockHash)
+	blockNumber, err := keeper.GetBlockHashMapping(ctx, blockHash)
+	if err != nil {
+		return []byte{}, err
+	}
 
 	res := types.QueryResBlockNumber{Number: blockNumber}
 	bz, err := codec.MarshalJSONIndent(keeper.cdc, res)
@@ -182,8 +192,13 @@ func queryAccount(ctx sdk.Context, path []string, keeper Keeper) ([]byte, error)
 	addr := ethcmn.HexToAddress(path[1])
 	so := keeper.GetOrNewStateObject(ctx, addr)
 
+	balance, err := utils.MarshalBigInt(so.Balance())
+	if err != nil {
+		return nil, err
+	}
+
 	res := types.QueryResAccount{
-		Balance:  utils.MarshalBigInt(so.Balance()),
+		Balance:  balance,
 		CodeHash: so.CodeHash(),
 		Nonce:    so.Nonce(),
 	}

x/evm/keeper/querier_test.go

@@ -0,0 +1,52 @@
+package keeper_test
+
+import (
+	"math/big"
+
+	"github.com/cosmos/ethermint/x/evm/types"
+
+	abci "github.com/tendermint/tendermint/abci/types"
+)
+
+func (suite *KeeperTestSuite) TestQuerier() {
+
+	testCases := []struct {
+		msg      string
+		path     []string
+		malleate func()
+		expPass  bool
+	}{
+		{"protocol version", []string{types.QueryProtocolVersion}, func() {}, true},
+		{"balance", []string{types.QueryBalance, addrHex}, func() {
+			suite.app.EvmKeeper.SetBalance(suite.ctx, address, big.NewInt(5))
+		}, true},
+		// {"balance", []string{types.QueryBalance, "0x01232"}, func() {}, false},
+		{"block number", []string{types.QueryBlockNumber, "0x0"}, func() {}, true},
+		{"storage", []string{types.QueryStorage, "0x0", "0x0"}, func() {}, true},
+		{"code", []string{types.QueryCode, "0x0"}, func() {}, true},
+		{"nonce", []string{types.QueryNonce, "0x0"}, func() {}, true},
+		// {"hash to height", []string{types.QueryHashToHeight, "0x0"}, func() {}, true},
+		{"tx logs", []string{types.QueryTxLogs, "0x0"}, func() {}, true},
+		// {"logs bloom", []string{types.QueryLogsBloom, "0x0"}, func() {}, true},
+		{"logs", []string{types.QueryLogs, "0x0"}, func() {}, true},
+		{"account", []string{types.QueryAccount, "0x0"}, func() {}, true},
+		{"unknown request", []string{"other"}, func() {}, false},
+	}
+
+	for i, tc := range testCases {
+		suite.Run("", func() {
+			tc := tc
+			suite.SetupTest() // reset
+			tc.malleate()
+
+			bz, err := suite.querier(suite.ctx, tc.path, abci.RequestQuery{})
+
+			if tc.expPass {
+				suite.Require().NoError(err, "valid test %d failed: %s", i, tc.msg)
+				suite.Require().NotZero(len(bz))
+			} else {
+				suite.Require().Error(err, "invalid test %d passed: %s", i, tc.msg)
+			}
+		})
+	}
+}