Skip to content

Commit

Permalink
refactor!: Remove proposer-based rewards (#12876)
Browse files Browse the repository at this point in the history
  • Loading branch information
alexanderbez committed Aug 16, 2022
1 parent fabf739 commit eef086f
Show file tree
Hide file tree
Showing 14 changed files with 325 additions and 366 deletions.
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())
}

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

0 comments on commit eef086f

Please sign in to comment.