docs: add docs on server v2 vote extensions #23010
Conversation
📝 WalkthroughWalkthroughThe pull request updates the documentation for ABCI vote extensions in version 2, focusing on explaining how to implement and use new server options for extending and verifying votes. The documentation provides detailed guidance on handlers for extending votes, verifying vote extensions, preparing and processing proposals, and using the PreBlocker function. The changes illustrate the new approach to handling vote extensions within the CometBFT framework, demonstrating practical implementation examples for developers. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant CometBFT as CometBFT Server
App->>CometBFT: Set ExtendVoteHandler
App->>CometBFT: Set VerifyVoteExtensionHandler
App->>CometBFT: Set PrepareProposalHandler
App->>CometBFT: Set ProcessProposalHandler
CometBFT-->>App: Request Vote Extension
App-->>CometBFT: Provide Vote Extension
CometBFT-->>App: Verify Vote Extension
App-->>CometBFT: Validation Result
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: 1
🧹 Nitpick comments (6)
docs/build/abci/03-vote-extensions.md (6)
124-125: Add context about v2 changesConsider adding a brief introduction explaining the key differences or improvements in v2 compared to v1, helping readers understand what to expect in this section.
134-140: Enhance the example implementationThe current example could be improved to demonstrate best practices:
- Add proper error handling
- Validate inputs
- Show a more realistic implementation instead of hardcoded values
Consider this enhanced example:
func CustomExtendVoteHandler() handlers.ExtendVoteHandler { - return func(ctx context.Context, rm store.ReaderMap, evr *v1.ExtendVoteRequest) (*v1.ExtendVoteResponse, error) { - return &v1.ExtendVoteResponse{ - VoteExtension: []byte("BTC=1234567.89;height=" + fmt.Sprint(evr.Height)), - }, nil + return func(ctx context.Context, rm store.ReaderMap, evr *v1.ExtendVoteRequest) (*v1.ExtendVoteResponse, error) { + if evr == nil { + return nil, fmt.Errorf("extend vote request cannot be nil") + } + + // Fetch real price data from your oracle + price, err := fetchCurrentPrice(ctx) + if err != nil { + return nil, fmt.Errorf("failed to fetch price: %w", err) + } + + voteExt := fmt.Sprintf("BTC=%f;height=%d", price, evr.Height) + return &v1.ExtendVoteResponse{ + VoteExtension: []byte(voteExt), + }, nil + } }
198-198: Fix grammar: Add missing commaAdd a comma after "them" in the sentence.
-The ProcessProposal handler can be used to recover the vote extensions from the first transaction and perform any necessary verification on them. In the example below +The ProcessProposal handler can be used to recover the vote extensions from the first transaction and perform any necessary verification on them, in the example below🧰 Tools
🪛 LanguageTool
[uncategorized] ~198-~198: Possible missing comma found.
Context: ...ry verification on them. In the example below we also use the `cometbft.ValidateVoteE...(AI_HYDRA_LEO_MISSING_COMMA)
260-260: Implement the TODO commentThe TODO comment should be implemented to show how to properly handle vote extensions.
Would you like me to provide an example implementation for handling the vote extensions in the ProcessProposal handler?
270-271: Fix grammatical error: Remove repeated wordRemove the duplicate "is" in the sentence.
-In v2, the `PreBlocker` function works in the same way as it does in v1. However, it is is now passed in as an option to `appbuilder.Build`. +In v2, the `PreBlocker` function works in the same way as it does in v1. However, it is now passed in as an option to `appbuilder.Build`.🧰 Tools
🪛 LanguageTool
[duplication] ~270-~270: Possible typo: you repeated a word
Context: ... same way as it does in v1. However, it is is now passed in as an option to `appbuild...(ENGLISH_WORD_REPEAT_RULE)
282-282: Add newline at end of fileAdd a newline character at the end of the file to follow Markdown best practices.
🧰 Tools
🪛 Markdownlint (0.37.0)
282-282: null
Files should end with a single newline character(MD047, single-trailing-newline)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/build/abci/03-vote-extensions.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
docs/build/abci/03-vote-extensions.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 LanguageTool
docs/build/abci/03-vote-extensions.md
[uncategorized] ~198-~198: Possible missing comma found.
Context: ...ry verification on them. In the example below we also use the `cometbft.ValidateVoteE...
(AI_HYDRA_LEO_MISSING_COMMA)
[duplication] ~270-~270: Possible typo: you repeated a word
Context: ... same way as it does in v1. However, it is is now passed in as an option to `appbuild...
(ENGLISH_WORD_REPEAT_RULE)
🪛 Markdownlint (0.37.0)
docs/build/abci/03-vote-extensions.md
282-282: null
Files should end with a single newline character
(MD047, single-trailing-newline)
| func CustomVerifyVoteExtensionHandler]() handlers.VerifyVoteExtensionHandler { | ||
| return func(context.Context, store.ReaderMap, *abci.VerifyVoteExtensionRequest) (*abci.VerifyVoteExtensionResponse, error) { | ||
| return &abci.VerifyVoteExtensionResponse{}, nil | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add proper verification logic
The current implementation returns an empty response without any verification. This could be misleading as production code should properly validate vote extensions.
Consider this implementation:
-func CustomVerifyVoteExtensionHandler]() handlers.VerifyVoteExtensionHandler {
- return func(context.Context, store.ReaderMap, *abci.VerifyVoteExtensionRequest) (*abci.VerifyVoteExtensionResponse, error) {
- return &abci.VerifyVoteExtensionResponse{}, nil
- }
+func CustomVerifyVoteExtensionHandler() handlers.VerifyVoteExtensionHandler {
+ return func(ctx context.Context, rm store.ReaderMap, req *abci.VerifyVoteExtensionRequest) (*abci.VerifyVoteExtensionResponse, error) {
+ if req == nil || len(req.VoteExtension) == 0 {
+ return nil, fmt.Errorf("invalid vote extension")
+ }
+
+ // Verify the format and data
+ if err := validateVoteExtensionFormat(req.VoteExtension); err != nil {
+ return nil, fmt.Errorf("invalid vote extension format: %w", err)
+ }
+
+ return &abci.VerifyVoteExtensionResponse{}, nil
+ }
}Committable suggestion skipped: line range outside the PR's diff.
julienrbrt
changed the title
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
PreBlockerfunction and its integration with vote extensions.