Open soareschen opened 7 months ago
awesome! Thank you for creating this issue @soareschen, lemme dive into it.
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.
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.
@soareschen Thank you for your detailed explanation, I will follow your instructions.
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?
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.
@thaodt let's move the discussion on the Nix issues here: https://github.com/informalsystems/hermes-sdk/discussions/234
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?
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
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.
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
While I'm trying to remove ChainHandle
dependency on WriteAckQuerier
, I noticed this snippet:
https://github.com/informalsystems/hermes-sdk/blob/3bb20a0dc3ca0e8116b60ead284fb79091369783/crates/cosmos/cosmos-chain-components/src/impls/queries/write_ack_event.rs#L45-L63
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.
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
.
Yes, in this case you need to extract the relevant logic from Hermes v1 functions, such as
query_packet_events
andquery_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 ofQualified::Equal
, so the other branches do not need to be copied.Ultimately, the actual parameters you need are values such as
rpc_client: &HttpClient
andrpc_address: &Url
, which you can get fromHasRpcClient
.
Understood. Im on it.
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 thecosmos-client-components
crate. We would then locate the correspondingCosmosSdkChain
ChainHandle
implementation atibc_relayer::chain::cosmos
, and copy over the implementation.The majority of implementation should be possible by including dependencies such as
CanQueryAbci
,HasGrpcAddress
, andHasRpcClient
. Other than that, the main difference for the new implementation is that we use async functions, and therefore there is no need to useblock_on
to call other async functions. For reference, one can look at the re-implementation that we have already done for components traits such asChainStatusQuerier
andClientStateBytesQuerier
.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.