From 3d440de123e416f111d7bdd3126b421d1c4f391d Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Fri, 16 Feb 2024 00:06:35 -0600 Subject: [PATCH] feat: distribute every 50 blocks (#527) * distribute every 10 blocks * total power logic in loop * distribute on block height 2 as well * set distribution to each block for tests * remove special case for test * add test displaying delayed logic * add clarifying comments, change to 20 * remove block height 2 hack * comment * Update allocation_test.go * test fixes, up to 50 blocks --- tests/e2e/distribution/suite.go | 5 ++ .../distribution/keeper/allocation_test.go | 74 +++++++++++++++++-- .../distribution/keeper/msg_server_test.go | 9 ++- x/distribution/abci.go | 19 +++-- 4 files changed, 93 insertions(+), 14 deletions(-) diff --git a/tests/e2e/distribution/suite.go b/tests/e2e/distribution/suite.go index 00b7884ab879..a7b8b50f195c 100644 --- a/tests/e2e/distribution/suite.go +++ b/tests/e2e/distribution/suite.go @@ -15,6 +15,7 @@ import ( clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli" "github.com/cosmos/cosmos-sdk/testutil/network" sdk "github.com/cosmos/cosmos-sdk/types" + distr "github.com/cosmos/cosmos-sdk/x/distribution" "github.com/cosmos/cosmos-sdk/x/distribution/client/cli" distrclitestutil "github.com/cosmos/cosmos-sdk/x/distribution/client/testutil" distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types" @@ -39,6 +40,10 @@ func NewE2ETestSuite(cfg network.Config) *E2ETestSuite { func (s *E2ETestSuite) SetupSuite() { s.T().Log("setting up e2e test suite") + // We distribute rewards every block here since we test the delayed distribution + // in another test. + distr.BlockMultipleToDistributeRewards = 1 + cfg := network.DefaultConfig(simapp.NewTestNetworkFixture) cfg.NumValidators = 1 s.cfg = cfg diff --git a/tests/integration/distribution/keeper/allocation_test.go b/tests/integration/distribution/keeper/allocation_test.go index 2090c17174e6..37c14d645044 100644 --- a/tests/integration/distribution/keeper/allocation_test.go +++ b/tests/integration/distribution/keeper/allocation_test.go @@ -14,6 +14,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/auth/types" bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper" banktestutil "github.com/cosmos/cosmos-sdk/x/bank/testutil" + dist "github.com/cosmos/cosmos-sdk/x/distribution" "github.com/cosmos/cosmos-sdk/x/distribution/keeper" "github.com/cosmos/cosmos-sdk/x/distribution/testutil" disttypes "github.com/cosmos/cosmos-sdk/x/distribution/types" @@ -71,6 +72,10 @@ func TestAllocateTokensToManyValidators(t *testing.T) { stakingKeeper *stakingkeeper.Keeper ) + // set distribute to every block for first test. + // we test the delayed distribution in the next test. + dist.BlockMultipleToDistributeRewards = 1 + app, err := simtestutil.Setup(testutil.AppConfig, &accountKeeper, &bankKeeper, @@ -137,23 +142,80 @@ func TestAllocateTokensToManyValidators(t *testing.T) { distrKeeper.AllocateTokens(ctx, 200, votes) // 98 outstanding rewards (100 less 2 to community pool) - require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: sdk.NewDecWithPrec(490, 1)}}, distrKeeper.GetValidatorOutstandingRewards(ctx, valAddrs[0]).Rewards) - require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: sdk.NewDecWithPrec(490, 1)}}, distrKeeper.GetValidatorOutstandingRewards(ctx, valAddrs[1]).Rewards) + firstValidator0OutstandingRewards := distrKeeper.GetValidatorOutstandingRewards(ctx, valAddrs[0]).Rewards + firstValidator1OutstandingRewards := distrKeeper.GetValidatorOutstandingRewards(ctx, valAddrs[1]).Rewards + require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: sdk.NewDecWithPrec(490, 1)}}, firstValidator0OutstandingRewards) + require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: sdk.NewDecWithPrec(490, 1)}}, firstValidator1OutstandingRewards) // 2 community pool coins require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: math.LegacyNewDec(2)}}, distrKeeper.GetFeePool(ctx).CommunityPool) // 50% commission for first proposer, (0.5 * 98%) * 100 / 2 = 23.25 - require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: sdk.NewDecWithPrec(2450, 2)}}, distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddrs[0]).Commission) + firstValidator0Commission := distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddrs[0]).Commission + require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: sdk.NewDecWithPrec(2450, 2)}}, firstValidator0Commission) // zero commission for second proposer - require.True(t, distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddrs[1]).Commission.IsZero()) + firstValidator1Commission := distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddrs[1]).Commission + require.True(t, firstValidator1Commission.IsZero()) // just staking.proportional for first proposer less commission = (0.5 * 98%) * 100 / 2 = 24.50 - require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: sdk.NewDecWithPrec(2450, 2)}}, distrKeeper.GetValidatorCurrentRewards(ctx, valAddrs[0]).Rewards) + firstValidator0CurrentRewards := distrKeeper.GetValidatorCurrentRewards(ctx, valAddrs[0]).Rewards + require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: sdk.NewDecWithPrec(2450, 2)}}, firstValidator0CurrentRewards) // proposer reward + staking.proportional for second proposer = (0.5 * (98%)) * 100 = 49 - require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: sdk.NewDecWithPrec(490, 1)}}, distrKeeper.GetValidatorCurrentRewards(ctx, valAddrs[1]).Rewards) + firstValidator1CurrentRewards := distrKeeper.GetValidatorCurrentRewards(ctx, valAddrs[1]).Rewards + require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: sdk.NewDecWithPrec(490, 1)}}, firstValidator1CurrentRewards) + + // test that the block height triggers the distribution + dist.BlockMultipleToDistributeRewards = 50 + + // block height is not a multiple, should not trigger allocation (no change in rewards) + ctx = ctx.WithBlockHeight(dist.BlockMultipleToDistributeRewards - 1) + app.BeginBlocker(ctx, abci.RequestBeginBlock{Header: tmproto.Header{ProposerAddress: valAddrs[0].Bytes()}, + LastCommitInfo: abci.CommitInfo{ + Votes: votes, + }, + }) + require.Equal(t, firstValidator0OutstandingRewards, distrKeeper.GetValidatorOutstandingRewards(ctx, valAddrs[0]).Rewards) + require.Equal(t, firstValidator1OutstandingRewards, distrKeeper.GetValidatorOutstandingRewards(ctx, valAddrs[1]).Rewards) + require.Equal(t, firstValidator0Commission, distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddrs[0]).Commission) + require.Equal(t, firstValidator1Commission, distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddrs[1]).Commission) + require.Equal(t, firstValidator0CurrentRewards, distrKeeper.GetValidatorCurrentRewards(ctx, valAddrs[0]).Rewards) + require.Equal(t, firstValidator1CurrentRewards, distrKeeper.GetValidatorCurrentRewards(ctx, valAddrs[1]).Rewards) + + // block height is a multiple, should trigger allocation + ctx = ctx.WithBlockHeight(dist.BlockMultipleToDistributeRewards) + + feesCollectedInt := bankKeeper.GetAllBalances(ctx, feeCollector.GetAddress()) + + // feesCollected was increased from last BeginBlocker call, then will occur again from the new BeginBlocker call, + // so we need to double the feesCollected to simulate the new BeginBlocker call + feesCollectedInt[0].Amount = feesCollectedInt[0].Amount.MulRaw(2) + feesCollected := sdk.NewDecCoinsFromCoins(feesCollectedInt...) + + communityTax := distrKeeper.GetCommunityTax(ctx) + voteMultiplier := math.LegacyOneDec().Sub(communityTax) + feeMultiplier := feesCollected.MulDecTruncate(voteMultiplier) + powerFraction := math.LegacyNewDec(100).QuoTruncate(math.LegacyNewDec(200)) + + newRewards := feeMultiplier.MulDecTruncate(powerFraction) + pendingRewards := firstValidator0OutstandingRewards.Add(newRewards...) + + pendingCommission := firstValidator0OutstandingRewards.Add(newRewards...) + pendingCommission[0].Amount = pendingCommission[0].Amount.Quo(sdk.NewDec(2)) + + app.BeginBlocker(ctx, abci.RequestBeginBlock{Header: tmproto.Header{ProposerAddress: valAddrs[0].Bytes()}, + LastCommitInfo: abci.CommitInfo{ + Votes: votes, + }, + }) + + require.Equal(t, pendingRewards, distrKeeper.GetValidatorOutstandingRewards(ctx, valAddrs[0]).Rewards) + require.Equal(t, pendingRewards, distrKeeper.GetValidatorOutstandingRewards(ctx, valAddrs[1]).Rewards) + require.Equal(t, pendingCommission, distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddrs[0]).Commission) + require.True(t, distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddrs[1]).Commission.IsZero()) + require.Equal(t, pendingCommission, distrKeeper.GetValidatorCurrentRewards(ctx, valAddrs[0]).Rewards) + require.Equal(t, pendingRewards, distrKeeper.GetValidatorCurrentRewards(ctx, valAddrs[1]).Rewards) } func TestAllocateTokensTruncation(t *testing.T) { diff --git a/tests/integration/distribution/keeper/msg_server_test.go b/tests/integration/distribution/keeper/msg_server_test.go index 59862f6d5c50..00b544d6ad15 100644 --- a/tests/integration/distribution/keeper/msg_server_test.go +++ b/tests/integration/distribution/keeper/msg_server_test.go @@ -2,6 +2,7 @@ package keeper_test import ( sdk "github.com/cosmos/cosmos-sdk/types" + banktestutil "github.com/cosmos/cosmos-sdk/x/bank/testutil" "github.com/cosmos/cosmos-sdk/x/distribution/types" ) @@ -159,7 +160,13 @@ func (s *KeeperTestSuite) TestCommunityPoolSpend() { for _, tc := range testCases { tc := tc s.Run(tc.name, func() { - _, err := s.msgServer.CommunityPoolSpend(s.ctx, tc.input) + amount := sdk.NewCoins(sdk.NewInt64Coin("stake", 100)) + s.Require().NoError(banktestutil.FundAccount(s.bankKeeper, s.ctx, s.addrs[0], amount)) + + err := s.distrKeeper.FundCommunityPool(s.ctx, amount, s.addrs[0]) + s.Require().Nil(err) + + _, err = s.msgServer.CommunityPoolSpend(s.ctx, tc.input) if tc.expErr { s.Require().Error(err) diff --git a/x/distribution/abci.go b/x/distribution/abci.go index 320f3961feaa..2b21fe67b225 100644 --- a/x/distribution/abci.go +++ b/x/distribution/abci.go @@ -11,20 +11,25 @@ import ( "github.com/cosmos/cosmos-sdk/x/distribution/types" ) +var BlockMultipleToDistributeRewards = int64(50) + // BeginBlocker sets the proposer for determining distribution during endblock // and distribute rewards for the previous block. func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, k keeper.Keeper) { defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker) - // determine the total power signing the block - var previousTotalPower int64 - for _, voteInfo := range req.LastCommitInfo.GetVotes() { - previousTotalPower += voteInfo.Validator.Power - } - // TODO this is Tendermint-dependent // ref https://github.com/cosmos/cosmos-sdk/issues/3095 - if ctx.BlockHeight() > 1 { + blockHeight := ctx.BlockHeight() + // only allocate rewards if the block height is greater than 1 + // and for every multiple of 50 blocks for performance reasons. + if blockHeight > 1 && blockHeight%BlockMultipleToDistributeRewards == 0 { + // determine the total power signing the block + var previousTotalPower int64 + for _, voteInfo := range req.LastCommitInfo.GetVotes() { + previousTotalPower += voteInfo.Validator.Power + } + k.AllocateTokens(ctx, previousTotalPower, req.LastCommitInfo.GetVotes()) }