Consumer Unit Test Cleanup (#274)

* dummies and keeper construction

* cleans

* dun works

* comment and progress save

* progess save

* finished keeper

* couple unit tests for prop order

* pr suggestions

* testify -> gomock

* small change

* operation order - small change

* wip

* relay test

* lol

* comments

* cleans

* smalls

* progress save

* final tests

* cleans

* GetProviderClient -> GetProviderClientID

* providerClient -> providerClientID

* changes for merging #276

* test comments

testutil/keeper/unit_test_helpers.go

@@ -10,15 +10,21 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper"
 	paramstypes "github.com/cosmos/cosmos-sdk/x/params/types"
+	consumerkeeper "github.com/cosmos/interchain-security/x/ccv/consumer/keeper"
 	providerkeeper "github.com/cosmos/interchain-security/x/ccv/provider/keeper"
 	"github.com/cosmos/interchain-security/x/ccv/types"
 	"github.com/stretchr/testify/require"
+	"github.com/tendermint/tendermint/crypto"
 	"github.com/tendermint/tendermint/libs/log"
 	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 	tmdb "github.com/tendermint/tm-db"
+
+	cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
+	"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
+	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
 )
 
-// Constructs a keeper and context object for unit tests, backed by an in-memory db.
+// Constructs a provider keeper and context object for unit tests, backed by an in-memory db.
 func GetProviderKeeperAndCtx(t testing.TB) (providerkeeper.Keeper, sdk.Context) {
 
 	cdc, storeKey, paramsSubspace, ctx := SetupInMemKeeper(t)
@@ -27,7 +33,7 @@ func GetProviderKeeperAndCtx(t testing.TB) (providerkeeper.Keeper, sdk.Context)
 		cdc,
 		storeKey,
 		paramsSubspace,
-		capabilitykeeper.ScopedKeeper{},
+		&MockScopedKeeper{},
 		&MockChannelKeeper{},
 		&MockPortKeeper{},
 		&MockConnectionKeeper{},
@@ -40,15 +46,36 @@ func GetProviderKeeperAndCtx(t testing.TB) (providerkeeper.Keeper, sdk.Context)
 	return k, ctx
 }
 
-// Constructs a keeper for unit tests, backed by an in-memory db,
+// Constructs a consumer keeper and context object for unit tests, backed by an in-memory db.
+func GetConsumerKeeperAndCtx(t testing.TB) (consumerkeeper.Keeper, sdk.Context) {
+
+	cdc, storeKey, paramsSubspace, ctx := SetupInMemKeeper(t)
+
+	k := consumerkeeper.NewKeeper(
+		cdc,
+		storeKey,
+		paramsSubspace,
+		&MockScopedKeeper{},
+		&MockChannelKeeper{},
+		&MockPortKeeper{},
+		&MockConnectionKeeper{},
+		&MockClientKeeper{},
+		&MockSlashingKeeper{},
+		&MockBankKeeper{},
+		&MockAccountKeeper{},
+		&MockIBCTransferKeeper{},
+		&MockIBCCoreKeeper{},
+		"",
+	)
+	return k, ctx
+}
+
+// Constructs a provider keeper for unit tests, backed by an in-memory db,
 // with ability to pass mocked or otherwise manipulated parameters.
-// Note: Use the dummy types defined in this file for keepers you don't wish to mock,
-// and SetupInMemKeeper() for other parameters you don't wish to manipulate.
-func GetProviderKeeperWithMocks(t testing.TB,
+func GetProviderKeeperWithMocks(
 	cdc *codec.ProtoCodec,
 	storeKey *storetypes.KVStoreKey,
 	paramsSubspace paramstypes.Subspace,
-	ctx sdk.Context,
 	capabilityKeeper capabilitykeeper.ScopedKeeper,
 	channelKeeper types.ChannelKeeper,
 	portKeeper types.PortKeeper,
@@ -59,7 +86,7 @@ func GetProviderKeeperWithMocks(t testing.TB,
 	accountKeeper types.AccountKeeper,
 ) providerkeeper.Keeper {
 
-	k := providerkeeper.NewKeeper(
+	return providerkeeper.NewKeeper(
 		cdc,
 		storeKey,
 		paramsSubspace,
@@ -73,7 +100,68 @@ func GetProviderKeeperWithMocks(t testing.TB,
 		accountKeeper,
 		"",
 	)
-	return k
+}
+
+// Constructs a consumer keeper for unit tests, backed by an in-memory db,
+// with ability to pass mocked or otherwise manipulated parameters.
+func GetCustomConsumerKeeperWithMocks(
+	cdc *codec.ProtoCodec,
+	storeKey *storetypes.KVStoreKey,
+	paramsSubspace paramstypes.Subspace,
+	capabilityKeeper types.ScopedKeeper,
+	channelKeeper types.ChannelKeeper,
+	portKeeper types.PortKeeper,
+	connectionKeeper types.ConnectionKeeper,
+	clientKeeper types.ClientKeeper,
+	slashingKeeper types.SlashingKeeper,
+	bankKeeper types.BankKeeper,
+	accountKeeper types.AccountKeeper,
+	ibcTransferKeeper types.IBCTransferKeeper,
+	ibcCoreKeeper types.IBCCoreKeeper,
+) consumerkeeper.Keeper {
+
+	return consumerkeeper.NewKeeper(
+		cdc,
+		storeKey,
+		paramsSubspace,
+		capabilityKeeper,
+		channelKeeper,
+		portKeeper,
+		connectionKeeper,
+		clientKeeper,
+		slashingKeeper,
+		bankKeeper,
+		accountKeeper,
+		ibcTransferKeeper,
+		ibcCoreKeeper,
+		"",
+	)
+}
+
+// Constructs a consumer keeper for unit tests, backed by an in-memory db,
+// with ability to pass manipulated parameters, but no mocked keepers.
+func GetCustomConsumerKeeper(
+	cdc *codec.ProtoCodec,
+	storeKey *storetypes.KVStoreKey,
+	paramsSubspace paramstypes.Subspace,
+) consumerkeeper.Keeper {
+
+	return consumerkeeper.NewKeeper(
+		cdc,
+		storeKey,
+		paramsSubspace,
+		&MockScopedKeeper{},
+		&MockChannelKeeper{},
+		&MockPortKeeper{},
+		&MockConnectionKeeper{},
+		&MockClientKeeper{},
+		&MockSlashingKeeper{},
+		&MockBankKeeper{},
+		&MockAccountKeeper{},
+		&MockIBCTransferKeeper{},
+		&MockIBCCoreKeeper{},
+		"",
+	)
 }
 
 func SetupInMemKeeper(t testing.TB) (*codec.ProtoCodec, *storetypes.KVStoreKey, paramstypes.Subspace, sdk.Context) {
@@ -98,3 +186,13 @@ func SetupInMemKeeper(t testing.TB) (*codec.ProtoCodec, *storetypes.KVStoreKey,
 	ctx := sdk.NewContext(stateStore, tmproto.Header{}, false, log.NewNopLogger())
 	return cdc, storeKey, paramsSubspace, ctx
 }
+
+type PrivateKey struct {
+	PrivKey cryptotypes.PrivKey
+}
+
+// Generates a public key for unit tests (abiding by tricky interface implementations from tm/sdk)
+func GenPubKey() (crypto.PubKey, error) {
+	privKey := PrivateKey{ed25519.GenPrivKey()}
+	return cryptocodec.ToTmPubKeyInterface(privKey.PrivKey.PubKey())
+}

x/ccv/consumer/keeper/genesis.go

@@ -48,7 +48,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, state *types.GenesisState) []abci.V
 		// Set default value for valset update ID
 		k.SetHeightValsetUpdateID(ctx, uint64(ctx.BlockHeight()), uint64(0))
 		// set provider client id.
-		k.SetProviderClient(ctx, clientID)
+		k.SetProviderClientID(ctx, clientID)
 	} else {
 		// verify that latest consensus state on provider client matches the initial validator set of restarted chain
 		// thus, IBC genesis MUST run before CCV consumer genesis
@@ -87,7 +87,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, state *types.GenesisState) []abci.V
 		k.SetUnbondingTime(ctx, unbondingTime)
 
 		// set provider client id
-		k.SetProviderClient(ctx, state.ProviderClientId)
+		k.SetProviderClientID(ctx, state.ProviderClientId)
 		// set provider channel id.
 		k.SetProviderChannel(ctx, state.ProviderChannelId)
 		// set all unbonding sequences
@@ -110,7 +110,7 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
 	}
 
 	if channelID, ok := k.GetProviderChannel(ctx); ok {
-		clientID, ok := k.GetProviderClient(ctx)
+		clientID, ok := k.GetProviderClientID(ctx)
 		if !ok {
 			panic("provider client does not exist")
 		}
@@ -131,7 +131,7 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
 		gs.MaturingPackets = maturingPackets
 		return gs
 	}
-	clientID, ok := k.GetProviderClient(ctx)
+	clientID, ok := k.GetProviderClientID(ctx)
 	// if provider clientID and channelID don't exist on the consumer chain, then CCV protocol is disabled for this chain
 	// return a disabled genesis state
 	if !ok {

x/ccv/consumer/keeper/genesis_test.go

@@ -33,7 +33,7 @@ func (suite *KeeperTestSuite) TestGenesis() {
 	portId := suite.consumerChain.App.(*app.App).ConsumerKeeper.GetPort(ctx)
 	suite.Require().Equal(consumertypes.PortID, portId)
 
-	clientId, ok := suite.consumerChain.App.(*app.App).ConsumerKeeper.GetProviderClient(ctx)
+	clientId, ok := suite.consumerChain.App.(*app.App).ConsumerKeeper.GetProviderClientID(ctx)
 	suite.Require().True(ok)
 	clientState, ok := suite.consumerChain.App.GetIBCKeeper().ClientKeeper.GetClientState(ctx, clientId)
 	suite.Require().True(ok)

x/ccv/consumer/keeper/keeper.go

@@ -9,7 +9,6 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/codec"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
-	capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper"
 	capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
 	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
 
@@ -29,7 +28,7 @@ type Keeper struct {
 	storeKey          sdk.StoreKey
 	cdc               codec.BinaryCodec
 	paramStore        paramtypes.Subspace
-	scopedKeeper      capabilitykeeper.ScopedKeeper
+	scopedKeeper      ccv.ScopedKeeper
 	channelKeeper     ccv.ChannelKeeper
 	portKeeper        ccv.PortKeeper
 	connectionKeeper  ccv.ConnectionKeeper
@@ -48,7 +47,7 @@ type Keeper struct {
 // collector (and not the provider chain)
 func NewKeeper(
 	cdc codec.BinaryCodec, key sdk.StoreKey, paramSpace paramtypes.Subspace,
-	scopedKeeper capabilitykeeper.ScopedKeeper,
+	scopedKeeper ccv.ScopedKeeper,
 	channelKeeper ccv.ChannelKeeper, portKeeper ccv.PortKeeper,
 	connectionKeeper ccv.ConnectionKeeper, clientKeeper ccv.ClientKeeper,
 	slashingKeeper ccv.SlashingKeeper, bankKeeper ccv.BankKeeper, accountKeeper ccv.AccountKeeper,
@@ -94,7 +93,7 @@ func (k *Keeper) SetHooks(sh ccv.ConsumerHooks) *Keeper {
 }
 
 // ChanCloseInit defines a wrapper function for the channel Keeper's function
-// in order to expose it to the ICS20 transfer handler.
+// Following ICS 004: https://github.com/cosmos/ibc/tree/main/spec/core/ics-004-channel-and-packet-semantics#closing-handshake
 func (k Keeper) ChanCloseInit(ctx sdk.Context, portID, channelID string) error {
 	capName := host.ChannelCapabilityPath(portID, channelID)
 	chanCap, ok := k.scopedKeeper.GetCapability(ctx, capName)
@@ -164,17 +163,17 @@ func (k Keeper) DeleteUnbondingTime(ctx sdk.Context) {
 	store.Delete(types.UnbondingTimeKey())
 }
 
-// SetProviderClient sets the provider clientID that is validating the chain.
+// SetProviderClientID sets the provider clientID that is validating the chain.
 // Set in InitGenesis
-func (k Keeper) SetProviderClient(ctx sdk.Context, clientID string) {
+func (k Keeper) SetProviderClientID(ctx sdk.Context, clientID string) {
 	store := ctx.KVStore(k.storeKey)
-	store.Set(types.ProviderClientKey(), []byte(clientID))
+	store.Set(types.ProviderClientIDKey(), []byte(clientID))
 }
 
-// GetProviderClient gets the provider clientID that is validating the chain.
-func (k Keeper) GetProviderClient(ctx sdk.Context) (string, bool) {
+// GetProviderClientID gets the provider clientID that is validating the chain.
+func (k Keeper) GetProviderClientID(ctx sdk.Context) (string, bool) {
 	store := ctx.KVStore(k.storeKey)
-	clientIdBytes := store.Get(types.ProviderClientKey())
+	clientIdBytes := store.Get(types.ProviderClientIDKey())
 	if clientIdBytes == nil {
 		return "", false
 	}
@@ -289,7 +288,7 @@ func (k Keeper) VerifyProviderChain(ctx sdk.Context, channelID string, connectio
 		return sdkerrors.Wrapf(conntypes.ErrConnectionNotFound, "connection not found for connection ID: %s", connectionID)
 	}
 	// Verify that client id is expected clientID
-	expectedClientId, ok := k.GetProviderClient(ctx)
+	expectedClientId, ok := k.GetProviderClientID(ctx)
 	if !ok {
 		return sdkerrors.Wrapf(clienttypes.ErrInvalidClient, "could not find provider client id")
 	}