From a53dddd945e6b99af88a717393427183abe79085 Mon Sep 17 00:00:00 2001 From: Alex Peters Date: Wed, 18 Mar 2020 15:15:11 +0100 Subject: [PATCH] Implement vote --- incubator/group/keeper.go | 23 ++- incubator/group/keeper_test.go | 252 +++++++++++++++++++++++++++++++++ incubator/group/msg.go | 40 +----- incubator/group/msg_test.go | 77 ++++++++++ incubator/group/types.go | 46 ++++-- incubator/group/typesupport.go | 46 ++++++ 6 files changed, 437 insertions(+), 47 deletions(-) create mode 100644 incubator/group/typesupport.go diff --git a/incubator/group/keeper.go b/incubator/group/keeper.go index 4cee637..331db93 100644 --- a/incubator/group/keeper.go +++ b/incubator/group/keeper.go @@ -294,6 +294,11 @@ func (k Keeper) GetGroupAccount(ctx sdk.Context, accountAddress sdk.AccAddress) return obj, k.groupAccountTable.GetOne(ctx, accountAddress.Bytes(), &obj) } +func (k Keeper) UpdateGroupAccount(ctx sdk.Context, obj *StdGroupAccountMetadata) error { + obj.Base.Version++ + return k.groupAccountTable.Save(ctx, obj) +} + func (k Keeper) GetGroupByGroupAccount(ctx sdk.Context, accountAddress sdk.AccAddress) (GroupMetadata, error) { obj, err := k.GetGroupAccount(ctx, accountAddress) if err != nil { @@ -307,9 +312,14 @@ func (k Keeper) GetGroupMemberByGroup(ctx sdk.Context, id GroupID) (orm.Iterator } func (k Keeper) Vote(ctx sdk.Context, id ProposalID, voters []sdk.AccAddress, choice Choice, comment string) error { - // voters !=0 - // comment within range - // within voting period + maxCommentSize := k.MaxCommentSize(ctx) + if len(comment) > maxCommentSize { + return errors.Wrap(ErrMaxLimit, "comment") + } + if len(voters) == 0 { + return errors.Wrap(ErrEmpty, "voters") + } + blockTime, err := types.TimestampProto(ctx.BlockTime()) if err != nil { return err @@ -499,7 +509,7 @@ func (k Keeper) ExecProposal(ctx sdk.Context, id ProposalID) error { func (k Keeper) GetProposal(ctx sdk.Context, id ProposalID) (ProposalI, error) { loaded := reflect.New(k.proposalModelType).Interface().(ProposalI) if _, err := k.proposalTable.GetOne(ctx, id.Uint64(), loaded); err != nil { - return nil, errors.Wrap(err, "load proposal source") + return nil, errors.Wrap(err, "load proposal") } return loaded, nil } @@ -572,3 +582,8 @@ func (k Keeper) CreateProposal(ctx sdk.Context, accountAddress sdk.AccAddress, c } return ProposalID(id), nil } + +func (k Keeper) GetVote(ctx sdk.Context, id ProposalID, voter sdk.AccAddress) (Vote, error) { + var v Vote + return v, k.voteTable.GetOne(ctx, Vote{Proposal: id, Voter: voter}.NaturalKey(), &v) +} diff --git a/incubator/group/keeper_test.go b/incubator/group/keeper_test.go index 949dcf7..946d804 100644 --- a/incubator/group/keeper_test.go +++ b/incubator/group/keeper_test.go @@ -343,6 +343,258 @@ func TestCreateProposal(t *testing.T) { } } +func TestVote(t *testing.T) { + amino := codec.New() + pKey, pTKey := sdk.NewKVStoreKey(params.StoreKey), sdk.NewTransientStoreKey(params.TStoreKey) + paramSpace := subspace.NewSubspace(amino, pKey, pTKey, group.DefaultParamspace) + + groupKey := sdk.NewKVStoreKey(group.StoreKeyName) + k := group.NewGroupKeeper(groupKey, paramSpace, baseapp.NewRouter(), &testdata.MyAppProposal{}) + blockTime := time.Now().UTC() + parentCtx := group.NewContext(pKey, pTKey, groupKey).WithBlockTime(blockTime) + defaultParams := group.DefaultParams() + paramSpace.SetParamSet(parentCtx, &defaultParams) + + members := []group.Member{ + {Address: []byte("valid-member-address"), Power: sdk.OneDec()}, + {Address: []byte("power-member-address"), Power: sdk.NewDec(2)}, + } + myGroupID, err := k.CreateGroup(parentCtx, []byte("valid--admin-address"), members, "test") + require.NoError(t, err) + + policy := group.ThresholdDecisionPolicy{ + Threshold: sdk.OneDec(), + Timout: types.Duration{Seconds: 1}, + } + accountAddr, err := k.CreateGroupAccount(parentCtx, []byte("valid--admin-address"), myGroupID, policy, "test") + require.NoError(t, err) + myProposalID, err := k.CreateProposal(parentCtx, accountAddr, "integration test", []sdk.AccAddress{[]byte("valid-member-address")}, nil) + require.NoError(t, err) + + specs := map[string]struct { + srcProposalID group.ProposalID + srcVoters []sdk.AccAddress + srcChoice group.Choice + srcComment string + srcCtx sdk.Context + doBefore func(ctx sdk.Context) + expErr bool + expVoteState group.Tally + expProposalStatus group.ProposalBase_Status + expResult group.ProposalBase_Result + }{ + "vote yes": { + srcProposalID: myProposalID, + srcVoters: []sdk.AccAddress{[]byte("valid-member-address")}, + srcChoice: group.Choice_YES, + expVoteState: group.Tally{ + YesCount: sdk.OneDec(), + NoCount: sdk.ZeroDec(), + AbstainCount: sdk.ZeroDec(), + VetoCount: sdk.ZeroDec(), + }, + expProposalStatus: group.ProposalBase_Submitted, + expResult: group.ProposalBase_Undefined, + }, + "vote no": { + srcProposalID: myProposalID, + srcVoters: []sdk.AccAddress{[]byte("valid-member-address")}, + srcChoice: group.Choice_NO, + expVoteState: group.Tally{ + YesCount: sdk.ZeroDec(), + NoCount: sdk.OneDec(), + AbstainCount: sdk.ZeroDec(), + VetoCount: sdk.ZeroDec(), + }, + expProposalStatus: group.ProposalBase_Submitted, + expResult: group.ProposalBase_Undefined, + }, + "vote abstain": { + srcProposalID: myProposalID, + srcVoters: []sdk.AccAddress{[]byte("valid-member-address")}, + srcChoice: group.Choice_ABSTAIN, + expVoteState: group.Tally{ + YesCount: sdk.ZeroDec(), + NoCount: sdk.ZeroDec(), + AbstainCount: sdk.OneDec(), + VetoCount: sdk.ZeroDec(), + }, + expProposalStatus: group.ProposalBase_Submitted, + expResult: group.ProposalBase_Undefined, + }, + "vote veto": { + srcProposalID: myProposalID, + srcVoters: []sdk.AccAddress{[]byte("valid-member-address")}, + srcChoice: group.Choice_VETO, + expVoteState: group.Tally{ + YesCount: sdk.ZeroDec(), + NoCount: sdk.ZeroDec(), + AbstainCount: sdk.ZeroDec(), + VetoCount: sdk.OneDec(), + }, + expProposalStatus: group.ProposalBase_Submitted, + expResult: group.ProposalBase_Undefined, + }, + "apply decision policy early": { + srcProposalID: myProposalID, + srcVoters: []sdk.AccAddress{[]byte("power-member-address")}, + srcChoice: group.Choice_YES, + expVoteState: group.Tally{ + YesCount: sdk.NewDec(2), + NoCount: sdk.ZeroDec(), + AbstainCount: sdk.ZeroDec(), + VetoCount: sdk.ZeroDec(), + }, + expProposalStatus: group.ProposalBase_Closed, + expResult: group.ProposalBase_Accepted, + }, + "comment too long": { + srcProposalID: myProposalID, + srcComment: strings.Repeat("a", 256), + srcVoters: []sdk.AccAddress{[]byte("valid-member-address")}, + srcChoice: group.Choice_NO, + expErr: true, + }, + "existing proposal required": { + srcProposalID: 9999, + srcVoters: []sdk.AccAddress{[]byte("valid-member-address")}, + srcChoice: group.Choice_NO, + expErr: true, + }, + "empty choice": { + srcProposalID: myProposalID, + srcVoters: []sdk.AccAddress{[]byte("valid-member-address")}, + expErr: true, + }, + "invalid choice": { + srcProposalID: myProposalID, + srcVoters: []sdk.AccAddress{[]byte("valid-member-address")}, + srcChoice: 5, + expErr: true, + }, + "all voters must be in group": { + srcProposalID: myProposalID, + srcVoters: []sdk.AccAddress{[]byte("valid-member-address"), []byte("non--member-address")}, + srcChoice: group.Choice_NO, + expErr: true, + }, + "voters must not include nil": { + srcProposalID: myProposalID, + srcVoters: []sdk.AccAddress{[]byte("valid-member-address"), nil}, + srcChoice: group.Choice_NO, + expErr: true, + }, + "voters must not be nil": { + srcProposalID: myProposalID, + srcChoice: group.Choice_NO, + expErr: true, + }, + "voters must not be empty": { + srcProposalID: myProposalID, + srcChoice: group.Choice_NO, + srcVoters: []sdk.AccAddress{}, + expErr: true, + }, + "admin that is not a group member can not vote": { + srcProposalID: myProposalID, + srcVoters: []sdk.AccAddress{[]byte("valid--admin-address")}, + srcChoice: group.Choice_NO, + expErr: true, + }, + "after timeout": { + srcProposalID: myProposalID, + srcVoters: []sdk.AccAddress{[]byte("valid-member-address")}, + srcChoice: group.Choice_NO, + srcCtx: parentCtx.WithBlockTime(blockTime.Add(time.Second)), + expErr: true, + }, + "closed already": { + srcProposalID: myProposalID, + srcVoters: []sdk.AccAddress{[]byte("valid-member-address")}, + srcChoice: group.Choice_NO, + doBefore: func(ctx sdk.Context) { + err := k.Vote(ctx, myProposalID, []sdk.AccAddress{[]byte("power-member-address")}, group.Choice_YES, "") + require.NoError(t, err) + }, + expErr: true, + }, + "voted already": { + srcProposalID: myProposalID, + srcVoters: []sdk.AccAddress{[]byte("valid-member-address")}, + srcChoice: group.Choice_NO, + doBefore: func(ctx sdk.Context) { + err := k.Vote(ctx, myProposalID, []sdk.AccAddress{[]byte("valid-member-address")}, group.Choice_YES, "") + require.NoError(t, err) + }, + expErr: true, + }, + "with group modified": { + srcProposalID: myProposalID, + srcVoters: []sdk.AccAddress{[]byte("valid-member-address")}, + srcChoice: group.Choice_NO, + doBefore: func(ctx sdk.Context) { + g, err := k.GetGroup(ctx, myGroupID) + require.NoError(t, err) + g.Comment = "modifed" + require.NoError(t, k.UpdateGroup(ctx, &g)) + }, + expErr: true, + }, + "with policy modified": { + srcProposalID: myProposalID, + srcVoters: []sdk.AccAddress{[]byte("valid-member-address")}, + srcChoice: group.Choice_NO, + doBefore: func(ctx sdk.Context) { + a, err := k.GetGroupAccount(ctx, accountAddr) + require.NoError(t, err) + a.Base.Comment = "modifed" + require.NoError(t, k.UpdateGroupAccount(ctx, &a)) + }, + expErr: true, + }, + } + for msg, spec := range specs { + t.Run(msg, func(t *testing.T) { + ctx := parentCtx + if !spec.srcCtx.IsZero() { + ctx = spec.srcCtx + } + ctx, _ = ctx.CacheContext() + + if spec.doBefore != nil { + spec.doBefore(ctx) + } + err := k.Vote(ctx, spec.srcProposalID, spec.srcVoters, spec.srcChoice, spec.srcComment) + if spec.expErr { + require.Error(t, err) + return + } + require.NoError(t, err) + + // and all votes are stored + for _, voter := range spec.srcVoters { + // then all data persisted + loaded, err := k.GetVote(ctx, spec.srcProposalID, voter) + require.NoError(t, err) + assert.Equal(t, spec.srcProposalID, loaded.Proposal) + assert.Equal(t, voter, loaded.Voter) + assert.Equal(t, spec.srcChoice, loaded.Choice) + assert.Equal(t, spec.srcComment, loaded.Comment) + submittedAt, err := types.TimestampFromProto(&loaded.SubmittedAt) + require.NoError(t, err) + assert.Equal(t, blockTime, submittedAt) + } + + // and proposal is updated + proposal, err := k.GetProposal(ctx, spec.srcProposalID) + require.NoError(t, err) + assert.Equal(t, spec.expVoteState, proposal.GetBase().VoteState) + assert.Equal(t, spec.expResult, proposal.GetBase().Result) + assert.Equal(t, spec.expProposalStatus, proposal.GetBase().Status) + }) + } +} + func TestLoadParam(t *testing.T) { amino := codec.New() pKey, pTKey := sdk.NewKVStoreKey(params.StoreKey), sdk.NewTransientStoreKey(params.TStoreKey) diff --git a/incubator/group/msg.go b/incubator/group/msg.go index 76bb72c..d7b7b63 100644 --- a/incubator/group/msg.go +++ b/incubator/group/msg.go @@ -64,24 +64,6 @@ func (m MsgCreateGroup) ValidateBasic() error { return nil } -type Members []Member - -func (ms Members) ValidateBasic() error { - index := make(map[string]struct{}, len(ms)) - for i := range ms { - member := ms[i] - if err := member.ValidateBasic(); err != nil { - return err - } - addr := string(member.Address) - if _, exists := index[addr]; exists { - return errors.Wrapf(ErrDuplicate, "address: %s", member.Address) - } - index[addr] = struct{}{} - } - return nil -} - func (m Member) ValidateBasic() error { if m.Address.Empty() { return sdkerrors.Wrap(ErrEmpty, "address") @@ -243,15 +225,8 @@ func (m *MsgProposeBase) ValidateBasic() error { if len(m.Proposers) == 0 { return sdkerrors.Wrap(ErrEmpty, "proposers") } - index := make(map[string]struct{}, len(m.Proposers)) - for _, p := range m.Proposers { - if err := sdk.VerifyAddressFormat(p); err != nil { - return sdkerrors.Wrap(err, "proposer") - } - if _, exists := index[string(p)]; exists { - return sdkerrors.Wrapf(ErrDuplicate, "proposer %q", p.String()) - } - index[string(p)] = struct{}{} + if err := AccAddresses(m.Proposers).ValidateBasic(); err != nil { + return sdkerrors.Wrap(err, "proposers") } return nil } @@ -325,19 +300,18 @@ func (m MsgVote) ValidateBasic() error { if len(m.Voters) == 0 { return errors.Wrap(ErrEmpty, "voters") } - for i := range m.Voters { - if err := sdk.VerifyAddressFormat(m.Voters[i]); err != nil { - return errors.Wrap(ErrInvalid, "voter") - } + if err := AccAddresses(m.Voters).ValidateBasic(); err != nil { + return sdkerrors.Wrap(err, "voters") } - // todo: prevent duplicates in votes, ignore or normalize later? - if m.Proposal == 0 { return errors.Wrap(ErrEmpty, "proposal") } if m.Choice == Choice_UNKNOWN { return errors.Wrap(ErrEmpty, "choice") } + if _, ok := Choice_name[int32(m.Choice)]; !ok { + return errors.Wrap(ErrInvalid, "choice") + } return nil } diff --git a/incubator/group/msg_test.go b/incubator/group/msg_test.go index 9d65021..b9620d2 100644 --- a/incubator/group/msg_test.go +++ b/incubator/group/msg_test.go @@ -304,3 +304,80 @@ func TestMsgProposeBase(t *testing.T) { }) } } +func TestMsgVote(t *testing.T) { + specs := map[string]struct { + src MsgVote + expErr bool + }{ + "all good with minimum fields set": { + src: MsgVote{ + Proposal: 1, + Choice: Choice_YES, + Voters: []sdk.AccAddress{[]byte("valid-member-address")}, + }, + }, + "proposal required": { + src: MsgVote{ + Choice: Choice_YES, + Voters: []sdk.AccAddress{[]byte("valid-member-address")}, + }, + expErr: true, + }, + "choice required": { + src: MsgVote{ + Proposal: 1, + Voters: []sdk.AccAddress{[]byte("valid-member-address")}, + }, + expErr: true, + }, + "valid choice required": { + src: MsgVote{ + Proposal: 1, + Choice: 5, + Voters: []sdk.AccAddress{[]byte("valid-member-address")}, + }, + expErr: true, + }, + "voter required": { + src: MsgVote{ + Proposal: 1, + Choice: Choice_YES, + }, + expErr: true, + }, + "valid voter address required": { + src: MsgVote{ + Proposal: 1, + Choice: Choice_YES, + Voters: []sdk.AccAddress{[]byte("invalid-member-address")}, + }, + expErr: true, + }, + "duplicate voters": { + src: MsgVote{ + Proposal: 1, + Choice: Choice_YES, + Voters: []sdk.AccAddress{[]byte("valid-member-address"), []byte("valid-member-address")}, + }, + expErr: true, + }, + "empty voters address not allowed": { + src: MsgVote{ + Proposal: 1, + Choice: Choice_YES, + Voters: []sdk.AccAddress{[]byte("valid-member-address"), nil, []byte("other-member-address")}, + }, + expErr: true, + }, + } + for msg, spec := range specs { + t.Run(msg, func(t *testing.T) { + err := spec.src.ValidateBasic() + if spec.expErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/incubator/group/types.go b/incubator/group/types.go index 410938c..7fba913 100644 --- a/incubator/group/types.go +++ b/incubator/group/types.go @@ -147,7 +147,32 @@ func (v Vote) NaturalKey() []byte { return result } -func (g Vote) ValidateBasic() error { +func (v Vote) ValidateBasic() error { + if len(v.Voter) == 0 { + return errors.Wrap(ErrEmpty, "voter") + } + if err := sdk.VerifyAddressFormat(v.Voter); err != nil { + return sdkerrors.Wrap(err, "voter") + } + if v.Proposal == 0 { + return errors.Wrap(ErrEmpty, "proposal") + } + if v.Choice == Choice_UNKNOWN { + return errors.Wrap(ErrEmpty, "choice") + } + if _, ok := Choice_name[int32(v.Choice)]; !ok { + return errors.Wrap(ErrInvalid, "choice") + } + if _, ok := Choice_name[int32(v.Choice)]; !ok { + return errors.Wrap(ErrInvalid, "choice") + } + t, err := types.TimestampFromProto(&v.SubmittedAt) + if err != nil { + return errors.Wrap(err, "submitted at") + } + if t.IsZero() { + return errors.Wrap(ErrEmpty, "submitted at") + } return nil } @@ -229,16 +254,10 @@ func (p ProposalBase) ValidateBasic() error { if len(p.Proposers) == 0 { return sdkerrors.Wrap(ErrEmpty, "proposers") } - index := make(map[string]struct{}, len(p.Proposers)) - for _, p := range p.Proposers { - if err := sdk.VerifyAddressFormat(p); err != nil { - return sdkerrors.Wrap(err, "proposer") - } - if _, exists := index[string(p)]; exists { - return sdkerrors.Wrapf(ErrDuplicate, "proposer %q", p.String()) - } - index[string(p)] = struct{}{} + if err := AccAddresses(p.Proposers).ValidateBasic(); err != nil { + return sdkerrors.Wrap(err, "proposers") } + if p.SubmittedAt.Seconds == 0 && p.SubmittedAt.Nanos == 0 { return sdkerrors.Wrap(ErrEmpty, "submitted at") } @@ -251,9 +270,16 @@ func (p ProposalBase) ValidateBasic() error { if p.Status == ProposalBase_PROPOSAL_STATUS_INVALID { return sdkerrors.Wrap(ErrEmpty, "status") } + if _, ok := ProposalBase_Status_name[int32(p.Status)]; !ok { + return sdkerrors.Wrap(ErrInvalid, "status") + } if p.Result == ProposalBase_PROPOSAL_RESULT_INVALID { return sdkerrors.Wrap(ErrEmpty, "result") } + if _, ok := ProposalBase_Result_name[int32(p.Result)]; !ok { + return sdkerrors.Wrap(ErrInvalid, "result") + } + if err := p.VoteState.ValidateBasic(); err != nil { return errors.Wrap(err, "vote state") } diff --git a/incubator/group/typesupport.go b/incubator/group/typesupport.go new file mode 100644 index 0000000..0b9afde --- /dev/null +++ b/incubator/group/typesupport.go @@ -0,0 +1,46 @@ +package group + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/pkg/errors" +) + +type Members []Member + +func (ms Members) ValidateBasic() error { + index := make(map[string]struct{}, len(ms)) + for i := range ms { + member := ms[i] + if err := member.ValidateBasic(); err != nil { + return err + } + addr := string(member.Address) + if _, exists := index[addr]; exists { + return errors.Wrapf(ErrDuplicate, "address: %s", member.Address) + } + index[addr] = struct{}{} + } + return nil +} + +type AccAddresses []sdk.AccAddress + +func (a AccAddresses) ValidateBasic() error { + index := make(map[string]struct{}, len(a)) + for i := range a { + accAddr := a[i] + if accAddr.Empty() { + return sdkerrors.Wrap(ErrEmpty, "address") + } + if err := sdk.VerifyAddressFormat(accAddr); err != nil { + return sdkerrors.Wrap(err, "address") + } + addr := string(accAddr) + if _, exists := index[addr]; exists { + return errors.Wrapf(ErrDuplicate, "address: %s", accAddr.String()) + } + index[addr] = struct{}{} + } + return nil +}