docs: update gov docs #22048
docs: update gov docs #22048
Conversation
julienrbrt
added
the
backport/v0.50.x
📝 WalkthroughWalkthroughThe changes introduce enhancements to the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (10)
docs/architecture/adr-069-gov-improvements.md (5)
Line range hint
32-47: LGTM: Clear structure for proposal types with a suggestion.The new
ProposalTypeenum effectively categorizes the different proposal types, including the new multiple choice and optimistic proposals. The reclassification of expedited proposals as a distinct type is a logical improvement.Consider adding a brief comment above each
PROPOSAL_TYPEto explain its purpose, similar to the existing comment forPROPOSAL_TYPE_UNSPECIFIED. This would enhance readability and understanding for developers new to the system.
Line range hint
62-68: LGTM with a typo correction: New parameters for optimistic proposals.The introduction of
optimistic_authorized_addresesandoptimistic_rejected_thresholdprovides necessary control for optimistic proposals.There's a typo in the parameter name. It should be "addresses" instead of "addreses".
-repeated string optimistic_authorized_addreses = 17 [(cosmos_proto.scalar) = "cosmos.AddressString"]; +repeated string optimistic_authorized_addresses = 17 [(cosmos_proto.scalar) = "cosmos.AddressString"];
Line range hint
70-101: LGTM: Clear structure for multiple choice proposals with a suggestion.The introduction of
MsgSubmitMultipleChoiceProposaland the SPAM vote option provides a clear structure for multiple choice proposals. The limitation to text-only proposals is a significant change that aligns with the ADR's objectives.Consider adding a brief explanation in the documentation about why multiple choice proposals cannot have messages to execute. This would help users understand the rationale behind this limitation.
Line range hint
103-156: LGTM: Comprehensive update to the voting system with a suggestion.The introduction of the SPAM vote option, the new
burn_spam_amountparameter, and the aliasing of vote options provide a comprehensive update to the voting system. These changes effectively support multiple choice proposals while maintaining backwards compatibility.Consider adding a brief explanation or example of how the SPAM threshold works in practice. This would help users understand how a proposal is marked as SPAM based on the weighted votes.
Line range hint
170-196: LGTM: Comprehensive overview of consequences and future plans.The document provides a clear overview of the consequences of these changes, including the impact on clients and backwards compatibility. The mention of future improvements shows ongoing commitment to enhancing the governance module.
Consider adding a brief timeline or roadmap for the mentioned future improvements to
x/gov. This would help users and developers anticipate and prepare for upcoming changes.x/gov/README.md (5)
Line range hint
93-131: LGTM: Comprehensive explanation of deposit mechanism with minor suggestionThe deposit section provides a thorough explanation of how deposits are handled in the governance module, including the refund and burn mechanisms. The introduction of new parameters (BurnVoteVeto, BurnVoteQuorum, BurnProposalDepositPrevote) adds clarity to the deposit handling process.
Consider adding commas after each parameter description for better readability:
- For other cases, they are three parameters that define if the deposit of a proposal should be burned or returned to the depositors. + For other cases, there are three parameters that define if the deposit of a proposal should be burned or returned to the depositors: - * `BurnVoteVeto` burns the proposal deposit if the proposal gets vetoed. - * `BurnVoteQuorum` burns the proposal deposit if the vote does not reach quorum. - * `BurnProposalDepositPrevote` burns the proposal deposit if it does not enter the voting phase. + * `BurnVoteVeto` burns the proposal deposit if the proposal gets vetoed, + * `BurnVoteQuorum` burns the proposal deposit if the vote does not reach quorum, + * `BurnProposalDepositPrevote` burns the proposal deposit if it does not enter the voting phase.🧰 Tools
🪛 LanguageTool
[uncategorized] ~128-~128: Possible missing comma found.
Context: ...roposal gets vetoed. *BurnVoteQuorumburns the proposal deposit if the vote does n...(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~129-~129: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... quorum. *BurnProposalDepositPrevoteburns the proposal deposit if it does not ent...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Line range hint
133-219: LGTM: Detailed explanation of voting process with minor suggestionThis section provides a comprehensive overview of the voting process, including clear explanations of participants, voting period, option set, and the introduction of weighted votes. The content is informative and well-structured.
Consider adding a comma in the explanation of the voting period for better clarity:
- The default value of `Voting period` is 2 weeks but is modifiable at genesis or governance. + The default value of `Voting period` is 2 weeks, but is modifiable at genesis or governance.
Line range hint
279-355: LGTM: Introduction of Constitution concept with minor suggestionThe State section briefly mentions the use of collections for state management, which is a good practice. The introduction of the Constitution concept is a significant and valuable addition to the governance module. It provides a way to define the purpose, norms, and expectations for a blockchain, which can be crucial for establishing a clear direction and resolving potential conflicts in the future. The examples provided help illustrate the potential uses of the constitution effectively.
There's a minor grammatical issue in the explanation of the default maximum lengths. Consider the following change:
- The default maximum length are: + The default maximum lengths are:🧰 Tools
🪛 LanguageTool
[grammar] ~351-~351: The verb form ‘are’ does not appear to fit in this context.
Context: ...v/keeper/config`). The default maximum length are: ```go reference https://github.com/co...(SINGULAR_NOUN_VERB_AGREEMENT)
Line range hint
425-470: LGTM: Comprehensive explanation of governance messages with minor suggestionThis section provides clear and detailed explanations of the various message types used in the governance module, including MsgSubmitProposal, MsgSubmitMultipleChoiceProposal, MsgDeposit, and MsgVote. The conditions for accepting deposits and votes are well-defined, which is crucial for understanding the governance process.
Consider adding a comma in the explanation of when deposits are accepted for better clarity:
- Once a proposal is submitted, if `Proposal.TotalDeposit < GovParams.MinDeposit`, Atom holders can send + Once a proposal is submitted, if `Proposal.TotalDeposit < GovParams.MinDeposit`, Atom holders can send🧰 Tools
🪛 LanguageTool
[uncategorized] ~448-~448: Possible missing comma found.
Context: ...alDeposit < GovParams.MinDeposit, Atom holders can sendMsgDeposit` transactions to i...(AI_HYDRA_LEO_MISSING_COMMA)
Line range hint
472-1285: LGTM: Comprehensive documentation with minor suggestionsThe remaining sections (Events, Parameters, Client, and Metadata) provide detailed and valuable information for developers and users of the governance module. The documentation of events, parameters, and client interactions (CLI, gRPC, and REST) is thorough and well-structured. The introduction of a standardized metadata structure for proposals and votes is a significant improvement that promotes consistency across chains.
Consider the following minor improvements:
In the Client section, consider using "can" instead of "are able to" for conciseness:
- bonded Atom holders are able to send `MsgVote` transactions to cast their vote on the proposal. + bonded Atom holders can send `MsgVote` transactions to cast their vote on the proposal.In the Metadata section, add a comma after "By default":
- By default all metadata fields have a 255 character length field + By default, all metadata fields have a 255 character length fieldIn the Vote metadata section, consider adding a hyphen to "255-character":
- Location: on-chain as json within 255 character limit + Location: on-chain as json within 255-character limitThese minor changes will improve the overall readability and consistency of the documentation.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (2)
- docs/architecture/adr-069-gov-improvements.md (1 hunks)
- x/gov/README.md (19 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
docs/architecture/adr-069-gov-improvements.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"x/gov/README.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 LanguageTool
x/gov/README.md
[uncategorized] ~128-~128: Possible missing comma found.
Context: ...roposal gets vetoed. *BurnVoteQuorumburns the proposal deposit if the vote does n...(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~129-~129: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... quorum. *BurnProposalDepositPrevoteburns the proposal deposit if it does not ent...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~148-~148: Possible missing comma found.
Context: ...ault value ofVoting periodis 2 weeks but is modifiable at genesis or governance....(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~255-~255: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ot inherit from the validator's vote. * If the delegator votes after its validator...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~256-~256: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ide its validator vote with its own. If the proposal is urgent, it is possible ...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~351-~351: The verb form ‘are’ does not appear to fit in this context.
Context: ...v/keeper/config`). The default maximum length are: ```go reference https://github.com/co...(SINGULAR_NOUN_VERB_AGREEMENT)
[uncategorized] ~448-~448: Possible missing comma found.
Context: ...alDeposit < GovParams.MinDeposit, Atom holders can sendMsgDeposit` transactions to i...(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~464-~464: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...starts. From there, bonded Atom holders are able to sendMsgVotetransactions to cast the...(BE_ABLE_TO)
[uncategorized] ~585-~585: Did you mean: “By default,”?
Context: ...t the on-chain actions they are taking. By default all metadata fields have a 255 characte...(BY_DEFAULT_COMMA)
[style] ~585-~585: As an alternative to the over-used intensifier ‘quite’, consider replacing this phrase.
Context: ... of proposals made by all groups may be quite large. Second, that client applications such ...(EN_WEAK_ADJECTIVE)
[grammar] ~609-~609: When ‘255-character’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...Vote Location: on-chain as json within 255 character limit (mirrors [group vote](../group/RE...(WORD_ESSAY_HYPHEN)
🔇 Additional comments (7)
docs/architecture/adr-069-gov-improvements.md (2)
9-9: LGTM: Status update is accurate.The status change to "ACCEPTED - Implemented" correctly reflects the current state of this ADR, indicating that the proposed changes have been implemented.
Line range hint
158-168: LGTM: Flexible tallying system.The introduction of the
CalculateVoteResultsAndVotingPowerFntype and the ability to customize the tallying function provide significant flexibility for chains. This change aligns well with the ADR's goal of makingx/govmore composable and extendable.x/gov/README.md (5)
Line range hint
1-24: LGTM: Clear and informative introductionThe introduction and abstract provide a comprehensive overview of the governance module, its features, and functionality. The content is well-structured and accurately describes the module's capabilities.
Line range hint
76-91: LGTM: Comprehensive explanation of proposal submissionThis section provides a clear and detailed explanation of the proposal submission process, including the right to submit proposals and the concept of proposal messages. The warning about the governance module's ability to execute any proposal is particularly important and well-placed.
🧰 Tools
🪛 LanguageTool
[grammar] ~82-~82: Possible subject-verb agreement error detected.
Context: ...self. Modules such asx/upgrade, that want to allow certain messages to be execute...(THIS_THAT_AGR)
[uncategorized] ~82-~82: Possible missing comma found.
Context: ...n messages to be executed by governance only should add a whitelist within the respe...(AI_HYDRA_LEO_MISSING_COMMA)
Line range hint
221-236: LGTM: Clear explanations of quorum concepts and proposal typesThese sections provide concise and clear definitions of various quorum concepts (Quorum, Expedited Quorum, Yes Quorum) and introduce new proposal types (Standard, Expedited, Optimistic, Multiple Choice). The explanations are accurate and highlight the added flexibility in the governance system.
Line range hint
238-273: LGTM: Comprehensive explanation of Threshold and InheritanceThese sections provide clear and detailed explanations of the Threshold concept and the Inheritance mechanism in voting. The information about how delegators inherit votes from validators and the potential implications in urgent proposals is particularly important for users to understand.
The mention of the ability to use a custom tally calculation function is a valuable addition for developers looking to customize the governance module's behavior.
🧰 Tools
🪛 LanguageTool
[style] ~255-~255: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ot inherit from the validator's vote. * If the delegator votes after its validator...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~256-~256: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ide its validator vote with its own. If the proposal is urgent, it is possible ...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
275-277: LGTM: Clear explanation of proposal execution with important additionsThis section provides a concise explanation of how proposals are executed after acceptance. The introduction of the "StatusFailed" status for proposals that fail to execute adds clarity to the possible outcomes of a proposal. Additionally, the mention of an upper limit on gas consumption during execution (ProposalExecutionGas) is an important consideration for developers working with the governance module.
(cherry picked from commit deec679) # Conflicts: # docs/architecture/adr-069-gov-improvements.md # x/gov/README.md
julienrbrt
added
backport/v0.52.x
|
@Mergifyio backport release/v0.52.x |
✅ Backports have been created
|
(cherry picked from commit deec679)
Co-authored-by: Julien Robert <[email protected]>
Description
ref: #21429
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...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers 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.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
Documentation