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

Update group proposal process or create new process for group votes #22

Closed
ryanchristo opened this issue Jul 11, 2023 · 7 comments · Fixed by #38
Closed

Update group proposal process or create new process for group votes #22

ryanchristo opened this issue Jul 11, 2023 · 7 comments · Fixed by #38
Assignees
Labels
dev Engineering task

Comments

@ryanchristo
Copy link
Member

ryanchristo commented Jul 11, 2023

Summary

While working on the group proposals process, we realized we need to also index group votes. When a user views a historical proposal, they should also be able to view who voted, when they voted, and possibly a rationale for their vote.

Proposal

(1) Index group proposal votes at the block height we index the group proposal as part of the same process
(2) Create a separate process for indexing votes that listens to EventVote and always stores the latest vote

Note: The UI also includes the weight of the member that voted but this is neither stored in the proposal or the vote. We may want to reconsider whether this is necessary or revisit in a followup.

@ryanchristo ryanchristo added dev Engineering task nice to have Not required within the greater context labels Jul 11, 2023
@wgwz
Copy link
Contributor

wgwz commented Jul 12, 2023

I spoke with @blushi about this topic in our 1-1 today. We came up with some new ideas on how we could make this work. Suppose that we listen for the EventVote and index those in a table called votes. We could utilize the same code in the indexer codebase for finding those events, as we do with the EventProposalPruned and EventRetire.

@blushi pointed out that in order to handle the case where a vote for a particular proposal is updated by the voter, we could use a primary key on the votes table (proposal_id, voter_address). This would mean that for each proposal we only would store at most one vote per address (because of the implicit unique constraint of that primary key). Furthermore we could use the ON CONFLICT DO UPDATE to update the votes.

We also talked about how we could mark the proposal_id column as a foreign key of the proposals table. This would allow us to write a graphql query that traverses that relationship, so that we get back a list of all the votes associated to each proposal.

@blushi
Copy link
Member

blushi commented Aug 8, 2023

Hey team! Please add your planning poker estimate with Zenhub @wgwz @ryanchristo

@wgwz wgwz self-assigned this Aug 22, 2023
@wgwz
Copy link
Contributor

wgwz commented Aug 23, 2023

Since we use EventProposalPruned for historical proposals, that means the voting period is closed.
Since this task only cares about historical votes, then we really don't need to listen to EventVote.
We only need to fetch state of the votes at the time of EventProposalPruned.
Does this sound right? /cc @ryanchristo
I think @blushi and I were missing this understanding when we spoke about this last.

@blushi
Copy link
Member

blushi commented Aug 28, 2023

Since we use EventProposalPruned for historical proposals, that means the voting period is closed. Since this task only cares about historical votes, then we really don't need to listen to EventVote. We only need to fetch state of the votes at the time of EventProposalPruned. Does this sound right? /cc @ryanchristo I think @blushi and I were missing this understanding when we spoke about this last.

Well it's possible that the votes get pruned before the proposal does, so in order to handle this case, we need to listen to EvenVote I think, see some previous comment about that regen-network/groups-ui#112 (review)

@ryanchristo
Copy link
Member Author

Sorry I missed this comment last week. Votes are pruned if (1) the proposal is aborted or withdrawn (2) votes are final (either all required votes have been cast or the voting period has expired). The block height will be different if the final vote does not also try to execute the proposal (immediately executing the proposal is an option when submitting the vote).

https://github.com/regen-network/cosmos-sdk/blob/main/x/group/keeper/keeper.go#L399

With that in mind, we should be listening for EventVote and checking whether the votes are final and then querying the votes at the block height prior to the votes being pruned. We could also update the votes as the events are emitted as long as we are taking into account that they will need to be updated if the voter changes their vote (i.e. a second event should update the one already stored rather than be ignored or adding a new vote, separate from indexing blocks, txs, msgs, etc).

@ryanchristo
Copy link
Member Author

Not sure if this is something we want to handle in the same process or not, separate probably makes more sense, so I crossed out the first option in the description. Not sure if one process with separate events is worth exploring but still possible. Either approach would be ok (one process that handles proposals and votes or one process for each proposals and votes).

@wgwz
Copy link
Contributor

wgwz commented Aug 28, 2023

@ryanchristo @blushi thanks for the clarifying comments, that helps.

re. the two approaches, i felt that the approach of extending the existing proposal process made sense if we could always reliably record the votes at the same time that EventProposalPruned gets handled. but since we can't do that, i think the separate process makes better sense.

@wgwz wgwz closed this as completed in #38 Sep 12, 2023
@ryanchristo ryanchristo removed the nice to have Not required within the greater context label Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev Engineering task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants