Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TokenizeShareLockStatus triggers proto linting errors #113

Open
xlab opened this issue Jun 24, 2023 · 0 comments · May be fixed by #112
Open

TokenizeShareLockStatus triggers proto linting errors #113

xlab opened this issue Jun 24, 2023 · 0 comments · May be fixed by #112

Comments

@xlab
Copy link
Contributor

xlab commented Jun 24, 2023

Hello! I've been picking upstream changes for rebasing onto 0.47.x and stumbled upon a new enum in the querier of x/staking that looks like this:

enum TokenizeShareLockStatus {
  LOCKED = 0;
  UNLOCKED = 1;
  LOCK_EXPIRING = 2;
}

This enum definition triggers 4 different linter checks in the newer 0.47.x branches that have them enabled by default.

{"path":"proto/cosmos/staking/v1beta1/query.proto","start_line":581,"start_column":1,"end_line":585,"end_column":2,"type":"COMMENT_ENUM","message":"Enum \"TokenizeShareLockStatus\" should have a non-empty comment for documentation."}
{"path":"proto/cosmos/staking/v1beta1/query.proto","start_line":582,"start_column":3,"end_line":582,"end_column":21,"type":"COMMENT_ENUM_VALUE","message":"Enum value \"LOCKED\" should have a non-empty comment for documentation."}
{"path":"proto/cosmos/staking/v1beta1/query.proto","start_line":582,"start_column":3,"end_line":582,"end_column":9,"type":"ENUM_VALUE_PREFIX","message":"Enum value name \"LOCKED\" should be prefixed with \"TOKENIZE_SHARE_LOCK_STATUS_\"."}
{"path":"proto/cosmos/staking/v1beta1/query.proto","start_line":582,"start_column":3,"end_line":582,"end_column":9,"type":"ENUM_ZERO_VALUE_SUFFIX","message":"Enum zero value name \"LOCKED\" should be suffixed with \"_UNSPECIFIED\"."}
{"path":"proto/cosmos/staking/v1beta1/query.proto","start_line":583,"start_column":3,"end_line":583,"end_column":21,"type":"COMMENT_ENUM_VALUE","message":"Enum value \"UNLOCKED\" should have a non-empty comment for documentation."}
{"path":"proto/cosmos/staking/v1beta1/query.proto","start_line":583,"start_column":3,"end_line":583,"end_column":11,"type":"ENUM_VALUE_PREFIX","message":"Enum value name \"UNLOCKED\" should be prefixed with \"TOKENIZE_SHARE_LOCK_STATUS_\"."}
{"path":"proto/cosmos/staking/v1beta1/query.proto","start_line":584,"start_column":3,"end_line":584,"end_column":21,"type":"COMMENT_ENUM_VALUE","message":"Enum value \"LOCK_EXPIRING\" should have a non-empty comment for documentation."}
{"path":"proto/cosmos/staking/v1beta1/query.proto","start_line":584,"start_column":3,"end_line":584,"end_column":16,"type":"ENUM_VALUE_PREFIX","message":"Enum value name \"LOCK_EXPIRING\" should be prefixed with \"TOKENIZE_SHARE_LOCK_STATUS_\"."}

To summarize, these are checks:

  • COMMENT_ENUM – easy to fix, just a missing comment
  • COMMENT_ENUM_VALUE – easy to fix, just a missing comment
  • ENUM_VALUE_PREFIX
  • ENUM_ZERO_VALUE_SUFFIX

Looking at the other enum definitions in Cosmos protos:

https://github.com/cosmos/cosmos-sdk/blob/48c3134afb56cc03f1c143eafa5737ae0b18ef84/proto/cosmos/gov/v1beta1/gov.proto#L126-L147

There is different style to it, to make it pass linter and make its values look nicer in the resulting code.

The most important thing is ENUM_ZERO_VALUE_SUFFIX which requires us to make the zero value represent nil and not use it anywhere. If that is done later than sooner, it will break query clients.

I've prepared a PR with fixes: #112

Feel free to merge as-is or incorporate into some bigger update.

@xlab xlab linked a pull request Jun 24, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant