docs: update bank docs #22053
docs: update bank docs #22053
Conversation
|
Warning Rate limit exceeded@sontrinh16 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 32 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe changes in the pull request focus on updating the Changes
Possibly related PRs
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 (3)
x/bank/README.md (3)
84-89: Enhanced description of supply Keeper interfaceThe changes provide more detailed information about the functionality of the supply Keeper interface, which is valuable for developers working with the bank module.
Consider revising line 85 for better readability:
-The supply `Keeper` interface also introduces new wrapper functions for the auth `Keeper` -and the bank `SendKeeper` in order to be able to: +The supply `Keeper` interface also introduces new wrapper functions for the auth `Keeper` +and the bank `SendKeeper` to:This change removes the redundant phrase "in order to be able to," making the sentence more concise.
🧰 Tools
🪛 LanguageTool
[style] ~85-~85: Consider a shorter alternative to avoid wordiness.
Context: ...authKeeperand the bankSendKeeperin order to be able to: * GetModuleAccounts by ...(IN_ORDER_TO_PREMIUM)
272-275: New information on send restrictionsThe addition of information about clearing send restrictions and their application during SendCoins and InputOutputCoins is valuable. However, there are some grammatical issues that should be addressed.
Consider the following revisions:
-Send restrictions can also be cleared by using `ClearSendRestriction`. +Send restrictions can be cleared using `ClearSendRestriction`. -During `SendCoins`, the send restriction is applied before coins are removed from the `from_address` and adding them to the `to_address`. +During `SendCoins`, the send restriction is applied before coins are removed from the `from_address` and added to the `to_address`. -During `InputOutputCoins`, the send restriction is applied after the input coins are removed, once for each output before the funds are added. +During `InputOutputCoins`, the send restriction is applied after the input coins are removed, once for each output before the funds are added.
281-283: Additional context on send restriction limitationsThe added information about the limitations of send restrictions provides important context for developers. However, there are several grammatical and clarity issues that need to be addressed.
Consider the following revisions:
-Thirdly, this issue could lead into a chain halt if a token is disabled and the token is moved in the begin/endblock. This is the last reason we see the current change as they are more damaging then beneficial for users. +Thirdly, this issue could lead to a chain halt if a token is disabled and the token is moved in the begin/end block. This is the final reason we see the current changes as more beneficial than damaging for users. -A send restriction function should make use of a custom value in the context to allow bypassing that specific restriction. +A send restriction function should use a custom value in the context to allow bypassing that specific restriction.These changes improve grammar, clarity, and correct the use of "then" to "than".
🧰 Tools
🪛 LanguageTool
[grammar] ~281-~281: The word ‘begin’ is not a noun. Did you mean “beginning” or “start”?
Context: ... disabled and the token is moved in the begin/endblock. This is the last reason we se...(PREPOSITION_VERB)
[uncategorized] ~281-~281: Possible missing comma found.
Context: ...s is the last reason we see the current change as they are more damaging then benefici...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~281-~281: “then” (at that time, later on) seems less likely than “than” (as in: greater than).
Context: ...urrent change as they are more damaging then beneficial for users. A send restricti...(AI_HYDRA_LEO_CP_THEN_THAN)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (1)
- x/bank/README.md (20 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
x/bank/README.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 LanguageTool
x/bank/README.md
[style] ~85-~85: Consider a shorter alternative to avoid wordiness.
Context: ...authKeeperand the bankSendKeeperin order to be able to: * GetModuleAccounts by ...(IN_ORDER_TO_PREMIUM)
[uncategorized] ~278-~278: Possible missing comma found.
Context: ...nd user accounts without restrictions. Secondly this limitation would limit the usage o...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~279-~279: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...al is used to get those tokens into the users account this would fall under the discr...(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[uncategorized] ~279-~279: Possible missing comma found.
Context: ...used to get those tokens into the users account this would fall under the discretion of...(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~279-~279: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ... to what they would like to do here. We can not make strong assumptions here. Thirdly,...(CAN_NOT_PREMIUM)
[grammar] ~281-~281: The word ‘begin’ is not a noun. Did you mean “beginning” or “start”?
Context: ... disabled and the token is moved in the begin/endblock. This is the last reason we se...(PREPOSITION_VERB)
[uncategorized] ~281-~281: Possible missing comma found.
Context: ...s is the last reason we see the current change as they are more damaging then benefici...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~281-~281: “then” (at that time, later on) seems less likely than “than” (as in: greater than).
Context: ...urrent change as they are more damaging then beneficial for users. A send restricti...(AI_HYDRA_LEO_CP_THEN_THAN)
[uncategorized] ~336-~336: Possible missing comma found.
Context: ...ut you don't want your send restriction applied you just need to apply custom value in...(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (9)
x/bank/README.md (9)
52-54: Improved clarity in total supply definitionThe changes provide a more precise explanation of what total supply represents in the context of the bank module. This improvement in clarity is beneficial for developers and users alike.
147-149: Improved explanation of denied addressesThe changes provide a clearer explanation of the concept of denied addresses in the bank module. This additional context helps developers understand the purpose and implementation of this feature.
196-197: Updated function signatures for InitGenesis and ExportGenesisThe function signatures for InitGenesis and ExportGenesis have been updated to include error handling and return types. This change improves the robustness of the module by allowing for proper error propagation.
214-214: Updated function signature for BurnCoinsThe BurnCoins function signature has been updated to accept a byte slice for the address instead of an sdk.AccAddress. This change might affect existing implementations.
Please ensure that all calls to BurnCoins throughout the codebase have been updated to match this new signature. You may want to run a search for all occurrences of BurnCoins to verify this change has been propagated correctly.
451-451: Minor formatting improvements in transfer eventsThe changes in the attribute values for transfer events (sender and recipient) improve consistency in the documentation.
Also applies to: 461-461
Line range hint
621-674: New CLI commands for querying balancesThe addition of new CLI commands for querying balances, including spendable balances, greatly enhances the functionality available to users. These commands provide more granular control over balance queries, allowing users to retrieve specific information as needed.
679-702: Updated documentation for denom-metadata commandsThe updates to the documentation for denom-metadata and denoms-metadata commands improve clarity and provide better examples for users.
Line range hint
705-730: New CLI commands for querying total supplyThe introduction of new CLI commands for querying total supply, including the ability to query for specific denominations, adds valuable functionality for users and developers working with the bank module.
747-766: New CLI commands for querying params and denom ownersThe addition of CLI commands for querying params and denom owners further expands the module's querying capabilities, providing users with more tools to inspect the state of the bank module.
julienrbrt
added
the
backport/v0.52.x
…s-sdk into son/check_bank_docs
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
x/bank/README.md (2)
84-89: Improved Keeper interface description with minor suggestionThe updated description of the supply Keeper interface provides more detailed information about its functionality, including new wrapper functions for the auth Keeper and bank SendKeeper. This is a valuable addition to the documentation.
Consider simplifying the phrase "in order to be able to" to "to" for conciseness:
-The supply `Keeper` interface also introduces new wrapper functions for the auth `Keeper` -and the bank `SendKeeper` in order to be able to: +The supply `Keeper` interface also introduces new wrapper functions for the auth `Keeper` +and the bank `SendKeeper` to:🧰 Tools
🪛 LanguageTool
[style] ~85-~85: Consider a shorter alternative to avoid wordiness.
Context: ...authKeeperand the bankSendKeeperin order to be able to: * GetModuleAccounts by ...(IN_ORDER_TO_PREMIUM)
279-282: Important information on send restriction limitations with minor suggestionsThis paragraph provides valuable insights into the design decisions and limitations of send restrictions, particularly for module-to-account and module-to-module transfers. This information is crucial for developers to understand the system's behavior.
Consider the following minor improvements for clarity and correctness:
- Replace "can not" with "cannot" for grammatical correctness.
- Restructure the last sentence for better readability. For example:
-We can not make strong assumptions here. +We cannot make strong assumptions in this case. -Thirdly, this issue could lead into a chain halt if a token is disabled and the token is moved in the begin/endblock. This is the last reason we see the current change as they are more damaging then beneficial for users. +Thirdly, this issue could lead to a chain halt if a disabled token is moved in the begin/end block. This potential for system-wide disruption is the final reason we view the current restrictions as more damaging than beneficial for users.🧰 Tools
🪛 LanguageTool
[style] ~279-~279: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ... to what they would like to do here. We can not make strong assumptions here. Thirdly,...(CAN_NOT_PREMIUM)
[grammar] ~281-~281: The word ‘begin’ is not a noun. Did you mean “beginning” or “start”?
Context: ... disabled and the token is moved in the begin/endblock. This is the last reason we se...(PREPOSITION_VERB)
[misspelling] ~281-~281: Did you mean “than”?
Context: ...urrent change as they are more damaging then beneficial for users. A send restricti...(LESS_MORE_THEN)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (1)
- x/bank/README.md (20 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
x/bank/README.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 LanguageTool
x/bank/README.md
[style] ~85-~85: Consider a shorter alternative to avoid wordiness.
Context: ...authKeeperand the bankSendKeeperin order to be able to: * GetModuleAccounts by ...(IN_ORDER_TO_PREMIUM)
[style] ~279-~279: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ... to what they would like to do here. We can not make strong assumptions here. Thirdly,...(CAN_NOT_PREMIUM)
[grammar] ~281-~281: The word ‘begin’ is not a noun. Did you mean “beginning” or “start”?
Context: ... disabled and the token is moved in the begin/endblock. This is the last reason we se...(PREPOSITION_VERB)
[misspelling] ~281-~281: Did you mean “than”?
Context: ...urrent change as they are more damaging then beneficial for users. A send restricti...(LESS_MORE_THEN)
🔇 Additional comments (8)
x/bank/README.md (8)
13-14: Improved clarity with "legacy vesting accounts"The change from "vesting accounts" to "legacy vesting accounts" provides better specificity and context. This is a good improvement in the documentation.
52-54: Enhanced explanation of total supplyThe expanded description of total supply provides a more comprehensive and accurate explanation. It now clearly states that the total supply includes all accounts within a chain, which is crucial for users to understand the scope of this metric.
196-197: Improved error handling in Genesis methodsThe updates to the
InitGenesisandExportGenesismethod signatures enhance error handling capabilities:
InitGenesisnow returns an error, allowing for proper error propagation.ExportGenesisnow returns both a pointer toGenesisStateand an error, improving state management and error handling.These changes align with Go best practices and allow for more robust implementation of these critical functions.
214-214: Enhanced flexibility in BurnCoins methodThe
BurnCoinsmethod now accepts the address parameter as a byte slice ([]byte) instead of a specific address type. This change offers greater flexibility and potentially improved efficiency when working with addresses at a lower level.
274-275: Valuable clarification on send restriction applicationThis addition provides crucial information about when send restrictions are applied in
SendCoinsandInputOutputCoinsoperations. It clarifies that restrictions are checked before coins are moved, which is essential knowledge for developers implementing or working with these functions.
336-337: Valuable guidance on bypassing send restrictionsThis addition provides essential information for developers on how to bypass send restrictions when necessary. The explanation of using a custom context value offers a flexible solution for cases where restrictions need to be selectively applied. This guidance is particularly useful for implementing more complex transaction logic.
595-599: Critical deprecation notice and migration guidanceThis warning provides essential information about the deprecation of the SendEnabled parameter and offers clear guidance on using the new send_enabled field in genesis. This addition is crucial for developers to understand the changes in the system and maintain backward compatibility while adopting new features.
Line range hint
621-766: Comprehensive expansion of CLI documentationThis extensive addition to the CLI documentation is a significant improvement. It includes:
- New commands for querying various types of balances (including spendable balances).
- Commands for querying denomination metadata and owners.
- Clear examples for each command.
The structure is consistent and easy to follow, making it an excellent resource for users interacting with the bank module via CLI. This comprehensive documentation will greatly assist developers and users in effectively utilizing the bank module's features.
(cherry picked from commit 9251d4e)
Co-authored-by: son trinh <[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
ModuleAccountandKeeperinterfaces, including method changes and new commands.SendEnabledparameter with a warning about backward compatibility.