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

Change a-chain/b-chain CLI flag names in favor of more explicit flag names in tx CLI commands #2410

Closed
16 of 21 tasks
seanchen1991 opened this issue Jul 15, 2022 · 5 comments · Fixed by #2446
Closed
16 of 21 tasks
Assignees
Milestone

Comments

@seanchen1991
Copy link
Contributor

seanchen1991 commented Jul 15, 2022

Summary

To quote @ancazamfir:

I propose we use chain/ counterparty instead of b-chain/ a-chain. Except the chain upgrade where we can use host-chain/ reference-chain (with the note that the reference-chain is the chain to be upgraded)

Problem Definition

I find it confusing, especially for handshake Tx-es, where we now use a-chain and b-chain but then in the help strings to make things clear we still use source and destination. For packets seems a bit better but again we use source and destination in the help to explain things.
The general idea for tx (raw) commands is that, regardless of the type of action (relay packet, client or handshake message), you instruct the relayer to send one or more messages to a (destination) chain by retrieving state values and proofs from another (source) chain. So I find using src/ dst more clear here. Or chain/ counterparty.
a-chain and b-chain work fine in the create connection/ channel where you just want to say "create a connection/channel" between chains a and b. And it results in sending Tx-es to both a and b.

I also find it confusing the sender/ receiver in tx upgrade-chain. We use host/ reference in the high level client upgrade. We should use the same here.

Proposal

Change the flag names of the tx CLI commands to use more descriptive flag names. See "Acceptance Criteria" below for a list of commands to update.

Acceptance Criteria

  • Change sender / receiver in tx upgrade-chain to use host / reference so that it matches upgrade client
  • Change tx chan-close-confirm
  • Change tx chan-close-init
  • Change tx chan-open-ack
  • Change tx chan-open-confirm
  • Change tx chan-open-init
  • Change tx chan-open-try
  • Change tx conn-ack
  • Change tx conn-confirm
  • Change tx conn-init
  • Change tx conn-try
  • Change tx ft-transfer
  • Change tx packet-ack
  • Change tx packet-recv
  • Fix tx raw packet-recv to remove the raw in ADR 010
  • Update ADR 010 to correctly reflect the changed flag names

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@seanchen1991
Copy link
Contributor Author

@ancazamfir proposed deprecating a-chain/b-chain in favor of chain/counterparty, though personally I'm a bit more in favor of src/dst. chain is a little bit ambiguous and doesn't necessarily communicate the fact that it is the source. I could be convinced otherwise though 🙂

@ancazamfir
Copy link
Collaborator

ancazamfir commented Jul 15, 2022

@ancazamfir proposed deprecating a-chain/b-chain in favor of chain/counterparty, though personally I'm a bit more in favor of src/dst. chain is a little bit ambiguous and doesn't necessarily communicate the fact that it is the source. I could be convinced otherwise though 🙂

tbh I also like src/ dst but got a feeling people don't really like that. it's also that chain/ counterparty are already used i believe in the high level commands.

@ancazamfir
Copy link
Collaborator

ancazamfir commented Jul 15, 2022

The general idea for tx (raw) commands is that, regardless of the type of action (relay packet, client or handshake message), you instruct the relayer to send one or more messages to a (destination) chain by retrieving state values and proofs from another (source) chain.

I think this (my own statement) is inaccurate. A better explanation is: "you instruct the relayer to attempt to advance the handshake state of an object on a (destination) chain by retrieving state values and proofs from another (source) chain."
(we can also consider the packet exchange a handshake)

My first statement is accurate in most of the cases except:

  • conn-try and conn-ack: here we need in fact to first send a Tx (client update) to the "source". I still have doubts this should be done in the tx case as you can do a separate client update CLI. This used to be the case early in development and it was useful. After a while, once all was working, it turned out to be more convenient to include it as part of conn-try and conn-ack so we changed it.
  • recv-packet if the packet is timed-out (or ordered channel is closed) we send the messages (update client and timeout) to the "source"

@adizere
Copy link
Member

adizere commented Jul 20, 2022

I don't see a big different in usability between src/ dst versus chain/ counterparty. Both are good, and whichever we choose would be ideal to document under the decision part of ADR 010, eg following Anca's suggestion we could have something like

  • for tx commands, we require a src/dst option, as intuitively these commands instruct the relayer to attempt to advance the state of an object (eg packet, channel handshake, connection handshake) on a (destination) chain by retrieving state values and proofs from another (source) chain."

@romac
Copy link
Member

romac commented Jul 21, 2022

Agreed, I too would suggest we go with src/dst here, as those sound better to me and are also more explicit and symmetric that chain/counterparty.

@adizere adizere added this to the v1.0.0 milestone Jul 27, 2022
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 a pull request may close this issue.

4 participants