address minor TODOs

x/committee/client/cli/tx.go

@@ -126,8 +126,6 @@ func GetCmdVote(cdc *codec.Codec) *cobra.Command {
 	}
 }
 
-// TODO This could replace the whole gov submit-proposal cmd. It would align how it works with how submiting proposal to committees works.
-// Requires removing and replacing the gov cmd in kvcli main.go
 // GetGovCmdSubmitProposal returns a command to submit a proposal to the gov module. It is passed to the gov module for use on its command subtree.
 func GetGovCmdSubmitProposal(cdc *codec.Codec) *cobra.Command {
 	cmd := &cobra.Command{

x/committee/client/rest/query.go

@@ -24,7 +24,9 @@ func registerQueryRoutes(cliCtx context.CLIContext, r *mux.Router) {
 	r.HandleFunc(fmt.Sprintf("/%s/proposals/{%s}/votes", types.ModuleName, RestProposalID), queryVotesOnProposalHandlerFn(cliCtx)).Methods("GET")
 }
 
-// ---------- Committees ----------
+// ------------------------------------------
+//				Committees
+// ------------------------------------------
 
 func queryCommitteesHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
@@ -85,7 +87,9 @@ func queryCommitteeHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
 	}
 }
 
-// ---------- Proposals ----------
+// ------------------------------------------
+//				Proposals
+// ------------------------------------------
 
 func queryProposalsHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
@@ -190,7 +194,9 @@ func queryProposerHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
 	}
 }
 
-// ---------- Votes ----------
+// ------------------------------------------
+//				Votes
+// ------------------------------------------
 
 func queryVotesOnProposalHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
@@ -224,27 +230,6 @@ func queryVotesOnProposalHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
 			return
 		}
 
-		// TODO should add this feature back
-		// var proposal types.Proposal
-		// if err := cliCtx.Codec.UnmarshalJSON(res, &proposal); err != nil {
-		// 	rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
-		// 	return
-		// }
-
-		// // For inactive proposals we must query the txs directly to get the votes
-		// // as they're no longer in state.
-		// propStatus := proposal.Status
-		// if !(propStatus == types.StatusVotingPeriod || propStatus == types.StatusDepositPeriod) {
-		// 	res, err = gcutils.QueryVotesByTxQuery(cliCtx, params)
-		// } else {
-		// 	res, _, err = cliCtx.QueryWithData("custom/gov/votes", bz)
-		// }
-
-		// if err != nil {
-		// 	rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error())
-		// 	return
-		// }
-
 		// Write response
 		cliCtx = cliCtx.WithHeight(height)
 		rest.PostProcessResponse(w, cliCtx, res)

x/committee/client/rest/tx.go

@@ -106,11 +106,10 @@ func postVoteHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
 	}
 }
 
-// -------- --------
+// This is a rest handler for for the gov module, that handles committee change/delete proposals.
-// TODO this could replace the POST gov/proposals endpoint, would need to overwrite routes in kvcli main, hacky
 type PostGovProposalReq struct {
 	BaseReq  rest.BaseReq     `json:"base_req" yaml:"base_req"`
-	Content  govtypes.Content `json:"content" yaml:"content"` //TODO use same PubProposal name?
+	Content  govtypes.Content `json:"content" yaml:"content"`
 	Proposer sdk.AccAddress   `json:"proposer" yaml:"proposer"`
 	Deposit  sdk.Coins        `json:"deposit" yaml:"deposit"`
 }

x/committee/keeper/keeper.go

@@ -33,7 +33,9 @@ func NewKeeper(cdc *codec.Codec, storeKey sdk.StoreKey, router govtypes.Router,
 	}
 }
 
-// ---------- Committees ----------
+// ------------------------------------------
+//				Committees
+// ------------------------------------------
 
 // GetCommittee gets a committee from the store.
 func (k Keeper) GetCommittee(ctx sdk.Context, committeeID uint64) (types.Committee, bool) {
@@ -76,7 +78,9 @@ func (k Keeper) IterateCommittees(ctx sdk.Context, cb func(committee types.Commi
 	}
 }
 
-// ---------- Proposals ----------
+// ------------------------------------------
+//				Proposals
+// ------------------------------------------
 
 // SetNextProposalID stores an ID to be used for the next created proposal
 func (k Keeper) SetNextProposalID(ctx sdk.Context, id uint64) {
@@ -167,7 +171,9 @@ func (k Keeper) IterateProposals(ctx sdk.Context, cb func(proposal types.Proposa
 	}
 }
 
-// ---------- Votes ----------
+// ------------------------------------------
+//				Votes
+// ------------------------------------------
 
 // GetVote gets a vote from the store.
 func (k Keeper) GetVote(ctx sdk.Context, proposalID uint64, voter sdk.AccAddress) (types.Vote, bool) {

x/committee/keeper/proposal_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/gov"
+	"github.com/cosmos/cosmos-sdk/x/params"
 	abci "github.com/tendermint/tendermint/abci/types"
 
 	"github.com/kava-labs/kava/app"
@@ -78,8 +79,7 @@ func (suite *KeeperTestSuite) TestSubmitProposal() {
 
 	for _, tc := range testcases {
 		suite.Run(tc.name, func() {
-			// Create local testApp because suite doesn't run the SetupTest function for subtests,
+			// Create local testApp because suite doesn't run the SetupTest function for subtests
-			// which would mean the app state is not be reset between subtests.
 			tApp := app.NewTestApp()
 			keeper := tApp.GetCommitteeKeeper()
 			ctx := tApp.NewContext(true, abci.Header{})
@@ -148,7 +148,7 @@ func (suite *KeeperTestSuite) TestAddVote() {
 
 	for _, tc := range testcases {
 		suite.Run(tc.name, func() {
-			// Create local testApp because suite doesn't run the SetupTest function for subtests, which would mean the app state is not be reset between subtests.
+			// Create local testApp because suite doesn't run the SetupTest function for subtests
 			tApp := app.NewTestApp()
 			keeper := tApp.GetCommitteeKeeper()
 			ctx := tApp.NewContext(true, abci.Header{Height: 1, Time: firstBlockTime})
@@ -217,7 +217,7 @@ func (suite *KeeperTestSuite) TestGetProposalResult() {
 
 	for _, tc := range testcases {
 		suite.Run(tc.name, func() {
-			// Create local testApp because suite doesn't run the SetupTest function for subtests, which would mean the app state is not be reset between subtests.
+			// Create local testApp because suite doesn't run the SetupTest function for subtests
 			tApp := app.NewTestApp()
 			keeper := tApp.GetCommitteeKeeper()
 			ctx := tApp.NewContext(true, abci.Header{Height: 1, Time: firstBlockTime})
@@ -294,7 +294,21 @@ func (suite *KeeperTestSuite) TestValidatePubProposal() {
 			pubProposal: nil,
 			expectPass:  false,
 		},
-		// TODO test case when the handler fails
+		{
+			name: "invalid (proposal handler fails)",
+			pubProposal: params.NewParameterChangeProposal(
+				"A Title",
+				"A description of this proposal.",
+				[]params.ParamChange{{
+					Subspace: "non existant",
+					Key:      "non existant",
+					Value:    "nonsense",
+				}},
+			),
+			expectPass: false,
+		},
+		// Some proposals can cause the proposal handler to panic.
+		// However panics will be caught when the proposal is first submitted so should avoid making it onto the chain.
 	}
 
 	for _, tc := range testcases {

x/committee/keeper/querier.go

@@ -36,7 +36,9 @@ func NewQuerier(keeper Keeper) sdk.Querier {
 	}
 }
 
-// ---------- Committees ----------
+// ------------------------------------------
+//				Committees
+// ------------------------------------------
 
 func queryCommittees(ctx sdk.Context, path []string, _ abci.RequestQuery, keeper Keeper) ([]byte, sdk.Error) {
 
@@ -72,7 +74,9 @@ func queryCommittee(ctx sdk.Context, path []string, req abci.RequestQuery, keepe
 	return bz, nil
 }
 
-// ---------- Proposals ----------
+// ------------------------------------------
+//				Proposals
+// ------------------------------------------
 
 func queryProposals(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Keeper) ([]byte, sdk.Error) {
 	var params types.QueryCommitteeParams
@@ -115,7 +119,9 @@ func queryProposal(ctx sdk.Context, path []string, req abci.RequestQuery, keeper
 	return bz, nil
 }
 
-// ---------- Votes ----------
+// ------------------------------------------
+//				Votes
+// ------------------------------------------
 
 func queryVotes(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Keeper) ([]byte, sdk.Error) {
 	var params types.QueryProposalParams
@@ -157,7 +163,9 @@ func queryVote(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Kee
 	return bz, nil
 }
 
-// ---------- Tally ----------
+// ------------------------------------------
+//				Tally
+// ------------------------------------------
 
 func queryTally(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Keeper) ([]byte, sdk.Error) {
 	var params types.QueryProposalParams

x/committee/types/msg.go

@@ -5,8 +5,8 @@ import (
 )
 
 const (
-	TypeMsgSubmitProposal = "submit_proposal" // TODO these are the same as the gov module, will there be collisions?
+	TypeMsgSubmitProposal = "commmittee_submit_proposal" // 'committee' prefix appended to avoid potential conflicts with gov msg types
-	TypeMsgVote           = "vote"
+	TypeMsgVote           = "committee_vote"
 )
 
 var _, _ sdk.Msg = MsgSubmitProposal{}, MsgVote{}
@@ -41,10 +41,6 @@ func (msg MsgSubmitProposal) ValidateBasic() sdk.Error {
 	if msg.Proposer.Empty() {
 		return sdk.ErrInvalidAddress(msg.Proposer.String())
 	}
-	// TODO
-	// if !IsValidProposalType(msg.Content.ProposalType()) {
-	// 	return ErrInvalidProposalType(DefaultCodespace, msg.Content.ProposalType())
-	// }
 
 	return msg.PubProposal.ValidateBasic()
 }

x/committee/types/permissions.go

@@ -110,6 +110,6 @@ func (perm ShutdownPermission) MarshalYAML() (interface{}, error) {
 }
 
 // TODO add more permissions?
-// - limit parameter changes to bew withing small ranges or fixed sets
+// - limit parameter changes to be within small ranges
 // - allow community spend proposals
 // - allow committee change proposals

x/committee/types/types.go

@@ -11,7 +11,9 @@ import (
 
 const MaxCommitteeDescriptionLength int = 5000
 
-// -------- Committees --------
+// ------------------------------------------
+//				Committees
+// ------------------------------------------
 
 // A Committee is a collection of addresses that are allowed to vote and enact any governance proposal that passes their permissions.
 type Committee struct {
@@ -94,10 +96,12 @@ type Permission interface {
 	Allows(PubProposal) bool
 }
 
-// -------- Proposals --------
+// ------------------------------------------
+//				Proposals
+// ------------------------------------------
 
 // PubProposal is an interface that all gov proposals defined in other modules must satisfy.
-type PubProposal = gov.Content // TODO find a better name
+type PubProposal = gov.Content
 
 type Proposal struct {
 	PubProposal `json:"pub_proposal" yaml:"pub_proposal"`
@@ -118,6 +122,10 @@ func (p Proposal) String() string {
 	return string(bz)
 }
 
+// ------------------------------------------
+//				Votes
+// ------------------------------------------
+
 type Vote struct {
 	ProposalID uint64         `json:"proposal_id" yaml:"proposal_id"`
 	Voter      sdk.AccAddress `json:"voter" yaml:"voter"`