-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
R4R: mint use total supply instead of power #3654
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #3654 +/- ##
===========================================
+ Coverage 60.79% 60.81% +0.01%
===========================================
Files 189 189
Lines 14170 14173 +3
===========================================
+ Hits 8615 8619 +4
+ Misses 5011 5010 -1
Partials 544 544 |
@@ -100,7 +100,8 @@ type ValidatorSet interface { | |||
|
|||
Validator(Context, ValAddress) Validator // get a particular validator by operator address | |||
ValidatorByConsAddr(Context, ConsAddress) Validator // get a particular validator by consensus address | |||
TotalPower(Context) Int // total power of the validator set | |||
TotalBondedTokens(Context) Int // total bonded tokens within the validator set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interested in the rationale behind the interface change here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Power refers to Tendermint Power which is intended to be used nearly nowhere besides for Tendermint (and slashing which needs to interface with tendermint responses) - this interface didn't make a difference pre-Tendermint Power Reduction (#3400) but now these two things are intended to mean very different values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed with @rigelrozanski - this is much clearer, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🍰
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK; tiny suggested changelog wording alteration
@@ -100,7 +100,8 @@ type ValidatorSet interface { | |||
|
|||
Validator(Context, ValAddress) Validator // get a particular validator by operator address | |||
ValidatorByConsAddr(Context, ConsAddress) Validator // get a particular validator by consensus address | |||
TotalPower(Context) Int // total power of the validator set | |||
TotalBondedTokens(Context) Int // total bonded tokens within the validator set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed with @rigelrozanski - this is much clearer, thanks
closes #3646
Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
Wrote tests
Updated relevant documentation (
docs/
)Added entries in
PENDING.md
with issue #rereviewed
Files changed
in the github PR explorerFor Admin Use: