test: improve fungible module coverage (#1762)

* Improve gas price coverage

* Improve msg update system contract coverage and cleanup

* Cleanup

* Improve readability of mocking evm calls methods

* Add error tests for existing system contract tests

* Cleanup duplication

* Cleanup comments

* Fix PR comments

* Add changelog entry

changelog.md

@@ -40,6 +40,7 @@
 
 * [1584](https://github.com/zeta-chain/node/pull/1584) - allow to run E2E tests on any networks
 * [1753](https://github.com/zeta-chain/node/pull/1753) - fix gosec errors on usage of rand package
+* [1762](https://github.com/zeta-chain/node/pull/1762) - improve coverage for fungibile module
 
 ### CI
 

testutil/keeper/fungible.go

@@ -1,16 +1,21 @@
 package keeper
 
 import (
+	"math/big"
 	"testing"
 
 	"github.com/cosmos/cosmos-sdk/codec"
 	"github.com/cosmos/cosmos-sdk/store"
 	storetypes "github.com/cosmos/cosmos-sdk/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	paramskeeper "github.com/cosmos/cosmos-sdk/x/params/keeper"
+	"github.com/evmos/ethermint/x/evm/statedb"
+	evmtypes "github.com/evmos/ethermint/x/evm/types"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 	tmdb "github.com/tendermint/tm-db"
 	fungiblemocks "github.com/zeta-chain/zetacore/testutil/keeper/mocks/fungible"
+	"github.com/zeta-chain/zetacore/testutil/sample"
 	fungiblemodule "github.com/zeta-chain/zetacore/x/fungible"
 	"github.com/zeta-chain/zetacore/x/fungible/keeper"
 	"github.com/zeta-chain/zetacore/x/fungible/types"
@@ -164,8 +169,73 @@ func GetFungibleObserverMock(t testing.TB, keeper *keeper.Keeper) *fungiblemocks
 	return fok
 }
 
-func GetFungibleEVMMock(t testing.TB, keeper *keeper.Keeper) *fungiblemocks.FungibleEVMKeeper {
+func GetFungibleEVMMock(t testing.TB, keeper *keeper.Keeper) *FungibleMockEVMKeeper {
 	fek, ok := keeper.GetEVMKeeper().(*fungiblemocks.FungibleEVMKeeper)
 	require.True(t, ok)
-	return fek
+	return &FungibleMockEVMKeeper{
+		FungibleEVMKeeper: fek,
+	}
+}
+
+type FungibleMockEVMKeeper struct {
+	*fungiblemocks.FungibleEVMKeeper
+}
+
+func (m *FungibleMockEVMKeeper) SetupMockEVMKeeperForSystemContractDeployment() {
+	gasRes := &evmtypes.EstimateGasResponse{Gas: 1000}
+	m.On("WithChainID", mock.Anything).Maybe().Return(mock.Anything)
+	m.On("ChainID").Maybe().Return(big.NewInt(1))
+	m.On(
+		"EstimateGas",
+		mock.Anything,
+		mock.Anything,
+	).Return(gasRes, nil)
+	m.MockEVMSuccessCallTimes(5)
+	m.On(
+		"GetAccount",
+		mock.Anything,
+		mock.Anything,
+	).Return(&statedb.Account{
+		Nonce: 1,
+	})
+	m.On(
+		"GetCode",
+		mock.Anything,
+		mock.Anything,
+	).Return([]byte{1, 2, 3})
+}
+
+func (m *FungibleMockEVMKeeper) MockEVMSuccessCallOnce() {
+	m.MockEVMSuccessCallOnceWithReturn(&evmtypes.MsgEthereumTxResponse{})
+}
+
+func (m *FungibleMockEVMKeeper) MockEVMSuccessCallTimes(times int) {
+	m.MockEVMSuccessCallTimesWithReturn(&evmtypes.MsgEthereumTxResponse{}, times)
+}
+
+func (m *FungibleMockEVMKeeper) MockEVMSuccessCallOnceWithReturn(ret *evmtypes.MsgEthereumTxResponse) {
+	m.MockEVMSuccessCallTimesWithReturn(ret, 1)
+}
+
+func (m *FungibleMockEVMKeeper) MockEVMSuccessCallTimesWithReturn(ret *evmtypes.MsgEthereumTxResponse, times int) {
+	if ret == nil {
+		ret = &evmtypes.MsgEthereumTxResponse{}
+	}
+	m.On(
+		"ApplyMessage",
+		mock.Anything,
+		mock.Anything,
+		mock.Anything,
+		mock.Anything,
+	).Return(ret, nil).Times(times)
+}
+
+func (m *FungibleMockEVMKeeper) MockEVMFailCallOnce() {
+	m.On(
+		"ApplyMessage",
+		mock.Anything,
+		mock.Anything,
+		mock.Anything,
+		mock.Anything,
+	).Return(&evmtypes.MsgEthereumTxResponse{}, sample.ErrSample).Once()
 }

x/fungible/keeper/evm_test.go

@@ -8,7 +8,6 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
-	evmkeeper "github.com/evmos/ethermint/x/evm/keeper"
 	evmtypes "github.com/evmos/ethermint/x/evm/types"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
@@ -18,7 +17,6 @@ import (
 	"github.com/zeta-chain/zetacore/testutil/contracts"
 	testkeeper "github.com/zeta-chain/zetacore/testutil/keeper"
 	"github.com/zeta-chain/zetacore/testutil/sample"
-	"github.com/zeta-chain/zetacore/x/fungible/keeper"
 	fungiblekeeper "github.com/zeta-chain/zetacore/x/fungible/keeper"
 	"github.com/zeta-chain/zetacore/x/fungible/types"
 )
@@ -33,20 +31,30 @@ func getValidChainID(t *testing.T) int64 {
 }
 
 // require that a contract has been deployed by checking stored code is non-empty.
-func assertContractDeployment(t *testing.T, k *evmkeeper.Keeper, ctx sdk.Context, contractAddress common.Address) {
+func assertContractDeployment(t *testing.T, k types.EVMKeeper, ctx sdk.Context, contractAddress common.Address) {
 	acc := k.GetAccount(ctx, contractAddress)
 	require.NotNil(t, acc)
 
 	code := k.GetCode(ctx, common.BytesToHash(acc.CodeHash))
 	require.NotEmpty(t, code)
 }
 
+func deploySystemContractsWithMockEvmKeeper(
+	t *testing.T,
+	ctx sdk.Context,
+	k *fungiblekeeper.Keeper,
+	mockEVMKeeper *testkeeper.FungibleMockEVMKeeper,
+) (wzeta, uniswapV2Factory, uniswapV2Router, connector, systemContract common.Address) {
+	mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment()
+	return deploySystemContracts(t, ctx, k, mockEVMKeeper)
+}
+
 // deploySystemContracts deploys the system contracts and returns their addresses.
 func deploySystemContracts(
 	t *testing.T,
 	ctx sdk.Context,
 	k *fungiblekeeper.Keeper,
-	evmk *evmkeeper.Keeper,
+	evmk types.EVMKeeper,
 ) (wzeta, uniswapV2Factory, uniswapV2Router, connector, systemContract common.Address) {
 	var err error
 
@@ -82,7 +90,7 @@ func deploySystemContracts(
 func assertExampleBarValue(
 	t *testing.T,
 	ctx sdk.Context,
-	k *keeper.Keeper,
+	k *fungiblekeeper.Keeper,
 	address common.Address,
 	expected int64,
 ) {
@@ -99,6 +107,7 @@ func assertExampleBarValue(
 		false,
 		"bar",
 	)
+	require.NoError(t, err)
 	unpacked, err := exampleABI.Unpack("bar", res.Ret)
 	require.NoError(t, err)
 	require.NotZero(t, len(unpacked))
@@ -263,6 +272,7 @@ func TestKeeper_DepositZRC20AndCallContract(t *testing.T) {
 			false,
 			"bar",
 		)
+		require.NoError(t, err)
 		unpacked, err := exampleABI.Unpack("bar", res.Ret)
 		require.NoError(t, err)
 		require.NotZero(t, len(unpacked))
@@ -448,13 +458,7 @@ func TestKeeper_CallEVMWithData(t *testing.T) {
 			mock.Anything,
 			&evmtypes.EthCallRequest{Args: args, GasCap: config.DefaultGasCap},
 		).Return(gasRes, nil)
-		mockEVMKeeper.On(
-			"ApplyMessage",
-			ctx,
-			mock.Anything,
-			mock.Anything,
-			mock.Anything,
-		).Return(msgRes, nil)
+		mockEVMKeeper.MockEVMSuccessCallOnce()
 
 		mockEVMKeeper.On("WithChainID", mock.Anything).Maybe().Return(ctx)
 		mockEVMKeeper.On("ChainID").Maybe().Return(big.NewInt(1))
@@ -497,13 +501,7 @@ func TestKeeper_CallEVMWithData(t *testing.T) {
 			mock.Anything,
 			sdk.AccAddress(fromAddr.Bytes()),
 		).Return(uint64(1), nil)
-		mockEVMKeeper.On(
-			"ApplyMessage",
-			ctx,
-			mock.Anything,
-			mock.Anything,
-			mock.Anything,
-		).Return(msgRes, nil)
+		mockEVMKeeper.MockEVMSuccessCallOnce()
 
 		mockEVMKeeper.On("WithChainID", mock.Anything).Maybe().Return(ctx)
 		mockEVMKeeper.On("ChainID").Maybe().Return(big.NewInt(1))
@@ -604,7 +602,6 @@ func TestKeeper_CallEVMWithData(t *testing.T) {
 			Data: (*hexutil.Bytes)(&data),
 		})
 		gasRes := &evmtypes.EstimateGasResponse{Gas: 1000}
-		msgRes := &evmtypes.MsgEthereumTxResponse{}
 
 		// Set up mocked methods
 		mockAuthKeeper.On(
@@ -617,13 +614,7 @@ func TestKeeper_CallEVMWithData(t *testing.T) {
 			mock.Anything,
 			&evmtypes.EthCallRequest{Args: args, GasCap: config.DefaultGasCap},
 		).Return(gasRes, nil)
-		mockEVMKeeper.On(
-			"ApplyMessage",
-			ctx,
-			mock.Anything,
-			mock.Anything,
-			mock.Anything,
-		).Return(msgRes, sample.ErrSample)
+		mockEVMKeeper.MockEVMFailCallOnce()
 
 		mockEVMKeeper.On("WithChainID", mock.Anything).Maybe().Return(ctx)
 		mockEVMKeeper.On("ChainID").Maybe().Return(big.NewInt(1))

x/fungible/keeper/foreign_coins.go

@@ -1,8 +1,6 @@
 package keeper
 
 import (
-	"fmt"
-
 	"github.com/cosmos/cosmos-sdk/store/prefix"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	ethcommon "github.com/ethereum/go-ethereum/common"
@@ -12,7 +10,7 @@ import (
 
 // SetForeignCoins set a specific foreignCoins in the store from its index
 func (k Keeper) SetForeignCoins(ctx sdk.Context, foreignCoins types.ForeignCoins) {
-	p := types.KeyPrefix(fmt.Sprintf("%s", types.ForeignCoinsKeyPrefix))
+	p := types.KeyPrefix(types.ForeignCoinsKeyPrefix)
 	store := prefix.NewStore(ctx.KVStore(k.storeKey), p)
 	b := k.cdc.MustMarshal(&foreignCoins)
 	store.Set(types.ForeignCoinsKey(
@@ -25,7 +23,7 @@ func (k Keeper) GetForeignCoins(
 	ctx sdk.Context,
 	zrc20Addr string,
 ) (val types.ForeignCoins, found bool) {
-	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(fmt.Sprintf("%s", types.ForeignCoinsKeyPrefix)))
+	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.ForeignCoinsKeyPrefix))
 
 	b := store.Get(types.ForeignCoinsKey(
 		zrc20Addr,
@@ -51,7 +49,7 @@ func (k Keeper) RemoveForeignCoins(
 
 // GetAllForeignCoinsForChain returns all foreignCoins on a given chain
 func (k Keeper) GetAllForeignCoinsForChain(ctx sdk.Context, foreignChainID int64) (list []types.ForeignCoins) {
-	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(fmt.Sprintf("%s", types.ForeignCoinsKeyPrefix)))
+	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.ForeignCoinsKeyPrefix))
 	iterator := sdk.KVStorePrefixIterator(store, []byte{})
 
 	defer iterator.Close()
@@ -68,7 +66,7 @@ func (k Keeper) GetAllForeignCoinsForChain(ctx sdk.Context, foreignChainID int64
 
 // GetAllForeignCoins returns all foreignCoins
 func (k Keeper) GetAllForeignCoins(ctx sdk.Context) (list []types.ForeignCoins) {
-	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(fmt.Sprintf("%s", types.ForeignCoinsKeyPrefix)))
+	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.ForeignCoinsKeyPrefix))
 	iterator := sdk.KVStorePrefixIterator(store, []byte{})
 
 	defer iterator.Close()

x/fungible/keeper/gas_coin_and_pool_test.go

@@ -19,7 +19,7 @@ func setupGasCoin(
 	t *testing.T,
 	ctx sdk.Context,
 	k *fungiblekeeper.Keeper,
-	evmk *evmkeeper.Keeper,
+	evmk types.EVMKeeper,
 	chainID int64,
 	assetName string,
 	symbol string,

x/fungible/keeper/gas_price.go

@@ -12,6 +12,9 @@ import (
 
 // SetGasPrice sets gas price on the system contract in zEVM; return the gasUsed and error code
 func (k Keeper) SetGasPrice(ctx sdk.Context, chainid *big.Int, gasPrice *big.Int) (uint64, error) {
+	if gasPrice == nil {
+		return 0, sdkerrors.Wrapf(types.ErrNilGasPrice, "gas price param should be set")
+	}
 	system, found := k.GetSystemContract(ctx)
 	if !found {
 		return 0, sdkerrors.Wrapf(types.ErrContractNotFound, "system contract state variable not found")