feat: x/rewards param store migration (#468)

* adding new msg proto

* implementing UpdateParams msg

* adding tests

* adding module migrations forx/rewards

* adding x/rewards to upgrade handlers

* adding migratestore test

* fixing lint

* addressing pr review comments

app/app.go

@@ -571,6 +571,7 @@ func NewArchwayApp(
 		app.Keepers.AccountKeeper,
 		app.Keepers.BankKeeper,
 		app.getSubspace(rewardsTypes.ModuleName),
+		govModuleAddr,
 	)
 
 	// Note we set up mint keeper after the x/rewards keeper

app/upgrades/latest/upgrades.go

@@ -27,6 +27,7 @@ import (
 
 	"github.com/archway-network/archway/app/keepers"
 	"github.com/archway-network/archway/app/upgrades"
+	rewardstypes "github.com/archway-network/archway/x/rewards/types"
 )
 
 // This upgrade handler is used for all the current changes to the protocol
@@ -78,6 +79,11 @@ var Upgrade = upgrades.Upgrade{
 				// wasm
 			case wasmtypes.ModuleName:
 				keyTable = wasmtypes.ParamKeyTable() //nolint:staticcheck
+
+				// archway modules
+			case rewardstypes.ModuleName:
+				keyTable = rewardstypes.ParamKeyTable()
+
 			default:
 				continue
 			}

proto/archway/rewards/v1/tx.proto

@@ -23,6 +23,12 @@ service Msg {
   // SetFlatFee sets or updates or removes the flat fee to interact with the
   // contract Method is authorized to the contract owner.
   rpc SetFlatFee(MsgSetFlatFee) returns (MsgSetFlatFeeResponse);
+
+  // UpdateParams defines a governance operation for updating the x/rewards
+  // module parameters. The authority is defined in the keeper.
+  //
+  // Since: archway v5 && cosmos-sdk 0.47
+  rpc UpdateParams(MsgUpdateParams) returns (MsgUpdateParamsResponse);
 }
 
 // MsgSetContractMetadata is the request for Msg.SetContractMetadata.
@@ -80,4 +86,26 @@ message MsgSetFlatFee {
 }
 
 // MsgSetFlatFeeResponse is the response for Msg.SetFlatFee.
-message MsgSetFlatFeeResponse {}
+message MsgSetFlatFeeResponse {}
+
+
+// MsgUpdateParams is the Msg/UpdateParams request type.
+//
+// Since: archway v5 && cosmos-sdk 0.47
+message MsgUpdateParams {
+  option (cosmos.msg.v1.signer) = "authority";
+
+  // authority is the address that controls the module (defaults to x/gov unless overwritten).
+  string authority = 1;
+
+  // params defines the x/rewards parameters to update.
+  //
+  // NOTE: All parameters must be supplied.
+  Params params = 2 [(gogoproto.nullable) = false, (gogoproto.jsontag) = "params,omitempty"];
+}
+
+// MsgUpdateParamsResponse defines the response structure for executing a
+// MsgUpdateParams message.
+//
+// Since: archway v5 && cosmos-sdk 0.47
+message MsgUpdateParamsResponse {}

x/rewards/exported/exported.go

@@ -0,0 +1,18 @@
+package exported
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
+)
+
+type (
+	ParamSet = paramtypes.ParamSet
+
+	// Subspace defines an interface that implements the legacy x/params Subspace
+	// type.
+	//
+	// NOTE: This is used solely for migration of x/params managed parameters.
+	Subspace interface {
+		GetParamSet(ctx sdk.Context, ps ParamSet)
+	}
+)

x/rewards/keeper/genesis.go

@@ -26,7 +26,9 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
 
 // InitGenesis initializes the module genesis state.
 func (k Keeper) InitGenesis(ctx sdk.Context, state *types.GenesisState) {
-	k.SetParams(ctx, state.Params)
+	if err := k.SetParams(ctx, state.Params); err != nil {
+		panic(err)
+	}
 	k.state.ContractMetadataState(ctx).Import(state.ContractsMetadata)
 	k.state.BlockRewardsState(ctx).Import(state.BlockRewards)
 	k.state.TxRewardsState(ctx).Import(state.TxRewards)

x/rewards/keeper/keeper.go

@@ -49,10 +49,11 @@ type Keeper struct {
 	trackingKeeper   TrackingKeeperExpected
 	authKeeper       AuthKeeperExpected
 	bankKeeper       BankKeeperExpected
+	authority        string // this should be the x/gov module account
 }
 
 // NewKeeper creates a new Keeper instance.
-func NewKeeper(cdc codec.Codec, key storetypes.StoreKey, contractInfoReader ContractInfoReaderExpected, trackingKeeper TrackingKeeperExpected, ak AuthKeeperExpected, bk BankKeeperExpected, ps paramTypes.Subspace) Keeper {
+func NewKeeper(cdc codec.Codec, key storetypes.StoreKey, contractInfoReader ContractInfoReaderExpected, trackingKeeper TrackingKeeperExpected, ak AuthKeeperExpected, bk BankKeeperExpected, ps paramTypes.Subspace, authority string) Keeper {
 	if !ps.HasKeyTable() {
 		ps = ps.WithKeyTable(types.ParamKeyTable())
 	}
@@ -65,6 +66,7 @@ func NewKeeper(cdc codec.Codec, key storetypes.StoreKey, contractInfoReader Cont
 		trackingKeeper:   trackingKeeper,
 		authKeeper:       ak,
 		bankKeeper:       bk,
+		authority:        authority,
 	}
 }
 
@@ -105,3 +107,8 @@ func (k Keeper) GetRewardsRecords(ctx sdk.Context, rewardsAddr sdk.AccAddress, p
 
 	return k.state.RewardsRecord(ctx).GetRewardsRecordByRewardsAddressPaginated(rewardsAddr, pageReq)
 }
+
+// GetAuthority returns the x/rewards module's authority.
+func (k Keeper) GetAuthority() string {
+	return k.authority
+}

x/rewards/keeper/migrations.go

@@ -0,0 +1,27 @@
+package keeper
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	v2 "github.com/archway-network/archway/x/rewards/migrations/v2"
+)
+
+// Migrator is a struct for handling in-place store migrations.
+type Migrator struct {
+	keeper Keeper
+}
+
+// NewMigrator returns a new Migrator.
+func NewMigrator(keeper Keeper) Migrator {
+	return Migrator{
+		keeper: keeper,
+	}
+}
+
+// Migrate1to2 migrates the x/rewards module state from the consensus
+// version 1 to version 2. Specifically, it takes the parameters that are currently stored
+// and managed by the x/params module and stores them directly into the x/rewards
+// module state.
+func (m Migrator) Migrate1to2(ctx sdk.Context) error {
+	return v2.MigrateStore(ctx, m.keeper.state.key, m.keeper.paramStore, m.keeper.cdc)
+}

x/rewards/keeper/msg_server.go

@@ -3,6 +3,7 @@ package keeper
 import (
 	"context"
 
+	errorsmod "cosmossdk.io/errors"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -112,3 +113,33 @@ func (s MsgServer) SetFlatFee(c context.Context, request *types.MsgSetFlatFee) (
 
 	return &types.MsgSetFlatFeeResponse{}, nil
 }
+
+// UpdateParams implements types.MsgServer.
+func (s MsgServer) UpdateParams(c context.Context, request *types.MsgUpdateParams) (*types.MsgUpdateParamsResponse, error) {
+	if request == nil {
+		return nil, status.Error(codes.InvalidArgument, "empty request")
+	}
+
+	ctx := sdk.UnwrapSDKContext(c)
+
+	_, err := sdk.AccAddressFromBech32(request.Authority)
+	if err != nil {
+		return nil, err // returning error "as is" since this should not happen due to the earlier ValidateBasic call
+	}
+
+	if request.GetAuthority() != s.keeper.GetAuthority() {
+		return nil, errorsmod.Wrap(types.ErrUnauthorized, "sender address is not authorized address to update module params")
+	}
+
+	err = request.GetParams().Validate() // need to explicitly validate as x/gov invokes this msg and it does not validate
+	if err != nil {
+		return nil, err
+	}
+
+	err = s.keeper.SetParams(ctx, request.GetParams())
+	if err != nil {
+		return nil, err
+	}
+
+	return &types.MsgUpdateParamsResponse{}, nil
+}

x/rewards/keeper/msg_server_test.go

@@ -8,6 +8,7 @@ import (
 
 	errorsmod "cosmossdk.io/errors"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
 
 	e2eTesting "github.com/archway-network/archway/e2e/testing"
 	"github.com/archway-network/archway/pkg/testutils"
@@ -341,3 +342,74 @@ func (s *KeeperTestSuite) TestMsgServer_SetFlatFee() {
 		})
 	}
 }
+
+func (s *KeeperTestSuite) TestMsgServer_UpdateParams() {
+	ctx, k := s.chain.GetContext(), s.chain.GetApp().Keepers.RewardsKeeper
+	account := s.chain.GetAccount(0)
+
+	server := keeper.NewMsgServer(k)
+
+	govAddress := s.chain.GetApp().Keepers.AccountKeeper.GetModuleAddress(govtypes.ModuleName)
+
+	testCases := []struct {
+		testCase    string
+		prepare     func() *rewardstypes.MsgUpdateParams
+		expectError bool
+	}{
+		{
+			testCase: "fail: invalid params",
+			prepare: func() *rewardstypes.MsgUpdateParams {
+				params := rewardstypes.DefaultParams()
+				params.InflationRewardsRatio = sdk.NewDecWithPrec(-2, 2)
+				return &rewardstypes.MsgUpdateParams{
+					Authority: govAddress.String(),
+					Params:    params,
+				}
+			},
+			expectError: true,
+		},
+		{
+			testCase: "fail: invalid authority address",
+			prepare: func() *rewardstypes.MsgUpdateParams {
+				return &rewardstypes.MsgUpdateParams{
+					Authority: "👻",
+					Params:    rewardstypes.DefaultParams(),
+				}
+			},
+			expectError: true,
+		},
+		{
+			testCase: "fail: authority address is not gov address",
+			prepare: func() *rewardstypes.MsgUpdateParams {
+				return &rewardstypes.MsgUpdateParams{
+					Authority: account.Address.String(),
+					Params:    rewardstypes.DefaultParams(),
+				}
+			},
+			expectError: true,
+		},
+		{
+			testCase: "ok: valid params with x/gov address",
+			prepare: func() *rewardstypes.MsgUpdateParams {
+				return &rewardstypes.MsgUpdateParams{
+					Authority: govAddress.String(),
+					Params:    rewardstypes.DefaultParams(),
+				}
+			},
+			expectError: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		s.Run(fmt.Sprintf("Case: %s", tc.testCase), func() {
+			req := tc.prepare()
+			res, err := server.UpdateParams(sdk.WrapSDKContext(ctx), req)
+			if tc.expectError {
+				s.Require().Error(err)
+			} else {
+				s.Require().NoError(err)
+				s.Require().Equal(&rewardstypes.MsgUpdateParamsResponse{}, res)
+			}
+		})
+	}
+}

x/rewards/keeper/params.go

@@ -8,39 +8,42 @@ import (
 
 // InflationRewardsRatio return inflation rewards params ratio.
 func (k Keeper) InflationRewardsRatio(ctx sdk.Context) (res sdk.Dec) {
-	k.paramStore.Get(ctx, types.InflationRewardsRatioParamKey, &res)
-	return
+	return k.GetParams(ctx).InflationRewardsRatio
 }
 
 // TxFeeRebateRatio return tx fee rebate rewards params ratio.
 func (k Keeper) TxFeeRebateRatio(ctx sdk.Context) (res sdk.Dec) {
-	k.paramStore.Get(ctx, types.TxFeeRebateRatioParamKey, &res)
-	return
+	return k.GetParams(ctx).TxFeeRebateRatio
 }
 
 // MaxWithdrawRecords return the maximum number of types.RewardsRecord objects used for the withdrawal operation.
 func (k Keeper) MaxWithdrawRecords(ctx sdk.Context) (res uint64) {
-	k.paramStore.Get(ctx, types.MaxWithdrawRecordsParamKey, &res)
-	return
+	return k.GetParams(ctx).MaxWithdrawRecords
 }
 
 func (k Keeper) MinimumPriceOfGas(ctx sdk.Context) sdk.DecCoin {
-	var res sdk.DecCoin
-	k.paramStore.Get(ctx, types.MinPriceOfGasParamKey, &res)
-	return res
+	return k.GetParams(ctx).MinPriceOfGas
 }
 
 // GetParams return all module parameters.
-func (k Keeper) GetParams(ctx sdk.Context) types.Params {
-	return types.NewParams(
-		k.InflationRewardsRatio(ctx),
-		k.TxFeeRebateRatio(ctx),
-		k.MaxWithdrawRecords(ctx),
-		k.MinimumPriceOfGas(ctx),
-	)
+func (k Keeper) GetParams(ctx sdk.Context) (params types.Params) {
+	store := ctx.KVStore(k.state.key)
+	bz := store.Get(types.ParamsKey)
+	if bz == nil {
+		return params
+	}
+
+	k.cdc.MustUnmarshal(bz, &params)
+	return params
 }
 
 // SetParams sets all module parameters.
-func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
-	k.paramStore.SetParamSet(ctx, &params)
+func (k Keeper) SetParams(ctx sdk.Context, params types.Params) error {
+	store := ctx.KVStore(k.state.key)
+	bz, err := k.cdc.Marshal(&params)
+	if err != nil {
+		return err
+	}
+	store.Set(types.ParamsKey, bz)
+	return nil
 }

x/rewards/migrations/v2/migrate.go

@@ -0,0 +1,33 @@
+package v2
+
+import (
+	"github.com/cosmos/cosmos-sdk/codec"
+	storetypes "github.com/cosmos/cosmos-sdk/store/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	"github.com/archway-network/archway/x/rewards/exported"
+	"github.com/archway-network/archway/x/rewards/types"
+)
+
+// MigrateStore migrates the x/rewards module state from the consensus version 1 to
+// version 2. Specifically, it takes the parameters that are currently stored
+// and managed by the x/params module and stores them directly into the x/rewards
+// module state.
+func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, legacySubspace exported.Subspace, cdc codec.BinaryCodec) error {
+	store := ctx.KVStore(storeKey)
+	var currParams types.Params
+	legacySubspace.GetParamSet(ctx, &currParams)
+
+	if err := currParams.Validate(); err != nil {
+		return err
+	}
+
+	bz, err := cdc.Marshal(&currParams)
+	if err != nil {
+		return err
+	}
+
+	store.Set(types.ParamsKey, bz)
+
+	return nil
+}