feat(client): migrate client params

modules/core/02-client/genesis.go

@@ -13,6 +13,9 @@ import (
 // InitGenesis initializes the ibc client submodule's state from a provided genesis
 // state.
 func InitGenesis(ctx sdk.Context, k keeper.Keeper, gs types.GenesisState) {
+	if err := gs.Params.Validate(); err != nil {
+		panic(fmt.Sprintf("invalid ibc client genesis state parameters: %v", err))
+	}
 	k.SetParams(ctx, gs.Params)
 
 	// Set all client metadata first. This will allow client keeper to overwrite client and consensus state keys

modules/core/02-client/keeper/keeper.go

@@ -27,26 +27,26 @@ import (
 // Keeper represents a type that grants read and write permissions to any client
 // state information
 type Keeper struct {
-	storeKey      storetypes.StoreKey
-	cdc           codec.BinaryCodec
-	paramSpace    paramtypes.Subspace
-	stakingKeeper types.StakingKeeper
-	upgradeKeeper types.UpgradeKeeper
+	storeKey       storetypes.StoreKey
+	cdc            codec.BinaryCodec
+	legacySubspace paramtypes.Subspace
+	stakingKeeper  types.StakingKeeper
+	upgradeKeeper  types.UpgradeKeeper
 }
 
 // NewKeeper creates a new NewKeeper instance
-func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, paramSpace paramtypes.Subspace, sk types.StakingKeeper, uk types.UpgradeKeeper) Keeper {
+func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace paramtypes.Subspace, sk types.StakingKeeper, uk types.UpgradeKeeper) Keeper {
 	// set KeyTable if it has not already been set
-	if !paramSpace.HasKeyTable() {
-		paramSpace = paramSpace.WithKeyTable(types.ParamKeyTable())
+	if !legacySubspace.HasKeyTable() {
+		legacySubspace = legacySubspace.WithKeyTable(types.ParamKeyTable())
 	}
 
 	return Keeper{
-		storeKey:      key,
-		cdc:           cdc,
-		paramSpace:    paramSpace,
-		stakingKeeper: sk,
-		upgradeKeeper: uk,
+		storeKey:       key,
+		cdc:            cdc,
+		legacySubspace: legacySubspace,
+		stakingKeeper:  sk,
+		upgradeKeeper:  uk,
 	}
 }
 
@@ -413,3 +413,23 @@ func (k Keeper) GetClientStatus(ctx sdk.Context, clientState exported.ClientStat
 	}
 	return clientState.Status(ctx, k.ClientStore(ctx, clientID), k.cdc)
 }
+
+// GetParams returns the total set of ibc-client parameters.
+func (k Keeper) GetParams(ctx sdk.Context) types.Params {
+	store := ctx.KVStore(k.storeKey)
+	bz := store.Get([]byte(types.ParamsKey))
+	if bz == nil {
+		return types.Params{}
+	}
+
+	var params types.Params
+	k.cdc.MustUnmarshal(bz, &params)
+	return params
+}
+
+// SetParams sets the total set of ibc-client parameters.
+func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
+	store := ctx.KVStore(k.storeKey)
+	bz := k.cdc.MustMarshal(&params)
+	store.Set([]byte(types.ParamsKey), bz)
+}

modules/core/02-client/keeper/keeper_test.go

@@ -450,3 +450,57 @@ func (suite KeeperTestSuite) TestIterateClientStates() { //nolint:govet // this
 		})
 	}
 }
+
+// TestDefaultSetParams tests the default params set are what is expected
+func (suite *KeeperTestSuite) TestDefaultSetParams() {
+	expParams := types.DefaultParams()
+
+	clientKeeper := suite.chainA.App.GetIBCKeeper().ClientKeeper
+	params := clientKeeper.GetParams(suite.chainA.GetContext())
+
+	suite.Require().Equal(expParams, params)
+	suite.Require().Equal(expParams.AllowedClients, clientKeeper.GetParams(suite.chainA.GetContext()).AllowedClients)
+}
+
+// TestParams tests that Param setting and retrieval works properly
+func (suite *KeeperTestSuite) TestParams() {
+	testCases := []struct {
+		name    string
+		input   types.Params
+		expPass bool
+	}{
+		{"success: set default params", types.DefaultParams(), true},
+		{"success: empty allowedClients", types.NewParams(), true},
+		{"success: subset of allowedClients", types.NewParams(exported.Tendermint, exported.Localhost), true},
+		{"failure: contains a single empty string value as allowedClient", types.NewParams(exported.Localhost, ""), false},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+
+		suite.Run(tc.name, func() {
+			suite.SetupTest() // reset
+			ctx := suite.chainA.GetContext()
+			err := tc.input.Validate()
+			suite.chainA.GetSimApp().IBCKeeper.ClientKeeper.SetParams(ctx, tc.input)
+			if tc.expPass {
+				suite.Require().NoError(err)
+				expected := tc.input
+				p := suite.chainA.GetSimApp().IBCKeeper.ClientKeeper.GetParams(ctx)
+				suite.Require().Equal(expected, p)
+			} else {
+				suite.Require().Error(err)
+			}
+		})
+	}
+}
+
+// TestUnsetParams tests that trying to get params that are not set returns empty params.
+func (suite *KeeperTestSuite) TestUnsetParams() {
+	suite.SetupTest()
+	ctx := suite.chainA.GetContext()
+	store := ctx.KVStore(suite.chainA.GetSimApp().GetKey(exported.StoreKey))
+	store.Delete([]byte(types.ParamsKey))
+
+	suite.Require().Equal(suite.chainA.GetSimApp().IBCKeeper.ClientKeeper.GetParams(ctx), types.Params{})
+}

modules/core/02-client/keeper/migrations.go

@@ -4,6 +4,7 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	v7 "github.com/cosmos/ibc-go/v7/modules/core/02-client/migrations/v7"
+	"github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
 )
 
 // Migrator is a struct for handling in-place store migrations.
@@ -31,3 +32,17 @@ func (m Migrator) Migrate2to3(ctx sdk.Context) error {
 func (m Migrator) Migrate3to4(ctx sdk.Context) error {
 	return v7.MigrateLocalhostClient(ctx, m.keeper)
 }
+
+// MigrateParams migrates from consensus version 4 to 5.
+// This migration takes the parameters that are currently stored and managed by x/params
+// and stores them directly in the ibc module's state.
+func (m Migrator) MigrateParams(ctx sdk.Context) error {
+	var params types.Params
+	m.keeper.legacySubspace.GetParamSet(ctx, &params)
+
+	if err := params.Validate(); err != nil {
+		return err
+	}
+	m.keeper.SetParams(ctx, params)
+	return nil
+}

modules/core/02-client/keeper/migrations_test.go

@@ -0,0 +1,43 @@
+package keeper_test
+
+import (
+	"github.com/cosmos/ibc-go/v7/modules/core/02-client/keeper"
+	"github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
+	ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported"
+)
+
+// TestMigrateParams tests the migration for the client params
+func (suite *KeeperTestSuite) TestMigrateParams() {
+	testCases := []struct {
+		name           string
+		malleate       func()
+		expectedParams types.Params
+	}{
+		{
+			"success: default params",
+			func() {
+				params := types.DefaultParams()
+				subspace := suite.chainA.GetSimApp().GetSubspace(ibcexported.ModuleName)
+				subspace.SetParamSet(suite.chainA.GetContext(), &params)
+			},
+			types.DefaultParams(),
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		suite.Run(tc.name, func() {
+			suite.SetupTest() // reset
+
+			tc.malleate()
+
+			ctx := suite.chainA.GetContext()
+			migrator := keeper.NewMigrator(suite.chainA.GetSimApp().IBCKeeper.ClientKeeper)
+			err := migrator.MigrateParams(ctx)
+			suite.Require().NoError(err)
+
+			params := suite.chainA.GetSimApp().IBCKeeper.ClientKeeper.GetParams(ctx)
+			suite.Require().Equal(tc.expectedParams, params)
+		})
+	}
+}

modules/core/02-client/types/codec.go

@@ -46,6 +46,7 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
 		&MsgUpdateClient{},
 		&MsgUpgradeClient{},
 		&MsgSubmitMisbehaviour{},
+		&MsgUpdateClientParams{},
 	)
 
 	msgservice.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc)

modules/core/02-client/types/keys.go

@@ -25,6 +25,9 @@ const (
 	// KeyNextClientSequence is the key used to store the next client sequence in
 	// the keeper.
 	KeyNextClientSequence = "nextClientSequence"
+
+	// ParamsKey is the store key for the IBC client parameters
+	ParamsKey = "clientParams"
 )
 
 // FormatClientIdentifier returns the client identifier with the sequence appended.

modules/core/02-client/types/msgs.go

@@ -15,6 +15,7 @@ var (
 	_ sdk.Msg = (*MsgUpdateClient)(nil)
 	_ sdk.Msg = (*MsgSubmitMisbehaviour)(nil)
 	_ sdk.Msg = (*MsgUpgradeClient)(nil)
+	_ sdk.Msg = (*MsgUpdateClientParams)(nil)
 
 	_ codectypes.UnpackInterfacesMessage = (*MsgCreateClient)(nil)
 	_ codectypes.UnpackInterfacesMessage = (*MsgUpdateClient)(nil)
@@ -264,3 +265,28 @@ func (msg MsgSubmitMisbehaviour) UnpackInterfaces(unpacker codectypes.AnyUnpacke
 	var misbehaviour exported.ClientMessage
 	return unpacker.UnpackAny(msg.Misbehaviour, &misbehaviour)
 }
+
+// NewMsgUpdateClientParams creates a new instance of MsgUpdateClientParams.
+func NewMsgUpdateClientParams(authority string, params Params) *MsgUpdateClientParams {
+	return &MsgUpdateClientParams{
+		Authority: authority,
+		Params:    params,
+	}
+}
+
+// GetSigners returns the expected signers for a MsgUpdateClientParams message.
+func (msg *MsgUpdateClientParams) GetSigners() []sdk.AccAddress {
+	accAddr, err := sdk.AccAddressFromBech32(msg.Authority)
+	if err != nil {
+		panic(err)
+	}
+	return []sdk.AccAddress{accAddr}
+}
+
+// ValidateBasic performs basic checks on a MsgUpdateClientParams.
+func (msg *MsgUpdateClientParams) ValidateBasic() error {
+	if _, err := sdk.AccAddressFromBech32(msg.Authority); err != nil {
+		return errorsmod.Wrapf(ibcerrors.ErrInvalidAddress, "string could not be parsed as address: %v", err)
+	}
+	return msg.Params.Validate()
+}

modules/core/02-client/types/msgs_test.go

@@ -609,3 +609,43 @@ func (suite *TypesTestSuite) TestMsgSubmitMisbehaviour_ValidateBasic() {
 		}
 	}
 }
+
+// TestMsgUpdateClientParams_ValidateBasic tests ValidateBasic for MsgUpdateClientParams
+func (suite *TypesTestSuite) TestMsgUpdateClientParams_ValidateBasic() {
+	authority := suite.chainA.App.GetIBCKeeper().GetAuthority()
+	testCases := []struct {
+		name    string
+		msg     *types.MsgUpdateClientParams
+		expPass bool
+	}{
+		{
+			"success: valid authority and params",
+			types.NewMsgUpdateClientParams(authority, types.DefaultParams()),
+			true,
+		},
+		{
+			"success: valid authority empty params",
+			types.NewMsgUpdateClientParams(authority, types.Params{}),
+			true,
+		},
+		{
+			"failure: invalid authority address",
+			types.NewMsgUpdateClientParams("invalid", types.DefaultParams()),
+			false,
+		},
+		{
+			"failure: invalid allowed client",
+			types.NewMsgUpdateClientParams(authority, types.NewParams("")),
+			false,
+		},
+	}
+
+	for _, tc := range testCases {
+		err := tc.msg.ValidateBasic()
+		if tc.expPass {
+			suite.Require().NoError(err, "valid case %s failed", tc.name)
+		} else {
+			suite.Require().Error(err, "invalid case %s passed", tc.name)
+		}
+	}
+}

modules/core/02-client/types/params.go

@@ -4,23 +4,11 @@ import (
 	"fmt"
 	"strings"
 
-	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
-
 	"github.com/cosmos/ibc-go/v7/modules/core/exported"
 )
 
-var (
-	// DefaultAllowedClients are the default clients for the AllowedClients parameter.
-	DefaultAllowedClients = []string{exported.Solomachine, exported.Tendermint, exported.Localhost}
-
-	// KeyAllowedClients is store's key for AllowedClients Params
-	KeyAllowedClients = []byte("AllowedClients")
-)
-
-// ParamKeyTable type declaration for parameters
-func ParamKeyTable() paramtypes.KeyTable {
-	return paramtypes.NewKeyTable().RegisterParamSet(&Params{})
-}
+// DefaultAllowedClients are the default clients for the AllowedClients parameter.
+var DefaultAllowedClients = []string{exported.Solomachine, exported.Tendermint, exported.Localhost}
 
 // NewParams creates a new parameter configuration for the ibc client module
 func NewParams(allowedClients ...string) Params {
@@ -39,13 +27,6 @@ func (p Params) Validate() error {
 	return validateClients(p.AllowedClients)
 }
 
-// ParamSetPairs implements params.ParamSet
-func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs {
-	return paramtypes.ParamSetPairs{
-		paramtypes.NewParamSetPair(KeyAllowedClients, p.AllowedClients, validateClients),
-	}
-}
-
 // IsAllowedClient checks if the given client type is registered on the allowlist.
 func (p Params) IsAllowedClient(clientType string) bool {
 	for _, allowedClient := range p.AllowedClients {
@@ -56,12 +37,8 @@ func (p Params) IsAllowedClient(clientType string) bool {
 	return false
 }
 
-func validateClients(i interface{}) error {
-	clients, ok := i.([]string)
-	if !ok {
-		return fmt.Errorf("invalid parameter type: %T", i)
-	}
-
+// validateClients checks that the given clients are not blank.
+func validateClients(clients []string) error {
 	for i, clientType := range clients {
 		if strings.TrimSpace(clientType) == "" {
 			return fmt.Errorf("client type %d cannot be blank", i)