diff --git a/PENDING.md b/PENDING.md index 712f9d72a..906b89ead 100644 --- a/PENDING.md +++ b/PENDING.md @@ -86,6 +86,8 @@ CLI flag. ### SDK +* \#3728 Truncate decimal multiplication & division in distribution to ensure + no more than the collected fees / inflation are distributed * \#3727 Return on zero-length (including []byte{}) PrefixEndBytes() calls * \#3559 fix occasional failing due to non-determinism in lcd test TestBonding where validator is unexpectedly slashed throwing off test calculations diff --git a/x/distribution/keeper/allocation.go b/x/distribution/keeper/allocation.go index 646784204..3f6c423c0 100644 --- a/x/distribution/keeper/allocation.go +++ b/x/distribution/keeper/allocation.go @@ -31,8 +31,8 @@ func (k Keeper) AllocateTokens(ctx sdk.Context, sumPrecommitPower, totalPower in // calculate proposer reward baseProposerReward := k.GetBaseProposerReward(ctx) bonusProposerReward := k.GetBonusProposerReward(ctx) - proposerMultiplier := baseProposerReward.Add(bonusProposerReward.Mul(fractionVotes)) - proposerReward := feesCollected.MulDec(proposerMultiplier) + proposerMultiplier := baseProposerReward.Add(bonusProposerReward.MulTruncate(fractionVotes)) + proposerReward := feesCollected.MulDecTruncate(proposerMultiplier) // pay proposer proposerValidator := k.stakingKeeper.ValidatorByConsAddr(ctx, proposer) @@ -50,8 +50,8 @@ func (k Keeper) AllocateTokens(ctx sdk.Context, sumPrecommitPower, totalPower in // TODO likely we should only reward validators who actually signed the block. // 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) + powerFraction := sdk.NewDec(vote.Validator.Power).QuoTruncate(sdk.NewDec(totalPower)) + reward := feesCollected.MulDecTruncate(voteMultiplier).MulDecTruncate(powerFraction) k.AllocateTokensToValidator(ctx, validator, reward) remaining = remaining.Sub(reward) } diff --git a/x/distribution/keeper/allocation_test.go b/x/distribution/keeper/allocation_test.go index d2c354a10..4ac5b3f65 100644 --- a/x/distribution/keeper/allocation_test.go +++ b/x/distribution/keeper/allocation_test.go @@ -106,3 +106,74 @@ func TestAllocateTokensToManyValidators(t *testing.T) { // proposer reward + staking.proportional for second proposer = (5 % + 0.5 * (93%)) * 100 = 51.5 require.Equal(t, sdk.DecCoins{{sdk.DefaultBondDenom, sdk.NewDecWithPrec(515, 1)}}, k.GetValidatorCurrentRewards(ctx, valOpAddr2).Rewards) } + +func TestAllocateTokensTruncation(t *testing.T) { + communityTax := sdk.NewDec(0) + ctx, _, k, sk, fck := CreateTestInputAdvanced(t, false, 1000000, communityTax) + sh := staking.NewHandler(sk) + + // initialize state + k.SetOutstandingRewards(ctx, sdk.DecCoins{}) + + // create validator with 10% commission + commission := staking.NewCommissionMsg(sdk.NewDecWithPrec(1, 1), sdk.NewDecWithPrec(1, 1), sdk.NewDec(0)) + msg := staking.NewMsgCreateValidator(valOpAddr1, valConsPk1, + sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(110)), staking.Description{}, commission, sdk.OneInt()) + require.True(t, sh(ctx, msg).IsOK()) + + // create second validator with 10% commission + commission = staking.NewCommissionMsg(sdk.NewDecWithPrec(1, 1), sdk.NewDecWithPrec(1, 1), sdk.NewDec(0)) + msg = staking.NewMsgCreateValidator(valOpAddr2, valConsPk2, + sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100)), staking.Description{}, commission, sdk.OneInt()) + require.True(t, sh(ctx, msg).IsOK()) + + // create third validator with 10% commission + commission = staking.NewCommissionMsg(sdk.NewDecWithPrec(1, 1), sdk.NewDecWithPrec(1, 1), sdk.NewDec(0)) + msg = staking.NewMsgCreateValidator(valOpAddr3, valConsPk3, + sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100)), staking.Description{}, commission, sdk.OneInt()) + require.True(t, sh(ctx, msg).IsOK()) + + abciValA := abci.Validator{ + Address: valConsPk1.Address(), + Power: 11, + } + abciValB := abci.Validator{ + Address: valConsPk2.Address(), + Power: 10, + } + abciValС := abci.Validator{ + Address: valConsPk3.Address(), + Power: 10, + } + + // assert initial state: zero outstanding rewards, zero community pool, zero commission, zero current rewards + require.True(t, k.GetOutstandingRewards(ctx).IsZero()) + require.True(t, k.GetFeePool(ctx).CommunityPool.IsZero()) + require.True(t, k.GetValidatorAccumulatedCommission(ctx, valOpAddr1).IsZero()) + require.True(t, k.GetValidatorAccumulatedCommission(ctx, valOpAddr2).IsZero()) + require.True(t, k.GetValidatorCurrentRewards(ctx, valOpAddr1).Rewards.IsZero()) + require.True(t, k.GetValidatorCurrentRewards(ctx, valOpAddr2).Rewards.IsZero()) + + // allocate tokens as if both had voted and second was proposer + fees := sdk.Coins{ + sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(634195840)), + } + fck.SetCollectedFees(fees) + votes := []abci.VoteInfo{ + { + Validator: abciValA, + SignedLastBlock: true, + }, + { + Validator: abciValB, + SignedLastBlock: true, + }, + { + Validator: abciValС, + SignedLastBlock: true, + }, + } + k.AllocateTokens(ctx, 31, 31, valConsAddr2, votes) + + require.True(t, k.GetOutstandingRewards(ctx).IsValid()) +}