cosmos / cosmos-sdk

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

Protobuf Golang v2 Migration #7488

Closed aaronc closed 4 months ago

aaronc commented 3 years ago

Summary

Given the precarious future of gogo protobuf, we should assess our dependence on it before reaching v1.0, and establish a working group to come up with a concrete design.

Problem Definition

When the Stargate protobuf migration began, https://github.com/gogo/protobuf appeared to be the clearly superior implementation compared to https://github.com/golang/protobuf. It is both faster and allows for more canonical go structures through extensions which we used extensively especially initially.

In March, the official go protobuf implementation announced a new API: https://blog.golang.org/protobuf-apiv2 and in May gogo protobuf announced that they were looking for new maintainers: https://github.com/gogo/protobuf/issues/691.

As of now the future of gogo protobuf looks precarious and it seems likely that it will fall into disuse in spite of being the superior implementation.

We are using gogoproto annotations quite extensively in our proto files currently and switching to the official go proto would likely mean needing to abandon any sort of amino support (#6190). Also, the official go proto implementation is still slower than gogo proto.

On other hand, as we are thinking towards a v1.0 (#7421), it would be ideal to release our stable API based on a protobuf library that is going to be maintained.

Proposal

For now, let's not do anything and wait and see how the community responds to the gogo protobuf situation. If gogo protobuf finds an active maintainer in the meantime, we may not need to do anything.

If it doesn't find a maintainer, before we release a v1.0 we should either:


For Admin Use

tac0turtle commented 3 years ago

abandon gogo protobuf and migrate to the official implementation, or decide to take on maintainership of gogo protobuf ourselves

To aid in this decision an outline of the used/needed features of gogoproto should be made. Many of the features can be recreated without using gogoproto.

aaronc commented 3 years ago

To aid in this decision an outline of the used/needed features of gogoproto should be made. Many of the features can be recreated without using gogoproto.

Here are the extensions we are using (based on the ripgrep command below):

❯ rg -o -I "\(gogoproto\..*?\)" proto | sort | uniq
(gogoproto.castrepeated)
(gogoproto.casttype)
(gogoproto.customname)
(gogoproto.customtype)
(gogoproto.description)
(gogoproto.embed)
(gogoproto.enumvalue_customname)
(gogoproto.equal)
(gogoproto.equal_all)
(gogoproto.goproto_enum_prefix)
(gogoproto.goproto_getters)
(gogoproto.goproto_getters_all)
(gogoproto.goproto_stringer)
(gogoproto.goproto_stringer_all)
(gogoproto.goproto_unrecognized)
(gogoproto.jsontag)
(gogoproto.moretags)
(gogoproto.nullable)
(gogoproto.stdduration)
(gogoproto.stdtime)
(gogoproto.stringer)
(gogoproto.stringer_all)
tac0turtle commented 3 years ago

Do we know what is required and what is being used because it's available? Differentiating what is modifying struct and what is not would be helpful as well. The items that are just methods we can implement as a side process similar to https://github.com/marbar3778/go-proto-wrapper/

aaronc commented 3 years ago

So if we could only choose one thing from that list I would say customtype. Currently, for instance we've now changed all go structs to use string for addresses instead of sdk.AccAddress. I believe grpc gateway had some issues with customtype there. string is not necessarily terrible, but it's inconvenient to be forced into this by the serialization layer.

tac0turtle commented 3 years ago

When talking to the gogoproto maintainer we agreed rewriting gogoproto was the best approach.

Customtype will require a decent amount of work, we will need to write a simple compiler that adheres to the proto v2 interface.

aaronc commented 3 years ago

When talking to the gogoproto maintainer we agreed rewriting gogoproto was the best approach.

Customtype will require a decent amount of work, we will need to write a simple compiler that adheres to the proto v2 interface.

Yes, I've been thinking the same thing. And I believe the proto v2 approach is in some ways a lot more flexible. If there are decent code generators it could be pretty good.

amaury1093 commented 3 years ago

+1 for moving away from gogoproto.

This might be an unfrequent use case, but gogoproto also breaks protobuf service reflection. ref: https://jbrandhorst.com/post/gogoproto/#reflection

aaronc commented 3 years ago

It wasn't that hard to write a custom protobuf service code generator: https://github.com/regen-network/regen-ledger/pull/207#issuecomment-739005986. So that implementing and calling this interfaces is more ergonomic in the SDK. I just forked the official grpc codegen, deleted a bunch of stuff for streaming which we don't need, and tweaked the types in a few places.

I imagine writing a proper go proto v2 code generator that does custom types and hides Anys isn't as hard as it sounds.

clevinson commented 3 years ago

This was discussed on today's architecture call:

The general consensus from this call was that a basic protobuf service code generator should probably live directly within the SDK repo (upstreamed from the work @aaronc recently did in Regen Ledger).

As for a more general golang proto v2 code generator, attendees were largely in favor of us writing our own code generator. We decided it should most likely live under its own repository (in the Cosmos org), so it can be used my multiple projects within the cosmos ecosystem (e.g. both cosmos-sdk and tendermint). There was some discussion of potentially treating this as a more general purpose project for the larger Golang community, but we were not convinced that taking on such a project would be worth the potential additional maintenance overhead of satisfying the larger Golang community's user needs.

tac0turtle commented 3 years ago

It wasn't that hard to write a custom protobuf service code generator: https://github.com/regen-network/regen-ledger/pull/207#issuecomment-739005986. So that implementing and calling this interfaces is more ergonomic in the SDK. I just forked the official grpc codegen, deleted a bunch of stuff for streaming which we don't need, and tweaked the types in a few places.

I imagine writing a proper go proto v2 code generator that does custom types and hides Anys isn't as hard as it sounds.

Awesome to see this work. Generally not a fan of forking like this as it can easily become burdensome to keep track of upstream changes if we modify things. This is what happened to gogoproto.

aaronc commented 3 years ago

@marbar3778 the idea is that we implement code generators compatible with the golang proto v2 spec. Especially for service code generators, the protobuf spec actually recommends custom code generators. I'm also not sure that's what happened to gogo proto - they tried to do a bunch of stuff that diverged pretty heavily from upstream.

Regarding golang proto v2 in general, I don't see a way around a custom code generator because the default implementation is 100% reflection based. So I would like us to talk about soon what it looks like to have a custom golang v2 generator that supports a very limited set of annotations instead of the whole grab bag that gogo proto supports.

aaronc commented 3 years ago

Small update: we are spinning out a small working group to align on a specification for a golang v2 code generator with the tendermint core team.

amaury1093 commented 3 years ago

The protov2 generator will live in a separate repo: https://github.com/cosmos/cosmos-proto. The 1st issue there contains some meeting notes from the working group.

tac0turtle commented 3 years ago

Can we close this issue? Don't think we need an issue in this repo till its time to migrate

dnavre commented 3 years ago

Hi guys, you can also consider using this newish lib: https://vitess.io/blog/2021-06-03-a-new-protobuf-generator-for-go/ https://github.com/planetscale/vtprotobuf

I'm not affiliated with the authors in any way, but though it might be useful to post it here.

tac0turtle commented 3 years ago

Hi guys, you can also consider using this newish lib: https://vitess.io/blog/2021-06-03-a-new-protobuf-generator-for-go/ https://github.com/planetscale/vtprotobuf

I'm not affiliated with the authors in any way, but though it might be useful to post it here.

we looked at it but are not a fan of another file, we talked and decided to attempt writing a custom generator against the proto apiv2 interface, that way we can extend more custom features.

faddat commented 2 years ago

how can I help with this?

aaronc commented 2 years ago

how can I help with this?

@faddat see In Progress and Ready columns here: https://github.com/orgs/cosmos/projects/10