-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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 build/abci
docs
#22067
docs: Update build/abci
docs
#22067
Conversation
📝 WalkthroughWalkthroughThe pull request includes updates to several documentation files related to the ABCI methods in version 2.0 of the CometBFT framework. The modifications clarify the functionality of various processes such as 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
|
There was a problem hiding this 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 (3)
docs/build/abci/02-process-proposal.md (1)
22-23
: Excellent clarification, consider minor wording adjustment.The added clarity regarding the requirements for custom
ProcessProposal
handlers is valuable. The explicit mention of checking both maximum block gas andmaxtxbytes
is an important addition.To further improve readability, consider this minor wording adjustment:
-your own `ProcessProposal` handler, you must ensure that the transactions -provided in the proposal DO NOT exceed the maximum block gas and `maxtxbytes` (if set). +your own `ProcessProposal` handler, you must ensure that the transactions +provided in the proposal do not exceed the maximum block gas and `maxtxbytes` (if set).This change maintains the emphasis while adhering to standard documentation style.
docs/build/abci/00-introduction.md (1)
22-22
: Approve with suggestions for clarity and grammar.The added information about custom handlers is valuable. However, I suggest the following improvements for clarity and grammar:
- Consider rephrasing "in conjunction with" to a more concise alternative.
- Add the article "an" before "application developer".
- Restructure the sentence for better flow.
Here's a suggested revision:
-To perform this manipulation on the application side, a custom handler must be implemented. By default, the Cosmos SDK provides `PrepareProposalHandler`, used in conjunction with an application specific mempool. A custom handler can be written by application developer, if a noop handler provided, all transactions are considered valid. +To perform this manipulation on the application side, a custom handler must be implemented. By default, the Cosmos SDK provides `PrepareProposalHandler`, which works with an application-specific mempool. An application developer can write a custom handler; if a no-op handler is provided, all transactions are considered valid.This revision addresses the wordiness, adds the missing article, and improves the overall sentence structure for better readability.
🧰 Tools
🪛 LanguageTool
[style] ~22-~22: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...providesPrepareProposalHandler
, used in conjunction with an application specific mempool. A cust...(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
[uncategorized] ~22-~22: You might be missing the article “an” here.
Context: ...ool. A custom handler can be written by application developer, if a noop handler provided, ...(AI_EN_LECTOR_MISSING_DETERMINER_AN)
docs/build/abci/03-vote-extensions.md (1)
Line range hint
1-139
: Approve overall changes and suggest minor improvement for clarity.The document has been successfully updated to reflect the changes in the Cosmos SDK version (v0.52.0-beta.1). The content remains informative and well-structured, providing comprehensive information about vote extensions in ABCI++.
To enhance clarity for readers, consider adding a brief note at the beginning of the document mentioning the version update. For example:
# Vote Extensions :::note Synopsis This section describes how the application can define and use vote extensions defined in ABCI++. (Updated for Cosmos SDK v0.52.0-beta.1) :::
This addition would help readers immediately understand the context of the document and its relevance to the current SDK version.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (4)
- docs/build/abci/00-introduction.md (1 hunks)
- docs/build/abci/01-prepare-proposal.md (2 hunks)
- docs/build/abci/02-process-proposal.md (1 hunks)
- docs/build/abci/03-vote-extensions.md (2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
docs/build/abci/00-introduction.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/build/abci/01-prepare-proposal.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/build/abci/02-process-proposal.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"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/00-introduction.md
[style] ~22-~22: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...providesPrepareProposalHandler
, used in conjunction with an application specific mempool. A cust...(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
[uncategorized] ~22-~22: You might be missing the article “an” here.
Context: ...ool. A custom handler can be written by application developer, if a noop handler provided, ...(AI_EN_LECTOR_MISSING_DETERMINER_AN)
🔇 Additional comments (7)
docs/build/abci/02-process-proposal.md (1)
Line range hint
1-37
: Overall improvements to documentation clarity and accuracy.The changes made to this document significantly enhance its quality:
- The updated link to the implementation ensures readers are directed to the most recent version.
- The clarification regarding custom
ProcessProposal
handlers provides valuable guidance for developers.- The overall refinement of the text improves readability and understanding of the
ProcessProposal
functionality.These updates contribute to more accurate and helpful documentation for the Cosmos SDK.
docs/build/abci/01-prepare-proposal.md (4)
Line range hint
1-30
: LGTM: Clear and accurate explanation of PrepareProposalThe updated text provides a comprehensive and clear explanation of the
PrepareProposal
process. It accurately describes the relationship between CometBFT's mempool and the application's control over block construction. The document also correctly mentions the deprecation of the priority mempool, which aligns with recent changes in the Cosmos SDK.
32-32
: LGTM: Updated reference linkThe reference link has been correctly updated to point to the newer version (v0.52.0-beta.1) of the Cosmos SDK repository. This change ensures that users are directed to the most recent and relevant documentation.
Line range hint
36-45
: LGTM: Corrected code snippetThe code snippet has been updated to remove an extra parenthesis from the
SetPrepareProposal
method call. This correction improves the accuracy of the example and ensures that users can copy and use the code without syntax errors.
Line range hint
1-45
: Excellent updates to the PrepareProposal documentationThe changes made to this document significantly improve its clarity and accuracy. The explanation of the
PrepareProposal
process is now more comprehensive, clearly distinguishing between CometBFT's mempool and the application's control over block construction. The mention of the deprecated priority mempool and the updated code references ensure that the documentation remains current with the latest Cosmos SDK developments.These updates will help developers better understand the
PrepareProposal
process and how to implement custom handlers if needed. Great job on improving the documentation!docs/build/abci/03-vote-extensions.md (2)
38-39
: Verify the updated GitHub link and ensure version consistency.The link has been updated to reflect the new version (v0.52.0-beta.1). Please verify that this link is valid and points to the correct documentation.
To maintain consistency, consider adding a version note here as well, similar to the suggestion for the previous link update. For example:
this process MUST be deterministic. The Cosmos SDK defines [`sdk.VerifyVoteExtensionHandler`](https://github.com/cosmos/cosmos-sdk/blob/v0.52.0-beta.1/types/abci.go#L29-L31) (updated for v0.52.0-beta.1):✅ Verification successful
GitHub link is valid and points to the correct version (v0.52.0-beta.1).
The provided link returns a 200 status code, confirming its validity and correctness.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify if the GitHub link is valid curl -s -o /dev/null -w "%{http_code}" https://github.com/cosmos/cosmos-sdk/blob/v0.52.0-beta.1/types/abci.goLength of output: 115
11-12
: Verify the updated GitHub link and consider adding a note about version changes.The link has been updated to reflect the new version (v0.52.0-beta.1). Please ensure that this link is valid and points to the correct documentation.
Consider adding a note about version changes to help users understand the context of the update. For example:
validator process. The Cosmos SDK defines [`baseapp.ExtendVoteHandler`](https://github.com/cosmos/cosmos-sdk/blob/v0.52.0-beta.1/types/abci.go#L26-L27) (updated for v0.52.0-beta.1):✅ Verification successful
GitHub link has been validated successfully.
The updated link points to the correct documentation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify if the GitHub link is valid curl -s -o /dev/null -w "%{http_code}" https://github.com/cosmos/cosmos-sdk/blob/v0.52.0-beta.1/types/abci.goLength of output: 115
@@ -14,12 +14,12 @@ and `ProcessProposal` for the new proposal. | |||
Here is the implementation of the default implementation: | |||
|
|||
```go reference | |||
https://github.com/cosmos/cosmos-sdk/blob/v0.50.0-alpha.0/baseapp/abci_utils.go#L153-L159 | |||
https://github.com/cosmos/cosmos-sdk/blob/v0.52.0-beta.1/baseapp/abci_utils.go#L224-L231 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
The linked implementation does not contain the DefaultProcessProposal
function.
The function DefaultProcessProposal
was not found in the specified lines of abci_utils.go
or anywhere else in the v0.52.0-beta.1
version of the repository. This suggests that the documentation may be outdated or referencing a non-existent implementation.
- Verify if the function has been renamed or relocated in a newer version.
- Update the documentation to reflect the current state of the codebase accurately.
🔗 Analysis chain
Verify the updated implementation link and consider adding a version note.
The link to the implementation has been updated to a newer version, which is good for keeping the documentation current. However, please verify that the linked code still accurately represents the default implementation of ProcessProposal
.
Consider adding a note that the link may change with future SDK versions, to help readers understand that they might need to adjust for their specific version.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the linked code represents the default ProcessProposal implementation
# Test: Check if the linked file and lines exist and contain the expected content
gh api repos/cosmos/cosmos-sdk/contents/baseapp/abci_utils.go?ref=v0.52.0-beta.1 | jq -r '.content' | base64 -d | sed -n '224,231p' | grep -q "func DefaultProcessProposal"
if [ $? -eq 0 ]; then
echo "The linked code appears to contain the default ProcessProposal implementation."
else
echo "The linked code may not accurately represent the default ProcessProposal implementation. Please verify."
fi
Length of output: 480
Script:
#!/bin/bash
# Description: Verify the linked code represents the default ProcessProposal implementation without using base64
# Fetch the raw content of the specified file and lines
gh api repos/cosmos/cosmos-sdk/contents/baseapp/abci_utils.go?ref=v0.52.0-beta.1 \
-H "Accept: application/vnd.github.v3.raw" | sed -n '224,231p' | grep -q "func DefaultProcessProposal"
if [ $? -eq 0 ]; then
echo "The linked code appears to contain the default ProcessProposal implementation."
else
echo "The linked code may not accurately represent the default ProcessProposal implementation. Please verify."
fi
Length of output: 420
Script:
#!/bin/bash
# Description: Search the entire abci_utils.go file for the DefaultProcessProposal function
# Search for the function in the specified file
if gh api repos/cosmos/cosmos-sdk/contents/baseapp/abci_utils.go?ref=v0.52.0-beta.1 \
-H "Accept: application/vnd.github.v3.raw" | grep -q "func DefaultProcessProposal"; then
echo "The function DefaultProcessProposal exists in baseapp/abci_utils.go."
else
echo "The function DefaultProcessProposal was not found in baseapp/abci_utils.go. Searching the entire repository."
# Search the entire repository for the function
gh api repos/cosmos/cosmos-sdk/contents?recursive=true | jq -r '.[]?.path' | grep '.go$' | while read -r file; do
gh api repos/cosmos/cosmos-sdk/contents/"$file"?ref=v0.52.0-beta.1 \
-H "Accept: application/vnd.github.v3.raw" | grep -H "func DefaultProcessProposal" && echo "Found in $file"
done
fi
Length of output: 502
Script:
#!/bin/bash
# Description: Search the entire repository for the DefaultProcessProposal function using ast-grep
ast-grep --lang go --pattern 'func DefaultProcessProposal($_)' .
Length of output: 66
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
@hieuvubk don't forget to assign you to be pages and tick them in the issue, so they don't get reviewed again. |
(cherry picked from commit d1426c7)
Co-authored-by: Hieu Vu <72878483+hieuvubk@users.noreply.github.com>
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...
!
in the type prefix if API or client breaking changeCHANGELOG.md
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...
Summary by CodeRabbit
PrepareProposal
,ProcessProposal
, and vote extensions to enhance user understanding.