Address v1beta1 proto breaking changes

[aaronc]

As reported by @webmaster128 on discord:

Hey SDK team, I noticed a change in message Vote in https://github.com/cosmos/cosmos-sdk/blob/v0.43.0-beta1/proto/cosmos/gov/v1beta1/gov.proto#L134-L136 which as discussed by @sunny Aggarwal | Sikka and @aaronc | Regen Network at #7802 (comment).

Now the problem for the client is: we cannot use the new .proto file for interacting with a 0.42 chain. The necessary field is simply missing (even if reserved). Why is this considered a non-breaking .proto change with no version bump in the pro namespace?

Correct my if I'm wrong, but my understanding was that a proto file is versioned by its namespace, not the SDK version.

Same for #8673 (comment). No multi-backend client is possible with that change. Seems like that Google describes there in their upgrading instructions assume a central server with multiple clients around it.

[amaury1093]

Some proposals we can do for v0.43:

Proposal 1. Remove all reserved fields in the proto files, replace with what we had before. Notes:

• we should also add backwards-compatability in the handlers. E.g. convert a Vote to a 100% WeightedVote.
• might need to double-check if we added new fields in Msgs, and how they work with proto unknown field rejectors.

Proposal 2. Bump proto packages to v1beta2 or even v1 (#9445)

It seems like proposal 1 is the easiest path for v0.43, but we're discussing with @webmaster128 next week to discuss a more general proto updates strategy.

[aaronc]

I think we should start with approach 1.

Although it might not be a bad idea to bump proto files to v1 if we can maintain compatibility with v1beta1 for 0.43... that seems like a bunch more work though.

I do have a concern that adding any field to a Msg could cause breaking changes for clients because of unknown field rejection and UI's populating fields that aren't supported. This may force us to separate query and tx proto packages - where queries can easily add fields but tx's must always bump the version to add fields. This would look like cosmos.bank.query.v1, cosmos.bank.tx.v1, etc. I'm not sure if this is necessary but we should definitely discuss.

[amaury1093]

This may force us to separate query and tx proto packages - where queries can easily add fields but tx's must always bump the version to add fields. This would look like cosmos.bank.query.v1, cosmos.bank.tx.v1, etc. I'm not sure if this is necessary but we should definitely discuss.

We could also just make sure to never add fields in Msgs (e.g. manually, during PR reviews). If someone needs to add a new field, then create a new Msg. It somehow feels complex and confusing to me to have query and tx packages with different versions for the same module.

[amaury1093]

Had a call with @webmaster128 and @robert-zaremba about proto bumping strategy.

The current proposed strategy for solving this issue is:

• aim for bumping all proto packages to v1 in v0.43
• ask the community if there's strong objection to bumping to v1 when v0.43 is already in the beta testing phase
• if there is opposition, fallback to Proposal to add backwards-compatability in keeper handlers.

Meeting notes

[webmaster128]

After thinking about this a little bit more: Do we know which modules contain incompatible changes between 0.42 and 0.43? Are there modules for which we can keep the v1beta1? Having to create case distictions for all higher level Stargate client code is going to be very tricky (and will lead to poor support of one of those versions).

[robert-zaremba]

@AmauryM was checking that.
For 0.43 we can only lift the modules with incompatible changes.
For 0.44 we will probably like to set v1 for all other modules anyway drop the beta tag which doesn't make sense any more.

[amaury1093]

I added a quick audit based on a git diff between in the meeting notes in #9446 (comment).

The 2 places where I see breaking changes are:

• cosmos.gov..v1beta1.Vote
• cosmos.upgrade.v1beta1.Plan

How do we feel about only bumping 2 modules to v1, and not others? It might give the wrong impression the gov & upgrade modules are "more production-ready" than others, which is wrong.

Having to create case distictions for all higher level Stargate client code is going to be very tricky (and will lead to poor support of one of those versions).

Could you explain more in details what you mean here?

[webmaster128]

It might give the wrong impression the gov & upgrade modules are "more production-ready" than others, which is wrong.

Yeah, releasing Stargate with the "beta" version was not great. But 🤷‍♂️ it will vanish over time.

How do we feel about only bumping 2 modules to v1, and not others?

I see your point regarding consustency. But this would reduce a lot of practical pain becauese 👇. The difference is really: SDK 0.43 support today vs. SDK 0.43 support one day soon™️.

Could you explain more in details what you mean here?

Have a look at SigningStargateClient.sendTokens: https://github.com/cosmos/cosmjs/blob/v0.25.4/packages/stargate/src/signingstargateclient.ts#L212-L227. In this simple case we fix the versioned proto file in 3 places:

1. https://github.com/cosmos/cosmjs/blob/v0.25.4/packages/stargate/src/signingstargateclient.ts#L219
2. https://github.com/cosmos/cosmjs/blob/v0.25.4/packages/stargate/src/encodeobjects.ts#L9
3. https://github.com/cosmos/cosmjs/blob/v0.25.4/packages/stargate/src/encodeobjects.ts#L3

So we'd have to duplicate all this code for different versions, even if the type did not change at all.

Second problem is I don't even know which type to use for which backend. Or can I query the SDK version in a reliable and stable way?

[amaury1093]

Makes sense. So current consensus is to:

• update gov & upgrade proto packages to v1 for v0.43.
• keep all other proto packages as v1beta1, update them all to v1 for v0.44.

[amaury1093]

Had another talk with @aaronc and @robert-zaremba, the latest decision we're going with is:

• NOT bump gov&upgrade to v1 for 0.43
  • Add error handling for deprecated fields, and graceful support for Vote->WeightedVote conversion
• Bump everything to v1 at once for 0.44

(Sorry for the back & forth, we're trying to get inputs from different parties)

Some reasons we advanced:

• a working group is working towards a Msg-based gov proposal mechanism, which would remove ProposalHandlers. This would break proto, so it doesn't make sense to bump gov to v1
• some clients still use MsgVote, it might be too premature to only support v1 just yet