Skip to content
New issue

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

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

Already on GitHub? # to your account

Inconsistency of message types and json keys in modules #3669

Closed
kwunyeung opened this issue Feb 17, 2019 · 2 comments · Fixed by #3673
Closed

Inconsistency of message types and json keys in modules #3669

kwunyeung opened this issue Feb 17, 2019 · 2 comments · Fixed by #3673
Labels
T: State Machine Breaking State machine breaking changes (impacts consensus). Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.
Milestone

Comments

@kwunyeung
Copy link
Contributor

@jackzampolin Here is a list of message types registered in the modules.

x/bank

  1. cosmos-sdk/Send
  2. cosmos-sdk/MultiSend

x/distributions

  1. cosmos-sdk/MsgWithdrawDelegationReward
  2. cosmos-sdk/MsgWithdrawValidatorCommission
  3. cosmos-sdk/MsgModifyWithdrawAddress

x/gov

  1. cosmos-sdk/MsgSubmitProposal
  2. cosmos-sdk/MsgDeposit
  3. cosmos-sdk/MsgVote

x/slashing

  1. cosmos-sdk/MsgUnjail

x/staking

  1. cosmos-sdk/MsgCreateValidator
  2. cosmos-sdk/MsgEditValidator
  3. cosmos-sdk/MsgDelegate
  4. cosmos-sdk/Undelegate
  5. cosmos-sdk/BeginRedelegate

x/ibc

  1. cosmos-sdk/IBCTransferMsg
  2. cosmos-sdk/IBCReceiveMsg

Some of them have "Msg" as prefix and some of them not while types in IBC have it as suffix. It looks like there is a convention that when you register concrete type, you put the type string the same as the struct but this is not the case as you have cdc.RegisterConcrete(MsgDelegate{}, "cosmos-sdk/MsgDelegate", nil) but cdc.RegisterConcrete(MsgUndelegate{}, "cosmos-sdk/Undelegate", nil). It will be easier to follow if you can keep a naming convention.

Another inconsistency is in the JSON keys. Some of them are using address and some of them are addr. This can be found in the x/staking module. The keys in create_validator are delegator_address and validator_address but in delegate they are delegator_addr and validator_addr.

This also make the tags a little bit hard to follow. When there are delegator_addr and validator_addr in the tx message, the tags would be delegator, source-validator, destination-validator, etc.

It will be great if these terms can be in consistence following a convention.

@alexanderbez
Copy link
Contributor

Thanks @kwunyeung -- seems like we have some cleaning up to do in some of the modules.

@alexanderbez alexanderbez added Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity. T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine). labels Feb 18, 2019
@jackzampolin
Copy link
Member

Thank you for the detailed bug report @kwunyeung!

@alexanderbez alexanderbez added T: State Machine Breaking State machine breaking changes (impacts consensus). and removed T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine). labels Feb 18, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
T: State Machine Breaking State machine breaking changes (impacts consensus). Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants