Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Votes sum invariant #330

Merged
merged 148 commits into from
Jun 8, 2021
Merged

Conversation

likhita-809
Copy link
Contributor

@likhita-809 likhita-809 commented Apr 22, 2021

Closes: #326
Add an invariant to verify that proposal tally corresponds to the sum of vote weights and proposal VoteState corresponds to the vote choice

likhita-809 and others added 30 commits February 22, 2021 16:27
Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com>
Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com>
Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com>
Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com>
Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com>
Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com>
Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com>
x/group/server/invariants.go Outdated Show resolved Hide resolved
x/group/server/invariants.go Outdated Show resolved Hide resolved
x/group/server/invariants.go Outdated Show resolved Hide resolved
@blushi
Copy link
Member

blushi commented May 19, 2021

@likhita-809 could you resolve the conflicts?

@blushi
Copy link
Member

blushi commented May 19, 2021

I've been thinking through this invariant a little bit and I believe it might bring some issues in the following case:
Some proposal is created and then there are some votes, the proposal passes and is executed. Later on, the corresponding group member weights are changed, which means that then it will result in a broken invariant while it shouldn't, because the invariant will get the current weights instead of the ones that were used at the time of the proposal.
So I can see several ways to solve this:

  • either we restrict the invariant to proposals where corresponding group account and group haven't changed (same versions as the one saved as part of the Proposal)
  • or we wait until we have some kind of snapshots implemented before going further with this

@likhita-809 likhita-809 reopened this May 19, 2021
x/group/server/invariants.go Outdated Show resolved Hide resolved
x/group/server/invariants.go Outdated Show resolved Hide resolved
x/group/server/invariants.go Outdated Show resolved Hide resolved
x/group/server/invariants.go Outdated Show resolved Hide resolved
x/group/server/invariants.go Outdated Show resolved Hide resolved
x/group/server/invariants.go Outdated Show resolved Hide resolved
x/group/server/invariants.go Outdated Show resolved Hide resolved
x/group/server/invariants.go Outdated Show resolved Hide resolved
x/group/server/invariants.go Outdated Show resolved Hide resolved
@likhita-809 likhita-809 requested a review from blushi May 21, 2021 12:11
x/group/server/invariants.go Outdated Show resolved Hide resolved
@likhita-809 likhita-809 requested a review from blushi June 3, 2021 05:36
Copy link
Member

@blushi blushi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre-approving, pending small nits

x/group/server/invariants.go Outdated Show resolved Hide resolved
x/group/server/invariants_test.go Outdated Show resolved Hide resolved
x/group/server/invariants_test.go Outdated Show resolved Hide resolved
@amaury1093 amaury1093 enabled auto-merge (squash) June 7, 2021 15:05
@amaury1093 amaury1093 merged commit ae66672 into master Jun 8, 2021
@amaury1093 amaury1093 deleted the likhita/proposal-tally-invariant branch June 8, 2021 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add new group invariant to verify that proposal tally corresponds to the sum of votes
5 participants