diff --git a/x/committee/client/cli/cli_test.go b/x/committee/client/cli/cli_test.go index 95e776ae..20b6c564 100644 --- a/x/committee/client/cli/cli_test.go +++ b/x/committee/client/cli/cli_test.go @@ -16,8 +16,8 @@ type CLITestSuite struct { } func (suite *CLITestSuite) SetupTest() { - ahpp := app.NewTestApp() - suite.cdc = ahpp.Codec() + tApp := app.NewTestApp() + suite.cdc = tApp.Codec() } func (suite *CLITestSuite) TestExampleCommitteeChangeProposal() { diff --git a/x/committee/handler_test.go b/x/committee/handler_test.go index 89d729fd..111702d2 100644 --- a/x/committee/handler_test.go +++ b/x/committee/handler_test.go @@ -112,6 +112,22 @@ func (suite *HandlerTestSuite) TestSubmitProposalMsg_Invalid() { }) } +func (suite *HandlerTestSuite) TestSubmitProposalMsg_Unregistered() { + msg := types.NewMsgSubmitProposal( + UnregisteredPubProposal{}, + suite.addresses[0], + 1, + ) + + res := suite.handler(suite.ctx, msg) + + suite.False(res.IsOK()) + suite.keeper.IterateProposals(suite.ctx, func(p types.Proposal) bool { + suite.Fail("proposal found when none should exist") + return true + }) +} + func (suite *HandlerTestSuite) TestMsgAddVote_ProposalPass() { previousCDPDebtThreshold := suite.app.GetCDPKeeper().GetParams(suite.ctx).DebtAuctionThreshold newDebtThreshold := previousCDPDebtThreshold.Add(i(1000000)) diff --git a/x/committee/integration_test.go b/x/committee/integration_test.go index ce5414df..a26d7300 100644 --- a/x/committee/integration_test.go +++ b/x/committee/integration_test.go @@ -18,3 +18,15 @@ func cs(coins ...sdk.Coin) sdk.Coins { return sdk.NewCoins(coins...) } func NewCommitteeGenesisState(cdc *codec.Codec, gs types.GenesisState) app.GenesisState { return app.GenesisState{types.ModuleName: cdc.MustMarshalJSON(gs)} } + +var _ types.PubProposal = UnregisteredPubProposal{} + +// UnregisteredPubProposal is a pubproposal type that is not registered on the amino codec. +type UnregisteredPubProposal struct{} + +func (UnregisteredPubProposal) GetTitle() string { return "unregistered" } +func (UnregisteredPubProposal) GetDescription() string { return "unregistered" } +func (UnregisteredPubProposal) ProposalRoute() string { return "unregistered" } +func (UnregisteredPubProposal) ProposalType() string { return "unregistered" } +func (UnregisteredPubProposal) ValidateBasic() sdk.Error { return nil } +func (UnregisteredPubProposal) String() string { return "unregistered" } diff --git a/x/committee/keeper/proposal_test.go b/x/committee/keeper/proposal_test.go index 9a963fc0..e8fddd5e 100644 --- a/x/committee/keeper/proposal_test.go +++ b/x/committee/keeper/proposal_test.go @@ -259,14 +259,14 @@ func committeeGenState(cdc *codec.Codec, committees []types.Committee, proposals return app.GenesisState{committee.ModuleName: cdc.MustMarshalJSON(gs)} } -type UnregisteredProposal struct { +type UnregisteredPubProposal struct { gov.TextProposal } -func (UnregisteredProposal) ProposalRoute() string { return "unregistered" } -func (UnregisteredProposal) ProposalType() string { return "unregistered" } +func (UnregisteredPubProposal) ProposalRoute() string { return "unregistered" } +func (UnregisteredPubProposal) ProposalType() string { return "unregistered" } -var _ types.PubProposal = UnregisteredProposal{} +var _ types.PubProposal = UnregisteredPubProposal{} func (suite *KeeperTestSuite) TestValidatePubProposal() { @@ -300,7 +300,7 @@ func (suite *KeeperTestSuite) TestValidatePubProposal() { }, { name: "invalid (unregistered)", - pubProposal: UnregisteredProposal{gov.TextProposal{Title: "A Title", Description: "A description of this proposal."}}, + pubProposal: UnregisteredPubProposal{gov.TextProposal{Title: "A Title", Description: "A description of this proposal."}}, expectErr: true, }, { diff --git a/x/committee/types/committee_test.go b/x/committee/types/committee_test.go new file mode 100644 index 00000000..1f7ec077 --- /dev/null +++ b/x/committee/types/committee_test.go @@ -0,0 +1,195 @@ +package types + +import ( + "testing" + "time" + + "github.com/cosmos/cosmos-sdk/x/gov" + "github.com/cosmos/cosmos-sdk/x/params" + "github.com/stretchr/testify/suite" +) + +var _ PubProposal = UnregisteredPubProposal{} + +type UnregisteredPubProposal struct { + gov.TextProposal +} + +func (UnregisteredPubProposal) ProposalRoute() string { return "unregistered" } +func (UnregisteredPubProposal) ProposalType() string { return "unregistered" } + +type TypesTestSuite struct { + suite.Suite +} + +func (suite *TypesTestSuite) TestCommittee_HasPermissionsFor() { + + testcases := []struct { + name string + permissions []Permission + pubProposal PubProposal + expectHasPermissions bool + }{ + { + name: "normal (single permission)", + permissions: []Permission{ParamChangePermission{ + AllowedParams: AllowedParams{ + { + Subspace: "cdp", + Key: "DebtThreshold", + Subkey: "", + }, + }}}, + pubProposal: params.NewParameterChangeProposal( + "A Title", + "A description of this proposal.", + []params.ParamChange{ + { + Subspace: "cdp", + Key: "DebtThreshold", + Subkey: "", + Value: `{"denom": "usdx", "amount": "1000000"}`, + }, + }, + ), + expectHasPermissions: true, + }, + { + name: "normal (multiple permissions)", + permissions: []Permission{ + ParamChangePermission{ + AllowedParams: AllowedParams{ + { + Subspace: "cdp", + Key: "DebtThreshold", + Subkey: "", + }, + }}, + TextPermission{}, + }, + pubProposal: gov.NewTextProposal("A Proposal Title", "A description of this proposal"), + expectHasPermissions: true, + }, + { + name: "overruling permission", + permissions: []Permission{ + ParamChangePermission{ + AllowedParams: AllowedParams{ + { + Subspace: "cdp", + Key: "DebtThreshold", + Subkey: "", + }, + }}, + GodPermission{}, + }, + pubProposal: params.NewParameterChangeProposal( + "A Title", + "A description of this proposal.", + []params.ParamChange{ + { + Subspace: "cdp", + Key: "CollateralParams", + Subkey: "", + Value: `[]`, + }, + }, + ), + expectHasPermissions: true, + }, + { + name: "no permissions", + permissions: nil, + pubProposal: params.NewParameterChangeProposal( + "A Title", + "A description of this proposal.", + []params.ParamChange{ + { + Subspace: "cdp", + Key: "CollateralParams", + Subkey: "", + Value: `[]`, + }, + }, + ), + expectHasPermissions: false, + }, + { + name: "split permissions", + // These permissions looks like they allow the param change proposal, however a proposal must pass a single permission independently of others. + permissions: []Permission{ + ParamChangePermission{ + AllowedParams: AllowedParams{ + { + Subspace: "cdp", + Key: "DebtThreshold", + Subkey: "", + }, + }}, + ParamChangePermission{ + AllowedParams: AllowedParams{ + { + Subspace: "cdp", + Key: "DebtParams", + Subkey: "", + }, + }}, + }, + pubProposal: params.NewParameterChangeProposal( + "A Title", + "A description of this proposal.", + []params.ParamChange{ + { + Subspace: "cdp", + Key: "DebtThreshold", + Subkey: "", + Value: `{"denom": "usdx", "amount": "1000000"}`, + }, + { + Subspace: "cdp", + Key: "DebtParams", + Subkey: "", + Value: `[]`, + }, + }, + ), + expectHasPermissions: false, + }, + { + name: "unregistered proposal", + permissions: []Permission{ + ParamChangePermission{ + AllowedParams: AllowedParams{ + { + Subspace: "cdp", + Key: "DebtThreshold", + Subkey: "", + }, + }}, + }, + pubProposal: UnregisteredPubProposal{gov.TextProposal{"A Title", "A description."}}, + expectHasPermissions: false, + }, + } + + for _, tc := range testcases { + suite.Run(tc.name, func() { + com := NewCommittee( + 12, + "a description of this committee", + nil, + tc.permissions, + d("0.5"), + 24*time.Hour, + ) + suite.Equal( + tc.expectHasPermissions, + com.HasPermissionsFor(tc.pubProposal), + ) + }) + } +} + +func TestTypesTestSuite(t *testing.T) { + suite.Run(t, new(TypesTestSuite)) +} diff --git a/x/committee/types/permissions.go b/x/committee/types/permissions.go index fcb1c0a4..77a66aa2 100644 --- a/x/committee/types/permissions.go +++ b/x/committee/types/permissions.go @@ -52,7 +52,7 @@ func (perm ParamChangePermission) Allows(p gov.Content) bool { func (perm ParamChangePermission) MarshalYAML() (interface{}, error) { valueToMarshal := struct { Type string `yaml:"type"` - AllowedParams AllowedParams `yaml:"allowed_params` + AllowedParams AllowedParams `yaml:"allowed_params"` }{ Type: "param_change_permission", AllowedParams: perm.AllowedParams, @@ -76,7 +76,21 @@ func (allowed AllowedParams) Contains(paramChange params.ParamChange) bool { return false } -// TODO add more permissions? -// - limit parameter changes to be within small ranges -// - allow community spend proposals -// - allow committee change proposals +// TextPermission allows any text governance proposal. +type TextPermission struct{} + +var _ Permission = TextPermission{} + +func (TextPermission) Allows(p gov.Content) bool { + _, ok := p.(gov.TextProposal) + return ok +} + +func (TextPermission) MarshalYAML() (interface{}, error) { + valueToMarshal := struct { + Type string `yaml:"type"` + }{ + Type: "text_permission", + } + return valueToMarshal, nil +} diff --git a/x/committee/types/permissions_test.go b/x/committee/types/permissions_test.go new file mode 100644 index 00000000..9794d10a --- /dev/null +++ b/x/committee/types/permissions_test.go @@ -0,0 +1,291 @@ +package types + +import ( + "testing" + + "github.com/cosmos/cosmos-sdk/x/gov" + "github.com/cosmos/cosmos-sdk/x/params" + "github.com/stretchr/testify/suite" +) + +type PermissionsTestSuite struct { + suite.Suite + + exampleAllowedParams AllowedParams +} + +func (suite *PermissionsTestSuite) SetupTest() { + suite.exampleAllowedParams = AllowedParams{ + { + Subspace: "cdp", + Key: "DebtThreshold", + Subkey: "", + }, + { + Subspace: "cdp", + Key: "SurplusThreshold", + Subkey: "", + }, + { + Subspace: "cdp", + Key: "CollateralParams", + Subkey: "", + }, + { + Subspace: "auction", + Key: "BidDuration", + Subkey: "", + }, + } +} + +func (suite *PermissionsTestSuite) TestParamChangePermission_Allows() { + testcases := []struct { + name string + allowedParams AllowedParams + pubProposal PubProposal + expectAllowed bool + }{ + { + name: "normal (single param)", + allowedParams: suite.exampleAllowedParams, + pubProposal: params.NewParameterChangeProposal( + "A Title", + "A description for this proposal.", + []params.ParamChange{ + { + Subspace: "cdp", + Key: "DebtThreshold", + Subkey: "", + Value: `{"denom": "usdx", "amount": "1000000"}`, + }, + }, + ), + expectAllowed: true, + }, + { + name: "normal (multiple params)", + allowedParams: suite.exampleAllowedParams, + pubProposal: params.NewParameterChangeProposal( + "A Title", + "A description for this proposal.", + []params.ParamChange{ + { + Subspace: "cdp", + Key: "DebtThreshold", + Subkey: "", + Value: `{"denom": "usdx", "amount": "1000000"}`, + }, + { + Subspace: "cdp", + Key: "CollateralParams", + Subkey: "", + Value: `[]`, + }, + }, + ), + expectAllowed: true, + }, + { + name: "not allowed (not in list)", + allowedParams: suite.exampleAllowedParams, + pubProposal: params.NewParameterChangeProposal( + "A Title", + "A description for this proposal.", + []params.ParamChange{ + { + Subspace: "cdp", + Key: "GlobalDebtLimit", + Subkey: "", + Value: `{"denom": "usdx", "amount": "1000000000"}`, + }, + }, + ), + expectAllowed: false, + }, + { + name: "not allowed (nil allowed params)", + allowedParams: nil, + pubProposal: params.NewParameterChangeProposal( + "A Title", + "A description for this proposal.", + []params.ParamChange{ + { + Subspace: "cdp", + Key: "DebtThreshold", + Subkey: "", + Value: `[{"denom": "usdx", "amount": "1000000"}]`, + }, + }, + ), + expectAllowed: false, + }, + { + name: "not allowed (mismatched pubproposal type)", + allowedParams: suite.exampleAllowedParams, + pubProposal: gov.NewTextProposal("A Title", "A description of this proposal."), + expectAllowed: false, + }, + { + name: "not allowed (nil pubproposal)", + allowedParams: suite.exampleAllowedParams, + pubProposal: nil, + expectAllowed: false, + }, + } + + for _, tc := range testcases { + suite.Run(tc.name, func() { + permission := ParamChangePermission{ + AllowedParams: tc.allowedParams, + } + suite.Equal( + tc.expectAllowed, + permission.Allows(tc.pubProposal), + ) + }) + } +} + +func (suite *PermissionsTestSuite) TestAllowedParams_Contains() { + testcases := []struct { + name string + allowedParams AllowedParams + testParam params.ParamChange + expectContained bool + }{ + { + name: "normal", + allowedParams: suite.exampleAllowedParams, + testParam: params.ParamChange{ + Subspace: "cdp", + Key: "DebtThreshold", + Subkey: "", + Value: `{"denom": "usdx", "amount": "1000000"}`, + }, + expectContained: true, + }, + { + name: "missing subspace", + allowedParams: suite.exampleAllowedParams, + testParam: params.ParamChange{ + Subspace: "", + Key: "DebtThreshold", + Subkey: "", + Value: `{"denom": "usdx", "amount": "1000000"}`, + }, + expectContained: false, + }, + { + name: "missing key", + allowedParams: suite.exampleAllowedParams, + testParam: params.ParamChange{ + Subspace: "cdp", + Key: "", + Subkey: "", + Value: `{"denom": "usdx", "amount": "1000000"}`, + }, + expectContained: false, + }, + { + name: "empty list", + allowedParams: AllowedParams{}, + testParam: params.ParamChange{ + Subspace: "cdp", + Key: "DebtThreshold", + Subkey: "", + Value: `{"denom": "usdx", "amount": "1000000"}`, + }, + expectContained: false, + }, + { + name: "nil list", + allowedParams: nil, + testParam: params.ParamChange{ + Subspace: "cdp", + Key: "DebtThreshold", + Subkey: "", + Value: `{"denom": "usdx", "amount": "1000000"}`, + }, + expectContained: false, + }, + { + name: "no param change", + allowedParams: suite.exampleAllowedParams, + testParam: params.ParamChange{}, + expectContained: false, + }, + { + name: "empty list and no param change", + allowedParams: AllowedParams{}, + testParam: params.ParamChange{}, + expectContained: false, + }, + } + + for _, tc := range testcases { + suite.Run(tc.name, func() { + suite.Require().Equal( + tc.expectContained, + tc.allowedParams.Contains(tc.testParam), + ) + }) + } +} + +func (suite *PermissionsTestSuite) TestTextPermission_Allows() { + testcases := []struct { + name string + pubProposal PubProposal + expectAllowed bool + }{ + { + name: "normal", + pubProposal: gov.NewTextProposal( + "A Title", + "A description for this proposal.", + ), + expectAllowed: true, + }, + { + name: "not allowed (wrong pubproposal type)", + pubProposal: params.NewParameterChangeProposal( + "A Title", + "A description for this proposal.", + []params.ParamChange{ + { + Subspace: "cdp", + Key: "DebtThreshold", + Subkey: "", + Value: `{"denom": "usdx", "amount": "1000000"}`, + }, + { + Subspace: "cdp", + Key: "CollateralParams", + Subkey: "", + Value: `[]`, + }, + }, + ), + expectAllowed: false, + }, + { + name: "not allowed (nil pubproposal)", + pubProposal: nil, + expectAllowed: false, + }, + } + + for _, tc := range testcases { + suite.Run(tc.name, func() { + permission := TextPermission{} + suite.Equal( + tc.expectAllowed, + permission.Allows(tc.pubProposal), + ) + }) + } +} +func TestPermissionsTestSuite(t *testing.T) { + suite.Run(t, new(PermissionsTestSuite)) +}