docs: pre,begin,end block #22127
Merged
docs: pre,begin,end block #22127
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
julienrbrt
added
the
backport/v0.52.x
📝 WalkthroughWalkthroughThe pull request introduces significant updates to several documentation files related to the Cosmos SDK, focusing on enhancing clarity and organization around module management and lifecycle methods. Key changes include the introduction of the Changes
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: 2
🧹 Outside diff range and nitpick comments (13)
docs/build/building-modules/06-preblock-beginblock-endblock.md (3)
18-27: Approve newPreBlockersection and improve warning clarityThe new section on
PreBlockeris well-written and provides clear information about its functionality and importance. The warning about deprecated consensus parameters is crucial for developers.Consider rephrasing the warning message for improved clarity:
-Modules are required to get the consensus params from the consensus module. Consensus params located in `sdk.Context` were deprecated and should be treated as unsafe. `sdk.Context` is deprecated due to it being a global state within the entire state machine, it has been replaced with `appmodule.Environment`. +Modules must retrieve consensus parameters from the consensus module. Consensus parameters in `sdk.Context` are deprecated and should be considered unsafe. `sdk.Context` is deprecated because it represents a global state within the entire state machine and has been replaced with `appmodule.Environment`.🧰 Tools
🪛 LanguageTool
[style] ~26-~26: This wording can make your sentence hard to follow. Try rephrasing for improved clarity.
Context: ... as unsafe.sdk.Contextis deprecated due to it being a global state within the entire state ...(DUE_TO_BECAUSE)
33-34: Approve addition ofPrepareProposalandProcessProposalinformationThe inclusion of information about
PrepareProposalandProcessProposalis valuable for developers. However, the wording can be slightly improved for clarity.Consider the following rewording for improved clarity:
-In 0.47.0, `PrepareProposal` and `ProcessProposal` were added that allow app developers to do arbitrary work at those phases, but they do not influence the work that will be done in `BeginBlock`, nor are they accessible from modules. +In version 0.47.0, `PrepareProposal` and `ProcessProposal` were introduced, allowing app developers to perform arbitrary work during these phases. However, they do not affect the operations in `BeginBlock` and are not accessible from modules.
47-53: Approve new "Implementation" sectionThe new "Implementation" section is a valuable addition, providing clear guidance on how to implement the
PreBlocker,BeginBlocker, andEndBlockercapabilities. The inclusion of a code reference is particularly helpful for developers.Consider adding a brief explanation of the code reference to provide more context for developers:
+The following code snippet shows the core interface that modules must implement to use these capabilities: ```go reference https://github.com/cosmos/cosmos-sdk/blob/core/v1.0.0-alpha.4/core/appmodule/v2/module.go#L22-L48</blockquote></details> <details> <summary>docs/learn/beginner/04-gas-fees.md (4)</summary><blockquote> Line range hint `1-54`: **Excellent improvements to the introduction and gas meter sections!** The expanded explanations provide a clearer understanding of gas, fees, and the gas meter interface. The added details about the gas meter methods are particularly helpful. Consider adding a brief example of how the gas meter is typically used in practice, to help readers better understand its application. --- `55-57`: **Great clarification on the main gas meter and automatic gas consumption!** The updates provide a more accurate description of how the main gas meter is initialized and used. The explanation of automatic gas consumption through the `GasKv` store is particularly helpful. Consider adding a brief note on best practices for manual gas consumption, to guide developers who might need to implement it in their modules. --- Line range hint `59-89`: **Excellent expansion of the block gas meter section!** The added information about the genesis phase, post-genesis behavior, and the mechanism for setting the block gas meter with a finite limit greatly improves the documentation. The explanation of how gas limits are enforced on a per-block basis is particularly valuable. Consider adding a brief explanation of how the block gas limit is determined or where it's configured, to provide a complete picture of the block gas meter's lifecycle. --- Line range hint `91-124`: **Great improvements to the AnteHandler section!** The refinements in the explanations of transaction type verification, signature verification, and gas price checking enhance the clarity of the documentation. The overall structure of the AnteHandler's responsibilities is now more comprehensible. Consider adding a brief note on how custom AnteHandlers can be implemented or extended, to guide developers who might need to modify the default behavior for their specific applications. </blockquote></details> <details> <summary>docs/build/building-modules/01-module-manager.md (2)</summary><blockquote> `246-246`: **Correct grammatical errors in the `BeginBlock` description.** There are two minor grammatical issues in this line: 1. Change "function of each modules" to "function of each module" 2. Change "emitted from each modules" to "emitted from all modules" These corrections will improve the readability and accuracy of the documentation. <details> <summary>🧰 Tools</summary> <details> <summary>🪛 LanguageTool</summary><blockquote> [uncategorized] ~246-~246: Loose punctuation mark. Context: ... `BeginBlock(ctx context.Context) error`: At the beginning of each block, this fu... (UNLIKELY_OPENING_PUNCTUATION) --- [grammar] ~246-~246: Use a singular noun after the quantifier ‘each’, or change it to “all”. Context: ...ock-beginblock-endblock.md) function of each modules implementing the `appmodule.HasBeginBlo... (EACH_EVERY_NNS) --- [grammar] ~246-~246: The noun should probably be in the singular form. Context: ...dvanced/08-events.md) emitted from each modules. * `EndBlock(ctx context.Context) error... (EVERY_EACH_SINGULAR) </blockquote></details> </details> --- `247-247`: **Correct grammatical error in the `EndBlock` description.** There is a minor grammatical issue in this line: 1. Change "function of each modules" to "function of each module" This correction will maintain consistency with the previous correction and improve the overall quality of the documentation. <details> <summary>🧰 Tools</summary> <details> <summary>🪛 LanguageTool</summary><blockquote> [uncategorized] ~247-~247: Loose punctuation mark. Context: ... * `EndBlock(ctx context.Context) error`: At the end of each block, this function... (UNLIKELY_OPENING_PUNCTUATION) --- [grammar] ~247-~247: Use a singular noun after the quantifier ‘each’, or change it to “all”. Context: ...ock-beginblock-endblock.md) function of each modules implementing the `appmodule.HasEndBlock... (EACH_EVERY_NNS) </blockquote></details> </details> </blockquote></details> <details> <summary>docs/learn/beginner/00-app-anatomy.md (1)</summary><blockquote> Line range hint `324-324`: **Suggestion for additional resources** While the document is comprehensive, it might be helpful to include a section or links to additional resources or tutorials for readers who want to dive deeper into specific topics. Consider adding a "Further Reading" or "Additional Resources" section at the end of the document with links to more detailed documentation or tutorials on specific components of a Cosmos SDK application. </blockquote></details> <details> <summary>docs/learn/advanced/00-baseapp.md (3)</summary><blockquote> Line range hint `184-196`: **Consider expanding the constructor section** The constructor section provides a good overview of the NewBaseApp function. To make it even more helpful for developers, consider: 1. Adding examples of common options used with NewBaseApp. 2. Providing a link to documentation detailing all available options. This additional information would give developers a more comprehensive understanding of BaseApp initialization. --- Line range hint `198-291`: **Add referenced visual aids to enhance understanding** The state updates section provides a comprehensive explanation of the different states in BaseApp and how they are updated during various phases of block processing. However, the document mentions visual aids that are not included in the markdown. To improve this section: 1. Add the referenced visual aids (diagrams or flowcharts) to illustrate the state update processes. 2. Ensure that each visual aid is properly captioned and referenced in the text. Including these visual elements will significantly enhance the reader's understanding of the complex state management processes in BaseApp. --- Line range hint `293-304`: **Expand on the x/consensus module's role** The ParamStore section effectively explains its purpose in managing consensus parameters. To provide more context and help developers understand the bigger picture: 1. Add a brief explanation of the x/consensus module and its relationship to ParamStore. 2. Include a link to the x/consensus module documentation for readers who want to dive deeper. This additional information will help developers better understand how ParamStore fits into the overall architecture of a Cosmos SDK application. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: .coderabbit.yml** **Review profile: CHILL** <details> <summary>📥 Commits</summary> Files that changed from the base of the PR and between dc1496029acaffbf52bc7eb71544f1478c7dcdd7 and f50c815f9e7cc6713289bbf647fda4603d4c1cdf. </details> <details> <summary>📒 Files selected for processing (6)</summary> * docs/build/building-modules/01-module-manager.md (1 hunks) * docs/build/building-modules/06-preblock-beginblock-endblock.md (3 hunks) * docs/build/building-modules/17-preblock.md (0 hunks) * docs/learn/advanced/00-baseapp.md (3 hunks) * docs/learn/beginner/00-app-anatomy.md (1 hunks) * docs/learn/beginner/04-gas-fees.md (1 hunks) </details> <details> <summary>💤 Files with no reviewable changes (1)</summary> * docs/build/building-modules/17-preblock.md </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>📓 Path-based instructions (5)</summary><blockquote> <details> <summary>docs/build/building-modules/01-module-manager.md (1)</summary> Pattern `**/*.md`: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness" </details> <details> <summary>docs/build/building-modules/06-preblock-beginblock-endblock.md (1)</summary> Pattern `**/*.md`: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness" </details> <details> <summary>docs/learn/advanced/00-baseapp.md (1)</summary> Pattern `**/*.md`: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness" </details> <details> <summary>docs/learn/beginner/00-app-anatomy.md (1)</summary> Pattern `**/*.md`: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness" </details> <details> <summary>docs/learn/beginner/04-gas-fees.md (1)</summary> Pattern `**/*.md`: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness" </details> </blockquote></details> <details> <summary>🪛 LanguageTool</summary><blockquote> <details> <summary>docs/build/building-modules/01-module-manager.md</summary><blockquote> [uncategorized] ~246-~246: Loose punctuation mark. Context: ... `BeginBlock(ctx context.Context) error`: At the beginning of each block, this fu... (UNLIKELY_OPENING_PUNCTUATION) --- [grammar] ~246-~246: Use a singular noun after the quantifier ‘each’, or change it to “all”. Context: ...ock-beginblock-endblock.md) function of each modules implementing the `appmodule.HasBeginBlo... (EACH_EVERY_NNS) --- [grammar] ~246-~246: The noun should probably be in the singular form. Context: ...dvanced/08-events.md) emitted from each modules. * `EndBlock(ctx context.Context) error... (EVERY_EACH_SINGULAR) --- [uncategorized] ~247-~247: Loose punctuation mark. Context: ... * `EndBlock(ctx context.Context) error`: At the end of each block, this function... (UNLIKELY_OPENING_PUNCTUATION) --- [grammar] ~247-~247: Use a singular noun after the quantifier ‘each’, or change it to “all”. Context: ...ock-beginblock-endblock.md) function of each modules implementing the `appmodule.HasEndBlock... (EACH_EVERY_NNS) --- [uncategorized] ~248-~248: Loose punctuation mark. Context: ...ontext) ([]abci.ValidatorUpdate, error)`: At the end of each block, this function... (UNLIKELY_OPENING_PUNCTUATION) --- [grammar] ~248-~248: Use a singular noun after the quantifier ‘each’, or change it to “all”. Context: ...ock-beginblock-endblock.md) function of each modules implementing the `module.HasABCIEndBloc... (EACH_EVERY_NNS) </blockquote></details> <details> <summary>docs/build/building-modules/06-preblock-beginblock-endblock.md</summary><blockquote> [style] ~26-~26: This wording can make your sentence hard to follow. Try rephrasing for improved clarity. Context: ... as unsafe. `sdk.Context` is deprecated due to it being a global state within the entire state ... (DUE_TO_BECAUSE) </blockquote></details> </blockquote></details> </details> <details> <summary>🔇 Additional comments (9)</summary><blockquote> <details> <summary>docs/build/building-modules/06-preblock-beginblock-endblock.md (1)</summary><blockquote> `43-43`: **Approve update on validator changes implementation** The updated information about implementing custom validator changes is accurate and provides valuable guidance for developers. The reference to v1 adds appropriate context for versioning. </blockquote></details> <details> <summary>docs/learn/beginner/04-gas-fees.md (1)</summary><blockquote> Line range hint `126-128`: **Clear explanation of GasWanted and GasUsed!** The retained explanation, with minor adjustments, provides a concise and clear understanding of how gas limits are set and reported. The distinction between GasWanted and GasUsed is well-articulated, which is crucial for developers to understand the gas consumption model. </blockquote></details> <details> <summary>docs/learn/beginner/00-app-anatomy.md (4)</summary><blockquote> `139-139`: **Improved clarity on BeginBlocker and EndBlocker composition** The added sentence provides a clearer explanation of how BeginBlocker and EndBlocker functions are composed, emphasizing that they primarily consist of the respective functions from each module. This addition enhances the reader's understanding of the application's structure. --- Line range hint `141-143`: **Important note on determinism and computational cost** The added note about determinism and computational cost in BeginBlocker and EndBlocker is crucial. It warns developers about potential pitfalls that could affect the blockchain's consistency and performance. --- Line range hint `4-324`: **Overall document structure and content** The document provides a comprehensive overview of the anatomy of a Cosmos SDK application. It covers key components such as the Node Client, Core Application File, Modules, and Application Interface. The explanations are clear and well-structured, with appropriate use of code examples and references to external resources. --- Line range hint `67-86`: **Clarification on PreBlocker method** The addition of the PreBlocker section provides important information about this new lifecycle method. It clearly explains its purpose, timing, and the potential impact on consensus parameters. This is valuable information for developers working with the latest version of the Cosmos SDK. </blockquote></details> <details> <summary>docs/learn/advanced/00-baseapp.md (3)</summary><blockquote> Line range hint `1-182`: **LGTM: Comprehensive introduction and type definition** The introduction and type definition sections provide a clear and thorough explanation of BaseApp, its core functionalities, and its important components. The content is well-structured and informative, offering developers a solid foundation for understanding the BaseApp abstraction in Cosmos SDK applications. --- Line range hint `306-1000`: **LGTM: Comprehensive coverage of BaseApp components** The remaining sections of the document, covering Service Routers, ABCI messages, and other BaseApp components, are exceptionally thorough and well-explained. Key points: 1. Service Routers section clearly explains both Msg and gRPC Query routers. 2. ABCI message handlers are detailed, including updates for ABCI 2.0. 3. Code references and in-depth explanations provide valuable insights into BaseApp's functionality. This comprehensive coverage will be extremely helpful for developers working with Cosmos SDK applications. The document successfully balances depth of information with clarity of explanation. --- Line range hint `1-1000`: **Excellent documentation with minor enhancement opportunities** This document on BaseApp is comprehensive, well-structured, and provides valuable insights into the core of Cosmos SDK applications. The explanations are clear, accurate, and include helpful code references. To further enhance this already strong documentation: 1. Add the mentioned visual aids in the state updates section. 2. Expand on the constructor options and x/consensus module as suggested. 3. Consider adding a summary or quick reference guide at the end for easy lookup of key concepts. These minor enhancements will elevate an already excellent document, making it an even more valuable resource for developers working with Cosmos SDK. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
+8
to
+9
| `PreBlocker`, `BeginBlocker` and `EndBlocker` are optional methods module developers can implement in their module. | ||
| They will be triggered at the beginning and at the end of each block respectively, when the [`BeginBlock`](../../learn/advanced/00-baseapp.md#beginblock) and [`EndBlock`](../../learn/advanced/00-baseapp.md#endblock) inside within ABCI `FinalizeBlock` |
There was a problem hiding this comment.
Approve addition of PreBlocker and fix grammatical issue
The inclusion of PreBlocker in the synopsis is correct and aligns with the changes. However, there's a minor grammatical issue in line 9.
Please apply the following change to fix the grammatical issue:
-They will be triggered at the beginning and at the end of each block respectively, when the [`BeginBlock`](../../learn/advanced/00-baseapp.md#beginblock) and [`EndBlock`](../../learn/advanced/00-baseapp.md#endblock) inside within ABCI `FinalizeBlock`
+They will be triggered at the beginning and at the end of each block respectively, when the [`BeginBlock`](../../learn/advanced/00-baseapp.md#beginblock) and [`EndBlock`](../../learn/advanced/00-baseapp.md#endblock) are called within ABCI `FinalizeBlock`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| `PreBlocker`, `BeginBlocker` and `EndBlocker` are optional methods module developers can implement in their module. | |
| They will be triggered at the beginning and at the end of each block respectively, when the [`BeginBlock`](../../learn/advanced/00-baseapp.md#beginblock) and [`EndBlock`](../../learn/advanced/00-baseapp.md#endblock) inside within ABCI `FinalizeBlock` | |
| `PreBlocker`, `BeginBlocker` and `EndBlocker` are optional methods module developers can implement in their module. | |
| They will be triggered at the beginning and at the end of each block respectively, when the [`BeginBlock`](../../learn/advanced/00-baseapp.md#beginblock) and [`EndBlock`](../../learn/advanced/00-baseapp.md#endblock) are called within ABCI `FinalizeBlock` |
| * `EndBlock(context.Context) ([]abci.ValidatorUpdate, error)`: At the end of each block, this function is called from [`BaseApp`](../../learn/advanced/00-baseapp.md#endblock) and, in turn, calls the [`EndBlock`](./06-beginblock-endblock.md) function of each modules implementing the `module.HasABCIEndBlock` interface, in the order defined in `OrderEndBlockers`. It creates a child [context](../../learn/advanced/02-context.md) with an event manager to aggregate [events](../../learn/advanced/08-events.md) emitted from all modules. The function returns an `abci` which contains the aforementioned events, as well as validator set updates (if any). | ||
| * `BeginBlock(ctx context.Context) error`: At the beginning of each block, this function is called from [`BaseApp`](../../learn/advanced/00-baseapp.md#beginblock) and, in turn, calls the [`BeginBlock`](./06-preblock-beginblock-endblock.md) function of each modules implementing the `appmodule.HasBeginBlocker` interface, in the order defined in `OrderBeginBlockers`. It creates a child [context](../../learn/advanced/02-context.md) with an event manager to aggregate [events](../../learn/advanced/08-events.md) emitted from each modules. | ||
| * `EndBlock(ctx context.Context) error`: At the end of each block, this function is called from [`BaseApp`](../../learn/advanced/00-baseapp.md#endblock) and, in turn, calls the [`EndBlock`](./06-preblock-beginblock-endblock.md) function of each modules implementing the `appmodule.HasEndBlocker` interface, in the order defined in `OrderEndBlockers`. It creates a child [context](../../learn/advanced/02-context.md) with an event manager to aggregate [events](../../learn/advanced/08-events.md) emitted from all modules. The function returns an `abci` which contains the aforementioned events, as well as validator set updates (if any). | ||
| * `EndBlock(context.Context) ([]abci.ValidatorUpdate, error)`: At the end of each block, this function is called from [`BaseApp`](../../learn/advanced/00-baseapp.md#endblock) and, in turn, calls the [`EndBlock`](./06-preblock-beginblock-endblock.md) function of each modules implementing the `module.HasABCIEndBlock` interface, in the order defined in `OrderEndBlockers`. It creates a child [context](../../learn/advanced/02-context.md) with an event manager to aggregate [events](../../learn/advanced/08-events.md) emitted from all modules. The function returns an `abci` which contains the aforementioned events, as well as validator set updates (if any). |
There was a problem hiding this comment.
Consolidate EndBlock descriptions and correct grammatical error.
This line appears to be redundant with the previous EndBlock description (line 247). Consider merging these two descriptions to avoid confusion. If there are important differences between the two EndBlock functions, please clarify them in a single, comprehensive description.
Additionally, there is a minor grammatical issue:
- Change "function of each modules" to "function of each module"
By consolidating the EndBlock descriptions and fixing the grammatical error, the documentation will be more concise and clear.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~248-~248: Loose punctuation mark.
Context: ...ontext) ([]abci.ValidatorUpdate, error)`: At the end of each block, this function...(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~248-~248: Use a singular noun after the quantifier ‘each’, or change it to “all”.
Context: ...ock-beginblock-endblock.md) function of each modules implementing the `module.HasABCIEndBloc...(EACH_EVERY_NNS)
tac0turtle
approved these changes
Oct 4, 2024
akhilkumarpilli
approved these changes
Oct 4, 2024
mergify bot
pushed a commit
that referenced
this pull request
Oct 4, 2024
(cherry picked from commit 080ff34)
12 tasks
julienrbrt
pushed a commit
that referenced
this pull request
Oct 4, 2024
This was referenced Oct 5, 2024
12 tasks
12 tasks
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.
Description
Closes: #XXXX
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
PreBlockerlifecycle method for module developers.BaseAppdocumentation to clarify its functionalities and lifecycle methods.00-app-anatomy.mddocument for better understanding of application structure and lifecycle.04-gas-fees.mddocument.