Committee Gov Module #386
Merged
Committee Gov Module #386
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rhuairahrighairidh
changed the title
denalimarsh
suggested changes
Apr 14, 2020
There was a problem hiding this comment.
Did a first pass and tested the module. Overall, the code is great and doesn't sacrifice robustness or clarity despite the module's complexity. I left some thoughts and a few revisions.
For the open questions:
- Adding no votes/vetos is part of a larger conversation about incentive structures. Until we scope out slashing mechanisms we're already accepting a simplified voting process, so I don't think we need to worry about it until then.
- I like ints but don't have a strong preference. Could also add additional committee field
name(or put a committee's name in the existingdescriptionfield). - Does the version upgrade impact our ability to fork Cosmos-sdk code?
- The tests capture the spirit of the module very well. The test suite is light in terms of line count (and maybe lines tested?) but it validates the core functionality.
karzak
requested changes
Apr 20, 2020
Co-Authored-By: Denali Marsh <denali@kava.io> Co-Authored-By: Kevin Davis <karzak@users.noreply.github.com>
fedekunze
reviewed
Apr 25, 2020
fedekunze
suggested changes
Apr 25, 2020
karzak
approved these changes
Apr 28, 2020
fedekunze
approved these changes
Apr 28, 2020
| type Committee struct { | ||
| ID uint64 `json:"id" yaml:"id"` | ||
| Description string `json:"description" yaml:"description"` | ||
| Members []sdk.AccAddress `json:"members" yaml:"members"` |
There was a problem hiding this comment.
q: how are members added? through regular param change proposals?
There was a problem hiding this comment.
The module has a couple of gov proposals that create/update/delete committees. So its up to the normal gov to add members, change permissions etc.
In theory these proposals could be submitted to committees, allowing them to change themselves or each other, but it's disabled currently.
denalimarsh
approved these changes
Apr 29, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In case something going wrong, we need a fast governance mechanism. This module creates "committees" that can vote on governance proposals quickly to update params, shutdown msg types, etc.
Disabling msg types will be handled by a different module.
See
x/committee/spec/README.mdfor an overview of how committees work.Open Questions