search

cosmos / cosmos-sdk

:chains: A Framework for Building High Value Public Blockchains :sparkles:
https://cosmos.network/
Apache License 2.0
6.24k stars 3.61k forks source link

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

Closed amaury1093 closed 2 years ago

amaury1093 commented 2 years ago

Summary of Bug

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

ref: https://github.com/cosmos/cosmos-sdk/pull/10210#discussion_r750170112

Version

After https://github.com/cosmos/cosmos-sdk/pull/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

robert-zaremba commented 2 years ago

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

amaury1093 commented 2 years ago

(Edit: removed old answer)

cosmos.base.tendermint needs:

https://github.com/cosmos/cosmos-sdk/blob/60483cd2bfd566622eef50db815d522a15f14d69/proto/cosmos/base/tendermint/v1beta1/query.proto#L9

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

aaronc commented 2 years ago

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 commented 2 years ago

closing as 0.35 was removed from the releases