feat: Allow multiple param update plans in the store at one time (#300)

* Allow multiple param update plans in the store at one time

* lint

* lint

* handle error for ExecuteParamUpdate

proto/ojo/oracle/v1/tx.proto

@@ -44,9 +44,9 @@ service Msg {
   rpc GovRemoveCurrencyDeviationThresholds(MsgGovRemoveCurrencyDeviationThresholds)
       returns (MsgGovRemoveCurrencyDeviationThresholdsResponse);
 
-  // GovCancelUpdateParams cancels a plan to update the oracle parameters.
-  rpc GovCancelUpdateParams(MsgGovCancelUpdateParams)
-      returns (MsgGovCancelUpdateParamsResponse);
+  // GovCancelUpdateParamPlan cancels a plan to update the oracle parameters.
+  rpc GovCancelUpdateParamPlan(MsgGovCancelUpdateParamPlan)
+      returns (MsgGovCancelUpdateParamPlanResponse);
 }
 
 // MsgAggregateExchangeRatePrevote represents a message to submit an aggregate
@@ -234,8 +234,8 @@ message MsgGovRemoveCurrencyDeviationThresholds {
 // MsgGovRemoveCurrencyDeviationThresholdsResponse defines the Msg/GovRemoveCurrencyDeviationThresholdsResponse response type.
 message MsgGovRemoveCurrencyDeviationThresholdsResponse {}
 
-// MsgGovCancelUpdateParams defines the Msg/GovCancelUpdateParams request type.
-message MsgGovCancelUpdateParams {
+// MsgGovCancelUpdateParamPlan defines the Msg/GovCancelUpdateParamPlan request type.
+message MsgGovCancelUpdateParamPlan {
   option (gogoproto.equal)            = true;
   option (gogoproto.goproto_getters)  = false;
   option (gogoproto.goproto_stringer) = false;
@@ -247,9 +247,12 @@ message MsgGovCancelUpdateParams {
   // title of the proposal
   string title = 2;
 
-   // description of the proposal
+  // description of the proposal
   string description = 3;
+
+  // height of param update plan to cancel
+  int64 height = 4;
 }
 
-// MsgGovCancelUpdateParamsResponse defines the Msg/GovCancelUpdateParamsResponse response type.
-message MsgGovCancelUpdateParamsResponse {}
+// MsgGovCancelUpdateParamPlanResponse defines the Msg/GovCancelUpdateParamPlanResponse response type.
+message MsgGovCancelUpdateParamPlanResponse {}

x/oracle/abci.go

@@ -14,11 +14,15 @@ import (
 func EndBlocker(ctx sdk.Context, k keeper.Keeper) error {
 	defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyEndBlocker)
 
-	// Check for Oracle parameter update plans and execute it if one is
-	// found and the update plan height is set to the current block.
-	plan, found := k.GetParamUpdatePlan(ctx)
-	if found && plan.ShouldExecute(ctx) {
-		k.ExecuteParamUpdatePlan(ctx, plan)
+	// Check for Oracle parameter update plans and execute plans that are
+	// at their plan height.
+	plans := k.GetParamUpdatePlans(ctx)
+	for _, plan := range plans {
+		if plan.ShouldExecute(ctx) {
+			if err := k.ExecuteParamUpdatePlan(ctx, plan); err != nil {
+				return err
+			}
+		}
 	}
 
 	params := k.GetParams(ctx)

x/oracle/abci_test.go

@@ -432,7 +432,7 @@ func (s *IntegrationTestSuite) TestUpdateOracleParams() {
 	app, ctx := s.app, s.ctx
 	blockHeight := ctx.BlockHeight()
 
-	// Schedule param update plan for current block height
+	// Schedule param update plans for at different block heights
 	err := app.OracleKeeper.ScheduleParamUpdatePlan(
 		ctx,
 		types.ParamUpdatePlan{
@@ -444,37 +444,53 @@ func (s *IntegrationTestSuite) TestUpdateOracleParams() {
 		},
 	)
 	s.Require().NoError(err)
-	_, found := s.app.OracleKeeper.GetParamUpdatePlan(s.ctx)
-	s.Require().Equal(true, found)
+	err = app.OracleKeeper.ScheduleParamUpdatePlan(
+		ctx,
+		types.ParamUpdatePlan{
+			Keys:   []string{"VoteThreshold"},
+			Height: blockHeight + 1,
+			Changes: types.Params{
+				VoteThreshold: sdk.NewDecWithPrec(50, 2),
+			},
+		},
+	)
+	s.Require().NoError(err)
+	plans := s.app.OracleKeeper.GetParamUpdatePlans(s.ctx)
+	s.Require().Len(plans, 2)
 
-	// Check Vote Threshold was updated
+	// Check Vote Threshold was updated by first plan
 	oracle.EndBlocker(ctx, app.OracleKeeper)
 	s.Require().Equal(sdk.NewDecWithPrec(40, 2), app.OracleKeeper.VoteThreshold(ctx))
 
+	// Check Vote Threshold was updated by second plan in next block
+	ctx = ctx.WithBlockHeight(blockHeight + 1)
+	oracle.EndBlocker(ctx, app.OracleKeeper)
+	s.Require().Equal(sdk.NewDecWithPrec(50, 2), app.OracleKeeper.VoteThreshold(ctx))
+
 	// Schedule param update plan for current block height and then cancel it
 	err = app.OracleKeeper.ScheduleParamUpdatePlan(
 		ctx,
 		types.ParamUpdatePlan{
 			Keys:   []string{"VoteThreshold"},
-			Height: blockHeight,
+			Height: blockHeight + 1,
 			Changes: types.Params{
-				VoteThreshold: sdk.NewDecWithPrec(50, 2),
+				VoteThreshold: sdk.NewDecWithPrec(60, 2),
 			},
 		},
 	)
 	s.Require().NoError(err)
-	_, found = s.app.OracleKeeper.GetParamUpdatePlan(s.ctx)
-	s.Require().Equal(true, found)
+	plans = s.app.OracleKeeper.GetParamUpdatePlans(s.ctx)
+	s.Require().Len(plans, 1)
 
 	// Cancel update
-	err = app.OracleKeeper.ClearParamUpdatePlan(ctx)
+	err = app.OracleKeeper.ClearParamUpdatePlan(ctx, uint64(blockHeight+1))
 	s.Require().NoError(err)
-	_, found = s.app.OracleKeeper.GetParamUpdatePlan(s.ctx)
-	s.Require().Equal(false, found)
+	plans = s.app.OracleKeeper.GetParamUpdatePlans(s.ctx)
+	s.Require().Len(plans, 0)
 
 	// Check Vote Threshold wasn't updated
 	oracle.EndBlocker(ctx, app.OracleKeeper)
-	s.Require().Equal(sdk.NewDecWithPrec(40, 2), app.OracleKeeper.VoteThreshold(ctx))
+	s.Require().Equal(sdk.NewDecWithPrec(50, 2), app.OracleKeeper.VoteThreshold(ctx))
 }
 
 func TestOracleTestSuite(t *testing.T) {

x/oracle/keeper/msg_server.go

@@ -348,10 +348,10 @@ func (ms msgServer) GovRemoveCurrencyDeviationThresholds(
 	return &types.MsgGovRemoveCurrencyDeviationThresholdsResponse{}, nil
 }
 
-func (ms msgServer) GovCancelUpdateParams(
+func (ms msgServer) GovCancelUpdateParamPlan(
 	goCtx context.Context,
-	msg *types.MsgGovCancelUpdateParams,
-) (*types.MsgGovCancelUpdateParamsResponse, error) {
+	msg *types.MsgGovCancelUpdateParamPlan,
+) (*types.MsgGovCancelUpdateParamPlanResponse, error) {
 	if msg.Authority != ms.authority {
 		err := errors.Wrapf(
 			types.ErrNoGovAuthority,
@@ -363,10 +363,10 @@ func (ms msgServer) GovCancelUpdateParams(
 	}
 
 	ctx := sdk.UnwrapSDKContext(goCtx)
-	err := ms.ClearParamUpdatePlan(ctx)
+	err := ms.ClearParamUpdatePlan(ctx, uint64(msg.Height))
 	if err != nil {
 		return nil, err
 	}
 
-	return &types.MsgGovCancelUpdateParamsResponse{}, nil
+	return &types.MsgGovCancelUpdateParamPlanResponse{}, nil
 }

x/oracle/keeper/msg_server_test.go

@@ -1150,13 +1150,14 @@ func (s *IntegrationTestSuite) TestMsgServer_GovRemoveCurrencyDeviationThreshold
 func (s *IntegrationTestSuite) TestMsgServer_CancelUpdateGovParams() {
 	govAccAddr := s.app.GovKeeper.GetGovernanceAccount(s.ctx).GetAddress().String()
 
-	// No plan exists
-	_, err := s.msgServer.GovCancelUpdateParams(s.ctx,
-		&types.MsgGovCancelUpdateParams{
+	// No plan exists at height
+	_, err := s.msgServer.GovCancelUpdateParamPlan(s.ctx,
+		&types.MsgGovCancelUpdateParamPlan{
 			Authority: govAccAddr,
+			Height:    100,
 		},
 	)
-	s.Require().ErrorContains(err, "No param update plan found: invalid request")
+	s.Require().ErrorContains(err, "No param update plan found at block height 100: invalid request")
 
 	// Schedule plan
 	_, err = s.msgServer.GovUpdateParams(s.ctx,
@@ -1176,13 +1177,14 @@ func (s *IntegrationTestSuite) TestMsgServer_CancelUpdateGovParams() {
 	s.Require().NoError(err)
 
 	// Plan exists now
-	_, err = s.msgServer.GovCancelUpdateParams(s.ctx,
-		&types.MsgGovCancelUpdateParams{
+	_, err = s.msgServer.GovCancelUpdateParamPlan(s.ctx,
+		&types.MsgGovCancelUpdateParamPlan{
 			Authority: govAccAddr,
+			Height:    100,
 		},
 	)
 	s.Require().NoError(err)
 
-	_, found := s.app.OracleKeeper.GetParamUpdatePlan(s.ctx)
-	s.Require().Equal(false, found)
+	plan := s.app.OracleKeeper.GetParamUpdatePlans(s.ctx)
+	s.Require().Len(plan, 0)
 }

x/oracle/keeper/param_update_plan.go

@@ -18,35 +18,59 @@ func (k Keeper) ScheduleParamUpdatePlan(ctx sdk.Context, plan types.ParamUpdateP
 	store := ctx.KVStore(k.storeKey)
 
 	bz := k.cdc.MustMarshal(&plan)
-	store.Set(types.KeyParamUpdatePlan(), bz)
+	store.Set(types.KeyParamUpdatePlan(uint64(plan.Height)), bz)
 
 	return nil
 }
 
 // ClearParamUpdatePlan will clear an upcoming param update plan if one exists and return
 // an error if one isn't found.
-func (k Keeper) ClearParamUpdatePlan(ctx sdk.Context) error {
-	store := ctx.KVStore(k.storeKey)
-	bz := store.Get(types.KeyParamUpdatePlan())
-	if bz == nil {
-		return types.ErrInvalidRequest.Wrap("No param update plan found")
+func (k Keeper) ClearParamUpdatePlan(ctx sdk.Context, planHeight uint64) error {
+	if !k.haveParamUpdatePlan(ctx, planHeight) {
+		return types.ErrInvalidRequest.Wrapf("No param update plan found at block height %d", planHeight)
 	}
 
-	store.Delete(types.KeyParamUpdatePlan())
+	store := ctx.KVStore(k.storeKey)
+	store.Delete(types.KeyParamUpdatePlan(planHeight))
 	return nil
 }
 
-// GetParamUpdatePlan will return whether an upcoming param update plan exists and the plan
-// if it does.
-func (k Keeper) GetParamUpdatePlan(ctx sdk.Context) (plan types.ParamUpdatePlan, havePlan bool) {
+// haveParamUpdatePlan will return whether a param update plan exists and the specified
+// plan height.
+func (k Keeper) haveParamUpdatePlan(ctx sdk.Context, planHeight uint64) bool {
 	store := ctx.KVStore(k.storeKey)
-	bz := store.Get(types.KeyParamUpdatePlan())
-	if bz == nil {
-		return plan, false
-	}
+	bz := store.Get(types.KeyParamUpdatePlan(planHeight))
+	return bz != nil
+}
+
+// GetParamUpdatePlans returns all the param update plans in the store.
+func (k Keeper) GetParamUpdatePlans(ctx sdk.Context) (plans []types.ParamUpdatePlan) {
+	k.IterateParamUpdatePlans(ctx, func(plan types.ParamUpdatePlan) bool {
+		plans = append(plans, plan)
+		return false
+	})
+
+	return plans
+}
 
-	k.cdc.MustUnmarshal(bz, &plan)
-	return plan, true
+// IterateParamUpdatePlans iterates rate over param update plans in the store
+func (k Keeper) IterateParamUpdatePlans(
+	ctx sdk.Context,
+	handler func(types.ParamUpdatePlan) bool,
+) {
+	store := ctx.KVStore(k.storeKey)
+
+	iter := sdk.KVStorePrefixIterator(store, types.KeyPrefixParamUpdatePlan)
+	defer iter.Close()
+
+	for ; iter.Valid(); iter.Next() {
+		var paramUpdatePlan types.ParamUpdatePlan
+		k.cdc.MustUnmarshal(iter.Value(), &paramUpdatePlan)
+
+		if handler(paramUpdatePlan) {
+			break
+		}
+	}
 }
 
 // ValidateParamChanges validates parameter changes against the existing oracle parameters.
@@ -107,7 +131,7 @@ func (k Keeper) ValidateParamChanges(ctx sdk.Context, keys []string, changes typ
 
 // ExecuteParamUpdatePlan will execute a given param update plan and emit a param
 // update event.
-func (k Keeper) ExecuteParamUpdatePlan(ctx sdk.Context, plan types.ParamUpdatePlan) {
+func (k Keeper) ExecuteParamUpdatePlan(ctx sdk.Context, plan types.ParamUpdatePlan) error {
 	for _, key := range plan.Keys {
 		switch key {
 		case string(types.KeyVotePeriod):
@@ -162,4 +186,7 @@ func (k Keeper) ExecuteParamUpdatePlan(ctx sdk.Context, plan types.ParamUpdatePl
 		sdk.NewAttribute(types.AttributeKeyNotifyPriceFeeder, "1"),
 	)
 	ctx.EventManager().EmitEvent(event)
+
+	// clear plan from store after executing it
+	return k.ClearParamUpdatePlan(ctx, uint64(plan.Height))
 }

x/oracle/types/keys.go

@@ -88,9 +88,9 @@ func KeyValidatorRewardSet() (key []byte) {
 	return util.ConcatBytes(0, KeyPrefixValidatorRewardSet)
 }
 
-// KeyParamUpdatePlan
-func KeyParamUpdatePlan() (key []byte) {
-	return util.ConcatBytes(0, KeyPrefixParamUpdatePlan)
+// KeyParamUpdatePlan - stored by *planHeight*
+func KeyParamUpdatePlan(planHeight uint64) (key []byte) {
+	return util.ConcatBytes(0, KeyPrefixParamUpdatePlan, util.UintWithNullPrefix(planHeight))
 }
 
 // ParseDenomAndBlockFromKey returns the denom and block contained in the *key*

x/oracle/types/msgs.go

@@ -14,7 +14,7 @@ var (
 	_ sdk.Msg = &MsgAggregateExchangeRatePrevote{}
 	_ sdk.Msg = &MsgAggregateExchangeRateVote{}
 	_ sdk.Msg = &MsgGovUpdateParams{}
-	_ sdk.Msg = &MsgGovCancelUpdateParams{}
+	_ sdk.Msg = &MsgGovCancelUpdateParamPlan{}
 )
 
 func NewMsgAggregateExchangeRatePrevote(
@@ -213,37 +213,37 @@ func (msg MsgGovUpdateParams) ValidateBasic() error {
 	return msg.Plan.ValidateBasic()
 }
 
-// NewMsgCancelUpdateParams will creates a new MsgGovCancelUpdateParams instance
-func NewMsgCancelUpdateParams(authority, title, description string) *MsgGovCancelUpdateParams {
-	return &MsgGovCancelUpdateParams{
+// NewMsgGovCancelUpdateParamPlan will creates a new MsgGovCancelUpdateParamPlan instance
+func NewMsgGovCancelUpdateParamPlan(authority, title, description string) *MsgGovCancelUpdateParamPlan {
+	return &MsgGovCancelUpdateParamPlan{
 		Authority:   authority,
 		Title:       title,
 		Description: description,
 	}
 }
 
 // Type implements Msg interface
-func (msg MsgGovCancelUpdateParams) Type() string { return sdk.MsgTypeURL(&msg) }
+func (msg MsgGovCancelUpdateParamPlan) Type() string { return sdk.MsgTypeURL(&msg) }
 
 // String implements the Stringer interface.
-func (msg MsgGovCancelUpdateParams) String() string {
+func (msg MsgGovCancelUpdateParamPlan) String() string {
 	out, _ := yaml.Marshal(msg)
 	return string(out)
 }
 
 // GetSignBytes implements Msg
-func (msg MsgGovCancelUpdateParams) GetSignBytes() []byte {
+func (msg MsgGovCancelUpdateParamPlan) GetSignBytes() []byte {
 	bz := ModuleCdc.MustMarshalJSON(&msg)
 	return sdk.MustSortJSON(bz)
 }
 
 // GetSigners implements Msg
-func (msg MsgGovCancelUpdateParams) GetSigners() []sdk.AccAddress {
+func (msg MsgGovCancelUpdateParamPlan) GetSigners() []sdk.AccAddress {
 	return checkers.Signers(msg.Authority)
 }
 
 // ValidateBasic implements Msg
-func (msg MsgGovCancelUpdateParams) ValidateBasic() error {
+func (msg MsgGovCancelUpdateParamPlan) ValidateBasic() error {
 	return checkers.ValidateProposal(msg.Title, msg.Description, msg.Authority)
 }
 

x/oracle/types/proposal.go

@@ -6,7 +6,7 @@ import (
 
 var (
 	proposalTypeMsgGovUpdateParams                      = MsgGovUpdateParams{}.String()
-	proposalTypeMsgGovCancelUpdateParams                = MsgGovCancelUpdateParams{}.String()
+	proposalTypeMsgGovCancelUpdateParams                = MsgGovCancelUpdateParamPlan{}.String()
 	proposalTypeMsgGovAddDenoms                         = MsgGovAddDenoms{}.String()
 	proposalTypeMsgGovRemoveCurrencyPairProviders       = MsgGovRemoveCurrencyPairProviders{}.String()
 	proposalTypeMsgGovRemoveCurrencyDeviationThresholds = MsgGovRemoveCurrencyDeviationThresholds{}.String()
@@ -36,19 +36,19 @@ func (msg *MsgGovUpdateParams) ProposalRoute() string { return RouterKey }
 func (msg *MsgGovUpdateParams) ProposalType() string { return proposalTypeMsgGovUpdateParams }
 
 // Implements Proposal Interface
-var _ gov.Content = &MsgGovCancelUpdateParams{}
+var _ gov.Content = &MsgGovCancelUpdateParamPlan{}
 
 // GetTitle returns the title of a community pool spend proposal.
-func (msg *MsgGovCancelUpdateParams) GetTitle() string { return msg.Title }
+func (msg *MsgGovCancelUpdateParamPlan) GetTitle() string { return msg.Title }
 
 // GetDescription returns the description of a community pool spend proposal.
-func (msg *MsgGovCancelUpdateParams) GetDescription() string { return msg.Description }
+func (msg *MsgGovCancelUpdateParamPlan) GetDescription() string { return msg.Description }
 
 // GetDescription returns the routing key of a community pool spend proposal.
-func (msg *MsgGovCancelUpdateParams) ProposalRoute() string { return RouterKey }
+func (msg *MsgGovCancelUpdateParamPlan) ProposalRoute() string { return RouterKey }
 
 // ProposalType returns the type of a community pool spend proposal.
-func (msg *MsgGovCancelUpdateParams) ProposalType() string {
+func (msg *MsgGovCancelUpdateParamPlan) ProposalType() string {
 	return proposalTypeMsgGovCancelUpdateParams
 }