diff --git a/x/committee/keeper/keeper.go b/x/committee/keeper/keeper.go index 55b3c875..535d3cb3 100644 --- a/x/committee/keeper/keeper.go +++ b/x/committee/keeper/keeper.go @@ -25,49 +25,51 @@ func NewKeeper(cdc *codec.Codec, storeKey sdk.StoreKey) Keeper { } } -/* TODO keeper methods - very similar to gov - -- SubmitProposal validate and store a proposal, additionally setting things like timeout -- GetProposal -- SetProposal - -- AddVote - add a vote to a particular proposal from a member -- GetVote -- SetVote - -- GetCommittee -- SetCommittee - -*/ - -func (k Keeper) SubmitProposal(ctx sdk.Context, proposal types.Proposal) sdk.Error { - // TODO Limit proposals to only be submitted by group members - - // Check group has permissions to enact proposal. As long as one permission allows the proposal then it goes through. Its the OR of all permissions. - committee, _ := k.GetCommittee(ctx, proposal.CommitteeID) - hasPermissions := false - for _, p := range committee.Permissions { - if p.Allows(proposal) { - hasPermissions = true - break - } +func (k Keeper) SubmitProposal(ctx sdk.Context, proposer sdk.AccAddress, proposal types.Proposal) (uint64, sdk.Error) { + // Limit proposals to only be submitted by committee members + com, found := k.GetCommittee(ctx, proposal.CommitteeID) + if !found { + return 0, sdk.ErrInternal("committee doesn't exist") } - if !hasPermissions { - return sdk.ErrInternal("committee does not have permissions to enact proposal") + if !com.HasMember(proposer) { + return 0, sdk.ErrInternal("only member can propose proposals") + } + + // Check proposal is valid + if err := proposal.ValidateBasic(); err != nil { + return 0, err + } + + // Check group has permissions to enact proposal. + if !com.HasPermissionsFor(proposal) { + return 0, sdk.ErrInternal("committee does not have permissions to enact proposal") } // TODO validate proposal by running it with cached context like how gov does it + // what if it's not valid now but will be in the future? - // TODO store the proposal, probably put it in a queue - - return nil + // Get a new ID and store the proposal + return k.StoreNewProposal(ctx, proposal) } -func (k Keeper) AddVote(ctx sdk.Context, msg types.MsgVote) sdk.Error { - /* TODO - - validate vote - - store vote - */ +func (k Keeper) AddVote(ctx sdk.Context, proposalID uint64, voter sdk.AccAddress) sdk.Error { + // Validate + proposal, found := k.GetProposal(ctx, proposalID) + if !found { + return sdk.ErrInternal("proposal not found") + } + com, found := k.GetCommittee(ctx, proposal.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}) + + // TODO close vote if tally has been reached return nil } @@ -96,6 +98,49 @@ func (k Keeper) DeleteCommittee(ctx sdk.Context, committeeID uint64) { store.Delete(types.GetKeyFromID(committeeID)) } +// 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) + store.Set(types.NextProposalIDKey, types.GetKeyFromID(id)) +} + +// GetNextProposalID reads the next available global ID from store +func (k Keeper) GetNextProposalID(ctx sdk.Context) (uint64, sdk.Error) { + store := ctx.KVStore(k.storeKey) + bz := store.Get(types.NextProposalIDKey) + if bz == nil { + return 0, sdk.ErrInternal("proposal ID not set at genesis") + } + return types.Uint64FromBytes(bz), nil +} + +// IncrementNextProposalID increments the next proposal ID in the store by 1. +func (k Keeper) IncrementNextProposalID(ctx sdk.Context) sdk.Error { + id, err := k.GetNextProposalID(ctx) + if err != nil { + return err + } + k.SetNextProposalID(ctx, id+1) + return nil +} + +// StoreNewProposal stores a proposal, adding a new ID +func (k Keeper) StoreNewProposal(ctx sdk.Context, proposal types.Proposal) (uint64, sdk.Error) { + newProposalID, err := k.GetNextProposalID(ctx) + if err != nil { + return 0, err + } + proposal.ID = newProposalID + + k.SetProposal(ctx, proposal) + + err = k.IncrementNextProposalID(ctx) + if err != nil { + return 0, err + } + return newProposalID, nil +} + // GetProposal gets a proposal from the store. func (k Keeper) GetProposal(ctx sdk.Context, proposalID uint64) (types.Proposal, bool) { store := prefix.NewStore(ctx.KVStore(k.storeKey), types.ProposalKeyPrefix) diff --git a/x/committee/keeper/keeper_test.go b/x/committee/keeper/keeper_test.go index e4fec90a..5244e9d2 100644 --- a/x/committee/keeper/keeper_test.go +++ b/x/committee/keeper/keeper_test.go @@ -24,15 +24,60 @@ type KeeperTestSuite struct { } func (suite *KeeperTestSuite) SetupTest() { - suite.app = app.NewTestApp() suite.keeper = suite.app.GetCommitteeKeeper() suite.ctx = suite.app.NewContext(true, abci.Header{}) _, suite.addresses = app.GeneratePrivKeyAddressPairs(2) } +func (suite *KeeperTestSuite) TestSubmitProposal() { + testcases := []struct { + name string + proposal types.Proposal + proposer sdk.AccAddress + expectPass bool + }{ + {name: "empty proposal", proposer: suite.addresses[0], expectPass: false}, + } + + for _, tc := range testcases { + suite.Run(tc.name, func() { + _, err := suite.keeper.SubmitProposal(suite.ctx, tc.proposer, tc.proposal) + if tc.expectPass { + suite.NoError(err) + // TODO suite.keeper.GetProposal(suite.ctx, tc.proposal.ID) + } else { + suite.NotNil(err) + } + }) + } +} + +func (suite *KeeperTestSuite) TestAddVote() { + testcases := []struct { + name string + proposalID uint64 + voter sdk.AccAddress + expectPass bool + }{ + {name: "no proposal", proposalID: 9999999, voter: suite.addresses[0], expectPass: false}, + } + + for _, tc := range testcases { + suite.Run(tc.name, func() { + err := suite.keeper.AddVote(suite.ctx, tc.proposalID, tc.voter) + if tc.expectPass { + suite.NoError(err) + // TODO GetVote + } else { + suite.NotNil(err) + } + }) + } +} + func (suite *KeeperTestSuite) TestGetSetDeleteCommittee() { - // test setup + // setup test com := types.Committee{ ID: 12, // TODO other fields diff --git a/x/committee/types/keys.go b/x/committee/types/keys.go index 09eff742..7ad1731d 100644 --- a/x/committee/types/keys.go +++ b/x/committee/types/keys.go @@ -32,7 +32,7 @@ var ( VoteKeyPrefix = []byte{0x02} // prefix for keys that store votes //AuctionByTimeKeyPrefix = []byte{0x01} // prefix for keys that are part of the auctionsByTime index - //NextAuctionIDKey = []byte{0x02} // key for the next auction id + NextProposalIDKey = []byte{0x03} // key for the next proposal id ) // GetKeyFromID returns the bytes to use as a key for a uint64 id @@ -57,6 +57,6 @@ func uint64ToBytes(id uint64) []byte { } // Uint64FromBytes converts some fixed length bytes back into a uint64. -func uint64FromBytes(bz []byte) uint64 { +func Uint64FromBytes(bz []byte) uint64 { return binary.BigEndian.Uint64(bz) } diff --git a/x/committee/types/types.go b/x/committee/types/types.go index 961a5a4c..4da5f720 100644 --- a/x/committee/types/types.go +++ b/x/committee/types/types.go @@ -12,6 +12,25 @@ type Committee struct { Permissions []Permission } +func (c Committee) HasMember(addr sdk.AccAddress) bool { + for _, m := range c.Members { + if m.Equals(addr) { + return true + } + } + return false +} + +// 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 { + for _, p := range c.Permissions { + if p.Allows(proposal) { + return true + } + } + return false +} + // Permission is anything with a method that validates whether a proposal is allowed by it or not. type Permission interface { Allows(gov.Content) bool @@ -20,6 +39,8 @@ type Permission interface { // GOV STUFF -------------------------- // Should be much the same as in gov module, except Proposals are linked to a committee ID. +var _ gov.Content = Proposal{} + type Proposal struct { gov.Content ID uint64 @@ -31,7 +52,7 @@ type Proposal struct { type Vote struct { ProposalID uint64 Voter sdk.AccAddress - Option byte + // Option byte // TODO for now don't need more than just a yes as options } // Genesis -------------------