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

fix: Add explicit root TLS configuration to gRPC clients #4161

Merged
merged 3 commits into from
Aug 27, 2024

Conversation

allthatjazzleo
Copy link
Contributor

@allthatjazzleo allthatjazzleo commented Aug 24, 2024

Closes: #4160

Description


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).

@allthatjazzleo allthatjazzleo changed the title fix: add explicit root tls fix: add explicit root tls configuration Aug 24, 2024
@allthatjazzleo
Copy link
Contributor Author

@romac @ljoss17
any chance to review it?

@romac
Copy link
Member

romac commented Aug 26, 2024

Good catch, thanks for opening this PR, much appreciated! We'll take a look ASAP!

@romac romac changed the title fix: add explicit root tls configuration fix: Add explicit root TLS configuration to gRPC clients Aug 26, 2024
@romac
Copy link
Member

romac commented Aug 26, 2024

LGTM, thanks a lot!

Copy link
Contributor

@ljoss17 ljoss17 left a comment

Choose a reason for hiding this comment

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

Thanks for handling this!

@romac
Copy link
Member

romac commented Aug 26, 2024

@allthatjazzleo Can you please add a changelog entry in .changelog/unreleased/bug-fixes?

@allthatjazzleo
Copy link
Contributor Author

@romac changelog entry done and resolve conflicts

@allthatjazzleo
Copy link
Contributor Author

@ljoss17
I rebase from the master and resolve those conflict from #4102
why do we want to use abci_query instead of grpc?

@ljoss17
Copy link
Contributor

ljoss17 commented Aug 26, 2024

@ljoss17 I rebase from the master and resolve those conflict from #4102 why do we want to use abci_query instead of grpc?

We had issues with this query using the gRPC. And there is a plan to eventually switch to RPC only endpoints.

@romac romac added this pull request to the merge queue Aug 27, 2024
Merged via the queue into informalsystems:master with commit f2d4e7f Aug 27, 2024
28 checks passed
@allthatjazzleo allthatjazzleo deleted the fix/tonic-0.12 branch October 2, 2024 12:54
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.

grpc tls endpoint no longer work with tonic 0.12
3 participants