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

refactor!: Remove proposer-based rewards #12876

Merged
merged 13 commits into from
Aug 16, 2022
327 changes: 167 additions & 160 deletions api/cosmos/distribution/v1beta1/distribution.pulsar.go

Large diffs are not rendered by default.

57 changes: 39 additions & 18 deletions proto/cosmos/distribution/v1beta1/distribution.proto
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
syntax = "proto3";
package cosmos.distribution.v1beta1;

option go_package = "github.com/cosmos/cosmos-sdk/x/distribution/types";
option go_package = "github.com/cosmos/cosmos-sdk/x/distribution/types";
option (gogoproto.equal_all) = true;

import "gogoproto/gogo.proto";
Expand All @@ -16,16 +16,22 @@ message Params {
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec",
(gogoproto.nullable) = false
];

// The base_proposer_reward and bonus_proposer_reward fields are deprecated
// and are no longer used in the x/distribution module's reward mechanism.
string base_proposer_reward = 2 [
(cosmos_proto.scalar) = "cosmos.Dec",
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec",
(gogoproto.nullable) = false
(gogoproto.nullable) = false,
deprecated = true
];
string bonus_proposer_reward = 3 [
(cosmos_proto.scalar) = "cosmos.Dec",
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec",
(gogoproto.nullable) = false
(gogoproto.nullable) = false,
deprecated = true
];

bool withdraw_addr_enabled = 4;
}

Expand All @@ -42,32 +48,40 @@ message Params {
// read that record)
// + one per validator for the zeroeth period, set on initialization
message ValidatorHistoricalRewards {
repeated cosmos.base.v1beta1.DecCoin cumulative_reward_ratio = 1
[(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.DecCoins", (gogoproto.nullable) = false];
repeated cosmos.base.v1beta1.DecCoin cumulative_reward_ratio = 1 [
(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.DecCoins",
(gogoproto.nullable) = false
];
uint32 reference_count = 2;
}

// ValidatorCurrentRewards represents current rewards and current
// period for a validator kept as a running counter and incremented
// each block as long as the validator's tokens remain constant.
message ValidatorCurrentRewards {
repeated cosmos.base.v1beta1.DecCoin rewards = 1
[(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.DecCoins", (gogoproto.nullable) = false];
repeated cosmos.base.v1beta1.DecCoin rewards = 1 [
(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.DecCoins",
(gogoproto.nullable) = false
];
uint64 period = 2;
}

// ValidatorAccumulatedCommission represents accumulated commission
// for a validator kept as a running counter, can be withdrawn at any time.
message ValidatorAccumulatedCommission {
repeated cosmos.base.v1beta1.DecCoin commission = 1
[(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.DecCoins", (gogoproto.nullable) = false];
repeated cosmos.base.v1beta1.DecCoin commission = 1 [
(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.DecCoins",
(gogoproto.nullable) = false
];
}

// ValidatorOutstandingRewards represents outstanding (un-withdrawn) rewards
// for a validator inexpensive to track, allows simple sanity checks.
message ValidatorOutstandingRewards {
repeated cosmos.base.v1beta1.DecCoin rewards = 1
[(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.DecCoins", (gogoproto.nullable) = false];
repeated cosmos.base.v1beta1.DecCoin rewards = 1 [
(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.DecCoins",
(gogoproto.nullable) = false
];
}

// ValidatorSlashEvent represents a validator slash event.
Expand All @@ -86,13 +100,16 @@ message ValidatorSlashEvent {
// ValidatorSlashEvents is a collection of ValidatorSlashEvent messages.
message ValidatorSlashEvents {
option (gogoproto.goproto_stringer) = false;
repeated ValidatorSlashEvent validator_slash_events = 1 [(gogoproto.nullable) = false];
repeated ValidatorSlashEvent validator_slash_events = 1
[(gogoproto.nullable) = false];
}

// FeePool is the global fee pool for distribution.
message FeePool {
repeated cosmos.base.v1beta1.DecCoin community_pool = 1
[(gogoproto.nullable) = false, (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.DecCoins"];
repeated cosmos.base.v1beta1.DecCoin community_pool = 1 [
(gogoproto.nullable) = false,
(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.DecCoins"
];
}

// CommunityPoolSpendProposal details a proposal for use of community funds,
Expand All @@ -107,8 +124,10 @@ message CommunityPoolSpendProposal {
string title = 1;
string description = 2;
string recipient = 3;
repeated cosmos.base.v1beta1.Coin amount = 4
[(gogoproto.nullable) = false, (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins"];
repeated cosmos.base.v1beta1.Coin amount = 4 [
(gogoproto.nullable) = false,
(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins"
];
}

// DelegatorStartingInfo represents the starting info for a delegator reward
Expand All @@ -135,8 +154,10 @@ message DelegationDelegatorReward {

string validator_address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];

repeated cosmos.base.v1beta1.DecCoin reward = 2
[(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.DecCoins", (gogoproto.nullable) = false];
repeated cosmos.base.v1beta1.DecCoin reward = 2 [
(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.DecCoins",
(gogoproto.nullable) = false
];
}

// CommunityPoolSpendProposalWithDeposit defines a CommunityPoolSpendProposal
Expand Down
10 changes: 3 additions & 7 deletions x/distribution/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,20 @@ import (
)

// BeginBlocker sets the proposer for determining distribution during endblock
// and distribute rewards for the previous block
// 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, sumPreviousPrecommitPower int64
var previousTotalPower int64
for _, voteInfo := range req.LastCommitInfo.GetVotes() {
previousTotalPower += voteInfo.Validator.Power
if voteInfo.SignedLastBlock {
sumPreviousPrecommitPower += voteInfo.Validator.Power
}
}

// TODO this is Tendermint-dependent
// ref https://github.com/cosmos/cosmos-sdk/issues/3095
if ctx.BlockHeight() > 1 {
previousProposer := k.GetPreviousProposerConsAddr(ctx)
k.AllocateTokens(ctx, sumPreviousPrecommitPower, previousTotalPower, previousProposer, req.LastCommitInfo.GetVotes())
k.AllocateTokens(ctx, previousTotalPower, req.LastCommitInfo.GetVotes())

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
}

// record the proposer for when we payout on the next block
Expand Down
18 changes: 14 additions & 4 deletions x/distribution/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ import (
"testing"

"cosmossdk.io/math"
"github.com/stretchr/testify/require"
abci "github.com/tendermint/tendermint/abci/types"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"

"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
Expand All @@ -14,9 +18,6 @@ import (
stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
"github.com/cosmos/cosmos-sdk/x/staking/teststaking"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
"github.com/stretchr/testify/require"
abci "github.com/tendermint/tendermint/abci/types"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
)

const (
Expand Down Expand Up @@ -116,9 +117,18 @@ func TestVerifyProposerRewardAssignement(t *testing.T) {
})
require.NotEmpty(t, app.Commit())

// Note, we used to ensure that the lazy validator's rewards are less than the
// non-lazy validator, given the assumption that proposer-based rewards are
// used. However, since proposer-based rewards are no longer used, the rewards
// should be the same.
//
// We keep this (modified) assertion in case we'd like to augment the test
// in the future.
//
// Ref: https://github.com/cosmos/cosmos-sdk/pull/12876
rewardsValidatorBeforeLazyValidator := distrKeeper.GetValidatorOutstandingRewardsCoins(ctx, validators[lazyValidatorIdx+1].addr)
rewardsLazyValidator := distrKeeper.GetValidatorOutstandingRewardsCoins(ctx, validators[lazyValidatorIdx].addr)
rewardsValidatorAfterLazyValidator := distrKeeper.GetValidatorOutstandingRewardsCoins(ctx, validators[lazyValidatorIdx+1].addr)
require.True(t, rewardsLazyValidator[0].Amount.LT(rewardsValidatorAfterLazyValidator[0].Amount))
require.True(t, rewardsLazyValidator[0].Amount.Equal(rewardsValidatorAfterLazyValidator[0].Amount))
require.Equal(t, rewardsValidatorBeforeLazyValidator, rewardsValidatorAfterLazyValidator)
}
54 changes: 5 additions & 49 deletions x/distribution/keeper/allocation.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package keeper

import (
"fmt"

"cosmossdk.io/math"
abci "github.com/tendermint/tendermint/abci/types"

Expand All @@ -11,15 +9,9 @@ import (
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
)

// AllocateTokens handles distribution of the collected fees
// bondedVotes is a list of (validator address, validator voted on last block flag) for all
// validators in the bonded set.
func (k Keeper) AllocateTokens(
ctx sdk.Context, sumPreviousPrecommitPower, totalPreviousPower int64,
previousProposer sdk.ConsAddress, bondedVotes []abci.VoteInfo,
) {
logger := k.Logger(ctx)

// AllocateTokens performs reward and fee distribution to all validators based
// on the F1 fee distribution specification.
func (k Keeper) AllocateTokens(ctx sdk.Context, totalPreviousPower int64, bondedVotes []abci.VoteInfo) {
// fetch and clear the collected fees for distribution, since this is
// called in BeginBlock, collected fees will be from the previous block
// (and distributed to the previous proposer)
Expand All @@ -42,46 +34,10 @@ func (k Keeper) AllocateTokens(
return
}

// calculate fraction votes
previousFractionVotes := math.LegacyNewDec(sumPreviousPrecommitPower).Quo(math.LegacyNewDec(totalPreviousPower))

// calculate previous proposer reward
baseProposerReward := k.GetBaseProposerReward(ctx)
bonusProposerReward := k.GetBonusProposerReward(ctx)
proposerMultiplier := baseProposerReward.Add(bonusProposerReward.MulTruncate(previousFractionVotes))
proposerReward := feesCollected.MulDecTruncate(proposerMultiplier)

// pay previous proposer
remaining := feesCollected
proposerValidator := k.stakingKeeper.ValidatorByConsAddr(ctx, previousProposer)

if proposerValidator != nil {
ctx.EventManager().EmitEvent(
sdk.NewEvent(
types.EventTypeProposerReward,
sdk.NewAttribute(sdk.AttributeKeyAmount, proposerReward.String()),
sdk.NewAttribute(types.AttributeKeyValidator, proposerValidator.GetOperator().String()),
),
)

k.AllocateTokensToValidator(ctx, proposerValidator, proposerReward)
remaining = remaining.Sub(proposerReward)
} else {
// previous proposer can be unknown if say, the unbonding period is 1 block, so
// e.g. a validator undelegates at block X, it's removed entirely by
// block X+1's endblock, then X+2 we need to refer to the previous
// proposer for X+1, but we've forgotten about them.
logger.Error(fmt.Sprintf(
"WARNING: Attempt to allocate proposer rewards to unknown proposer %s. "+
"This should happen only if the proposer unbonded completely within a single block, "+
"which generally should not happen except in exceptional circumstances (or fuzz testing). "+
"We recommend you investigate immediately.",
previousProposer.String()))
}

// calculate fraction allocated to validators
remaining := feesCollected
communityTax := k.GetCommunityTax(ctx)
voteMultiplier := math.LegacyOneDec().Sub(proposerMultiplier).Sub(communityTax)
voteMultiplier := math.LegacyOneDec().Sub(communityTax)

// allocate tokens proportionally to voting power
// TODO consider parallelizing later, ref https://github.com/cosmos/cosmos-sdk/pull/3099#discussion_r246276376
Expand Down
25 changes: 15 additions & 10 deletions x/distribution/keeper/allocation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,21 +134,26 @@ func TestAllocateTokensToManyValidators(t *testing.T) {
SignedLastBlock: true,
},
}
distrKeeper.AllocateTokens(ctx, 200, 200, valConsAddr2, votes)
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(465, 1)}}, distrKeeper.GetValidatorOutstandingRewards(ctx, valAddrs[0]).Rewards)
require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: sdk.NewDecWithPrec(515, 1)}}, distrKeeper.GetValidatorOutstandingRewards(ctx, valAddrs[1]).Rewards)
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)

// 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 * 93%) * 100 / 2 = 23.25
require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: sdk.NewDecWithPrec(2325, 2)}}, distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddrs[0]).Commission)

// 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)

// zero commission for second proposer
require.True(t, distrKeeper.GetValidatorAccumulatedCommission(ctx, valAddrs[1]).Commission.IsZero())
// just staking.proportional for first proposer less commission = (0.5 * 93%) * 100 / 2 = 23.25
require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: sdk.NewDecWithPrec(2325, 2)}}, distrKeeper.GetValidatorCurrentRewards(ctx, valAddrs[0]).Rewards)
// proposer reward + staking.proportional for second proposer = (5 % + 0.5 * (93%)) * 100 = 51.5
require.Equal(t, sdk.DecCoins{{Denom: sdk.DefaultBondDenom, Amount: sdk.NewDecWithPrec(515, 1)}}, distrKeeper.GetValidatorCurrentRewards(ctx, valAddrs[1]).Rewards)

// 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)

// 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)
}

func TestAllocateTokensTruncation(t *testing.T) {
Expand Down Expand Up @@ -235,7 +240,7 @@ func TestAllocateTokensTruncation(t *testing.T) {
SignedLastBlock: true,
},
}
distrKeeper.AllocateTokens(ctx, 31, 31, sdk.ConsAddress(valConsPk2.Address()), votes)
distrKeeper.AllocateTokens(ctx, 31, votes)

require.True(t, distrKeeper.GetValidatorOutstandingRewards(ctx, valAddrs[0]).Rewards.IsValid())
require.True(t, distrKeeper.GetValidatorOutstandingRewards(ctx, valAddrs[1]).Rewards.IsValid())
Expand Down
12 changes: 1 addition & 11 deletions x/distribution/keeper/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package keeper

import (
"cosmossdk.io/math"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/distribution/types"
)
Expand Down Expand Up @@ -39,17 +40,6 @@ func (k Keeper) GetCommunityTax(ctx sdk.Context) math.LegacyDec {
return k.GetParams(ctx).CommunityTax
}

// GetBaseProposerReward returns the current distribution base proposer rate.
func (k Keeper) GetBaseProposerReward(ctx sdk.Context) math.LegacyDec {
return k.GetParams(ctx).BaseProposerReward
}

// GetBonusProposerReward returns the current distribution bonus proposer reward
// rate.
func (k Keeper) GetBonusProposerReward(ctx sdk.Context) (percent sdk.Dec) {
return k.GetParams(ctx).BonusProposerReward
}

// GetWithdrawAddrEnabled returns the current distribution withdraw address
// enabled parameter.
func (k Keeper) GetWithdrawAddrEnabled(ctx sdk.Context) (enabled bool) {
Expand Down
2 changes: 1 addition & 1 deletion x/distribution/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (
)

// ConsensusVersion defines the current x/distribution module consensus version.
const ConsensusVersion = 3
const ConsensusVersion = 4

var (
_ module.AppModule = AppModule{}
Expand Down
14 changes: 1 addition & 13 deletions x/distribution/simulation/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ import (
)

const (
keyCommunityTax = "communitytax"
keyBaseProposerReward = "baseproposerreward"
keyBonusProposerReward = "bonusproposerreward"
keyCommunityTax = "communitytax"
)

// ParamChanges defines the parameters that can be modified by param change proposals
Expand All @@ -27,15 +25,5 @@ func ParamChanges(r *rand.Rand) []simtypes.ParamChange {
return fmt.Sprintf("\"%s\"", GenCommunityTax(r))
},
),
simulation.NewSimParamChange(types.ModuleName, keyBaseProposerReward,
func(r *rand.Rand) string {
return fmt.Sprintf("\"%s\"", GenBaseProposerReward(r))
},
),
simulation.NewSimParamChange(types.ModuleName, keyBonusProposerReward,
func(r *rand.Rand) string {
return fmt.Sprintf("\"%s\"", GenBonusProposerReward(r))
},
),
}
}
4 changes: 1 addition & 3 deletions x/distribution/simulation/params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,11 @@ func TestParamChanges(t *testing.T) {
subspace string
}{
{"distribution/communitytax", "communitytax", "\"0.120000000000000000\"", "distribution"},
{"distribution/baseproposerreward", "baseproposerreward", "\"0.280000000000000000\"", "distribution"},
{"distribution/bonusproposerreward", "bonusproposerreward", "\"0.180000000000000000\"", "distribution"},
}

paramChanges := simulation.ParamChanges(r)

require.Len(t, paramChanges, 3)
require.Len(t, paramChanges, 1)

for i, p := range paramChanges {
require.Equal(t, expected[i].composedKey, p.ComposedKey())
Expand Down
Loading