improve code organisation

x/committee/keeper/keeper.go

@@ -33,115 +33,7 @@ func NewKeeper(cdc *codec.Codec, storeKey sdk.StoreKey, router govtypes.Router)
 	}
 }
 
-func (k Keeper) SubmitProposal(ctx sdk.Context, proposer sdk.AccAddress, committeeID uint64, pubProposal types.PubProposal) (uint64, sdk.Error) {
+// ---------- Committees ----------
-	// Limit proposals to only be submitted by committee members
-	com, found := k.GetCommittee(ctx, committeeID)
-	if !found {
-		return 0, sdk.ErrInternal("committee doesn't exist")
-	}
-	if !com.HasMember(proposer) {
-		return 0, sdk.ErrInternal("only member can propose proposals")
-	}
-
-	// Check committee has permissions to enact proposal.
-	if !com.HasPermissionsFor(pubProposal) {
-		return 0, sdk.ErrInternal("committee does not have permissions to enact proposal")
-	}
-
-	// Check proposal is valid
-	// TODO what if it's not valid now but will be in the future?
-	// TODO does this need to be before permission check?
-	if err := k.ValidatePubProposal(ctx, pubProposal); err != nil {
-		return 0, err
-	}
-
-	// Get a new ID and store the proposal
-	return k.StoreNewProposal(ctx, committeeID, pubProposal)
-}
-
-func (k Keeper) AddVote(ctx sdk.Context, proposalID uint64, voter sdk.AccAddress) sdk.Error {
-	// Validate
-	pr, found := k.GetProposal(ctx, proposalID)
-	if !found {
-		return sdk.ErrInternal("proposal not found")
-	}
-	com, found := k.GetCommittee(ctx, pr.CommitteeID)
-	if !found {
-		return sdk.ErrInternal("committee disbanded")
-	}
-	if !com.HasMember(voter) {
-		return sdk.ErrInternal("not authorized to vote on proposal")
-	}
-
-	// Store vote, overwriting any prior vote
-	k.SetVote(ctx, types.Vote{ProposalID: proposalID, Voter: voter})
-
-	return nil
-}
-
-func (k Keeper) CloseOutProposal(ctx sdk.Context, proposalID uint64) sdk.Error {
-	pr, found := k.GetProposal(ctx, proposalID)
-	if !found {
-		return sdk.ErrInternal("proposal not found")
-	}
-	com, found := k.GetCommittee(ctx, pr.CommitteeID)
-	if !found {
-		return sdk.ErrInternal("committee disbanded")
-	}
-
-	var votes []types.Vote
-	k.IterateVotes(ctx, proposalID, func(vote types.Vote) bool {
-		votes = append(votes, vote)
-		return false
-	})
-	if sdk.NewDec(int64(len(votes))).GTE(types.VoteThreshold.MulInt64(int64(len(com.Members)))) { // TODO move vote counting stuff to committee methods // TODO add timeout check here - close if expired regardless of votes
-		// eneact vote
-		// The proposal handler may execute state mutating logic depending
-		// on the proposal content. If the handler fails, no state mutation
-		// is written and the error message is logged.
-		handler := k.router.GetRoute(pr.ProposalRoute())
-		cacheCtx, writeCache := ctx.CacheContext()
-		err := handler(cacheCtx, pr.PubProposal) // need to pass pubProposal as the handlers type assert it into the concrete types
-		if err == nil {
-			// write state to the underlying multi-store
-			writeCache()
-		} // if handler returns error, then still delete the proposal - it's still over, but send an event
-
-		// delete proposal and votes
-		k.DeleteProposal(ctx, proposalID)
-		for _, v := range votes {
-			k.DeleteVote(ctx, v.ProposalID, v.Voter)
-		}
-	} else {
-		return sdk.ErrInternal("note enough votes to close proposal")
-	}
-	return nil
-}
-
-func (k Keeper) ValidatePubProposal(ctx sdk.Context, pubProposal types.PubProposal) sdk.Error {
-	// TODO not sure if the basic validation is required - should be run in msg.ValidateBasic
-	if pubProposal == nil {
-		return sdk.ErrInternal("proposal is empty")
-	}
-	if err := pubProposal.ValidateBasic(); err != nil {
-		return err
-	}
-
-	if !k.router.HasRoute(pubProposal.ProposalRoute()) {
-		return sdk.ErrInternal("no handler found for proposal")
-	}
-
-	// Execute the proposal content in a cache-wrapped context to validate the
-	// actual parameter changes before the proposal proceeds through the
-	// governance process. State is not persisted.
-	cacheCtx, _ := ctx.CacheContext()
-	handler := k.router.GetRoute(pubProposal.ProposalRoute())
-	if err := handler(cacheCtx, pubProposal); err != nil {
-		return err
-	}
-
-	return nil
-}
 
 // GetCommittee gets a committee from the store.
 func (k Keeper) GetCommittee(ctx sdk.Context, committeeID uint64) (types.Committee, bool) {
@@ -168,6 +60,8 @@ func (k Keeper) DeleteCommittee(ctx sdk.Context, committeeID uint64) {
 	store.Delete(types.GetKeyFromID(committeeID))
 }
 
+// ---------- Proposals ----------
+
 // SetNextProposalID stores an ID to be used for the next created proposal
 func (k Keeper) SetNextProposalID(ctx sdk.Context, id uint64) {
 	store := ctx.KVStore(k.storeKey)
@@ -240,22 +134,7 @@ func (k Keeper) DeleteProposal(ctx sdk.Context, proposalID uint64) {
 	store.Delete(types.GetKeyFromID(proposalID))
 }
 
-// IterateVotes provides an iterator over all stored votes for a given proposal.
+// ---------- Votes ----------
-// For each vote, cb will be called. If cb returns true, the iterator will close and stop.
-func (k Keeper) IterateVotes(ctx sdk.Context, proposalID uint64, cb func(vote types.Vote) (stop bool)) {
-	// iterate over the section of the votes store that has all votes for a particular proposal
-	iterator := sdk.KVStorePrefixIterator(ctx.KVStore(k.storeKey), append(types.VoteKeyPrefix, types.GetKeyFromID(proposalID)...))
-
-	defer iterator.Close()
-	for ; iterator.Valid(); iterator.Next() {
-		var vote types.Vote
-		k.cdc.MustUnmarshalBinaryLengthPrefixed(iterator.Value(), &vote)
-
-		if cb(vote) {
-			break
-		}
-	}
-}
 
 // GetVote gets a vote from the store.
 func (k Keeper) GetVote(ctx sdk.Context, proposalID uint64, voter sdk.AccAddress) (types.Vote, bool) {
@@ -281,3 +160,20 @@ func (k Keeper) DeleteVote(ctx sdk.Context, proposalID uint64, voter sdk.AccAddr
 	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.VoteKeyPrefix)
 	store.Delete(types.GetVoteKey(proposalID, voter))
 }
+
+// IterateVotes provides an iterator over all stored votes for a given proposal.
+// For each vote, cb will be called. If cb returns true, the iterator will close and stop.
+func (k Keeper) IterateVotes(ctx sdk.Context, proposalID uint64, cb func(vote types.Vote) (stop bool)) {
+	// iterate over the section of the votes store that has all votes for a particular proposal
+	iterator := sdk.KVStorePrefixIterator(ctx.KVStore(k.storeKey), append(types.VoteKeyPrefix, types.GetKeyFromID(proposalID)...))
+
+	defer iterator.Close()
+	for ; iterator.Valid(); iterator.Next() {
+		var vote types.Vote
+		k.cdc.MustUnmarshalBinaryLengthPrefixed(iterator.Value(), &vote)
+
+		if cb(vote) {
+			break
+		}
+	}
+}

x/committee/keeper/keeper_test.go

@@ -1,13 +1,11 @@
 package keeper_test
 
 import (
-	"reflect"
 	"testing"
 
 	"github.com/stretchr/testify/suite"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	"github.com/cosmos/cosmos-sdk/x/gov"
 	abci "github.com/tendermint/tendermint/abci/types"
 
 	"github.com/kava-labs/kava/app"
@@ -32,235 +30,6 @@ func (suite *KeeperTestSuite) SetupTest() {
 	_, suite.addresses = app.GeneratePrivKeyAddressPairs(5)
 }
 
-func (suite *KeeperTestSuite) TestSubmitProposal() {
-	normalCom := types.Committee{
-		ID:          12,
-		Members:     suite.addresses[:2],
-		Permissions: []types.Permission{types.GodPermission{}},
-	}
-	noPermissionsCom := types.Committee{
-		ID:          12,
-		Members:     suite.addresses[:2],
-		Permissions: []types.Permission{},
-	}
-
-	testcases := []struct {
-		name        string
-		committee   types.Committee
-		pubProposal types.PubProposal
-		proposer    sdk.AccAddress
-		committeeID uint64
-		expectPass  bool
-	}{
-		{
-			name:        "normal",
-			committee:   normalCom,
-			pubProposal: gov.NewTextProposal("A Title", "A description of this proposal."),
-			proposer:    normalCom.Members[0],
-			committeeID: normalCom.ID,
-			expectPass:  true,
-		},
-		{
-			name:        "invalid proposal",
-			committee:   normalCom,
-			pubProposal: nil,
-			proposer:    normalCom.Members[0],
-			committeeID: normalCom.ID,
-			expectPass:  false,
-		},
-		{
-			name: "missing committee",
-			// no committee
-			pubProposal: gov.NewTextProposal("A Title", "A description of this proposal."),
-			proposer:    suite.addresses[0],
-			committeeID: 0,
-			expectPass:  false,
-		},
-		{
-			name:        "not a member",
-			committee:   normalCom,
-			pubProposal: gov.NewTextProposal("A Title", "A description of this proposal."),
-			proposer:    suite.addresses[4],
-			committeeID: normalCom.ID,
-			expectPass:  false,
-		},
-		{
-			name:        "not enough permissions",
-			committee:   noPermissionsCom,
-			pubProposal: gov.NewTextProposal("A Title", "A description of this proposal."),
-			proposer:    noPermissionsCom.Members[0],
-			committeeID: noPermissionsCom.ID,
-			expectPass:  false,
-		},
-	}
-
-	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.
-			tApp := app.NewTestApp()
-			keeper := tApp.GetCommitteeKeeper()
-			ctx := tApp.NewContext(true, abci.Header{})
-			tApp.InitializeFromGenesisStates()
-			// setup committee (if required)
-			if !(reflect.DeepEqual(tc.committee, types.Committee{})) {
-				keeper.SetCommittee(ctx, tc.committee)
-			}
-
-			id, err := keeper.SubmitProposal(ctx, tc.proposer, tc.committeeID, tc.pubProposal)
-
-			if tc.expectPass {
-				suite.NoError(err)
-				_, found := keeper.GetProposal(ctx, id)
-				suite.True(found)
-			} else {
-				suite.NotNil(err)
-			}
-		})
-	}
-}
-
-func (suite *KeeperTestSuite) TestAddVote() {
-	normalCom := types.Committee{
-		ID:          12,
-		Members:     suite.addresses[:2],
-		Permissions: []types.Permission{types.GodPermission{}},
-	}
-
-	testcases := []struct {
-		name       string
-		proposalID uint64
-		voter      sdk.AccAddress
-		expectPass bool
-	}{
-		{
-			name:       "normal",
-			proposalID: types.DefaultNextProposalID,
-			voter:      normalCom.Members[0],
-			expectPass: true,
-		},
-		{
-			name:       "nonexistent proposal",
-			proposalID: 9999999,
-			voter:      normalCom.Members[0],
-			expectPass: false,
-		},
-		{
-			name:       "voter not committee member",
-			proposalID: types.DefaultNextProposalID,
-			voter:      suite.addresses[4],
-			expectPass: false,
-		},
-	}
-
-	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.
-			tApp := app.NewTestApp()
-			keeper := tApp.GetCommitteeKeeper()
-			ctx := tApp.NewContext(true, abci.Header{})
-			tApp.InitializeFromGenesisStates()
-
-			// setup the committee and proposal
-			keeper.SetCommittee(ctx, normalCom)
-			_, err := keeper.SubmitProposal(ctx, normalCom.Members[0], normalCom.ID, gov.NewTextProposal("A Title", "A description of this proposal."))
-			suite.NoError(err)
-
-			err = keeper.AddVote(ctx, tc.proposalID, tc.voter)
-
-			if tc.expectPass {
-				suite.NoError(err)
-				_, found := keeper.GetVote(ctx, tc.proposalID, tc.voter)
-				suite.True(found)
-			} else {
-				suite.NotNil(err)
-			}
-		})
-	}
-}
-
-func (suite *KeeperTestSuite) TestCloseOutProposal() {
-	// setup test
-	suite.app.InitializeFromGenesisStates()
-	// TODO replace below with genesis state
-	normalCom := types.Committee{
-		ID:          12,
-		Members:     suite.addresses[:2],
-		Permissions: []types.Permission{types.GodPermission{}},
-	}
-	suite.keeper.SetCommittee(suite.ctx, normalCom)
-	pprop := gov.NewTextProposal("A Title", "A description of this proposal.")
-	id, err := suite.keeper.SubmitProposal(suite.ctx, normalCom.Members[0], normalCom.ID, pprop)
-	suite.NoError(err)
-	err = suite.keeper.AddVote(suite.ctx, id, normalCom.Members[0])
-	suite.NoError(err)
-	err = suite.keeper.AddVote(suite.ctx, id, normalCom.Members[1])
-	suite.NoError(err)
-
-	// run test
-	err = suite.keeper.CloseOutProposal(suite.ctx, id)
-
-	// check
-	suite.NoError(err)
-	_, found := suite.keeper.GetProposal(suite.ctx, id)
-	suite.False(found)
-	suite.keeper.IterateVotes(suite.ctx, id, func(v types.Vote) bool {
-		suite.Fail("found vote when none should exist")
-		return false
-	})
-
-}
-
-type UnregisteredProposal struct {
-	gov.TextProposal
-}
-
-func (UnregisteredProposal) ProposalRoute() string { return "unregistered" }
-func (UnregisteredProposal) ProposalType() string  { return "unregistered" }
-
-var _ types.PubProposal = UnregisteredProposal{}
-
-func (suite *KeeperTestSuite) TestValidatePubProposal() {
-
-	testcases := []struct {
-		name        string
-		pubProposal types.PubProposal
-		expectPass  bool
-	}{
-		{
-			name:        "valid",
-			pubProposal: gov.NewTextProposal("A Title", "A description of this proposal."),
-			expectPass:  true,
-		},
-		{
-			name:        "invalid (missing title)",
-			pubProposal: gov.TextProposal{Description: "A description of this proposal."},
-			expectPass:  false,
-		},
-		{
-			name:        "invalid (unregistered)",
-			pubProposal: UnregisteredProposal{gov.TextProposal{Title: "A Title", Description: "A description of this proposal."}},
-			expectPass:  false,
-		},
-		{
-			name:        "invalid (nil)",
-			pubProposal: nil,
-			expectPass:  false,
-		},
-		// TODO test case when the handler fails
-	}
-
-	for _, tc := range testcases {
-		suite.Run(tc.name, func() {
-			err := suite.keeper.ValidatePubProposal(suite.ctx, tc.pubProposal)
-			if tc.expectPass {
-				suite.NoError(err)
-			} else {
-				suite.NotNil(err)
-			}
-		})
-	}
-}
-
 func (suite *KeeperTestSuite) TestGetSetDeleteCommittee() {
 	// setup test
 	com := types.Committee{

x/committee/keeper/proposal.go

@@ -0,0 +1,117 @@
+package keeper
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	"github.com/kava-labs/kava/x/committee/types"
+)
+
+func (k Keeper) SubmitProposal(ctx sdk.Context, proposer sdk.AccAddress, committeeID uint64, pubProposal types.PubProposal) (uint64, sdk.Error) {
+	// Limit proposals to only be submitted by committee members
+	com, found := k.GetCommittee(ctx, committeeID)
+	if !found {
+		return 0, sdk.ErrInternal("committee doesn't exist")
+	}
+	if !com.HasMember(proposer) {
+		return 0, sdk.ErrInternal("only member can propose proposals")
+	}
+
+	// Check committee has permissions to enact proposal.
+	if !com.HasPermissionsFor(pubProposal) {
+		return 0, sdk.ErrInternal("committee does not have permissions to enact proposal")
+	}
+
+	// Check proposal is valid
+	// TODO what if it's not valid now but will be in the future?
+	// TODO does this need to be before permission check?
+	if err := k.ValidatePubProposal(ctx, pubProposal); err != nil {
+		return 0, err
+	}
+
+	// Get a new ID and store the proposal
+	return k.StoreNewProposal(ctx, committeeID, pubProposal)
+}
+
+func (k Keeper) AddVote(ctx sdk.Context, proposalID uint64, voter sdk.AccAddress) sdk.Error {
+	// Validate
+	pr, found := k.GetProposal(ctx, proposalID)
+	if !found {
+		return sdk.ErrInternal("proposal not found")
+	}
+	com, found := k.GetCommittee(ctx, pr.CommitteeID)
+	if !found {
+		return sdk.ErrInternal("committee disbanded")
+	}
+	if !com.HasMember(voter) {
+		return sdk.ErrInternal("not authorized to vote on proposal")
+	}
+
+	// Store vote, overwriting any prior vote
+	k.SetVote(ctx, types.Vote{ProposalID: proposalID, Voter: voter})
+
+	return nil
+}
+
+func (k Keeper) CloseOutProposal(ctx sdk.Context, proposalID uint64) sdk.Error {
+	pr, found := k.GetProposal(ctx, proposalID)
+	if !found {
+		return sdk.ErrInternal("proposal not found")
+	}
+	com, found := k.GetCommittee(ctx, pr.CommitteeID)
+	if !found {
+		return sdk.ErrInternal("committee disbanded")
+	}
+
+	var votes []types.Vote
+	k.IterateVotes(ctx, proposalID, func(vote types.Vote) bool {
+		votes = append(votes, vote)
+		return false
+	})
+	if sdk.NewDec(int64(len(votes))).GTE(types.VoteThreshold.MulInt64(int64(len(com.Members)))) { // TODO move vote counting stuff to committee methods // TODO add timeout check here - close if expired regardless of votes
+		// eneact vote
+		// The proposal handler may execute state mutating logic depending
+		// on the proposal content. If the handler fails, no state mutation
+		// is written and the error message is logged.
+		handler := k.router.GetRoute(pr.ProposalRoute())
+		cacheCtx, writeCache := ctx.CacheContext()
+		err := handler(cacheCtx, pr.PubProposal) // need to pass pubProposal as the handlers type assert it into the concrete types
+		if err == nil {
+			// write state to the underlying multi-store
+			writeCache()
+		} // if handler returns error, then still delete the proposal - it's still over, but send an event
+
+		// delete proposal and votes
+		k.DeleteProposal(ctx, proposalID)
+		for _, v := range votes {
+			k.DeleteVote(ctx, v.ProposalID, v.Voter)
+		}
+	} else {
+		return sdk.ErrInternal("note enough votes to close proposal")
+	}
+	return nil
+}
+
+func (k Keeper) ValidatePubProposal(ctx sdk.Context, pubProposal types.PubProposal) sdk.Error {
+	// TODO not sure if the basic validation is required - should be run in msg.ValidateBasic
+	if pubProposal == nil {
+		return sdk.ErrInternal("proposal is empty")
+	}
+	if err := pubProposal.ValidateBasic(); err != nil {
+		return err
+	}
+
+	if !k.router.HasRoute(pubProposal.ProposalRoute()) {
+		return sdk.ErrInternal("no handler found for proposal")
+	}
+
+	// Execute the proposal content in a cache-wrapped context to validate the
+	// actual parameter changes before the proposal proceeds through the
+	// governance process. State is not persisted.
+	cacheCtx, _ := ctx.CacheContext()
+	handler := k.router.GetRoute(pubProposal.ProposalRoute())
+	if err := handler(cacheCtx, pubProposal); err != nil {
+		return err
+	}
+
+	return nil
+}

x/committee/keeper/proposal_test.go

@@ -0,0 +1,241 @@
+package keeper_test
+
+import (
+	"reflect"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/gov"
+	abci "github.com/tendermint/tendermint/abci/types"
+
+	"github.com/kava-labs/kava/app"
+	"github.com/kava-labs/kava/x/committee/types"
+)
+
+func (suite *KeeperTestSuite) TestSubmitProposal() {
+	normalCom := types.Committee{
+		ID:          12,
+		Members:     suite.addresses[:2],
+		Permissions: []types.Permission{types.GodPermission{}},
+	}
+	noPermissionsCom := types.Committee{
+		ID:          12,
+		Members:     suite.addresses[:2],
+		Permissions: []types.Permission{},
+	}
+
+	testcases := []struct {
+		name        string
+		committee   types.Committee
+		pubProposal types.PubProposal
+		proposer    sdk.AccAddress
+		committeeID uint64
+		expectPass  bool
+	}{
+		{
+			name:        "normal",
+			committee:   normalCom,
+			pubProposal: gov.NewTextProposal("A Title", "A description of this proposal."),
+			proposer:    normalCom.Members[0],
+			committeeID: normalCom.ID,
+			expectPass:  true,
+		},
+		{
+			name:        "invalid proposal",
+			committee:   normalCom,
+			pubProposal: nil,
+			proposer:    normalCom.Members[0],
+			committeeID: normalCom.ID,
+			expectPass:  false,
+		},
+		{
+			name: "missing committee",
+			// no committee
+			pubProposal: gov.NewTextProposal("A Title", "A description of this proposal."),
+			proposer:    suite.addresses[0],
+			committeeID: 0,
+			expectPass:  false,
+		},
+		{
+			name:        "not a member",
+			committee:   normalCom,
+			pubProposal: gov.NewTextProposal("A Title", "A description of this proposal."),
+			proposer:    suite.addresses[4],
+			committeeID: normalCom.ID,
+			expectPass:  false,
+		},
+		{
+			name:        "not enough permissions",
+			committee:   noPermissionsCom,
+			pubProposal: gov.NewTextProposal("A Title", "A description of this proposal."),
+			proposer:    noPermissionsCom.Members[0],
+			committeeID: noPermissionsCom.ID,
+			expectPass:  false,
+		},
+	}
+
+	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.
+			tApp := app.NewTestApp()
+			keeper := tApp.GetCommitteeKeeper()
+			ctx := tApp.NewContext(true, abci.Header{})
+			tApp.InitializeFromGenesisStates()
+			// setup committee (if required)
+			if !(reflect.DeepEqual(tc.committee, types.Committee{})) {
+				keeper.SetCommittee(ctx, tc.committee)
+			}
+
+			id, err := keeper.SubmitProposal(ctx, tc.proposer, tc.committeeID, tc.pubProposal)
+
+			if tc.expectPass {
+				suite.NoError(err)
+				_, found := keeper.GetProposal(ctx, id)
+				suite.True(found)
+			} else {
+				suite.NotNil(err)
+			}
+		})
+	}
+}
+
+func (suite *KeeperTestSuite) TestAddVote() {
+	normalCom := types.Committee{
+		ID:          12,
+		Members:     suite.addresses[:2],
+		Permissions: []types.Permission{types.GodPermission{}},
+	}
+
+	testcases := []struct {
+		name       string
+		proposalID uint64
+		voter      sdk.AccAddress
+		expectPass bool
+	}{
+		{
+			name:       "normal",
+			proposalID: types.DefaultNextProposalID,
+			voter:      normalCom.Members[0],
+			expectPass: true,
+		},
+		{
+			name:       "nonexistent proposal",
+			proposalID: 9999999,
+			voter:      normalCom.Members[0],
+			expectPass: false,
+		},
+		{
+			name:       "voter not committee member",
+			proposalID: types.DefaultNextProposalID,
+			voter:      suite.addresses[4],
+			expectPass: false,
+		},
+	}
+
+	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.
+			tApp := app.NewTestApp()
+			keeper := tApp.GetCommitteeKeeper()
+			ctx := tApp.NewContext(true, abci.Header{})
+			tApp.InitializeFromGenesisStates()
+
+			// setup the committee and proposal
+			keeper.SetCommittee(ctx, normalCom)
+			_, err := keeper.SubmitProposal(ctx, normalCom.Members[0], normalCom.ID, gov.NewTextProposal("A Title", "A description of this proposal."))
+			suite.NoError(err)
+
+			err = keeper.AddVote(ctx, tc.proposalID, tc.voter)
+
+			if tc.expectPass {
+				suite.NoError(err)
+				_, found := keeper.GetVote(ctx, tc.proposalID, tc.voter)
+				suite.True(found)
+			} else {
+				suite.NotNil(err)
+			}
+		})
+	}
+}
+
+func (suite *KeeperTestSuite) TestCloseOutProposal() {
+	// setup test
+	suite.app.InitializeFromGenesisStates()
+	// TODO replace below with genesis state
+	normalCom := types.Committee{
+		ID:          12,
+		Members:     suite.addresses[:2],
+		Permissions: []types.Permission{types.GodPermission{}},
+	}
+	suite.keeper.SetCommittee(suite.ctx, normalCom)
+	pprop := gov.NewTextProposal("A Title", "A description of this proposal.")
+	id, err := suite.keeper.SubmitProposal(suite.ctx, normalCom.Members[0], normalCom.ID, pprop)
+	suite.NoError(err)
+	err = suite.keeper.AddVote(suite.ctx, id, normalCom.Members[0])
+	suite.NoError(err)
+	err = suite.keeper.AddVote(suite.ctx, id, normalCom.Members[1])
+	suite.NoError(err)
+
+	// run test
+	err = suite.keeper.CloseOutProposal(suite.ctx, id)
+
+	// check
+	suite.NoError(err)
+	_, found := suite.keeper.GetProposal(suite.ctx, id)
+	suite.False(found)
+	suite.keeper.IterateVotes(suite.ctx, id, func(v types.Vote) bool {
+		suite.Fail("found vote when none should exist")
+		return false
+	})
+
+}
+
+type UnregisteredProposal struct {
+	gov.TextProposal
+}
+
+func (UnregisteredProposal) ProposalRoute() string { return "unregistered" }
+func (UnregisteredProposal) ProposalType() string  { return "unregistered" }
+
+var _ types.PubProposal = UnregisteredProposal{}
+
+func (suite *KeeperTestSuite) TestValidatePubProposal() {
+
+	testcases := []struct {
+		name        string
+		pubProposal types.PubProposal
+		expectPass  bool
+	}{
+		{
+			name:        "valid",
+			pubProposal: gov.NewTextProposal("A Title", "A description of this proposal."),
+			expectPass:  true,
+		},
+		{
+			name:        "invalid (missing title)",
+			pubProposal: gov.TextProposal{Description: "A description of this proposal."},
+			expectPass:  false,
+		},
+		{
+			name:        "invalid (unregistered)",
+			pubProposal: UnregisteredProposal{gov.TextProposal{Title: "A Title", Description: "A description of this proposal."}},
+			expectPass:  false,
+		},
+		{
+			name:        "invalid (nil)",
+			pubProposal: nil,
+			expectPass:  false,
+		},
+		// TODO test case when the handler fails
+	}
+
+	for _, tc := range testcases {
+		suite.Run(tc.name, func() {
+			err := suite.keeper.ValidatePubProposal(suite.ctx, tc.pubProposal)
+			if tc.expectPass {
+				suite.NoError(err)
+			} else {
+				suite.NotNil(err)
+			}
+		})
+	}
+}

x/committee/types/keys.go

@@ -30,7 +30,6 @@ var (
 	CommitteeKeyPrefix = []byte{0x00} // prefix for keys that store committees
 	ProposalKeyPrefix  = []byte{0x01} // prefix for keys that store proposals
 	VoteKeyPrefix      = []byte{0x02} // prefix for keys that store votes
-	//AuctionByTimeKeyPrefix = []byte{0x01} // prefix for keys that are part of the auctionsByTime index
 
 	NextProposalIDKey = []byte{0x03} // key for the next proposal id
 )
@@ -44,11 +43,6 @@ func GetVoteKey(proposalID uint64, voter sdk.AccAddress) []byte {
 	return append(GetKeyFromID(proposalID), voter.Bytes()...)
 }
 
-// // GetAuctionByTimeKey returns the key for iterating auctions by time
-// func GetAuctionByTimeKey(endTime time.Time, auctionID uint64) []byte {
-// 	return append(sdk.FormatTimeBytes(endTime), Uint64ToBytes(auctionID)...)
-// }
-
 // Uint64ToBytes converts a uint64 into fixed length bytes for use in store keys.
 func uint64ToBytes(id uint64) []byte {
 	bz := make([]byte, 8)

x/committee/types/types.go

@@ -5,6 +5,8 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/gov"
 )
 
+// -------- Committees --------
+
 var VoteThreshold sdk.Dec = sdk.MustNewDecFromStr("0.75")
 
 // A Committee is a collection of addresses that are allowed to vote and enact any governance proposal that passes their permissions.
@@ -23,8 +25,9 @@ func (c Committee) HasMember(addr sdk.AccAddress) bool {
 	return false
 }
 
+// HasPermissionsFor returns whether the committee is authorized to enact a proposal.
 // As long as one permission allows the proposal then it goes through. Its the OR of all permissions.
-func (c Committee) HasPermissionsFor(proposal gov.Content) bool {
+func (c Committee) HasPermissionsFor(proposal PubProposal) bool {
 	for _, p := range c.Permissions {
 		if p.Allows(proposal) {
 			return true
@@ -35,22 +38,18 @@ func (c Committee) HasPermissionsFor(proposal gov.Content) bool {
 
 // Permission is anything with a method that validates whether a proposal is allowed by it or not.
 type Permission interface {
-	Allows(gov.Content) bool
+	Allows(PubProposal) bool
 }
 
-// GOV STUFF --------------------------
+// -------- Proposals --------
-// Should be much the same as in gov module, except Proposals are linked to a committee ID.
 
-// TODO not needed? var _ gov.Content = Proposal{}
+// 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 // TODO better name
 
 type Proposal struct {
 	PubProposal
 	ID          uint64
 	CommitteeID uint64
-	// TODO
-	// could store votes on the proposal object
 }
 
 type Vote struct {
@@ -58,7 +57,3 @@ type Vote struct {
 	Voter      sdk.AccAddress
 	// Option     byte // TODO for now don't need more than just a yes as options
 }
-
-// Genesis -------------------
-// Ok just to dump everything to json and reload - if time involved then begin blocker will take care of closing expired proposals. And it won't enact proposals because they would've been immediately enacted before the halt if they passed.
-// committee, proposals, votes