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

Fix validator power decrease bug #1566

Merged
merged 7 commits into from
Jul 6, 2018

Conversation

cwgoes
Copy link
Contributor

@cwgoes cwgoes commented Jul 6, 2018

  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Updated CHANGELOG.md
  • Updated Gaia/Examples
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)
  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)

@rigelrozanski
Copy link
Contributor

Are no updates sent back to Tendermint at all in this circumstance? i.e. does it create a change but just not the correct one?

@cwgoes
Copy link
Contributor Author

cwgoes commented Jul 6, 2018

Are no updates sent back to Tendermint at all in this circumstance? i.e. does it create a change but just not the correct one?

No updates are sent back - as demonstrated in the testcase, and just reading through the logic in UpdateValidator, we set the Tendermint update key if (a) the validator's power is increasing (still bonded), (b) the validator switches from unbonded to bonded, or (c) the validator switches from bonded to unbonded - but if the validator stays bonded but decreases in power, we don't set any Tendermint update at all.

@cwgoes cwgoes changed the title WIP: Fix validator power decrease bug Fix validator power decrease bug Jul 6, 2018
@codecov
Copy link

codecov bot commented Jul 6, 2018

Codecov Report

Merging #1566 into develop will increase coverage by 0.02%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop    #1566      +/-   ##
===========================================
+ Coverage    64.07%   64.09%   +0.02%     
===========================================
  Files          122      122              
  Lines         6666     6670       +4     
===========================================
+ Hits          4271     4275       +4     
  Misses        2148     2148              
  Partials       247      247

@cwgoes
Copy link
Contributor Author

cwgoes commented Jul 6, 2018

Ready for review.

I think we should try to simplify the maze of conditionals in UpdateBondedValidators in a future PR, perhaps using case statements or separating the iteration / store update steps more cleanly.

@rigelrozanski
Copy link
Contributor

Totally agree that this is a great place for case statements to make more clear - opened an issue #1584

Copy link
Contributor

@rigelrozanski rigelrozanski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simple enough changes! Thanks!

@rigelrozanski rigelrozanski merged commit 723e057 into develop Jul 6, 2018
@rigelrozanski rigelrozanski deleted the cwgoes/validator-power-decrease-bug branch July 6, 2018 22:00
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 this pull request may close these issues.

2 participants