Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

R4R: Alternative negative coins fix (floor multiplication) #3728

Merged
merged 7 commits into from
Feb 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions x/distribution/keeper/allocation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
Expand Down
71 changes: 71 additions & 0 deletions x/distribution/keeper/allocation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}