From 9165c796b40bb88bc8fcef293295249d3d497728 Mon Sep 17 00:00:00 2001 From: atheeshp <59333759+atheeshp@users.noreply.github.com> Date: Fri, 11 Mar 2022 18:41:10 +0530 Subject: [PATCH] refactor: x/group proposal tally at the end of voting period (#11310) ## Description Closes: #11246 --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) --- x/group/keeper/keeper.go | 69 ++++++++++ x/group/keeper/keeper_test.go | 135 ++++++++++++++++++++ x/group/module/abci.go | 12 ++ x/group/module/abci_test.go | 233 ++++++++++++++++++++++++++++++++++ x/group/module/module.go | 3 +- 5 files changed, 451 insertions(+), 1 deletion(-) create mode 100644 x/group/module/abci.go create mode 100644 x/group/module/abci_test.go diff --git a/x/group/keeper/keeper.go b/x/group/keeper/keeper.go index 96f05a53e..103a9b86a 100644 --- a/x/group/keeper/keeper.go +++ b/x/group/keeper/keeper.go @@ -10,6 +10,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" authmiddleware "github.com/cosmos/cosmos-sdk/x/auth/middleware" "github.com/cosmos/cosmos-sdk/x/group" + "github.com/cosmos/cosmos-sdk/x/group/errors" "github.com/cosmos/cosmos-sdk/x/group/internal/orm" ) @@ -35,6 +36,7 @@ const ( ProposalTableSeqPrefix byte = 0x31 ProposalByGroupPolicyIndexPrefix byte = 0x32 ProposalByProposerIndexPrefix byte = 0x33 + ProposalsByVotingPeriodEndPrefix byte = 0x34 // Vote Table VoteTablePrefix byte = 0x40 @@ -66,6 +68,7 @@ type Keeper struct { proposalTable orm.AutoUInt64Table proposalByGroupPolicyIndex orm.Index proposalByProposerIndex orm.Index + proposalsByVotingPeriodEnd orm.Index // Vote Table voteTable orm.PrimaryKeyTable @@ -181,6 +184,13 @@ func NewKeeper(storeKey storetypes.StoreKey, cdc codec.Codec, router *authmiddle if err != nil { panic(err.Error()) } + k.proposalsByVotingPeriodEnd, err = orm.NewIndex(proposalTable, ProposalsByVotingPeriodEndPrefix, func(value interface{}) ([]interface{}, error) { + votingPeriodEnd := value.(*group.Proposal).VotingPeriodEnd + return []interface{}{sdk.FormatTimeBytes(votingPeriodEnd)}, nil + }, []byte{}) + if err != nil { + panic(err.Error()) + } k.proposalTable = *proposalTable // Vote Table @@ -230,3 +240,62 @@ func (k Keeper) MaxMetadataLength() uint64 { return k.config.MaxMetadataLen } func (k Keeper) GetGroupSequence(ctx sdk.Context) uint64 { return k.groupTable.Sequence().CurVal(ctx.KVStore(k.key)) } + +func (k Keeper) iterateProposalsByVPEnd(ctx sdk.Context, cb func(proposal group.Proposal) (bool, error)) error { + timeBytes := sdk.FormatTimeBytes(ctx.BlockTime()) + it, err := k.proposalsByVotingPeriodEnd.PrefixScan(ctx.KVStore(k.key), nil, timeBytes) + + if err != nil { + return err + } + defer it.Close() + + var proposal group.Proposal + for { + _, err := it.LoadNext(&proposal) + if errors.ErrORMIteratorDone.Is(err) { + break + } + if err != nil { + return err + } + + stop, err := cb(proposal) + if err != nil { + return err + } + if stop { + break + } + } + + return nil +} + +func (k Keeper) UpdateTallyOfVPEndProposals(ctx sdk.Context) error { + k.iterateProposalsByVPEnd(ctx, func(proposal group.Proposal) (bool, error) { + + policyInfo, err := k.getGroupPolicyInfo(ctx, proposal.Address) + if err != nil { + return true, err + } + + electorate, err := k.getGroupInfo(ctx, policyInfo.GroupId) + if err != nil { + return true, err + } + + err = k.doTallyAndUpdate(ctx, &proposal, electorate, policyInfo) + if err != nil { + return true, err + } + + if err := k.proposalTable.Update(ctx.KVStore(k.key), proposal.Id, &proposal); err != nil { + return true, err + } + + return false, nil + }) + + return nil +} diff --git a/x/group/keeper/keeper_test.go b/x/group/keeper/keeper_test.go index 1ce1cdaea..06f0c5c2e 100644 --- a/x/group/keeper/keeper_test.go +++ b/x/group/keeper/keeper_test.go @@ -20,6 +20,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/group" "github.com/cosmos/cosmos-sdk/x/group/internal/math" "github.com/cosmos/cosmos-sdk/x/group/keeper" + "github.com/cosmos/cosmos-sdk/x/group/module" ) type TestSuite struct { @@ -31,6 +32,7 @@ type TestSuite struct { addrs []sdk.AccAddress groupID uint64 groupPolicyAddr sdk.AccAddress + policy group.DecisionPolicy keeper keeper.Keeper blockTime time.Time } @@ -72,6 +74,7 @@ func (s *TestSuite) SetupTest() { s.Require().NoError(err) policyRes, err := s.keeper.CreateGroupPolicy(s.ctx, policyReq) s.Require().NoError(err) + s.policy = policy addr, err := sdk.AccAddressFromBech32(policyRes.Address) s.Require().NoError(err) s.groupPolicyAddr = addr @@ -2443,6 +2446,138 @@ func (s *TestSuite) TestExecProposal() { } } +func (s *TestSuite) TestProposalsByVPEnd() { + addrs := s.addrs + addr2 := addrs[1] + groupPolicy := s.groupPolicyAddr + + votingPeriod := s.policy.GetVotingPeriod() + ctx := s.sdkCtx + now := time.Now() + + msgSend := &banktypes.MsgSend{ + FromAddress: s.groupPolicyAddr.String(), + ToAddress: addr2.String(), + Amount: sdk.Coins{sdk.NewInt64Coin("test", 100)}, + } + + proposers := []string{addr2.String()} + + specs := map[string]struct { + preRun func(sdkCtx sdk.Context) uint64 + proposalId uint64 + admin string + expErrMsg string + newCtx sdk.Context + tallyRes group.TallyResult + expStatus group.ProposalStatus + expExecutorResult group.ProposalResult + }{ + "tally updated after voting power end": { + preRun: func(sdkCtx sdk.Context) uint64 { + return submitProposal(sdkCtx, s, []sdk.Msg{msgSend}, proposers) + }, + admin: proposers[0], + newCtx: ctx.WithBlockTime(now.Add(votingPeriod).Add(time.Hour)), + tallyRes: group.DefaultTallyResult(), + expStatus: group.PROPOSAL_STATUS_SUBMITTED, + expExecutorResult: group.PROPOSAL_RESULT_UNFINALIZED, + }, + "tally within voting period": { + preRun: func(sdkCtx sdk.Context) uint64 { + return submitProposal(s.ctx, s, []sdk.Msg{msgSend}, proposers) + }, + admin: proposers[0], + newCtx: ctx, + tallyRes: group.DefaultTallyResult(), + expStatus: group.PROPOSAL_STATUS_SUBMITTED, + expExecutorResult: group.PROPOSAL_RESULT_UNFINALIZED, + }, + "tally within voting period(with votes)": { + preRun: func(sdkCtx sdk.Context) uint64 { + return submitProposalAndVote(s.ctx, s, []sdk.Msg{msgSend}, proposers, group.VOTE_OPTION_YES) + }, + admin: proposers[0], + newCtx: ctx, + tallyRes: group.DefaultTallyResult(), + expStatus: group.PROPOSAL_STATUS_SUBMITTED, + expExecutorResult: group.PROPOSAL_RESULT_UNFINALIZED, + }, + "tally after voting period(with votes)": { + preRun: func(sdkCtx sdk.Context) uint64 { + return submitProposalAndVote(s.ctx, s, []sdk.Msg{msgSend}, proposers, group.VOTE_OPTION_YES) + }, + admin: proposers[0], + newCtx: ctx.WithBlockTime(now.Add(votingPeriod).Add(time.Hour)), + tallyRes: group.TallyResult{ + YesCount: "2", + NoCount: "0", + NoWithVetoCount: "0", + AbstainCount: "0", + }, + expStatus: group.PROPOSAL_STATUS_CLOSED, + expExecutorResult: group.PROPOSAL_RESULT_ACCEPTED, + }, + "tally of closed proposal": { + preRun: func(sdkCtx sdk.Context) uint64 { + pId := submitProposal(s.ctx, s, []sdk.Msg{msgSend}, proposers) + _, err := s.keeper.WithdrawProposal(s.ctx, &group.MsgWithdrawProposal{ + ProposalId: pId, + Address: groupPolicy.String(), + }) + + s.Require().NoError(err) + return pId + }, + admin: proposers[0], + newCtx: ctx, + tallyRes: group.DefaultTallyResult(), + expStatus: group.PROPOSAL_STATUS_WITHDRAWN, + expExecutorResult: group.PROPOSAL_RESULT_UNFINALIZED, + }, + "tally of closed proposal (with votes)": { + preRun: func(sdkCtx sdk.Context) uint64 { + pId := submitProposalAndVote(s.ctx, s, []sdk.Msg{msgSend}, proposers, group.VOTE_OPTION_YES) + _, err := s.keeper.WithdrawProposal(s.ctx, &group.MsgWithdrawProposal{ + ProposalId: pId, + Address: groupPolicy.String(), + }) + + s.Require().NoError(err) + return pId + }, + admin: proposers[0], + newCtx: ctx, + tallyRes: group.DefaultTallyResult(), + expStatus: group.PROPOSAL_STATUS_WITHDRAWN, + expExecutorResult: group.PROPOSAL_RESULT_UNFINALIZED, + }, + } + + for msg, spec := range specs { + spec := spec + s.Run(msg, func() { + pId := spec.preRun(s.sdkCtx) + + module.EndBlocker(spec.newCtx, s.keeper) + resp, err := s.keeper.Proposal(spec.newCtx, &group.QueryProposalRequest{ + ProposalId: pId, + }) + + if spec.expErrMsg != "" { + s.Require().Error(err) + s.Require().Contains(err.Error(), spec.expErrMsg) + return + } + + s.Require().NoError(err) + s.Require().Equal(resp.GetProposal().FinalTallyResult, spec.tallyRes) + s.Require().Equal(resp.GetProposal().Status, spec.expStatus) + s.Require().Equal(resp.GetProposal().Result, spec.expExecutorResult) + }) + } +} + func (s *TestSuite) TestLeaveGroup() { addrs := simapp.AddTestAddrsIncremental(s.app, s.sdkCtx, 7, sdk.NewInt(3000000)) admin1 := addrs[0] diff --git a/x/group/module/abci.go b/x/group/module/abci.go new file mode 100644 index 000000000..90e900beb --- /dev/null +++ b/x/group/module/abci.go @@ -0,0 +1,12 @@ +package module + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/group/keeper" +) + +func EndBlocker(ctx sdk.Context, k keeper.Keeper) { + if err := k.UpdateTallyOfVPEndProposals(ctx); err != nil { + panic(err) + } +} diff --git a/x/group/module/abci_test.go b/x/group/module/abci_test.go new file mode 100644 index 000000000..f303a38d0 --- /dev/null +++ b/x/group/module/abci_test.go @@ -0,0 +1,233 @@ +package module_test + +import ( + "context" + "testing" + "time" + + "github.com/cosmos/cosmos-sdk/simapp" + "github.com/cosmos/cosmos-sdk/types" + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + "github.com/cosmos/cosmos-sdk/x/group" + "github.com/cosmos/cosmos-sdk/x/group/module" + "github.com/stretchr/testify/require" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" +) + +func TestEndBlocker(t *testing.T) { + app := simapp.Setup(t, false) + ctx := app.BaseApp.NewContext(false, tmproto.Header{}) + addrs := simapp.AddTestAddrsIncremental(app, ctx, 4, types.NewInt(30000000)) + + // Initial group, group policy and balance setup + members := []group.Member{ + {Address: addrs[1].String(), Weight: "1"}, {Address: addrs[2].String(), Weight: "2"}, + } + + groupRes, err := app.GroupKeeper.CreateGroup(ctx, &group.MsgCreateGroup{ + Admin: addrs[0].String(), + Members: members, + }) + + require.NoError(t, err) + groupID := groupRes.GroupId + + policy := group.NewThresholdDecisionPolicy( + "2", + time.Second, + 0, + ) + + policyReq := &group.MsgCreateGroupPolicy{ + Admin: addrs[0].String(), + GroupId: groupID, + } + + err = policyReq.SetDecisionPolicy(policy) + require.NoError(t, err) + policyRes, err := app.GroupKeeper.CreateGroupPolicy(ctx, policyReq) + require.NoError(t, err) + + groupPolicyAddr, err := types.AccAddressFromBech32(policyRes.Address) + require.NoError(t, err) + + votingPeriod := policy.GetVotingPeriod() + now := time.Now() + + msgSend := &banktypes.MsgSend{ + FromAddress: groupPolicyAddr.String(), + ToAddress: addrs[3].String(), + Amount: types.Coins{types.NewInt64Coin("test", 100)}, + } + + proposers := []string{addrs[2].String()} + + specs := map[string]struct { + preRun func(sdkCtx types.Context) uint64 + proposalId uint64 + admin string + expErrMsg string + newCtx types.Context + tallyRes group.TallyResult + expStatus group.ProposalStatus + expExecutorResult group.ProposalResult + }{ + "tally updated after voting power end": { + preRun: func(sdkCtx types.Context) uint64 { + pId, err := submitProposal(app, sdkCtx, []types.Msg{msgSend}, proposers, groupPolicyAddr) + require.NoError(t, err) + return pId + }, + admin: proposers[0], + newCtx: ctx.WithBlockTime(now.Add(votingPeriod).Add(time.Hour)), + tallyRes: group.DefaultTallyResult(), + expStatus: group.PROPOSAL_STATUS_SUBMITTED, + expExecutorResult: group.PROPOSAL_RESULT_UNFINALIZED, + }, + "tally within voting period": { + preRun: func(sdkCtx types.Context) uint64 { + pId, err := submitProposal(app, sdkCtx, []types.Msg{msgSend}, proposers, groupPolicyAddr) + require.NoError(t, err) + + return pId + }, + admin: proposers[0], + newCtx: ctx, + tallyRes: group.DefaultTallyResult(), + expStatus: group.PROPOSAL_STATUS_SUBMITTED, + expExecutorResult: group.PROPOSAL_RESULT_UNFINALIZED, + }, + "tally within voting period(with votes)": { + preRun: func(sdkCtx types.Context) uint64 { + pId, err := submitProposalAndVote(app, ctx, []types.Msg{msgSend}, proposers, groupPolicyAddr, group.VOTE_OPTION_YES) + require.NoError(t, err) + + return pId + }, + admin: proposers[0], + newCtx: ctx, + tallyRes: group.DefaultTallyResult(), + expStatus: group.PROPOSAL_STATUS_SUBMITTED, + expExecutorResult: group.PROPOSAL_RESULT_UNFINALIZED, + }, + "tally after voting period(with votes)": { + preRun: func(sdkCtx types.Context) uint64 { + pId, err := submitProposalAndVote(app, ctx, []types.Msg{msgSend}, proposers, groupPolicyAddr, group.VOTE_OPTION_YES) + require.NoError(t, err) + + return pId + }, + admin: proposers[0], + newCtx: ctx.WithBlockTime(now.Add(votingPeriod).Add(time.Hour)), + tallyRes: group.TallyResult{ + YesCount: "2", + NoCount: "0", + NoWithVetoCount: "0", + AbstainCount: "0", + }, + expStatus: group.PROPOSAL_STATUS_CLOSED, + expExecutorResult: group.PROPOSAL_RESULT_ACCEPTED, + }, + "tally of closed proposal": { + preRun: func(sdkCtx types.Context) uint64 { + pId, err := submitProposal(app, sdkCtx, []types.Msg{msgSend}, proposers, groupPolicyAddr) + require.NoError(t, err) + + _, err = app.GroupKeeper.WithdrawProposal(ctx, &group.MsgWithdrawProposal{ + ProposalId: pId, + Address: groupPolicyAddr.String(), + }) + + require.NoError(t, err) + return pId + }, + admin: proposers[0], + newCtx: ctx, + tallyRes: group.DefaultTallyResult(), + expStatus: group.PROPOSAL_STATUS_WITHDRAWN, + expExecutorResult: group.PROPOSAL_RESULT_UNFINALIZED, + }, + "tally of closed proposal (with votes)": { + preRun: func(sdkCtx types.Context) uint64 { + pId, err := submitProposalAndVote(app, ctx, []types.Msg{msgSend}, proposers, groupPolicyAddr, group.VOTE_OPTION_YES) + require.NoError(t, err) + + _, err = app.GroupKeeper.WithdrawProposal(ctx, &group.MsgWithdrawProposal{ + ProposalId: pId, + Address: groupPolicyAddr.String(), + }) + + require.NoError(t, err) + return pId + }, + admin: proposers[0], + newCtx: ctx, + tallyRes: group.DefaultTallyResult(), + expStatus: group.PROPOSAL_STATUS_WITHDRAWN, + expExecutorResult: group.PROPOSAL_RESULT_UNFINALIZED, + }, + } + + for msg, spec := range specs { + t.Run(msg, func(t *testing.T) { + spec := spec + pId := spec.preRun(ctx) + + module.EndBlocker(spec.newCtx, app.GroupKeeper) + resp, err := app.GroupKeeper.Proposal(spec.newCtx, &group.QueryProposalRequest{ + ProposalId: pId, + }) + + if spec.expErrMsg != "" { + require.Error(t, err) + require.Contains(t, err.Error(), spec.expErrMsg) + return + } + + require.NoError(t, err) + require.Equal(t, resp.GetProposal().FinalTallyResult, spec.tallyRes) + require.Equal(t, resp.GetProposal().Status, spec.expStatus) + require.Equal(t, resp.GetProposal().Result, spec.expExecutorResult) + }) + } +} + +func submitProposal( + app *simapp.SimApp, ctx context.Context, msgs []types.Msg, + proposers []string, groupPolicyAddr types.AccAddress) (uint64, error) { + proposalReq := &group.MsgSubmitProposal{ + Address: groupPolicyAddr.String(), + Proposers: proposers, + } + err := proposalReq.SetMsgs(msgs) + if err != nil { + return 0, err + } + + proposalRes, err := app.GroupKeeper.SubmitProposal(ctx, proposalReq) + if err != nil { + return 0, err + } + + return proposalRes.ProposalId, nil +} + +func submitProposalAndVote( + app *simapp.SimApp, ctx context.Context, msgs []types.Msg, + proposers []string, groupPolicyAddr types.AccAddress, voteOption group.VoteOption) (uint64, error) { + myProposalID, err := submitProposal(app, ctx, msgs, proposers, groupPolicyAddr) + if err != nil { + return 0, err + } + + _, err = app.GroupKeeper.Vote(ctx, &group.MsgVote{ + ProposalId: myProposalID, + Voter: proposers[0], + Option: voteOption, + }) + if err != nil { + return 0, err + } + + return myProposalID, nil +} diff --git a/x/group/module/module.go b/x/group/module/module.go index 78102cfe9..086728422 100644 --- a/x/group/module/module.go +++ b/x/group/module/module.go @@ -154,8 +154,9 @@ func (AppModule) ConsensusVersion() uint64 { return 1 } func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {} -// EndBlock does nothing +// EndBlock implements the group module's EndBlock. func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { + EndBlocker(ctx, am.keeper) return []abci.ValidatorUpdate{} }