From a7e68bf1eafb3fef80aea0b33f89f584df7fb290 Mon Sep 17 00:00:00 2001 From: atheeshp <59333759+atheeshp@users.noreply.github.com> Date: Tue, 29 Mar 2022 19:08:39 +0530 Subject: [PATCH] chore: remove votes sum invariant (#11483) ## Description We don't need votes sum invariant anymore since we are pruning votes after updating tally of the proposals. Closes: #11475 --- ### 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/invariants.go | 186 +----------------- x/group/keeper/invariants_test.go | 306 ------------------------------ 2 files changed, 1 insertion(+), 491 deletions(-) diff --git a/x/group/keeper/invariants.go b/x/group/keeper/invariants.go index 15213fb3f..9b98e69f8 100644 --- a/x/group/keeper/invariants.go +++ b/x/group/keeper/invariants.go @@ -12,16 +12,11 @@ import ( "github.com/cosmos/cosmos-sdk/x/group/internal/orm" ) -const ( - votesInvariant = "Tally-Votes" - weightInvariant = "Group-TotalWeight" - votesSumInvariant = "Tally-Votes-Sum" -) +const weightInvariant = "Group-TotalWeight" // RegisterInvariants registers all group invariants func RegisterInvariants(ir sdk.InvariantRegistry, keeper Keeper) { ir.RegisterRoute(group.ModuleName, weightInvariant, GroupTotalWeightInvariant(keeper)) - ir.RegisterRoute(group.ModuleName, votesSumInvariant, TallyVotesSumInvariant(keeper)) } // GroupTotalWeightInvariant checks that group's TotalWeight must be equal to the sum of its members. @@ -32,15 +27,6 @@ func GroupTotalWeightInvariant(keeper Keeper) sdk.Invariant { } } -// TallyVotesSumInvariant checks that proposal FinalTallyResult must correspond to the vote option, -// for proposals with PROPOSAL_STATUS_CLOSED status. -func TallyVotesSumInvariant(keeper Keeper) sdk.Invariant { - return func(ctx sdk.Context) (string, bool) { - msg, broken := TallyVotesSumInvariantHelper(ctx, keeper.key, keeper.proposalTable, keeper.groupMemberTable, keeper.voteByProposalIndex, keeper.groupPolicyTable) - return sdk.FormatInvariant(group.ModuleName, votesSumInvariant, msg), broken - } -} - func GroupTotalWeightInvariantHelper(ctx sdk.Context, key storetypes.StoreKey, groupTable orm.AutoUInt64Table, groupMemberByGroupIndex orm.Index) (string, bool) { var msg string @@ -113,173 +99,3 @@ func GroupTotalWeightInvariantHelper(ctx sdk.Context, key storetypes.StoreKey, g } return msg, broken } - -func TallyVotesSumInvariantHelper(ctx sdk.Context, key storetypes.StoreKey, proposalTable orm.AutoUInt64Table, groupMemberTable orm.PrimaryKeyTable, voteByProposalIndex orm.Index, groupPolicyTable orm.PrimaryKeyTable) (string, bool) { - var msg string - var broken bool - - proposalIt, err := proposalTable.PrefixScan(ctx.KVStore(key), 1, math.MaxUint64) - if err != nil { - msg += fmt.Sprintf("PrefixScan failure on proposal table\n%v\n", err) - return msg, broken - } - defer proposalIt.Close() - - for { - var proposal group.Proposal - _, err = proposalIt.LoadNext(&proposal) - if errors.ErrORMIteratorDone.Is(err) { - break - } - if err != nil { - msg += fmt.Sprintf("LoadNext failure on proposal table iterator\n%v\n", err) - return msg, broken - } - - // Only look at proposals that are closed, i.e. for which FinalTallyResult has been computed - if proposal.Status != group.PROPOSAL_STATUS_CLOSED { - continue - } - - totalVotingWeight, err := groupmath.NewNonNegativeDecFromString("0") - if err != nil { - msg += fmt.Sprintf("error while parsing positive dec zero for total voting weight\n%v\n", err) - return msg, broken - } - yesVoteWeight, err := groupmath.NewNonNegativeDecFromString("0") - if err != nil { - msg += fmt.Sprintf("error while parsing positive dec zero for yes voting weight\n%v\n", err) - return msg, broken - } - noVoteWeight, err := groupmath.NewNonNegativeDecFromString("0") - if err != nil { - msg += fmt.Sprintf("error while parsing positive dec zero for no voting weight\n%v\n", err) - return msg, broken - } - abstainVoteWeight, err := groupmath.NewNonNegativeDecFromString("0") - if err != nil { - msg += fmt.Sprintf("error while parsing positive dec zero for abstain voting weight\n%v\n", err) - return msg, broken - } - vetoVoteWeight, err := groupmath.NewNonNegativeDecFromString("0") - if err != nil { - msg += fmt.Sprintf("error while parsing positive dec zero for veto voting weight\n%v\n", err) - return msg, broken - } - - var groupPolicy group.GroupPolicyInfo - err = groupPolicyTable.GetOne(ctx.KVStore(key), orm.PrimaryKey(&group.GroupPolicyInfo{Address: proposal.Address}), &groupPolicy) - if err != nil { - msg += fmt.Sprintf("group policy not found for address: %s\n%v\n", proposal.Address, err) - return msg, broken - } - - if proposal.GroupPolicyVersion != groupPolicy.Version { - msg += fmt.Sprintf("group policy with address %s was modified\n", groupPolicy.Address) - return msg, broken - } - - voteIt, err := voteByProposalIndex.Get(ctx.KVStore(key), proposal.Id) - if err != nil { - msg += fmt.Sprintf("error while returning vote iterator for proposal with ID %d\n%v\n", proposal.Id, err) - return msg, broken - } - defer voteIt.Close() - - for { - var vote group.Vote - _, err := voteIt.LoadNext(&vote) - if errors.ErrORMIteratorDone.Is(err) { - break - } - if err != nil { - msg += fmt.Sprintf("LoadNext failure on voteByProposalIndex index iterator\n%v\n", err) - return msg, broken - } - - var groupMem group.GroupMember - err = groupMemberTable.GetOne(ctx.KVStore(key), orm.PrimaryKey(&group.GroupMember{GroupId: groupPolicy.GroupId, Member: &group.Member{Address: vote.Voter}}), &groupMem) - if err != nil { - msg += fmt.Sprintf("group member not found with group ID %d and group member %s\n%v\n", groupPolicy.GroupId, vote.Voter, err) - return msg, broken - } - - curMemVotingWeight, err := groupmath.NewNonNegativeDecFromString(groupMem.Member.Weight) - if err != nil { - msg += fmt.Sprintf("error while parsing non-negative decimal for group member %s\n%v\n", groupMem.Member.Address, err) - return msg, broken - } - totalVotingWeight, err = groupmath.Add(totalVotingWeight, curMemVotingWeight) - if err != nil { - msg += fmt.Sprintf("decimal addition error while adding current member voting weight to total voting weight\n%v\n", err) - return msg, broken - } - - switch vote.Option { - case group.VOTE_OPTION_YES: - yesVoteWeight, err = groupmath.Add(yesVoteWeight, curMemVotingWeight) - if err != nil { - msg += fmt.Sprintf("decimal addition error\n%v\n", err) - return msg, broken - } - case group.VOTE_OPTION_NO: - noVoteWeight, err = groupmath.Add(noVoteWeight, curMemVotingWeight) - if err != nil { - msg += fmt.Sprintf("decimal addition error\n%v\n", err) - return msg, broken - } - case group.VOTE_OPTION_ABSTAIN: - abstainVoteWeight, err = groupmath.Add(abstainVoteWeight, curMemVotingWeight) - if err != nil { - msg += fmt.Sprintf("decimal addition error\n%v\n", err) - return msg, broken - } - case group.VOTE_OPTION_NO_WITH_VETO: - vetoVoteWeight, err = groupmath.Add(vetoVoteWeight, curMemVotingWeight) - if err != nil { - msg += fmt.Sprintf("decimal addition error\n%v\n", err) - return msg, broken - } - } - } - - totalProposalVotes, err := proposal.FinalTallyResult.TotalCounts() - if err != nil { - msg += fmt.Sprintf("error while getting total weighted votes of proposal with ID %d\n%v\n", proposal.Id, err) - return msg, broken - } - proposalYesCount, err := proposal.FinalTallyResult.GetYesCount() - if err != nil { - msg += fmt.Sprintf("error while getting the weighted sum of yes votes for proposal with ID %d\n%v\n", proposal.Id, err) - return msg, broken - } - proposalNoCount, err := proposal.FinalTallyResult.GetNoCount() - if err != nil { - msg += fmt.Sprintf("error while getting the weighted sum of no votes for proposal with ID %d\n%v\n", proposal.Id, err) - return msg, broken - } - proposalAbstainCount, err := proposal.FinalTallyResult.GetAbstainCount() - if err != nil { - msg += fmt.Sprintf("error while getting the weighted sum of abstain votes for proposal with ID %d\n%v\n", proposal.Id, err) - return msg, broken - } - proposalVetoCount, err := proposal.FinalTallyResult.GetNoWithVetoCount() - if err != nil { - msg += fmt.Sprintf("error while getting the weighted sum of veto votes for proposal with ID %d\n%v\n", proposal.Id, err) - return msg, broken - } - - if totalProposalVotes.Cmp(totalVotingWeight) != 0 { - broken = true - msg += fmt.Sprintf("proposal FinalTallyResult must correspond to the sum of votes weights\nProposal with ID %d has total proposal votes %s, but got sum of votes weights %s\n", proposal.Id, totalProposalVotes.String(), totalVotingWeight.String()) - break - } - - if (yesVoteWeight.Cmp(proposalYesCount) != 0) || (noVoteWeight.Cmp(proposalNoCount) != 0) || (abstainVoteWeight.Cmp(proposalAbstainCount) != 0) || (vetoVoteWeight.Cmp(proposalVetoCount) != 0) { - broken = true - msg += fmt.Sprintf("proposal FinalTallyResult must correspond to the sum of all votes\nProposal with ID %d FinalTallyResult must correspond to the sum of all votes\n", proposal.Id) - break - } - } - return msg, broken -} diff --git a/x/group/keeper/invariants_test.go b/x/group/keeper/invariants_test.go index 308d2b6a1..c21565203 100644 --- a/x/group/keeper/invariants_test.go +++ b/x/group/keeper/invariants_test.go @@ -2,7 +2,6 @@ package keeper_test import ( "testing" - "time" "github.com/stretchr/testify/suite" "github.com/tendermint/tendermint/libs/log" @@ -148,308 +147,3 @@ func (s *invariantTestSuite) TestGroupTotalWeightInvariant() { } } - -func (s *invariantTestSuite) TestTallyVotesSumInvariant() { - sdkCtx, _ := s.ctx.CacheContext() - curCtx, cdc, key := sdkCtx, s.cdc, s.key - - // Group Table - groupTable, err := orm.NewAutoUInt64Table([2]byte{keeper.GroupTablePrefix}, keeper.GroupTableSeqPrefix, &group.GroupInfo{}, cdc) - s.Require().NoError(err) - - // Group Policy Table - groupPolicyTable, err := orm.NewPrimaryKeyTable([2]byte{keeper.GroupPolicyTablePrefix}, &group.GroupPolicyInfo{}, cdc) - s.Require().NoError(err) - - // Group Member Table - groupMemberTable, err := orm.NewPrimaryKeyTable([2]byte{keeper.GroupMemberTablePrefix}, &group.GroupMember{}, cdc) - s.Require().NoError(err) - - // Proposal Table - proposalTable, err := orm.NewAutoUInt64Table([2]byte{keeper.ProposalTablePrefix}, keeper.ProposalTableSeqPrefix, &group.Proposal{}, cdc) - s.Require().NoError(err) - - // Vote Table - voteTable, err := orm.NewPrimaryKeyTable([2]byte{keeper.VoteTablePrefix}, &group.Vote{}, cdc) - s.Require().NoError(err) - - voteByProposalIndex, err := orm.NewIndex(voteTable, keeper.VoteByProposalIndexPrefix, func(value interface{}) ([]interface{}, error) { - return []interface{}{value.(*group.Vote).ProposalId}, nil - }, group.Vote{}.ProposalId) - s.Require().NoError(err) - - _, _, adminAddr := testdata.KeyTestPubAddr() - _, _, addr1 := testdata.KeyTestPubAddr() - _, _, addr2 := testdata.KeyTestPubAddr() - - votingPeriodEnd := curCtx.BlockTime().Add(time.Second * 600) - - specs := map[string]struct { - groupsInfo *group.GroupInfo - groupPolicy *group.GroupPolicyInfo - groupMembers []*group.GroupMember - proposal *group.Proposal - votes []*group.Vote - expBroken bool - }{ - "invariant not broken": { - groupsInfo: &group.GroupInfo{ - Id: 1, - Admin: adminAddr.String(), - Version: 1, - TotalWeight: "7", - }, - groupPolicy: &group.GroupPolicyInfo{ - Address: addr1.String(), - GroupId: 1, - Admin: adminAddr.String(), - Version: 1, - }, - groupMembers: []*group.GroupMember{ - { - GroupId: 1, - Member: &group.Member{ - Address: addr1.String(), - Weight: "4", - }, - }, - { - GroupId: 1, - Member: &group.Member{ - Address: addr2.String(), - Weight: "3", - }, - }, - }, - proposal: &group.Proposal{ - Id: 1, - Address: addr1.String(), - Proposers: []string{addr1.String()}, - SubmitTime: curCtx.BlockTime(), - GroupVersion: 1, - GroupPolicyVersion: 1, - Status: group.PROPOSAL_STATUS_CLOSED, - Result: group.PROPOSAL_RESULT_ACCEPTED, - FinalTallyResult: group.TallyResult{YesCount: "4", NoCount: "3", AbstainCount: "0", NoWithVetoCount: "0"}, - VotingPeriodEnd: votingPeriodEnd, - ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN, - }, - votes: []*group.Vote{ - { - ProposalId: 1, - Voter: addr1.String(), - Option: group.VOTE_OPTION_YES, - SubmitTime: curCtx.BlockTime(), - }, - { - ProposalId: 1, - Voter: addr2.String(), - Option: group.VOTE_OPTION_NO, - SubmitTime: curCtx.BlockTime(), - }, - }, - expBroken: false, - }, - "proposal not closed ignored": { - groupsInfo: &group.GroupInfo{ - Id: 1, - Admin: adminAddr.String(), - Version: 1, - TotalWeight: "7", - }, - groupPolicy: &group.GroupPolicyInfo{ - Address: addr1.String(), - GroupId: 1, - Admin: adminAddr.String(), - Version: 1, - }, - groupMembers: []*group.GroupMember{ - { - GroupId: 1, - Member: &group.Member{ - Address: addr1.String(), - Weight: "4", - }, - }, - { - GroupId: 1, - Member: &group.Member{ - Address: addr2.String(), - Weight: "3", - }, - }, - }, - proposal: &group.Proposal{ - Id: 1, - Address: addr1.String(), - Proposers: []string{addr1.String()}, - SubmitTime: curCtx.BlockTime(), - GroupVersion: 1, - GroupPolicyVersion: 1, - Status: group.PROPOSAL_STATUS_SUBMITTED, - Result: group.PROPOSAL_RESULT_UNFINALIZED, - FinalTallyResult: group.TallyResult{YesCount: "0", NoCount: "0", AbstainCount: "0", NoWithVetoCount: "0"}, - VotingPeriodEnd: votingPeriodEnd, - ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN, - }, - votes: []*group.Vote{ - { - ProposalId: 1, - Voter: addr1.String(), - Option: group.VOTE_OPTION_YES, - SubmitTime: curCtx.BlockTime(), - }, - }, - expBroken: false, - }, - "proposal tally must correspond to the sum of vote weights": { - groupsInfo: &group.GroupInfo{ - Id: 1, - Admin: adminAddr.String(), - Version: 1, - TotalWeight: "5", - }, - groupPolicy: &group.GroupPolicyInfo{ - Address: addr1.String(), - GroupId: 1, - Admin: adminAddr.String(), - Version: 1, - }, - groupMembers: []*group.GroupMember{ - { - GroupId: 1, - Member: &group.Member{ - Address: addr1.String(), - Weight: "2", - }, - }, - { - GroupId: 1, - Member: &group.Member{ - Address: addr2.String(), - Weight: "3", - }, - }, - }, - proposal: &group.Proposal{ - Id: 1, - Address: addr1.String(), - Proposers: []string{addr1.String()}, - SubmitTime: curCtx.BlockTime(), - GroupVersion: 1, - GroupPolicyVersion: 1, - Status: group.PROPOSAL_STATUS_CLOSED, - Result: group.PROPOSAL_RESULT_ACCEPTED, - FinalTallyResult: group.TallyResult{YesCount: "6", NoCount: "0", AbstainCount: "0", NoWithVetoCount: "0"}, - VotingPeriodEnd: votingPeriodEnd, - ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN, - }, - votes: []*group.Vote{ - { - ProposalId: 1, - Voter: addr1.String(), - Option: group.VOTE_OPTION_YES, - SubmitTime: curCtx.BlockTime(), - }, - { - ProposalId: 1, - Voter: addr2.String(), - Option: group.VOTE_OPTION_YES, - SubmitTime: curCtx.BlockTime(), - }, - }, - expBroken: true, - }, - "proposal FinalTallyResult must correspond to the vote option": { - groupsInfo: &group.GroupInfo{ - Id: 1, - Admin: adminAddr.String(), - Version: 1, - TotalWeight: "7", - }, - groupPolicy: &group.GroupPolicyInfo{ - Address: addr1.String(), - GroupId: 1, - Admin: adminAddr.String(), - Version: 1, - }, - groupMembers: []*group.GroupMember{ - { - GroupId: 1, - Member: &group.Member{ - Address: addr1.String(), - Weight: "4", - }, - }, - { - GroupId: 1, - Member: &group.Member{ - Address: addr2.String(), - Weight: "3", - }, - }, - }, - proposal: &group.Proposal{ - Id: 1, - Address: addr1.String(), - Proposers: []string{addr1.String()}, - SubmitTime: curCtx.BlockTime(), - GroupVersion: 1, - GroupPolicyVersion: 1, - Status: group.PROPOSAL_STATUS_CLOSED, - Result: group.PROPOSAL_RESULT_ACCEPTED, - FinalTallyResult: group.TallyResult{YesCount: "4", NoCount: "3", AbstainCount: "0", NoWithVetoCount: "0"}, - VotingPeriodEnd: votingPeriodEnd, - ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN, - }, - votes: []*group.Vote{ - { - ProposalId: 1, - Voter: addr1.String(), - Option: group.VOTE_OPTION_YES, - SubmitTime: curCtx.BlockTime(), - }, - { - ProposalId: 1, - Voter: addr2.String(), - Option: group.VOTE_OPTION_ABSTAIN, - SubmitTime: curCtx.BlockTime(), - }, - }, - expBroken: true, - }, - } - - for _, spec := range specs { - cacheCurCtx, _ := curCtx.CacheContext() - groupsInfo := spec.groupsInfo - proposal := spec.proposal - groupPolicy := spec.groupPolicy - groupMembers := spec.groupMembers - votes := spec.votes - - _, err := groupTable.Create(cacheCurCtx.KVStore(key), groupsInfo) - s.Require().NoError(err) - - err = groupPolicy.SetDecisionPolicy(group.NewThresholdDecisionPolicy("1", time.Second, 0)) - s.Require().NoError(err) - err = groupPolicyTable.Create(cacheCurCtx.KVStore(key), groupPolicy) - s.Require().NoError(err) - - for i := 0; i < len(groupMembers); i++ { - err = groupMemberTable.Create(cacheCurCtx.KVStore(key), groupMembers[i]) - s.Require().NoError(err) - } - - _, err = proposalTable.Create(cacheCurCtx.KVStore(key), proposal) - s.Require().NoError(err) - - for i := 0; i < len(votes); i++ { - err = voteTable.Create(cacheCurCtx.KVStore(key), votes[i]) - s.Require().NoError(err) - } - - _, broken := keeper.TallyVotesSumInvariantHelper(cacheCurCtx, key, *proposalTable, *groupMemberTable, voteByProposalIndex, *groupPolicyTable) - s.Require().Equal(spec.expBroken, broken) - } -}