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

Update gRPC version to the latest one and keep it compatible with Tendermint | gogoproto #8392

Closed
4 tasks
robert-zaremba opened this issue Jan 19, 2021 · 7 comments
Labels
C: gRPC Issues and PRs related to the gRPC service and HTTP gateway. dependencies Pull requests that update a dependency file
Milestone

Comments

@robert-zaremba
Copy link
Collaborator

robert-zaremba commented Jan 19, 2021

Summary

In the 0.40.1 release, we encountered a problem with google.golang.org/grpc:

  • Tendermint requires v1.35.0
  • SDK has v1.33.2

So, the dependency resolver uses v1.35.0 for both. Unfortunately this breaks SDK.

Proposal

Update gRPC to the latest version and fix the problem

Reference:


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@robert-zaremba robert-zaremba added dependencies Pull requests that update a dependency file C: gRPC Issues and PRs related to the gRPC service and HTTP gateway. labels Jan 19, 2021
@robert-zaremba robert-zaremba added this to the v0.41 milestone Jan 19, 2021
@amaury1093
Copy link
Contributor

For anyone debugging this, something happened between grpc v1.33.2 and v1.34.0, ref: #8162.

@clevinson
Copy link
Contributor

As an intermediate step we should update documentation to add the replace line in go.mod (which was done in Gaia for Stargate release), as many chains will run into problems ohterwise.

@robert-zaremba robert-zaremba changed the title Update gRPC version to the latest one and keep it compatible with Tendermint Update gRPC version to the latest one and keep it compatible with Tendermint | gogoproto Feb 23, 2021
@robert-zaremba
Copy link
Collaborator Author

@AmauryM was checking it and he says it's not possible to upgrade gRPC go proto for the compatibility with latest gogoproto.

I don't know what's failing in gRPC. But, maybe we can do a small patch there?

@amaury1093 amaury1093 removed their assignment Feb 23, 2021
@amaury1093
Copy link
Contributor

amaury1093 commented Feb 24, 2021

I'm not sure even that gRPC-go will accept that patch.

See grpc/grpc-go#4094

gogoproto is not officially supported by gRPC-Go.

If you want a codec that suits your needs better, it's easy to install your own. Just copy/paste this code into another package, update the parts that are important, and import it in your main package after any grpc imports. (If you don't control main, you can give it a new name and use the Codec Call-/Dial-/Server- Options.)

other people having the same issue: grpc/grpc-go#4192.


TLDR: imo the correct way to solve this issue is to switch to go protobuf v2: #7488.

I'm proposing to close this issue once the docs #8658 are merged.

@robert-zaremba
Copy link
Collaborator Author

Thanks for checking it.
Moving to google/protobuf/v2 is a long task. I'm wondering if it would make sense to to make this quicker solution to fork grpc and do required changes. Probably that wold keeps us long with gogoproto.
What do you think?

@amaury1093
Copy link
Contributor

amaury1093 commented Feb 24, 2021

My view is that there's no rush to update to the latest version of grpc-go for now: no security issue, and everything works fine in the SDK.

It's ultimately a tradeoff between:

  • how long is v2 going to take?
  • how hard is it to maintain our fork of grpc-go and/or gogoproto?

My current understanding is that v2 is maybe going to take 2-3mo, while maintaining our fork is hard (debugging encoding issues is not fun) and burdensome (we already fork gogoproto, do we might also need to fork grpc-go). With these estimations, the status quo seems the best solution.

Happy to hear other thoughts.

@clevinson
Copy link
Contributor

We've been discussing this on our Regen internal call today, closing this issue as docs have been merged from #8658. The larger follow-up will be taken care of with the custom codegenerator work we've been planning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: gRPC Issues and PRs related to the gRPC service and HTTP gateway. dependencies Pull requests that update a dependency file
Projects
None yet
Development

No branches or pull requests

4 participants