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

docs: Update mint docs #22108

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

docs: Update mint docs #22108

wants to merge 4 commits into from

Conversation

lucaslopezf
Copy link
Contributor

@lucaslopezf lucaslopezf commented Oct 3, 2024

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...

  • included the correct type prefix in the PR title, you can find examples of the prefixes below:
  • confirmed ! in the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • reviewed "Files changed" and left comments if necessary
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • updated the relevant documentation or specification, including comments for documenting Go code
  • confirmed all CI checks have passed

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...

  • confirmed the correct type prefix in the PR title
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

Summary by CodeRabbit

  • Documentation
    • Enhanced clarity and detail regarding the minting mechanism and its parameters.
    • Introduced a new MintFn function for user-defined minting logic, replacing the deprecated InflationCalculationFn.
    • Updated Params section to include a new MaxSupply parameter for controlling token minting limits.
    • Expanded client interaction sections with new commands for querying the mint module via CLI, gRPC, and REST.

Copy link
Contributor

coderabbitai bot commented Oct 3, 2024

Warning

Rate limit exceeded

@lucaslopezf has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 29 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Files that changed from the base of the PR and between b1c21f6 and f9007bf.

📝 Walkthrough
📝 Walkthrough
📝 Walkthrough

Walkthrough

The documentation for the x/mint module has been significantly updated to improve clarity regarding the minting mechanism and its parameters. A new MintFn function has been introduced, replacing the deprecated InflationCalculationFn, allowing for user-defined minting logic. The Params section now includes a MaxSupply parameter. The documentation also refines the calculation sections and expands client interaction commands for better usability.

Changes

File Path Change Summary
x/mint/README.md Updated documentation for minting mechanism, introduced MintFn, added MaxSupply parameter, refined calculations, expanded client commands.
x/mint/mint.go Updated function signature from InflationCalculationFn to MintFn.
x/mint/proto/cosmos/mint/v1beta1/mint.proto Added MaxSupply parameter to Params structure.

Possibly related PRs

  • docs: Update nft docs #22060: The updates to the x/nft module documentation include new sections on "Queries" and "Keeper Functions," which may relate to the overall enhancements in the documentation style and structure similar to the updates made in the x/mint module documentation.
  • docs: Update upgrade docs #22105: The changes in the x/upgrade module documentation include the addition of new CLI commands and REST endpoints, reflecting a similar approach to enhancing usability and clarity in the documentation as seen in the x/mint module updates.
  • docs: Update slashing docs #22107: The updates to the x/slashing module documentation introduce a new command for governance proposals, which aligns with the focus on improving documentation clarity and functionality in the x/mint module.
  • docs(x/consensus): fixes #22114: The minor fixes in the x/consensus module documentation aim to enhance clarity and correct errors, paralleling the comprehensive updates made to the x/mint documentation for improved understanding.

Suggested reviewers

  • akhilkumarpilli
  • julienrbrt
  • tac0turtle

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@lucaslopezf lucaslopezf marked this pull request as ready for review October 3, 2024 15:47
@lucaslopezf lucaslopezf requested review from facundomedica and a team as code owners October 3, 2024 15:47
Copy link
Member

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to refactor much more in the concepts to make the mint function "primary" and put the InflationCalculationFn second and mark it as legacy
Some diagrams need to be changed as well.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (4)
x/mint/README.md (4)

Line range hint 95-125: State section accurately updated with new MaxSupply parameter.

The State section has been correctly updated to include the new MaxSupply parameter in the Params structure. The explanation that a value of '0' indicates an unlimited supply is helpful.

Consider adding a brief explanation of how the MaxSupply parameter affects the minting process, if applicable. This would provide more context on its importance and usage.


Line range hint 127-165: Excellent addition of the Epoch minting section.

The new Epoch minting section provides a clear and comprehensive explanation of the updated minting logic. The introduction of the MintFn function and its flexibility is well-documented. The warning about BeginBlock behavior is crucial for developers.

Consider adding a brief example of how to implement a custom MintFn. This would help developers better understand how to leverage this new flexibility in their applications.


Line range hint 167-252: Block based minting section provides clear continuity with previous versions.

This section effectively explains how block-based minting is still supported, maintaining compatibility with previous versions. The descriptions of the inflation rate calculation and related functions are accurate and helpful.

Consider adding a brief note on when developers might choose block-based minting over epoch-based minting. This would help readers understand the trade-offs between the two approaches.


Line range hint 280-480: Client section comprehensively updated with new commands and endpoints.

The Client section has been thoroughly updated to include new CLI commands, gRPC endpoints, and REST endpoints. The addition of the update-params-proposal command and the inclusion of the MaxSupply parameter in the responses are particularly noteworthy.

Consider adding a brief note on the permissions required to use the update-params-proposal command. This would help users understand who can initiate parameter changes and under what circumstances.

📜 Review details

Configuration used: .coderabbit.yml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between cd2c8ea and 353f62f.

📒 Files selected for processing (1)
  • x/mint/README.md (4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
x/mint/README.md (1)

Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"

🔇 Additional comments (3)
x/mint/README.md (3)

Line range hint 1-93: Excellent updates to the Concepts section!

The additions to the Concepts section, particularly the new "Provisions" subsection, significantly enhance the documentation. The clear explanation of how provisions are calculated and distributed, along with the practical example, greatly improves understanding of the minting process.


Line range hint 254-266: Parameters section accurately updated with MaxSupply.

The Parameters section has been correctly updated to include the new MaxSupply parameter. The note clarifying that a value of '0' indicates unlimited supply is helpful for users.


Line range hint 268-278: Events section remains accurate and complete.

The Events section correctly describes the events emitted by the minting module. No changes were necessary in this section, and it remains consistent with the module's functionality.

@julienrbrt julienrbrt added the backport/v0.52.x PR scheduled for inclusion in the v0.52's next stable release label Oct 3, 2024
@julienrbrt julienrbrt self-assigned this Oct 3, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (6)
x/mint/README.md (6)

53-67: Great addition of the Inflation section!

The new section on Inflation provides clear and valuable information about this key concept in the x/mint module. It effectively explains the dynamic nature of inflation, its boundaries, and how it's calculated.

To further enhance this section, consider adding a brief explanation of how the BondedRatio and GoalBonded values in the inflation formula affect the inflation rate. This would help readers understand the relationship between bonding and inflation more clearly.


Line range hint 77-97: Improved clarity in the Usage per Block section

The renaming of this section to "Usage per Block (default function)" and the updated content provide a clearer explanation of how provisions are calculated and used in each block. The included example is particularly helpful for understanding the concept.

To make this section even more valuable, consider adding a brief note about how this default behavior might differ when using a custom MintFn. This would help readers understand the flexibility of the new minting mechanism.


Line range hint 135-159: Excellent addition of the Minting Methods section

The new Minting Methods section provides a comprehensive overview of the updated minting mechanism, including both epoch-based and block-based minting. The introduction of the MintFn function and its signature is particularly valuable for developers.

To enhance this section further:

  1. Consider adding a brief example of how to implement a custom MintFn for epoch-based minting. This would help developers understand how to leverage this flexibility in practice.
  2. In the warning about BeginBlock, it might be helpful to provide a code snippet showing how to properly handle the epoch ID check to ensure early return when necessary.

162-198: Comprehensive update to the Default configuration section

The updated Default configuration section, particularly the detailed flowchart, provides an excellent visual representation of the DefaultMintFn process. The inclusion of the MaxSupply parameter and its impact on minting is well-explained and aligns with the module updates.

To further improve this section:

  1. Consider adding a brief textual explanation of the key decision points in the flowchart, especially regarding the MaxSupply checks. This would complement the visual representation and ensure clarity for all readers.
  2. It might be helpful to provide a code snippet or pseudocode example of how the MaxSupply adjustment is implemented in the actual code.

Line range hint 200-250: Well-updated Calculations section

The revised Calculations section provides a clear and detailed explanation of the inflation rate calculation, NextAnnualProvisions, and BlockProvision. The inclusion of code snippets is particularly helpful for understanding the implementation details.

To enhance this section further:

  1. Consider adding a brief example of how one might implement a custom InflationCalculationFn. This would help developers understand how to leverage this flexibility in practice.
  2. In the NextInflationRate function, it might be helpful to add comments explaining the purpose of each step in the calculation, especially for the inflation rate change and capping logic.

Line range hint 371-524: Comprehensive update to CLI, gRPC, and REST sections

The CLI, gRPC, and REST sections have been thoroughly updated to reflect the changes in the mint module. The inclusion of new commands and endpoints, along with clear examples, provides valuable guidance for users interacting with the module through various interfaces.

To further enhance these sections:

  1. Consider adding a brief explanation or example of how to use the new MaxSupply parameter in queries or transactions, if applicable.
  2. For the update-params-proposal command, it might be helpful to provide a more comprehensive example that includes multiple parameters, showcasing how to update several parameters at once.
📜 Review details

Configuration used: .coderabbit.yml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 353f62f and b1c21f6.

📒 Files selected for processing (1)
  • x/mint/README.md (11 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
x/mint/README.md (1)

Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"

🪛 LanguageTool
x/mint/README.md

[uncategorized] ~51-~51: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ... is responsible for minting new tokens, implementations details can be found [here](#default-co...

(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)

🔇 Additional comments (1)
x/mint/README.md (1)

Line range hint 260-270: Clear update to the Parameters section

The updated Parameters section effectively incorporates the new MaxSupply parameter, providing a comprehensive overview of all minting module parameters. The table format is clear and easy to understand, with appropriate examples for each parameter.

The note about '0' indicating unlimited supply for the MaxSupply parameter is a crucial piece of information that helps prevent potential misunderstandings.

x/mint/README.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/v0.52.x PR scheduled for inclusion in the v0.52's next stable release C:x/mint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants