Add update params to marker module (#1991)

* add proto def for update prarams

* add msg server impl

* add marker params update event, finish msg server endpoint, refactor params and tests

* add changelong

* add cli

* fix comments in protos

* fix lint

* remove old test cases

* update test

* fix command adding and refactor name

* fix error message

* remove testing of deprecated value

CHANGELOG.md

@@ -92,6 +92,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * Add `UpdateParams` and `Params` query rpc endpoints to modules.
   * `ibcratelimit` add `UpdateParams` endpoint and deprecate `GovUpdateParams` [#1984](https://github.com/provenance-io/provenance/pull/1984).
   * `attribute` add `UpdateParams` endpoint and cli [#1987](https://github.com/provenance-io/provenance/pull/1987).
+  * `marker` add `UpdateParams` endpoint and cli [#1991](https://github.com/provenance-io/provenance/pull/1991).
 * Update the exchange `commitment-settlement-fee-calc` cli query to utilize the keyring [#2001](https://github.com/provenance-io/provenance/pull/2001).
 
 ### Client Breaking

client/docs/swagger-ui/swagger.yaml

@@ -106932,6 +106932,11 @@ definitions:
     title: >-
       MsgUpdateForcedTransferResponse defines the Msg/UpdateForcedTransfer
       response type
+  provenance.marker.v1.MsgUpdateParamsResponse:
+    type: object
+    description: >-
+      MsgUpdateParamsResponse is a response message for the UpdateParams
+      endpoint.
   provenance.marker.v1.MsgUpdateRequiredAttributesResponse:
     type: object
     title: >-

proto/provenance/ibcratelimit/v1/tx.proto

@@ -19,7 +19,7 @@ service Msg {
     option deprecated = true;
   }
 
-  // UpdateParams is a governance proposal endpoint for updating the exchange module's params.
+  // UpdateParams is a governance proposal endpoint for updating the ibcratelimit module's params.
   rpc UpdateParams(MsgUpdateParamsRequest) returns (MsgUpdateParamsResponse);
 }
 

proto/provenance/marker/v1/marker.proto

@@ -208,3 +208,10 @@ message EventSetNetAssetValue {
   string volume = 3;
   string source = 4;
 }
+
+// EventMarkerParamsUpdated event emitted when marker params are updated.
+message EventMarkerParamsUpdated {
+  string enable_governance        = 1;
+  string unrestricted_denom_regex = 2;
+  string max_supply               = 3;
+}

proto/provenance/marker/v1/tx.proto

@@ -76,8 +76,8 @@ service Msg {
   rpc WithdrawEscrowProposal(MsgWithdrawEscrowProposalRequest) returns (MsgWithdrawEscrowProposalResponse);
   // SetDenomMetadataProposal is a governance proposal to set marker metadata
   rpc SetDenomMetadataProposal(MsgSetDenomMetadataProposalRequest) returns (MsgSetDenomMetadataProposalResponse);
-
-  // TODO: UpdateParams
+  // UpdateParams is a governance proposal endpoint for updating the marker module's params.
+  rpc UpdateParams(MsgUpdateParamsRequest) returns (MsgUpdateParamsResponse);
 }
 
 // MsgGrantAllowanceRequest validates permission to create a fee grant based on marker admin access. If
@@ -462,4 +462,18 @@ message MsgSetDenomMetadataProposalRequest {
 }
 
 // MsgSetDenomMetadataProposalResponse defines the Msg/SetDenomMetadataProposal response type
-message MsgSetDenomMetadataProposalResponse {}
+message MsgSetDenomMetadataProposalResponse {}
+
+// MsgUpdateParamsRequest is a request message for the UpdateParams endpoint.
+message MsgUpdateParamsRequest {
+  option (cosmos.msg.v1.signer) = "authority";
+
+  // authority should be the governance module account address.
+  string authority = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
+
+  // params are the new param values to set.
+  Params params = 2 [(gogoproto.nullable) = false];
+}
+
+// MsgUpdateParamsResponse is a response message for the UpdateParams endpoint.
+message MsgUpdateParamsResponse {}

x/marker/client/cli/cli_test.go

@@ -2552,3 +2552,62 @@ func (s *IntegrationTestSuite) TestUpdateParamsProposal() {
 		})
 	}
 }
+
+func (s *IntegrationTestSuite) TestUpdateMarkerParamsCmd() {
+	testCases := []struct {
+		name         string
+		cmd          *cobra.Command
+		args         []string
+		expectErr    string
+		expectedCode uint32
+	}{
+		{
+			name: "update marker params, should succeed",
+			cmd:  markercli.GetUpdateMarkerParamsCmd(),
+			args: []string{
+				"true",
+				"[a-zA-Z][a-zA-Z0-9\\-\\.]{2,83}",
+				"1000000",
+			},
+			expectedCode: 0,
+		},
+		{
+			name: "update marker params, should fail incorrect governance flag",
+			cmd:  markercli.GetUpdateMarkerParamsCmd(),
+			args: []string{
+				"invalid",
+				"[a-zA-Z][a-zA-Z0-9\\-\\.]{2,83}",
+				"1000000",
+			},
+			expectErr: `invalid enable governance flag: strconv.ParseBool: parsing "invalid": invalid syntax`,
+		},
+		{
+			name: "update marker params, should fail incorrect maxSupply",
+			cmd:  markercli.GetUpdateMarkerParamsCmd(),
+			args: []string{
+				"true",
+				"[a-zA-Z][a-zA-Z0-9\\-\\.]{2,83}",
+				"invalid",
+			},
+			expectErr: `invalid max supply: "invalid"`,
+		},
+	}
+
+	for _, tc := range testCases {
+		s.Run(tc.name, func() {
+			tc.args = append(tc.args,
+				"--title", fmt.Sprintf("title: %v", tc.name),
+				"--summary", fmt.Sprintf("summary: %v", tc.name),
+				"--deposit=1000000stake",
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, s.testnet.Validators[0].Address.String()),
+				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
+				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
+				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewInt64Coin(s.cfg.BondDenom, 10)).String()),
+			)
+			testcli.NewTxExecutor(tc.cmd, tc.args).
+				WithExpErrMsg(tc.expectErr).
+				WithExpCode(tc.expectedCode).
+				Execute(s.T(), s.testnet)
+		})
+	}
+}

x/marker/client/cli/tx.go

@@ -11,6 +11,7 @@ import (
 	"github.com/spf13/pflag"
 
 	cerrs "cosmossdk.io/errors"
+	sdkmath "cosmossdk.io/math"
 	"cosmossdk.io/x/feegrant"
 
 	"github.com/cosmos/cosmos-sdk/client"
@@ -95,6 +96,7 @@ func NewTxCmd() *cobra.Command {
 		GetCmdRemoveAdministratorProposal(),
 		GetCmdChangeStatusProposal(),
 		GetCmdWithdrawEscrowProposal(),
+		GetUpdateMarkerParamsCmd(),
 	)
 	return txCmd
 }
@@ -1551,3 +1553,49 @@ func generateOrBroadcastOptGovProp(clientCtx client.Context, flagSet *pflag.Flag
 	}
 	return tx.GenerateOrBroadcastTxCLI(clientCtx, flagSet, msg)
 }
+
+// GetUpdateMarkerParamsCmd creates a command to update the marker module's params via governance proposal.
+func GetUpdateMarkerParamsCmd() *cobra.Command {
+	cmd := &cobra.Command{
+		Use:     "update-marker-params <enable-governance> <unrestricted-denom-regex> <max-supply>",
+		Short:   "Update the marker module's params via governance proposal",
+		Long:    "Submit an update marker params via governance proposal along with an initial deposit.",
+		Args:    cobra.ExactArgs(3),
+		Example: fmt.Sprintf(`%[1]s tx marker update-marker-params true "[a-zA-Z][a-zA-Z0-9\\-\\.]{2,83}" 1000000000000 --deposit 50000nhash`, version.AppName),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			clientCtx, err := client.GetClientTxContext(cmd)
+			if err != nil {
+				return err
+			}
+
+			flagSet := cmd.Flags()
+			authority := provcli.GetAuthority(flagSet)
+
+			enableGovernance, err := strconv.ParseBool(args[0])
+			if err != nil {
+				return fmt.Errorf("invalid enable governance flag: %w", err)
+			}
+
+			unrestrictedDenomRegex := args[1]
+
+			maxSupply, ok := sdkmath.NewIntFromString(args[2])
+			if !ok {
+				return fmt.Errorf("invalid max supply: %q", args[2])
+			}
+
+			msg := types.NewMsgUpdateParamsRequest(
+				enableGovernance,
+				unrestrictedDenomRegex,
+				maxSupply,
+				authority,
+			)
+			return provcli.GenerateOrBroadcastTxCLIAsGovProp(clientCtx, flagSet, msg)
+		},
+	}
+
+	govcli.AddGovPropFlagsToCmd(cmd)
+	provcli.AddAuthorityFlagToCmd(cmd)
+	flags.AddTxFlagsToCmd(cmd)
+
+	return cmd
+}

x/marker/keeper/keeper.go

@@ -3,6 +3,7 @@ package keeper
 import (
 	"errors"
 	"fmt"
+	"strings"
 
 	"cosmossdk.io/log"
 	storetypes "cosmossdk.io/store/types"
@@ -203,6 +204,19 @@ func (k Keeper) GetAuthority() string {
 	return k.authority
 }
 
+// IsAuthority returns true if the provided address bech32 string is the authority address.
+func (k Keeper) IsAuthority(addr string) bool {
+	return strings.EqualFold(k.authority, addr)
+}
+
+// ValidateAuthority returns an error if the provided address is not the authority.
+func (k Keeper) ValidateAuthority(addr string) error {
+	if !k.IsAuthority(addr) {
+		return govtypes.ErrInvalidSigner.Wrapf("expected %q got %q", k.GetAuthority(), addr)
+	}
+	return nil
+}
+
 // IsSendDeny returns true if sender address is denied for marker
 func (k Keeper) IsSendDeny(ctx sdk.Context, markerAddr, senderAddr sdk.AccAddress) bool {
 	store := ctx.KVStore(k.storeKey)

x/marker/keeper/marker.go

@@ -346,7 +346,7 @@ func (k Keeper) IncreaseSupply(ctx sdk.Context, marker types.MarkerAccountI, coi
 
 	inCirculation := sdk.NewCoin(marker.GetDenom(), k.bankKeeper.GetSupply(ctx, marker.GetDenom()).Amount)
 	total := inCirculation.Add(coin)
-	maxAllowed := sdk.NewCoin(marker.GetDenom(), k.GetParams(ctx).MaxSupply)
+	maxAllowed := sdk.NewCoin(marker.GetDenom(), k.GetMaxSupply(ctx))
 	if total.Amount.GT(maxAllowed.Amount) {
 		return fmt.Errorf(
 			"requested supply %s exceeds maximum allowed value %s", total.Amount.String(), maxAllowed.Amount.String())

x/marker/keeper/msg_server.go

@@ -820,3 +820,19 @@ func (k msgServer) SetDenomMetadataProposal(goCtx context.Context, msg *types.Ms
 
 	return &types.MsgSetDenomMetadataProposalResponse{}, nil
 }
+
+// UpdateParams is a governance proposal endpoint for updating the marker module's params.
+func (k msgServer) UpdateParams(goCtx context.Context, msg *types.MsgUpdateParamsRequest) (*types.MsgUpdateParamsResponse, error) {
+	ctx := sdk.UnwrapSDKContext(goCtx)
+
+	if err := k.ValidateAuthority(msg.Authority); err != nil {
+		return nil, err
+	}
+
+	k.SetParams(ctx, msg.Params)
+	if err := ctx.EventManager().EmitTypedEvent(types.NewEventMarkerParamsUpdated(msg.Params.EnableGovernance, msg.Params.GetUnrestrictedDenomRegex(), msg.Params.MaxSupply)); err != nil {
+		return nil, err
+	}
+
+	return &types.MsgUpdateParamsResponse{}, nil
+}

x/marker/keeper/msg_server_test.go

@@ -1845,3 +1845,51 @@ func (s *MsgServerTestSuite) TestSetDenomMetadataProposal() {
 		})
 	}
 }
+
+func (s *MsgServerTestSuite) TestMsgUpdateParamsRequest() {
+	authority := s.app.MarkerKeeper.GetAuthority()
+
+	testCases := []struct {
+		name   string
+		msg    *types.MsgUpdateParamsRequest
+		expErr string
+	}{
+		{
+			name: "successfully update params",
+			msg: &types.MsgUpdateParamsRequest{
+				Authority: authority,
+				Params: types.NewParams(
+					true,
+					"[a-zA-Z][a-zA-Z0-9\\-\\.]{2,83}",
+					sdkmath.NewInt(1000000000000),
+				),
+			},
+		},
+		{
+			name: "fail to update params, invalid authority",
+			msg: &types.MsgUpdateParamsRequest{
+				Authority: "invalidAuthority",
+				Params: types.NewParams(
+					true,
+					"[a-zA-Z][a-zA-Z0-9\\-\\.]{2,83}",
+					sdkmath.NewInt(1000000000000),
+				),
+			},
+			expErr: `expected "cosmos10d07y265gmmuvt4z0w9aw880jnsr700j6zn9kn" got "invalidAuthority": expected gov account as only signer for proposal message`,
+		},
+	}
+
+	for _, tc := range testCases {
+		s.Run(tc.name, func() {
+			ctx := sdk.WrapSDKContext(s.ctx)
+			res, err := s.msgServer.UpdateParams(ctx, tc.msg)
+			if len(tc.expErr) > 0 {
+				s.Require().EqualError(err, tc.expErr, "UpdateParams error")
+				s.Require().Nil(res, "UpdateParams response")
+			} else {
+				s.Require().NoError(err, "UpdateParams error")
+				s.Require().NotNil(res, "UpdateParams response")
+			}
+		})
+	}
+}

x/marker/keeper/params.go

@@ -34,11 +34,6 @@ func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
 	store.Set(types.MarkerParamStoreKey, bz)
 }
 
-// Deprecated: GetMaxTotalSupply is kept for backwards compatibility.
-func (k Keeper) GetMaxTotalSupply(ctx sdk.Context) (max uint64) {
-	return k.GetParams(ctx).MaxTotalSupply
-}
-
 // GetMaxSupply returns the current parameter value for the max allowed supply.
 func (k Keeper) GetMaxSupply(ctx sdk.Context) (max sdkmath.Int) {
 	return k.GetParams(ctx).MaxSupply

x/marker/keeper/params_test.go

@@ -33,18 +33,16 @@ func TestParamTestSuite(t *testing.T) {
 
 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(uint64(0), defaultParams.MaxTotalSupply, "MaxTotalSupply is deprecated and should not be set")
 	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),
@@ -53,7 +51,6 @@ func (s *ParamTestSuite) TestGetSetParams() {
 	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/spec/07_events.md

@@ -16,6 +16,7 @@ The marker module emits the following events:
   - [Transfer](#transfer)
   - [Set Denom Metadata](#set-denom-metadata)
   - [Set Net Asset Value](#set-net-asset-value)
+  - [Marker Params Updated](#marker-params-updated)
 
 
 
@@ -216,3 +217,16 @@ Type: `provenance.marker.v1.EventSetNetAssetValue`
 | Price         | \{token amount the marker is valued at for volume\} |
 | Volume        | \{total volume/shares associated with price\}       |
 | Source        | \{source address of caller\}                        |
+
+---
+## Marker Params Updated
+
+Fires when an `EventMarkerParamsUpdated` event occurs, indicating that the marker module's parameters have been updated via a governance proposal.
+
+Type: `provenance.marker.v1.EventMarkerParamsUpdated`
+
+| Attribute Key           | Attribute Value                                     |
+|-------------------------|-----------------------------------------------------|
+| EnableGovernance        | \{value for if governance control is enabled\}      |
+| UnrestrictedDenomRegex  | \{regex for unrestricted denom validation\}         | 
+| MaxSupply               | \{value for the max allowed supply\}                |