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

feat: refactor ICQ module documentation [NTRN-349] #219

Open
wants to merge 36 commits into
base: main
Choose a base branch
from

Conversation

sotnikov-s
Copy link
Contributor

@sotnikov-s sotnikov-s commented Aug 1, 2024

DO NOT MERGE! ACTION REQUIRED AFTER NEUTRON PR HAS BEEN MERGED

task

https://hadronlabs.atlassian.net/browse/NTRN-349

this PR

Redesigns the interchainqueries module documentation by applying the docs design technique described here https://docs.divio.com/documentation-system:

  • simplifies the Overview section by giving only a high level definition to the reader and by moving several topics from the Overview section to Explanation, How To and Known bugs sections;
  • introduces the Explanation section that contains detailed description of various concepts and design decisions of the module;
  • introduces the How To section that contains step-by-step instructions on typical user actions;
  • introduces the API section that describes the module's interface;
  • introduces the Known bugs section that lists currently known issues and possible workarounds related to the module;

related PRs

before the merge

  • after the related PR to Neutron repo merge and a release creation, update the links to go msg/query types and make sure the links work properly.

Copy link
Contributor

@joldie777 joldie777 left a comment

Choose a reason for hiding this comment

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

Fix conflicts please

… ICQ results removal, RPC node configuration)
@sotnikov-s sotnikov-s marked this pull request as ready for review November 14, 2024 11:09
Copy link
Collaborator

@pr0n00gler pr0n00gler left a comment

Choose a reason for hiding this comment

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

Request/Response links lead to v4 version of Neutron (v5 is the latest)

@sotnikov-s
Copy link
Contributor Author

Request/Response links lead to v4 version of Neutron (v5 is the latest)

I find it pointless until the neutron core PR has been merged. When it has been merged and a release created, I'll update the links to the new version with comments introduced in the related PR to neutron core


## How to register and handle a KV Interchain Query with custom keys

If your KV Interchain Query cannot be covered with the helpers from the [Interchain Queries related package](https://docs.rs/neutron-sdk/0.11.0/neutron_sdk/interchain_queries/v045/register_queries/index.html) of [neutron-sdk](https://docs.rs/neutron-sdk/0.11.0/neutron_sdk), you will need to define the KVKeys for your query yourself. For this particular example, let's register an [Account](https://github.com/cosmos/cosmos-sdk/blob/853dbbf3e84900214137805d78e325ecd56fd68f/proto/cosmos/auth/v1beta1/query.proto#L27-L31) Interchain Query to `cosmos-hub` `v21.0.0`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

to cosmos-hub v21.0.0

A bit above in the How to find out what transaction filter to use section you use v19.0.0. Shouldn't be a consistency here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't mean to be consistent in this regard to any extent. why? that's two different examples on two different topics

Copy link
Collaborator

Choose a reason for hiding this comment

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

From the perspective of a reader and if i'm going through this doc, i need at first install Gaia v19 and then for some reason i need v21 - doesn't seems convenient

Copy link
Contributor Author

Choose a reason for hiding this comment

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

additionally to this the currently running version of gaia might be v22 or even greated. we don't want to be this much up-to-date and consistent I think. howto section contains brief guides which target is to show the direction in which to go and the main idea, not a whole tutorial a user will follow from the beginning and to the end. I don't think anyone will really do these steps one by one and they're not designed for this purpose

Copy link
Collaborator

@pr0n00gler pr0n00gler Nov 25, 2024

Choose a reason for hiding this comment

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

we don't want to be this much up-to-date

Agree.

and consistent

Don't quite agree.

I think the consistency is really important in tutorials

Copy link
Contributor Author

Choose a reason for hiding this comment

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

alrighty, I'll do that. as soon as this PR is merged: cosmos/chain-registry#5628

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

quasisamurai
quasisamurai previously approved these changes Nov 22, 2024
Copy link
Contributor

@quasisamurai quasisamurai left a comment

Choose a reason for hiding this comment

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

lgtm!

docs/neutron/modules/interchain-queries/explanation.md Outdated Show resolved Hide resolved
docs/neutron/modules/interchain-queries/explanation.md Outdated Show resolved Hide resolved
docs/neutron/modules/interchain-queries/explanation.md Outdated Show resolved Hide resolved
docs/neutron/modules/interchain-queries/how-to.md Outdated Show resolved Hide resolved

## Configuring your own remote chain RPC node for TX ICQ usage

If running your own RPC node for the target chain, make sure to configure its `pruning` parameter in the [app.toml](https://docs.cosmos.network/v0.50/learn/advanced/config) file and `indexer` parameter in the [config.toml](https://docs.cometbft.com/v0.38/core/configuration) file the way it is sufficient for the transactions filter you define for your queries.
Copy link
Contributor

Choose a reason for hiding this comment

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

are you sure the 'pruning' params are even related to tx index size?
As far as i understand, 'pruning-*' is about application specific storage only, same time 'min-retain-blocks' param in charge of block storage size, not sure which one defines tx indexer behavior

Copy link
Contributor Author

Choose a reason for hiding this comment

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

asked the devops team for help re this


4. The smart contract registers a KV Interchain Query with the necessary set of keys. The registered query is saved in the `interchainqueries` module's state.

5. An [Interchain Query relayer](/neutron/modules/interchain-queries/explanation#what-is-an-interchain-query-relayer) reads the state of the `interchainqueries` module, finds the registered query and its parameters, and performs the `abci_query` RPC. This call returns a set of key-value pairs along with proofs from the `IAVL tree`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
5. An [Interchain Query relayer](/neutron/modules/interchain-queries/explanation#what-is-an-interchain-query-relayer) reads the state of the `interchainqueries` module, finds the registered query and its parameters, and performs the `abci_query` RPC. This call returns a set of key-value pairs along with proofs from the `IAVL tree`.
5. An [Interchain Query relayer](/neutron/modules/interchain-queries/explanation#what-is-an-interchain-query-relayer) reads the state of the `interchainqueries` module, finds the registered query and its parameters, and performs the `abci_query` query to RPC. This call returns a set of key-value pairs along with proofs from the `IAVL tree`.

?

@@ -47,7 +46,7 @@ Typical Flow of TX Interchain Queries Usage:

4. The smart contract registers a TX Interchain Query with the specified set of transaction filters. The registered query is saved in the `interchainqueries` module's state.

5. An Interchain Query relayer reads the state of the `interchainqueries` module, finds the registered query and its parameters, and performs the `tx_search` RPC. This call returns a list of transactions that match the filters and have been successfully processed on the remote chain.
5. An [Interchain Query relayer](/neutron/modules/interchain-queries/explanation#what-is-an-interchain-query-relayer) reads the state of the `interchainqueries` module, finds the registered query and its parameters, and performs the `tx_search` RPC. This call returns a list of transactions that match the filters and have been successfully processed on the remote chain.
Copy link
Collaborator

Choose a reason for hiding this comment

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

tx_search query to RPC.

Or am i mistaken?

## Overview

[Interchain Queries](/neutron/modules/interchain-queries/overview) allow smart contracts to make queries to a remote chain. An ICQ Relayer is a required component for making them possible. It acts as a facilitator between the Neutron chain and a querying chain, gathering queries that are needed to be performed from the Neutron, actually performing them, and eventually making the results available for the Neutron's smart contracts. These three main responsibilities are described in details below.

If you are a smart contracts developer and up to develop your dApp on Neutron, you will most likely need your own ICQ Relayer to manage your Interchain Queries.
If you are a smart contracts developer and up to develop your dApp on Neutron, you will most likely need your own ICQ Relayer to manage your Interchain Queries.
Copy link
Collaborator

Choose a reason for hiding this comment

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

you will most likely

You need your own ICQ relayer instance for sure, there is no public one


KV Interchain Queries rely heavily on the storage layout of the remote chain, including the paths to IAVL leaf nodes and the data models used to represent the stored information. The accuracy and functionality of KV Interchain Queries depend entirely on the correctness of the paths and the consistency of the data models. This tight coupling introduces several considerations and potential challenges for dApp developers. The [neutron-sdk interchain queries related package](https://docs.rs/neutron-sdk/0.11.0/neutron_sdk/interchain_queries/index.html#) includes target chain version separation precisely for this reasoning.

- Each KV Interchain Query requires the exact path to the desired entry in the IAVL tree of the remote chain. These paths are specific to the version and implementation of the modules on the remote chain. If a path is incorrect or becomes outdated, the query will fail.
Copy link
Collaborator

@pr0n00gler pr0n00gler Nov 29, 2024

Choose a reason for hiding this comment

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

If a path is incorrect or becomes outdated, the query will fail.

I think it would be more proper to say, that not a query itself will fail but you will either empty, broken or invalid data. Which is more dangerous than just a query failure.

Copy link
Collaborator

@pr0n00gler pr0n00gler Nov 29, 2024

Choose a reason for hiding this comment

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

Ah, you are saying that below, my bad 🤦

Copy link
Contributor

@NeverHappened NeverHappened left a comment

Choose a reason for hiding this comment

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

love it! added few optional comments


4. The smart contract registers a KV Interchain Query with the necessary set of keys. The registered query is saved in the `interchainqueries` module's state.

5. An [Interchain Query relayer](/neutron/modules/interchain-queries/explanation#what-is-an-interchain-query-relayer) reads the state of the `interchainqueries` module, finds the registered query and its parameters, and performs the `abci_query` RPC. This call returns a set of key-value pairs along with proofs from the `IAVL tree`.
Copy link
Contributor

@NeverHappened NeverHappened Nov 29, 2024

Choose a reason for hiding this comment

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

right now I think it works more like:

  • gets the query from chain, checks that it's time to update data for given query. if it is so, it makes step 6


- **Monitoring registered Interchain Queries**: Fetching the Interchain Queries that need processing from the `interchainqueries` module's state.

- **Executing Interchain Queries**: Accessing the remote chain's state based on the parameters defined in the Interchain Query and obtaining proofs for the retrieved data.
Copy link
Contributor

Choose a reason for hiding this comment

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

How about Fetching Interchain Queries data and proofs? Executing Interchain Queries seems ambiguous


#### 1. Find an existing IBC connection using an explorer

Visit the [map of zones](https://mapofzones.com/zones/neutron-1/peers?columnKey=ibcVolumeIn&period=7d). You may find multiple connections between the two chains. For Neutron and CosmosHub, we’ll use `connection-0`.
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe need to specify that this the connection-0 is the connection id on the neutron side

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.

6 participants