chore: Add "Since:" on proto doc comments

[amaury1093]

Description

ref: #10406 (reply in thread)

For clients to know whether a protobuf feature is available for a certain SDK version, we decided to use the @since doc comment inside protobuf files.

---

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

• included the correct type prefix in the PR title
• added ! to the type prefix if API or client breaking change
• targeted the correct branch (see PR Targeting)
• provided a link to the relevant issue or specification
• followed the guidelines for building modules
• included the necessary unit and integration tests
• added a changelog entry to CHANGELOG.md
• included comments for documenting Go code
• updated the relevant documentation or specification
• reviewed "Files changed" and left comments if necessary
• confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

• confirmed the correct type prefix in the PR title
• confirmed ! in the type prefix if API or client breaking change
• confirmed all author checklist items have been addressed
• reviewed state machine logic
• reviewed API design and naming
• reviewed documentation is accurate
• reviewed tests and test coverage
• manually tested (if applicable)

[codecov]

Codecov Report

Merging #10434 (42d543e) into master (b565069) will increase coverage by 0.03%.
The diff coverage is n/a.

❗ Current head 42d543e differs from pull request most recent head 83a30b8. Consider uploading reports for the commit 83a30b8 to get more accurate results

@@            Coverage Diff             @@
##           master   #10434      +/-   ##
==========================================
+ Coverage   64.16%   64.19%   +0.03%     
==========================================
  Files         575      575              
  Lines       54923    54923              
==========================================
+ Hits        35239    35256      +17     
+ Misses      17682    17664      -18     
- Partials     2002     2003       +1     

Impacted Files	Coverage Δ
crypto/keys/internal/ecdsa/privkey.go	82.45% <0.00%> (-1.76%)	⬇️
x/distribution/simulation/operations.go	90.27% <0.00%> (+9.72%)	⬆️

[amaury1093]

other ideas that came up:

• @since cosmos-sdk 0.43
• @since github.com/cosmos/[email protected] (but that's not actually a valid github or go module tag)
• @since 0.43

Opinions?

[webmaster128]

Turns out there is a bit of a collision then using the JSDoc/TSDoc style @since. It is represented as if it's about the JS/TS library version. See https://jsdoc.app/tags-since.html The same problem would apply for types generated in Java.

I think it's better to avoid this conflict and just make it plain text for the callers, starting with Since:.

Since: cosmos-sdk 0.43 would be my favourite. It is clean, refers to the repo name and allows us to split by whitespace when parsing.

[aaronc]

I think that's actually okay if we stick to strict API based versioning - the npm or maven package version would line up with the go version. I know want to have a high level user friendly package. But for individual low level proto packages I think could make a lot of sense to have own npm package per proto package with the exact same API version. Then the @since tag would work nicely with existing tools.

[amaury1093]

I think that's actually okay if we stick to strict API based versioning - the npm or maven package version would line up with the go version

Personally I actually don't see a lot of advantages for having the npm/maven/go/proto types versions all be the same. On the other hand, one disadvantage I see is the need to lock proto bumping speed with JS/Java/Rust bumping speed.

Simon mentioned yesterday about CosmJS using the long library for big integers. If CosmJS decides to change that to another lib (e.g. the native TypeScript bigint) which is a breaking change, they would need to wait for the proto pkg version to bump... which by design would take a long time.

[webmaster128]

While I agree with the idea of module specific packes that are close to Cosmos SDK versioning, enforcing the same version number across different client ecosystems seems to be very restricting. There can be breaking changes in the code generation that are not related to the API itself as Amury described. Some ecosystems structure in smaller packages, some in larger. Also note that the code generation does not necessarily happen in a type library. You can always add types in the application. Then you suddenly bind Cosmos SDK version to the version of the app.

[webmaster128]

Looks good to me. Do you mind backporting this to 0.44?

[amaury1093]

For authz, I added this comment on all messages, and on the Query and Msg services.

Would it be possible to add it on the top-level package only?

syntax = "proto3";
// Since: cosmos-sdk 0.43
package cosmos.authz.v1beta1;

(note: that's what I did for cosmos.base.reflection.v2alpha1 for now, because there's a lot of stuff inside that file).

[webmaster128]

Good question. I agree there should be package-level documentation. However, there does not seem to be a point in the *.proto definitions where a package is created. It's created implicitly once you assign messages to it and as a consequence this is spread across multiple files.

In ts-proto it looks like file-level comments in the very top are copied to the .ts output, but later comments and package comments are not:

--- a/proto/cosmos/auth/v1beta1/auth.proto
+++ b/proto/cosmos/auth/v1beta1/auth.proto
@@ -1,4 +1,9 @@
+// Foo
 syntax = "proto3";
+
+// Bar
+
+// Since: cosmos-sdk 0.42
 package cosmos.auth.v1beta1;
 
 import "cosmos_proto/cosmos.proto";

becomes

--- a/src/cosmos/auth/v1beta1/auth.ts
+++ b/src/cosmos/auth/v1beta1/auth.ts
@@ -5,6 +5,8 @@ import { Any } from "../../../google/protobuf/any";
 
 export const protobufPackage = "cosmos.auth.v1beta1";
 
+/** Foo */
+
 /**
  * BaseAccount defines a base account type. It contains all the necessary fields
  * for basic accou

So what about putting it in the top of the file?

[amaury1093]

For a dev that imports CosmJS, would their IDE intellisense show "Foo" somewhere?

It would be ideal if that comment showed up everytime they typed cosmos.authz{...}

[webmaster128]

For a dev that imports CosmJS, would their IDE intellisense show "Foo" somewhere?

No, but at least its visible when you dig into it or have it available for support.

It would be ideal if that comment showed up everytime they typed cosmos.authz{...}

Unfortunately we don't get the proto packages bundled into a common object. They are imported by file like this. See also cosmos/cosmjs#647.

[amaury1093]

Personally I also find it more elegant to put it in the top of the file. I did that in f74fd81.

ts-proto outputs the comments just fine, it would be great to get an idea on other languages (e.g. Rust) too. Duplicating those comments is cumbersome, but also safer (will be outputed on intellisense0. Any thoughts @aaronc ?

[amaury1093]

R4R. Hopefully I didn't forget any place (I used git diff on .proto files)

[webmaster128]

LGTM, thanks!