Name param store migration (#1937)

* remove param store, convert params to using module's store * rename default value variable, add params tests * add migration method, add remove todos * update comment for marker key, add migration log tests * add method calls to upgrades * change key name for each module
provenance-io · Apr 18, 2024 · 5aaecbd · 5aaecbd
1 parent 0a8592c
commit 5aaecbd
Show file tree

Hide file tree

Showing 17 changed files with 145 additions and 76 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -62,6 +62,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
   *  Marker module param migration [#1934](https://github.com/provenance-io/provenance/pull/1934)
   *  Metadata module param migration [#1932](https://github.com/provenance-io/provenance/pull/1932)
   *  Msgfees module param migration [#1936](https://github.com/provenance-io/provenance/pull/1936)
+  *  Name module param migration [#1937](https://github.com/provenance-io/provenance/pull/1937)
 * Restore the hold module [#1930](https://github.com/provenance-io/provenance/pull/1930).
 * Restore gov-prop cli commands and fix next key decoding [#1930](https://github.com/provenance-io/provenance/pull/1930).
 * Switch to InputOutputCoinsProv for exchange transfers [#1930](https://github.com/provenance-io/provenance/pull/1930).

diff --git a/app/app.go b/app/app.go
@@ -551,9 +551,7 @@ func New(
 	hooksTransferModule := ibchooks.NewIBCMiddleware(&rateLimitingTransferModule, &app.HooksICS4Wrapper)
 	app.TransferStack = &hooksTransferModule
 
-	app.NameKeeper = namekeeper.NewKeeper(
-		appCodec, keys[nametypes.StoreKey], app.GetSubspace(nametypes.ModuleName),
-	)
+	app.NameKeeper = namekeeper.NewKeeper(appCodec, keys[nametypes.StoreKey])
 
 	app.AttributeKeeper = attributekeeper.NewKeeper(
 		appCodec, keys[attributetypes.StoreKey], app.GetSubspace(attributetypes.ModuleName), app.AccountKeeper, &app.NameKeeper,
@@ -1361,7 +1359,6 @@ func initParamsKeeper(appCodec codec.BinaryCodec, legacyAmino *codec.LegacyAmino
 	paramsKeeper.Subspace(govtypes.ModuleName).WithKeyTable(govtypesv1.ParamKeyTable())
 	paramsKeeper.Subspace(crisistypes.ModuleName)
 
-	paramsKeeper.Subspace(nametypes.ModuleName) // TODO[1760]: params: Migrate name params.
 	paramsKeeper.Subspace(wasmtypes.ModuleName)
 	paramsKeeper.Subspace(triggertypes.ModuleName) // TODO[1760]: params: Migrate trigger params.
 

diff --git a/app/upgrades.go b/app/upgrades.go
@@ -19,6 +19,7 @@ import (
 	markertypes "github.com/provenance-io/provenance/x/marker/types"
 	metadatatypes "github.com/provenance-io/provenance/x/metadata/types"
 	msgfeestypes "github.com/provenance-io/provenance/x/msgfees/types"
+	nametypes "github.com/provenance-io/provenance/x/name/types"
 )
 
 // appUpgrade is an internal structure for defining all things for an upgrade.
@@ -70,6 +71,7 @@ var upgrades = map[string]appUpgrade{
 			migrateMarkerParams(ctx, app)
 			migrateMetadataOSLocatorParams(ctx, app)
 			migrateMsgFeesParams(ctx, app)
+			migrateNameParams(ctx, app)
 
 			vm, err = runModuleMigrations(ctx, app, vm)
 			if err != nil {
@@ -110,6 +112,7 @@ var upgrades = map[string]appUpgrade{
 			migrateMarkerParams(ctx, app)
 			migrateMetadataOSLocatorParams(ctx, app)
 			migrateMsgFeesParams(ctx, app)
+			migrateNameParams(ctx, app)
 
 			vm, err = runModuleMigrations(ctx, app, vm)
 			if err != nil {
@@ -400,6 +403,35 @@ func migrateMetadataOSLocatorParams(ctx sdk.Context, app *App) {
 
 }
 
+// migrateNameParams migrates to new Name Params store
+// TODO: Remove with the umber handlers.
+func migrateNameParams(ctx sdk.Context, app *App) {
+	ctx.Logger().Info("Migrating name params.")
+	nameParamSpace := app.ParamsKeeper.Subspace(nametypes.ModuleName)
+
+	params := nametypes.DefaultParams()
+
+	// TODO: all param keys from types/params with the umber handlers.
+	if nameParamSpace.Has(ctx, nametypes.ParamStoreKeyMaxNameLevels) {
+		nameParamSpace.Get(ctx, nametypes.ParamStoreKeyMaxNameLevels, &params.MaxNameLevels)
+	}
+
+	if nameParamSpace.Has(ctx, nametypes.ParamStoreKeyMaxSegmentLength) {
+		nameParamSpace.Get(ctx, nametypes.ParamStoreKeyMaxSegmentLength, &params.MaxSegmentLength)
+	}
+
+	if nameParamSpace.Has(ctx, nametypes.ParamStoreKeyMinSegmentLength) {
+		nameParamSpace.Get(ctx, nametypes.ParamStoreKeyMinSegmentLength, &params.MinSegmentLength)
+	}
+
+	if nameParamSpace.Has(ctx, nametypes.ParamStoreKeyAllowUnrestrictedNames) {
+		nameParamSpace.Get(ctx, nametypes.ParamStoreKeyAllowUnrestrictedNames, &params.AllowUnrestrictedNames)
+	}
+	app.NameKeeper.SetParams(ctx, params)
+
+	ctx.Logger().Info("Done migrating name params.")
+}
+
 // migrateMsgFeesParams migrates to new MsgFees Params store
 // TODO: Remove with the umber handlers.
 func migrateMsgFeesParams(ctx sdk.Context, app *App) {

diff --git a/app/upgrades_test.go b/app/upgrades_test.go
@@ -389,6 +389,8 @@ func (s *UpgradeTestSuite) TestUmberRC1() {
 		"INF Done migrating metadata os locator params.",
 		"INF Migrating msgfees params.",
 		"INF Done migrating msgfees params.",
+		"INF Migrating name params.",
+		"INF Done migrating name 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.",
@@ -416,6 +418,8 @@ func (s *UpgradeTestSuite) TestUmber() {
 		"INF Done migrating metadata os locator params.",
 		"INF Migrating msgfees params.",
 		"INF Done migrating msgfees params.",
+		"INF Migrating name params.",
+		"INF Done migrating name 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.",

diff --git a/x/marker/keeper/params.go b/x/marker/keeper/params.go
@@ -15,7 +15,7 @@ func (k Keeper) GetParams(ctx sdk.Context) (params types.Params) {
 	params = types.DefaultParams() // Assuming a method that returns default parameters
 
 	// Deserialize parameters if they are set
-	if bz := store.Get(types.ParamStoreKey); bz != nil {
+	if bz := store.Get(types.MarkerParamStoreKey); bz != nil {
 		k.cdc.MustUnmarshal(bz, &params)
 	}
 
@@ -26,7 +26,7 @@ func (k Keeper) GetParams(ctx sdk.Context) (params types.Params) {
 func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
 	store := ctx.KVStore(k.storeKey)
 	bz := k.cdc.MustMarshal(&params)
-	store.Set(types.ParamStoreKey, bz)
+	store.Set(types.MarkerParamStoreKey, bz)
 }
 
 // Deprecated: GetMaxTotalSupply is kept for backwards compatibility.

diff --git a/x/marker/types/key.go b/x/marker/types/key.go
@@ -39,8 +39,8 @@ var (
 	// NetAssetValuePrefix prefix for net asset values of markers
 	NetAssetValuePrefix = []byte{0x04}
 
-	// ParamStoreKey prefix for marker module's params
-	ParamStoreKey = []byte{0x05}
+	// MarkerParamStoreKey key for marker module's params
+	MarkerParamStoreKey = []byte{0x05}
 )
 
 // MarkerAddress returns the module account address for the given denomination

diff --git a/x/msgfees/keeper/params.go b/x/msgfees/keeper/params.go
@@ -11,7 +11,7 @@ func (k Keeper) GetParams(ctx sdk.Context) (params types.Params) {
 	store := ctx.KVStore(k.storeKey)
 	params = types.DefaultParams() // Initialize with defaults
 
-	bz := store.Get(types.ParamStoreKey)
+	bz := store.Get(types.MsgFeesParamStoreKey)
 	if bz != nil {
 		k.cdc.MustUnmarshal(bz, &params) // Deserialize parameters
 	}
@@ -22,7 +22,7 @@ func (k Keeper) GetParams(ctx sdk.Context) (params types.Params) {
 func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
 	store := ctx.KVStore(k.storeKey)
 	bz := k.cdc.MustMarshal(&params) // Serialize parameters
-	store.Set(types.ParamStoreKey, bz)
+	store.Set(types.MsgFeesParamStoreKey, bz)
 }
 
 // GetFloorGasPrice returns the current minimum gas price in sdk.Coin used in calculations for charging additional fees

diff --git a/x/msgfees/types/keys.go b/x/msgfees/types/keys.go
@@ -32,8 +32,8 @@ func GetMsgFeeKey(msgType string) []byte {
 var (
 	//MsgFeeKeyPrefix prefix for msgfee entry
 	MsgFeeKeyPrefix = []byte{0x00}
-	// ParamStoreKey key for msgfees module's params
-	ParamStoreKey = []byte{0x01}
+	// MsgFeesParamStoreKey key for msgfees module's params
+	MsgFeesParamStoreKey = []byte{0x01}
 )
 
 func GetCompositeKey(msgType string, recipient string) string {

diff --git a/x/name/keeper/keeper.go b/x/name/keeper/keeper.go
@@ -12,16 +12,12 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
-	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
 
 	"github.com/provenance-io/provenance/x/name/types"
 )
 
 // Keeper defines the name module Keeper
 type Keeper struct {
-	// The reference to the Paramstore to get and set account specific params
-	paramSpace paramtypes.Subspace
-
 	// Key to access the key-value store from sdk.Context.
 	storeKey storetypes.StoreKey
 
@@ -42,18 +38,11 @@ type Keeper struct {
 func NewKeeper(
 	cdc codec.BinaryCodec,
 	key storetypes.StoreKey,
-	paramSpace paramtypes.Subspace,
 ) Keeper {
-	// set KeyTable if it has not already been set
-	if !paramSpace.HasKeyTable() {
-		paramSpace = paramSpace.WithKeyTable(types.ParamKeyTable())
-	}
-
 	return Keeper{
-		storeKey:   key,
-		paramSpace: paramSpace,
-		cdc:        cdc,
-		authority:  authtypes.NewModuleAddress(govtypes.ModuleName).String(),
+		storeKey:  key,
+		cdc:       cdc,
+		authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(),
 	}
 }
 

diff --git a/x/name/keeper/params.go b/x/name/keeper/params.go
@@ -2,57 +2,44 @@ package keeper
 
 import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
-
 	"github.com/provenance-io/provenance/x/name/types"
 )
 
-// GetParams returns the total set of name parameters with fall thorugh to default values.
+// GetParams returns the total set of name parameters with fallback to default values.
 func (k Keeper) GetParams(ctx sdk.Context) (params types.Params) {
-	return types.Params{
-		MaxSegmentLength:       k.GetMaxSegmentLength(ctx),
-		MinSegmentLength:       k.GetMinSegmentLength(ctx),
-		MaxNameLevels:          k.GetMaxNameLevels(ctx),
-		AllowUnrestrictedNames: k.GetAllowUnrestrictedNames(ctx),
+	store := ctx.KVStore(k.storeKey)
+	params = types.DefaultParams() // Assuming DefaultParams initializes all defaults
+
+	bz := store.Get(types.NameParamStoreKey) // General key for all parameters
+	if bz != nil {
+		k.cdc.MustUnmarshal(bz, &params) // Deserialize parameters from bytes
 	}
+	return params
 }
 
-// SetParams sets the distribution parameters to the param space.
+// SetParams sets the name parameters to the store.
 func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
-	k.paramSpace.SetParamSet(ctx, &params)
+	store := ctx.KVStore(k.storeKey)
+	bz := k.cdc.MustMarshal(&params) // Serialize parameters to bytes
+	store.Set(types.NameParamStoreKey, bz)
 }
 
-// GetMaxNameLevels returns the current maximum number of name segments allowed (or default if unset)
-func (k Keeper) GetMaxNameLevels(ctx sdk.Context) (max uint32) {
-	max = types.DefaultMaxSegments
-	if k.paramSpace.Has(ctx, types.ParamStoreKeyMaxNameLevels) {
-		k.paramSpace.Get(ctx, types.ParamStoreKeyMaxNameLevels, &max)
-	}
-	return
+// GetMaxNameLevels returns the current maximum number of name segments allowed.
+func (k Keeper) GetMaxNameLevels(ctx sdk.Context) uint32 {
+	return k.GetParams(ctx).MaxNameLevels
 }
 
-// GetMaxSegmentLength returns the current maximum length allowed for a name segment (or default if unset)
-func (k Keeper) GetMaxSegmentLength(ctx sdk.Context) (max uint32) {
-	max = types.DefaultMaxSegmentLength
-	if k.paramSpace.Has(ctx, types.ParamStoreKeyMaxSegmentLength) {
-		k.paramSpace.Get(ctx, types.ParamStoreKeyMaxSegmentLength, &max)
-	}
-	return
+// GetMaxSegmentLength returns the current maximum length allowed for a name segment.
+func (k Keeper) GetMaxSegmentLength(ctx sdk.Context) uint32 {
+	return k.GetParams(ctx).MaxSegmentLength
 }
 
-// GetMinSegmentLength returns the current minimum allowed name segment length (or default if unset)
-func (k Keeper) GetMinSegmentLength(ctx sdk.Context) (min uint32) {
-	min = types.DefaultMinSegmentLength
-	if k.paramSpace.Has(ctx, types.ParamStoreKeyMinSegmentLength) {
-		k.paramSpace.Get(ctx, types.ParamStoreKeyMinSegmentLength, &min)
-	}
-	return
+// GetMinSegmentLength returns the current minimum allowed name segment length.
+func (k Keeper) GetMinSegmentLength(ctx sdk.Context) uint32 {
+	return k.GetParams(ctx).MinSegmentLength
 }
 
-// GetAllowUnrestrictedNames returns the current unrestricted names allowed parameter (or default if unset)
-func (k Keeper) GetAllowUnrestrictedNames(ctx sdk.Context) (enabled bool) {
-	enabled = types.DefaultAllowUnrestrictedNames
-	if k.paramSpace.Has(ctx, types.ParamStoreKeyAllowUnrestrictedNames) {
-		k.paramSpace.Get(ctx, types.ParamStoreKeyAllowUnrestrictedNames, &enabled)
-	}
-	return
+// GetAllowUnrestrictedNames returns whether unrestricted names are allowed.
+func (k Keeper) GetAllowUnrestrictedNames(ctx sdk.Context) bool {
+	return k.GetParams(ctx).AllowUnrestrictedNames
 }
diff --git a/x/name/keeper/params_test.go b/x/name/keeper/params_test.go
@@ -0,0 +1,57 @@
+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/name/types"
+	"github.com/stretchr/testify/suite"
+)
+
+type NameParamTestSuite struct {
+	suite.Suite
+
+	app *app.App
+	ctx sdk.Context
+}
+
+func (s *NameParamTestSuite) SetupTest() {
+	s.app = simapp.Setup(s.T())
+	s.ctx = s.app.BaseApp.NewContextLegacy(false, cmtproto.Header{Time: time.Now()})
+}
+
+func TestNameParamTestSuite(t *testing.T) {
+	suite.Run(t, new(NameParamTestSuite))
+}
+
+func (s *NameParamTestSuite) TestGetSetParams() {
+	defaultParams := s.app.NameKeeper.GetParams(s.ctx)
+	s.Require().Equal(types.DefaultMaxNameLevels, defaultParams.MaxNameLevels, "Default MaxNameLevels should match")
+	s.Require().Equal(types.DefaultMaxSegmentLength, defaultParams.MaxSegmentLength, "Default MaxSegmentLength should match")
+	s.Require().Equal(types.DefaultMinSegmentLength, defaultParams.MinSegmentLength, "Default MinSegmentLength should match")
+	s.Require().Equal(types.DefaultAllowUnrestrictedNames, defaultParams.AllowUnrestrictedNames, "Default AllowUnrestrictedNames should match")
+
+	newMaxNameLevels := uint32(10)
+	newMaxSegmentLength := uint32(15)
+	newMinSegmentLength := uint32(2)
+	newAllowUnrestrictedNames := false
+
+	newParams := types.Params{
+		MaxNameLevels:          newMaxNameLevels,
+		MaxSegmentLength:       newMaxSegmentLength,
+		MinSegmentLength:       newMinSegmentLength,
+		AllowUnrestrictedNames: newAllowUnrestrictedNames,
+	}
+
+	s.app.NameKeeper.SetParams(s.ctx, newParams)
+
+	updatedParams := s.app.NameKeeper.GetParams(s.ctx)
+	s.Require().Equal(newMaxNameLevels, updatedParams.MaxNameLevels, "Updated MaxNameLevels should match")
+	s.Require().Equal(newMaxSegmentLength, updatedParams.MaxSegmentLength, "Updated MaxSegmentLength should match")
+	s.Require().Equal(newMinSegmentLength, updatedParams.MinSegmentLength, "Updated MinSegmentLength should match")
+	s.Require().Equal(newAllowUnrestrictedNames, updatedParams.AllowUnrestrictedNames, "Updated AllowUnrestrictedNames should match")
+}
diff --git a/x/name/keeper/proposal_handler_test.go b/x/name/keeper/proposal_handler_test.go
@@ -28,7 +28,7 @@ type IntegrationTestSuite struct {
 func (s *IntegrationTestSuite) SetupSuite() {
 	s.app = provenance.Setup(s.T())
 	s.ctx = s.app.BaseApp.NewContext(false)
-	s.k = namekeeper.NewKeeper(s.app.AppCodec(), s.app.GetKey(nametypes.ModuleName), s.app.GetSubspace(nametypes.ModuleName))
+	s.k = namekeeper.NewKeeper(s.app.AppCodec(), s.app.GetKey(nametypes.ModuleName))
 	s.accountAddr = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address())
 	s.k.SetNameRecord(s.ctx, "test.root", s.accountAddr, false)
 }

diff --git a/x/name/keeper/query_server.go b/x/name/keeper/query_server.go
@@ -15,10 +15,7 @@ var _ types.QueryServer = Keeper{}
 // Params queries params of distribution 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
 }
 
 // Resolve returns the address a name resolves to or an error.

diff --git a/x/name/simulation/proposals_test.go b/x/name/simulation/proposals_test.go
@@ -26,7 +26,7 @@ func TestCreateRootNameProposalContents(t *testing.T) {
 	accounts := simtypes.RandomAccounts(r, 3)
 
 	// execute ProposalContents function
-	weightedProposalContent := simulation.ProposalContents(keeper.NewKeeper(app.AppCodec(), app.GetKey(types.ModuleName), app.GetSubspace(types.ModuleName)))
+	weightedProposalContent := simulation.ProposalContents(keeper.NewKeeper(app.AppCodec(), app.GetKey(types.ModuleName)))
 	require.Len(t, weightedProposalContent, 1)
 
 	w0 := weightedProposalContent[0]

diff --git a/x/name/types/keys.go b/x/name/types/keys.go
@@ -25,6 +25,8 @@ var (
 	NameKeyPrefix = []byte{0x03}
 	// AddressKeyPrefix is a prefix added to keys for indexing name records by address.
 	AddressKeyPrefix = []byte{0x05}
+	// NameParamStoreKey key for marker module's params
+	NameParamStoreKey = []byte{0x06}
 )
 
 // GetNameKeyPrefix converts a name into key format.