search

iqlusioninc / liquidity-staking-module

Other
87 stars 29 forks source link

TokenizeShareLockStatus triggers proto linting errors #113

Open xlab opened 1 year ago

xlab commented 1 year ago

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:

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: https://github.com/iqlusioninc/liquidity-staking-module/pull/112

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