-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
docs: update authz docs #22062
docs: update authz docs #22062
Conversation
📝 WalkthroughWalkthroughThe changes in this pull request primarily focus on updating the documentation in 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
x/authz/README.md (3)
41-43
: Approved with a minor suggestion for improvementThe changes in this section enhance clarity and align well with the PR objectives. The distinction between the authz and auth modules is now more clearly explained.
Consider revising line 41 for better grammar:
- Authorization is an interface that must be implemented by a concrete authorization logic to validate and execute grants. Authorizations are extensible and can be defined for any Msg service method, even if the Msg method is defined outside of the module. + Authorization is an interface that must be implemented by concrete authorization logic to validate and execute grants. Authorizations are extensible and can be defined for any Msg service method, even if the Msg method is defined outside the module.🧰 Tools
🪛 LanguageTool
[style] ~41-~41: This phrase is redundant. Consider using “outside”.
Context: ...thod, even if the Msg method is defined outside of the module. See theSendAuthorization
...(OUTSIDE_OF)
87-94
: Approved with a minor suggestion for improvementThe updates to the StakeAuthorization section provide more detailed and accurate information, which aligns well with the PR objectives. The SDK version update is correct and consistent with the changes.
Consider revising line 87 for better style:
- `StakeAuthorization` implements the `Authorization` interface for messages in the [staking module](https://docs.cosmos.network/main/build/modules/staking). It takes an `AuthorizationType` to specify whether you want to authorize delegation, undelegation, redelegation or cancel unbonding delegation, each of which must be authorized separately. It also takes an optional `MaxTokens` that keeps track of a limit to the amount of tokens that can be delegated/undelegated/redelegated. + `StakeAuthorization` implements the `Authorization` interface for messages in the [staking module](https://docs.cosmos.network/main/build/modules/staking). It takes an `AuthorizationType` to specify whether you want to authorize delegation, undelegation, redelegation or cancel unbonding delegation, each of which must be authorized separately. It also takes an optional `MaxTokens` that keeps track of a limit to the number of tokens that can be delegated/undelegated/redelegated.This change replaces "amount of tokens" with "number of tokens" for better grammatical accuracy when referring to countable items.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~87-~87: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ...ens` that keeps track of a limit to the amount of tokens that can be delegated/undeleg...(AMOUNTOF_TO_NUMBEROF)
99-101
: Approved with suggestions for minor grammatical improvementsThe updates to the Gas section provide more detailed and specific information about gas costs associated with StakeAuthorization and grant revocation. This information is valuable for developers and aligns with the PR objectives of improving documentation clarity.
Consider the following grammatical improvements:
- In line 99:
- To prevent DoS attacks, granting `StakeAuthorization`s with `x/authz` incurs gas. `StakeAuthorization` allows you to authorize another account to delegate, undelegate, or redelegate tokens to validators. The granter can define a list of validators for which they allow or deny delegations. The Cosmos SDK then iterates over these lists and charge 10 gas for each validator included in both lists. + To prevent DoS attacks, granting `StakeAuthorization`s with `x/authz` incurs gas. `StakeAuthorization` allows you to authorize another account to delegate, undelegate, or redelegate tokens to validators. The granter can define a list of validators for which they allow or deny delegations. The Cosmos SDK then iterates over these lists and charges 10 gas units for each validator included in both lists.
- In line 101:
- Since the state maintains a list of granter-grantee pairs with same expiration, we iterate over this list to remove the grant from the list (in case of any revoke of particular `msgType`), charging 20 gas for each iteration. + Since the state maintains a list of granter-grantee pairs with same expiration, we iterate over this list to remove the grant from the list (in case of any revoke of particular `msgType`), charging 20 gas units for each iteration.These changes ensure grammatical correctness when referring to gas costs.
🧰 Tools
🪛 LanguageTool
[grammar] ~99-~99: After the number ‘10’, use a plural noun. Did you mean “gases”, “gasses”?
Context: ...iterates over these lists and charge 10 gas for each validator included in both lis...(CD_NNU)
[grammar] ~101-~101: After the number ‘20’, use a plural noun. Did you mean “gases”, “gasses”?
Context: ...e of particularmsgType
), charging 20 gas for each iteration. ## State ### Gran...(CD_NNU)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (1)
- x/authz/README.md (9 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
x/authz/README.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 LanguageTool
x/authz/README.md
[style] ~41-~41: This phrase is redundant. Consider using “outside”.
Context: ...thod, even if the Msg method is defined outside of the module. See theSendAuthorization
...(OUTSIDE_OF)
[uncategorized] ~87-~87: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ...ens` that keeps track of a limit to the amount of tokens that can be delegated/undeleg...(AMOUNTOF_TO_NUMBEROF)
[grammar] ~99-~99: After the number ‘10’, use a plural noun. Did you mean “gases”, “gasses”?
Context: ...iterates over these lists and charge 10 gas for each validator included in both lis...(CD_NNU)
[grammar] ~101-~101: After the number ‘20’, use a plural noun. Did you mean “gases”, “gasses”?
Context: ...e of particularmsgType
), charging 20 gas for each iteration. ## State ### Gran...(CD_NNU)
🔇 Additional comments (4)
x/authz/README.md (4)
121-124
: Approved: Improved clarity on grant pruning processThe updates to the GrantQueue section provide more detailed and specific information about the grant pruning process. The addition of the maximum number of grants that can be deleted in each EndBlock run (200) is particularly valuable for developers understanding the system's behavior. This change aligns well with the PR objectives of improving documentation clarity and accuracy.
Line range hint
179-187
: Approved: Clear documentation for new MsgRevokeAll messageThe addition of the MsgRevokeAll section is a valuable update to the documentation. It clearly explains the purpose of this new message type, which allows a granter to revoke all authorizations they have issued. The section also provides important information about the conditions under which the message handling should fail.
This addition aligns well with the PR objectives of updating documentation for new features in version 0.52 of the Cosmos SDK. The explanation is concise yet comprehensive, providing developers with the necessary information to understand and use this new functionality.
317-329
: Approved: Clear documentation for new 'revoke-all' CLI commandThe addition of the 'revoke-all' command documentation in the Transactions section is a valuable update. It clearly explains the purpose of this new CLI command, which allows a granter to revoke all authorizations they have created. The section provides a concise explanation of the command's syntax and includes a practical example of its usage.
This addition aligns perfectly with the PR objectives of updating documentation for new features in version 0.52 of the Cosmos SDK. The explanation is user-friendly and provides CLI users with the necessary information to understand and use this new functionality effectively.
Line range hint
1-365
: Approved: Comprehensive update aligning with PR objectivesThis update to the
x/authz/README.md
file successfully achieves the objectives outlined in the PR. The documentation has been thoroughly revised to reflect the changes in Cosmos SDK version 0.52, including:
- Introduction of new features such as MsgRevokeAll and the 'revoke-all' CLI command.
- Clarification and expansion of existing sections, providing more detailed information about authorizations, gas costs, and the grant pruning process.
- Updates to code references and links to match the new SDK version.
These changes significantly improve the documentation's clarity, accuracy, and completeness. The updates will be valuable for developers working with the authz module in Cosmos SDK v0.52.
While there were a few minor grammatical and stylistic suggestions, the overall quality of the documentation is high and meets the standards expected for the Cosmos SDK.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~87-~87: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ...ens` that keeps track of a limit to the amount of tokens that can be delegated/undeleg...(AMOUNTOF_TO_NUMBEROF)
[grammar] ~99-~99: After the number ‘10’, use a plural noun. Did you mean “gases”, “gasses”?
Context: ...iterates over these lists and charge 10 gas for each validator included in both lis...(CD_NNU)
[grammar] ~101-~101: After the number ‘20’, use a plural noun. Did you mean “gases”, “gasses”?
Context: ...e of particularmsgType
), charging 20 gas for each iteration. ## State ### Gran...(CD_NNU)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
(cherry picked from commit 09be7c7)
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.md
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.
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
revoke-all
command in the CLI for theauthz
module, allowing users to revoke all authorizations created by them.Documentation
x/authz/README.md
.StakeAuthorization
and clarified gas charges associated with authorizations.