Attribute param store addition (#1927)

* switch to using module store instead of param space

* add param tests

* Add migration handler for attribute values

* add upgrades test, add remove todos, fix param space

* fix logging test, fix lint issue with delegation logging

* remove param space, fix param query, add changelog, fix migration method

CHANGELOG.md

@@ -55,6 +55,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * Bump the SDK to `v0.50.5-pio-1` (from an earlier ephemeral version) [#1897](https://github.com/provenance-io/provenance/pull/1897).
 * Removed `rewards` module [#1905](https://github.com/provenance-io/provenance/pull/1905).
 * Remove unused navs [#1920](https://github.com/provenance-io/provenance/issues/1920).
+* Migrate module params from param space to module store.
+  *  Attribute module param migration [#1927](https://github.com/provenance-io/provenance/pull/1927)
 
 ### Dependencies
 

app/app.go

@@ -1361,11 +1361,10 @@ func initParamsKeeper(appCodec codec.BinaryCodec, legacyAmino *codec.LegacyAmino
 	paramsKeeper.Subspace(govtypes.ModuleName).WithKeyTable(govtypesv1.ParamKeyTable())
 	paramsKeeper.Subspace(crisistypes.ModuleName)
 
-	paramsKeeper.Subspace(metadatatypes.ModuleName)  // TODO[1760]: params: Migrate metadata params.
-	paramsKeeper.Subspace(markertypes.ModuleName)    // TODO[1760]: params: Migrate marker params.
-	paramsKeeper.Subspace(nametypes.ModuleName)      // TODO[1760]: params: Migrate name params.
-	paramsKeeper.Subspace(attributetypes.ModuleName) // TODO[1760]: params: Migrate attribute params.
-	paramsKeeper.Subspace(msgfeestypes.ModuleName)   // TODO[1760]: params: Migrate msgFees params.
+	paramsKeeper.Subspace(metadatatypes.ModuleName) // TODO[1760]: params: Migrate metadata params.
+	paramsKeeper.Subspace(markertypes.ModuleName)   // TODO[1760]: params: Migrate marker params.
+	paramsKeeper.Subspace(nametypes.ModuleName)     // TODO[1760]: params: Migrate name params.
+	paramsKeeper.Subspace(msgfeestypes.ModuleName)  // TODO[1760]: params: Migrate msgFees params.
 	paramsKeeper.Subspace(wasmtypes.ModuleName)
 	paramsKeeper.Subspace(triggertypes.ModuleName) // TODO[1760]: params: Migrate trigger params.
 

app/upgrades.go

@@ -13,6 +13,7 @@ import (
 	paramstypes "github.com/cosmos/cosmos-sdk/x/params/types"
 	"github.com/cosmos/ibc-go/v8/modules/core/exported"
 	ibctmmigrations "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint/migrations"
+	attributetypes "github.com/provenance-io/provenance/x/attribute/types"
 )
 
 // appUpgrade is an internal structure for defining all things for an upgrade.
@@ -55,6 +56,8 @@ var upgrades = map[string]appUpgrade{
 				return nil, err
 			}
 
+			migrateAttributeParams(ctx, app)
+
 			vm, err = runModuleMigrations(ctx, app, vm)
 			if err != nil {
 				return nil, err
@@ -85,6 +88,8 @@ var upgrades = map[string]appUpgrade{
 				return nil, err
 			}
 
+			migrateAttributeParams(ctx, app)
+
 			vm, err = runModuleMigrations(ctx, app, vm)
 			if err != nil {
 				return nil, err
@@ -188,7 +193,7 @@ var _ = runModuleMigrations
 // removeInactiveValidatorDelegations unbonds all delegations from inactive validators, triggering their removal from the validator set.
 // This should be applied in most upgrades.
 func removeInactiveValidatorDelegations(ctx sdk.Context, app *App) {
-	ctx.Logger().Info(fmt.Sprintf("Removing inactive validator delegations."))
+	ctx.Logger().Info("Removing inactive validator delegations.")
 
 	sParams, perr := app.StakingKeeper.GetParams(ctx)
 	if perr != nil {
@@ -282,3 +287,17 @@ func migrateBaseappParams(ctx sdk.Context, app *App) error {
 	ctx.Logger().Info("Done migrating legacy params.")
 	return nil
 }
+
+// migrateAttributeParams migrates to new Attribute Params store
+// TODO: Remove with the umber handlers.
+func migrateAttributeParams(ctx sdk.Context, app *App) {
+	ctx.Logger().Info("Migrating attribute params.")
+	attributeParamSpace := app.ParamsKeeper.Subspace(attributetypes.ModuleName).WithKeyTable(attributetypes.ParamKeyTable())
+	maxValueLength := uint32(attributetypes.DefaultMaxValueLength)
+	// TODO: remove attributetypes.ParamStoreKeyMaxValueLength with the umber handlers.
+	if attributeParamSpace.Has(ctx, attributetypes.ParamStoreKeyMaxValueLength) {
+		attributeParamSpace.Get(ctx, attributetypes.ParamStoreKeyMaxValueLength, &maxValueLength)
+	}
+	app.AttributeKeeper.SetParams(ctx, attributetypes.Params{MaxValueLength: uint32(maxValueLength)})
+	ctx.Logger().Info("Done migrating attribute params.")
+}

app/upgrades_test.go

@@ -379,10 +379,14 @@ func (s *UpgradeTestSuite) TestUmberRC1() {
 		"INF Done pruning expired consensus states for IBC.",
 		"INF Migrating legacy params.",
 		"INF Done migrating legacy params.",
+		"INF Migrating attribute params.",
+		"INF Done migrating attribute params.",
+		"INF Starting module migrations. This may take a significant amount of time to complete. Do not restart node.",
 		"INF Updating IBC AllowedClients.",
 		"INF Done updating IBC AllowedClients.",
-		"INF Starting module migrations. This may take a significant amount of time to complete. Do not restart node.",
-		"INF removing all delegations from validators that have been inactive (unbonded) for 21 days",
+		"INF Removing inactive validator delegations.",
+		"INF Threshold: 21 days",
+		"INF A total of 0 inactive (unbonded) validators have had all their delegators removed.",
 	}
 
 	s.AssertUpgradeHandlerLogs("umber-rc1", expInLog, nil)
@@ -394,10 +398,14 @@ func (s *UpgradeTestSuite) TestUmber() {
 		"INF Done pruning expired consensus states for IBC.",
 		"INF Migrating legacy params.",
 		"INF Done migrating legacy params.",
+		"INF Migrating attribute params.",
+		"INF Done migrating attribute params.",
+		"INF Starting module migrations. This may take a significant amount of time to complete. Do not restart node.",
 		"INF Updating IBC AllowedClients.",
 		"INF Done updating IBC AllowedClients.",
-		"INF Starting module migrations. This may take a significant amount of time to complete. Do not restart node.",
-		"INF removing all delegations from validators that have been inactive (unbonded) for 21 days",
+		"INF Removing inactive validator delegations.",
+		"INF Threshold: 21 days",
+		"INF A total of 0 inactive (unbonded) validators have had all their delegators removed.",
 	}
 
 	s.AssertUpgradeHandlerLogs("umber", expInLog, nil)

x/attribute/keeper/keeper.go

@@ -24,9 +24,6 @@ type Handler func(record types.Attribute) error
 
 // Keeper defines the attribute module Keeper
 type Keeper struct {
-	// The reference to the Paramstore to get and set attribute specific params
-	paramSpace paramtypes.Subspace
-
 	// Used to ensure accounts exist for addresses.
 	authKeeper types.AccountKeeper
 	// The keeper used for ensuring names resolve to owners.
@@ -51,13 +48,8 @@ func NewKeeper(
 	cdc codec.BinaryCodec, key storetypes.StoreKey, paramSpace paramtypes.Subspace,
 	authKeeper types.AccountKeeper, nameKeeper types.NameKeeper,
 ) Keeper {
-	if !paramSpace.HasKeyTable() {
-		paramSpace = paramSpace.WithKeyTable(types.ParamKeyTable())
-	}
-
 	keeper := Keeper{
 		storeKey:   key,
-		paramSpace: paramSpace,
 		authKeeper: authKeeper,
 		nameKeeper: nameKeeper,
 		cdc:        cdc,

x/attribute/keeper/param_test.go

@@ -0,0 +1,52 @@
+package keeper_test
+
+import (
+	"testing"
+	"time"
+
+	cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/provenance-io/provenance/app"
+	simapp "github.com/provenance-io/provenance/app"
+	"github.com/provenance-io/provenance/x/attribute/types"
+	"github.com/stretchr/testify/suite"
+)
+
+type ParamTestSuite struct {
+	suite.Suite
+
+	app *app.App
+	ctx sdk.Context
+
+	startBlockTime time.Time
+}
+
+func (s *ParamTestSuite) SetupTest() {
+	s.app = simapp.Setup(s.T())
+	s.startBlockTime = time.Now()
+	s.ctx = s.app.BaseApp.NewContextLegacy(false, cmtproto.Header{Time: s.startBlockTime})
+}
+
+func TestParamTestSuite(t *testing.T) {
+	suite.Run(t, new(ParamTestSuite))
+}
+
+func (s *ParamTestSuite) TestGetSetParams() {
+	defaultParams := s.app.AttributeKeeper.GetParams(s.ctx)
+	s.Require().Equal(int(types.DefaultMaxValueLength), int(defaultParams.MaxValueLength), "GetParams() Default max value length should match")
+
+	defaultValueLength := s.app.AttributeKeeper.GetMaxValueLength(s.ctx)
+	s.Require().Equal(int(types.DefaultMaxValueLength), int(defaultValueLength), "GetMaxValueLength() Default max value length should match")
+
+	newMaxValueLength := uint32(2048)
+	newParams := types.Params{
+		MaxValueLength: newMaxValueLength,
+	}
+	s.app.AttributeKeeper.SetParams(s.ctx, newParams)
+
+	updatedParams := s.app.AttributeKeeper.GetParams(s.ctx)
+	s.Require().Equal(int(newMaxValueLength), int(updatedParams.MaxValueLength), "GetParams() Updated max value length should match")
+
+	updatedValueLength := s.app.AttributeKeeper.GetMaxValueLength(s.ctx)
+	s.Require().Equal(int(newMaxValueLength), int(updatedValueLength), "GetMaxValueLength() Updated max value length should match")
+}

x/attribute/keeper/params.go

@@ -6,23 +6,34 @@ import (
 	"github.com/provenance-io/provenance/x/attribute/types"
 )
 
-// GetParams returns the total set of account parameters.
+// GetParams returns the attribute Params.
 func (k Keeper) GetParams(ctx sdk.Context) (params types.Params) {
-	return types.Params{
-		MaxValueLength: k.GetMaxValueLength(ctx),
+	store := ctx.KVStore(k.storeKey)
+	bz := store.Get(types.AttributeParamPrefix)
+	if bz == nil {
+		return types.Params{
+			MaxValueLength: types.DefaultMaxValueLength,
+		}
 	}
+	err := k.cdc.Unmarshal(bz, &params)
+	if err != nil {
+		panic(err)
+	}
+	return params
 }
 
-// SetParams sets the account parameters to the param space.
+// SetParams sets the account parameters to the param store.
 func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
-	k.paramSpace.SetParamSet(ctx, &params)
+	bz, err := k.cdc.Marshal(&params)
+	if err != nil {
+		panic(err)
+	}
+
+	store := ctx.KVStore(k.storeKey)
+	store.Set(types.AttributeParamPrefix, bz)
 }
 
-// GetMaxValueLength returns the current distribution community tax (or default if unset)
+// GetMaxValueLength returns the max value for attribute length.
 func (k Keeper) GetMaxValueLength(ctx sdk.Context) (maxValueLength uint32) {
-	maxValueLength = types.DefaultMaxValueLength
-	if k.paramSpace.Has(ctx, types.ParamStoreKeyMaxValueLength) {
-		k.paramSpace.Get(ctx, types.ParamStoreKeyMaxValueLength, &maxValueLength)
-	}
-	return
+	return k.GetParams(ctx).MaxValueLength
 }

x/attribute/keeper/query_server.go

@@ -20,10 +20,7 @@ var _ types.QueryServer = Keeper{}
 // Params queries params of attribute module
 func (k Keeper) Params(c context.Context, _ *types.QueryParamsRequest) (*types.QueryParamsResponse, error) {
 	ctx := sdk.UnwrapSDKContext(c)
-	var params types.Params
-	k.paramSpace.GetParamSet(ctx, &params)
-
-	return &types.QueryParamsResponse{Params: params}, nil
+	return &types.QueryParamsResponse{Params: k.GetParams(ctx)}, nil
 }
 
 // Attribute queries for a specific attribute

x/attribute/types/keys.go

@@ -31,6 +31,7 @@ var (
 	AttributeKeyPrefix           = []byte{0x02}
 	AttributeAddrLookupKeyPrefix = []byte{0x03}
 	AttributeExpirationKeyPrefix = []byte{0x04}
+	AttributeParamPrefix         = []byte{0x05}
 )
 
 // AddrAttributeKey creates a key for an account attribute

x/attribute/types/params.go

@@ -14,6 +14,7 @@ const (
 )
 
 // Parameter store keys
+// TODO: remove with the umber (v1.19.x) handlers.
 var (
 	ParamStoreKeyMaxValueLength = []byte("MaxValueLength")
 )
@@ -25,6 +26,7 @@ func (params Params) String() string {
 }
 
 // ParamKeyTable for slashing module
+// TODO: remove with the umber (v1.19.x) handlers.
 func ParamKeyTable() paramtypes.KeyTable {
 	return paramtypes.NewKeyTable().RegisterParamSet(&Params{})
 }
@@ -39,6 +41,7 @@ func NewParams(
 }
 
 // ParamSetPairs - Implements params.ParamSet
+// TODO: remove with the umber (v1.19.x) handlers.
 func (params *Params) ParamSetPairs() paramtypes.ParamSetPairs {
 	return paramtypes.ParamSetPairs{
 		paramtypes.NewParamSetPair(ParamStoreKeyMaxValueLength, &params.MaxValueLength, validateMaxValueLength),