Skip to content
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: updates due to 02-client routing refactoring #6049

Merged
merged 9 commits into from
May 22, 2024

Conversation

crodriguezvega
Copy link
Contributor

@crodriguezvega crodriguezvega commented Mar 21, 2024

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.

  • Targeted PR against the correct branch (see CONTRIBUTING.md).
  • Linked to GitHub issue with discussion and accepted design, OR link to spec that describes this work.
  • Code follows the module structure standards and Go style guide.
  • Wrote unit and integration tests.
  • Updated relevant documentation (docs/).
  • Added relevant godoc comments.
  • Provide a conventional commit message to follow the repository standards.
  • Include a descriptive changelog entry when appropriate. This may be left to the discretion of the PR reviewers. (e.g. chores should be omitted from changelog)
  • Re-reviewed Files changed in the GitHub PR explorer.
  • Review SonarCloud Report in the comment section below once CI passes.

Summary by CodeRabbit

  • New Features

    • Enhanced IBC light client module with new methods for state updates, misbehaviour handling, client recovery, and upgrade verification.
  • Documentation

    • Updated multiple documentation files for clarity and accuracy.
    • Corrected terminology in various documents and code comments from "comprises of" to "is comprised of."
  • Refactor

    • Consolidated state verification functions into two generic methods: VerifyMembership and VerifyNonMembership.

Copy link
Contributor

coderabbitai bot commented Mar 21, 2024

Walkthrough

Walkthrough

The recent updates have significantly enhanced the IBC light client module by introducing new methods for managing client states, handling misbehaviors, and verifying upgrades. Additionally, the 08-wasm module now serves as a proxy for light clients. Documentation improvements include clarifying method signatures, enhancing descriptions, and correcting grammatical errors.

Changes

Files/Paths Change Summaries
docs/docs/03-light-clients/01-developer-guide/02-light-client-module.md Added new methods to LightClientModule interface for managing client states and handling misbehaviors.
docs/docs/03-light-clients/01-developer-guide/05-updates-and-misbehaviour.md Updated interfaces for client message handling and misbehavior in the LightClientModule.
docs/docs/03-light-clients/04-wasm/02-concepts.md Enhanced 08-wasm module to act as a proxy for light clients.
docs/docs/03-light-clients/04-wasm/03-integration.md Corrected code comments and improved setup instructions for 08-wasm module.
docs/docs/03-light-clients/01-developer-guide/07-proofs.md Consolidated state verification functions and updated method parameters and comments.
docs/versioned_docs/version-v7.3.x/03-light-clients/01-developer-guide/06-proofs.md Corrected wording regarding IBC existence proofs.
docs/versioned_docs/version-v8.1.x/03-light-clients/01-developer-guide/01-overview.md Clarified access control in IBC light client modules.
docs/versioned_docs/version-v8.1.x/03-light-clients/01-developer-guide/06-proofs.md Updated description of IBC existence proofs.
modules/apps/callbacks/testing/simapp/app.go, modules/light-clients/08-wasm/testing/simapp/app.go, testing/README.md, testing/simapp/app.go Corrected various comments for clarity and consistency.
docs/versioned_docs/version-v7.5.x/03-light-clients/01-developer-guide/06-proofs.md Revised wording in IBC existence proofs explanation.

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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 13

Configuration used: .coderabbit.yml

Commits Files that changed from the base of the PR and between 06fd8eb and 5409d7f.
Files selected for processing (12)
  • docs/docs/03-light-clients/01-developer-guide/01-overview.md (1 hunks)
  • docs/docs/03-light-clients/01-developer-guide/02-light-client-module.md (3 hunks)
  • docs/docs/03-light-clients/01-developer-guide/05-updates-and-misbehaviour.md (3 hunks)
  • docs/docs/03-light-clients/01-developer-guide/06-upgrades.md (2 hunks)
  • docs/docs/03-light-clients/01-developer-guide/07-proofs.md (2 hunks)
  • docs/docs/03-light-clients/01-developer-guide/08-proposals.md (1 hunks)
  • docs/docs/03-light-clients/01-developer-guide/09-setup.md (1 hunks)
  • docs/docs/03-light-clients/02-localhost/03-client-state.md (1 hunks)
  • docs/docs/03-light-clients/02-localhost/05-state-verification.md (1 hunks)
  • docs/docs/03-light-clients/04-wasm/02-concepts.md (1 hunks)
  • docs/docs/03-light-clients/04-wasm/03-integration.md (1 hunks)
  • docs/docs/03-light-clients/04-wasm/04-messages.md (1 hunks)
Additional comments: 19
docs/docs/03-light-clients/02-localhost/05-state-verification.md (2)
  • 11-11: The documentation mentions the transition from ClientState to LightClientModule for state verification methods. Ensure that this change is reflected across all relevant parts of the documentation and codebase to maintain consistency.
  • 13-13: Consider adding a comma after "packets" for better readability: "When verifying channel state in handshakes or processing packets, the 09-localhost client can simply compare bytes..."
docs/docs/03-light-clients/02-localhost/03-client-state.md (1)
  • 25-30: Ensure that the error handling in CreateLocalhostClient is comprehensive and aligns with the overall error handling strategy of the ibc-go project. Specifically, consider whether additional context or logging could be beneficial when the route is not found.
docs/docs/03-light-clients/01-developer-guide/08-proposals.md (1)
  • 11-11: The renaming of CheckSubstituteAndUpdateState to RecoverClient is a significant change. Ensure that this renaming is consistently applied across all documentation, code comments, and examples to avoid confusion.
docs/docs/03-light-clients/04-wasm/04-messages.md (1)
  • 30-30: The update to the link for the Initialize function implementation is crucial for ensuring that readers are directed to the correct version of the code. Verify that all links in the documentation point to the correct and most relevant versions of the code or documentation.
docs/docs/03-light-clients/01-developer-guide/02-light-client-module.md (1)
  • 30-54: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [11-80]

The addition of new methods to the LightClientModule interface significantly enhances the functionality and flexibility of light client modules. Ensure that these methods are well-documented, with clear examples and use cases provided to help developers understand how to implement and use them effectively.

docs/docs/03-light-clients/01-developer-guide/07-proofs.md (1)
  • 32-47: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [11-11]

Consider revising "IBC uses merkle proofs in order to verify the state of a remote counterparty state machine" to avoid wordiness. A more concise version could be "IBC uses merkle proofs to verify the state of a remote counterparty state machine."

docs/docs/03-light-clients/04-wasm/02-concepts.md (1)
  • 14-14: The explanation of the 08-wasm module acting as a proxy light client is clear and informative. It's important for developers to understand the distinction between a regular light client and a proxy light client, and this section does a good job of explaining that. However, adding a brief example of how a Wasm light client is uploaded and utilized through the 08-wasm module could further enhance understanding.
docs/docs/03-light-clients/01-developer-guide/06-upgrades.md (2)
  • 18-28: The update to the VerifyUpgradeAndUpdateState method, changing its parameters to include LightClientModule instead of ClientState, is a significant change. It's crucial to ensure that the documentation clearly explains the rationale behind this change and how it affects the upgrade process. Additionally, providing an example of using the updated method could greatly aid developers in understanding its application.
  • 18-22: Consider revising the note to improve clarity and conciseness. For example: "Note: Proof heights are not included because upgrades are expected to pass only at the current revision's last committed height. Clients must ensure that the current revision's planned last height is encoded in the proof verification process to prevent premature upgrades. This precaution helps ensure that upgrade plans, which may be cancelled or modified, are only executed as intended."
docs/docs/03-light-clients/01-developer-guide/05-updates-and-misbehaviour.md (2)
  • 33-41: The documentation effectively explains the role of the ClientMessage interface and its methods in handling updates and misbehaviour. It might be helpful to include a note on the importance of implementing these methods correctly to ensure the security and reliability of the light client.

Consider adding a note emphasizing the critical nature of correctly implementing the ClientMessage interface methods to maintain the security and reliability of the light client.

  • 30-49: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [46-56]

The section on VerifyClientMessage provides a clear explanation of its purpose and requirements. However, it would be beneficial to explicitly mention that the method should differentiate between different types of ClientMessage (e.g., Header, Misbehaviour) and handle them accordingly.

+ It's crucial that `VerifyClientMessage` differentiates between different types of `ClientMessage`, such as `Header` and `Misbehaviour`, and processes them according to their specific verification logic.
docs/docs/03-light-clients/01-developer-guide/01-overview.md (1)
  • 23-29: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [26-33]

The documentation provides a comprehensive overview of the interfaces required for IBC light client module developers. It might enhance clarity to briefly mention the significance of each interface in the context of IBC operations.

Consider adding a sentence or two summarizing the role of each interface (LightClientModule, ClientState, ConsensusState, ClientMessage) in the broader context of IBC operations for enhanced clarity.

docs/docs/03-light-clients/04-wasm/03-integration.md (6)
  • 71-81: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [10-10]

Near line 10, there's a possible spelling mistake in the link text "x/wasm module". Ensure the module name is correctly spelled as it's crucial for accurate documentation.

  • 71-81: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [12-12]

The sentence starting near line 12 does not begin with an uppercase letter. Consider revising for consistency and readability.

  • 71-81: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [222-222]

The sentence starting with "- DataDir is the" does not begin with an uppercase letter. Consider revising for consistency.

  • 71-81: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [223-223]

The sentence starting with "- SupportedCapabilities is a" does not begin with an uppercase letter. Consider revising for consistency.

  • 71-81: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [224-224]

The sentence starting with "- MemoryCacheSize sets" does not begin with an uppercase letter. Consider revising for consistency.

  • 71-81: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [225-225]

The sentence starting with "- ContractDebugMode is a" does not begin with an uppercase letter. Consider revising for consistency.


## `VerifyUpgradeAndUpdateState` method

`VerifyUpgradeAndUpdateState` provides a path to upgrading clients given an upgraded `ClientState`, upgraded `ConsensusState` and proofs for each. See section [Implementing `VerifyUpgradeAndUpdateState`](./06-upgrades.md#implementing-verifyupgradeandupdatestate) for more information.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file should end with a single newline character to comply with common coding standards and ensure compatibility with various text editors and version control systems.

+ 

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.

Suggested change
`VerifyUpgradeAndUpdateState` provides a path to upgrading clients given an upgraded `ClientState`, upgraded `ConsensusState` and proofs for each. See section [Implementing `VerifyUpgradeAndUpdateState`](./06-upgrades.md#implementing-verifyupgradeandupdatestate) for more information.
`VerifyUpgradeAndUpdateState` provides a path to upgrading clients given an upgraded `ClientState`, upgraded `ConsensusState` and proofs for each. See section [Implementing `VerifyUpgradeAndUpdateState`](./06-upgrades.md#implementing-verifyupgradeandupdatestate) for more information.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 11

Configuration used: .coderabbit.yml

Commits Files that changed from the base of the PR and between 5409d7f and ebe19d5.
Files selected for processing (4)
  • docs/docs/03-light-clients/01-developer-guide/02-light-client-module.md (3 hunks)
  • docs/docs/03-light-clients/01-developer-guide/05-updates-and-misbehaviour.md (3 hunks)
  • docs/docs/03-light-clients/04-wasm/02-concepts.md (1 hunks)
  • docs/docs/03-light-clients/04-wasm/03-integration.md (3 hunks)
Additional comments: 10
docs/docs/03-light-clients/01-developer-guide/05-updates-and-misbehaviour.md (2)
  • 10-25: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [9-9]

The term "misbehaviour" is used throughout the document, which is British English. While not incorrect, it's important to ensure consistency with the rest of the documentation. If the rest of the ibc-go documentation uses American English, consider changing "misbehaviour" to "misbehavior".

  • 18-18: Consider rephrasing "purposefully kept generic in order to give the maximum amount of flexibility" to avoid wordiness. For example: "intentionally generic to maximize flexibility".
docs/docs/03-light-clients/04-wasm/03-integration.md (8)
  • 14-14: The term 'setup' in "required to set up the 08-wasm module" should be two words when used as a verb. This change has already been correctly applied based on previous feedback.
  • 75-78: The process of adding a route for the Wasm light client module to the client router is correctly implemented. This is a crucial step for integrating the Wasm light client module with the IBC client router, ensuring that the module can handle client-related operations.
  • 133-133: Consider adding a comma after "keeper" for better readability in "When it comes to instantiating 08-wasm's keeper, there are two recommended ways of doing it." This change has already been correctly applied based on previous feedback.
  • 139-139: The phrase "otherwise unexpected behaviour is likely to happen" could be improved for clarity. Consider rephrasing to separate the adverb "otherwise" from the sentence. This change has already been correctly applied based on previous feedback.
  • 139-139: "In order to share the Wasm VM instance, please follow the guideline below." Consider adding a comma after "instance" for better readability. This change has already been correctly applied based on previous feedback.
  • 11-17: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [219-219]

There's a grammatical error in "instantiating a Wasm VM in app.go and pass it to 08-wasm's". It should be "and pass it to". This change has already been correctly applied based on previous feedback.

  • 11-17: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [333-333]

The phrase "then it is not necessary to do anything in order to allow" could be simplified to "it is not necessary to do anything to allow" for conciseness. This change has already been correctly applied based on previous feedback.

  • 11-17: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [386-386]

"In order to use the 08-wasm module, chains must register" simplifies the original sentence for clarity. This change has already been correctly applied based on previous feedback.

@@ -8,6 +8,10 @@ slug: /ibc/light-clients/light-client-module

# Implementing the `LightClientModule` interface

## `RegisterStoreProvider` method

`RegisterStoreProvider` is called by core IBC when a `LightClientModule` is added to the router. It allows the `LightClientModule` to set a `ClientStoreProvider` ([interface defined in `modules/core/exported`](https://github.com/cosmos/ibc-go/blob/06fd8eb5ee1697e3b43be7528a6e42f5e4a4613c/modules/core/exported/client.go#L49-L52)) which supplies isolated prefix client stores to IBC light client instances.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small note: we might remove this #6028. Leaving comment to keep track.

@charleenfei
Copy link
Contributor

took the liberty of implementing some of the coderabbit suggestions!

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 5

Configuration used: .coderabbit.yml

Commits Files that changed from the base of the PR and between ebe19d5 and 25190d2.
Files selected for processing (8)
  • docs/docs/03-light-clients/01-developer-guide/07-proofs.md (3 hunks)
  • docs/versioned_docs/version-v7.3.x/03-light-clients/01-developer-guide/06-proofs.md (1 hunks)
  • docs/versioned_docs/version-v8.1.x/03-light-clients/01-developer-guide/01-overview.md (1 hunks)
  • docs/versioned_docs/version-v8.1.x/03-light-clients/01-developer-guide/06-proofs.md (1 hunks)
  • modules/apps/callbacks/testing/simapp/app.go (1 hunks)
  • modules/light-clients/08-wasm/testing/simapp/app.go (1 hunks)
  • testing/README.md (1 hunks)
  • testing/simapp/app.go (1 hunks)
Files skipped from review due to trivial changes (2)
  • modules/light-clients/08-wasm/testing/simapp/app.go
  • testing/simapp/app.go
Additional comments: 5
docs/versioned_docs/version-v8.1.x/03-light-clients/01-developer-guide/06-proofs.md (1)
  • 21-21: The grammatical correction from "comprises of" to "is comprised of" improves the clarity and correctness of the documentation. Good job on this change.
docs/versioned_docs/version-v7.3.x/03-light-clients/01-developer-guide/06-proofs.md (1)
  • 21-21: The grammatical correction from "comprises of" to "is comprised of" improves the clarity and correctness of the documentation. Good job on this change.
docs/docs/03-light-clients/01-developer-guide/07-proofs.md (1)
  • 32-47: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [35-56]

The updates to the parameters and comments of the VerifyMembership and VerifyNonMembership methods enhance clarity and provide better guidance for developers. These changes are well-integrated into the documentation.

docs/versioned_docs/version-v8.1.x/03-light-clients/01-developer-guide/01-overview.md (1)
  • 26-26: The adjustment in the description clarifies the requirements for developers regarding the implementation of specific interfaces from the modules/core/exported package of ibc-go. This enhances the documentation's clarity and usefulness.
modules/apps/callbacks/testing/simapp/app.go (1)
  • 758-758: The comment update improves grammatical correctness and clarity, making the documentation more informative.

testing/README.md Show resolved Hide resolved
testing/README.md Show resolved Hide resolved
testing/README.md Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Review Status

Configuration used: .coderabbit.yml

Commits Files that changed from the base of the PR and between 25190d2 and 3c4c2a7.
Files selected for processing (3)
  • modules/apps/callbacks/testing/simapp/app.go (1 hunks)
  • modules/light-clients/08-wasm/testing/simapp/app.go (1 hunks)
  • testing/simapp/app.go (1 hunks)
Files skipped from review due to trivial changes (2)
  • modules/apps/callbacks/testing/simapp/app.go
  • testing/simapp/app.go
Additional Context Used
Path-based Instructions (1)
modules/light-clients/08-wasm/testing/simapp/app.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

Additional comments not posted (1)
modules/light-clients/08-wasm/testing/simapp/app.go (1)

813-813: The correction in the comment improves grammatical accuracy and clarity.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Actionable comments outside the diff hunks (6)
docs/docs/03-light-clients/04-wasm/03-integration.md (6)

Line range hint 10-10: Correct the spelling mistake in the link text.

- [`x/wasm` module](https://github.com/CosmWasm/wasmd/tree/main/x/wasm)
+ [`x/wasm` module](https://github.com/CosmWasm/wasmd/tree/main/x/wasm)

Line range hint 220-220: Correct the typo in the text.

- an pass it to 08-wasm's
+ and pass it to 08-wasm's

Line range hint 222-222: Ensure that the sentence starts with an uppercase letter.

- `DataDir` is the [directory for Wasm blobs and various caches](https://github.com/CosmWasm/wasmvm/blob/v2.0.0/lib.go#L25).
+ `DataDir` is the [directory for Wasm blobs and various caches](https://github.com/CosmWasm/wasmvm/blob/v2.0.0/lib.go#L25).

Line range hint 223-223: Ensure that the sentence starts with an uppercase letter.

- `SupportedCapabilities` is a [list of capabilities supported by the chain](https://github.com/CosmWasm/wasmvm/blob/v2.0.0/lib.go#L26).
+ `SupportedCapabilities` is a [list of capabilities supported by the chain](https://github.com/CosmWasm/wasmvm/blob/v2.0.0/lib.go#L26).

Line range hint 224-224: Ensure that the sentence starts with an uppercase letter.

- `MemoryCacheSize` sets [the size in MiB of an in-memory cache for e.g. module caching](https://github.com/CosmWasm/wasmvm/blob/v2.0.0/lib.go#L29C16-L29C104).
+ `MemoryCacheSize` sets [the size in MiB of an in-memory cache for e.g. module caching](https://github.com/CosmWasm/wasmvm/blob/v2.0.0/lib.go#L29C16-L29C104).

Line range hint 225-225: Ensure that the sentence starts with an uppercase letter.

- `ContractDebugMode` is a [flag to enable/disable printing debug logs from the contract to STDOUT](https://github.com/CosmWasm/wasmvm/blob/v2.0.0/lib.go#L28).
+ `ContractDebugMode` is a [flag to enable/disable printing debug logs from the contract to STDOUT](https://github.com/CosmWasm/wasmvm/blob/v2.0.0/lib.go#L28).

@@ -11,7 +11,7 @@ Learn how to integrate the `08-wasm` module in a chain binary and about the reco

## `app.go` setup
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that the section title starts with an uppercase letter.

- ## `app.go` setup
+ ## `App.go` Setup

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.

Suggested change
## `app.go` setup
## `App.go` Setup

@@ -125,13 +130,13 @@

## Keeper instantiation

When it comes to instantiating `08-wasm`'s keeper there are two recommended ways of doing it. Choosing one or the other will depend on whether the chain already integrates [`x/wasm`](https://github.com/CosmWasm/wasmd/tree/main/x/wasm) or not.
When it comes to instantiating `08-wasm`'s keeper, there are two recommended ways of doing it. Choosing one or the other will depend on whether the chain already integrates [`x/wasm`](https://github.com/CosmWasm/wasmd/tree/main/x/wasm) or not.

### If `x/wasm` is present

If the chain where the module is integrated uses `x/wasm` then we recommend that both `08-wasm` and `x/wasm` share the same Wasm VM instance. Having two separate Wasm VM instances is still possible, but care should be taken to make sure that both instances do not share the directory when the VM stores blobs and various caches, otherwise unexpected behaviour is likely to happen (from `x/wasm` v0.51 and `08-wasm` v0.2.0+ibc-go-v8.3-wasmvm-v2.0 this will be forbidden anyway, since wasmvm v2.0.0 and above will not allow two different Wasm VM instances to shared the same data folder).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separate the adverb "otherwise" from the sentence for clarity.

Consider rephrasing to improve clarity:

- otherwise unexpected behaviour is likely to happen
+ otherwise, unexpected behaviour is likely to happen

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.

Suggested change
If the chain where the module is integrated uses `x/wasm` then we recommend that both `08-wasm` and `x/wasm` share the same Wasm VM instance. Having two separate Wasm VM instances is still possible, but care should be taken to make sure that both instances do not share the directory when the VM stores blobs and various caches, otherwise unexpected behaviour is likely to happen (from `x/wasm` v0.51 and `08-wasm` v0.2.0+ibc-go-v8.3-wasmvm-v2.0 this will be forbidden anyway, since wasmvm v2.0.0 and above will not allow two different Wasm VM instances to shared the same data folder).
If the chain where the module is integrated uses `x/wasm` then we recommend that both `08-wasm` and `x/wasm` share the same Wasm VM instance. Having two separate Wasm VM instances is still possible, but care should be taken to make sure that both instances do not share the directory when the VM stores blobs and various caches, otherwise, unexpected behaviour is likely to happen (from `x/wasm` v0.51 and `08-wasm` v0.2.0+ibc-go-v8.3-wasmvm-v2.0 this will be forbidden anyway, since wasmvm v2.0.0 and above will not allow two different Wasm VM instances to shared the same data folder).

Use American English spelling for consistency in documentation.

- behaviour
+ behavior

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.

Suggested change
If the chain where the module is integrated uses `x/wasm` then we recommend that both `08-wasm` and `x/wasm` share the same Wasm VM instance. Having two separate Wasm VM instances is still possible, but care should be taken to make sure that both instances do not share the directory when the VM stores blobs and various caches, otherwise unexpected behaviour is likely to happen (from `x/wasm` v0.51 and `08-wasm` v0.2.0+ibc-go-v8.3-wasmvm-v2.0 this will be forbidden anyway, since wasmvm v2.0.0 and above will not allow two different Wasm VM instances to shared the same data folder).
If the chain where the module is integrated uses `x/wasm` then we recommend that both `08-wasm` and `x/wasm` share the same Wasm VM instance. Having two separate Wasm VM instances is still possible, but care should be taken to make sure that both instances do not share the directory when the VM stores blobs and various caches, otherwise unexpected behavior is likely to happen (from `x/wasm` v0.51 and `08-wasm` v0.2.0+ibc-go-v8.3-wasmvm-v2.0 this will be forbidden anyway, since wasmvm v2.0.0 and above will not allow two different Wasm VM instances to shared the same data folder).

@@ -125,13 +130,13 @@

## Keeper instantiation

When it comes to instantiating `08-wasm`'s keeper there are two recommended ways of doing it. Choosing one or the other will depend on whether the chain already integrates [`x/wasm`](https://github.com/CosmWasm/wasmd/tree/main/x/wasm) or not.
When it comes to instantiating `08-wasm`'s keeper, there are two recommended ways of doing it. Choosing one or the other will depend on whether the chain already integrates [`x/wasm`](https://github.com/CosmWasm/wasmd/tree/main/x/wasm) or not.

### If `x/wasm` is present

If the chain where the module is integrated uses `x/wasm` then we recommend that both `08-wasm` and `x/wasm` share the same Wasm VM instance. Having two separate Wasm VM instances is still possible, but care should be taken to make sure that both instances do not share the directory when the VM stores blobs and various caches, otherwise unexpected behaviour is likely to happen (from `x/wasm` v0.51 and `08-wasm` v0.2.0+ibc-go-v8.3-wasmvm-v2.0 this will be forbidden anyway, since wasmvm v2.0.0 and above will not allow two different Wasm VM instances to shared the same data folder).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplify the phrase for conciseness.

- In order to share the Wasm VM instance, please follow the guideline below.
+ To share the Wasm VM instance, please follow the guideline below.

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.

Suggested change


### If `x/wasm` is present

If the chain where the module is integrated uses `x/wasm` then we recommend that both `08-wasm` and `x/wasm` share the same Wasm VM instance. Having two separate Wasm VM instances is still possible, but care should be taken to make sure that both instances do not share the directory when the VM stores blobs and various caches, otherwise unexpected behaviour is likely to happen (from `x/wasm` v0.51 and `08-wasm` v0.2.0+ibc-go-v8.3-wasmvm-v2.0 this will be forbidden anyway, since wasmvm v2.0.0 and above will not allow two different Wasm VM instances to shared the same data folder).

In order to share the Wasm VM instance please follow the guideline below. Please note that this requires `x/wasm` v0.41 or above.
In order to share the Wasm VM instance, please follow the guideline below. Please note that this requires `x/wasm` v0.41 or above.

- Instantiate the Wasm VM in `app.go` with the parameters of your choice.
- [Create an `Option` with this Wasm VM instance](https://github.com/CosmWasm/wasmd/blob/db93d7b6c7bb6f4a340d74b96a02cec885729b59/x/wasm/keeper/options.go#L21-L25).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that the link is correctly formatted.

- [Create an `Option` with this Wasm VM instance](https://github.com/CosmWasm/wasmd/blob/db93d7b6c7bb6f4a340d74b96a02cec885729b59/x/wasm/keeper/options.go#L21-L25)
+ [Create an `Option` with this Wasm VM instance](https://github.com/CosmWasm/wasmd/blob/db93d7b6c7bb6f4a340d74b96a02cec885729b59/x/wasm/keeper/options.go#L21-L25)

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.

Suggested change
- [Create an `Option` with this Wasm VM instance](https://github.com/CosmWasm/wasmd/blob/db93d7b6c7bb6f4a340d74b96a02cec885729b59/x/wasm/keeper/options.go#L21-L25).
- [Create an `Option` with this Wasm VM instance](https://github.com/CosmWasm/wasmd/blob/db93d7b6c7bb6f4a340d74b96a02cec885729b59/x/wasm/keeper/options.go#L21-L25).

Copy link

sonarcloud bot commented Apr 22, 2024

Quality Gate Passed Quality Gate passed for 'ibc-go'

Issues
7 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Outside diff range and nitpick comments (1)
docs/docs/03-light-clients/01-developer-guide/02-light-client-module.md (1)

Line range hint 26-26: Improve grammatical structure for clarity.

- This field is returned in the response of the gRPC [`ibc.core.client.v1.Query/ClientStatus`](https://github.com/cosmos/ibc-go/blob/v7.0.0/modules/core/02-client/types/query.pb.go#L665) endpoint.
+ This field is returned to the response of the gRPC [`ibc.core.client.v1.Query/ClientStatus`](https://github.com/cosmos/ibc-go/blob/v7.0.0/modules/core/02-client/types/query.pb.go#L665) endpoint.
Review Details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits Files that changed from the base of the PR and between ee2c93f and 55bf147.
Files selected for processing (1)
  • docs/docs/03-light-clients/01-developer-guide/02-light-client-module.md (3 hunks)
Additional Context Used
LanguageTool (2)
docs/docs/03-light-clients/01-developer-guide/02-light-client-module.md (2)

Near line 26: The usual collocation for “returned” is “to”, not “in”.
Context: ...rted/client.go#L22-L32). This field is returned in the response of the gRPC [`ibc.core.cli...


Near line 50: Possible missing comma found.
Context: ...m appropriate state changes on a client state given that misbehaviour has been detect...

Path-based Instructions (1)
docs/docs/03-light-clients/01-developer-guide/02-light-client-module.md (1)

Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"


## `UpdateStateOnMisbehaviour` method

`UpdateStateOnMisbehaviour` should perform appropriate state changes on a client state given that misbehaviour has been detected and verified. See section [`UpdateStateOnMisbehaviour`](05-updates-and-misbehaviour.md#updatestateonmisbehaviour) for more information.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insert a comma for better readability.

- `UpdateStateOnMisbehaviour` should perform appropriate state changes on a client state given that misbehaviour has been detected and verified.
+ `UpdateStateOnMisbehaviour` should perform appropriate state changes on a client state, given that misbehaviour has been detected and verified.

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.

Suggested change
`UpdateStateOnMisbehaviour` should perform appropriate state changes on a client state given that misbehaviour has been detected and verified. See section [`UpdateStateOnMisbehaviour`](05-updates-and-misbehaviour.md#updatestateonmisbehaviour) for more information.
`UpdateStateOnMisbehaviour` should perform appropriate state changes on a client state, given that misbehaviour has been detected and verified. See section [`UpdateStateOnMisbehaviour`](05-updates-and-misbehaviour.md#updatestateonmisbehaviour) for more information.

@@ -8,6 +8,10 @@ slug: /ibc/light-clients/light-client-module

# Implementing the `LightClientModule` interface

## `RegisterStoreProvider` method

`RegisterStoreProvider` is called by core IBC when a `LightClientModule` is added to the router. It allows the `LightClientModule` to set a `ClientStoreProvider` ([interface defined in `modules/core/exported`](https://github.com/cosmos/ibc-go/blob/06fd8eb5ee1697e3b43be7528a6e42f5e4a4613c/modules/core/exported/client.go#L49-L52)) which supplies isolated prefix client stores to IBC light client instances.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarify the beginning of the sentence for grammatical consistency.

- `RegisterStoreProvider` is called by core IBC when a `LightClientModule` is added to the router.
+ The `RegisterStoreProvider` method is called by core IBC when a `LightClientModule` is added to the router.

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.

Suggested change
`RegisterStoreProvider` is called by core IBC when a `LightClientModule` is added to the router. It allows the `LightClientModule` to set a `ClientStoreProvider` ([interface defined in `modules/core/exported`](https://github.com/cosmos/ibc-go/blob/06fd8eb5ee1697e3b43be7528a6e42f5e4a4613c/modules/core/exported/client.go#L49-L52)) which supplies isolated prefix client stores to IBC light client instances.
`RegisterStoreProvider` is called by core IBC when a `LightClientModule` is added to the router. It allows the `LightClientModule` to set a `ClientStoreProvider` ([interface defined in `modules/core/exported`](https://github.com/cosmos/ibc-go/blob/06fd8eb5ee1697e3b43be7528a6e42f5e4a4613c/modules/core/exported/client.go#L49-L52)) which supplies isolated prefix client stores to IBC light client instances.
```
```suggestion
The `RegisterStoreProvider` method is called by core IBC when a `LightClientModule` is added to the router. It allows the `LightClientModule` to set a `ClientStoreProvider` ([interface defined in `modules/core/exported`](https://github.com/cosmos/ibc-go/blob/06fd8eb5ee1697e3b43be7528a6e42f5e4a4613c/modules/core/exported/client.go#L49-L52)) which supplies isolated prefix client stores to IBC light client instances.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (8)
docs/versioned_docs/version-v7.5.x/03-light-clients/01-developer-guide/06-proofs.md (2)

Line range hint 64-64: Add a hyphen to "zero proof" for clarity.

- A zero proof height is now allowed by core IBC and may be passed into `VerifyMembership` and `VerifyNonMembership`.
+ A zero-proof height is now allowed by core IBC and may be passed into `VerifyMembership` and `VerifyNonMembership`.

Line range hint 64-64: Add a hyphen to "zero proof" in the second occurrence.

- Light clients are responsible for returning an error if a zero proof height is invalid behaviour.
+ Light clients are responsible for returning an error if a zero-proof height is invalid behaviour.
docs/docs/03-light-clients/04-wasm/03-integration.md (6)

Line range hint 236-236: Correct a typo in the documentation.

- (e.g. instantiating a Wasm VM in app.go an pass it to 08-wasm's [`NewKeeperWithVM` constructor function]
+ (e.g. instantiating a Wasm VM in app.go and pass it to 08-wasm's [`NewKeeperWithVM` constructor function]

Line range hint 281-281: Add a comma for better readability.

- Currently the only option available is the `WithQueryPlugins` option, which allows registration of custom query plugins for the `08-wasm` module.
+ Currently, the only option available is the `WithQueryPlugins` option, which allows registration of custom query plugins for the `08-wasm` module.

Line range hint 282-282: Add a comma before 'and' to connect two independent clauses.

- The use of this API is optional and it is only required if the chain wants to register custom query plugins for the `08-wasm` module.
+ The use of this API is optional, and it is only required if the chain wants to register custom query plugins for the `08-wasm` module.

Line range hint 303-303: Correct the word usage for clarity.

- The `Stargate` querier appends the user defined accept list of query routes to a default list defined by the `08-wasm` module.
+ The `Stargate` querier appends the user-defined accept list of query routes to a default list defined by the `08-wasm` module.

Line range hint 349-349: Simplify the sentence for conciseness.

- then it is not necessary to do anything in order to allow the creation of `08-wasm` clients.
+ then it is not necessary to do anything to allow the creation of `08-wasm` clients.

Line range hint 402-402: Simplify the sentence for conciseness in the context of adding snapshot support.

- In order to use the `08-wasm` module chains are required to register the `WasmSnapshotter` extension in the snapshot manager.
+ To use the `08-wasm` module, chains are required to register the `WasmSnapshotter` extension in the snapshot manager.
Review Details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits Files that changed from the base of the PR and between 55bf147 and fcb78b8.
Files selected for processing (2)
  • docs/docs/03-light-clients/04-wasm/03-integration.md (3 hunks)
  • docs/versioned_docs/version-v7.5.x/03-light-clients/01-developer-guide/06-proofs.md (1 hunks)
Additional Context Used
LanguageTool (14)
docs/docs/03-light-clients/04-wasm/03-integration.md (10)

Near line 153: The word “otherwise” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...ry when the VM stores blobs and various caches, otherwise unexpected behaviour is likely to happe...


Near line 154: Consider a shorter alternative to avoid wordiness.
Context: ...ances to shared the same data folder). In order to share the Wasm VM instance, please foll...


Near line 236: Did you mean “and pass it”?
Context: ...(e.g. instantiating a Wasm VM in app.go an pass it to 08-wasm's [NewKeeperWithVM constru...


Near line 281: A comma may be missing after the conjunctive/linking adverb ‘Currently’.
Context: ...ns API inspired by the one in x/wasm. Currently the only option available is the `WithQ...


Near line 282: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... module. The use of this API is optional and it is only required if the chain wants ...


Near line 303: Did you mean “except”?
Context: ...rgate` querier appends the user defined accept list of query routes to a default list ...


Near line 304: Unpaired symbol: ‘"’ seems to be missing
Context: ...ceptListdefines a single query route:"/ibc.core.client.v1.Query/VerifyMembership"`. This allows for light client smart co...


Near line 349: Consider a shorter alternative to avoid wordiness.
Context: ...then it is not necessary to do anything in order to allow the creation of 08-wasm clients...


Near line 349: Consider a shorter alternative to avoid wordiness.
Context: ...-solomachine", "07-tendermint"]), then in order to use the 08-wasm` module chains must up...


Near line 402: Consider a shorter alternative to avoid wordiness.
Context: ... } } ``` ## Adding snapshot support In order to use the 08-wasm module chains are req...

docs/versioned_docs/version-v7.5.x/03-light-clients/01-developer-guide/06-proofs.md (4)

Near line 11: Consider a shorter alternative to avoid wordiness.
Context: ...istence proofs IBC uses merkle proofs in order to verify the state of a remote counterpar...


Near line 13: ‘with regard to’ might be wordy. Consider a shorter alternative.
Context: ...binary merkle tree format. Specifically with regard to IBC, core IBC maintains its own IAVL st...


Near line 64: There should be a hyphen between ‘zero’ and ‘proof’ unless this is a model number designation.
Context: ...r non-packet processing verification. A zero proof height is now allowed by core IBC and m...


Near line 64: There should be a hyphen between ‘zero’ and ‘proof’ unless this is a model number designation.
Context: ...responsible for returning an error if a zero proof height is invalid behaviour. Please re...

Markdownlint (4)
docs/docs/03-light-clients/04-wasm/03-integration.md (2)

10: Expected: 0 or 2; Actual: 1
Trailing spaces


8: null
Multiple top-level headings in the same document

docs/versioned_docs/version-v7.5.x/03-light-clients/01-developer-guide/06-proofs.md (2)

9: Expected: 0 or 2; Actual: 1
Trailing spaces


9: null
Multiple top-level headings in the same document

Path-based Instructions (2)
docs/versioned_docs/version-v7.5.x/03-light-clients/01-developer-guide/06-proofs.md (1)

Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"

docs/docs/03-light-clients/04-wasm/03-integration.md (1)

Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"

Learnings (1)
docs/docs/03-light-clients/04-wasm/03-integration.md (1)
User: damiannolan
PR: cosmos/ibc-go#5923
File: modules/light-clients/08-wasm/keeper/keeper_no_vm.go:14-42
Timestamp: 2024-03-15T11:03:19.887Z
Learning: The `NewKeeperWithVM` and `NewKeeperWithConfig` functions in the `08-wasm` module of the Cosmos IBC Go repository are designed to be placeholders that will panic if used without the necessary wasmvm dependencies, specifically when the binary is compiled with `ibcwasm_novm`. This design ensures explicit and controlled usage under specific compilation conditions.
Additional comments not posted (1)
docs/versioned_docs/version-v7.5.x/03-light-clients/01-developer-guide/06-proofs.md (1)

21-21: Correct grammatical error in the description of existence proofs.

- an IBC existence proof is comprised of two  proofs
+ an IBC existence proof is comprised of two proofs

Likely invalid or redundant comment.


### If `x/wasm` is present

If the chain where the module is integrated uses `x/wasm` then we recommend that both `08-wasm` and `x/wasm` share the same Wasm VM instance. Having two separate Wasm VM instances is still possible, but care should be taken to make sure that both instances do not share the directory when the VM stores blobs and various caches, otherwise unexpected behaviour is likely to happen (from `x/wasm` v0.51 and `08-wasm` v0.2.0+ibc-go-v8.3-wasmvm-v2.0 this will be forbidden anyway, since wasmvm v2.0.0 and above will not allow two different Wasm VM instances to shared the same data folder).

In order to share the Wasm VM instance please follow the guideline below. Please note that this requires `x/wasm` v0.41 or above.
In order to share the Wasm VM instance, please follow the guideline below. Please note that this requires `x/wasm` v0.41 or above.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct the grammatical structure for clarity.

- In order to share the Wasm VM instance, please follow the guideline below.
+ To share the Wasm VM instance, please follow the guidelines below.

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.

Suggested change
In order to share the Wasm VM instance, please follow the guideline below. Please note that this requires `x/wasm` v0.41 or above.
To share the Wasm VM instance, please follow the guidelines below. Please note that this requires `x/wasm` v0.41 or above.

@@ -27,7 +27,7 @@

## `app.go` setup

The sample code below shows the relevant integration points in `app.go` required to setup the `08-wasm` module in a chain binary. Since `08-wasm` is a light client module itself, please check out as well the section [Integrating light clients](../../01-ibc/02-integration.md#integrating-light-clients) for more information:
The sample code below shows the relevant integration points in `app.go` required to set up the `08-wasm` module in a chain binary. Since `08-wasm` is a light client module itself, please check out as well the section [Integrating light clients](../../01-ibc/02-integration.md#integrating-light-clients) for more information:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure consistent terminology in the setup instructions.

- The sample code below shows the relevant integration points in `app.go` required to set up the `08-wasm` module in a chain binary.
+ The sample code below shows the relevant integration points in `app.go` required to set up the `08-wasm` module in a chain binary. Ensure that the `08-wasm` module is properly integrated as per these guidelines.

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.

Suggested change
The sample code below shows the relevant integration points in `app.go` required to set up the `08-wasm` module in a chain binary. Since `08-wasm` is a light client module itself, please check out as well the section [Integrating light clients](../../01-ibc/02-integration.md#integrating-light-clients) for more information:
The sample code below shows the relevant integration points in `app.go` required to set up the `08-wasm` module in a chain binary. Ensure that the `08-wasm` module is properly integrated as per these guidelines. Since `08-wasm` is a light client module itself, please check out as well the section [Integrating light clients](../../01-ibc/02-integration.md#integrating-light-clients) for more information:

@@ -141,13 +146,13 @@

## Keeper instantiation

When it comes to instantiating `08-wasm`'s keeper there are two recommended ways of doing it. Choosing one or the other will depend on whether the chain already integrates [`x/wasm`](https://github.com/CosmWasm/wasmd/tree/main/x/wasm) or not.
When it comes to instantiating `08-wasm`'s keeper, there are two recommended ways of doing it. Choosing one or the other will depend on whether the chain already integrates [`x/wasm`](https://github.com/CosmWasm/wasmd/tree/main/x/wasm) or not.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarify the sharing of Wasm VM instances to avoid potential misconfigurations.

- Having two separate Wasm VM instances is still possible, but care should be taken to make sure that both instances do not share the directory when the VM stores blobs and various caches, otherwise unexpected behaviour is likely to happen
+ Having two separate Wasm VM instances is still possible, but it is crucial to ensure that both instances do not share the directory where the VM stores blobs and various caches. Otherwise, unexpected behavior is likely to occur.

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.

Suggested change
When it comes to instantiating `08-wasm`'s keeper, there are two recommended ways of doing it. Choosing one or the other will depend on whether the chain already integrates [`x/wasm`](https://github.com/CosmWasm/wasmd/tree/main/x/wasm) or not.
When it comes to instantiating `08-wasm`'s keeper, there are two recommended ways of doing it. Choosing one or the other will depend on whether the chain already integrates [`x/wasm`](https://github.com/CosmWasm/wasmd/tree/main/x/wasm) or not.
Having two separate Wasm VM instances is still possible, but it is crucial to ensure that both instances do not share the directory where the VM stores blobs and various caches. Otherwise, unexpected behavior is likely to occur.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Review Details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits Files that changed from the base of the PR and between fcb78b8 and 44c68b7.
Files selected for processing (3)
  • docs/docs/03-light-clients/01-developer-guide/07-proofs.md (3 hunks)
  • docs/versioned_docs/version-v7.5.x/03-light-clients/01-developer-guide/06-proofs.md (1 hunks)
  • docs/versioned_docs/version-v8.3.x/03-light-clients/01-developer-guide/06-proofs.md (1 hunks)
Additional Context Used
LanguageTool (16)
docs/docs/03-light-clients/01-developer-guide/07-proofs.md (6)

Near line 11: Consider a shorter alternative to avoid wordiness.
Context: ...xistence proofs IBC uses merkle proofs in order to verify the state of a remote counterpar...


Near line 13: ‘with regard to’ might be wordy. Consider a shorter alternative.
Context: ...binary merkle tree format. Specifically with regard to IBC, core IBC maintains its own IAVL st...


Near line 19: Possible missing comma found.
Context: ...roofs Existence proofs are used in IBC transactions which involve verification of counterpa...


Near line 29: Possible missing article found.
Context: ...roofs if the counterparty does not have ability to prove absence. Since the verificatio...


Near line 69: There should be a hyphen between ‘zero’ and ‘proof’ unless this is a model number designation.
Context: ...r non-packet processing verification. A zero proof height is now allowed by core IBC and m...


Near line 69: There should be a hyphen between ‘zero’ and ‘proof’ unless this is a model number designation.
Context: ...responsible for returning an error if a zero proof height is invalid behaviour. Please re...

docs/versioned_docs/version-v7.5.x/03-light-clients/01-developer-guide/06-proofs.md (5)

Near line 11: Consider a shorter alternative to avoid wordiness.
Context: ...istence proofs IBC uses merkle proofs in order to verify the state of a remote counterpar...


Near line 13: ‘with regard to’ might be wordy. Consider a shorter alternative.
Context: ...binary merkle tree format. Specifically with regard to IBC, core IBC maintains its own IAVL st...


Near line 19: Possible missing comma found.
Context: ...roofs Existence proofs are used in IBC transactions which involve verification of counterpa...


Near line 64: There should be a hyphen between ‘zero’ and ‘proof’ unless this is a model number designation.
Context: ...r non-packet processing verification. A zero proof height is now allowed by core IBC and m...


Near line 64: There should be a hyphen between ‘zero’ and ‘proof’ unless this is a model number designation.
Context: ...responsible for returning an error if a zero proof height is invalid behaviour. Please re...

docs/versioned_docs/version-v8.3.x/03-light-clients/01-developer-guide/06-proofs.md (5)

Near line 11: Consider a shorter alternative to avoid wordiness.
Context: ...xistence proofs IBC uses merkle proofs in order to verify the state of a remote counterpar...


Near line 13: ‘with regard to’ might be wordy. Consider a shorter alternative.
Context: ...binary merkle tree format. Specifically with regard to IBC, core IBC maintains its own IAVL st...


Near line 19: Possible missing comma found.
Context: ...roofs Existence proofs are used in IBC transactions which involve verification of counterpa...


Near line 64: There should be a hyphen between ‘zero’ and ‘proof’ unless this is a model number designation.
Context: ...r non-packet processing verification. A zero proof height is now allowed by core IBC and m...


Near line 64: There should be a hyphen between ‘zero’ and ‘proof’ unless this is a model number designation.
Context: ...responsible for returning an error if a zero proof height is invalid behaviour. Please re...

Markdownlint (4)
docs/docs/03-light-clients/01-developer-guide/07-proofs.md (1)

9: null
Multiple top-level headings in the same document

docs/versioned_docs/version-v7.5.x/03-light-clients/01-developer-guide/06-proofs.md (2)

9: Expected: 0 or 2; Actual: 1
Trailing spaces


9: null
Multiple top-level headings in the same document

docs/versioned_docs/version-v8.3.x/03-light-clients/01-developer-guide/06-proofs.md (1)

9: null
Multiple top-level headings in the same document

Path-based Instructions (3)
docs/versioned_docs/version-v8.3.x/03-light-clients/01-developer-guide/06-proofs.md (1)

Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"

docs/versioned_docs/version-v7.5.x/03-light-clients/01-developer-guide/06-proofs.md (1)

Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"

docs/docs/03-light-clients/01-developer-guide/07-proofs.md (1)

Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"

@@ -18,7 +18,7 @@ For the purposes of ibc-go, there are two types of proofs which are important: e

Existence proofs are used in IBC transactions which involve verification of counterparty state for transactions which will result in the writing of provable state. For example, this includes verification of IBC store state for handshakes and packets.

Put simply, existence proofs prove that a particular key and value exists in the tree. Under the hood, an IBC existence proof comprises of two proofs: an IAVL proof that the key exists in IBC store/IBC root hash, and a proof that the IBC root hash exists in the multistore root hash.
Put simply, existence proofs prove that a particular key and value exists in the tree. Under the hood, an IBC existence proof is comprised of two proofs: an IAVL proof that the key exists in IBC store/IBC root hash, and a proof that the IBC root hash exists in the multistore root hash.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider revising the phrase "is comprised of" to "comprises" for grammatical correctness and conciseness.

- is comprised of two proofs
+ comprises two proofs

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.

Suggested change
Put simply, existence proofs prove that a particular key and value exists in the tree. Under the hood, an IBC existence proof is comprised of two proofs: an IAVL proof that the key exists in IBC store/IBC root hash, and a proof that the IBC root hash exists in the multistore root hash.
Put simply, existence proofs prove that a particular key and value exists in the tree. Under the hood, an IBC existence proof comprises two proofs: an IAVL proof that the key exists in IBC store/IBC root hash, and a proof that the IBC root hash exists in the multistore root hash.

@@ -18,7 +18,7 @@ For the purposes of ibc-go, there are two types of proofs which are important: e

Existence proofs are used in IBC transactions which involve verification of counterparty state for transactions which will result in the writing of provable state. For example, this includes verification of IBC store state for handshakes and packets.

Put simply, existence proofs prove that a particular key and value exists in the tree. Under the hood, an IBC existence proof comprises of two proofs: an IAVL proof that the key exists in IBC store/IBC root hash, and a proof that the IBC root hash exists in the multistore root hash.
Put simply, existence proofs prove that a particular key and value exists in the tree. Under the hood, an IBC existence proof is comprised of two proofs: an IAVL proof that the key exists in IBC store/IBC root hash, and a proof that the IBC root hash exists in the multistore root hash.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider revising the phrase "is comprised of" to "comprises" for grammatical correctness and conciseness.

- is comprised of two proofs
+ comprises two proofs

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.

Suggested change
Put simply, existence proofs prove that a particular key and value exists in the tree. Under the hood, an IBC existence proof is comprised of two proofs: an IAVL proof that the key exists in IBC store/IBC root hash, and a proof that the IBC root hash exists in the multistore root hash.
Put simply, existence proofs prove that a particular key and value exists in the tree. Under the hood, an IBC existence proof comprises two proofs: an IAVL proof that the key exists in IBC store/IBC root hash, and a proof that the IBC root hash exists in the multistore root hash.

@@ -18,7 +18,7 @@ For the purposes of ibc-go, there are two types of proofs which are important: e

Existence proofs are used in IBC transactions which involve verification of counterparty state for transactions which will result in the writing of provable state. For example, this includes verification of IBC store state for handshakes and packets.

Put simply, existence proofs prove that a particular key and value exists in the tree. Under the hood, an IBC existence proof comprises of two proofs: an IAVL proof that the key exists in IBC store/IBC root hash, and a proof that the IBC root hash exists in the multistore root hash.
Put simply, existence proofs prove that a particular key and value exists in the tree. Under the hood, an IBC existence proof is comprised of two proofs: an IAVL proof that the key exists in IBC store/IBC root hash, and a proof that the IBC root hash exists in the multistore root hash.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider revising the phrase "is comprised of" to "comprises" for grammatical correctness and conciseness.

- is comprised of two proofs
+ comprises two proofs

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.

Suggested change
Put simply, existence proofs prove that a particular key and value exists in the tree. Under the hood, an IBC existence proof is comprised of two proofs: an IAVL proof that the key exists in IBC store/IBC root hash, and a proof that the IBC root hash exists in the multistore root hash.
Put simply, existence proofs prove that a particular key and value exists in the tree. Under the hood, an IBC existence proof comprises two proofs: an IAVL proof that the key exists in IBC store/IBC root hash, and a proof that the IBC root hash exists in the multistore root hash.

Copy link

sonarcloud bot commented May 22, 2024

Quality Gate Passed Quality Gate passed for 'ibc-go'

Issues
8 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@crodriguezvega crodriguezvega merged commit 180501e into main May 22, 2024
79 checks passed
@crodriguezvega crodriguezvega deleted the carlos/update-light-client-docs branch May 22, 2024 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants