Remove dependency on ChainHandle in cosmos-client-components crate

[soareschen]

For the initial development of Hermes SDK, we reuse the existing ChainHandle implementation in Hermes v1 in order to speed up the prototyping. With the new code base maturing, it is now better to reimplement the corresponding methods inside Hermes SDK, so that we can eventually remove the dependency on the Hermes v1 code base.

For the re-implementation, we would identify all uses of with_blocking_chain_handle in the cosmos-client-components crate. We would then locate the corresponding CosmosSdkChain ChainHandle implementation at ibc_relayer::chain::cosmos, and copy over the implementation.

The majority of implementation should be possible by including dependencies such as CanQueryAbci, HasGrpcAddress, and HasRpcClient. Other than that, the main difference for the new implementation is that we use async functions, and therefore there is no need to use block_on to call other async functions. For reference, one can look at the re-implementation that we have already done for components traits such as ChainStatusQuerier and ClientStateBytesQuerier.

Following is a list of component implementations that use with_blocking_chain_handle in the implementation:

• ChannelHandshakePayloadBuilder
• CreateClientPayloadBuilder
• UpdateClientPayloadBuilder
• ConnectionHandshakeMessageBuilder
• ConnectionHandshakePayloadBuilder
• AckPacketPayloadBuilder
• ReceivePacketPayloadBuilder
• TimeoutUnorderedPacketPayloadBuilder
• CounterpartyChainIdQuerier (@thaodt)
• ConnectionEndQuerier
• ConsensusStateHeightQuerier
• ConsensusStateHeightsQuerier
• ConsensusStateQuerier
• ReceivedPacketQuerier (@thaodt)
• WriteAckQuerier (@thaodt)

The re-implementation should be done in small steps, with each PR only focus on one or few of the re-implementations.

[thaodt]

awesome! Thank you for creating this issue @soareschen, lemme dive into it.

[thaodt]

hi @soareschen I'm doing the first PR, if I understand correctly, the CGP's mentioning 3 kinds of contexts: Chain context, Relayer context and Tx context, am I right?

Besides that, I want to ask how to verify my changes, I run cargo test and here is the error output:

so the test is not functional testing, am I right? It means I will need to spawn a local interchain to check it.

[soareschen]

CGP is a programming technique for defining modular contexts. There are several abstract contexts in Hermes SDK, such as Relay, Chain, Runtime, Bootstrap, Logging, Encoding, etc. For each abstract context, there are concrete implementations of the context, such as CosmosRelay, CosmosChain, HermesRuntime, etc.

For the integration tests, we make use of chain executable such as gaiad and simd to test that the relayer is working correctly with the real chains. You can install the chain executables in any way, and expose them in the $PATH environment variable. The standard way for us to load the chain executable is using Nix Flakes.

You can refer to the commands used on the CI to see how the integration tests can be run locally. For example, the Cosmos integration tests can be run using:

nix shell .#cargo-nextest .#protobuf .#gaia .#celestia-app .#ibc-go-v8-simapp -c \
    cargo nextest run -p hermes-cosmos-integration-tests \
    --test-threads=2

Note that you can also run nix shell .#package-name to include the chain executable into your shell. For example, nix shell .#gaia would include gaiad in the shell environment. After that, you can run commands such as cargo test -p hermes-cosmos-integration-tests without having to specify the Nix packages every time.

[thaodt]

@soareschen Thank you for your detailed explanation, I will follow your instructions.

[thaodt]

hi @soareschen Im trying to use the command in github workflow file to run tests and I faced this error:

I thought my change was made it failed and I tried to switch into the main branch, delete the flake.lock file, but still failed. Can you please assist further?

[thaodt]

After upgrading to the latest nix version, the test command is getting build but then I got the error dial tcp: lookup proxy.golang.org: i/o timeout. The solution is export the env var GOPROXY=direct fixed the issue.

And very soon after that, it failed just because the connection refused while that port was available on my machine.

Changing from localhost to 127.0.0.1 makes it worked great.
And the problem has been resolved. Anw the experience was interesting for me personally.
Just curious @soareschen have you faced these problems before?

I will do some more updates and create PRs soon.

[soareschen]

@thaodt let's move the discussion on the Nix issues here: #234

[thaodt]

hi @soareschen I saw a lot of implementations for a while, sorry for delay because of some personal reasons and now I think I'm good to move on. I just realized that there are only a few dependencies left on ChainHandle, do you want me to continue with them? Or would you like to give me another GFI?

[thaodt]

For ChannelHandshakePayloadBuilder I saw it to be replaced by ChannelOpenTryPayloadBuilder, I guess you may have a plan for it, so I won't touch.
for CreateClientPayloadBuilder and UpdateClientPayloadBuilder I saw you created 2 new issues, can you share the details what expectation for those? I may help.
and below are a few remaining components that I see:

• AckPacketPayloadBuilder
• ReceivePacketPayloadBuilder
• TimeoutUnorderedPacketPayloadBuilder
• CounterpartyChainIdQuerier
• ReceivedPacketQuerier
• WriteAckQuerier

I may try with the component has suffix Querier

[soareschen]

Hey @thaodt, you are right that we are doing quick refactoring on the payload builders to generalize the implementation to work on non-Cosmos chains. We won't touch the querier methods just yet, so you can give them a try if you like.

[thaodt]

Hey @thaodt, you are right that we are doing quick refactoring on the payload builders to generalize the implementation to work on non-Cosmos chains. We won't touch the querier methods just yet, so you can give them a try if you like.

Thanks @soareschen. I'm working on these: CounterpartyChainIdQuerier, ReceivedPacketQuerier and WriteAckQuerier

[thaodt]

While I'm trying to remove ChainHandle dependency on WriteAckQuerier, I noticed this snippet:

hermes-sdk/crates/cosmos/cosmos-chain-components/src/impls/queries/write_ack_event.rs

Lines 45 to 63 in 3bb20a0

	let ibc_events = query_write_ack_events(
	&chain_handle,
	&path_ident,
	&[packet.sequence],
	query_height,
	)
	.map_err(Chain::raise_error)?;
	let write_ack = ibc_events.into_iter().find_map(|event_with_height| {
	let event = event_with_height.event;
	if let IbcEvent::WriteAcknowledgement(write_ack) = event {
	Some(write_ack)
	} else {
	None
	}
	});
	Ok(write_ack)

the query_write_ack_events fn belongs to the link part in relayer v1. It receives a chain_handle as a parameter, I suppose I need to move this logic also or there is a way to achieve the same thing. Could you please shed some light on this @soareschen ? Thanks.

[soareschen]

Yes, in this case you need to extract the relevant logic from Hermes v1 functions, such as query_packet_events and query_packets_from_block. The new implementation do not need to be as general as the v1 functions. For example, it only need to handle the case of Qualified::Equal, so the other branches do not need to be copied.

Ultimately, the actual parameters you need are values such as rpc_client: &HttpClient and rpc_address: &Url, which you can get from HasRpcClient.

[thaodt]

Yes, in this case you need to extract the relevant logic from Hermes v1 functions, such as query_packet_events and query_packets_from_block. The new implementation do not need to be as general as the v1 functions. For example, it only need to handle the case of Qualified::Equal, so the other branches do not need to be copied.

Ultimately, the actual parameters you need are values such as rpc_client: &HttpClient and rpc_address: &Url, which you can get from HasRpcClient.

Understood. Im on it.