docs: ICA queries #6068
docs: ICA queries #6068
Conversation
crodriguezvega
requested review from
colin-axner,
AdityaSripal,
charleenfei,
damiannolan,
chatton,
DimitrisJim and
srdtrk
as code owners
WalkthroughThe recent updates focus on enhancing the Interchain Accounts module by improving control flow, behavior, and usability. Changes include modifications to initialization parameters, addition of query functionality for executing safe queries on the host chain, and migration updates for clarity and efficiency in development from v8 to v9. Changes
Possibly related issues
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: .coderabbit.yml
Files selected for processing (4)
- docs/docs/02-apps/02-interchain-accounts/04-integration.md (1 hunks)
- docs/docs/02-apps/02-interchain-accounts/05-messages.md (1 hunks)
- docs/docs/05-migrations/13-v8-to-v9.md (1 hunks)
- modules/apps/27-interchain-accounts/host/types/msgs_test.go (2 hunks)
Additional comments: 4
modules/apps/27-interchain-accounts/host/types/msgs_test.go (1)
- 122-145: The tests for
MsgModuleQuerySafeGetSignersare well-implemented and cover the necessary cases.docs/docs/02-apps/02-interchain-accounts/05-messages.md (1)
- 75-98: The documentation for executing queries on the host chain using
MsgModuleQuerySafeis clear and provides a useful example. This addition enhances the understanding of how to utilizeMsgModuleQuerySafewithinInterchainPacketData.docs/docs/05-migrations/13-v8-to-v9.md (1)
- 52-64: The documentation accurately describes the addition of
queryRouterto theNewKeeperconstructor in the ICS27 module, which is a significant change for migration from v8 to v9.docs/docs/02-apps/02-interchain-accounts/04-integration.md (1)
- 85-94: The documentation for integrating the Interchain Accounts module, including the modifications to the
app.ICAControllerKeeperandapp.ICAHostKeeperinitializations, is clear and informative. This addition provides valuable guidance for integrating the module.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: .coderabbit.yml
Files selected for processing (1)
- docs/docs/05-migrations/13-v8-to-v9.md (1 hunks)
Additional comments: 1
docs/docs/05-migrations/13-v8-to-v9.md (1)
- 52-63: The addition of
queryRouterto theNewKeeperconstructor is correctly documented and aligns with the PR objectives. However, ensure that the documentation clearly explains the purpose ofqueryRouterand how it should be used, especially for new users unfamiliar with the concept.
There was a problem hiding this comment.
Actionable comments posted: 3
Review Status
Configuration used: .coderabbit.yml
Files selected for processing (2)
- docs/docs/05-migrations/13-v8-to-v9.md (1 hunks)
- modules/apps/27-interchain-accounts/host/types/msgs_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- modules/apps/27-interchain-accounts/host/types/msgs_test.go
Additional Context Used
Path-based Instructions (1)
docs/docs/05-migrations/13-v8-to-v9.md (1)
**/*.md:
Assess the documentation for misspellings, grammatical errors, missing documentation and correctness
Additional comments not posted (3)
docs/docs/05-migrations/13-v8-to-v9.md (3)
47-67: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [10-10]
There's a possible spelling mistake found near line 10. Consider revising "ibc-go" to "IBC-Go" to maintain consistency with the document title and ensure proper noun capitalization.
47-67: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [29-29]
Multiple sentences are concatenated without proper spacing after periods. This issue occurs throughout the document and affects readability.
Consider adding a space after each period to improve the document's readability.
52-63: The addition ofqueryRouterto theNewKeeperconstructor in ICS27 is correctly documented. However, the code block could be enhanced for clarity by aligning the parameters.Consider aligning the parameters in the code block for better readability.
| - Functions `ConstructUpdateTMClientHeader` and `ConstructUpdateTMClientHeaderWithTrustedHeight` of `TestChain` type have been replaced with `IBCClientHeader`. This function will construct a `07-tendermint` header to update the light client on the counterparty chain. The trusted height must be passed in as a non-zero height. | ||
| - `GetValsAtHeight` has been renamed to `GetTrustedValidators` | ||
|
|
||
| ### ICS27 - Interchain Accounts | ||
|
|
||
| In [#5785](https://github.com/cosmos/ibc-go/pull/5785) the list of arguments of the `NewKeeper` constructor function of the host submodule was extended with an extra argument for the gRPC query router that the submodule uses when executing a [`MsgModuleQuerySafe`](https://github.com/cosmos/ibc-go/blob/eecfa5c09a4c38a5c9f2cc2a322d2286f45911da/proto/ibc/applications/interchain_accounts/host/v1/tx.proto#L41-L51) to perform queries that are module safe: | ||
|
|
||
| ```diff | ||
| func NewKeeper( | ||
| cdc codec.Codec, key storetypes.StoreKey, legacySubspace icatypes.ParamSubspace, | ||
| ics4Wrapper porttypes.ICS4Wrapper, channelKeeper icatypes.ChannelKeeper, | ||
| portKeeper icatypes.PortKeeper, accountKeeper icatypes.AccountKeeper, | ||
| scopedKeeper exported.ScopedKeeper, msgRouter icatypes.MessageRouter, | ||
| + queryRouter icatypes.QueryRouter, | ||
| authority string, | ||
| ) Keeper | ||
| ``` | ||
|
|
||
| ## Relayers | ||
|
|
||
| - Renaming of event attribute keys in [#5603](https://github.com/cosmos/ibc-go/pull/5603). |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [19-19]
The note about golang semantic versioning could be clarified for better understanding. Consider specifying that this applies to the import paths in Go modules.
- **Note:** ibc-go supports golang semantic versioning and therefore all imports must be updated on major version releases.
+ **Note:** IBC-Go adheres to Go's semantic versioning, requiring updates to import paths in Go modules upon major version releases.📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [46-46]
The term "favour" is used, which is British English. While not incorrect, the document should maintain consistent use of American English if that's the standard being followed.
- The `mock.PV` type has been removed in favour of [`cmttypes.MockPV`](https://github.com/cometbft/cometbft/blob/v0.38.5/types/priv_validator.go#L50)
+ The `mock.PV` type has been removed in favor of [`cmttypes.MockPV`](https://github.com/cometbft/cometbft/blob/v0.38.5/types/priv_validator.go#L50)📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [77-77]
There's a repeated whitespace before CheckSubstituteAndUpdateState in the table mapping ClientState interface functions to LightClientModule interface functions. This could be a typo or formatting issue.
- |`UpdateStateOnMisbehaviour` |`UpdateStateOnMisbehaviour` | |`CheckSubstituteAndUpdateState`|`RecoverClient` |
+ |`UpdateStateOnMisbehaviour` |`UpdateStateOnMisbehaviour` | |`CheckSubstituteAndUpdateState`|`RecoverClient` |
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: .coderabbit.yml
Files selected for processing (1)
- modules/apps/27-interchain-accounts/host/types/msgs_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- modules/apps/27-interchain-accounts/host/types/msgs_test.go
Additional Context Used
|
Description
closes: #XXXX
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/).godoccomments.Files changedin the GitHub PR explorer.SonarCloud Reportin the comment section below once CI passes.Summary by CodeRabbit
Documentation
MsgModuleQuerySafe.New Features
Tests