Merge pull request #2895 from cosmos/jae/blockly-mint

Clip withdrawal tokens
cosmos · Nov 25, 2018 · 5ddd549 · 5ddd549
2 parents 4959289 + 50e119a
commit 5ddd549
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 15 deletions.
diff --git a/types/decimal.go b/types/decimal.go
@@ -263,6 +263,7 @@ func (d Dec) IsInteger() bool {
 	return new(big.Int).Rem(d.Int, precisionReuse).Sign() == 0
 }
 
+// format decimal state
 func (d Dec) Format(s fmt.State, verb rune) {
 	s.Write([]byte(d.String()))
 }

diff --git a/x/distribution/keeper/validator.go b/x/distribution/keeper/validator.go
@@ -74,10 +74,6 @@ func (k Keeper) WithdrawValidatorRewardsAll(ctx sdk.Context, operatorAddr sdk.Va
 	accAddr := sdk.AccAddress(operatorAddr.Bytes())
 	withdraw := k.withdrawDelegationRewardsAll(ctx, accAddr)
 
-	//if withdraw.AmountOf {
-	//return types.ErrNoValidatorDistInfo(k.codespace)
-	//}
-
 	// withdrawal validator commission rewards
 	valInfo := k.GetValidatorDistInfo(ctx, operatorAddr)
 	wc := k.GetWithdrawContext(ctx, operatorAddr)

diff --git a/x/distribution/types/dec_coin.go b/x/distribution/types/dec_coin.go
@@ -20,6 +20,13 @@ func NewDecCoin(denom string, amount int64) DecCoin {
 	}
 }
 
+func NewDecCoinFromDec(denom string, amount sdk.Dec) DecCoin {
+	return DecCoin{
+		Denom:  denom,
+		Amount: amount,
+	}
+}
+
 func NewDecCoinFromCoin(coin sdk.Coin) DecCoin {
 	return DecCoin{
 		Denom:  coin.Denom,

diff --git a/x/distribution/types/delegator_info.go b/x/distribution/types/delegator_info.go
@@ -49,12 +49,8 @@ func (di DelegationDistInfo) WithdrawRewards(wc WithdrawContext, vi ValidatorDis
 	fp := wc.FeePool
 	vi = vi.UpdateTotalDelAccum(wc.Height, totalDelShares)
 
+	// Break out to prevent a divide by zero.
 	if vi.DelAccum.Accum.IsZero() {
-		// NOTE: Under the situation which a delegation was created with
-		//       zero shares, this next line of code would ensure
-		//       that no delegation-accum invariance relative to the total
-		//       delegation accum are kept by validator_info. Ideally however
-		//       delegations with zero shares should never be created.
 		di.DelPoolWithdrawalHeight = wc.Height
 		return di, vi, fp, DecCoins{}
 	}
@@ -64,12 +60,18 @@ func (di DelegationDistInfo) WithdrawRewards(wc WithdrawContext, vi ValidatorDis
 	accum := di.GetDelAccum(wc.Height, delegatorShares)
 	di.DelPoolWithdrawalHeight = wc.Height
 
-	var withdrawalTokens DecCoins
-	if accum.Equal(vi.DelAccum.Accum) {
-		// required due to rounding faults
-		withdrawalTokens = vi.DelPool
-	} else {
-		withdrawalTokens = vi.DelPool.MulDec(accum).QuoDec(vi.DelAccum.Accum)
+	withdrawalTokens := vi.DelPool.MulDec(accum).QuoDec(vi.DelAccum.Accum)
+
+	// Clip withdrawal tokens by pool, due to possible rounding errors.
+	// This rounding error may be introduced upon multiplication since
+	// we're clipping decimal digits, and then when we divide by a number ~1 or
+	// < 1, the error doesn't get "buried", and if << 1 it'll get amplified.
+	// more: https://github.com/cosmos/cosmos-sdk/issues/2888#issuecomment-441387987
+	for i, decCoin := range withdrawalTokens {
+		poolDenomAmount := vi.DelPool.AmountOf(decCoin.Denom)
+		if decCoin.Amount.GT(poolDenomAmount) {
+			withdrawalTokens[i] = NewDecCoinFromDec(decCoin.Denom, poolDenomAmount)
+		}
 	}
 
 	// defensive check for impossible accum ratios