feat!: ics parameter migration (#1408)

* Added MsgUpdateParams

* added handlers MsgUpdateParams and migration for legacy parameter updates

* Changed to store service

* Revert "Changed to store service"

This reverts commit e47868f.

* use module's store instead of x/param parameter space

* Added migration + tests

* Addressed review comments

* Addressed comments

proto/interchain_security/ccv/provider/v1/query.proto

@@ -81,6 +81,10 @@ service Query {
     option (google.api.http).get =
         "/interchain_security/ccv/provider/registered_consumer_reward_denoms";
   }
+
+  rpc QueryParams(QueryParamsRequest) returns (QueryParamsResponse) {
+    option (google.api.http).get = "/interchain_security/ccv/provider/params";
+  }
 }
 
 message QueryConsumerGenesisRequest { string chain_id = 1; }
@@ -187,3 +191,12 @@ message QueryRegisteredConsumerRewardDenomsRequest {}
 message QueryRegisteredConsumerRewardDenomsResponse {
   repeated string denoms = 1;
 }
+
+// Request to query values set for provider parameters
+message QueryParamsRequest {}
+
+// QueryParamsResponse is response type for the Query/Params RPC method.
+message QueryParamsResponse {
+  // params holds all the parameters of this module.
+  interchain_security.ccv.provider.v1.Params params = 1 [ (gogoproto.nullable) = false ];
+}

proto/interchain_security/ccv/provider/v1/tx.proto

@@ -12,6 +12,7 @@ import "gogoproto/gogo.proto";
 import "cosmos_proto/cosmos.proto";
 import "cosmos/msg/v1/msg.proto";
 import "ibc/core/client/v1/client.proto";
+import "interchain_security/ccv/provider/v1/provider.proto";
 
 // Msg defines the Msg service.
 service Msg {
@@ -28,8 +29,26 @@ service Msg {
 
   rpc ChangeRewardDenoms(MsgChangeRewardDenoms)
     returns (MsgChangeRewardDenomsResponse);
+
+  rpc UpdateParams(MsgUpdateParams)
+  returns (MsgUpdateParamsResponse);
 }
 
+
+
+// MsgUpdateParams is the Msg/UpdateParams request type
+message MsgUpdateParams {
+  option (cosmos.msg.v1.signer) = "signer";
+
+  // signer is the address of the governance account.
+  string signer = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
+
+  // params defines the x/provider parameters to update.
+  Params params = 2 [(gogoproto.nullable) = false];
+}
+
+message MsgUpdateParamsResponse {}
+
 message MsgAssignConsumerKey {
   option (gogoproto.equal) = false;
   option (gogoproto.goproto_getters) = false;

x/ccv/consumer/keeper/keeper.go

@@ -39,7 +39,6 @@ type Keeper struct {
 	storeKey         storetypes.StoreKey // TODO: maybe needs to be removed?
 	storeService     store.KVStoreService
 	cdc              codec.BinaryCodec
-	paramStore       paramtypes.Subspace
 	scopedKeeper     ccv.ScopedKeeper
 	channelKeeper    ccv.ChannelKeeper
 	portKeeper       ccv.PortKeeper
@@ -83,7 +82,6 @@ func NewKeeper(
 		authority:               authority,
 		storeKey:                key,
 		cdc:                     cdc,
-		paramStore:              paramSpace,
 		scopedKeeper:            scopedKeeper,
 		channelKeeper:           channelKeeper,
 		portKeeper:              portKeeper,
@@ -108,9 +106,8 @@ func NewKeeper(
 // Used only in testing.
 func NewNonZeroKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, paramSpace paramtypes.Subspace) Keeper {
 	return Keeper{
-		storeKey:   key,
-		cdc:        cdc,
-		paramStore: paramSpace,
+		storeKey: key,
+		cdc:      cdc,
 	}
 }
 
@@ -120,45 +117,38 @@ func (k *Keeper) SetStandaloneStakingKeeper(sk ccv.StakingKeeper) {
 	k.standaloneStakingKeeper = sk
 }
 
-// SetParamSpace sets the param space for the consumer keeper.
-// Note: this is only used for testing!
-func (k *Keeper) SetParamSpace(ctx sdk.Context, ps paramtypes.Subspace) {
-	k.paramStore = ps
-}
-
 // Validates that the consumer keeper is initialized with non-zero and
 // non-nil values for all its fields. Otherwise this method will panic.
 func (k Keeper) mustValidateFields() {
 	// Ensures no fields are missed in this validation
 	// TODO: @MSalopek hangle this better
-	if reflect.ValueOf(k).NumField() != 20 {
+	if reflect.ValueOf(k).NumField() != 19 {
 		panic(fmt.Sprintf("number of fields in consumer keeper is not 19 - have %d", reflect.ValueOf(k).NumField())) // incorrect number
 	}
 
 	if k.validatorAddressCodec == nil || k.consensusAddressCodec == nil {
 		panic("validator and/or consensus address codec are nil")
 	}
 
-	// Note 17 / 20 fields will be validated,
+	// Note 16 / 19 fields will be validated,
 	// hooks are explicitly set after the constructor,
 	// stakingKeeper is optionally set after the constructor,
 	ccv.PanicIfZeroOrNil(k.storeKey, "storeKey")                           // 1
 	ccv.PanicIfZeroOrNil(k.cdc, "cdc")                                     // 2
-	ccv.PanicIfZeroOrNil(k.paramStore, "paramStore")                       // 3
-	ccv.PanicIfZeroOrNil(k.scopedKeeper, "scopedKeeper")                   // 4
-	ccv.PanicIfZeroOrNil(k.channelKeeper, "channelKeeper")                 // 5
-	ccv.PanicIfZeroOrNil(k.portKeeper, "portKeeper")                       // 6
-	ccv.PanicIfZeroOrNil(k.connectionKeeper, "connectionKeeper")           // 7
-	ccv.PanicIfZeroOrNil(k.clientKeeper, "clientKeeper")                   // 8
-	ccv.PanicIfZeroOrNil(k.slashingKeeper, "slashingKeeper")               // 9
-	ccv.PanicIfZeroOrNil(k.bankKeeper, "bankKeeper")                       // 10
-	ccv.PanicIfZeroOrNil(k.authKeeper, "authKeeper")                       // 11
-	ccv.PanicIfZeroOrNil(k.ibcTransferKeeper, "ibcTransferKeeper")         // 12
-	ccv.PanicIfZeroOrNil(k.ibcCoreKeeper, "ibcCoreKeeper")                 // 13
-	ccv.PanicIfZeroOrNil(k.feeCollectorName, "feeCollectorName")           // 14
-	ccv.PanicIfZeroOrNil(k.authority, "authority")                         // 15
-	ccv.PanicIfZeroOrNil(k.validatorAddressCodec, "validatorAddressCodec") // 16
-	ccv.PanicIfZeroOrNil(k.consensusAddressCodec, "consensusAddressCodec") // 17
+	ccv.PanicIfZeroOrNil(k.scopedKeeper, "scopedKeeper")                   // 3
+	ccv.PanicIfZeroOrNil(k.channelKeeper, "channelKeeper")                 // 4
+	ccv.PanicIfZeroOrNil(k.portKeeper, "portKeeper")                       // 5
+	ccv.PanicIfZeroOrNil(k.connectionKeeper, "connectionKeeper")           // 6
+	ccv.PanicIfZeroOrNil(k.clientKeeper, "clientKeeper")                   // 7
+	ccv.PanicIfZeroOrNil(k.slashingKeeper, "slashingKeeper")               // 8
+	ccv.PanicIfZeroOrNil(k.bankKeeper, "bankKeeper")                       // 9
+	ccv.PanicIfZeroOrNil(k.authKeeper, "authKeeper")                       // 10
+	ccv.PanicIfZeroOrNil(k.ibcTransferKeeper, "ibcTransferKeeper")         // 11
+	ccv.PanicIfZeroOrNil(k.ibcCoreKeeper, "ibcCoreKeeper")                 // 12
+	ccv.PanicIfZeroOrNil(k.feeCollectorName, "feeCollectorName")           // 13
+	ccv.PanicIfZeroOrNil(k.authority, "authority")                         // 14
+	ccv.PanicIfZeroOrNil(k.validatorAddressCodec, "validatorAddressCodec") // 15
+	ccv.PanicIfZeroOrNil(k.consensusAddressCodec, "consensusAddressCodec") // 16
 }
 
 // Logger returns a module-specific logger.

x/ccv/consumer/keeper/legacy_params.go

@@ -0,0 +1,111 @@
+package keeper
+
+import (
+	"time"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
+
+	ccvtypes "github.com/cosmos/interchain-security/v3/x/ccv/types"
+)
+
+// Legacy: used for migration only!
+// GetConsumerParamsLegacy returns the params for the consumer ccv module from x/param subspace
+// which will be deprecated soon
+func (k Keeper) GetConsumerParamsLegacy(ctx sdk.Context, paramSpace paramtypes.Subspace) ccvtypes.Params {
+	return ccvtypes.NewParams(
+		getEnabled(ctx, paramSpace),
+		getBlocksPerDistributionTransmission(ctx, paramSpace),
+		getDistributionTransmissionChannel(ctx, paramSpace),
+		getProviderFeePoolAddrStr(ctx, paramSpace),
+		getCCVTimeoutPeriod(ctx, paramSpace),
+		getTransferTimeoutPeriod(ctx, paramSpace),
+		getConsumerRedistributionFrac(ctx, paramSpace),
+		getHistoricalEntries(ctx, paramSpace),
+		getUnbondingPeriod(ctx, paramSpace),
+		getSoftOptOutThreshold(ctx, paramSpace),
+		getRewardDenoms(ctx, paramSpace),
+		getProviderRewardDenoms(ctx, paramSpace),
+	)
+}
+
+// getEnabled returns the enabled flag for the consumer module
+func getEnabled(ctx sdk.Context, paramStore paramtypes.Subspace) bool {
+	var enabled bool
+	paramStore.Get(ctx, ccvtypes.KeyEnabled, &enabled)
+	return enabled
+}
+
+func getBlocksPerDistributionTransmission(ctx sdk.Context, paramStore paramtypes.Subspace) int64 {
+	var bpdt int64
+	paramStore.Get(ctx, ccvtypes.KeyBlocksPerDistributionTransmission, &bpdt)
+	return bpdt
+}
+
+func getDistributionTransmissionChannel(ctx sdk.Context, paramStore paramtypes.Subspace) string {
+	var s string
+	paramStore.Get(ctx, ccvtypes.KeyDistributionTransmissionChannel, &s)
+	return s
+}
+
+func getProviderFeePoolAddrStr(ctx sdk.Context, paramStore paramtypes.Subspace) string {
+	var s string
+	paramStore.Get(ctx, ccvtypes.KeyProviderFeePoolAddrStr, &s)
+	return s
+}
+
+// getCCVTimeoutPeriod returns the timeout period for sent ccv related ibc packets
+func getCCVTimeoutPeriod(ctx sdk.Context, paramStore paramtypes.Subspace) time.Duration {
+	var p time.Duration
+	paramStore.Get(ctx, ccvtypes.KeyCCVTimeoutPeriod, &p)
+	return p
+}
+
+// getTransferTimeoutPeriod returns the timeout period for sent transfer related ibc packets
+func getTransferTimeoutPeriod(ctx sdk.Context, paramStore paramtypes.Subspace) time.Duration {
+	var p time.Duration
+	paramStore.Get(ctx, ccvtypes.KeyTransferTimeoutPeriod, &p)
+	return p
+}
+
+// getConsumerRedistributionFrac returns the fraction of tokens allocated to the consumer redistribution
+// address during distribution events. The fraction is a string representing a
+// decimal number. For example "0.75" would represent 75%.
+func getConsumerRedistributionFrac(ctx sdk.Context, paramStore paramtypes.Subspace) string {
+	var str string
+	paramStore.Get(ctx, ccvtypes.KeyConsumerRedistributionFrac, &str)
+	return str
+}
+
+// getHistoricalEntries returns the number of historical info entries to persist in store
+func getHistoricalEntries(ctx sdk.Context, paramStore paramtypes.Subspace) int64 {
+	var n int64
+	paramStore.Get(ctx, ccvtypes.KeyHistoricalEntries, &n)
+	return n
+}
+
+func getUnbondingPeriod(ctx sdk.Context, paramStore paramtypes.Subspace) time.Duration {
+	var period time.Duration
+	paramStore.Get(ctx, ccvtypes.KeyConsumerUnbondingPeriod, &period)
+	return period
+}
+
+// getSoftOptOutThreshold returns the percentage of validators at the bottom of the set
+// that can opt out of running the consumer chain
+func getSoftOptOutThreshold(ctx sdk.Context, paramStore paramtypes.Subspace) string {
+	var str string
+	paramStore.Get(ctx, ccvtypes.KeySoftOptOutThreshold, &str)
+	return str
+}
+
+func getRewardDenoms(ctx sdk.Context, paramStore paramtypes.Subspace) []string {
+	var denoms []string
+	paramStore.Get(ctx, ccvtypes.KeyRewardDenoms, &denoms)
+	return denoms
+}
+
+func getProviderRewardDenoms(ctx sdk.Context, paramStore paramtypes.Subspace) []string {
+	var denoms []string
+	paramStore.Get(ctx, ccvtypes.KeyProviderRewardDenoms, &denoms)
+	return denoms
+}

x/ccv/consumer/keeper/migration.go

@@ -21,6 +21,19 @@ func NewMigrator(ccvConsumerKeeper Keeper, ccvConsumerParamSpace paramtypes.Subs
 	return Migrator{ccvConsumerKeeper: ccvConsumerKeeper, ccvConsumerParamSpace: ccvConsumerParamSpace}
 }
 
+// MigrateParams migrates the consumers module's parameters from the x/params subspace to the
+// consumer modules store.
+func (m Migrator) MigrateParams(ctx sdk.Context) error {
+	params := m.ccvConsumerKeeper.GetConsumerParamsLegacy(ctx, m.ccvConsumerParamSpace)
+	err := params.Validate()
+	if err != nil {
+		return err
+	}
+	m.ccvConsumerKeeper.SetParams(ctx, params)
+	m.ccvConsumerKeeper.Logger(ctx).Info("successfully migrated provider parameters")
+	return nil
+}
+
 // MigrateConsumerPacketData migrates consumer packet data according to
 // https://github.com/cosmos/interchain-security/pull/1037
 //

x/ccv/consumer/keeper/migration_test.go

@@ -3,9 +3,13 @@ package keeper_test
 import (
 	"testing"
 
+	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
 	"github.com/stretchr/testify/require"
 
 	testutil "github.com/cosmos/interchain-security/v3/testutil/keeper"
+
+	"github.com/cosmos/interchain-security/v3/x/ccv/consumer/keeper"
+
 	ccvtypes "github.com/cosmos/interchain-security/v3/x/ccv/types"
 )
 
@@ -61,3 +65,33 @@ func TestMigrateConsumerPacketData(t *testing.T) {
 	require.Equal(t, uint64(88), obtainedPackets[1].GetVscMaturedPacketData().ValsetUpdateId)
 	require.Equal(t, uint64(99), obtainedPackets[2].GetVscMaturedPacketData().ValsetUpdateId)
 }
+
+func TestMigrateParams(t *testing.T) {
+	params := testutil.NewInMemKeeperParams(t)
+	consumerKeeper, ctx, ctrl, _ := testutil.GetConsumerKeeperAndCtx(t, params)
+	defer ctrl.Finish()
+
+	testCases := []struct {
+		name          string
+		legacyParams  func() paramtypes.Subspace
+		expetedParams ccvtypes.Params
+	}{
+		{
+			"default params",
+			func() paramtypes.Subspace {
+				subspace := params.ParamsSubspace
+				defaultParams := ccvtypes.DefaultParams()
+				subspace.SetParamSet(ctx, &defaultParams)
+				return *subspace
+			},
+			ccvtypes.DefaultParams(),
+		},
+	}
+	for _, tc := range testCases {
+		migrator := keeper.NewMigrator(consumerKeeper, tc.legacyParams())
+		err := migrator.MigrateParams(ctx)
+		require.NoError(t, err)
+		params := consumerKeeper.GetConsumerParams(ctx)
+		require.Equal(t, tc.expetedParams, params)
+	}
+}