Marker param store migration (#1934)

* migrate marker params to own store

* add param tests

* remove init of param space, add migrate method and tests, add removal todos

* add change log

CHANGELOG.md

@@ -59,6 +59,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * Update genutil for sdk 50 [#1760](https://github.com/provenance-io/provenance/issues/1760).
 * Migrate module params from param space to module store.
   *  Attribute module param migration [#1927](https://github.com/provenance-io/provenance/pull/1927)
+  *  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)
 
 ### Dependencies

app/app.go

@@ -569,10 +569,10 @@ func New(
 	}
 
 	app.MarkerKeeper = markerkeeper.NewKeeper(
-		appCodec, keys[markertypes.StoreKey], app.GetSubspace(markertypes.ModuleName),
-		app.AccountKeeper, app.BankKeeper, app.AuthzKeeper, app.FeeGrantKeeper,
-		app.AttributeKeeper, app.NameKeeper, app.TransferKeeper, markerReqAttrBypassAddrs,
-		NewGroupCheckerFunc(app.GroupKeeper),
+		appCodec, keys[markertypes.StoreKey], app.AccountKeeper,
+		app.BankKeeper, app.AuthzKeeper, app.FeeGrantKeeper,
+		app.AttributeKeeper, app.NameKeeper, app.TransferKeeper,
+		markerReqAttrBypassAddrs, NewGroupCheckerFunc(app.GroupKeeper),
 	)
 
 	app.MetadataKeeper = metadatakeeper.NewKeeper(
@@ -1361,7 +1361,6 @@ func initParamsKeeper(appCodec codec.BinaryCodec, legacyAmino *codec.LegacyAmino
 	paramsKeeper.Subspace(govtypes.ModuleName).WithKeyTable(govtypesv1.ParamKeyTable())
 	paramsKeeper.Subspace(crisistypes.ModuleName)
 
-	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)

app/upgrades.go

@@ -14,6 +14,7 @@ import (
 	"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"
+	markertypes "github.com/provenance-io/provenance/x/marker/types"
 	metadatatypes "github.com/provenance-io/provenance/x/metadata/types"
 )
 
@@ -58,6 +59,7 @@ var upgrades = map[string]appUpgrade{
 			}
 
 			migrateAttributeParams(ctx, app)
+			migrateMarkerParams(ctx, app)
 			migrateMetadataOSLocatorParams(ctx, app)
 
 			vm, err = runModuleMigrations(ctx, app, vm)
@@ -91,6 +93,7 @@ var upgrades = map[string]appUpgrade{
 			}
 
 			migrateAttributeParams(ctx, app)
+			migrateMarkerParams(ctx, app)
 			migrateMetadataOSLocatorParams(ctx, app)
 
 			vm, err = runModuleMigrations(ctx, app, vm)
@@ -305,6 +308,47 @@ func migrateAttributeParams(ctx sdk.Context, app *App) {
 	ctx.Logger().Info("Done migrating attribute params.")
 }
 
+// migrateMarkerParams migrates to new Marker Params store
+// TODO: Remove with the umber handlers.
+func migrateMarkerParams(ctx sdk.Context, app *App) {
+	ctx.Logger().Info("Migrating marker params.")
+	markerParamSpace := app.ParamsKeeper.Subspace(markertypes.ModuleName).WithKeyTable(markertypes.ParamKeyTable())
+
+	params := markertypes.DefaultParams()
+
+	// TODO: remove markertypes.ParamStoreKeyMaxTotalSupply with the umber handlers.
+	if markerParamSpace.Has(ctx, markertypes.ParamStoreKeyMaxTotalSupply) {
+		var maxTotalSupply uint64
+		markerParamSpace.Get(ctx, markertypes.ParamStoreKeyMaxTotalSupply, &maxTotalSupply)
+		params.MaxTotalSupply = maxTotalSupply
+	}
+
+	// TODO: remove markertypes.ParamStoreKeyEnableGovernance with the umber handlers.
+	if markerParamSpace.Has(ctx, markertypes.ParamStoreKeyEnableGovernance) {
+		var enableGovernance bool
+		markerParamSpace.Get(ctx, markertypes.ParamStoreKeyEnableGovernance, &enableGovernance)
+		params.EnableGovernance = enableGovernance
+	}
+
+	// TODO: remove markertypes.ParamStoreKeyUnrestrictedDenomRegex with the umber handlers.
+	if markerParamSpace.Has(ctx, markertypes.ParamStoreKeyUnrestrictedDenomRegex) {
+		var unrestrictedDenomRegex string
+		markerParamSpace.Get(ctx, markertypes.ParamStoreKeyUnrestrictedDenomRegex, &unrestrictedDenomRegex)
+		params.UnrestrictedDenomRegex = unrestrictedDenomRegex
+	}
+
+	// TODO: remove markertypes.ParamStoreKeyMaxSupply with the umber handlers.
+	if markerParamSpace.Has(ctx, markertypes.ParamStoreKeyMaxSupply) {
+		var maxSupply string
+		markerParamSpace.Get(ctx, markertypes.ParamStoreKeyMaxSupply, &maxSupply)
+		params.MaxSupply = markertypes.StringToBigInt(maxSupply)
+	}
+
+	app.MarkerKeeper.SetParams(ctx, params)
+
+	ctx.Logger().Info("Done migrating marker params.")
+}
+
 // migrateAttributeParams migrates to new Metadata Os Locator Params store
 // TODO: Remove with the umber handlers.
 func migrateMetadataOSLocatorParams(ctx sdk.Context, app *App) {
@@ -317,4 +361,5 @@ func migrateMetadataOSLocatorParams(ctx sdk.Context, app *App) {
 	}
 	app.MetadataKeeper.SetOSLocatorParams(ctx, metadatatypes.OSLocatorParams{MaxUriLength: uint32(maxValueLength)})
 	ctx.Logger().Info("Done migrating metadata os locator params.")
+
 }

app/upgrades_test.go

@@ -381,6 +381,8 @@ func (s *UpgradeTestSuite) TestUmberRC1() {
 		"INF Done migrating legacy params.",
 		"INF Migrating attribute params.",
 		"INF Done migrating attribute params.",
+		"INF Migrating marker params.",
+		"INF Done migrating marker params.",
 		"INF Migrating metadata os locator params.",
 		"INF Done migrating metadata os locator params.",
 		"INF Starting module migrations. This may take a significant amount of time to complete. Do not restart node.",
@@ -402,6 +404,8 @@ func (s *UpgradeTestSuite) TestUmber() {
 		"INF Done migrating legacy params.",
 		"INF Migrating attribute params.",
 		"INF Done migrating attribute params.",
+		"INF Migrating marker params.",
+		"INF Done migrating marker params.",
 		"INF Migrating metadata os locator params.",
 		"INF Done migrating metadata os locator params.",
 		"INF Starting module migrations. This may take a significant amount of time to complete. Do not restart node.",

x/marker/keeper/keeper.go

@@ -11,7 +11,6 @@ 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"
 	ibctypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
 
 	"github.com/provenance-io/provenance/x/marker/types"
@@ -43,9 +42,6 @@ type MarkerKeeperI interface {
 
 // Keeper defines the name module Keeper
 type Keeper struct {
-	// The reference to the Paramstore to get and set account specific params
-	paramSpace paramtypes.Subspace
-
 	// To check whether accounts exist for addresses.
 	authKeeper types.AccountKeeper
 
@@ -100,7 +96,6 @@ type Keeper struct {
 func NewKeeper(
 	cdc codec.BinaryCodec,
 	key storetypes.StoreKey,
-	paramSpace paramtypes.Subspace,
 	authKeeper types.AccountKeeper,
 	bankKeeper types.BankKeeper,
 	authzKeeper types.AuthzKeeper,
@@ -111,12 +106,7 @@ func NewKeeper(
 	reqAttrBypassAddrs []sdk.AccAddress,
 	checker types.GroupChecker,
 ) Keeper {
-	if !paramSpace.HasKeyTable() {
-		paramSpace = paramSpace.WithKeyTable(types.ParamKeyTable())
-	}
-
 	rv := Keeper{
-		paramSpace:            paramSpace,
 		authKeeper:            authKeeper,
 		authzKeeper:           authzKeeper,
 		bankKeeper:            bankKeeper,

x/marker/keeper/keeper_test.go

@@ -24,7 +24,6 @@ import (
 	distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types"
 	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
 	"github.com/cosmos/cosmos-sdk/x/group"
-	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 
 	simapp "github.com/provenance-io/provenance/app"
@@ -3029,7 +3028,7 @@ func TestBypassAddrsLocked(t *testing.T) {
 		sdk.AccAddress("addrs[4]____________"),
 	}
 
-	mk := markerkeeper.NewKeeper(nil, nil, paramtypes.NewSubspace(nil, nil, nil, nil, "test"), nil, &dummyBankKeeper{}, nil, nil, nil, nil, nil, addrs, nil)
+	mk := markerkeeper.NewKeeper(nil, nil, nil, &dummyBankKeeper{}, nil, nil, nil, nil, nil, addrs, nil)
 
 	// Now that the keeper has been created using the provided addresses, change the first byte of
 	// the first address to something else. Then, get the addresses back from the keeper and make

x/marker/keeper/params.go

@@ -5,75 +5,55 @@ import (
 	"regexp"
 
 	sdkmath "cosmossdk.io/math"
-
 	sdk "github.com/cosmos/cosmos-sdk/types"
-
 	"github.com/provenance-io/provenance/x/marker/types"
 )
 
-// GetParams returns the total set of distribution parameters.
+// GetParams returns the total set of marker parameters.
 func (k Keeper) GetParams(ctx sdk.Context) (params types.Params) {
-	return types.Params{
-		MaxTotalSupply:         k.GetMaxTotalSupply(ctx),
-		EnableGovernance:       k.GetEnableGovernance(ctx),
-		UnrestrictedDenomRegex: k.GetUnrestrictedDenomRegex(ctx),
-		MaxSupply:              k.GetMaxSupply(ctx),
+	store := ctx.KVStore(k.storeKey)
+	params = types.DefaultParams() // Assuming a method that returns default parameters
+
+	// Deserialize parameters if they are set
+	if bz := store.Get(types.ParamStoreKey); bz != nil {
+		k.cdc.MustUnmarshal(bz, &params)
 	}
+
+	return params
 }
 
-// SetParams sets the distribution parameters to the param space.
+// SetParams sets the marker 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)
+	store.Set(types.ParamStoreKey, bz)
 }
 
-// GetMaxTotalSupply is deprecated.
 // Deprecated: GetMaxTotalSupply is kept for backwards compatibility.
 func (k Keeper) GetMaxTotalSupply(ctx sdk.Context) (max uint64) {
-	max = types.DefaultMaxTotalSupply
-	if k.paramSpace.Has(ctx, types.ParamStoreKeyMaxTotalSupply) {
-		k.paramSpace.Get(ctx, types.ParamStoreKeyMaxTotalSupply, &max)
-	}
-	return
+	return k.GetParams(ctx).MaxTotalSupply
 }
 
-// GetMaxSupply return the current parameter value for the max allowed supply (or default if unset)
+// GetMaxSupply returns the current parameter value for the max allowed supply.
 func (k Keeper) GetMaxSupply(ctx sdk.Context) (max sdkmath.Int) {
-	max = types.StringToBigInt(types.DefaultMaxSupply)
-	if k.paramSpace.Has(ctx, types.ParamStoreKeyMaxSupply) {
-		k.paramSpace.Get(ctx, types.ParamStoreKeyMaxSupply, &max)
-	}
-	return
+	return k.GetParams(ctx).MaxSupply
 }
 
-// GetEnableGovernance returns the current parameter value for enabling governance control (or default if unset)
+// GetEnableGovernance returns whether governance control is enabled.
 func (k Keeper) GetEnableGovernance(ctx sdk.Context) (enabled bool) {
-	enabled = types.DefaultEnableGovernance
-	if k.paramSpace.Has(ctx, types.ParamStoreKeyEnableGovernance) {
-		k.paramSpace.Get(ctx, types.ParamStoreKeyEnableGovernance, &enabled)
-	}
-	return
+	return k.GetParams(ctx).EnableGovernance
 }
 
-// GetUnrestrictedDenomRegex returns the current parameter value for enabling governance control (or default if unset)
+// GetUnrestrictedDenomRegex returns the regex for unrestricted denom validation.
 func (k Keeper) GetUnrestrictedDenomRegex(ctx sdk.Context) (regex string) {
-	regex = types.DefaultUnrestrictedDenomRegex
-	if k.paramSpace.Has(ctx, types.ParamStoreKeyUnrestrictedDenomRegex) {
-		k.paramSpace.Get(ctx, types.ParamStoreKeyUnrestrictedDenomRegex, &regex)
-		// use the default value for empty regex expressions.
-		if len(regex) == 0 {
-			regex = types.DefaultUnrestrictedDenomRegex
-		}
-	}
-	return
+	return k.GetParams(ctx).UnrestrictedDenomRegex
 }
 
 // ValidateUnrestictedDenom checks if the supplied denom is valid based on the module params
 func (k Keeper) ValidateUnrestictedDenom(ctx sdk.Context, denom string) error {
 	// Anchors are enforced on the denom validation expression.  Similar to how the SDK does hits.
 	// https://github.com/cosmos/cosmos-sdk/blob/512b533242d34926972a8fc2f5639e8cf182f5bd/types/coin.go#L625
 	exp := k.GetUnrestrictedDenomRegex(ctx)
-
-	// Safe to use must compile here because the regular expression is validated on parameter set.
 	r := regexp.MustCompile(fmt.Sprintf(`^%s$`, exp))
 	if !r.MatchString(denom) {
 		return fmt.Errorf("invalid denom [%s] (fails unrestricted marker denom validation %s)", denom, exp)

x/marker/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/marker/types"
+	"github.com/stretchr/testify/suite"
+)
+
+type ParamTestSuite struct {
+	suite.Suite
+
+	app *app.App
+	ctx sdk.Context
+}
+
+func (s *ParamTestSuite) SetupTest() {
+	s.app = simapp.Setup(s.T())
+	s.ctx = s.app.BaseApp.NewContextLegacy(false, cmtproto.Header{Time: time.Now()})
+}
+
+func TestParamTestSuite(t *testing.T) {
+	suite.Run(t, new(ParamTestSuite))
+}
+
+func (s *ParamTestSuite) TestGetSetParams() {
+	defaultParams := s.app.MarkerKeeper.GetParams(s.ctx)
+	s.Require().Equal(types.DefaultMaxTotalSupply, defaultParams.MaxTotalSupply, "Default MaxTotalSupply should match")
+	s.Require().Equal(types.DefaultEnableGovernance, defaultParams.EnableGovernance, "Default EnableGovernance should match")
+	s.Require().Equal(types.DefaultUnrestrictedDenomRegex, defaultParams.UnrestrictedDenomRegex, "Default UnrestrictedDenomRegex should match")
+	s.Require().Equal(types.StringToBigInt(types.DefaultMaxSupply), defaultParams.MaxSupply, "Default MaxSupply should match")
+
+	newMaxTotalSupply := uint64(2000000)
+	newEnableGovernance := false
+	newUnrestrictedDenomRegex := "^xyz.*$"
+	newMaxSupply := "3000000"
+
+	newParams := types.Params{
+		MaxTotalSupply:         newMaxTotalSupply,
+		EnableGovernance:       newEnableGovernance,
+		UnrestrictedDenomRegex: newUnrestrictedDenomRegex,
+		MaxSupply:              types.StringToBigInt(newMaxSupply),
+	}
+
+	s.app.MarkerKeeper.SetParams(s.ctx, newParams)
+
+	updatedParams := s.app.MarkerKeeper.GetParams(s.ctx)
+	s.Require().Equal(newMaxTotalSupply, updatedParams.MaxTotalSupply, "Updated MaxTotalSupply should match")
+	s.Require().Equal(newEnableGovernance, updatedParams.EnableGovernance, "Updated EnableGovernance should match")
+	s.Require().Equal(newUnrestrictedDenomRegex, updatedParams.UnrestrictedDenomRegex, "Updated UnrestrictedDenomRegex should match")
+	s.Require().Equal(types.StringToBigInt(newMaxSupply), updatedParams.MaxSupply, "Updated MaxSupply should match")
+}

x/marker/keeper/proposal_handler_test.go

@@ -32,7 +32,7 @@ type IntegrationTestSuite struct {
 func (s *IntegrationTestSuite) SetupSuite() {
 	s.app = provenance.Setup(s.T())
 	s.ctx = s.app.BaseApp.NewContext(false)
-	s.k = markerkeeper.NewKeeper(s.app.AppCodec(), s.app.GetKey(markertypes.ModuleName), s.app.GetSubspace(markertypes.ModuleName), s.app.AccountKeeper, s.app.BankKeeper, s.app.AuthzKeeper, s.app.FeeGrantKeeper, s.app.AttributeKeeper, s.app.NameKeeper, s.app.TransferKeeper, nil, nil)
+	s.k = markerkeeper.NewKeeper(s.app.AppCodec(), s.app.GetKey(markertypes.ModuleName), s.app.AccountKeeper, s.app.BankKeeper, s.app.AuthzKeeper, s.app.FeeGrantKeeper, s.app.AttributeKeeper, s.app.NameKeeper, s.app.TransferKeeper, nil, nil)
 	s.accountAddr = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address())
 }
 

x/marker/keeper/query_server.go

@@ -20,10 +20,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
 }
 
 // AllMarkers returns a list of all markers on the blockchain

x/marker/simulation/proposals_test.go

@@ -32,7 +32,6 @@ func TestProposalContents(t *testing.T) {
 		keeper.NewKeeper(
 			app.AppCodec(),
 			app.GetKey(types.ModuleName),
-			app.GetSubspace(types.ModuleName),
 			app.AccountKeeper,
 			app.BankKeeper,
 			app.AuthzKeeper,

x/marker/types/key.go

@@ -38,6 +38,9 @@ var (
 
 	// NetAssetValuePrefix prefix for net asset values of markers
 	NetAssetValuePrefix = []byte{0x04}
+
+	// ParamStoreKey prefix for marker module's params
+	ParamStoreKey = []byte{0x05}
 )
 
 // MarkerAddress returns the module account address for the given denomination

x/marker/types/params.go

@@ -22,6 +22,7 @@ const (
 	DefaultUnrestrictedDenomRegex = `[a-zA-Z][a-zA-Z0-9\-\.]{2,83}`
 )
 
+// TODO: remove with the umber (v1.19.x) handlers.
 var (
 	// ParamStoreKeyEnableGovernance indicates if governance proposal management of markers is enabled
 	ParamStoreKeyEnableGovernance = []byte("EnableGovernance")
@@ -34,6 +35,7 @@ var (
 )
 
 // ParamKeyTable for marker module
+// TODO: remove with the umber (v1.19.x) handlers.
 func ParamKeyTable() paramtypes.KeyTable {
 	return paramtypes.NewKeyTable().RegisterParamSet(&Params{})
 }
@@ -54,6 +56,7 @@ func NewParams(
 }
 
 // ParamSetPairs - Implements params.ParamSet
+// TODO: remove with the umber (v1.19.x) handlers.
 func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs {
 	return paramtypes.ParamSetPairs{
 		paramtypes.NewParamSetPair(ParamStoreKeyEnableGovernance, &p.EnableGovernance, validateEnableGovernance),