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

ADR for channel and connection handshake commands #637

Closed
5 tasks
ancazamfir opened this issue Feb 11, 2021 · 6 comments · Fixed by #676
Closed
5 tasks

ADR for channel and connection handshake commands #637

ancazamfir opened this issue Feb 11, 2021 · 6 comments · Fixed by #676
Assignees
Labels
O: usability Objective: cause to improve the user experience (UX) and ease using the product
Milestone

Comments

@ancazamfir
Copy link
Collaborator

ancazamfir commented Feb 11, 2021

Crate

relayer, relayer-cli

Summary

Proposal for the relayer CLIs (tx raw excluded)

The proposal is summarized in ADR 006.

Problem Definition

Currently the relayer CLIs do not cover user needs and offer a poor UX. See for example #628.

Proposal

I propose the following new/ changed CLIs:

  • Follow the Go relayer commands for channel and connection handshakes (?):

    • hermes channel handshake.. should be renamed to hermes tx channel ..
    • add new hermes tx connection...
  • Add options to commands so they can establish new connections and channels, finalize any unfinished handshakes, reuse clients, connection, etc:

    • hermes tx connection <src-chain> [<dst-chain>] [--src-client <client-id>] [--dst-client <client-id>] [--src-connection <connection-id>]

    • Examples:

      • hermex tx connection ibc-0 ibc-1 - creates new clients and establishes a new connection between the two chains
        • Should it look for already created clients, check if the trust model is the same as in the config.toml and reuse those clients? And create new clients only if some to use are not found. I believe the Go relayer does this.
      • hermex tx connection ibc-0 ibc-1 --src-client client-0 --dst-client client-1 - establishes a new connection using the specified clients. Note that only one client may be specified, the other is created (with the same question as above)
      • hermex tx connection ibc-0 --src-connection connection-0 - finalizes the handshake for connection-0
    • hermes tx channel <src-chain> [<dst-chain>] [--src-connection <connection-id>] [--src-port <port-id>] [--dst-port <port-id>] [--src-channel <channel-id>]

    • Examples:

      • hermex tx channel ibc-0 ibc-1 --src-port transfer --dst-port transfer - establishes a new connection (with new clients) and a new channel between the two ports (we have this today)
        • Similar question as for connections: should this look for any connection between the two chains and reuse it?
      • hermex tx channel ibc-0 --src-connection connection-0 --src-port transfer --dst-port transfer - establishes a new channel using connection-1, between the two ports. connection-0 must exist, if not in open state it will finalize the connection handshake before establishing the new channel
      • hermex tx channel ibc-0 --src-port transfer--src-channel channel-0 - finalizes the handshake for channel-0
  • I propose we change hermes start ... command:

    • hermes start <src-chain> [<dst-chain>] [--src-port <port-id>] [--dst-port <port-id>] [--src-channel <channel-id>]
      • hermes start ibc-0 ibc-1 - don't allow this anymore, the equivalent would be the one below
        • this is currently looking in config for the connection.paths sections and uses the ports specified in the first one
        • I believe is the only command that uses that portion of the config so we could remove it
      • hermes start ibc-0 ibc-1 --src-port transfer --dst-port transfer - creates new clients, connection, channel between the two chains and ports
        • same question as before: should it reuse existing connection if one exists?
      • hermes start ibc-0 --src-port transfer --src-channel channel-0 - finalizes any pending handshakes (for channel-0 and maybe the connection that the channel uses) and starts the event listening loop
  • There is a recurrent question above: when an identifier for a required client or connection is not specified should a command look for an existing client or connection or create new ones? Should we add a --reuse-if-exist type flag to allow both behaviors?


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@ancazamfir ancazamfir added this to the v0.1.1 milestone Feb 11, 2021
@ancazamfir ancazamfir changed the title channel and connection handshakes commands channel and connection handshake commands Feb 11, 2021
@ancazamfir ancazamfir mentioned this issue Feb 11, 2021
5 tasks
@ancazamfir
Copy link
Collaborator Author

hermes start ibc-0 ibc-1 - don't allow this anymore,

More details on this, it will be allowed in the future (v0.3) but it would do something else:

  • hermes start - passive relaying (*), listens to all events from all chains (specified in config.toml) and acts on them. Doesn't initiate anything. Other commands are used to create clients, send conn-init, chan-init, packets.
  • hermes start ibc-0 ibc-1 - same but only between two chains

(*) this means progressing connection handshakes, channel (open or close) handshakes, and relaying recv packets, acknowledgment and timeouts. On start the relayer will re-create any missed events by scanning the chain states.

@adizere adizere added the O: usability Objective: cause to improve the user experience (UX) and ease using the product label Feb 12, 2021
@adizere
Copy link
Member

adizere commented Feb 12, 2021

  • hermes tx connection <src-chain> [<dst-chain>] [--src-client <client-id>] [--dst-client <client-id>] [--src-connection <connection-id>]
  • Examples:
    • hermex tx connection ibc-0 ibc-1 - creates new clients and establishes a new connection between the two chains
      • Should it look for already created clients, check if the trust model is the same as in the config.toml and reuse those clients? And create new clients only if some to use are not found. I believe the Go relayer does this.
    • hermex tx connection ibc-0 ibc-1 --src-client client-0 --dst-client client-1 - establishes a new connection using the specified clients. Note that only one client may be specified, the other is created (with the same question as above)
    • hermex tx connection ibc-0 --src-connection connection-0 - finalizes the handshake for connection-0

This sounds very nice & comprehensive. But I'm wondering if it's motivated by an actual use-case than would benefit. This command would be more advanced than necessary given the requirements from #628.

In particular the last part (finalizing the conn open handshake) seems to cover a corner-case that are valuable for devs and testing, but not sure if a user would bother understanding the semantics of partially-open connections to run the handshake to completion. WDYT?

@ancazamfir
Copy link
Collaborator Author

The handshake completion was mentioned by @brapse in the last IBC meeting. The reuse of existing client and connections (when not specified) was mentioned a few times and Go relayer does that. I think it makes sense. But we can start with partial functionality (e.g. not implement the --src-c.. flag and therefore assert that the connection/ channel is open) as long as we make the CLIs flexible to take new options and flags in the future.

@adizere
Copy link
Member

adizere commented Feb 12, 2021

The handshake completion was mentioned by @brapse in the last IBC meeting.
The reuse of existing client and connections (when not specified) was mentioned a few times and Go relayer does that. I think it makes sense.

Thank you, I just wanted to make sure they are both useful to someone and a priority, as I haven't seen these requirements articulated before.

@adizere
Copy link
Member

adizere commented Feb 12, 2021

Here's what I would propose w.r.t connection lifecycle:

Command: hermes tx connection <src-chain> <dst-chain>

Semantics: Find or create the connection

More specifically, there are multiple cases to consider. This could be a rough procedure:

  1. Query all connections on both of the chains

    • TODO: unclear if we should query both chains.

  2. A connection end exists on one of the two chains,

  • subcase (a): If a connection is already Open on both chains, and is usable, then return with the description of the two connection ends

    • If the connection is not "usable" go to step 3.
    • TODO: define what is a "usable" connection? probably in terms of clients' stored headers

  • (b): If at least one of the two connection ends is not in state Open

    • In this case, run the connection handshake to completion;
    • If any error occurs, retry 2x, output warnings, and eventually assume case 3 below.
  1. No connection has been (even partially) established on either of the two chains:
  • Create a new one: go to step 4.
  1. If either of the following condition holds:
  • no clients exists (or one of the chains is missing the client), or
  • clients may exist but have different trust model than Hermes' config file
    then:
    • log warning, then create clients (potentially only on the chain missing the client)
    • upon success: handle the connection: go to case 6 below.
    • upon failure to create any of the two clients: retry 2x, output warning
    • eventually abort the command (log error) if the retries fail.
  1. If clients exist but have stale headers, then:
    • output warning & update clients.
    • Upon failure to update any of the two clients:
      • retry 2x more, log warnings each time;
      • eventually, if both retries fail, follow the actions for case 4: create new clients (with retry 2x, etc.).
    • Upon success: go to 6.

The precondition for reach point 6 below is that there should be two clients with relatively fresh headers.

TODO: define what 'stale headers' mean

  1. Perform the connection handshake protocol
  • each step may be retried 2x more if a failure is encountered;
  • after the given retries have been consumed and still no success, give up (log error) and exit the command
  • upon success: return the description of the two connection ends

General:

  • we should a parameter (and remove strategy) to configure how many times should Hermes retry (default: 2 or 3).
  • I would avoid trying to handle corner-cases via the CLI (which I find cumbersome) such as
    • hermex tx connection ibc-0 ibc-1 --src-client client-0 --dst-client client-1 for reusing specific clients, or
    • hermex tx connection ibc-0 --src-connection connection-0 for finishing a specific connection
  • the above procedure can be made simpler, because steps 4 & 5 can be encapsulated by a client (ForeignClient) abstraction, which should guarantee the same find or create semantics.

@ancazamfir
Copy link
Collaborator Author

ancazamfir commented Feb 16, 2021

Summarizing below a discussion with @adizere here. For next release:

  • create new connection

    • with new clients
      hermes tx connection ibc-0 ibc-1 --delay <delay>
    • with existing clients
      hermes tx connection ibc-0 --src-client-id <client-id> --dst-client-id <client-id> --delay <delay>
  • create new channel

    • with new connection and clients
      hermes tx channel ibc-0 ibc-1 --src-port <port-id> --dst-port <port-id> --order <order> --version <version>
    • with existing specified connection
      hermes tx channel ibc-0 --src-connection <connection-id> -src-port <port-id> --dst-port <port-id> --order <order> --version <version>
  • finalize handshake for partially established connection
    hermes tx channel ibc-0 --src-connection <connection-id>

  • finalize handshake for partially established channel
    hermes tx channel ibc-0 --src-channel <channel-id>

  • relay packets over existing channel:
    hermes start ibc-0 --src-channel <channel-id>

  • relay packets over a new channel that uses an existing connection:
    hermes start ibc-0 --src-connection <connection-id> -src-port <port-id> --dst-port <port-id> --order <order> --version <version>

  • relay packets over a new channel that uses a new connection that build on an existing client:
    hermes start ibc-0 --src-client-id <client-id> --dst-client-id <client-id> --delay <delay>

We will write a small ADR with a few more details on this and review.

@adizere adizere modified the milestones: v0.1.1, 03.2021 Feb 18, 2021
@adizere adizere changed the title channel and connection handshake commands ADR for channel and connection handshake commands Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O: usability Objective: cause to improve the user experience (UX) and ease using the product
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants