docs(core): create docs for environment and core API #22156
Conversation
📝 WalkthroughWalkthroughThe pull request includes substantial revisions to the documentation for the Cosmos SDK, specifically focusing on encoding mechanisms and the introduction of a new "Core" package. The updates clarify the transition from 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
|
| be backed by a merkle-tree store, but developers can provide their own implementations if | ||
| needed. In the case of the `KVStoreService` implementation provided in `Environment`, module | ||
| developers should understand that calling `OpenKVStore` will return a store already scoped to | ||
| the module's prefix. The wiring for this scoping is specified in `runtime`. |
There was a problem hiding this comment.
I couldn't find any docs for runtime to link out to, do we want some?
There was a problem hiding this comment.
Yes, we need an explanation of what is runtime in concepts too 👍🏾
I think the order should be, baseapp, transaction, core, runtime and then the rest, but we can do the ordering later.
| that needs to make changes to the store, but may need to be rolled back if an error occurs. | ||
| Below is a contrived example based on the `x/epoch` module's BeginBlocker logic. | ||
|
|
||
| ```go reference |
There was a problem hiding this comment.
should be just go, reference is when having a link
| be backed by a merkle-tree store, but developers can provide their own implementations if | ||
| needed. In the case of the `KVStoreService` implementation provided in `Environment`, module | ||
| developers should understand that calling `OpenKVStore` will return a store already scoped to | ||
| the module's prefix. The wiring for this scoping is specified in `runtime`. |
There was a problem hiding this comment.
Yes, we need an explanation of what is runtime in concepts too 👍🏾
I think the order should be, baseapp, transaction, core, runtime and then the rest, but we can do the ordering later.
julienrbrt
added
the
backport/v0.52.x
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (12)
docs/learn/advanced/17-core.md (7)
1-24: Minor grammatical improvements neededThe content provides a clear introduction to the Core package and the Environment struct. However, there are a few minor grammatical issues that can be addressed:
- Line 21: Consider adding a comma after "Historically" for improved clarity.
- Line 22: Replace "an" with "a" before "sdk.Context" as it doesn't start with a vowel sound.
- Line 23: The phrase "in many cases" is used frequently. Consider rephrasing to "in numerous scenarios" or "in various situations" for variety.
These small changes will enhance the readability and professionalism of the document.
🧰 Tools
🪛 LanguageTool
[typographical] ~21-~21: Consider adding a comma after ‘Historically’ for more clarity.
Context: ...ppmodule/v2/environment.go#L16-L29 ``` Historically the SDK has used an [sdk.Context](02-co...(RB_LY_COMMA)
[misspelling] ~22-~22: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...-L29 ``` Historically the SDK has used an sdk.Context to pass ar...(EN_A_VS_AN)
[style] ~23-~23: The phrase ‘in many cases’ is used quite frequently. Consider using a less frequent alternative to set your writing apart from others.
Context: ...is intended to replace ansdk.Contextin many cases.sdk.Contextwill be deprecated in th...(IN_MANY_STYLE_CASES)
26-53: Branch Service section looks good with a minor suggestionThe Branch Service section provides a clear explanation of its purpose and usage, along with a relevant code example. The content is informative and well-structured.
Consider rephrasing the sentence on line 30 to strengthen the wording:
"This is useful for executing code that requires store modifications but may need to be rolled back if an error occurs."Overall, this section effectively communicates the functionality of the BranchService.
🧰 Tools
🪛 LanguageTool
[style] ~30-~30: Consider shortening or rephrasing this to strengthen your wording.
Context: ...useful for executing code that needs to make changes to the store, but may need to be rolled ba...(MAKE_CHANGES)
54-66: Event Service section is informative with a minor correction neededThe Event Service section provides a clear explanation of its functionality and its relationship to the EventManager. The content is well-structured and informative.
Please add a comma after "SDK" on line 58 to improve readability:
"For information on how to emit events and their meaning in the SDK, see the Events document."Additionally, on line 61, add a comma after "Roughly speaking" as it's an introductory phrase.
The code reference provided is appropriate and adds value to the explanation.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~58-~58: Possible missing comma found.
Context: ...to emit events and their meaning in the SDK see the Events document...(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~61-~61: This introductory phrase requires a comma.
Context: ... deprecated and removed in the future. Roughly speaking legacyEmitTypeEventmaps toEmita...(RB_SPEAKING_COMMA)
68-76: Gas Service section is well-explained with a minor correction neededThe Gas Service section provides a concise and clear explanation of its functionality and usage. The content effectively communicates the purpose of the Gas Service.
Please add a comma before "but" on line 71 to separate the two independent clauses:
"Gas consumption is largely handled at the framework level for transaction processing and state access, but modules can choose to use the gas service directly if needed."The code reference provided is appropriate and enhances the explanation.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~71-~71: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... transaction processing and state access but modules can choose to use the gas servi...(COMMA_COMPOUND_SENTENCE_2)
78-104: Header Service sections are well-explained with minor improvements neededThe Header Service and Custom Header Service sections provide clear and informative explanations of their functionalities and implementation. The content effectively communicates the purpose and customization options for the Header Service.
Please make the following minor improvements:
On line 89, hyphenate "service-oriented" as it's used as a modifier: "Core's service-oriented architecture (SOA) allows for chain developers to define a custom implementation of the
HeaderServiceinterface."On line 92, add a comma after the parenthetical: "Note, this example is taken from
runtime/v2but could easily be adapted toruntime/v1(the default runtime 0.52)."The code references provided are appropriate and enhance the explanations for both the Header Service and Custom Header Service.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~89-~89: When ‘service-oriented’ is used as a modifier, it is usually spelled with a hyphen.
Context: ... ``` ### Custom Header Service Core's service oriented architecture (SOA) allows for chain developers to de...(SERVICE_LEVEL_HYPHEN)
[uncategorized] ~92-~92: Possible missing comma found.
Context: ...(when using depinject is shown below). Note this example is taken fromruntime/v2...(AI_HYDRA_LEO_MISSING_COMMA)
106-118: Query and Message Router Service section is informative with a minor correction neededThe Query and Message Router Service section provides a clear explanation of these services and their importance in module decoupling. The content effectively communicates the purpose and benefits of these services.
Please make the following minor correction:
On line 108, add the article "an" before "implementation": "Both the query and message router services are an implementation of the same interface,
router.Service."The code reference provided is appropriate and adds value to the explanation.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~108-~108: Possible missing article found.
Context: ...e query and message router services are implementation of the same interface,router.Service...(AI_HYDRA_LEO_MISSING_AN)
[style] ~117-~117: Consider a shorter alternative to avoid wordiness.
Context: ...ch require a dynamic dispatch mechanism in order to function. ## TransactionService ```go...(IN_ORDER_TO_PREMIUM)
131-141: KVStore Service section is informative with a minor suggestionThe KVStore Service section provides a clear explanation of its functionality and usage. The content effectively communicates the purpose of the KVStore Service and its implementation details.
Consider rephrasing the last sentence on line 117 to avoid wordiness:
"The wiring for this scoping is specified inruntime."The code reference provided is appropriate and adds value to the explanation.
Overall, this section is well-written and informative.
docs/learn/advanced/05-encoding.md (5)
19-27: Approve changes with a minor suggestion for clarityThe revisions to the introduction and encoding protocols section provide a clear and concise explanation of the current encoding practices in the Cosmos SDK. The transition from
go-aminotogogoprotobufis well-emphasized, and the limitations of Amino are clearly stated.Consider rephrasing line 27 for improved clarity:
- JSON) in order to support the Amino JSON sign mode, and for JSON RPC endpoints. + JSON) to support the Amino JSON sign mode and JSON RPC endpoints.This minor change removes the redundant "in order to" phrase, making the sentence more concise.
🧰 Tools
🪛 LanguageTool
[style] ~27-~27: Consider a shorter alternative to avoid wordiness.
Context: ...only used to generate JSON (Amino JSON) in order to support the Amino JSON sign mode, and f...(IN_ORDER_TO_PREMIUM)
35-47: Approve changes with a suggestion for additional contextThe updates to the storage encoding and ProtoCodec sections provide clear guidance for module developers. The emphasis on Protobuf encoding and the explanation of
ProtoCodecare well-articulated.Consider adding a brief explanation or link to more information about the
collectionspackage mentioned in lines 37-38. This would provide readers with context on why this package is recommended and how it relates to encoding and decoding state.
Line range hint
49-70: Approve changes with a suggestion for additional informationThe new section on Gogoproto and the guidelines for protobuf message definitions are excellent additions to the documentation. They provide clear and specific guidance for developers working with Protobuf in the Cosmos SDK context.
Consider adding a brief note or link to documentation explaining how to generate code using these annotations. This would provide a complete picture for developers who are new to working with Protobuf in the Cosmos SDK.
🧰 Tools
🪛 LanguageTool
[style] ~27-~27: Consider a shorter alternative to avoid wordiness.
Context: ...only used to generate JSON (Amino JSON) in order to support the Amino JSON sign mode, and f...(IN_ORDER_TO_PREMIUM)
Line range hint
95-232: Approve changes with a suggestion for additional clarityThe section on Interface Encoding and Usage of
Anyis comprehensive and well-explained. It provides clear examples of how to useAnyfor interface encoding, including packing and unpacking. The explanation of theUnpackInterfacesmethod and the real-life examples from the Cosmos SDK are particularly valuable for developers.Consider adding a brief note or visual diagram illustrating the relationship between
Any,InterfaceRegistry, andUnpackInterfaces. This could help developers better understand how these components work together in the context of interface encoding.🧰 Tools
🪛 LanguageTool
[style] ~27-~27: Consider a shorter alternative to avoid wordiness.
Context: ...only used to generate JSON (Amino JSON) in order to support the Amino JSON sign mode, and f...(IN_ORDER_TO_PREMIUM)
Line range hint
275-293: Approve changes to FAQ section with a suggestion for additional informationThe FAQ section provides concise and useful guidance for creating modules using protobuf encoding. The coverage of module types, naming conventions, and links to style guides is helpful for developers.
Consider adding a brief example or link to a sample module that demonstrates best practices in protobuf encoding. This could provide a practical reference for developers implementing these guidelines.
🧰 Tools
🪛 LanguageTool
[style] ~27-~27: Consider a shorter alternative to avoid wordiness.
Context: ...only used to generate JSON (Amino JSON) in order to support the Amino JSON sign mode, and f...(IN_ORDER_TO_PREMIUM)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (2)
- docs/learn/advanced/05-encoding.md (1 hunks)
- docs/learn/advanced/17-core.md (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
docs/learn/advanced/05-encoding.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/learn/advanced/17-core.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 LanguageTool
docs/learn/advanced/05-encoding.md
[style] ~27-~27: Consider a shorter alternative to avoid wordiness.
Context: ...only used to generate JSON (Amino JSON) in order to support the Amino JSON sign mode, and f...(IN_ORDER_TO_PREMIUM)
docs/learn/advanced/17-core.md
[typographical] ~21-~21: Consider adding a comma after ‘Historically’ for more clarity.
Context: ...ppmodule/v2/environment.go#L16-L29 ``` Historically the SDK has used an [sdk.Context](02-co...(RB_LY_COMMA)
[misspelling] ~22-~22: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...-L29 ``` Historically the SDK has used an sdk.Context to pass ar...(EN_A_VS_AN)
[style] ~23-~23: The phrase ‘in many cases’ is used quite frequently. Consider using a less frequent alternative to set your writing apart from others.
Context: ...is intended to replace ansdk.Contextin many cases.sdk.Contextwill be deprecated in th...(IN_MANY_STYLE_CASES)
[style] ~30-~30: Consider shortening or rephrasing this to strengthen your wording.
Context: ...useful for executing code that needs to make changes to the store, but may need to be rolled ba...(MAKE_CHANGES)
[uncategorized] ~58-~58: Possible missing comma found.
Context: ...to emit events and their meaning in the SDK see the Events document...(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~61-~61: This introductory phrase requires a comma.
Context: ... deprecated and removed in the future. Roughly speaking legacyEmitTypeEventmaps toEmita...(RB_SPEAKING_COMMA)
[uncategorized] ~71-~71: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... transaction processing and state access but modules can choose to use the gas servi...(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~89-~89: When ‘service-oriented’ is used as a modifier, it is usually spelled with a hyphen.
Context: ... ``` ### Custom Header Service Core's service oriented architecture (SOA) allows for chain developers to de...(SERVICE_LEVEL_HYPHEN)
[uncategorized] ~92-~92: Possible missing comma found.
Context: ...(when using depinject is shown below). Note this example is taken fromruntime/v2...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~108-~108: Possible missing article found.
Context: ...e query and message router services are implementation of the same interface,router.Service...(AI_HYDRA_LEO_MISSING_AN)
[style] ~117-~117: Consider a shorter alternative to avoid wordiness.
Context: ...ch require a dynamic dispatch mechanism in order to function. ## TransactionService ```go...(IN_ORDER_TO_PREMIUM)
🔇 Additional comments (3)
docs/learn/advanced/17-core.md (1)
120-130: TransactionService section is well-explained and conciseThe TransactionService section provides a clear and concise explanation of its functionality and usage. The content effectively communicates the purpose of the TransactionService and its various execution modes.
The code reference provided is appropriate and enhances the explanation.
Overall, this section is well-written and informative.
docs/learn/advanced/05-encoding.md (2)
Line range hint
72-93: Approve changes to Transaction Encoding sectionThe section on Transaction Encoding provides a clear and concise explanation of how Protobuf is used for encoding and decoding transactions in the Cosmos SDK. The references to
TxEncoderandTxDecoderobjects, along with links to implementation examples and ADR-020, offer valuable resources for developers seeking to understand or implement transaction encoding.🧰 Tools
🪛 LanguageTool
[style] ~27-~27: Consider a shorter alternative to avoid wordiness.
Context: ...only used to generate JSON (Amino JSON) in order to support the Amino JSON sign mode, and f...(IN_ORDER_TO_PREMIUM)
Line range hint
234-273: Approve changes toAny's TypeURL sectionThe section on
Any's TypeURL provides a clear explanation of how TypeURLs are structured in the Cosmos SDK, highlighting important differences from other implementations. The guidelines for using helper functions from"github.com/cosmos/cosmos-proto/anyutil"are practical and help maintain compatibility. The provided example effectively illustrates the difference in TypeURL format.🧰 Tools
🪛 LanguageTool
[style] ~27-~27: Consider a shorter alternative to avoid wordiness.
Context: ...only used to generate JSON (Amino JSON) in order to support the Amino JSON sign mode, and f...(IN_ORDER_TO_PREMIUM)
(cherry picked from commit 9076487)
#22159) Co-authored-by: Matt Kocubinski <[email protected]> Co-authored-by: Julien Robert <[email protected]>
Description
Ref: #21429
Removes most references to Amino in the encoding docs, and clarifies some points about Amino JSON.
Adds docs for core as used in Environment.
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
go-aminotogogoprotobuf, emphasizing Protobuf as the primary encoding method.Environment,BranchService, andEvent Service.