Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Brief relayer refactor to improve testing and add semantic dependencies #448

Merged
merged 11 commits into from
Dec 11, 2020

Conversation

adizere
Copy link
Member

@adizere adizere commented Dec 10, 2020

Closes: #447

High-level view:

  • replaces Clone with DynClone bound for ChainHandle
  • replaces impl ChainHandle argument type with Box<dyn ChainHandle> wherever necessary
  • Modified the relayer Connection type to store its semantic dependencies, namely the two clients
  • Modified the relayer ForeignClient type to store its semantic deps, namely the two chains handlers, and also added some tests for the update method

For contributor use:

  • Updated the Unreleased section of CHANGELOG.md with the issue.
  • If applicable: Unit tests written, added test to CI.
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Updated relevant documentation (docs/) and code comments.
  • Re-reviewed Files changed in the Github PR explorer.

@codecov-io
Copy link

codecov-io commented Dec 11, 2020

Codecov Report

Merging #448 (1e9335b) into master (b1b37f5) will increase coverage by 23.5%.
The diff coverage is 67.0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #448      +/-   ##
=========================================
+ Coverage    13.6%   37.1%   +23.5%     
=========================================
  Files          69     158      +89     
  Lines        3752   12063    +8311     
  Branches     1374    4765    +3391     
=========================================
+ Hits          513    4486    +3973     
- Misses       2618    6903    +4285     
- Partials      621     674      +53     
Impacted Files Coverage Δ
modules/src/events.rs 0.0% <0.0%> (ø)
modules/src/ics02_client/events.rs 0.0% <ø> (ø)
modules/src/ics02_client/msgs.rs 0.0% <0.0%> (ø)
modules/src/ics02_client/raw.rs 0.0% <0.0%> (ø)
modules/src/ics03_connection/error.rs 13.6% <0.0%> (-19.7%) ⬇️
modules/src/ics04_channel/error.rs 75.0% <ø> (+50.0%) ⬆️
modules/src/ics04_channel/events.rs 0.0% <0.0%> (ø)
modules/src/ics04_channel/msgs/chan_close_init.rs 58.6% <ø> (ø)
modules/src/ics04_channel/msgs/chan_open_ack.rs 65.4% <ø> (ø)
...odules/src/ics04_channel/msgs/chan_open_confirm.rs 62.5% <ø> (ø)
... and 289 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a42c0c...f44a12b. Read the comment docs.

@adizere
Copy link
Member Author

adizere commented Dec 11, 2020

Tested v-0 against stargate-4. The relayer seems quite fast, note that client creation took around 500ms, connection handshake took 1s and channel handshake also 1sec:
I think the connections were already setup, hence it looked like the relayer acted very fast.

adi@styx:ibc-rs $ cargo run --bin relayer -- -c dev-env-config.toml v-0
    Finished dev [unoptimized + debuginfo] target(s) in 0.20s
     Running `target/debug/relayer -c dev-env-config.toml v-0`
Dec 11 14:53:01.168  INFO relayer::event::monitor: running listener chain.id=ibc0
Dec 11 14:53:01.195  INFO relayer::event::monitor: running listener chain.id=ibc1
Dec 11 14:53:01.404  INFO relayer::foreign_client: 🍭  Client on ChainId { id: "ibc0", version: 0 } is created ClientId("ibconeclient")

Dec 11 14:53:01.879  INFO relayer::foreign_client: 🍭  Client on ChainId { id: "ibc1", version: 0 } is created ClientId("ibczeroclient")

Dec 11 14:53:02.949  INFO relayer::connection: 🥂  🥂  🥂  Connection handshake finished for [ConnectionConfig {
    a_config: ConnectionSideConfig {
        chain_id: ChainId {
            id: "ibc0",
            version: 0,
        },
        connection_id: ConnectionId(
            "ibconeconnection",
        ),
        client_id: ClientId(
            "ibconeclient",
        ),
    },
    b_config: ConnectionSideConfig {
        chain_id: ChainId {
            id: "ibc1",
            version: 0,
        },
        connection_id: ConnectionId(
            "ibczeroconnection",
        ),
        client_id: ClientId(
            "ibczeroclient",
        ),
    },
}]
Dec 11 14:53:04.042  INFO relayer::channel: 🥳  🥳  🥳  Channel handshake finished for ChannelConfig {
    ordering: Unordered,
    a_config: ChannelConfigSide {
        chain_id: ChainId {
            id: "ibc0",
            version: 0,
        },
        connection_id: ConnectionId(
            "ibconeconnection",
        ),
        client_id: ClientId(
            "ibconeclient",
        ),
        port_id: PortId(
            "transfer",
        ),
        channel_id: ChannelId(
            "ibconexfer",
        ),
    },
    b_config: ChannelConfigSide {
        chain_id: ChainId {
            id: "ibc1",
            version: 0,
        },
        connection_id: ConnectionId(
            "ibczeroconnection",
        ),
        client_id: ClientId(
            "ibczeroclient",
        ),
        port_id: PortId(
            "transfer",
        ),
        channel_id: ChannelId(
            "ibczeroxfer",
        ),
    },
}
Confirm that connection handshake are very slow:
Dec 11 15:04:08.040  INFO relayer::connection: 🥂  ConnInit ConnectionSideConfig { chain_id: ChainId { id: "ibc0", version: 0 }, connection_id: ConnectionId("ibconeconnection1"), client_id: ClientId("ibconeclient") }
Dec 11 15:04:08.040  INFO relayer::connection: elapsed time 10

Dec 11 15:04:20.689  INFO relayer::connection: 🥂  ConnTry ConnectionSideConfig { chain_id: ChainId { id: "ibc1", version: 0 }, connection_id: ConnectionId("ibczeroconnection1"), client_id: ClientId("ibczeroclient") }
Dec 11 15:04:20.690  INFO relayer::connection: elapsed time 12

Dec 11 15:04:33.672  INFO relayer::connection: 🥂  ConnTry ConnectionSideConfig { chain_id: ChainId { id: "ibc1", version: 0 }, connection_id: ConnectionId("ibczeroconnection1"), client_id: ClientId("ibczeroclient") }
Dec 11 15:04:33.672  INFO relayer::connection: elapsed time 12

Dec 11 15:04:46.055  INFO relayer::connection: 🥂  ConnAck ConnectionSideConfig { chain_id: ChainId { id: "ibc0", version: 0 }, connection_id: ConnectionId("ibconeconnection1"), client_id: ClientId("ibconeclient") }
Dec 11 15:04:46.056  INFO relayer::connection: elapsed time 12


Dec 11 15:04:53.220  INFO relayer::connection: 🥂  ConnConfirm ConnectionSideConfig { chain_id: ChainId { id: "ibc1", version: 0 }, connection_id: ConnectionId("ibczeroconnection1"), client_id: ClientId("ibczeroclient") }
Dec 11 15:04:53.220  INFO relayer::connection: elapsed time 7
Channel handshake

Using any other port than transfer will not work:

Dec 11 15:07:24.416  INFO relayer::channel: 🥳  ChanInit ChannelConfigSide { chain_id: ChainId { id: "ibc0", version: 0 }, connection_id: ConnectionId("ibconeconnection1"), client_id: ClientId("ibconeclient"), port_id: PortId("transfer1"), channel_id: ChannelId("ibconexfer1") }
Dec 11 15:07:24.417  INFO relayer::channel: elapsed time 8

Dec 11 15:07:29.810  INFO relayer::channel: 🥳  ChanInit ChannelConfigSide { chain_id: ChainId { id: "ibc0", version: 0 }, connection_id: ConnectionId("ibconeconnection1"), client_id: ClientId("ibconeclient"), port_id: PortId("transfer1"), channel_id: ChannelId("ibconexfer1") }
Dec 11 15:07:29.810  INFO relayer::channel: elapsed time 5

Dec 11 15:07:34.158  INFO relayer::channel: 🥳  ChanInit ChannelConfigSide { chain_id: ChainId { id: "ibc0", version: 0 }, connection_id: ConnectionId("ibconeconnection1"), client_id: ClientId("ibconeclient"), port_id: PortId("transfer1"), channel_id: ChannelId("ibconexfer1") }
Dec 11 15:07:34.159  INFO relayer::channel: elapsed time 4

Dec 11 15:07:38.529  INFO relayer::channel: 🥳  ChanInit ChannelConfigSide { chain_id: ChainId { id: "ibc0", version: 0 }, connection_id: ConnectionId("ibconeconnection1"), client_id: ClientId("ibconeclient"), port_id: PortId("transfer1"), channel_id: ChannelId("ibconexfer1") }
Dec 11 15:07:38.530  INFO relayer::channel: elapsed time 4

Dec 11 15:07:42.886  INFO relayer::channel: 🥳  ChanInit ChannelConfigSide { chain_id: ChainId { id: "ibc0", version: 0 }, connection_id: ConnectionId("ibconeconnection1"), client_id: ClientId("ibconeclient"), port_id: PortId("transfer1"), channel_id: ChannelId("ibconexfer1") }
Dec 11 15:07:42.886  INFO relayer::channel: elapsed time 4

Dec 11 15:07:47.265  INFO relayer::channel: 🥳  ChanInit ChannelConfigSide { chain_id: ChainId { id: "ibc0", version: 0 }, connection_id: ConnectionId("ibconeconnection1"), client_id: ClientId("ibconeclient"), port_id: PortId("transfer1"), channel_id: ChannelId("ibconexfer1") }
Dec 11 15:07:47.265  INFO relayer::channel: elapsed time 4

Dec 11 15:07:51.634  INFO relayer::channel: 🥳  ChanInit ChannelConfigSide { chain_id: ChainId { id: "ibc0", version: 0 }, connection_id: ConnectionId("ibconeconnection1"), client_id: ClientId("ibconeclient"), port_id: PortId("transfer1"), channel_id: ChannelId("ibconexfer1") }
Dec 11 15:07:51.635  INFO relayer::channel: elapsed time 4

Dec 11 15:07:55.983  INFO relayer::channel: 🥳  ChanInit ChannelConfigSide { chain_id: ChainId { id: "ibc0", version: 0 }, connection_id: ConnectionId("ibconeconnection1"), client_id: ClientId("ibconeclient"), port_id: PortId("transfer1"), channel_id: ChannelId("ibconexfer1") }
Dec 11 15:07:55.984  INFO relayer::channel: elapsed time 4

Dec 11 15:08:00.391  INFO relayer::channel: 🥳  ChanInit ChannelConfigSide { chain_id: ChainId { id: "ibc0", version: 0 }, connection_id: ConnectionId("ibconeconnection1"), client_id: ClientId("ibconeclient"), port_id: PortId("transfer1"), channel_id: ChannelId("ibconexfer1") }
Dec 11 15:08:00.391  INFO relayer::channel: elapsed time 4

Dec 11 15:08:04.805  INFO relayer::channel: 🥳  ChanInit ChannelConfigSide { chain_id: ChainId { id: "ibc0", version: 0 }, connection_id: ConnectionId("ibconeconnection1"), client_id: ClientId("ibconeclient"), port_id: PortId("transfer1"), channel_id: ChannelId("ibconexfer1") }
Dec 11 15:08:04.805  INFO relayer::channel: elapsed time 4

error: relayer-cli fatal error: failed

With the correct transfer port, channel handshake takes on the order of tens of seconds:

Dec 11 15:25:11.258  INFO relayer::channel: 🥳  ChanInit ChannelConfigSide { chain_id: ChainId { id: "ibc0", version: 0 }, connection_id: ConnectionId("ibconeconnection1"), client_id: ClientId("ibconeclient"), port_id: PortId("transfer"), channel_id: ChannelId("ibconexfer1") }
Dec 11 15:25:11.258  INFO relayer::channel: elapsed time 11

Dec 11 15:25:17.941  INFO relayer::channel: 🥳  ChanTry ChannelConfigSide { chain_id: ChainId { id: "ibc1", version: 0 }, connection_id: ConnectionId("ibczeroconnection1"), client_id: ClientId("ibczeroclient"), port_id: PortId("transfer"), channel_id: ChannelId("ibczeroxfer1") }
Dec 11 15:25:17.941  INFO relayer::channel: elapsed time 6

Dec 11 15:25:26.598  INFO relayer::channel: 🥳  ChanAck ChannelConfigSide { chain_id: ChainId { id: "ibc0", version: 0 }, connection_id: ConnectionId("ibconeconnection1"), client_id: ClientId("ibconeclient"), port_id: PortId("transfer"), channel_id: ChannelId("ibconexfer1") }
Dec 11 15:25:26.601  INFO relayer::channel: elapsed time 8

Dec 11 15:25:34.458  INFO relayer::channel: 🥳  ChanConfirm ChannelConfigSide { chain_id: ChainId { id: "ibc1", version: 0 }, connection_id: ConnectionId("ibczeroconnection1"), client_id: ClientId("ibczeroclient"), port_id: PortId("transfer"), channel_id: ChannelId("ibczeroxfer1") }
Dec 11 15:25:34.458  INFO relayer::channel: elapsed time 7

Dec 11 15:25:35.597  INFO relayer::channel: 🥳  🥳  🥳  Channel handshake finished for ChannelConfig {
    ordering: Unordered,
    a_config: ChannelConfigSide {
        chain_id: ChainId {
            id: "ibc0",
            version: 0,
        },
        connection_id: ConnectionId(
            "ibconeconnection1",
        ),
        client_id: ClientId(
            "ibconeclient",
        ),
        port_id: PortId(
            "transfer",
        ),
        channel_id: ChannelId(
            "ibconexfer1",
        ),
    },
    b_config: ChannelConfigSide {
        chain_id: ChainId {
            id: "ibc1",
            version: 0,
        },
        connection_id: ConnectionId(
            "ibczeroconnection1",
        ),
        client_id: ClientId(
            "ibczeroclient",
        ),
        port_id: PortId(
            "transfer",
        ),
        channel_id: ChannelId(
            "ibczeroxfer1",
        ),
    },
}

@ancazamfir
Copy link
Collaborator

Tested v-0 against stargate-4. The relayer seems quite fast, note that client creation took around 500ms, connection handshake took ~1s and channel handshake also ~1sec:

That's probably because they are all created already by the Go relayer. Otherwise you should have seen the handshake steps.
You need to change the ids in the config file to something different in order to go through the whole setup process.

Copy link
Collaborator

@ancazamfir ancazamfir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thanks Adi!

@adizere adizere merged commit 39ff928 into master Dec 11, 2020
@adizere adizere deleted the adi/381_tests branch December 11, 2020 16:53
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
…es (informalsystems#448)

* Starting refactor for Connection & Channel relayer objects

* Make chain handle object-safe via dyn_clone.

* Went back to clone() method [h/t Romain for suggesting clone_trait_object!]

* Refactoring Connection and ForeignClient

* Added tests for client update

* Added height check in ForeignClient update tests

* Removed useless test/code

* Added changelog entry

* Consolidating names (cf. review of informalsystems#364).

* Added more semantic deps
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make trait ChainHandle object-safe & clonable
3 participants