-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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 some learn/advanced #22255
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request includes extensive updates to documentation files related to the Cosmos SDK, specifically focusing on the Changes
Possibly related PRs
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: 3
🧹 Outside diff range and nitpick comments (14)
docs/learn/advanced/03-node.md (3)
35-35
: LGTM: Updated CometBFT link, consider adding a note about version changeThe link has been correctly updated to reflect the new version (v1.0.0-rc1) of CometBFT. This is a significant version change from v0.37.0.
Consider adding a brief note in the surrounding text to highlight this major version update of CometBFT, as it might have implications for developers using this documentation.
63-81
: LGTM: Updated multiple SDK links, consider adding version change notesAll links have been correctly updated to reflect the new versions of the Cosmos SDK (v0.52.0-beta.2 or v0.52.0-beta.1). The line number changes suggest some code restructuring in the new version.
To improve clarity for readers:
- Consider adding a brief note at the beginning of this section mentioning that the code references are for SDK version v0.52.0-beta.2 (or the latest stable version if this changes).
- If there are any significant changes in the referenced code that affect the explanation, consider updating the surrounding text to reflect these changes.
Inconsistent SDK Version References
The
docs/learn/advanced/03-node.md
file contains inconsistent SDK version references. Specifically, line 75 referencesv0.52.0-beta.1
while other links usev0.52.0-beta.2
. Please update the version in line 75 to maintain consistency.🔗 Analysis chain
Line range hint
1-95
: Overall: Documentation updated for newer SDK version, consider reviewing contentThe document has been successfully updated with new version numbers and links to the latest Cosmos SDK (v0.52.0-beta.2) and CometBFT (v1.0.0-rc1) repositories. These updates ensure that readers are referencing the most current code.
To ensure the documentation remains accurate and helpful:
- Please review the referenced code in the new SDK and CometBFT versions to confirm that the explanations in this document are still accurate and complete.
- Consider adding a note at the beginning of the document stating the SDK and CometBFT versions that this documentation refers to.
- If there have been any significant changes in the functionality or structure of the referenced code, update the explanations accordingly.
To verify the consistency of version references throughout the document, you can run the following script:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent version references in the document # Test: Check for consistent SDK version references echo "Checking SDK version references:" grep -n "cosmos-sdk/blob/v" docs/learn/advanced/03-node.md # Test: Check for consistent CometBFT version references echo "Checking CometBFT version references:" grep -n "cometbft/blob/v" docs/learn/advanced/03-node.md # Note: The output should show consistent version numbers. Any discrepancies should be addressed.Length of output: 947
🧰 Tools
🪛 LanguageTool
[style] ~92-~92: Consider a shorter alternative to avoid wordiness.
Context: ... will query all the blocks sequentially in order to catch up. Then, it will wait for new bl...(IN_ORDER_TO_PREMIUM)
[style] ~92-~92: Consider a shorter alternative to avoid wordiness.
Context: ...ls and block signatures from validators in order to make progress. ## Other commands <!-- ...(IN_ORDER_TO_PREMIUM)
docs/learn/advanced/02-context.md (3)
35-36
: Clarify the deprecation of CheckTx Mode and introduction of execMode.The documentation has been updated to reflect changes in how transaction execution modes are handled:
- The CheckTx Mode is now marked as deprecated.
- A new field,
execMode
, has been introduced to replace it.To improve clarity and completeness:
- Consider adding a brief explanation of why CheckTx Mode is deprecated and how
execMode
improves upon it.- Provide examples of the possible values for
execMode
and their meanings.- If applicable, mention any migration steps or considerations for developers using the old CheckTx Mode.
Line range hint
50-50
: Enhance the description of the CometInfo field.The addition of the CometInfo field is a valuable update to the Context structure. To further improve the documentation:
- Consider expanding on how CometInfo relates to or differs from the existing Header field.
- Provide a brief example of how developers might use CometInfo in their applications.
- If applicable, mention any performance considerations or best practices when using CometInfo.
These additions would help developers better understand the significance and proper usage of this new field within the Cosmos SDK ecosystem.
Line range hint
51-51
: Expand on the purpose and usage of the HeaderInfo field.The addition of the HeaderInfo field enhances the Context structure with more specific header-related information. To improve the documentation:
- Clarify how HeaderInfo differs from or complements the existing Header field.
- Provide examples of scenarios where developers would specifically use HeaderInfo over other fields.
- If applicable, mention any performance implications or best practices when working with HeaderInfo.
These additions would provide developers with a clearer understanding of when and how to utilize this new field effectively within their Cosmos SDK applications.
docs/learn/advanced/04-store.md (1)
218-218
: Suggestion: Add a comma after "Otherwise"To improve readability and adhere to proper grammar, consider adding a comma after "Otherwise" in the following sentence:
-Otherwise it uses the following default: +Otherwise, it uses the following default:This small change will enhance the clarity of the documentation.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~218-~218: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...thKVGasConfigmethod of the
context`. Otherwise it uses the following default: ```go r...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
docs/learn/advanced/01-transactions.md (4)
28-39
: LGTM! Comprehensive update to the Tx interface description.The updated link and additional method descriptions provide a more complete overview of the Tx interface. This change enhances the documentation's clarity and usefulness for developers.
Consider adding a brief explanation of when or why a developer might need to use each of these methods, especially for the newly added ones like
Hash()
,GetSenders()
, andGetGasLimit()
. This could provide more context and help developers understand when to utilize these methods in their applications.
Line range hint
56-112
: Great addition of new sign modes and updated references!The introduction of SIGN_MODE_DIRECT_AUX and SIGN_MODE_TEXTUAL, along with their explanations, significantly enhances the documentation. This update provides developers with more options and better understanding of transaction signing in different scenarios.
To improve clarity, consider adding a brief comparison table or summary that highlights the key differences and use cases for each sign mode (SIGN_MODE_DIRECT, SIGN_MODE_LEGACY_AMINO_JSON, SIGN_MODE_DIRECT_AUX, and SIGN_MODE_TEXTUAL). This would help developers quickly understand which mode is most appropriate for their specific use case.
Line range hint
173-214
: Well-organized overview of transaction broadcasting methods!The updated link to the gRPC service proto and the external documentation links ensure that readers have access to the most current information. The section provides a comprehensive overview of the different methods for broadcasting transactions.
To further enhance this section, consider adding brief code snippets or command examples for each broadcasting method (CLI, gRPC, REST, and CometBFT RPC). This would provide readers with quick, practical references for implementing each method.
Line range hint
1-214
: Excellent update to the Transactions documentation!This comprehensive update significantly improves the Transactions documentation for the Cosmos SDK. Key improvements include:
- Updated references to the latest SDK version (v0.52.0-beta.2).
- Introduction of new sign modes (SIGN_MODE_DIRECT_AUX and SIGN_MODE_TEXTUAL).
- Addition of information about custom sign modes.
- Expanded explanation of Messages and their implementation.
- Updated links to external documentation and resources.
These changes provide developers with more accurate, up-to-date, and detailed information about transaction handling in the Cosmos SDK.
To further enhance this document, consider adding a "Table of Contents" at the beginning. This would help readers quickly navigate to specific sections of interest, especially given the document's length and the variety of topics covered.
docs/learn/advanced/00-baseapp.md (3)
74-74
: Minor formatting issue: Remove extra spaceThere's an extra space after "decode" in the sentence. Consider removing it for better readability.
-raw transaction bytes relayed by the underlying CometBFT engine. +raw transaction bytes relayed by the underlying CometBFT engine.The update to the TxDecoder link reflecting the new version (v0.52.0-beta.2) is correct.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~74-~74: Loose punctuation mark.
Context: [email protected]/types#TxDecoder): It is used to decode raw transaction ...(UNLIKELY_OPENING_PUNCTUATION)
219-227
: Approved changes with minor suggestionThe updated description of the msgServiceRouter provides better clarity on the nature of the default routers and the possibility of stateful routing mechanisms. The link update to v0.52.0-beta.2 is correct.
Consider adding a comma after "mechanisms" in the following sentence for improved readability:
-However, some applications may want to make use of more stateful routing mechanisms such as allowing governance to disable certain routes or point them to new modules for upgrade purposes. +However, some applications may want to make use of more stateful routing mechanisms, such as allowing governance to disable certain routes or point them to new modules for upgrade purposes.🧰 Tools
🪛 LanguageTool
[uncategorized] ~219-~219: Possible missing comma found.
Context: ...nt to make use of more stateful routing mechanisms such as allowing governance to disable ...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~225-~225: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...grpcQueryRouter
, which maps modules' fully-qualified service methods (string
, defined in t...(HYPHENATED_LY_ADVERB_ADJECTIVE)
225-227
: Approved changes with minor suggestionThe updated description of the grpcQueryRouter is clear and accurate.
Consider removing the hyphen in "fully-qualified" as it's not necessary when the first word is an adverb ending in 'ly':
-Just like the `msgServiceRouter`, the `grpcQueryRouter` is initialized with all the query routes using the application's [module manager](../../build/building-modules/01-module-manager.md) (via the `RegisterServices` method), which itself is initialized with all the application's modules in the application's [constructor](../beginner/00-app-anatomy.md#constructor-function). +Just like the `msgServiceRouter`, the `grpcQueryRouter` is initialized with all the query routes using the application's [module manager](../../build/building-modules/01-module-manager.md) (via the `RegisterServices` method), which itself is initialized with all the application's modules in the application's [constructor](../beginner/00-app-anatomy.md#constructor-function).🧰 Tools
🪛 LanguageTool
[uncategorized] ~225-~225: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...grpcQueryRouter
, which maps modules' fully-qualified service methods (string
, defined in t...(HYPHENATED_LY_ADVERB_ADJECTIVE)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (5)
- docs/learn/advanced/00-baseapp.md (16 hunks)
- docs/learn/advanced/01-transactions.md (8 hunks)
- docs/learn/advanced/02-context.md (2 hunks)
- docs/learn/advanced/03-node.md (2 hunks)
- docs/learn/advanced/04-store.md (10 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
docs/learn/advanced/00-baseapp.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/learn/advanced/01-transactions.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/learn/advanced/02-context.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/learn/advanced/03-node.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/learn/advanced/04-store.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 LanguageTool
docs/learn/advanced/00-baseapp.md
[uncategorized] ~74-~74: Loose punctuation mark.
Context: [email protected]/types#TxDecoder): It is used to decode raw transaction ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~219-~219: Possible missing comma found.
Context: ...nt to make use of more stateful routing mechanisms such as allowing governance to disable ...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~225-~225: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...grpcQueryRouter
, which maps modules' fully-qualified service methods (string
, defined in t...(HYPHENATED_LY_ADVERB_ADJECTIVE)
[grammar] ~240-~240: The verb “implement” needs to be in the to-infinitive form.
Context: ...building on top of the Cosmos SDK don't need implement the ABCI themselves, asBaseApp
comes...(MISSING_TO_BEFORE_A_VERB)
docs/learn/advanced/01-transactions.md
[style] ~112-~112: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ... sign mode to the Cosmos-SDK. While we can not accept the implementation of the sign m...(CAN_NOT_PREMIUM)
docs/learn/advanced/04-store.md
[uncategorized] ~218-~218: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...thKVGasConfigmethod of the
context`. Otherwise it uses the following default: ```go r...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
🔇 Additional comments (21)
docs/learn/advanced/03-node.md (3)
27-27
: LGTM: Updated link to newer SDK versionThe link has been correctly updated to reflect the new version (v0.52.0-beta.2) of the Cosmos SDK. The line numbers have also been adjusted appropriately.
41-41
: LGTM: Updated simapp main.go linkThe link has been correctly updated to reflect the new version (v0.52.0-beta.2) of the Cosmos SDK, consistent with earlier updates in this document.
89-89
: LGTM: Updated start.go linkThe link has been correctly updated to reflect the new version (v0.52.0-beta.2) of the Cosmos SDK. The slight change in line numbers doesn't indicate any significant restructuring of the referenced code.
docs/learn/advanced/02-context.md (1)
Line range hint
1-151
: Conduct a general review for consistency and completeness.Given the significant updates to the Context structure, including new fields and deprecations, it's important to ensure the entire document remains consistent and complete. Please consider the following:
- Review all sections of the document to ensure they align with the latest changes in the Context structure.
- Check if any examples or code snippets throughout the document need updating to reflect the new fields or deprecated elements.
- Verify that the "Pre-requisite Readings" section is still up-to-date and relevant.
- Consider adding a "Changes from Previous Versions" section to highlight major updates for developers familiar with older versions.
- Ensure that the explanation of store branching and the Go Context Package sections are still accurate and relevant with the new Context structure.
This general review will help maintain the overall quality and accuracy of the documentation, making it more valuable for developers working with the latest version of the Cosmos SDK.
#!/bin/bash # Check for any remaining references to old versions or deprecated fields rg -i "v0.50.0|checktx mode" docs/learn/advanced/02-context.md🧰 Tools
🪛 LanguageTool
[style] ~37-~37: Consider a shorter alternative to avoid wordiness.
Context: ...ees.md) price a node is willing to take in order to include a transaction in its block. Thi...(IN_ORDER_TO_PREMIUM)
docs/learn/advanced/04-store.md (11)
39-40
: LGTM: Updated link and line numbersThe link to the
CacheWrapper
interface has been correctly updated to reflect the latest version (store/v1.1.1). The slight change in line numbers is expected due to potential minor changes in the source file.
61-62
: LGTM: Updated link and line numbersThe link to the
Committer
interface has been correctly updated to reflect the latest version (store/v1.1.1). The slight change in line numbers is expected due to potential minor changes in the source file.
75-76
: LGTM: Updated link and line numbersThe link to the
Multistore
interface has been correctly updated to reflect the latest version (store/v1.1.1). The slight change in line numbers is expected due to potential minor changes in the source file.
85-86
: LGTM: Updated link and line numbersThe link to the
CommitMultiStore
interface has been correctly updated to reflect the latest version (store/v1.1.1). The slight change in line numbers is expected due to potential minor changes in the source file.
88-91
: LGTM: Improved clarity and updated referenceThe changes in this segment are well-executed:
- The link to the
rootMulti.Store
has been correctly updated to reflect the latest version (store/v1.1.1).- The text has been modified to improve clarity, now explicitly stating that
rootMulti.Store
is the "go-to implementation" of theCommitMultiStore
interface.- The slight change in line numbers is expected due to potential minor changes in the source file.
These updates enhance the accuracy and readability of the documentation.
98-101
: LGTM: Updated link and line numbersThe link to the
cachemulti.Store
has been correctly updated to reflect the latest version (store/v1.1.1). The slight change in line numbers is expected due to potential minor changes in the source file.
131-132
: LGTM: Updated link and line numbersThe link to the IAVL Store has been correctly updated to reflect the latest version (store/v1.1.1). The slight change in line numbers is expected due to potential minor changes in the source file.
147-148
: LGTM: Updated link and line numbersThe link to the
dbadapter.Store
has been correctly updated to reflect the latest version (store/v1.1.1). The slight change in line numbers is expected due to potential minor changes in the source file.
157-158
: LGTM: Updated link and line numbersThe link to the
Transient.Store
has been correctly updated to reflect the latest version (store/v1.1.1). The slight change in line numbers is expected due to potential minor changes in the source file.
171-172
: LGTM: Updated link and line numbersThe link to the
context.TransientStore()
method has been correctly updated to reflect the newer version of the SDK (v0.52.0-beta.2). The slight change in line numbers is expected due to potential minor changes in the source file.
123-124
: LGTM: Updated link and line numbers, but verify code relevanceThe link to the bank module's keeper example has been correctly updated to reflect a newer version of the SDK (v0.52.0-beta.2). However, the significant change in line numbers (from 115-135 to 114-134) suggests substantial changes to the file.
Please run the following script to verify that the code snippet is still relevant and accurately represents the current implementation:
docs/learn/advanced/01-transactions.md (2)
Line range hint
116-139
: Excellent clarification on Messages and their implementation!The expanded explanation of sdk.Msg and its relation to Protobuf Msg services provides valuable context for module developers. This update enhances the understanding of how messages are defined and implemented in the Cosmos SDK.
The updated link to ABCI Messages and the additional details about the SDK app router and MsgServer interface are particularly helpful. This information will assist developers in better understanding the message handling process in the SDK.
Line range hint
144-171
: LGTM! Updated references and clear explanation of transaction generation.The updated links to the latest SDK version (v0.52.0-beta.2) for TxBuilder, builder, StdTxBuilder, and TxConfig interfaces ensure that the documentation remains current. The explanation of how to use TxConfig and TxBuilder for transaction generation is clear and helpful for developers.
The pseudo-code snippet demonstrating how to generate and encode a transaction is particularly useful. It provides a practical example that developers can easily follow and adapt to their needs.
docs/learn/advanced/00-baseapp.md (4)
Line range hint
240-292
: Excellent addition of Prepare Proposal sectionThe new section on Prepare Proposal is a valuable addition to the documentation. It clearly explains the purpose of the PrepareProposal function, its role in ABCI++, and its implementation in the Cosmos SDK. The content is accurate and provides good insights into the transaction processing and proposal preparation.
The explanation of how PrepareProposal complements ProcessProposal is particularly useful for understanding the overall proposal handling process.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~257-~257: Possible missing comma found.
Context: ....Msg`'s. This is done after stateless checks as stateful checks are more computati...(AI_HYDRA_LEO_MISSING_COMMA)
Line range hint
294-314
: Excellent addition of Process Proposal sectionThe new section on Process Proposal is a valuable addition to the documentation. It provides a comprehensive explanation of the ProcessProposal function, its role in the ABCI message flow, and its implementation in the Cosmos SDK. The content is accurate, clear, and highlights important considerations such as the coherence property and potential liveness implications.
The explanation of how ProcessProposal complements PrepareProposal and the emphasis on the importance of correct implementation are particularly useful for developers working with the Cosmos SDK.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~312-~312: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...edsGasWanted
, the execution is halted and the changes made to the cached copy of ...(COMMA_COMPOUND_SENTENCE_2)
Line range hint
314-354
: Approved updates to CheckTx sectionThe CheckTx section has been significantly improved with more detailed information about the process, including:
- Clarification on gas handling and fee calculation.
- More detailed explanation of the checks performed during CheckTx.
- Updated links to the correct version (v0.52.0-beta.2) of the codebase.
These changes provide a more comprehensive understanding of the CheckTx process in the Cosmos SDK.
The updates are accurate and enhance the documentation's value for developers working with the Cosmos SDK.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~312-~312: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...edsGasWanted
, the execution is halted and the changes made to the cached copy of ...(COMMA_COMPOUND_SENTENCE_2)
Line range hint
562-576
: Valuable addition of ExtendVote and VerifyVoteExtension sectionsThe new sections on ExtendVote and VerifyVoteExtension are excellent additions to the documentation. They provide clear explanations of these ABCI++ methods and their current implementation in the Cosmos SDK. Key points:
- ExtendVote is explained as a method to extend pre-commit votes with arbitrary data.
- VerifyVoteExtension is described as a method to verify the data returned by ExtendVote.
- Both are currently implemented as no-ops in the Cosmos SDK.
These additions enhance the documentation's coverage of ABCI++ features and provide valuable information for developers working with or extending the Cosmos SDK.
@@ -20,7 +20,7 @@ The `context` is a data structure intended to be passed from function to functio | |||
The Cosmos SDK `Context` is a custom data structure that contains Go's stdlib [`context`](https://pkg.go.dev/context) as its base, and has many additional types within its definition that are specific to the Cosmos SDK. The `Context` is integral to transaction processing in that it allows modules to easily access their respective [store](./04-store.md#base-layer-kvstores) in the [`multistore`](./04-store.md#multistore) and retrieve transactional context such as the block header and gas meter. | |||
|
|||
```go reference | |||
https://github.com/cosmos/cosmos-sdk/blob/v0.50.0-alpha.0/types/context.go#L41-L67 | |||
https://github.com/cosmos/cosmos-sdk/blob/v0.52.0-beta.2/types/context.go#L40-L64 |
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
Documentation Needs Update:
The updated link to the Context definition is functional. However, the documentation references CheckTx Mode
, which is deprecated and replaced by execMode
. Please update the documentation to reflect this change for consistency with the latest Cosmos SDK version.
🔗 Analysis chain
Verify the updated link and its content.
The link to the Context definition has been updated to a newer version of the Cosmos SDK (v0.52.0-beta.2). Please ensure that:
- The link is functional and points to the correct location.
- The content at the new link accurately represents the current Context definition.
- There are no significant changes in the Context structure that need to be reflected in the surrounding documentation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the updated link
curl -I https://github.com/cosmos/cosmos-sdk/blob/v0.52.0-beta.2/types/context.go
Length of output: 232
There is the opportunity to add your own custom sign mode to the Cosmos-SDK. While we can not accept the implementation of the sign mode to the repository, we can accept a pull request to add the custom signmode to the SignMode enum located [here](https://github.com/cosmos/cosmos-sdk/blob/v0.52.0-beta.2/proto/cosmos/tx/signing/v1beta1/signing.proto#L9-L17) | ||
|
||
## Transaction Process |
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.
Valuable addition on custom sign modes!
This new section provides helpful information for developers who need to implement custom sign modes. It's great to see the documentation addressing advanced use cases.
There's a minor grammatical issue in the text. Please apply the following correction:
-There is the opportunity to add your own custom sign mode to the Cosmos-SDK. While we can not accept the implementation of the sign mode to the repository, we can accept a pull request to add the custom signmode to the SignMode enum located [here]
+There is an opportunity to add your own custom sign mode to the Cosmos SDK. While we cannot accept the implementation of the sign mode in the repository, we can accept a pull request to add the custom sign mode to the SignMode enum located [here]
📝 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.
There is the opportunity to add your own custom sign mode to the Cosmos-SDK. While we can not accept the implementation of the sign mode to the repository, we can accept a pull request to add the custom signmode to the SignMode enum located [here](https://github.com/cosmos/cosmos-sdk/blob/v0.52.0-beta.2/proto/cosmos/tx/signing/v1beta1/signing.proto#L9-L17) | |
## Transaction Process | |
There is an opportunity to add your own custom sign mode to the Cosmos SDK. While we cannot accept the implementation of the sign mode in the repository, we can accept a pull request to add the custom sign mode to the SignMode enum located [here](https://github.com/cosmos/cosmos-sdk/blob/v0.52.0-beta.2/proto/cosmos/tx/signing/v1beta1/signing.proto#L9-L17) | |
## Transaction Process |
🧰 Tools
🪛 LanguageTool
[style] ~112-~112: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ... sign mode to the Cosmos-SDK. While we can not accept the implementation of the sign m...(CAN_NOT_PREMIUM)
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! The rabbit did catch typos.
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: 0
🧹 Outside diff range and nitpick comments (3)
docs/learn/advanced/00-baseapp.md (3)
74-74
: Approve link update with minor formatting fixThe link update to the newer version of
TxDecoder
is correct. However, there are extra spaces after the link that should be removed for consistency.Please remove the extra spaces after the link:
-[`TxDecoder`](https://pkg.go.dev/github.com/cosmos/[email protected]/types#TxDecoder): It is used to decode raw transaction bytes relayed by the underlying CometBFT engine. +[`TxDecoder`](https://pkg.go.dev/github.com/cosmos/[email protected]/types#TxDecoder): It is used to decode raw transaction bytes relayed by the underlying CometBFT engine.🧰 Tools
🪛 LanguageTool
[uncategorized] ~74-~74: Loose punctuation mark.
Context: [email protected]/types#TxDecoder): It is used to decode raw transaction ...(UNLIKELY_OPENING_PUNCTUATION)
227-227
: Approve gRPC Query Router updates with minor suggestionThe updates to the gRPC Query Router section provide valuable information about its functionality, including the explanation of stateless and stateful routing. This addition enhances the documentation's usefulness for developers.
Consider a minor grammatical improvement:
-Just like the `msgServiceRouter`, the `grpcQueryRouter` is initialized with all the query routes using the application's [module manager](../../build/building-modules/01-module-manager.md) (via the `RegisterServices` method), which itself is initialized with all the application's modules in the application's [constructor](../beginner/00-app-anatomy.md#constructor-function). +Just like the `msgServiceRouter`, the `grpcQueryRouter` is initialized with all the query routes using the application's [module manager](../../build/building-modules/01-module-manager.md) (via the `RegisterServices` method), which is itself initialized with all the application's modules in the application's [constructor](../beginner/00-app-anatomy.md#constructor-function).
240-240
: Approve ABCI 2.0 Messages updates with grammatical correctionThe updated list of main ABCI 2.0 messages is correct and reflects the current state of the Cosmos SDK. The added information about developers not needing to implement ABCI themselves is helpful.
Please correct the grammatical error in the sentence about developers:
-Developers building on top of the Cosmos SDK don't need implement the ABCI themselves, as `BaseApp` comes with a built-in implementation of the interface. +Developers building on top of the Cosmos SDK don't need to implement the ABCI themselves, as `BaseApp` comes with a built-in implementation of the interface.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (1)
- docs/learn/advanced/00-baseapp.md (16 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
docs/learn/advanced/00-baseapp.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 LanguageTool
docs/learn/advanced/00-baseapp.md
[uncategorized] ~74-~74: Loose punctuation mark.
Context: [email protected]/types#TxDecoder): It is used to decode raw transaction ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~225-~225: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...grpcQueryRouter
, which maps modules' fully-qualified service methods (string
, defined in t...(HYPHENATED_LY_ADVERB_ADJECTIVE)
🔇 Additional comments (7)
docs/learn/advanced/00-baseapp.md (7)
219-219
: Approve changes to Msg Service Router sectionThe updates to the Msg Service Router section provide more clarity on its role and correctly update the link to the latest version. These changes improve the documentation's accuracy and consistency.
258-258
: Approve addition of Prepare Proposal sectionThe new Prepare Proposal section is a valuable addition to the documentation. It clearly explains the purpose of the PrepareProposal function, its role in ABCI++, and provides a detailed overview of its implementation. This information will be helpful for developers working with the Cosmos SDK.
292-292
: Approve addition of Process Proposal sectionThe new Process Proposal section is an excellent addition to the documentation. It provides a comprehensive explanation of the ProcessProposal function, its role in block validation, and its relationship with PrepareProposal. The section also highlights important considerations for developers, such as the potential impact on network liveness. This information is crucial for understanding the ABCI++ workflow in the Cosmos SDK.
314-314
: Approve updates to CheckTx sectionThe updates to the CheckTx section provide more detailed and accurate information about the process, particularly regarding gas calculation and usage. These changes enhance the documentation's completeness and will be valuable for developers working with the Cosmos SDK.
406-411
: Approve addition of AnteHandler warningThe new warning about ante handlers and inner messages is a crucial addition to the documentation. It clearly explains the limitations of default ante handlers when dealing with embedded messages in complex transactions. This information is essential for developers working with modules like x/authz or x/gov and helps prevent potential security issues.
493-493
: Approve updates to RunMsgs sectionThe updates to the RunMsgs section provide more detailed and accurate information about the message routing process. The explanation of how messages are routed to the appropriate module using the MsgServiceRouter is clear and informative. These changes enhance the documentation's completeness and will be valuable for developers working with the Cosmos SDK.
562-562
: Approve addition of ExtendVote and VerifyVoteExtension sectionsThe new sections on ExtendVote and VerifyVoteExtension are valuable additions to the documentation. They provide clear explanations of these ABCI++ functions and their current implementation in the Cosmos SDK. This information is important for developers who need to understand the full range of ABCI++ methods and their usage in the SDK.
(cherry picked from commit 77254b7)
Description
Ref:
#21429
This PR updates learn/advanced:
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
BaseApp
,Transactions
,Context
,Node
, andStore
documentation in the Cosmos SDK.