add permissions tests

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() {

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))

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" }

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,
 		},
 		{

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))
+}

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
+}

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))
+}