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

Allow disabling of ICS31 Cross Chain Queries #4041

Merged
merged 8 commits into from
Jun 21, 2024
Merged

Allow disabling of ICS31 Cross Chain Queries #4041

merged 8 commits into from
Jun 21, 2024

Conversation

ljoss17
Copy link
Contributor

@ljoss17 ljoss17 commented Jun 17, 2024 •
edited
Loading

Closes: #4040

Description

This PR adds a configuration which disables the relaying of ICS31 Cross Chain Queries

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/).
  • Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

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

@romac romac mentioned this pull request Jun 17, 2024
Closed
5 tasks
@romac
Copy link
Member

romac commented Jun 18, 2024

Reminder to add the new option to config.toml

ancazamfir
ancazamfir approved these changes Jun 19, 2024
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.

Looks good, thanks @ljoss17 ! Maybe we should open an issue or discussion to review, evaluate, consolidate the filtering features.

@ljoss17
Copy link
Contributor Author

ljoss17 commented Jun 20, 2024

Looks good, thanks @ljoss17 ! Maybe we should open an issue or discussion to review, evaluate, consolidate the filtering features.

Sure I'll open an issue for the filtering. I also found out these methods which could be either removed or used, https://github.com/informalsystems/hermes/blob/v1.9.0/crates/relayer/src/supervisor.rs#L317

@ljoss17 ljoss17 marked this pull request as ready for review June 20, 2024 07:27
@ljoss17 ljoss17 mentioned this pull request Jun 20, 2024
Open
5 tasks
@ljoss17
Copy link
Contributor Author

ljoss17 commented Jun 20, 2024

Looks good, thanks @ljoss17 ! Maybe we should open an issue or discussion to review, evaluate, consolidate the filtering features.

I opened this issue #4052

romac
romac requested changes Jun 21, 2024
View reviewed changes
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.

Awesome, just left one nitpick

crates/relayer/src/worker.rs Outdated Show resolved Hide resolved
@ljoss17 ljoss17 added this pull request to the merge queue Jun 21, 2024
Merged via the queue into master with commit 7e3f11e Jun 21, 2024
31 checks passed
@ljoss17 ljoss17 deleted the luca_joss/allow-icq-disabling branch June 21, 2024 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ancazamfir ancazamfir ancazamfir approved these changes

@romac romac romac approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Option to disable ICQ
3 participants
@ljoss17 @romac @ancazamfir