From 32e0f14de2aae7eb613025e13b53b32dde0ba33f Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Mon, 15 Oct 2018 22:09:13 -0400 Subject: [PATCH] rework to correctly use previous vote info --- cmd/gaia/app/app.go | 7 ++--- x/distribution/abci_app.go | 28 ++++++++--------- x/distribution/keeper/allocation.go | 6 ++-- x/distribution/keeper/allocation_test.go | 12 ++----- x/distribution/keeper/delegation_test.go | 24 ++++---------- x/distribution/keeper/keeper.go | 40 +++++------------------- x/distribution/keeper/keeper_test.go | 15 ++------- x/distribution/keeper/key.go | 5 +-- x/distribution/keeper/test_common.go | 4 +-- x/distribution/keeper/validator_test.go | 20 +++--------- 10 files changed, 46 insertions(+), 115 deletions(-) diff --git a/cmd/gaia/app/app.go b/cmd/gaia/app/app.go index d388081d88..2c72e3aaf9 100644 --- a/cmd/gaia/app/app.go +++ b/cmd/gaia/app/app.go @@ -110,7 +110,7 @@ func NewGaiaApp(logger log.Logger, db dbm.DB, traceStore io.Writer, baseAppOptio ) app.distrKeeper = distr.NewKeeper( app.cdc, - app.keyDistr, app.tkeyDistr, + app.keyDistr, app.paramsKeeper.Subspace(distr.DefaultParamspace), app.bankKeeper, app.stakeKeeper, app.feeCollectionKeeper, app.RegisterCodespace(stake.DefaultCodespace), @@ -178,6 +178,8 @@ func MakeCodec() *codec.Codec { // application updates every end block func (app *GaiaApp) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock { tags := slashing.BeginBlocker(ctx, req, app.slashingKeeper) + + // distribute rewards from previous block distr.BeginBlocker(ctx, req, app.distrKeeper) return abci.ResponseBeginBlock{ @@ -189,9 +191,6 @@ func (app *GaiaApp) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) ab // nolint: unparam func (app *GaiaApp) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock { - // distribute rewards - distr.EndBlocker(ctx, app.distrKeeper) - tags := gov.EndBlocker(ctx, app.govKeeper) validatorUpdates := stake.EndBlocker(ctx, app.stakeKeeper) diff --git a/x/distribution/abci_app.go b/x/distribution/abci_app.go index 2ac2c57c63..2bdcadb6a3 100644 --- a/x/distribution/abci_app.go +++ b/x/distribution/abci_app.go @@ -9,8 +9,19 @@ import ( // set the proposer for determining distribution during endblock func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, k keeper.Keeper) { + + if ctx.BlockHeight() > 1 { + previousPercentPrecommitVotes := getPreviousPercentPrecommitVotes(req) + previousProposer := k.GetPreviousProposerConsAddr(ctx) + k.AllocateFees(ctx, previousPercentPrecommitVotes, previousProposer) + } + consAddr := sdk.ConsAddress(req.Header.ProposerAddress) - k.SetProposerConsAddr(ctx, consAddr) + k.SetPreviousProposerConsAddr(ctx, consAddr) +} + +// percent precommit votes for the previous block +func getPreviousPercentPrecommitVotes(req abci.RequestBeginBlock) sdk.Dec { // determine the total number of signed power totalPower, sumPrecommitPower := int64(0), int64(0) @@ -22,18 +33,7 @@ func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, k keeper.Keeper) } if totalPower == 0 { - k.SetPercentPrecommitVotes(ctx, sdk.ZeroDec()) - return + return sdk.ZeroDec() } - - percentPrecommitVotes := sdk.NewDec(sumPrecommitPower).Quo(sdk.NewDec(totalPower)) - k.SetPercentPrecommitVotes(ctx, percentPrecommitVotes) -} - -// allocate fees -func EndBlocker(ctx sdk.Context, k keeper.Keeper) { - if ctx.BlockHeight() < 2 { - return - } - k.AllocateFees(ctx) + return sdk.NewDec(sumPrecommitPower).Quo(sdk.NewDec(totalPower)) } diff --git a/x/distribution/keeper/allocation.go b/x/distribution/keeper/allocation.go index bd083adebe..e6dd1c9696 100644 --- a/x/distribution/keeper/allocation.go +++ b/x/distribution/keeper/allocation.go @@ -8,12 +8,11 @@ import ( ) // Allocate fees handles distribution of the collected fees -func (k Keeper) AllocateFees(ctx sdk.Context) { +func (k Keeper) AllocateFees(ctx sdk.Context, percentVotes sdk.Dec, proposer sdk.ConsAddress) { ctx.Logger().With("module", "x/distribution").Error(fmt.Sprintf("allocation height: %v", ctx.BlockHeight())) // get the proposer of this block - proposerConsAddr := k.GetProposerConsAddr(ctx) - proposerValidator := k.stakeKeeper.ValidatorByConsAddr(ctx, proposerConsAddr) + proposerValidator := k.stakeKeeper.ValidatorByConsAddr(ctx, proposer) proposerDist := k.GetValidatorDistInfo(ctx, proposerValidator.GetOperator()) // get the fees which have been getting collected through all the @@ -22,7 +21,6 @@ func (k Keeper) AllocateFees(ctx sdk.Context) { feesCollectedDec := types.NewDecCoins(feesCollected) // allocated rewards to proposer - percentVotes := k.GetPercentPrecommitVotes(ctx) baseProposerReward := k.GetBaseProposerReward(ctx) bonusProposerReward := k.GetBonusProposerReward(ctx) proposerMultiplier := baseProposerReward.Add(bonusProposerReward.Mul(percentVotes)) diff --git a/x/distribution/keeper/allocation_test.go b/x/distribution/keeper/allocation_test.go index 453f16bca2..441739ebe4 100644 --- a/x/distribution/keeper/allocation_test.go +++ b/x/distribution/keeper/allocation_test.go @@ -41,9 +41,7 @@ func TestAllocateFeesBasic(t *testing.T) { feeInputs := sdk.NewInt(100) fck.SetCollectedFees(sdk.Coins{sdk.NewCoin(denom, feeInputs)}) require.Equal(t, feeInputs, fck.GetCollectedFees(ctx).AmountOf(denom)) - keeper.SetProposerConsAddr(ctx, valConsAddr1) - keeper.SetPercentPrecommitVotes(ctx, sdk.OneDec()) - keeper.AllocateFees(ctx) + keeper.AllocateFees(ctx, sdk.OneDec(), valConsAddr1) // verify that these fees have been received by the feePool percentProposer := sdk.NewDecWithPrec(5, 2) @@ -70,9 +68,7 @@ func TestAllocateFeesWithCommunityTax(t *testing.T) { // allocate 100 denom of fees feeInputs := sdk.NewInt(100) fck.SetCollectedFees(sdk.Coins{sdk.NewCoin(denom, feeInputs)}) - keeper.SetProposerConsAddr(ctx, valConsAddr1) - keeper.SetPercentPrecommitVotes(ctx, sdk.OneDec()) - keeper.AllocateFees(ctx) + keeper.AllocateFees(ctx, sdk.OneDec(), valConsAddr1) // verify that these fees have been received by the feePool feePool := keeper.GetFeePool(ctx) @@ -100,10 +96,8 @@ func TestAllocateFeesWithPartialPrecommitPower(t *testing.T) { // allocate 100 denom of fees feeInputs := sdk.NewInt(100) fck.SetCollectedFees(sdk.Coins{sdk.NewCoin(denom, feeInputs)}) - keeper.SetProposerConsAddr(ctx, valConsAddr1) percentPrecommitVotes := sdk.NewDecWithPrec(25, 2) - keeper.SetPercentPrecommitVotes(ctx, percentPrecommitVotes) - keeper.AllocateFees(ctx) + keeper.AllocateFees(ctx, percentPrecommitVotes, valConsAddr1) // verify that these fees have been received by the feePool feePool := keeper.GetFeePool(ctx) diff --git a/x/distribution/keeper/delegation_test.go b/x/distribution/keeper/delegation_test.go index 3eecfafdd2..8415c708a9 100644 --- a/x/distribution/keeper/delegation_test.go +++ b/x/distribution/keeper/delegation_test.go @@ -30,9 +30,7 @@ func TestWithdrawDelegationRewardBasic(t *testing.T) { feeInputs := sdk.NewInt(100) fck.SetCollectedFees(sdk.Coins{sdk.NewCoin(denom, feeInputs)}) require.Equal(t, feeInputs, fck.GetCollectedFees(ctx).AmountOf(denom)) - keeper.SetProposerConsAddr(ctx, valConsAddr1) - keeper.SetPercentPrecommitVotes(ctx, sdk.OneDec()) - keeper.AllocateFees(ctx) + keeper.AllocateFees(ctx, sdk.OneDec(), valConsAddr1) // withdraw delegation ctx = ctx.WithBlockHeight(1) @@ -66,9 +64,7 @@ func TestWithdrawDelegationRewardWithCommission(t *testing.T) { feeInputs := sdk.NewInt(100) fck.SetCollectedFees(sdk.Coins{sdk.NewCoin(denom, feeInputs)}) require.Equal(t, feeInputs, fck.GetCollectedFees(ctx).AmountOf(denom)) - keeper.SetProposerConsAddr(ctx, valConsAddr1) - keeper.SetPercentPrecommitVotes(ctx, sdk.OneDec()) - keeper.AllocateFees(ctx) + keeper.AllocateFees(ctx, sdk.OneDec(), valConsAddr1) // withdraw delegation ctx = ctx.WithBlockHeight(1) @@ -108,9 +104,7 @@ func TestWithdrawDelegationRewardTwoDelegators(t *testing.T) { feeInputs := sdk.NewInt(100) fck.SetCollectedFees(sdk.Coins{sdk.NewCoin(denom, feeInputs)}) require.Equal(t, feeInputs, fck.GetCollectedFees(ctx).AmountOf(denom)) - keeper.SetProposerConsAddr(ctx, valConsAddr1) - keeper.SetPercentPrecommitVotes(ctx, sdk.OneDec()) - keeper.AllocateFees(ctx) + keeper.AllocateFees(ctx, sdk.OneDec(), valConsAddr1) // delegator 1 withdraw delegation ctx = ctx.WithBlockHeight(1) @@ -152,9 +146,7 @@ func TestWithdrawDelegationRewardTwoDelegatorsUneven(t *testing.T) { feeInputs := sdk.NewInt(90) fck.SetCollectedFees(sdk.Coins{sdk.NewCoin(denom, feeInputs)}) require.Equal(t, feeInputs, fck.GetCollectedFees(ctx).AmountOf(denom)) - keeper.SetProposerConsAddr(ctx, valConsAddr1) - keeper.SetPercentPrecommitVotes(ctx, sdk.OneDec()) - keeper.AllocateFees(ctx) + keeper.AllocateFees(ctx, sdk.OneDec(), valConsAddr1) ctx = ctx.WithBlockHeight(1) // delegator 1 withdraw delegation early, delegator 2 just keeps it's accum @@ -168,9 +160,7 @@ func TestWithdrawDelegationRewardTwoDelegatorsUneven(t *testing.T) { feeInputs = sdk.NewInt(180) fck.SetCollectedFees(sdk.Coins{sdk.NewCoin(denom, feeInputs)}) require.Equal(t, feeInputs, fck.GetCollectedFees(ctx).AmountOf(denom)) - keeper.SetProposerConsAddr(ctx, valConsAddr1) - keeper.SetPercentPrecommitVotes(ctx, sdk.OneDec()) - keeper.AllocateFees(ctx) + keeper.AllocateFees(ctx, sdk.OneDec(), valConsAddr1) ctx = ctx.WithBlockHeight(2) // delegator 2 now withdraws everything it's entitled to @@ -238,9 +228,7 @@ func TestWithdrawDelegationRewardsAll(t *testing.T) { feeInputs := sdk.NewInt(1000) fck.SetCollectedFees(sdk.Coins{sdk.NewCoin(denom, feeInputs)}) require.Equal(t, feeInputs, fck.GetCollectedFees(ctx).AmountOf(denom)) - keeper.SetProposerConsAddr(ctx, valConsAddr1) - keeper.SetPercentPrecommitVotes(ctx, sdk.OneDec()) - keeper.AllocateFees(ctx) + keeper.AllocateFees(ctx, sdk.OneDec(), valConsAddr1) // withdraw delegation ctx = ctx.WithBlockHeight(1) diff --git a/x/distribution/keeper/keeper.go b/x/distribution/keeper/keeper.go index 356e33312b..0ccf76ca63 100644 --- a/x/distribution/keeper/keeper.go +++ b/x/distribution/keeper/keeper.go @@ -10,7 +10,6 @@ import ( // keeper of the stake store type Keeper struct { storeKey sdk.StoreKey - storeTKey sdk.StoreKey cdc *codec.Codec paramSpace params.Subspace bankKeeper types.BankKeeper @@ -21,12 +20,11 @@ type Keeper struct { codespace sdk.CodespaceType } -func NewKeeper(cdc *codec.Codec, key, tkey sdk.StoreKey, paramSpace params.Subspace, ck types.BankKeeper, +func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, paramSpace params.Subspace, ck types.BankKeeper, sk types.StakeKeeper, fck types.FeeCollectionKeeper, codespace sdk.CodespaceType) Keeper { keeper := Keeper{ storeKey: key, - storeTKey: tkey, cdc: cdc, paramSpace: paramSpace.WithTypeTable(ParamTypeTable()), bankKeeper: ck, @@ -60,12 +58,12 @@ func (k Keeper) SetFeePool(ctx sdk.Context, feePool types.FeePool) { //______________________________________________________________________ // set the proposer public key for this block -func (k Keeper) GetProposerConsAddr(ctx sdk.Context) (consAddr sdk.ConsAddress) { - tstore := ctx.KVStore(k.storeTKey) +func (k Keeper) GetPreviousProposerConsAddr(ctx sdk.Context) (consAddr sdk.ConsAddress) { + store := ctx.KVStore(k.storeKey) - b := tstore.Get(ProposerKey) + b := store.Get(ProposerKey) if b == nil { - panic("Proposer cons address was likely not set in begin block") + panic("Previous proposer not set") } k.cdc.MustUnmarshalBinary(b, &consAddr) @@ -73,32 +71,10 @@ func (k Keeper) GetProposerConsAddr(ctx sdk.Context) (consAddr sdk.ConsAddress) } // get the proposer public key for this block -func (k Keeper) SetProposerConsAddr(ctx sdk.Context, consAddr sdk.ConsAddress) { - tstore := ctx.KVStore(k.storeTKey) +func (k Keeper) SetPreviousProposerConsAddr(ctx sdk.Context, consAddr sdk.ConsAddress) { + store := ctx.KVStore(k.storeKey) b := k.cdc.MustMarshalBinary(consAddr) - tstore.Set(ProposerKey, b) -} - -//______________________________________________________________________ - -// set the proposer public key for this block -func (k Keeper) GetPercentPrecommitVotes(ctx sdk.Context) (percentPrecommitVotes sdk.Dec) { - tstore := ctx.KVStore(k.storeTKey) - - b := tstore.Get(PercentPrecommitVotesKey) - if b == nil { - panic("Proposer cons address was likely not set in begin block") - } - - k.cdc.MustUnmarshalBinary(b, &percentPrecommitVotes) - return -} - -// get the proposer public key for this block -func (k Keeper) SetPercentPrecommitVotes(ctx sdk.Context, percentPrecommitVotes sdk.Dec) { - tstore := ctx.KVStore(k.storeTKey) - b := k.cdc.MustMarshalBinary(percentPrecommitVotes) - tstore.Set(PercentPrecommitVotesKey, b) + store.Set(ProposerKey, b) } //______________________________________________________________________ diff --git a/x/distribution/keeper/keeper_test.go b/x/distribution/keeper/keeper_test.go index b2ebb6581c..8244305113 100644 --- a/x/distribution/keeper/keeper_test.go +++ b/x/distribution/keeper/keeper_test.go @@ -8,23 +8,14 @@ import ( "github.com/stretchr/testify/require" ) -func TestSetGetProposerConsAddr(t *testing.T) { +func TestSetGetPreviousProposerConsAddr(t *testing.T) { ctx, _, keeper, _, _ := CreateTestInputDefault(t, false, 0) - keeper.SetProposerConsAddr(ctx, valConsAddr1) - res := keeper.GetProposerConsAddr(ctx) + keeper.SetPreviousProposerConsAddr(ctx, valConsAddr1) + res := keeper.GetPreviousProposerConsAddr(ctx) require.True(t, res.Equals(valConsAddr1), "expected: %v got: %v", valConsAddr1.String(), res.String()) } -func TestSetGetPercentPrecommitVotes(t *testing.T) { - ctx, _, keeper, _, _ := CreateTestInputDefault(t, false, 0) - - someDec := sdk.NewDec(333) - keeper.SetPercentPrecommitVotes(ctx, someDec) - res := keeper.GetPercentPrecommitVotes(ctx) - require.True(sdk.DecEq(t, someDec, res)) -} - func TestSetGetCommunityTax(t *testing.T) { ctx, _, keeper, _, _ := CreateTestInputDefault(t, false, 0) diff --git a/x/distribution/keeper/key.go b/x/distribution/keeper/key.go index 0f9644f777..2e59890810 100644 --- a/x/distribution/keeper/key.go +++ b/x/distribution/keeper/key.go @@ -10,10 +10,7 @@ var ( ValidatorDistInfoKey = []byte{0x01} // prefix for each key to a validator distribution DelegationDistInfoKey = []byte{0x02} // prefix for each key to a delegation distribution DelegatorWithdrawInfoKey = []byte{0x03} // prefix for each key to a delegator withdraw info - - // transient - ProposerKey = []byte{0x00} // key for storing the proposer operator address - PercentPrecommitVotesKey = []byte{0x01} // key for storing the power of the precommit validators + ProposerKey = []byte{0x04} // key for storing the proposer operator address // params store ParamStoreKeyCommunityTax = []byte("community-tax") diff --git a/x/distribution/keeper/test_common.go b/x/distribution/keeper/test_common.go index 871812a119..59b615ec8a 100644 --- a/x/distribution/keeper/test_common.go +++ b/x/distribution/keeper/test_common.go @@ -84,7 +84,6 @@ func CreateTestInputAdvanced(t *testing.T, isCheckTx bool, initCoins int64, sdk.Context, auth.AccountMapper, Keeper, stake.Keeper, DummyFeeCollectionKeeper) { keyDistr := sdk.NewKVStoreKey("distr") - tkeyDistr := sdk.NewTransientStoreKey("transient_distr") keyStake := sdk.NewKVStoreKey("stake") tkeyStake := sdk.NewTransientStoreKey("transient_stake") keyAcc := sdk.NewKVStoreKey("acc") @@ -95,7 +94,6 @@ func CreateTestInputAdvanced(t *testing.T, isCheckTx bool, initCoins int64, db := dbm.NewMemDB() ms := store.NewCommitMultiStore(db) - ms.MountStoreWithDB(tkeyDistr, sdk.StoreTypeTransient, nil) ms.MountStoreWithDB(keyDistr, sdk.StoreTypeIAVL, db) ms.MountStoreWithDB(tkeyStake, sdk.StoreTypeTransient, nil) ms.MountStoreWithDB(keyStake, sdk.StoreTypeIAVL, db) @@ -130,7 +128,7 @@ func CreateTestInputAdvanced(t *testing.T, isCheckTx bool, initCoins int64, } fck := DummyFeeCollectionKeeper{} - keeper := NewKeeper(cdc, keyDistr, tkeyDistr, pk.Subspace(DefaultParamspace), ck, sk, fck, types.DefaultCodespace) + keeper := NewKeeper(cdc, keyDistr, pk.Subspace(DefaultParamspace), ck, sk, fck, types.DefaultCodespace) // set the distribution hooks on staking sk = sk.WithHooks(keeper.Hooks()) diff --git a/x/distribution/keeper/validator_test.go b/x/distribution/keeper/validator_test.go index bdac19b93f..57c9a8b815 100644 --- a/x/distribution/keeper/validator_test.go +++ b/x/distribution/keeper/validator_test.go @@ -23,9 +23,7 @@ func TestWithdrawValidatorRewardsAllNoDelegator(t *testing.T) { feeInputs := sdk.NewInt(100) fck.SetCollectedFees(sdk.Coins{sdk.NewCoin(denom, feeInputs)}) require.Equal(t, feeInputs, fck.GetCollectedFees(ctx).AmountOf(denom)) - keeper.SetProposerConsAddr(ctx, valConsAddr1) - keeper.SetPercentPrecommitVotes(ctx, sdk.OneDec()) - keeper.AllocateFees(ctx) + keeper.AllocateFees(ctx, sdk.OneDec(), valConsAddr1) // withdraw self-delegation reward ctx = ctx.WithBlockHeight(1) @@ -57,9 +55,7 @@ func TestWithdrawValidatorRewardsAllDelegatorNoCommission(t *testing.T) { feeInputs := sdk.NewInt(100) fck.SetCollectedFees(sdk.Coins{sdk.NewCoin(denom, feeInputs)}) require.Equal(t, feeInputs, fck.GetCollectedFees(ctx).AmountOf(denom)) - keeper.SetProposerConsAddr(ctx, valConsAddr1) - keeper.SetPercentPrecommitVotes(ctx, sdk.OneDec()) - keeper.AllocateFees(ctx) + keeper.AllocateFees(ctx, sdk.OneDec(), valConsAddr1) // withdraw self-delegation reward ctx = ctx.WithBlockHeight(1) @@ -93,9 +89,7 @@ func TestWithdrawValidatorRewardsAllDelegatorWithCommission(t *testing.T) { feeInputs := sdk.NewInt(100) fck.SetCollectedFees(sdk.Coins{sdk.NewCoin(denom, feeInputs)}) require.Equal(t, feeInputs, fck.GetCollectedFees(ctx).AmountOf(denom)) - keeper.SetProposerConsAddr(ctx, valConsAddr1) - keeper.SetPercentPrecommitVotes(ctx, sdk.OneDec()) - keeper.AllocateFees(ctx) + keeper.AllocateFees(ctx, sdk.OneDec(), valConsAddr1) // withdraw validator reward ctx = ctx.WithBlockHeight(1) @@ -135,9 +129,7 @@ func TestWithdrawValidatorRewardsAllMultipleValidator(t *testing.T) { feeInputs := sdk.NewInt(1000) fck.SetCollectedFees(sdk.Coins{sdk.NewCoin(denom, feeInputs)}) require.Equal(t, feeInputs, fck.GetCollectedFees(ctx).AmountOf(denom)) - keeper.SetProposerConsAddr(ctx, valConsAddr1) - keeper.SetPercentPrecommitVotes(ctx, sdk.OneDec()) - keeper.AllocateFees(ctx) + keeper.AllocateFees(ctx, sdk.OneDec(), valConsAddr1) // withdraw validator reward ctx = ctx.WithBlockHeight(1) @@ -183,9 +175,7 @@ func TestWithdrawValidatorRewardsAllMultipleDelegator(t *testing.T) { feeInputs := sdk.NewInt(100) fck.SetCollectedFees(sdk.Coins{sdk.NewCoin(denom, feeInputs)}) require.Equal(t, feeInputs, fck.GetCollectedFees(ctx).AmountOf(denom)) - keeper.SetProposerConsAddr(ctx, valConsAddr1) - keeper.SetPercentPrecommitVotes(ctx, sdk.OneDec()) - keeper.AllocateFees(ctx) + keeper.AllocateFees(ctx, sdk.OneDec(), valConsAddr1) // withdraw validator reward ctx = ctx.WithBlockHeight(1)