docs: add instructions to change DefaultGenesis #21680
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces significant changes to the handling of 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
|
ziscky
changed the title
![coderabbitai[bot]](https://github.com/avatars/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
docs/build/building-apps/01-app-go-di.md (1)
176-176: Add a blank line before the fenced code block.To adhere to the Markdown formatting best practices, add a blank line before the fenced code block. This improves the readability of the documentation.
Apply this change:
-```go + +```goTools
Markdownlint
176-176: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- docs/build/building-apps/01-app-go-di.md (1 hunks)
Additional context used
Path-based instructions (1)
docs/build/building-apps/01-app-go-di.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Markdownlint
docs/build/building-apps/01-app-go-di.md
176-176: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
Additional comments not posted (5)
docs/build/building-apps/01-app-go-di.md (5)
121-222: Valuable addition to the documentation, but consider moving to a separate page and updating for v0.52.The new content on modifying the
DefaultGenesisparameters for modules provides helpful insights for developers looking to customize their Cosmos SDK applications. The code examples clearly demonstrate the different approaches available.However, as mentioned in the previous review comment, this content may be better suited in a separate documentation page, rather than being included in the "Overview of
app_di.go" section. This would help keep the documentation more focused and organized.Additionally, it's important to ensure that the documentation and code examples are up-to-date with the latest version of the Cosmos SDK (v0.52). Some concepts or APIs mentioned here may have changed or been deprecated in newer versions.
Tools
Markdownlint
176-176: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
129-157: Code example for option 1 looks good.The code example demonstrating how to use a custom
CustomStakingModuleto override theDefaultGenesismethod using theModuleBasicsvariable is clear and easy to understand. It shows how to wrap the existing staking module and provide a custom implementation.
159-167: Code example for option 2 is clear and concise.The code example for option 2 demonstrates how to create a basic module manager from the existing module manager and override the staking module's
AppModuleBasicimplementation. This approach is straightforward and allows for targeted customization of specific modules.
168-174: Code example for option 3 showcases the use ofdepinject.The code example for option 3 illustrates how to use
depinject.Supplyto provide custom module basics. This approach leverages dependency injection to customize the behavior of specific modules. It's a clean and flexible way to extend or modify module functionality.
181-208: Code example for customizingDefaultGenesisusing the new module manager is well-structured.The code example demonstrates how to create a
CustomStakingModulethat embeds thestaking.AppModuleand overrides theDefaultGenesismethod. It shows two options for integrating the custom module:
- Creating a new module manager with the custom staking module.
- Overriding the existing module manager's staking module with the custom one.
Both approaches are clearly explained and the code is properly structured.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- docs/build/building-apps/06-app-go-genesis.md (1 hunks)
Additional context used
Path-based instructions (1)
docs/build/building-apps/06-app-go-genesis.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Markdownlint
docs/build/building-apps/06-app-go-genesis.md
6-6: null
First line in a file should be a top-level heading(MD041, first-line-heading, first-line-h1)
Additional comments not posted (1)
docs/build/building-apps/06-app-go-genesis.md (1)
13-53: LGTM!The Go code example is well-structured and follows best practices. It provides a clear demonstration of how to modify the
DefaultGenesisparameters for the staking module. The two options for overriding the module manager are also helpful for different use cases.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (5)
docs/build/building-apps/06-app-go-genesis.md (5)
1-6: Add a top-level heading to improve document structureTo enhance the structure and readability of the documentation, please add a top-level heading (e.g.,
# Modifying the DefaultGenesis) as the first line after the YAML front matter. This aligns with Markdown best practices and improves the overall document hierarchy.Apply this change after the YAML front matter:
--- sidebar_position: 1 --- +# Modifying the DefaultGenesis ### Modifying the `DefaultGenesis`Tools
Markdownlint
6-6: null
First line in a file should be a top-level heading(MD041, first-line-heading, first-line-h1)
6-8: Enhance the introduction for better context and clarityWhile the introduction provides a brief overview, it could be expanded to offer more context and clarity for developers. Consider adding:
- A brief explanation of what DefaultGenesis parameters are and why one might want to modify them.
- A mention of the specific example (staking module) that will be used in the document.
- A note about the two options that will be presented (for depinject and non-depinject users).
This additional information will help readers better understand the purpose and scope of the document.
Tools
Markdownlint
6-6: null
First line in a file should be a top-level heading(MD041, first-line-heading, first-line-h1)
19-19: Improve comment clarity for DefaultGenesis methodAs suggested in a previous review, please update the comment for better clarity:
-// DefaultGenesis will override the Staking module DefaultGenesis AppModule method. +// DefaultGenesis will override the Staking module DefaultGenesis AppModuleBasic method.This change more accurately reflects that the method is overriding the
AppModuleBasicinterface method.
14-45: Add inline comments to explain key steps in the code exampleTo improve the readability and understanding of the code example, consider adding inline comments to explain key steps and concepts. This will help developers better grasp the process of modifying the DefaultGenesis.
Here are some suggested additions:
// CustomStakingModule wraps the original staking.AppModule and overrides the DefaultGenesis method type CustomStakingModule struct { staking.AppModule cdc codec.Codec } // DefaultGenesis overrides the Staking module DefaultGenesis AppModuleBasic method func (cm CustomStakingModule) DefaultGenesis() json.RawMessage { // Customize the default parameters params := stakingtypes.DefaultParams() params.BondDenom = "mydenom" // Create and return a custom genesis state return cm.cdc.MustMarshalJSON(&stakingtypes.GenesisState{ Params: params, }) } // Option 1: For non-depinject users - create a new module manager moduleManager := module.NewManagerFromMap(map[string]appmodule.AppModule{ stakingtypes.ModuleName: CustomStakingModule{cdc: appCodec, AppModule: staking.NewAppModule(...)}, // other modules ... }) // Option 2: For depinject users - override the existing module manager depinject.Inject( // ... provider/invoker/supplier &moduleManager, ) // Replace the original staking module with the custom one oldStakingModule, _ := moduleManager.Modules()[stakingtypes.ModuleName].(staking.AppModule) moduleManager.Modules()[stakingtypes.ModuleName] = CustomStakingModule{ AppModule: oldStakingModule, cdc: appCodec, }These additional comments will help clarify the purpose of each section and improve the overall understanding of the code.
51-51: Add a conclusion section to summarize key pointsTo enhance the overall structure and reinforce the main concepts, consider adding a conclusion section at the end of the document. This section could:
- Summarize the key steps for modifying the DefaultGenesis.
- Highlight the differences between the depinject and non-depinject approaches.
- Remind readers of the importance of carefully considering changes to default genesis values.
A conclusion will help readers consolidate their understanding and provide a clear takeaway from the document.
Here's a suggested addition:
## Conclusion In this guide, we've explored how to modify the DefaultGenesis parameters for modules in the Cosmos SDK: 1. Create a custom module that wraps the original module and overrides the DefaultGenesis method. 2. Integrate the custom module into your application using either: - A new module manager (for non-depinject users) - Overriding the existing module manager (for depinject users) 3. Replace the original module with the custom one in the module manager. Remember to carefully consider the implications of changing default genesis values, as they can significantly impact your blockchain's initial state and behavior.
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- docs/build/building-apps/06-app-go-genesis.md (1 hunks)
Additional context used
Path-based instructions (1)
docs/build/building-apps/06-app-go-genesis.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Markdownlint
docs/build/building-apps/06-app-go-genesis.md
6-6: null
First line in a file should be a top-level heading(MD041, first-line-heading, first-line-h1)
julienrbrt
added
the
backport/v0.52.x
julienrbrt
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
docs/build/building-apps/06-app-go-genesis.md (4)
6-11: Adjust heading level for consistency.To maintain a proper document structure, change the heading level from H3 to H2. This will create a logical hierarchy with the suggested top-level heading.
-### Modifying the `DefaultGenesis` +## Modifying the `DefaultGenesis`🧰 Tools
Markdownlint
6-6: null
First line in a file should be a top-level heading(MD041, first-line-heading, first-line-h1)
19-19: Improve method override comment.For better clarity, please update the comment as suggested in the previous review:
-// DefaultGenesis will override the Staking module DefaultGenesis AppModuleBasic method. +// DefaultGenesis will override the Staking module DefaultGenesis AppModule method.
29-29: Enhance the description for option 1.To provide more context, consider expanding the comment for option 1:
-// option 1 ( for depinject users ): override previous module manager +// Option 1 (for depinject users): Override the existing module manager +// This approach modifies the injected module manager in-place
41-49: Improve comment formatting and consistency.For better readability and consistency, consider the following changes:
- Format the option 2 comment similar to option 1.
- Capitalize the comment for setting the module manager.
-// option 2 ( for non depinject users ): use new module manager +// Option 2 (for non depinject users): Create a new module manager +// This approach creates a fresh module manager with the custom module // ... (code remains unchanged) -// set the module manager +// Set the module manager app.ModuleManager = moduleManager
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (1)
- docs/build/building-apps/06-app-go-genesis.md (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
docs/build/building-apps/06-app-go-genesis.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Markdownlint
docs/build/building-apps/06-app-go-genesis.md
6-6: null
First line in a file should be a top-level heading(MD041, first-line-heading, first-line-h1)
🔇 Additional comments not posted (1)
docs/build/building-apps/06-app-go-genesis.md (1)
1-49: Overall, the document provides clear and valuable information.This documentation on modifying the DefaultGenesis is well-structured and informative. It provides clear examples for both depinject and non-depinject users, which is very helpful. The suggested improvements will further enhance its readability and consistency.
Great job on creating this comprehensive guide!
🧰 Tools
Markdownlint
6-6: null
First line in a file should be a top-level heading(MD041, first-line-heading, first-line-h1)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
docs/build/building-apps/06-app-go-genesis.md (3)
1-5: Add a top-level (H1) heading after the front matterTo improve the document structure and address the Markdownlint hint, please add an H1 heading after the front matter. This will also make the document more accessible and easier to navigate.
Apply this change:
--- sidebar_position: 1 --- +# Modifying the DefaultGenesis + ### Modifying the `DefaultGenesis`🧰 Tools
Markdownlint
5-5: null
First line in a file should be a top-level heading(MD041, first-line-heading, first-line-h1)
7-9: Enhance the explanatory text for clarityWhile the current explanation covers the main points, it could be more detailed to ensure clarity for all users. Consider expanding on the following:
- Briefly explain what
DefaultGenesisis and why someone might want to modify it.- Clarify the difference between depinject and non-depinject users, and why there are two separate options.
- Provide a brief introduction to each option before the code example.
This will help users better understand the context and choose the appropriate method for their use case.
17-17: Update the comment for clarityAs suggested in a previous review, the comment could be more precise.
Apply this change:
-// DefaultGenesis will override the Staking module DefaultGenesis AppModuleBasic method. +// DefaultGenesis will override the Staking module DefaultGenesis AppModule method.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (1)
- docs/build/building-apps/06-app-go-genesis.md (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
docs/build/building-apps/06-app-go-genesis.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Markdownlint
docs/build/building-apps/06-app-go-genesis.md
5-5: null
First line in a file should be a top-level heading(MD041, first-line-heading, first-line-h1)
| // option 1 ( for depinject users ): override previous module manager | ||
| depinject.Inject( | ||
| // ... provider/invoker/supplier | ||
| &moduleManager, | ||
| ) | ||
|
|
||
| oldStakingModule,_ := moduleManager.Modules()[stakingtypes.ModuleName].(staking.AppModule) | ||
| moduleManager.Modules()[stakingtypes.ModuleName] = CustomStakingModule{ | ||
| AppModule: oldStakingModule, | ||
| cdc: appCodec, | ||
| } | ||
|
|
||
| // option 2 ( for non depinject users ): use new module manager | ||
| moduleManager := module.NewManagerFromMap(map[string]appmodule.AppModule{ | ||
| stakingtypes.ModuleName: CustomStakingModule{cdc: appCodec, AppModule: staking.NewAppModule(...)}, | ||
| // other modules ... | ||
| }) | ||
|
|
||
| // set the module manager | ||
| app.ModuleManager = moduleManager | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve the flow of the integration example
The current example could be restructured to provide a clearer flow of operations. Consider the following suggestions:
- First, show how to create or obtain a module manager.
- Then, demonstrate how to modify it (option 2 for non-depinject users, option 1 for depinject users).
- Finally, show how to set the modified module manager on the app.
Also, for option 1, it's not clear where moduleManager comes from. Consider adding a brief explanation or showing its initialization.
Here's a suggested restructure:
// Option 1: For non-depinject users
moduleManager := module.NewManagerFromMap(map[string]appmodule.AppModule{
stakingtypes.ModuleName: CustomStakingModule{cdc: appCodec, AppModule: staking.NewAppModule(...)},
// other modules ...
})
// Option 2: For depinject users
// Assuming moduleManager is injected
depinject.Inject(
// ... provider/invoker/supplier
&moduleManager,
)
// Modify the module manager (applies to both options)
oldStakingModule, _ := moduleManager.Modules()[stakingtypes.ModuleName].(staking.AppModule)
moduleManager.Modules()[stakingtypes.ModuleName] = CustomStakingModule{
AppModule: oldStakingModule,
cdc: appCodec,
}
// Set the modified module manager on the app
app.ModuleManager = moduleManagerThere was a problem hiding this comment.
Good suggestion from the bot imho @ziscky
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
However, option 1 should still be depinject
(cherry picked from commit 617d547)
* main: docs: amend docs for 52 changes (#21992) test: migrate e2e/authz to system tests (#21819) refactor(runtime/v2): use StoreBuilder (#21989) feat(schema): add API descriptors, struct, oneof & list types, and wire encoding spec (#21482) docs: add instructions to change DefaultGenesis (#21680) feat(x/staking)!: Add metadata field to validator info (#21315) chore(x/authz)!: Remove account keeper dependency (#21632)
* main: docs: amend docs for 52 changes (#21992) test: migrate e2e/authz to system tests (#21819) refactor(runtime/v2): use StoreBuilder (#21989) feat(schema): add API descriptors, struct, oneof & list types, and wire encoding spec (#21482) docs: add instructions to change DefaultGenesis (#21680) feat(x/staking)!: Add metadata field to validator info (#21315) chore(x/authz)!: Remove account keeper dependency (#21632) chore(contributing): delete link (#21990) test(gov): Migrate e2e to system test (#21927) test: e2e/client to system tests (#21981)
Description
Closes: #11008
Add documentation on how to change the default genesis.
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
CustomStakingModulefor enhanced control over the Staking module's behavior.BondDenom.Improvements
These changes enhance the modularity and configurability of the Cosmos SDK application framework, making it easier for developers to tailor module behaviors.