From 65faddb5aa036f2eb616939470a41bf461982520 Mon Sep 17 00:00:00 2001 From: Ruaridh Date: Tue, 3 May 2022 19:09:43 +0100 Subject: [PATCH] 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 | 11 + migrate/v0_17/kava_committee.go | 149 +++++++++++ migrate/v0_17/testdata/genesis-v16.json | 55 +++- migrate/v0_17/testdata/genesis-v17.json | 106 +++++++- x/committee/types/param_permissions_test.go | 272 ++++++++++++++++++++ x/committee/types/permissions.go | 43 ++++ 6 files changed, 622 insertions(+), 14 deletions(-) create mode 100644 migrate/v0_17/kava_committee.go diff --git a/migrate/v0_17/kava.go b/migrate/v0_17/kava.go index b0a20d19..212cf0c6 100644 --- a/migrate/v0_17/kava.go +++ b/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) + } } diff --git a/migrate/v0_17/kava_committee.go b/migrate/v0_17/kava_committee.go new file mode 100644 index 00000000..0a8f571d --- /dev/null +++ b/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) +} diff --git a/migrate/v0_17/testdata/genesis-v16.json b/migrate/v0_17/testdata/genesis-v16.json index a6a198bc..a60d6a49 100644 --- a/migrate/v0_17/testdata/genesis-v16.json +++ b/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", diff --git a/migrate/v0_17/testdata/genesis-v17.json b/migrate/v0_17/testdata/genesis-v17.json index ec093a68..0cf4bcbf 100644 --- a/migrate/v0_17/testdata/genesis-v17.json +++ b/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", diff --git a/x/committee/types/param_permissions_test.go b/x/committee/types/param_permissions_test.go index 12b7d868..0fc6dc2c 100644 --- a/x/committee/types/param_permissions_test.go +++ b/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) + }) + } +} diff --git a/x/committee/types/permissions.go b/x/committee/types/permissions.go index 90ff775d..e6a9f8f9 100644 --- a/x/committee/types/permissions.go +++ b/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{}