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

Add --show-counterparty flag to hermes query channels #2463

Merged
merged 15 commits into from
Jul 27, 2022

Conversation

AlianBenabdallah
Copy link
Contributor

@AlianBenabdallah AlianBenabdallah commented Jul 25, 2022

Closes: #2429

Description

This PR introduces a new flag for hermes query channels which outputs every channel along with its corresponding port, and the counterparty chain's id in a pretty way.

Example with 3 chains:

hermes query channels --chain ibc-1 --show-counterparty
Success:
ibc-1: transfer/channel-0 --- ibc-0: transfer/channel-0
ibc-1: transfer/channel-1 --- ibc-2: transfer/channel-1
hermes query channels --chain ibc-2 --show-counterparty
Success:
ibc-2: transfer/channel-0 --- ibc-0: transfer/channel-1
ibc-2: transfer/channel-1 --- ibc-1: transfer/channel-1
hermes query channels --chain ibc-0 --show-counterparty
Success:
ibc-0: transfer/channel-0 --- ibc-1: transfer/channel-0
ibc-0: transfer/channel-1 --- ibc-2: transfer/channel-0

Filtering is also working :

hermes query channels --chain ibc-0 --show-counterparty --counterparty-chain ibc-1
Success:
ibc-0: transfer/channel-0 --- ibc-1: transfer/channel-0

PR author checklist:

  • Added changelog entry, using unclog.
  • Added tests: integration (for Hermes) or unit/mock tests (for modules).
  • Linked to GitHub issue.
  • Updated code comments and documentation (e.g., docs/).

Reviewer checklist:

  • Reviewed Files changed in the GitHub PR explorer.
  • Manually tested (in case integration/unit/mock tests are absent).

Copy link
Member

@romac romac left a comment

Choose a reason for hiding this comment

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

Great job, Ali! Left a few suggestions inline.

output.iter().try_for_each(|pretty_print| {
write!(
f,
"\n{}: {}/{} --- {}: {}/{}",
Copy link
Member

Choose a reason for hiding this comment

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

To make things slightly prettier, let's use a unicode arrow instead of ---?

For instance

ibc-1: transfer/channel-0 ⟷ ibc-0: transfer/channel-0
ibc-1: transfer/channel-1 ⟷ ibc-2: transfer/channel-1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unicode arrows are pretty small on my console :
image

Copy link
Member

Choose a reason for hiding this comment

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

Then let's stick with ASCII!

Copy link
Member

Choose a reason for hiding this comment

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

Wonder why your terminal does that though? Perhaps it's the font?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no idea. I could push a new version with the double arrow if you want to try with your terminal as well.

Copy link
Member

Choose a reason for hiding this comment

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

Interesting, they also render funny in iTerm on macOS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we can use another emoji ?

@romac romac changed the title --show-counterparty flag for hermes query channels Add --show-counterparty flag to hermes query channels Jul 26, 2022
AlianBenabdallah and others added 5 commits July 26, 2022 18:10
Co-authored-by: Sean Chen <seanchen11235@gmail.com>
…erparty.md

Co-authored-by: Sean Chen <seanchen11235@gmail.com>
Co-authored-by: Sean Chen <seanchen11235@gmail.com>
@adizere adizere requested review from seanchen1991 and removed request for seanchen1991 July 27, 2022 08:39
@adizere adizere assigned seanchen1991 and unassigned adizere Jul 27, 2022
Copy link
Contributor

@seanchen1991 seanchen1991 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@seanchen1991 seanchen1991 merged commit 5c96db7 into master Jul 27, 2022
@seanchen1991 seanchen1991 deleted the ali/output_pretty branch July 27, 2022 11:31
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
…stems#2463)

* added --show-counterparty flag

* added test

* changelog entry and fmt

* Rename 2431-show-counterparty.md to 2429-show-counterparty.md

* Update relayer-cli/src/commands/query/channels.rs

Co-authored-by: Romain Ruetschi <romain@informal.systems>

* Update 2429-show-counterparty.md

* Update 2429-show-counterparty.md

* fix typo

* fixed tests

* update doc

* Update relayer-cli/src/commands/query/channels.rs

Co-authored-by: Sean Chen <seanchen11235@gmail.com>

* Update .changelog/unreleased/features/ibc-relayer-cli/2429-show-counterparty.md

Co-authored-by: Sean Chen <seanchen11235@gmail.com>

* Update relayer-cli/src/commands/query/channels.rs

Co-authored-by: Sean Chen <seanchen11235@gmail.com>

* modified error handling

* modify error handling

Co-authored-by: Romain Ruetschi <romain@informal.systems>
Co-authored-by: Sean Chen <seanchen11235@gmail.com>
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.

hermes query channels --chain chain_id to output the counterparty chain_id
4 participants