Phase4

[hu55a1n1]

Implements Phase 4 of #1 - Add support for IBC Connection machinery

• complete/import the ics03 machinery from ibc-rs
• state will now need to be able to store connection ends
• should now be able to manually complete connection handshake between two chains

Testing

1. Follow the steps outlined in the README.md to setup 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 basecoin-0 and ibc-0 ->

$ RUST_BACKTRACE=1 hermes create connection ibc-0 basecoin-0
# ...
Success: Connection {
    delay_period: 0ns,
    a_side: ConnectionSide {
        chain: ProdChainHandle {
            chain_id: ChainId {
                id: "ibc-0",
                version: 0,
            },
            runtime_sender: Sender { .. },
        },
        client_id: ClientId(
            "07-tendermint-0",
        ),
        connection_id: Some(
            ConnectionId(
                "connection-0",
            ),
        ),
    },
    b_side: ConnectionSide {
        chain: ProdChainHandle {
            chain_id: ChainId {
                id: "basecoin-0",
                version: 0,
            },
            runtime_sender: Sender { .. },
        },
        client_id: ClientId(
            "07-tendermint-0",
        ),
        connection_id: Some(
            ConnectionId(
                "connection-0",
            ),
        ),
    },
}

[hu55a1n1]

Note that opening a connection in the opposite direction may fail intermittently - this is a known bug in Tendermock's AVL impl. See the tracking issue here -> CharlyCst/tendermock#2

[hu55a1n1]

Also note that we must depend on the basecoin/phase-4 branch of ibc-rs for multiple reasons which include ->

• Verification methods in the ibc-rs module are incomplete and call todo!().
• basecoin-rs uses a different ProofSpec and ibc-rs currently assumes that all Tendermint chains use the Cosmos SDK specific ProofSpec.

[thanethomson]

Sorry for the lag in getting to this. I've been trying to set this up today and I haven't been successful in creating a connection unfortunately.

See this gist for my output logs.

When you say this:

Also note that we must depend on the basecoin/phase-4 branch of ibc-rs for multiple reasons

Does that mean that I need to build hermes from that branch?

[hu55a1n1]

Does that mean that I need to build hermes from that branch?

That comment is outdated, sorry for not updating it. We need to build hermes from the basecoin/phase-4-1 branch, because we need a version of hermes that has the same proto defs and support for custom ProofSpecs. We also need to tell hermes to use basecoin's ProofSpecs in the config.toml.
I'll update the README.md with this info.

[[chains]]
id = 'basecoin-0'
rpc_addr = 'http://127.0.0.1:26357'
grpc_addr = 'http://127.0.0.1:9093'
websocket_addr = 'ws://localhost:26357/websocket'
rpc_timeout = '10s'
account_prefix = 'cosmos'
key_name = 'testkey'
store_prefix = 'ibc'
gas_price = { price = 0.001, denom = 'stake' }
clock_drift = '5s'
trusting_period = '14days'
proof_specs = '''
[
  {
    "leaf_spec": {
      "hash": 1,
      "prehash_key": 0,
      "prehash_value": 0,
      "length": 0,
      "prefix": [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
    },
    "inner_spec": {
      "child_order": [
        0,
        1,
        2
      ],
      "child_size": 32,
      "min_prefix_length": 0,
      "max_prefix_length": 64,
      "empty_child": [
        0,
        32
      ],
      "hash": 1
    },
    "max_depth": 0,
    "min_depth": 0
  },
  {
    "leaf_spec": {
      "hash": 1,
      "prehash_key": 0,
      "prehash_value": 0,
      "length": 0,
      "prefix": [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
    },
    "inner_spec": {
      "child_order": [
        0,
        1,
        2
      ],
      "child_size": 32,
      "min_prefix_length": 0,
      "max_prefix_length": 64,
      "empty_child": [
        0,
        32
      ],
      "hash": 1
    },
    "max_depth": 0,
    "min_depth": 0
  }
]
'''

[thanethomson]

Overall this looks great @hu55a1n1! 🎉 Well done! A lot of complex stuff going on here.

I just had a few questions, but I don't think that should stop you from merging this. If my questions should be addressed in the code we can create follow-up issues.

[thanethomson]

For interest's sake, is there any other pattern that would work here that doesn't involve using trait objects?

No need to change this now, I was just wondering.

[hu55a1n1]

Good question! I've been thinking about this myself recently. 🤔 One possible alternative would be to have an enum with one variant per module, but that would imply that we know of all supported modules beforehand and that is not desirable if we think of this code as part of a framework.
Furthermore, ICS25 requires that ->

The module set should be dynamic: chains should be able to add and destroy modules, which can themselves bind to and unbind from ports, at will with a persistent IBC handler.

So I am not aware of a better alternative but happy to discuss other ideas/solutions.