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

Bump cosmos.base.tendermint to v1 or v1beta2 #10560

Closed
4 tasks
amaury1093 opened this issue Nov 16, 2021 · 8 comments
Closed
4 tasks

Bump cosmos.base.tendermint to v1 or v1beta2 #10560

amaury1093 opened this issue Nov 16, 2021 · 8 comments
Labels
C: Proto Proto definition and proto release T: Proto Breaking Protobuf breaking changes: never don't do this!

Comments

@amaury1093
Copy link
Contributor

Summary of Bug

Bumping to TM v0.35 introduced some proto-breaking changes. We should bump packages.

ref: #10210 (comment)

Version

After #10210

Steps to Reproduce

See related "Protobuf Breakage" failed CI: https://github.com/cosmos/cosmos-sdk/runs/4228974654?check_suite_focus=true

Proposal

Bump cosmos.base.tendermint to v1 or v1beta2.

Notes: See ADR 044 for why we need to bump


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@amaury1093 amaury1093 added T: Proto Breaking Protobuf breaking changes: never don't do this! C: Proto Proto definition and proto release labels Nov 16, 2021
@robert-zaremba
Copy link
Collaborator

What's the goal and advantage of keeping "beta"?

@amaury1093
Copy link
Contributor Author

amaury1093 commented Nov 24, 2021

(Edit: removed old answer)

cosmos.base.tendermint needs:

import "cosmos/base/query/v1beta1/pagination.proto";

If we bump cosmos.base.tendermint to v1, then we need to bump pagination to v1 too (because a stable version should not depend on a beta version). This would change a lot of other modules query.proto files.

@tac0turtle
Copy link
Member

Second to this what are your thought in generating Tendermint proto files in the sdk. The reason being Tendermint doesn't version the proto files so we could in the sdk?

@aaronc
Copy link
Member

aaronc commented Nov 24, 2021

Second to this what are your thought in generating Tendermint proto files in the sdk. The reason being Tendermint doesn't version the proto files so we could in the sdk?

So this could cause problems https://developers.google.com/protocol-buffers/docs/reference/go/faq#namespace-conflict. But, I do believe that the golang and gogo proto type registries are totally separate (thought I haven't confirmed this). So in theory we can have the same proto types with different golang and gogo proto generated code in the same binary. So maybe if we generated the tendermint types with pulsar that could work.

Is there a reason why tendermint isn't versioning it's protobuf files?

Another solution is to remove all dependencies on tm proto types in the SDK entirely.

Either way, I would like to be able to serve up the Cosmos SDK proto types in the buf schema registry and would like to get this working soon as I think it will make things easier for client development that is actively in progress.

@aaronc
Copy link
Member

aaronc commented Nov 24, 2021

So I looked into this a bit. There are just a few dependencies on tendermint right. I think I agree with your proposal now @marbar3778. I think we just need to attach a version to the tendermint packages and we manage those ourselves which I think is what you're suggesting. How about tendermint.v0_35, etc? Then we can do codegen ourselves and get everything working nicely with the schema registry too. Do you think one of you could handle this @AmauryM @marbar3778 ?

@tac0turtle
Copy link
Member

yea!! this is what i meant!! I can help with this.

@aaronc
Copy link
Member

aaronc commented Nov 24, 2021

yea!! this is what i meant!! I can help with this.

awesome thanks! so some proto files from third_party can just be deleted like the confio/ics23 stuff which the sdk no longer uses. and of course the tm stuff would then leave third_party and go in proto/ directly.

@tac0turtle
Copy link
Member

closing as 0.35 was removed from the releases

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: Proto Proto definition and proto release T: Proto Breaking Protobuf breaking changes: never don't do this!
Projects
None yet
Development

No branches or pull requests

4 participants