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

Staking renaming #3289

Closed
rigelrozanski opened this issue Jan 12, 2019 · 3 comments · Fixed by #3314
Closed

Staking renaming #3289

rigelrozanski opened this issue Jan 12, 2019 · 3 comments · Fixed by #3314
Assignees
Labels
C:x/staking good first issue T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine).

Comments

@rigelrozanski
Copy link
Contributor

rigelrozanski commented Jan 12, 2019

in the interest of changing APIs prelaunch (sooner than later) as I've been reviewing staking there are a few renames which I think we ought to update:

  • MsgBeginUnbonding -> MsgUndelegate
    • Begin unbonding doesn't really make sense because the validator may be unbonding (in which case the tokens have already begun unbonding) or unbonded (tokens do not enter into an UnbondingDelegation object)
  • field name Delegation -> Amount (these are type sdk.Coins) in MsgCreateValidator and MsgDelegate
    • Delegation is an ambiguous field name because of the Delegation type in x/stake/types/delegation.go
  • Validator.UnbondingMinTime -> Validator.UnbondingCompletionTime stay consistent with unbonding delegation object/thinking

CC @cwgoes @jackzampolin @faboweb

@rigelrozanski rigelrozanski added C:x/staking T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine). prelaunch labels Jan 12, 2019
@rigelrozanski rigelrozanski self-assigned this Jan 12, 2019
@rigelrozanski rigelrozanski removed their assignment Jan 12, 2019
@jackzampolin
Copy link
Member

jackzampolin commented Jan 14, 2019

I like both of these changes. Will this also necessitate any changes on the query side? Would be interested to hear @faboweb's thoughts from the Voyager side of things.

@faboweb
Copy link
Contributor

faboweb commented Jan 15, 2019

I agree with those changes. 👍

@cwgoes
Copy link
Contributor

cwgoes commented Jan 15, 2019

All listed changes look fine to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:x/staking good first issue T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants