Phase 5

[hu55a1n1]

Implements Phase 5 of informalsystems/hermes#1 - Add support IBC Channel machinery
(Resolves informalsystems/hermes#37)

• complete/import the ics04 machinery from ibc-rs
• state will now need to be able to store channel ends and packet data
• should now be able to manually complete channel handshakes between two chains and send packets

Testing

1. Follow the steps outlined in the README.md to set up a test environment running a basecoin chain (i.e. basecoin-0) and two ibc chains (ibc-0 & ibc-1).
2. Run the following command to create a connection between ibc-0 and basecoin-0 ->

$ RUST_BACKTRACE=1 hermes create channel ibc-0 basecoin-0 --port-a transfer --port-b transfer
# ...
Success: Channel {
    ordering: Unordered,
    a_side: ChannelSide {
        chain: ProdChainHandle {
            chain_id: ChainId {
                id: "ibc-0",
                version: 0,
            },
            runtime_sender: Sender { .. },
        },
        client_id: ClientId(
            "07-tendermint-1",
        ),
        connection_id: ConnectionId(
            "connection-1",
        ),
        port_id: PortId(
            "transfer",
        ),
        channel_id: Some(
            ChannelId(
                "channel-1",
            ),
        ),
    },
    b_side: ChannelSide {
        chain: ProdChainHandle {
            chain_id: ChainId {
                id: "basecoin-0",
                version: 0,
            },
            runtime_sender: Sender { .. },
        },
        client_id: ClientId(
            "07-tendermint-1",
        ),
        connection_id: ConnectionId(
            "connection-1",
        ),
        port_id: PortId(
            "transfer",
        ),
        channel_id: Some(
            ChannelId(
                "channel-1",
            ),
        ),
    },
    connection_delay: 0ns,
    version: Some(
        "ics20-1",
    ),
}

3. To create a channel in the opposite direction, run -> RUST_BACKTRACE=1 hermes create channel basecoin-0 ibc-0 --port-a transfer --port-b transfer

[hu55a1n1]

We're now depending on the basecoin/phase-5-1 branch of ibc-rs, which is basically the master branch with informalsystems/hermes#1711 & informalsystems/hermes#1746 merged. So we can depend on the next release once those two PRs are merged. 👍

[thanethomson]

Overall looks good to me! Just a few questions inline.

I'm glad we have the integration test now - it was quite expensive previously to have to verify the implementation locally on my side 😅 I'm equally glad that the test passes, of course 😁

[thanethomson]

Why do you need to access the module's store at this level? Was it not sufficient to just have the commit() method on the module itself, and then let the module handle interaction with its store? (Asking for context)

[hu55a1n1]

Thanks for reviewing, @thanethomson! 🙏
I replaced the commit() method with the store() method because some modules need to share their module stores with gRPC service instances. I am working on a better design for this on the typed-stores branch - I'll document this in a separate issue.

[thanethomson]

For interest's sake, what does this entail? I tried poking around in https://github.com/cosmos/ibc-go but couldn't find the corresponding code (haven't spent a huge amount of time reading through it though, so I probably just missed it).

[hu55a1n1]

I was thinking we should do something like this -> https://github.com/cosmos/cosmos-sdk/blob/master/x/capability/keeper/keeper.go#L280
Also, we might need to modify the API on the ibc-rs module side -> https://github.com/informalsystems/ibc-rs/issues/1769

[thanethomson]

Just thinking about packet sequences in general in light of our previous conversation about Tendermint app concurrency, this for me makes me think that reading from/writing to a single channel/port will have to be synchronous, right? I mean, we could still write to multiple channel/port combinations in parallel, but a single channel/port will have to only be accessible from a single thread/coroutine?

Not something actionable right now, of course, just trying to think of the longer-term concurrency implications.

[hu55a1n1]

This is an important observation and we should definitely keep this in mind. In the current design, modules have exclusive write access to their module stores. These stores are shared with the gRPC query services which only require read access. None of the message gRPC services have been implemented yet. So, although module implementations are required to be MT-safe, the module stores are never written in parallel because (AFAIK) tendermint ABCI deliverTx is never called in parallel.