Update committee permissions for kava-10 (#1234)

* add committee migration skeleton

* add helper methods to help modify permissions

* modify permissions in migration

* tidy up committee migrations into own file

migrate/v0_17/kava.go

@@ -19,6 +19,7 @@ import (
 	auctiontypes "github.com/kava-labs/kava/x/auction/types"
 	v017bep3 "github.com/kava-labs/kava/x/bep3/legacy/v0_17"
 	bep3types "github.com/kava-labs/kava/x/bep3/types"
+	committeetypes "github.com/kava-labs/kava/x/committee/types"
 	incentivetypes "github.com/kava-labs/kava/x/incentive/types"
 	savingstypes "github.com/kava-labs/kava/x/savings/types"
 )
@@ -102,4 +103,14 @@ func migrateAppState(appState genutiltypes.AppMap, clientCtx client.Context) {
 
 		appState[bep3types.ModuleName] = codec.MustMarshalJSON(migratedState)
 	}
+
+	// x/committee
+	if appState[committeetypes.ModuleName] != nil {
+		var genState committeetypes.GenesisState
+		codec.MustUnmarshalJSON(appState[committeetypes.ModuleName], &genState)
+
+		migratedState := migrateCommitteePermissions(genState)
+
+		appState[committeetypes.ModuleName] = codec.MustMarshalJSON(&migratedState)
+	}
 }

migrate/v0_17/kava_committee.go

@@ -0,0 +1,149 @@
+package v0_17
+
+import (
+	auctiontypes "github.com/kava-labs/kava/x/auction/types"
+	cdptypes "github.com/kava-labs/kava/x/cdp/types"
+	committeetypes "github.com/kava-labs/kava/x/committee/types"
+	hardtypes "github.com/kava-labs/kava/x/hard/types"
+)
+
+func migrateCommitteePermissions(genState committeetypes.GenesisState) committeetypes.GenesisState {
+
+	var newCommittees committeetypes.Committees
+	for _, committee := range genState.GetCommittees() {
+		switch committee.GetDescription() {
+		case "Hard Governance Committee":
+			committee = fixHardPermissions(committee)
+		case "Kava Stability Committee":
+			committee = fixStabilityPermissions(committee)
+		}
+		newCommittees = append(newCommittees, committee)
+	}
+
+	packedCommittees, err := committeetypes.PackCommittees(newCommittees)
+	if err != nil {
+		panic(err)
+	}
+	genState.Committees = packedCommittees
+	return genState
+}
+
+func fixStabilityPermissions(committee committeetypes.Committee) committeetypes.Committee {
+	permissions := committee.GetPermissions()
+
+	// get first params change permission in committee
+	var perm *committeetypes.ParamsChangePermission
+	var permIndex int
+	var found bool
+	for permIndex = range permissions {
+		p, ok := permissions[permIndex].(*committeetypes.ParamsChangePermission)
+		if ok {
+			perm = p
+			found = true
+			break
+		}
+	}
+	if !found {
+		panic("ParamsChangePermission not found")
+	}
+
+	appendSubparamRequirement(
+		perm.AllowedParamsChanges,
+		hardtypes.ModuleName, string(hardtypes.KeyMoneyMarkets),
+		committeetypes.SubparamRequirement{
+			Key: "denom",
+			Val: "ibc/799FDD409719A1122586A629AE8FCA17380351A51C1F47A80A1B8E7F2A491098",
+			AllowedSubparamAttrChanges: []string{
+				"borrow_limit",
+				"interest_rate_model",
+				"keeper_reward_percentage",
+				"reserve_factor",
+				"spot_market_id",
+			},
+		},
+	)
+
+	appendSubparamRequirement(
+		perm.AllowedParamsChanges,
+		cdptypes.ModuleName, string(cdptypes.KeyCollateralParams),
+		committeetypes.SubparamRequirement{
+			Key: "type",
+			Val: "ust-a",
+			AllowedSubparamAttrChanges: []string{
+				"auction_size",
+				"check_collateralization_index_count",
+				"debt_limit",
+				"keeper_reward_percentage",
+				"stability_fee",
+			},
+		},
+	)
+
+	perm.AllowedParamsChanges.Delete(auctiontypes.ModuleName, "BidDuration")
+
+	perm.AllowedParamsChanges.Set(committeetypes.AllowedParamsChange{
+		Subspace: auctiontypes.ModuleName,
+		Key:      string(auctiontypes.KeyForwardBidDuration),
+	})
+	perm.AllowedParamsChanges.Set(committeetypes.AllowedParamsChange{
+		Subspace: auctiontypes.ModuleName,
+		Key:      string(auctiontypes.KeyReverseBidDuration),
+	})
+
+	// update committee
+	permissions[permIndex] = perm
+	committee.SetPermissions(permissions)
+
+	return committee
+}
+
+func fixHardPermissions(committee committeetypes.Committee) committeetypes.Committee {
+	permissions := committee.GetPermissions()
+
+	// get first params change permission in committee
+	var perm *committeetypes.ParamsChangePermission
+	var permIndex int
+	var found bool
+	for permIndex = range permissions {
+		p, ok := permissions[permIndex].(*committeetypes.ParamsChangePermission)
+		if ok {
+			perm = p
+			found = true
+			break
+		}
+	}
+	if !found {
+		panic("ParamsChangePermission not found")
+	}
+
+	appendSubparamRequirement(
+		perm.AllowedParamsChanges,
+		hardtypes.ModuleName, string(hardtypes.KeyMoneyMarkets),
+		committeetypes.SubparamRequirement{
+			Key: "denom",
+			Val: "ibc/799FDD409719A1122586A629AE8FCA17380351A51C1F47A80A1B8E7F2A491098",
+			AllowedSubparamAttrChanges: []string{
+				"borrow_limit",
+				"interest_rate_model",
+				"keeper_reward_percentage",
+				"reserve_factor",
+				"spot_market_id",
+			},
+		},
+	)
+
+	// update committee
+	permissions[permIndex] = perm
+	committee.SetPermissions(permissions)
+
+	return committee
+}
+
+func appendSubparamRequirement(allowed committeetypes.AllowedParamsChanges, subspace, key string, requirement committeetypes.SubparamRequirement) {
+	apc, found := allowed.Get(subspace, key)
+	if !found {
+		panic("AllowedParamsChange not found")
+	}
+	apc.MultiSubparamsRequirements = append(apc.MultiSubparamsRequirements, requirement)
+	allowed.Set(apc)
+}

migrate/v0_17/testdata/genesis-v16.json

@@ -1560,6 +1560,51 @@
                         "val": "busd:usd:30",
                         "allowed_subparam_attr_changes": ["active"]
                       },
+                      {
+                        "key": "market_id",
+                        "val": "kava:usd",
+                        "allowed_subparam_attr_changes": ["active"]
+                      },
+                      {
+                        "key": "market_id",
+                        "val": "kava:usd:30",
+                        "allowed_subparam_attr_changes": ["active"]
+                      },
+                      {
+                        "key": "market_id",
+                        "val": "hard:usd",
+                        "allowed_subparam_attr_changes": ["active"]
+                      },
+                      {
+                        "key": "market_id",
+                        "val": "hard:usd:30",
+                        "allowed_subparam_attr_changes": ["active"]
+                      },
+                      {
+                        "key": "market_id",
+                        "val": "usdx:usd",
+                        "allowed_subparam_attr_changes": ["active"]
+                      },
+                      {
+                        "key": "market_id",
+                        "val": "usdx:usd:30",
+                        "allowed_subparam_attr_changes": ["active"]
+                      },
+                      {
+                        "key": "market_id",
+                        "val": "usdx:usd:720",
+                        "allowed_subparam_attr_changes": ["active"]
+                      },
+                      {
+                        "key": "market_id",
+                        "val": "swp:usd",
+                        "allowed_subparam_attr_changes": ["active"]
+                      },
+                      {
+                        "key": "market_id",
+                        "val": "swp:usd:30",
+                        "allowed_subparam_attr_changes": ["active"]
+                      },
                       {
                         "key": "market_id",
                         "val": "atom:usd",
@@ -1762,7 +1807,7 @@
                   },
                   {
                     "subspace": "incentive",
-                    "key": "HardDelegatorRewardPeriods",
+                    "key": "DelegatorRewardPeriods",
                     "single_subparam_allowed_attrs": [],
                     "multi_subparams_requirements": []
                   },
@@ -1873,7 +1918,8 @@
                     ]
                   }
                 ]
-              }
+              },
+              { "@type": "/kava.committee.v1beta1.TextPermission" }
             ],
             "vote_threshold": "0.500000000000000000",
             "proposal_duration": "600s",
@@ -1906,7 +1952,7 @@
                   },
                   {
                     "subspace": "incentive",
-                    "key": "HardDelegatorRewardPeriods",
+                    "key": "DelegatorRewardPeriods",
                     "single_subparam_allowed_attrs": [],
                     "multi_subparams_requirements": []
                   },
@@ -1917,7 +1963,8 @@
                     "multi_subparams_requirements": []
                   }
                 ]
-              }
+              },
+              { "@type": "/kava.committee.v1beta1.TextPermission" }
             ],
             "vote_threshold": "0.500000000000000000",
             "proposal_duration": "600s",

migrate/v0_17/testdata/genesis-v17.json

@@ -707,12 +707,6 @@
               {
                 "@type": "/kava.committee.v1beta1.ParamsChangePermission",
                 "allowed_params_changes": [
-                  {
-                    "subspace": "auction",
-                    "key": "BidDuration",
-                    "single_subparam_allowed_attrs": [],
-                    "multi_subparams_requirements": []
-                  },
                   {
                     "subspace": "auction",
                     "key": "IncrementSurplus",
@@ -888,6 +882,17 @@
                           "keeper_reward_percentage",
                           "stability_fee"
                         ]
+                      },
+                      {
+                        "key": "type",
+                        "val": "ust-a",
+                        "allowed_subparam_attr_changes": [
+                          "auction_size",
+                          "check_collateralization_index_count",
+                          "debt_limit",
+                          "keeper_reward_percentage",
+                          "stability_fee"
+                        ]
                       }
                     ]
                   },
@@ -990,6 +995,51 @@
                         "val": "busd:usd:30",
                         "allowed_subparam_attr_changes": ["active"]
                       },
+                      {
+                        "key": "market_id",
+                        "val": "kava:usd",
+                        "allowed_subparam_attr_changes": ["active"]
+                      },
+                      {
+                        "key": "market_id",
+                        "val": "kava:usd:30",
+                        "allowed_subparam_attr_changes": ["active"]
+                      },
+                      {
+                        "key": "market_id",
+                        "val": "hard:usd",
+                        "allowed_subparam_attr_changes": ["active"]
+                      },
+                      {
+                        "key": "market_id",
+                        "val": "hard:usd:30",
+                        "allowed_subparam_attr_changes": ["active"]
+                      },
+                      {
+                        "key": "market_id",
+                        "val": "usdx:usd",
+                        "allowed_subparam_attr_changes": ["active"]
+                      },
+                      {
+                        "key": "market_id",
+                        "val": "usdx:usd:30",
+                        "allowed_subparam_attr_changes": ["active"]
+                      },
+                      {
+                        "key": "market_id",
+                        "val": "usdx:usd:720",
+                        "allowed_subparam_attr_changes": ["active"]
+                      },
+                      {
+                        "key": "market_id",
+                        "val": "swp:usd",
+                        "allowed_subparam_attr_changes": ["active"]
+                      },
+                      {
+                        "key": "market_id",
+                        "val": "swp:usd:30",
+                        "allowed_subparam_attr_changes": ["active"]
+                      },
                       {
                         "key": "market_id",
                         "val": "atom:usd",
@@ -1136,8 +1186,31 @@
                           "keeper_reward_percentage",
                           "reserve_factor"
                         ]
+                      },
+                      {
+                        "key": "denom",
+                        "val": "ibc/799FDD409719A1122586A629AE8FCA17380351A51C1F47A80A1B8E7F2A491098",
+                        "allowed_subparam_attr_changes": [
+                          "borrow_limit",
+                          "interest_rate_model",
+                          "keeper_reward_percentage",
+                          "reserve_factor",
+                          "spot_market_id"
+                        ]
                       }
                     ]
+                  },
+                  {
+                    "subspace": "auction",
+                    "key": "ForwardBidDuration",
+                    "single_subparam_allowed_attrs": [],
+                    "multi_subparams_requirements": []
+                  },
+                  {
+                    "subspace": "auction",
+                    "key": "ReverseBidDuration",
+                    "single_subparam_allowed_attrs": [],
+                    "multi_subparams_requirements": []
                   }
                 ]
               },
@@ -1192,7 +1265,7 @@
                   },
                   {
                     "subspace": "incentive",
-                    "key": "HardDelegatorRewardPeriods",
+                    "key": "DelegatorRewardPeriods",
                     "single_subparam_allowed_attrs": [],
                     "multi_subparams_requirements": []
                   },
@@ -1299,11 +1372,23 @@
                           "reserve_factor",
                           "spot_market_id"
                         ]
+                      },
+                      {
+                        "key": "denom",
+                        "val": "ibc/799FDD409719A1122586A629AE8FCA17380351A51C1F47A80A1B8E7F2A491098",
+                        "allowed_subparam_attr_changes": [
+                          "borrow_limit",
+                          "interest_rate_model",
+                          "keeper_reward_percentage",
+                          "reserve_factor",
+                          "spot_market_id"
+                        ]
                       }
                     ]
                   }
                 ]
-              }
+              },
+              { "@type": "/kava.committee.v1beta1.TextPermission" }
             ],
             "vote_threshold": "0.500000000000000000",
             "proposal_duration": "600s",
@@ -1336,7 +1421,7 @@
                   },
                   {
                     "subspace": "incentive",
-                    "key": "HardDelegatorRewardPeriods",
+                    "key": "DelegatorRewardPeriods",
                     "single_subparam_allowed_attrs": [],
                     "multi_subparams_requirements": []
                   },
@@ -1347,7 +1432,8 @@
                     "multi_subparams_requirements": []
                   }
                 ]
-              }
+              },
+              { "@type": "/kava.committee.v1beta1.TextPermission" }
             ],
             "vote_threshold": "0.500000000000000000",
             "proposal_duration": "600s",

x/committee/types/param_permissions_test.go

@@ -6,6 +6,7 @@ import (
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	paramsproposal "github.com/cosmos/cosmos-sdk/x/params/types/proposal"
+	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 	tmtime "github.com/tendermint/tendermint/types/time"
@@ -831,3 +832,274 @@ func (s *ParamsChangeTestSuite) TestParamsChangePermission_NoSubparamRequirement
 func TestParamsChangeTestSuite(t *testing.T) {
 	suite.Run(t, new(ParamsChangeTestSuite))
 }
+
+func TestAllowedParamsChanges_Get(t *testing.T) {
+
+	exampleAPCs := types.AllowedParamsChanges{
+		{
+			Subspace:                   "subspaceA",
+			Key:                        "key1",
+			SingleSubparamAllowedAttrs: []string{"attribute1"},
+		},
+		{
+			Subspace:                   "subspaceA",
+			Key:                        "key2",
+			SingleSubparamAllowedAttrs: []string{"attribute2"},
+		},
+	}
+
+	type args struct {
+		subspace, key string
+	}
+	testCases := []struct {
+		name  string
+		apcs  types.AllowedParamsChanges
+		args  args
+		found bool
+		out   types.AllowedParamsChange
+	}{
+		{
+			name: "when element exists it is found",
+			apcs: exampleAPCs,
+			args: args{
+				subspace: "subspaceA",
+				key:      "key2",
+			},
+			found: true,
+			out:   exampleAPCs[1],
+		},
+		{
+			name: "when element doesn't exist it isn't found",
+			apcs: exampleAPCs,
+			args: args{
+				subspace: "subspaceB",
+				key:      "key1",
+			},
+			found: false,
+		},
+		{
+			name: "when slice is nil, no elements are found",
+			apcs: nil,
+			args: args{
+				subspace: "",
+				key:      "",
+			},
+			found: false,
+		},
+		{
+			name: "when slice is empty, no elements are found",
+			apcs: types.AllowedParamsChanges{},
+			args: args{
+				subspace: "subspaceA",
+				key:      "key1",
+			},
+			found: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			out, found := tc.apcs.Get(tc.args.subspace, tc.args.key)
+			require.Equal(t, tc.found, found)
+			require.Equal(t, tc.out, out)
+		})
+	}
+}
+
+func TestAllowedParamsChanges_Set(t *testing.T) {
+
+	exampleAPCs := types.AllowedParamsChanges{
+		{
+			Subspace:                   "subspaceA",
+			Key:                        "key1",
+			SingleSubparamAllowedAttrs: []string{"attribute1"},
+		},
+		{
+			Subspace:                   "subspaceA",
+			Key:                        "key2",
+			SingleSubparamAllowedAttrs: []string{"attribute2"},
+		},
+	}
+
+	type args struct {
+		subspace, key string
+	}
+	testCases := []struct {
+		name string
+		apcs types.AllowedParamsChanges
+		arg  types.AllowedParamsChange
+		out  types.AllowedParamsChanges
+	}{
+		{
+			name: "when element isn't present it is added",
+			apcs: exampleAPCs,
+			arg: types.AllowedParamsChange{
+				Subspace:                   "subspaceB",
+				Key:                        "key1",
+				SingleSubparamAllowedAttrs: []string{"attribute1"},
+			},
+			out: append(exampleAPCs, types.AllowedParamsChange{
+				Subspace:                   "subspaceB",
+				Key:                        "key1",
+				SingleSubparamAllowedAttrs: []string{"attribute1"},
+			}),
+		},
+		{
+			name: "when element matches, it is overwritten",
+			apcs: exampleAPCs,
+			arg: types.AllowedParamsChange{
+				Subspace:                   "subspaceA",
+				Key:                        "key2",
+				SingleSubparamAllowedAttrs: []string{"attribute3"},
+			},
+			out: types.AllowedParamsChanges{
+				{
+					Subspace:                   "subspaceA",
+					Key:                        "key1",
+					SingleSubparamAllowedAttrs: []string{"attribute1"},
+				},
+				{
+					Subspace:                   "subspaceA",
+					Key:                        "key2",
+					SingleSubparamAllowedAttrs: []string{"attribute3"},
+				},
+			},
+		},
+		{
+			name: "when element matches, it is overwritten",
+			apcs: exampleAPCs,
+			arg: types.AllowedParamsChange{
+				Subspace:                   "subspaceA",
+				Key:                        "key2",
+				SingleSubparamAllowedAttrs: []string{"attribute3"},
+			},
+			out: types.AllowedParamsChanges{
+				{
+					Subspace:                   "subspaceA",
+					Key:                        "key1",
+					SingleSubparamAllowedAttrs: []string{"attribute1"},
+				},
+				{
+					Subspace:                   "subspaceA",
+					Key:                        "key2",
+					SingleSubparamAllowedAttrs: []string{"attribute3"},
+				},
+			},
+		},
+		{
+			name: "when slice is nil, elements are added",
+			apcs: nil,
+			arg: types.AllowedParamsChange{
+				Subspace:                   "subspaceA",
+				Key:                        "key2",
+				SingleSubparamAllowedAttrs: []string{"attribute3"},
+			},
+			out: types.AllowedParamsChanges{
+				{
+					Subspace:                   "subspaceA",
+					Key:                        "key2",
+					SingleSubparamAllowedAttrs: []string{"attribute3"},
+				},
+			},
+		},
+		{
+			name: "when slice is empty, elements are added",
+			apcs: types.AllowedParamsChanges{},
+			arg: types.AllowedParamsChange{
+				Subspace:                   "subspaceA",
+				Key:                        "key2",
+				SingleSubparamAllowedAttrs: []string{"attribute3"},
+			},
+			out: types.AllowedParamsChanges{
+				{
+					Subspace:                   "subspaceA",
+					Key:                        "key2",
+					SingleSubparamAllowedAttrs: []string{"attribute3"},
+				},
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			(&tc.apcs).Set(tc.arg)
+			require.Equal(t, tc.out, tc.apcs)
+		})
+	}
+}
+
+func TestAllowedParamsChanges_Delete(t *testing.T) {
+
+	exampleAPCs := types.AllowedParamsChanges{
+		{
+			Subspace:                   "subspaceA",
+			Key:                        "key1",
+			SingleSubparamAllowedAttrs: []string{"attribute1"},
+		},
+		{
+			Subspace:                   "subspaceA",
+			Key:                        "key2",
+			SingleSubparamAllowedAttrs: []string{"attribute2"},
+		},
+	}
+
+	type args struct {
+		subspace, key string
+	}
+	testCases := []struct {
+		name string
+		apcs types.AllowedParamsChanges
+		args args
+		out  types.AllowedParamsChanges
+	}{
+		{
+			name: "when element exists it is removed",
+			apcs: exampleAPCs,
+			args: args{
+				subspace: "subspaceA",
+				key:      "key2",
+			},
+			out: types.AllowedParamsChanges{
+				{
+					Subspace:                   "subspaceA",
+					Key:                        "key1",
+					SingleSubparamAllowedAttrs: []string{"attribute1"},
+				},
+			},
+		},
+		{
+			name: "when element doesn't exist, none are removed",
+			apcs: exampleAPCs,
+			args: args{
+				subspace: "subspaceB",
+				key:      "key1",
+			},
+			out: exampleAPCs,
+		},
+		{
+			name: "when slice is nil, nothing happens",
+			apcs: nil,
+			args: args{
+				subspace: "subspaceA",
+				key:      "key1",
+			},
+			out: nil,
+		},
+		{
+			name: "when slice is empty, nothing happens",
+			apcs: types.AllowedParamsChanges{},
+			args: args{
+				subspace: "subspaceA",
+				key:      "key1",
+			},
+			out: types.AllowedParamsChanges{},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			(&tc.apcs).Delete(tc.args.subspace, tc.args.key)
+			require.Equal(t, tc.out, tc.apcs)
+		})
+	}
+}

x/committee/types/permissions.go

@@ -112,6 +112,49 @@ func (perm ParamsChangePermission) Allows(ctx sdk.Context, pk ParamKeeper, p Pub
 
 type AllowedParamsChanges []AllowedParamsChange
 
+// Get searches the allowedParamsChange slice for the first item matching a subspace and key.
+// It returns false if not found.
+func (changes AllowedParamsChanges) Get(subspace, key string) (AllowedParamsChange, bool) {
+	for _, apc := range changes {
+		if apc.Subspace == subspace && apc.Key == key {
+			return apc, true
+		}
+	}
+	return AllowedParamsChange{}, false
+}
+
+// Set adds a new AllowedParamsChange, overwriting the first exiting item with matching subspace and key.
+func (changes *AllowedParamsChanges) Set(newChange AllowedParamsChange) {
+	for i, apc := range *changes {
+		if apc.Subspace == newChange.Subspace && apc.Key == newChange.Key {
+			(*changes)[i] = newChange
+			return
+		}
+	}
+	*changes = append(*changes, newChange)
+}
+
+// Delete removes the first AllowedParamsChange matching subspace and key.
+func (changes *AllowedParamsChanges) Delete(subspace, key string) {
+	var found bool
+	var foundAt int
+
+	for i, apc := range *changes {
+		if apc.Subspace == subspace && apc.Key == key {
+			found = true
+			foundAt = i
+			break
+		}
+	}
+	if !found {
+		return
+	}
+	*changes = append(
+		(*changes)[:foundAt],
+		(*changes)[foundAt+1:]...,
+	)
+}
+
 // filterByParamChange returns all targeted AllowedParamsChange that matches a given ParamChange's subspace and key.
 func (changes AllowedParamsChanges) filterByParamChange(paramChange paramsproposal.ParamChange) AllowedParamsChanges {
 	filtered := []AllowedParamsChange{}