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.

Sign up for GitHub

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

Jump to bottom

Remove light client commands and configuration #793

Merged
merged 12 commits into from
Apr 7, 2021
Merged

Remove light client commands and configuration #793

merged 12 commits into from
Apr 7, 2021

Conversation

romac
Copy link
Member

@romac romac commented Apr 1, 2021 •
edited
Loading

See: #673

Description

Remove the light client configuration from the global configuration and remove the light add and light rm commands.

The peer id for the light client is now fetched at light client initialization time from the RPC address configured for the chain.

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.

@romac romac mentioned this pull request Apr 1, 2021
Merged
5 tasks
@codecov-io

This comment has been minimized.

Sign in to view
@romac romac force-pushed the romac/remove-light-cmds branch from d647103 to 9d26933 Compare April 1, 2021 15:16
@adizere adizere linked an issue Apr 6, 2021 that may be closed by this pull request
light rm: do not throw an error if there are no peers to remove #782
Closed
5 tasks
@romac romac linked an issue Apr 6, 2021 that may be closed by this pull request
Where should thetrust_threshold and trusting_period configuration params live? #356
Closed
5 tasks
@romac romac marked this pull request as ready for review April 6, 2021 14:41
@romac romac requested a review from ancazamfir as a code owner April 6, 2021 14:41
@romac romac requested review from adizere and andynog April 6, 2021 14:41
@romac romac mentioned this pull request Apr 6, 2021
Merged
15 tasks
@romac romac linked an issue Apr 7, 2021 that may be closed by this pull request
User reported that cannot add light client using public rpc endpoint #791
Closed
5 tasks
ancazamfir
ancazamfir reviewed Apr 7, 2021
View reviewed changes
config_example.toml Outdated Show resolved Hide resolved
andynog
andynog approved these changes Apr 7, 2021
View reviewed changes
Copy link
Contributor

@andynog andynog left a comment

Choose a reason for hiding this comment

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

The changes looks good to me. The e2e test changes look good to me too and all the new configs seem to be fine. I'm assuming the docs updates will be handled on another issue so it's all good. Good job. Thanks !

ancazamfir
ancazamfir approved these changes Apr 7, 2021
View reviewed changes
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.

Only a couple of minor changes, otherwise it looks great! Thanks Romain!

@ancazamfir
Copy link
Collaborator

FYI, I did some tests with cosmoshub-4 and even with 30sec timeouts i see errors on client update once in a while. We timeout in runtime's build_header() (coming many times from tendermint-rs light client verify).
There are other timeouts also on init_event_monitor(). It may be the full node I am using that it's busy, not sure. I think these are not related to this PR but I will open an issue to investigate.

adizere and others added 4 commits April 7, 2021 17:30
@adizere @romac
Fix ibc-proto recompilation bug (#802)
ee40c6e 
* Fixed overwritten cosmos files

* changelog

* Fix typo in proto-compiler README

* Fix bug where proto-compiler didn't find SDK repo if already cloned

Co-authored-by: Romain Ruetschi <romain@informal.systems>
@romac
Add option to specify which events to listen for in listen command (#…
0d31c26 
…804)
@romac
Update config_example.toml
d0ef290
@romac
Rename init-clients to init-hermes
a8bb251
@romac romac merged commit 1c72c28 into master Apr 7, 2021
@romac romac deleted the romac/remove-light-cmds branch April 7, 2021 15:45
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
@romac @adizere
Remove light client commands and configuration (informalsystems#793)
044943b 
* Remove light client commands and configuration

* Remove light client init code in scripts

* Update the changelog

* Use rpc_timeout field from chain config

* Update config files and ADRs

* Add --locked flag when compiling hermes in init-clients

* Fix ibc-proto recompilation bug (informalsystems#802)

* Fixed overwritten cosmos files

* changelog

* Fix typo in proto-compiler README

* Fix bug where proto-compiler didn't find SDK repo if already cloned

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

* Add option to specify which events to listen for in `listen` command (informalsystems#804)

* Update config_example.toml

* Rename init-clients to init-hermes

Co-authored-by: Adi Seredinschi <adi@informal.systems>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andynog andynog andynog approved these changes

@ancazamfir ancazamfir ancazamfir approved these changes

@adizere adizere Awaiting requested review from adizere

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@romac @codecov-io @ancazamfir @andynog @adizere