R4R: Cap(clip) reward to remaining coins in AllocateTokens #3726
Conversation
jaekwon
requested review from
cwgoes,
ebuchman and
rigelrozanski
as code owners
| // ref https://github.com/cosmos/cosmos-sdk/issues/2525#issuecomment-430838701 | ||
| powerFraction := sdk.NewDec(vote.Validator.Power).Quo(sdk.NewDec(totalPower)) | ||
| reward := feesCollected.MulDec(voteMultiplier).MulDec(powerFraction) | ||
| reward = reward.Cap(remaining) |
There was a problem hiding this comment.
Hmm is this the best way to fix? If this is necessary, it seems like our multiplication somewhere rounds up where it ought to always floor, so instead I wonder if we should change the multiplication (MulDec maybe) to always floor, or add a version that does and use it here.
|
CI is failing and if we do decide to go with this approach, it should have a pending log entry. |
|
Complementary fix with truncation: #3728 |
Codecov Report
@@ Coverage Diff @@
## develop #3726 +/- ##
==========================================
Coverage ? 61.46%
==========================================
Files ? 189
Lines ? 14057
Branches ? 0
==========================================
Hits ? 8640
Misses ? 4876
Partials ? 541 |
I thought we were going with #3728? |
They're complementary, the additional defensive coding is fine I think. |
|
This is failing tests and needs a rebase. |
jaekwon
force-pushed
the
jae/distr_allocation_bug
branch
from
0dd3a35 to
85e3a6a
Compare
Related to #3722
OUTSTANDING TODO: pull out #3722's test into its own test that would fail without these changes.