rework to correctly use previous vote info

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)
 

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
-	}
-
-	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
+		return sdk.ZeroDec()
 	}
-	k.AllocateFees(ctx)
+	return sdk.NewDec(sumPrecommitPower).Quo(sdk.NewDec(totalPower))
 }

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

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)

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)

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,45 +58,23 @@ 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)
 	return
 }
 
 // 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)
 }
 
 //______________________________________________________________________

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)
 

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

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

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)