Implement ADR 009: ChainEndpoint and ChainHandle methods standardization

[plafer]

Closes: #2192

Description

I chose not to implement tendermint_proto::Protobuf on the new domain types, as we never need to encode/decode the request itself. We either

1. (gRPC) convert it to the "raw type" and pass that to tonic, or
2. (tendermint rpc) call abci_query using fields inside the request

---

PR author checklist:

• Added changelog entry, using unclog.
• Added tests: integration (for Hermes) or unit/mock tests (for modules).
• Linked to GitHub issue.
• Updated code comments and documentation (e.g., docs/).

Reviewer checklist:

• Reviewed Files changed in the GitHub PR explorer.
• Manually tested (in case integration/unit/mock tests are absent).

[plafer]

There are only 2 "proto types" left in query reponses: MerkleProof and PacketState. Should we create a domain type for those too? If so, I'm not sure what they should look like.

[romac]

There are only 2 "proto types" left in query reponses: MerkleProof and PacketState. Should we create a domain type for those too? If so, I'm not sure what they should look like.

We already have one for MerkleProof which we should use: https://github.com/informalsystems/ibc-rs/blob/39606d28908134a6be940f65830e464a6e44d567/modules/src/core/ics23_commitment/merkle.rs#L39

Let's therefore introduce one for PacketState as well.

[plafer]

I just noticed that the ChainEndpoint::proven_*() methods each have an analogous query_*() method which performs the same query except that they don't return a proof.

I suggest we remove the proven_*() methods from ChainEndpoint and ChainHandle, and change the provable query_*() functions as follows:

// previous
fn query_channel(&self, request: QueryChannelRequest) -> Result<ChannelEnd, Error>;
fn proven_channel(&self, request: QueryChannelRequest) -> Result<(ChannelEnd, MerkleProof), Error>;

// new
fn query_channel(&self, request: QueryChannelRequest, include_proof: PerformQuery) -> Result<(ChannelEnd, Option<MerkleProof>), Error>;

enum PerformQuery {
    WithProof,
    WithoutProof,
};

I introduced PerformQuery to avoid boolean blindness.

Example calls:

chain.query_channel(request, PerformQuery::WithProof);
chain.query_channel(request, PerformQuery::WithoutProof);

An alternative name for PerformQuery I thought of:

enum IncludeProof {
    Yes,
    No,
};
chain.query_channel(request, IncludeProof::Yes);
chain.query_channel(request, IncludeProof::No);

I personally prefer merging the two functions into one, as we expose fewer methods while preserving the same functionality.

[ancazamfir]

I suggest we remove the proven_*() methods from ChainEndpoint and ChainHandle, and change the provable query_*() functions as follows:

I think it makes sense. A note though that the simple query_ APIs are generally (*) gRPC, the proven_ perform abci_query (RPC to tendermint who does app abci_query). When we first implemented the proven_ APIs there was no support for proofs in gRPC. But now there is. Maybe we should then:

• (*) change all query_.. for application state to use gRPC (we also have query_.. methods that retrieve blockchain info, i.e. they will stay RPCs to tendermint)
• change all proven_... to use the gRPC

wrt to query height, if you look at the gRPC request proto defs they do not contain the height at which the query should be done. This is "hidden" in the gRPC metadata. For and example on how this is done with gRPC you can take a look at https://github.com/informalsystems/ibc-rs/blob/c5c89dc82b1466101066e69d1228cda439da1d4b/relayer/src/chain/cosmos.rs#L931-L937

Also I don't know if there is a way to make a gRPC request and specify that there shouldn't be any proof included. If that's the case, we should look at performance differences between abci queries without proof and gRCP (with proof always??), especially if the APIs used by the packet relaying are affected.

[seanchen1991]

Question: What's a RawMerkleProof and when is it appropriate to use it vs. a "regular" MerkleProof?

[plafer]

The RawMerkleProof is the type that is defined in ibc-go's protobuf files. MerkleProof here is our own domain type for it, and hence we should use it everywhere in our codebase except when it's time to interact with a gRPC endpoint (where we convert our MerkleProof back into a RawMerkleProof to be sent on the wire)