Change signature of NewKeeper to directly accept a ConsensusHost (cos…

…mos#6084) * chore: change signature of NewKeeper to directly accept a ConsensusHost * chore: updated PR number in CHANGELOG * chore: add panic on invalid staking keeper --------- Co-authored-by: Charly <[email protected]> Co-authored-by: Carlos Rodriguez <[email protected]>
bznein · Apr 5, 2024 · 9f6fca8 · 9f6fca8
1 parent bce50ea
commit 9f6fca8
Show file tree

Hide file tree

Showing 8 changed files with 25 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -43,6 +43,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (core/02-client, light-clients) [\#5806](https://github.com/cosmos/ibc-go/pull/5806) Decouple light client routing from their encoding structure.
 * (core/04-channel) [\#5991](https://github.com/cosmos/ibc-go/pull/5991) The client CLI `QueryLatestConsensusState` has been removed.
 * (light-clients/06-solomachine) [\#6037](https://github.com/cosmos/ibc-go/pull/6037) Remove `Initialize` function from `ClientState` and move logic to `Initialize` function of `LightClientModule`.
+* (core/02-client) [\#6084](https://github.com/cosmos/ibc-go/pull/6084) Removed `stakingKeeper` as an argument to `NewKeeper` and replaced with a `ConsensusHost` implementation.
 
 ### State Machine Breaking
 

diff --git a/modules/apps/callbacks/testing/simapp/app.go b/modules/apps/callbacks/testing/simapp/app.go
@@ -407,7 +407,7 @@ func NewSimApp(
 	app.UpgradeKeeper = upgradekeeper.NewKeeper(skipUpgradeHeights, runtime.NewKVStoreService(keys[upgradetypes.StoreKey]), appCodec, homePath, app.BaseApp, authtypes.NewModuleAddress(govtypes.ModuleName).String())
 
 	app.IBCKeeper = ibckeeper.NewKeeper(
-		appCodec, keys[ibcexported.StoreKey], app.GetSubspace(ibcexported.ModuleName), app.StakingKeeper, app.UpgradeKeeper, scopedIBCKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(),
+		appCodec, keys[ibcexported.StoreKey], app.GetSubspace(ibcexported.ModuleName), ibctm.NewConsensusHost(app.StakingKeeper), app.UpgradeKeeper, scopedIBCKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(),
 	)
 
 	// NOTE: The mock ContractKeeper is only created for testing.

diff --git a/modules/core/02-client/keeper/keeper.go b/modules/core/02-client/keeper/keeper.go
@@ -33,7 +33,7 @@ type Keeper struct {
 }
 
 // NewKeeper creates a new NewKeeper instance
-func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace types.ParamSubspace, sk types.StakingKeeper, uk types.UpgradeKeeper) Keeper {
+func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace types.ParamSubspace, consensusHost types.ConsensusHost, uk types.UpgradeKeeper) Keeper {
 	router := types.NewRouter(key)
 	localhostModule := localhost.NewLightClientModule(cdc, key)
 	router.AddRoute(exported.Localhost, localhostModule)
@@ -42,7 +42,7 @@ func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace ty
 		storeKey:       key,
 		cdc:            cdc,
 		router:         router,
-		consensusHost:  ibctm.NewConsensusHost(sk),
+		consensusHost:  consensusHost,
 		legacySubspace: legacySubspace,
 		upgradeKeeper:  uk,
 	}

diff --git a/modules/core/keeper/keeper.go b/modules/core/keeper/keeper.go
@@ -40,13 +40,14 @@ type Keeper struct {
 // NewKeeper creates a new ibc Keeper
 func NewKeeper(
 	cdc codec.BinaryCodec, key storetypes.StoreKey, paramSpace types.ParamSubspace,
-	stakingKeeper clienttypes.StakingKeeper, upgradeKeeper clienttypes.UpgradeKeeper,
+	consensusHost clienttypes.ConsensusHost, upgradeKeeper clienttypes.UpgradeKeeper,
 	scopedKeeper capabilitykeeper.ScopedKeeper, authority string,
 ) *Keeper {
 	// panic if any of the keepers passed in is empty
-	if isEmpty(stakingKeeper) {
-		panic(errors.New("cannot initialize IBC keeper: empty staking keeper"))
+	if isEmpty(consensusHost) {
+		panic(errors.New("cannot initialize IBC keeper: empty consensus host"))
 	}
+
 	if isEmpty(upgradeKeeper) {
 		panic(errors.New("cannot initialize IBC keeper: empty upgrade keeper"))
 	}
@@ -59,7 +60,7 @@ func NewKeeper(
 		panic(errors.New("authority must be non-empty"))
 	}
 
-	clientKeeper := clientkeeper.NewKeeper(cdc, key, paramSpace, stakingKeeper, upgradeKeeper)
+	clientKeeper := clientkeeper.NewKeeper(cdc, key, paramSpace, consensusHost, upgradeKeeper)
 	connectionKeeper := connectionkeeper.NewKeeper(cdc, key, paramSpace, &clientKeeper)
 	portKeeper := portkeeper.NewKeeper(scopedKeeper)
 	channelKeeper := channelkeeper.NewKeeper(cdc, key, &clientKeeper, &connectionKeeper, &portKeeper, scopedKeeper)

diff --git a/modules/core/keeper/keeper_test.go b/modules/core/keeper/keeper_test.go
@@ -9,13 +9,13 @@ import (
 
 	upgradekeeper "cosmossdk.io/x/upgrade/keeper"
 
-	stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 
 	capabilitykeeper "github.com/cosmos/ibc-go/modules/capability/keeper"
 	clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
 	ibcexported "github.com/cosmos/ibc-go/v8/modules/core/exported"
 	ibckeeper "github.com/cosmos/ibc-go/v8/modules/core/keeper"
+	ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint"
 	ibctesting "github.com/cosmos/ibc-go/v8/testing"
 )
 
@@ -61,7 +61,7 @@ func (MockStakingKeeper) UnbondingTime(_ context.Context) (time.Duration, error)
 // It verifies if ibckeeper.NewKeeper panic when any of the keepers passed in is empty.
 func (suite *KeeperTestSuite) TestNewKeeper() {
 	var (
-		stakingKeeper  clienttypes.StakingKeeper
+		consensusHost  clienttypes.ConsensusHost
 		upgradeKeeper  clienttypes.UpgradeKeeper
 		scopedKeeper   capabilitykeeper.ScopedKeeper
 		newIBCKeeperFn func()
@@ -72,21 +72,11 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
 		malleate func()
 		expPass  bool
 	}{
-		{"failure: empty staking keeper value", func() {
-			emptyStakingKeeperValue := stakingkeeper.Keeper{}
-
-			stakingKeeper = emptyStakingKeeperValue
-		}, false},
-		{"failure: empty staking keeper pointer", func() {
-			emptyStakingKeeperPointer := &stakingkeeper.Keeper{}
-
-			stakingKeeper = emptyStakingKeeperPointer
+		{"failure: empty consensus host value", func() {
+			consensusHost = &ibctm.ConsensusHost{}
 		}, false},
-		{"failure: empty mock staking keeper", func() {
-			// use a different implementation of clienttypes.StakingKeeper
-			emptyMockStakingKeeper := MockStakingKeeper{}
-
-			stakingKeeper = emptyMockStakingKeeper
+		{"failure: nil consensus host value", func() {
+			consensusHost = nil
 		}, false},
 		{"failure: empty upgrade keeper value", func() {
 			emptyUpgradeKeeperValue := upgradekeeper.Keeper{}
@@ -109,7 +99,7 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
 					suite.chainA.GetSimApp().AppCodec(),
 					suite.chainA.GetSimApp().GetKey(ibcexported.StoreKey),
 					suite.chainA.GetSimApp().GetSubspace(ibcexported.ModuleName),
-					stakingKeeper,
+					consensusHost,
 					upgradeKeeper,
 					scopedKeeper,
 					"", // authority
@@ -119,8 +109,7 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
 		{"success: replace stakingKeeper with non-empty MockStakingKeeper", func() {
 			// use a different implementation of clienttypes.StakingKeeper
 			mockStakingKeeper := MockStakingKeeper{"not empty"}
-
-			stakingKeeper = mockStakingKeeper
+			consensusHost = ibctm.NewConsensusHost(mockStakingKeeper)
 		}, true},
 	}
 
@@ -135,14 +124,14 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
 					suite.chainA.GetSimApp().AppCodec(),
 					suite.chainA.GetSimApp().GetKey(ibcexported.StoreKey),
 					suite.chainA.GetSimApp().GetSubspace(ibcexported.ModuleName),
-					stakingKeeper,
+					consensusHost,
 					upgradeKeeper,
 					scopedKeeper,
 					suite.chainA.App.GetIBCKeeper().GetAuthority(),
 				)
 			}
 
-			stakingKeeper = suite.chainA.GetSimApp().StakingKeeper
+			consensusHost = ibctm.NewConsensusHost(suite.chainA.GetSimApp().StakingKeeper)
 			upgradeKeeper = suite.chainA.GetSimApp().UpgradeKeeper
 			scopedKeeper = suite.chainA.GetSimApp().ScopedIBCKeeper
 

diff --git a/modules/light-clients/07-tendermint/consensus_host.go b/modules/light-clients/07-tendermint/consensus_host.go
@@ -34,6 +34,10 @@ type StakingKeeper interface {
 
 // NewConsensusHost creates and returns a new ConsensusHost for tendermint consensus.
 func NewConsensusHost(stakingKeeper clienttypes.StakingKeeper) clienttypes.ConsensusHost {
+	if stakingKeeper == nil {
+		panic("staking keeper cannot be nil")
+	}
+
 	return &ConsensusHost{
 		stakingKeeper: stakingKeeper,
 	}

diff --git a/modules/light-clients/08-wasm/testing/simapp/app.go b/modules/light-clients/08-wasm/testing/simapp/app.go
@@ -416,7 +416,7 @@ func NewSimApp(
 	app.UpgradeKeeper = upgradekeeper.NewKeeper(skipUpgradeHeights, runtime.NewKVStoreService(keys[upgradetypes.StoreKey]), appCodec, homePath, app.BaseApp, authtypes.NewModuleAddress(govtypes.ModuleName).String())
 
 	app.IBCKeeper = ibckeeper.NewKeeper(
-		appCodec, keys[ibcexported.StoreKey], app.GetSubspace(ibcexported.ModuleName), app.StakingKeeper, app.UpgradeKeeper, scopedIBCKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(),
+		appCodec, keys[ibcexported.StoreKey], app.GetSubspace(ibcexported.ModuleName), ibctm.NewConsensusHost(app.StakingKeeper), app.UpgradeKeeper, scopedIBCKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(),
 	)
 
 	// Register the proposal types

diff --git a/testing/simapp/app.go b/testing/simapp/app.go
@@ -408,7 +408,7 @@ func NewSimApp(
 	app.UpgradeKeeper = upgradekeeper.NewKeeper(skipUpgradeHeights, runtime.NewKVStoreService(keys[upgradetypes.StoreKey]), appCodec, homePath, app.BaseApp, authtypes.NewModuleAddress(govtypes.ModuleName).String())
 
 	app.IBCKeeper = ibckeeper.NewKeeper(
-		appCodec, keys[ibcexported.StoreKey], app.GetSubspace(ibcexported.ModuleName), app.StakingKeeper, app.UpgradeKeeper, scopedIBCKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(),
+		appCodec, keys[ibcexported.StoreKey], app.GetSubspace(ibcexported.ModuleName), ibctm.NewConsensusHost(app.StakingKeeper), app.UpgradeKeeper, scopedIBCKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(),
 	)
 	// Register the proposal types
 	// Deprecated: Avoid adding new handlers, instead use the new proposal flow