imp(ibc-testkit): Tendermint proof verifications via integration test

[rnbguy]

Closes: #398

Description

Adds integration tests for combinations of MockHost and TendermintHost.

• Client creation
• Connection creation
• Channel creation
• Packet relay

Since it includes TendermintHost, these tests also exercise commitment proof validations in Tendermint light clients.

The PR also includes a unit test for minimal proof validations.

Additionally, to maintain the ics23 proof spec, a main_store is introduced in MockGenericContext - that stores the IBC store root at IBC commitment prefix path. The blocks are now generated with the root of the main_store.

To keep the proof retrieval minimal, I had to modify the ProvableContext::get_proof of MockIbcStore. Before it returned the encoded bytes of a singleCommitmentProof. Now it returns the encoded bytes of MerkleProofs, which is essentially a list of CommitmentProofs corresponding to a MerklePath.

Two shortcomings of basecoin_store:

• No support for cosmos-sdk specs feat(store): support cosmos-sdk proof specs informalsystems/basecoin-rs#171
• No support for non-membership proofs feat(store): support non membership proof informalsystems/basecoin-rs#158

---

PR author checklist:

• Added changelog entry, using unclog.
• Added tests.
• Linked to GitHub issue.
• Updated code comments and documentation (e.g., docs/).
• Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

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

[Farhad-Shabani]

Great work on this PR @rnbguy. Adding the commitment proof tests! 🎉
I left a few thoughts in the comments, and as we discussed before we move on:

• Please review all the newly defined methods and add docstrings.
• Since we're moving away from the RelayerContext, let's rename and organize TypedIntegration under the relayer directory.

[Farhad-Shabani]

I am thinking our new mock machinery will be going to be an example of IBC module implementation, and even showcasing part of the ibc-testkit in our documentations later. Therefore, I'm unsure about adding this field because commitment proofs usually come from the host storage, not the host's module system, which is the case here too. So, we logically shouldn't need to store it again under the IBC module.

[rnbguy]

Very good point. Actually, I am not sure what proof the IBC gRPC query should return. Is it just the single proof within the IBC store? Is the relayer responsible for getting the proof from the host for the commitment_prefix proof?

I don't see the code for this in ibc-go. So let's clarify this with ibc-go team.

[Farhad-Shabani]

Yeah, I see. Things differ from ibc-go.
This is a symptom of our QueryContext design. By making the ProverContext the primary trait, we are implicitly requiring the IBC module to be aware of proofs, which logically should not be the case.
Let's proceed for now as I don't see an immediate alternative. However, this issue should be addressed, as it may lead people in the wrong direction.

[rnbguy]

This is a symptom of our QueryContext design. By making the ProverContext the primary trait, we are implicitly requiring the IBC module to be aware of proofs, which logically should not be the case.

Very good point. I admit that when I implemented this last year, I didn't have this context - so I didn't worry about it.

Carlo's explanation regarding this:

It should include two proofs: one for the IAVL tree of the IBC store (i.e. the proof of inclusion within the IBC store) and another one for Simple Tree (i.e. the proof of inclusion of the IBC store in the multistore).

It seems that the IBC module should be aware of IBC commitment prefix proofs. We should do this like we have host_* methods.

pub trait ProvableContext {
    fn get_ibc_commitment_prefix_proof_on_host(&self, height: Height) -> Option<MerkleProof>
    fn get_local_store_proof(&self, height: Height, path: &Path) -> Option<MerkleProof>;
    fn get_proof(&self, height: Height, path: &Path) -> Option<MerkleProof> {
        let commitment_prefix_proof = self.get_ibc_commitment_prefix_proof_on_host(height);
        let local_proof = get_local_store_proof(height, path);
        return make_merkle_proof_chain([commitment_prefix_proof, local_proof]);
    }
}

[Farhad-Shabani]

As discussed, the architecture of Cosmos SDK chains may not directly apply to other non-Cosmos chains or rollups. Let's explore this further as a separate issue. Refactoring these parts would likely require a bit more studying and experimentation.

[rnbguy]

Introduced this to generalize the parameters needed to convert a block to a light client header.

[Farhad-Shabani]

To be honest, I found this new method a bit confusing and difficult to grasp its purpose. Since we have tight plans, I made some simplifications (410b8ad) so that can move forward and wrap up ibc-testkit refactoring. I hope that works for you!