Merge PR #3728: Alternative negative coins fix (floor multiplication)

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

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)
 	}

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())
+}