Protobuf package versioning

[aaronc]

@AmauryM @webmaster128 and I were discussing today some issues CosmJS is facing upgrading to v0.44 protobuf files. So even though all the changes are backwards compatible - i.e. a v0.42 CosmJS client can talk to a v0.44 chain, it will be a bit complicated for a v0.44 CosmJS to talk to a v0.42 chain. Most things should work, but some things will be a bit complicated for the v0.44 client to understand what things won't work against v0.42 and will either cause an error or get ignored entirely.

I'll quote @webmaster128 on some of these issues:

In https://github.com/confio/cosmjs-types/pull/8/files you see the diff I get when upgrading v1beta1 types from 0.42 to 0.44. Now assume I override those files with the latest version and want to be compatible to 0.42-0.44 backends. I have the following questions:

• How do I know if Accounts query is available? Reflection I guess, ok, will try.
• Do I get values in Metadata.name and Metadata.symbol?
• Does the TotalSupply query support pagination?
• Can I use PageRequest.reverse?
• Does VersionInfo.cosmosSdkVersion contain a value?
• Is MsgVoteWeighted available? Reflection I guess, ok, will try.
• Can I use GetTxsEventRequest.orderBy?
• Can I use SimulateRequest.txBytes?

Those questions are the reason we have no 0.44 support yet, because I hesitate to override types.

Here are the solutions I can think of:

1. Reflection: rely on gRPC reflection and cosmos.base.reflection - this should provide all of the information at runtime and should be enabled on stargate+ plus chains. I’m not sure how the DevX for this will be? Maybe it’s a bit complicated?
2. Annotations: introduce annotations into proto files which indicate when a given query/field was added
3. Bump Proto Version for Minor Changes: bump the proto version when we are adding new things but still support old proto versions as much as possible - this way a client can develop against a single major version of an API and just check if that’s supported. with this you’d end up with scenarios where a chain might support v1 partially (because it’s being deprecated), and say v2 and v3 fully - and a reflection service could tell which API versions are supported. maybe this is preferable to gRPC reflection?
4. New Functionality Goes in New Proto Versions: similar to the (3) but just add new things to new proto versions without copying over everything. So v1 would have one layer of functionality, v2 would add some new or changed methods, etc.

---

It seems like (1) will be a bit cumbersome for devs because it doesn't take advantage of static typing - a lot of runtime feature checks will be required.

(2) is something that we could retroactively add to v0.44 to support clients.

I think (3) might be a bit too heavy-handed because you'd have a lot of stuff duplicated in multiple proto packages with subtle differences. For instance, it would be hard for a chain to support all v1 chains while using some v2 features if they happen to be available.

For this reason, I'm sort of leaning towards approach (4) where only new/significantly changed things would go in new proto packages and older packages would start gradually getting full or partial deprecation annotations. Also, this would mean that clients don't need to refactor existing functionality and change package paths when they want to support just some functionality from a new version.

I do think it's probably okay to add new fields to response types in existing proto packages without causing much confusion, but clients would just need to assume they're nullable which should probably always be the case with protobuf types.

---

What do others think? @webmaster128 @AmauryM ?

[amaury1093]

I have some doubts about (4):

1. If we're adding one query rpc between v2 and v3 to an existing Query service, wouldn't we need to copy the whole Query service and all its Request/Response methods from v2 to v3? It would still be hard to find the diff. (another "worse" example: adding the reverse bool field to pagination)

2. As I understand, we're never actually removing anything from proto defs, ever, is that correct? You mention "full or partial deprecation annotations", how can a client know if a feature is removed (i.e. node will error) or just deprecated (i.e. will work for now).

I personally think (3) is the simplest solution. We just need to be disciplined in showing a clear changelog. We obviously should strive for the maximum backwards-compatibility possible, but if a non-backwards-compatible breaking change arises, we'll still have some flexibility.

[aaronc]

I have some doubts about (4):

1. If we're adding one query rpc between v2 and v3 to an existing Query service, wouldn't we need to copy the whole Query service and all its Request/Response methods from v2 to v3? It would still be hard to find the diff. (another "worse" example: adding the reverse bool field to pagination)

Yes the reverse pagination example is very concerning because that's a dependency of many Query services so they would all need to be redefined to take advantage of this one feature... which kind of has me leaning in the direction of relying on annotations (2) or reflection (1).

2. As I understand, we're never actually removing anything from proto defs, ever, is that correct? You mention "full or partial deprecation annotations", how can a client know if a feature is removed (i.e. node will error) or just deprecated (i.e. will work for now).

There would need to be annotations in the proto files or the client would need to use the reflection service I think.

I personally think (3) is the simplest solution. We just need to be disciplined in showing a clear changelog. We obviously should strive for the maximum backwards-compatibility possible, but if a non-backwards-compatible breaking change arises, we'll still have some flexibility.

The big downside I see of (3) are that it discourages rapid adoption of new features. Migrating to a newer v2 or v3 API in your code-base would mean that you need to completely disable support for older chains. Because the proto type URLs would change, older chains would simply not understand your client at all. This would mean that clients would be very hesitant to support newer features and we'd end up in the situation we have with web browsers where new features take a long time to be adopted and technical debt accumulates for many years. This feels like almost the worst case outcome to be honest.

[webmaster128]

My original idea was to have one client lib package per backend. Then the client dev has to decide once which chain they want to connect to but then can laverage full static typing of the whole API. However, there are clearly downsides of this approch. If we assume the API is more or less the same across multiple SDK versions, we can ask the client dev to make case distictions where needed. E.g. a reverse feature might be in the types but does not work on all chains. Some fields are expected to be empty in SDK < 0.44 such as cosmosSdkVersion. But for the vast majority of users those differences do not matter. They want the happy path to be easy and the rest they'll figure out with testing.

In order to support that, (2) sounds like a great start. As you can see in https://github.com/confio/cosmjs-types/blob/372c0256faa5b37576a03d44be100b2033a2a828/src/cosmos/bank/v1beta1/bank.ts#L35-L87, the field-level annotations propagate into TS types and will be available in modern IDEs for both TS and JS developers. Having the information there in place would be great (e.g. "Added in Cosmos SDK 0.44"). If such annotations get some sort of common format, they can even be used by linters (you told me you want to support 0.44-0.46, don't use this 0.48+ feature). This linter does not yet exist but someone will write it eventually as Android did it with Java comments.

I'm not sure how to work with reflections (1). I guess I cannot use them for static typing. It could however be a way to make nice case distinctions at runtime. This would be a lot of code, requests and testing for each functionality as it moves compile time checks to run time checks. Having reflections is nice for manual debugging for grpc though.

[amaury1093]

One small addition about (2): if I understood your paragraph, adding annotated comments like the @since directive is helpful, and we can add them. But on top of comments, (2) would also add Protobuf annotation/options, see example, we would add for example [(cosmos_proto.since) = "1.1"].

If you decide to fork ts-proto on day (which IMO might bring some benefits), ts-proto can read those proto annotation pretty easily, and generate (or not) some code depending on their values.

[webmaster128]

if I understood your paragraph, adding annotated comments like the @since directive is helpful, and we can add them.

Yes, cool.

on top of comments, (2) would also add Protobuf annotation/options, see example, we would add for example [(cosmos_proto.since) = "1.1"].

Ah, hmm, so far I was thinking about doc comments when we said "annotations". I looked around and found very little documentation. But those things in brackets after the field seem to be called custom field options. Makes sense 👍 for better processing. For now I'd rather have the info in both places.

ts-proto can read those proto annotation pretty easily, and generate (or not) some code depending on their values.

Yes, much better than the text processing approach I envisioned.

[amaury1093]

We discussed this in a call:

• don't use both anotations and doc comments. Let's focus on doc comments first @since
• format:
  • for now: @since Cosmos SDK 0.44
  • if we switch to some API versioning: @since 1.2
  • Create a PR, maybe ask other people for formatting ideas chore: Add "Since:" on proto doc comments #10434

[robert-zaremba]

For now I'd rather have the info in both places.

A compiler can figure out what to do with the annotation and how to use them when generating a code.