docs(consensus): enhance README with comprehensive module documentation #25300
Conversation
📝 WalkthroughWalkthroughDocumentation-only update: expanded x/consensus README from a single-line description to a detailed, governance-focused guide covering parameter categories, usage (MsgUpdateParams), queries, architecture components, and security constraints. No code or API changes. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
x/consensus/README.md (3)
36-38: Add ABCI param example (VoteExtensionsEnableHeight).Since ABCI params are documented above, include them in the usage snippet for completeness.
Validator: &types.ValidatorParams{ PubKeyTypes: []string{"ed25519"}, }, + Abci: &types.ABCIParams{ + VoteExtensionsEnableHeight: 123456, + },Also applies to: 45-61
28-33: Document cross-constraint: Evidence.MaxBytes must not exceed Block.MaxBytes.Prevents invalid configurations.
- `MaxBytes`: Maximum evidence size in bytes + +> Note: Evidence.MaxBytes must be ≤ Block.MaxBytes.
79-82: Grammar nit.- - Updates are subject to governance approval process + - Updates are subject to the governance approval process.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (1)
x/consensus/README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
x/consensus/README.md
[grammar] ~25-~25: There might be a mistake here.
Context: ... MaxBytes: Maximum block size in bytes - MaxGas: Maximum gas per block ### Evidence Pa...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...ocks: Maximum age of evidence in blocks - MaxAgeDuration: Maximum age of evidence in time - Max...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...ration: Maximum age of evidence in time - MaxBytes`: Maximum evidence size in bytes ### Va...
(QB_NEW_EN)
[grammar] ~73-~73: There might be a mistake here.
Context: ...Manages parameter storage and validation - Types: Defines message types and param...
(QB_NEW_EN)
[grammar] ~74-~74: There might be a mistake here.
Context: ...s message types and parameter structures - Module: Integrates with the Cosmos SDK...
(QB_NEW_EN)
[grammar] ~81-~81: There might be a mistake here.
Context: ...efore application - Updates are subject to governance approval process
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
| Authority: "cosmos1...", // governance authority | ||
| Block: &types.BlockParams{ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clarify authority: use the gov module address, not a user address.
Show the canonical value to avoid copy/paste errors.
- Authority: "cosmos1...", // governance authority
+ Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(), // governance authorityAdditional imports for the snippet:
import (
govtypes "cosmossdk.io/x/gov/types"
authtypes "cosmossdk.io/x/auth/types"
)🤖 Prompt for AI Agents
In x/consensus/README.md around lines 47-48, the example Authority value uses a
user address but should use the governance module account address to avoid
copy/paste errors; replace the literal string with the canonical gov module
address obtained from authtypes.NewModuleAddress(govtypes.ModuleName).String(),
and add the recommended imports (govtypes and authtypes) to the snippet so the
example compiles and clearly shows the intended value.
| MaxAgeNumBlocks: 100000, | ||
| MaxAgeDuration: "48h", | ||
| MaxBytes: 1048576, | ||
| }, |
There was a problem hiding this comment.
Use time.Duration for MaxAgeDuration (not a string).
The Go snippet won’t compile as shown; MaxAgeDuration is a duration, not a quoted string.
- MaxAgeDuration: "48h",
+ MaxAgeDuration: 48 * time.Hour,Add import above the snippet:
import "time"🤖 Prompt for AI Agents
In x/consensus/README.md around lines 53 to 56, the example sets MaxAgeDuration
as a quoted string which won't compile because the field expects a
time.Duration; change the value to a time.Duration (e.g. time.Hour*48 or use
time.ParseDuration when demonstrating runtime parsing) and add the time import
at the top of the snippet (import "time") so the example compiles.
| Validator: &types.ValidatorParams{ | ||
| PubKeyTypes: []string{"ed25519", "secp256k1"}, | ||
| }, |
There was a problem hiding this comment.
Remove secp256k1 from Validator PubKeyTypes (validators use ed25519 only).
Including "secp256k1" here is misleading for CometBFT validator keys.
- PubKeyTypes: []string{"ed25519", "secp256k1"},
+ PubKeyTypes: []string{"ed25519"},📝 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.
| Validator: &types.ValidatorParams{ | |
| PubKeyTypes: []string{"ed25519", "secp256k1"}, | |
| }, | |
| Validator: &types.ValidatorParams{ | |
| PubKeyTypes: []string{"ed25519"}, | |
| }, |
🤖 Prompt for AI Agents
In x/consensus/README.md around lines 57 to 59, the Validator PubKeyTypes list
incorrectly includes "secp256k1" even though CometBFT validators use ed25519
only; remove "secp256k1" from the PubKeyTypes array so it contains just
"ed25519" and update any surrounding text if necessary to reflect that
validators accept ed25519 keys only.
| cosmos-sdk query consensus params | ||
| ``` |
There was a problem hiding this comment.
Fix CLI example; “cosmos-sdk” isn’t a binary.
Use the chain’s daemon name.
-cosmos-sdk query consensus params
+<appd> q consensus params📝 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.
| cosmos-sdk query consensus params | |
| ``` | |
| <appd> q consensus params |
🤖 Prompt for AI Agents
In x/consensus/README.md around lines 68-69, the CLI example uses the
non-existent binary "cosmos-sdk"; replace it with the chain daemon binary name
(or a clear placeholder) so the command reads like "<daemon> query consensus
params" (e.g., "gaiad query consensus params" or "YOUR_DAEMON_NAME query
consensus params"). Ensure the README uses a consistent placeholder or the
actual project daemon name and update any adjacent examples to match.
|
@technicallyty to review when he returns |
Improve x/consensus README.md with:
This makes the module more discoverable and easier to understand
for developers working with consensus parameters.
Summary by CodeRabbit