diff --git a/types/decimal.go b/types/decimal.go index d840bc85988e..5b6a6f4b2f80 100644 --- 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 index 1a53b19bc0f1..119622edd211 100644 --- 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 index 68c822b9e03e..c5d9f360ad65 100644 --- 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 index 8d3ef0ea112c..28ad412486f2 100644 --- 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