Merge PR #3359: Round down when calculating rewards (fixes F1 sim bug)

Author: cwgoes

PENDING.md

@@ -30,6 +30,7 @@ BREAKING CHANGES
   * [\#1894](https://github.com/cosmos/cosmos-sdk/pull/1894) `version` command now shows latest commit, vendor dir hash, and build machine info.
 
 * SDK
+  * [distribution] \#3359 Always round down when calculating rewards-to-be-withdrawn in F1 fee distribution
   * [staking] \#2513 Validator power type from Dec -> Int
   * [staking] \#3233 key and value now contain duplicate fields to simplify code
   * [\#3064](https://github.com/cosmos/cosmos-sdk/issues/3064) Sanitize `sdk.Coin` denom. Coins denoms are now case insensitive, i.e. 100fooToken equals to 100FOOTOKEN.

types/dec_coin.go

@@ -201,6 +201,19 @@ func (coins DecCoins) MulDec(d Dec) DecCoins {
 	return res
 }
 
+// multiply all the coins by a decimal, truncating
+func (coins DecCoins) MulDecTruncate(d Dec) DecCoins {
+	res := make([]DecCoin, len(coins))
+	for i, coin := range coins {
+		product := DecCoin{
+			Denom:  coin.Denom,
+			Amount: coin.Amount.MulTruncate(d),
+		}
+		res[i] = product
+	}
+	return res
+}
+
 // divide all the coins by a decimal
 func (coins DecCoins) QuoDec(d Dec) DecCoins {
 	res := make([]DecCoin, len(coins))
@@ -214,6 +227,19 @@ func (coins DecCoins) QuoDec(d Dec) DecCoins {
 	return res
 }
 
+// divide all the coins by a decimal, truncating
+func (coins DecCoins) QuoDecTruncate(d Dec) DecCoins {
+	res := make([]DecCoin, len(coins))
+	for i, coin := range coins {
+		quotient := DecCoin{
+			Denom:  coin.Denom,
+			Amount: coin.Amount.QuoTruncate(d),
+		}
+		res[i] = quotient
+	}
+	return res
+}
+
 // returns the amount of a denom from deccoins
 func (coins DecCoins) AmountOf(denom string) Dec {
 	switch len(coins) {

types/decimal.go

@@ -228,6 +228,17 @@ func (d Dec) Mul(d2 Dec) Dec {
 	return Dec{chopped}
 }
 
+// multiplication truncate
+func (d Dec) MulTruncate(d2 Dec) Dec {
+	mul := new(big.Int).Mul(d.Int, d2.Int)
+	chopped := chopPrecisionAndTruncate(mul)
+
+	if chopped.BitLen() > 255+DecimalPrecisionBits {
+		panic("Int overflow")
+	}
+	return Dec{chopped}
+}
+
 // multiplication
 func (d Dec) MulInt(i Int) Dec {
 	mul := new(big.Int).Mul(d.Int, i.i)
@@ -254,6 +265,22 @@ func (d Dec) Quo(d2 Dec) Dec {
 	return Dec{chopped}
 }
 
+// quotient truncate
+func (d Dec) QuoTruncate(d2 Dec) Dec {
+
+	// multiply precision twice
+	mul := new(big.Int).Mul(d.Int, precisionReuse)
+	mul.Mul(mul, precisionReuse)
+
+	quo := new(big.Int).Quo(mul, d2.Int)
+	chopped := chopPrecisionAndTruncate(quo)
+
+	if chopped.BitLen() > 255+DecimalPrecisionBits {
+		panic("Int overflow")
+	}
+	return Dec{chopped}
+}
+
 // quotient
 func (d Dec) QuoInt(i Int) Dec {
 	mul := new(big.Int).Quo(d.Int, i.i)

x/distribution/keeper/delegation.go

@@ -16,13 +16,14 @@ func (k Keeper) initializeDelegation(ctx sdk.Context, val sdk.ValAddress, del sd
 
 	// calculate delegation stake in tokens
 	// we don't store directly, so multiply delegation shares * (tokens per share)
-	stake := delegation.GetShares().Mul(validator.GetDelegatorShareExRate())
+	// note: necessary to truncate so we don't allow withdrawing more rewards than owed
+	stake := delegation.GetShares().MulTruncate(validator.GetDelegatorShareExRate())
 	k.SetDelegatorStartingInfo(ctx, val, del, types.NewDelegatorStartingInfo(previousPeriod, stake, uint64(ctx.BlockHeight())))
 }
 
 // calculate the rewards accrued by a delegation between two periods
 func (k Keeper) calculateDelegationRewardsBetween(ctx sdk.Context, val sdk.Validator,
-	startingPeriod, endingPeriod uint64, staking sdk.Dec) (rewards sdk.DecCoins) {
+	startingPeriod, endingPeriod uint64, stake sdk.Dec) (rewards sdk.DecCoins) {
 	// sanity check
 	if startingPeriod > endingPeriod {
 		panic("startingPeriod cannot be greater than endingPeriod")
@@ -32,7 +33,8 @@ func (k Keeper) calculateDelegationRewardsBetween(ctx sdk.Context, val sdk.Valid
 	starting := k.GetValidatorHistoricalRewards(ctx, val.GetOperator(), startingPeriod)
 	ending := k.GetValidatorHistoricalRewards(ctx, val.GetOperator(), endingPeriod)
 	difference := ending.CumulativeRewardRatio.Minus(starting.CumulativeRewardRatio)
-	rewards = difference.MulDec(staking)
+	// note: necessary to truncate so we don't allow withdrawing more rewards than owed
+	rewards = difference.MulDecTruncate(stake)
 	return
 }
 
@@ -54,7 +56,8 @@ func (k Keeper) calculateDelegationRewards(ctx sdk.Context, val sdk.Validator, d
 			func(height uint64, event types.ValidatorSlashEvent) (stop bool) {
 				endingPeriod := event.ValidatorPeriod
 				rewards = rewards.Plus(k.calculateDelegationRewardsBetween(ctx, val, startingPeriod, endingPeriod, stake))
-				stake = stake.Mul(sdk.OneDec().Sub(event.Fraction))
+				// note: necessary to truncate so we don't allow withdrawing more rewards than owed
+				stake = stake.MulTruncate(sdk.OneDec().Sub(event.Fraction))
 				startingPeriod = endingPeriod
 				return false
 			},

x/distribution/keeper/validator.go

@@ -38,7 +38,8 @@ func (k Keeper) incrementValidatorPeriod(ctx sdk.Context, val sdk.Validator) uin
 
 		current = sdk.DecCoins{}
 	} else {
-		current = rewards.Rewards.QuoDec(sdk.NewDecFromInt(val.GetTokens()))
+		// note: necessary to truncate so we don't allow withdrawing more rewards than owed
+		current = rewards.Rewards.QuoDecTruncate(sdk.NewDecFromInt(val.GetTokens()))
 	}
 
 	// fetch historical rewards for last period