Merge PR #2599 from cosmos/jae/dist_refactor

* Rename Pool -> DelRewards; PoolCommission -> ValCommision
* FeePool.Pool -> FeePool.ValPool
* WithdrawalHeight->DelPoolWithdrawalHeight
* OnValidatorBeginUnbonding
* Caught the bug's tail
* Update vi.FeePoolWithdrawalHeight upon bonding
* Fix staking slashUnbondingDelegation bug; fixes simulator failure #9

Makefile

@@ -169,7 +169,7 @@ test_sim_gaia_nondeterminism:
 
 test_sim_gaia_fast:
 	@echo "Running quick Gaia simulation. This may take several minutes..."
-	@go test ./cmd/gaia/app -run TestFullGaiaSimulation -SimulationEnabled=true -SimulationNumBlocks=400 -SimulationBlockSize=200 -SimulationCommit=true -v -timeout 24h
+	@go test ./cmd/gaia/app -run TestFullGaiaSimulation -SimulationEnabled=true -SimulationNumBlocks=400 -SimulationBlockSize=200 -SimulationCommit=true -SimulationSeed=9 -v -timeout 24h
 
 test_sim_gaia_multi_seed:
 	@echo "Running multi-seed Gaia simulation. This may take awhile!"

cmd/gaia/app/app.go

@@ -332,22 +332,26 @@ func NewHooks(dh distr.Hooks, sh slashing.Hooks) Hooks {
 var _ sdk.StakingHooks = Hooks{}
 
 // nolint
-func (h Hooks) OnValidatorCreated(ctx sdk.Context, addr sdk.ValAddress) {
+func (h Hooks) OnValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) {
-	h.dh.OnValidatorCreated(ctx, addr)
+	h.dh.OnValidatorCreated(ctx, valAddr)
 }
-func (h Hooks) OnValidatorModified(ctx sdk.Context, addr sdk.ValAddress) {
+func (h Hooks) OnValidatorModified(ctx sdk.Context, valAddr sdk.ValAddress) {
-	h.dh.OnValidatorModified(ctx, addr)
+	h.dh.OnValidatorModified(ctx, valAddr)
 }
-func (h Hooks) OnValidatorRemoved(ctx sdk.Context, addr sdk.ValAddress) {
+func (h Hooks) OnValidatorRemoved(ctx sdk.Context, valAddr sdk.ValAddress) {
-	h.dh.OnValidatorRemoved(ctx, addr)
+	h.dh.OnValidatorRemoved(ctx, valAddr)
 }
-func (h Hooks) OnValidatorBonded(ctx sdk.Context, addr sdk.ConsAddress, operator sdk.ValAddress) {
+func (h Hooks) OnValidatorBonded(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) {
-	h.dh.OnValidatorBonded(ctx, addr, operator)
+	h.dh.OnValidatorBonded(ctx, consAddr, valAddr)
-	h.sh.OnValidatorBonded(ctx, addr, operator)
+	h.sh.OnValidatorBonded(ctx, consAddr, valAddr)
 }
-func (h Hooks) OnValidatorBeginUnbonding(ctx sdk.Context, addr sdk.ConsAddress, operator sdk.ValAddress) {
+func (h Hooks) OnValidatorPowerDidChange(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) {
-	h.dh.OnValidatorBeginUnbonding(ctx, addr, operator)
+	h.dh.OnValidatorPowerDidChange(ctx, consAddr, valAddr)
-	h.sh.OnValidatorBeginUnbonding(ctx, addr, operator)
+	h.sh.OnValidatorPowerDidChange(ctx, consAddr, valAddr)
+}
+func (h Hooks) OnValidatorBeginUnbonding(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) {
+	h.dh.OnValidatorBeginUnbonding(ctx, consAddr, valAddr)
+	h.sh.OnValidatorBeginUnbonding(ctx, consAddr, valAddr)
 }
 func (h Hooks) OnDelegationCreated(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) {
 	h.dh.OnDelegationCreated(ctx, delAddr, valAddr)

types/stake.go

@@ -111,12 +111,13 @@ type DelegationSet interface {
 
 // event hooks for staking validator object
 type StakingHooks interface {
-	OnValidatorCreated(ctx Context, address ValAddress)  // Must be called when a validator is created
+	OnValidatorCreated(ctx Context, valAddr ValAddress)  // Must be called when a validator is created
-	OnValidatorModified(ctx Context, address ValAddress) // Must be called when a validator's state changes
+	OnValidatorModified(ctx Context, valAddr ValAddress) // Must be called when a validator's state changes
-	OnValidatorRemoved(ctx Context, address ValAddress)  // Must be called when a validator is deleted
+	OnValidatorRemoved(ctx Context, valAddr ValAddress)  // Must be called when a validator is deleted
 
-	OnValidatorBonded(ctx Context, address ConsAddress, operator ValAddress)         // Must be called when a validator is bonded
+	OnValidatorBonded(ctx Context, consAddr ConsAddress, valAddr ValAddress)         // Must be called when a validator is bonded
-	OnValidatorBeginUnbonding(ctx Context, address ConsAddress, operator ValAddress) // Must be called when a validator begins unbonding
+	OnValidatorBeginUnbonding(ctx Context, consAddr ConsAddress, valAddr ValAddress) // Must be called when a validator begins unbonding
+	OnValidatorPowerDidChange(ctx Context, consAddr ConsAddress, valAddr ValAddress) // Called at EndBlock when a validator's power did change
 
 	OnDelegationCreated(ctx Context, delAddr AccAddress, valAddr ValAddress)        // Must be called when a delegation is created
 	OnDelegationSharesModified(ctx Context, delAddr AccAddress, valAddr ValAddress) // Must be called when a delegation's shares are modified

x/distribution/keeper/allocation.go

@@ -27,8 +27,8 @@ func (k Keeper) AllocateTokens(ctx sdk.Context, percentVotes sdk.Dec, proposer s
 	// apply commission
 	commission := proposerReward.MulDec(proposerValidator.GetCommission())
 	remaining := proposerReward.Minus(commission)
-	proposerDist.PoolCommission = proposerDist.PoolCommission.Plus(commission)
+	proposerDist.ValCommission = proposerDist.ValCommission.Plus(commission)
-	proposerDist.Pool = proposerDist.Pool.Plus(remaining)
+	proposerDist.DelPool = proposerDist.DelPool.Plus(remaining)
 
 	// allocate community funding
 	communityTax := k.GetCommunityTax(ctx)
@@ -38,7 +38,7 @@ func (k Keeper) AllocateTokens(ctx sdk.Context, percentVotes sdk.Dec, proposer s
 
 	// set the global pool within the distribution module
 	poolReceived := feesCollectedDec.Minus(proposerReward).Minus(communityFunding)
-	feePool.Pool = feePool.Pool.Plus(poolReceived)
+	feePool.ValPool = feePool.ValPool.Plus(poolReceived)
 
 	k.SetValidatorDistInfo(ctx, proposerDist)
 	k.SetFeePool(ctx, feePool)

x/distribution/keeper/allocation_test.go

@@ -35,7 +35,7 @@ func TestAllocateTokensBasic(t *testing.T) {
 
 	// initial fee pool should be empty
 	feePool := keeper.GetFeePool(ctx)
-	require.Nil(t, feePool.Pool)
+	require.Nil(t, feePool.ValPool)
 
 	// allocate 100 denom of fees
 	feeInputs := sdk.NewInt(100)
@@ -48,8 +48,8 @@ func TestAllocateTokensBasic(t *testing.T) {
 	percentRemaining := sdk.OneDec().Sub(percentProposer)
 	feePool = keeper.GetFeePool(ctx)
 	expRes := sdk.NewDecFromInt(feeInputs).Mul(percentRemaining)
-	require.Equal(t, 1, len(feePool.Pool))
+	require.Equal(t, 1, len(feePool.ValPool))
-	require.True(sdk.DecEq(t, expRes, feePool.Pool[0].Amount))
+	require.True(sdk.DecEq(t, expRes, feePool.ValPool[0].Amount))
 }
 
 func TestAllocateTokensWithCommunityTax(t *testing.T) {
@@ -76,8 +76,8 @@ func TestAllocateTokensWithCommunityTax(t *testing.T) {
 	percentProposer := sdk.NewDecWithPrec(5, 2)
 	percentRemaining := sdk.OneDec().Sub(communityTax.Add(percentProposer))
 	expRes := sdk.NewDecFromInt(feeInputs).Mul(percentRemaining)
-	require.Equal(t, 1, len(feePool.Pool))
+	require.Equal(t, 1, len(feePool.ValPool))
-	require.True(sdk.DecEq(t, expRes, feePool.Pool[0].Amount))
+	require.True(sdk.DecEq(t, expRes, feePool.ValPool[0].Amount))
 }
 
 func TestAllocateTokensWithPartialPrecommitPower(t *testing.T) {
@@ -105,6 +105,6 @@ func TestAllocateTokensWithPartialPrecommitPower(t *testing.T) {
 	percentProposer := sdk.NewDecWithPrec(1, 2).Add(sdk.NewDecWithPrec(4, 2).Mul(percentPrecommitVotes))
 	percentRemaining := sdk.OneDec().Sub(communityTax.Add(percentProposer))
 	expRes := sdk.NewDecFromInt(feeInputs).Mul(percentRemaining)
-	require.Equal(t, 1, len(feePool.Pool))
+	require.Equal(t, 1, len(feePool.ValPool))
-	require.True(sdk.DecEq(t, expRes, feePool.Pool[0].Amount))
+	require.True(sdk.DecEq(t, expRes, feePool.ValPool[0].Amount))
 }

x/distribution/keeper/hooks.go

@@ -6,32 +6,40 @@ import (
 )
 
 // Create a new validator distribution record
-func (k Keeper) onValidatorCreated(ctx sdk.Context, addr sdk.ValAddress) {
+func (k Keeper) onValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) {
 
 	height := ctx.BlockHeight()
 	vdi := types.ValidatorDistInfo{
-		OperatorAddr:            addr,
+		OperatorAddr:            valAddr,
 		FeePoolWithdrawalHeight: height,
-		Pool:                    types.DecCoins{},
-		PoolCommission:          types.DecCoins{},
 		DelAccum:                types.NewTotalAccum(height),
+		DelPool:                 types.DecCoins{},
+		ValCommission:           types.DecCoins{},
 	}
 	k.SetValidatorDistInfo(ctx, vdi)
 }
 
-// Withdrawal all validator rewards
+// Withdraw all validator rewards
-func (k Keeper) onValidatorModified(ctx sdk.Context, addr sdk.ValAddress) {
+func (k Keeper) onValidatorModified(ctx sdk.Context, valAddr sdk.ValAddress) {
 	// This doesn't need to be run at genesis
 	if ctx.BlockHeight() > 0 {
-		if err := k.WithdrawValidatorRewardsAll(ctx, addr); err != nil {
+		if err := k.WithdrawValidatorRewardsAll(ctx, valAddr); err != nil {
 			panic(err)
 		}
 	}
 }
 
+// XXX Consider removing this after debugging.
+func (k Keeper) onValidatorPowerDidChange(ctx sdk.Context, valAddr sdk.ValAddress) {
+	vi := k.GetValidatorDistInfo(ctx, valAddr)
+	if vi.FeePoolWithdrawalHeight != ctx.BlockHeight() {
+		panic("expected validator dist info FeePoolWithdrawalHeight to be updated, but was not.")
+	}
+}
+
 // Withdrawal all validator distribution rewards and cleanup the distribution record
-func (k Keeper) onValidatorRemoved(ctx sdk.Context, addr sdk.ValAddress) {
+func (k Keeper) onValidatorRemoved(ctx sdk.Context, valAddr sdk.ValAddress) {
-	k.RemoveValidatorDistInfo(ctx, addr)
+	k.RemoveValidatorDistInfo(ctx, valAddr)
 }
 
 //_________________________________________________________________________________________
@@ -41,9 +49,9 @@ func (k Keeper) onDelegationCreated(ctx sdk.Context, delAddr sdk.AccAddress,
 	valAddr sdk.ValAddress) {
 
 	ddi := types.DelegationDistInfo{
-		DelegatorAddr:    delAddr,
+		DelegatorAddr:           delAddr,
-		ValOperatorAddr:  valAddr,
+		ValOperatorAddr:         valAddr,
-		WithdrawalHeight: ctx.BlockHeight(),
+		DelPoolWithdrawalHeight: ctx.BlockHeight(),
 	}
 	k.SetDelegationDistInfo(ctx, ddi)
 }
@@ -77,14 +85,14 @@ var _ sdk.StakingHooks = Hooks{}
 func (k Keeper) Hooks() Hooks { return Hooks{k} }
 
 // nolint
-func (h Hooks) OnValidatorCreated(ctx sdk.Context, addr sdk.ValAddress) {
+func (h Hooks) OnValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) {
-	h.k.onValidatorCreated(ctx, addr)
+	h.k.onValidatorCreated(ctx, valAddr)
 }
-func (h Hooks) OnValidatorModified(ctx sdk.Context, addr sdk.ValAddress) {
+func (h Hooks) OnValidatorModified(ctx sdk.Context, valAddr sdk.ValAddress) {
-	h.k.onValidatorModified(ctx, addr)
+	h.k.onValidatorModified(ctx, valAddr)
 }
-func (h Hooks) OnValidatorRemoved(ctx sdk.Context, addr sdk.ValAddress) {
+func (h Hooks) OnValidatorRemoved(ctx sdk.Context, valAddr sdk.ValAddress) {
-	h.k.onValidatorRemoved(ctx, addr)
+	h.k.onValidatorRemoved(ctx, valAddr)
 }
 func (h Hooks) OnDelegationCreated(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) {
 	h.k.onValidatorModified(ctx, valAddr)
@@ -97,9 +105,12 @@ func (h Hooks) OnDelegationSharesModified(ctx sdk.Context, delAddr sdk.AccAddres
 func (h Hooks) OnDelegationRemoved(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) {
 	h.k.onDelegationRemoved(ctx, delAddr, valAddr)
 }
-func (h Hooks) OnValidatorBeginUnbonding(ctx sdk.Context, _ sdk.ConsAddress, addr sdk.ValAddress) {
+func (h Hooks) OnValidatorBeginUnbonding(ctx sdk.Context, _ sdk.ConsAddress, valAddr sdk.ValAddress) {
-	h.k.onValidatorModified(ctx, addr)
+	h.k.onValidatorModified(ctx, valAddr)
 }
-func (h Hooks) OnValidatorBonded(ctx sdk.Context, _ sdk.ConsAddress, addr sdk.ValAddress) {
+func (h Hooks) OnValidatorBonded(ctx sdk.Context, _ sdk.ConsAddress, valAddr sdk.ValAddress) {
-	h.k.onValidatorModified(ctx, addr)
+	h.k.onValidatorModified(ctx, valAddr)
+}
+func (h Hooks) OnValidatorPowerDidChange(ctx sdk.Context, _ sdk.ConsAddress, valAddr sdk.ValAddress) {
+	h.k.onValidatorPowerDidChange(ctx, valAddr)
 }

x/distribution/types/delegator_info.go

@@ -6,18 +6,18 @@ import (
 
 // distribution info for a delegation - used to determine entitled rewards
 type DelegationDistInfo struct {
-	DelegatorAddr    sdk.AccAddress `json:"delegator_addr"`
+	DelegatorAddr           sdk.AccAddress `json:"delegator_addr"`
-	ValOperatorAddr  sdk.ValAddress `json:"val_operator_addr"`
+	ValOperatorAddr         sdk.ValAddress `json:"val_operator_addr"`
-	WithdrawalHeight int64          `json:"withdrawal_height"` // last time this delegation withdrew rewards
+	DelPoolWithdrawalHeight int64          `json:"del_pool_withdrawal_height"` // last time this delegation withdrew rewards
 }
 
 func NewDelegationDistInfo(delegatorAddr sdk.AccAddress, valOperatorAddr sdk.ValAddress,
 	currentHeight int64) DelegationDistInfo {
 
 	return DelegationDistInfo{
-		DelegatorAddr:    delegatorAddr,
+		DelegatorAddr:           delegatorAddr,
-		ValOperatorAddr:  valOperatorAddr,
+		ValOperatorAddr:         valOperatorAddr,
-		WithdrawalHeight: currentHeight,
+		DelPoolWithdrawalHeight: currentHeight,
 	}
 }
 
@@ -40,13 +40,13 @@ func (di DelegationDistInfo) WithdrawRewards(fp FeePool, vi ValidatorDistInfo,
 
 	vi, fp = vi.TakeFeePoolRewards(fp, height, totalBonded, vdTokens, commissionRate)
 
-	blocks := height - di.WithdrawalHeight
+	blocks := height - di.DelPoolWithdrawalHeight
-	di.WithdrawalHeight = height
+	di.DelPoolWithdrawalHeight = height
 	accum := delegatorShares.MulInt(sdk.NewInt(blocks))
-	withdrawalTokens := vi.Pool.MulDec(accum).QuoDec(vi.DelAccum.Accum)
+	withdrawalTokens := vi.DelPool.MulDec(accum).QuoDec(vi.DelAccum.Accum)
-	remainingTokens := vi.Pool.Minus(withdrawalTokens)
+	remDelPool := vi.DelPool.Minus(withdrawalTokens)
 
-	vi.Pool = remainingTokens
+	vi.DelPool = remDelPool
 	vi.DelAccum.Accum = vi.DelAccum.Accum.Sub(accum)
 
 	return di, vi, fp, withdrawalTokens

x/distribution/types/delegator_info_test.go

@@ -26,31 +26,31 @@ func TestWithdrawRewards(t *testing.T) {
 
 	// simulate adding some stake for inflation
 	height = 10
-	fp.Pool = DecCoins{NewDecCoin("stake", 1000)}
+	fp.ValPool = DecCoins{NewDecCoin("stake", 1000)}
 
 	// withdraw rewards
 	di1, vi, fp, rewardRecv1 := di1.WithdrawRewards(fp, vi, height, totalBondedTokens,
 		validatorTokens, validatorDelShares, di1Shares, commissionRate)
 
-	assert.Equal(t, height, di1.WithdrawalHeight)
+	assert.Equal(t, height, di1.DelPoolWithdrawalHeight)
 	assert.True(sdk.DecEq(t, sdk.NewDec(900), fp.TotalValAccum.Accum))
-	assert.True(sdk.DecEq(t, sdk.NewDec(900), fp.Pool[0].Amount))
+	assert.True(sdk.DecEq(t, sdk.NewDec(900), fp.ValPool[0].Amount))
-	assert.True(sdk.DecEq(t, sdk.NewDec(49), vi.Pool[0].Amount))
+	assert.True(sdk.DecEq(t, sdk.NewDec(49), vi.DelPool[0].Amount))
-	assert.True(sdk.DecEq(t, sdk.NewDec(2), vi.PoolCommission[0].Amount))
+	assert.True(sdk.DecEq(t, sdk.NewDec(2), vi.ValCommission[0].Amount))
 	assert.True(sdk.DecEq(t, sdk.NewDec(49), rewardRecv1[0].Amount))
 
 	// add more blocks and inflation
 	height = 20
-	fp.Pool[0].Amount = fp.Pool[0].Amount.Add(sdk.NewDec(1000))
+	fp.ValPool[0].Amount = fp.ValPool[0].Amount.Add(sdk.NewDec(1000))
 
 	// withdraw rewards
 	di2, vi, fp, rewardRecv2 := di2.WithdrawRewards(fp, vi, height, totalBondedTokens,
 		validatorTokens, validatorDelShares, di2Shares, commissionRate)
 
-	assert.Equal(t, height, di2.WithdrawalHeight)
+	assert.Equal(t, height, di2.DelPoolWithdrawalHeight)
 	assert.True(sdk.DecEq(t, sdk.NewDec(1800), fp.TotalValAccum.Accum))
-	assert.True(sdk.DecEq(t, sdk.NewDec(1800), fp.Pool[0].Amount))
+	assert.True(sdk.DecEq(t, sdk.NewDec(1800), fp.ValPool[0].Amount))
-	assert.True(sdk.DecEq(t, sdk.NewDec(49), vi.Pool[0].Amount))
+	assert.True(sdk.DecEq(t, sdk.NewDec(49), vi.DelPool[0].Amount))
-	assert.True(sdk.DecEq(t, sdk.NewDec(4), vi.PoolCommission[0].Amount))
+	assert.True(sdk.DecEq(t, sdk.NewDec(4), vi.ValCommission[0].Amount))
 	assert.True(sdk.DecEq(t, sdk.NewDec(98), rewardRecv2[0].Amount))
 }

x/distribution/types/fee_pool.go

@@ -34,7 +34,7 @@ func (ta TotalAccum) UpdateForNewHeight(height int64, accumCreatedPerBlock sdk.D
 // global fee pool for distribution
 type FeePool struct {
 	TotalValAccum TotalAccum `json:"val_accum"`      // total valdator accum held by validators
-	Pool          DecCoins   `json:"pool"`           // funds for all validators which have yet to be withdrawn
+	ValPool       DecCoins   `json:"val_pool"`       // funds for all validators which have yet to be withdrawn
 	CommunityPool DecCoins   `json:"community_pool"` // pool for community funds yet to be spent
 }
 
@@ -49,7 +49,7 @@ func (f FeePool) UpdateTotalValAccum(height int64, totalBondedTokens sdk.Dec) Fe
 func InitialFeePool() FeePool {
 	return FeePool{
 		TotalValAccum: NewTotalAccum(0),
-		Pool:          DecCoins{},
+		ValPool:       DecCoins{},
 		CommunityPool: DecCoins{},
 	}
 }

x/distribution/types/validator_info.go

@@ -8,20 +8,20 @@ import (
 type ValidatorDistInfo struct {
 	OperatorAddr sdk.ValAddress `json:"operator_addr"`
 
-	FeePoolWithdrawalHeight int64    `json:"global_withdrawal_height"` // last height this validator withdrew from the global pool
+	FeePoolWithdrawalHeight int64 `json:"fee_pool_withdrawal_height"` // last height this validator withdrew from the global pool
-	Pool                    DecCoins `json:"pool"`                     // rewards owed to delegators, commission has already been charged (includes proposer reward)
-	PoolCommission          DecCoins `json:"pool_commission"`          // commission collected by this validator (pending withdrawal)
 
-	DelAccum TotalAccum `json:"del_accum"` // total proposer pool accumulation factor held by delegators
+	DelAccum      TotalAccum `json:"del_accum"`      // total accumulation factor held by delegators
+	DelPool       DecCoins   `json:"del_pool"`       // rewards owed to delegators, commission has already been charged (includes proposer reward)
+	ValCommission DecCoins   `json:"val_commission"` // commission collected by this validator (pending withdrawal)
 }
 
 func NewValidatorDistInfo(operatorAddr sdk.ValAddress, currentHeight int64) ValidatorDistInfo {
 	return ValidatorDistInfo{
 		OperatorAddr:            operatorAddr,
 		FeePoolWithdrawalHeight: currentHeight,
-		Pool:                    DecCoins{},
+		DelPool:                 DecCoins{},
-		PoolCommission:          DecCoins{},
 		DelAccum:                NewTotalAccum(currentHeight),
+		ValCommission:           DecCoins{},
 	}
 }
 
@@ -46,6 +46,7 @@ func (vi ValidatorDistInfo) TakeFeePoolRewards(fp FeePool, height int64, totalBo
 	fp = fp.UpdateTotalValAccum(height, totalBonded)
 
 	if fp.TotalValAccum.Accum.IsZero() {
+		vi.FeePoolWithdrawalHeight = height
 		return vi, fp
 	}
 
@@ -57,16 +58,16 @@ func (vi ValidatorDistInfo) TakeFeePoolRewards(fp FeePool, height int64, totalBo
 	if accum.GT(fp.TotalValAccum.Accum) {
 		panic("individual accum should never be greater than the total")
 	}
-	withdrawalTokens := fp.Pool.MulDec(accum).QuoDec(fp.TotalValAccum.Accum)
+	withdrawalTokens := fp.ValPool.MulDec(accum).QuoDec(fp.TotalValAccum.Accum)
-	remainingTokens := fp.Pool.Minus(withdrawalTokens)
+	remValPool := fp.ValPool.Minus(withdrawalTokens)
 
 	commission := withdrawalTokens.MulDec(commissionRate)
 	afterCommission := withdrawalTokens.Minus(commission)
 
 	fp.TotalValAccum.Accum = fp.TotalValAccum.Accum.Sub(accum)
-	fp.Pool = remainingTokens
+	fp.ValPool = remValPool
-	vi.PoolCommission = vi.PoolCommission.Plus(commission)
+	vi.ValCommission = vi.ValCommission.Plus(commission)
-	vi.Pool = vi.Pool.Plus(afterCommission)
+	vi.DelPool = vi.DelPool.Plus(afterCommission)
 
 	return vi, fp
 }
@@ -77,8 +78,8 @@ func (vi ValidatorDistInfo) WithdrawCommission(fp FeePool, height int64,
 
 	vi, fp = vi.TakeFeePoolRewards(fp, height, totalBonded, vdTokens, commissionRate)
 
-	withdrawalTokens := vi.PoolCommission
+	withdrawalTokens := vi.ValCommission
-	vi.PoolCommission = DecCoins{} // zero
+	vi.ValCommission = DecCoins{} // zero
 
 	return vi, fp, withdrawalTokens
 }

x/distribution/types/validator_info_test.go

@@ -26,29 +26,29 @@ func TestTakeFeePoolRewards(t *testing.T) {
 
 	// simulate adding some stake for inflation
 	height = 10
-	fp.Pool = DecCoins{NewDecCoin("stake", 1000)}
+	fp.ValPool = DecCoins{NewDecCoin("stake", 1000)}
 
 	vi1, fp = vi1.TakeFeePoolRewards(fp, height, totalBondedTokens, validatorTokens1, commissionRate1)
 	require.True(sdk.DecEq(t, sdk.NewDec(900), fp.TotalValAccum.Accum))
-	assert.True(sdk.DecEq(t, sdk.NewDec(900), fp.Pool[0].Amount))
+	assert.True(sdk.DecEq(t, sdk.NewDec(900), fp.ValPool[0].Amount))
-	assert.True(sdk.DecEq(t, sdk.NewDec(100-2), vi1.Pool[0].Amount))
+	assert.True(sdk.DecEq(t, sdk.NewDec(100-2), vi1.DelPool[0].Amount))
-	assert.True(sdk.DecEq(t, sdk.NewDec(2), vi1.PoolCommission[0].Amount))
+	assert.True(sdk.DecEq(t, sdk.NewDec(2), vi1.ValCommission[0].Amount))
 
 	vi2, fp = vi2.TakeFeePoolRewards(fp, height, totalBondedTokens, validatorTokens2, commissionRate2)
 	require.True(sdk.DecEq(t, sdk.NewDec(500), fp.TotalValAccum.Accum))
-	assert.True(sdk.DecEq(t, sdk.NewDec(500), fp.Pool[0].Amount))
+	assert.True(sdk.DecEq(t, sdk.NewDec(500), fp.ValPool[0].Amount))
-	assert.True(sdk.DecEq(t, sdk.NewDec(400-12), vi2.Pool[0].Amount))
+	assert.True(sdk.DecEq(t, sdk.NewDec(400-12), vi2.DelPool[0].Amount))
-	assert.True(sdk.DecEq(t, vi2.PoolCommission[0].Amount, sdk.NewDec(12)))
+	assert.True(sdk.DecEq(t, vi2.ValCommission[0].Amount, sdk.NewDec(12)))
 
 	// add more blocks and inflation
 	height = 20
-	fp.Pool[0].Amount = fp.Pool[0].Amount.Add(sdk.NewDec(1000))
+	fp.ValPool[0].Amount = fp.ValPool[0].Amount.Add(sdk.NewDec(1000))
 
 	vi3, fp = vi3.TakeFeePoolRewards(fp, height, totalBondedTokens, validatorTokens3, commissionRate3)
 	require.True(sdk.DecEq(t, sdk.NewDec(500), fp.TotalValAccum.Accum))
-	assert.True(sdk.DecEq(t, sdk.NewDec(500), fp.Pool[0].Amount))
+	assert.True(sdk.DecEq(t, sdk.NewDec(500), fp.ValPool[0].Amount))
-	assert.True(sdk.DecEq(t, sdk.NewDec(1000-40), vi3.Pool[0].Amount))
+	assert.True(sdk.DecEq(t, sdk.NewDec(1000-40), vi3.DelPool[0].Amount))
-	assert.True(sdk.DecEq(t, vi3.PoolCommission[0].Amount, sdk.NewDec(40)))
+	assert.True(sdk.DecEq(t, vi3.ValCommission[0].Amount, sdk.NewDec(40)))
 }
 
 func TestWithdrawCommission(t *testing.T) {
@@ -63,23 +63,23 @@ func TestWithdrawCommission(t *testing.T) {
 
 	// simulate adding some stake for inflation
 	height = 10
-	fp.Pool = DecCoins{NewDecCoin("stake", 1000)}
+	fp.ValPool = DecCoins{NewDecCoin("stake", 1000)}
 
 	// for a more fun staring condition, have an non-withdraw update
 	vi, fp = vi.TakeFeePoolRewards(fp, height, totalBondedTokens, validatorTokens, commissionRate)
 	require.True(sdk.DecEq(t, sdk.NewDec(900), fp.TotalValAccum.Accum))
-	assert.True(sdk.DecEq(t, sdk.NewDec(900), fp.Pool[0].Amount))
+	assert.True(sdk.DecEq(t, sdk.NewDec(900), fp.ValPool[0].Amount))
-	assert.True(sdk.DecEq(t, sdk.NewDec(100-2), vi.Pool[0].Amount))
+	assert.True(sdk.DecEq(t, sdk.NewDec(100-2), vi.DelPool[0].Amount))
-	assert.True(sdk.DecEq(t, sdk.NewDec(2), vi.PoolCommission[0].Amount))
+	assert.True(sdk.DecEq(t, sdk.NewDec(2), vi.ValCommission[0].Amount))
 
 	// add more blocks and inflation
 	height = 20
-	fp.Pool[0].Amount = fp.Pool[0].Amount.Add(sdk.NewDec(1000))
+	fp.ValPool[0].Amount = fp.ValPool[0].Amount.Add(sdk.NewDec(1000))
 
 	vi, fp, commissionRecv := vi.WithdrawCommission(fp, height, totalBondedTokens, validatorTokens, commissionRate)
 	require.True(sdk.DecEq(t, sdk.NewDec(1800), fp.TotalValAccum.Accum))
-	assert.True(sdk.DecEq(t, sdk.NewDec(1800), fp.Pool[0].Amount))
+	assert.True(sdk.DecEq(t, sdk.NewDec(1800), fp.ValPool[0].Amount))
-	assert.True(sdk.DecEq(t, sdk.NewDec(200-4), vi.Pool[0].Amount))
+	assert.True(sdk.DecEq(t, sdk.NewDec(200-4), vi.DelPool[0].Amount))
-	assert.Zero(t, len(vi.PoolCommission))
+	assert.Zero(t, len(vi.ValCommission))
 	assert.True(sdk.DecEq(t, sdk.NewDec(4), commissionRecv[0].Amount))
 }

x/slashing/hooks.go

@@ -51,16 +51,18 @@ func (k Keeper) Hooks() Hooks {
 }
 
 // Implements sdk.ValidatorHooks
-func (h Hooks) OnValidatorBonded(ctx sdk.Context, address sdk.ConsAddress, operator sdk.ValAddress) {
+func (h Hooks) OnValidatorBonded(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) {
-	h.k.onValidatorBonded(ctx, address, operator)
+	h.k.onValidatorBonded(ctx, consAddr, valAddr)
 }
 
 // Implements sdk.ValidatorHooks
-func (h Hooks) OnValidatorBeginUnbonding(ctx sdk.Context, address sdk.ConsAddress, operator sdk.ValAddress) {
+func (h Hooks) OnValidatorBeginUnbonding(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) {
-	h.k.onValidatorBeginUnbonding(ctx, address, operator)
+	h.k.onValidatorBeginUnbonding(ctx, consAddr, valAddr)
 }
 
 // nolint - unused hooks
+func (h Hooks) OnValidatorPowerDidChange(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) {
+}
 func (h Hooks) OnValidatorCreated(_ sdk.Context, _ sdk.ValAddress)                           {}
 func (h Hooks) OnValidatorModified(_ sdk.Context, _ sdk.ValAddress)                          {}
 func (h Hooks) OnValidatorRemoved(_ sdk.Context, _ sdk.ValAddress)                           {}

x/stake/keeper/hooks.go

@@ -6,32 +6,38 @@ import (
 )
 
 // Expose the hooks if present
-func (k Keeper) OnValidatorCreated(ctx sdk.Context, address sdk.ValAddress) {
+func (k Keeper) OnValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) {
 	if k.hooks != nil {
-		k.hooks.OnValidatorCreated(ctx, address)
+		k.hooks.OnValidatorCreated(ctx, valAddr)
 	}
 }
-func (k Keeper) OnValidatorModified(ctx sdk.Context, address sdk.ValAddress) {
+func (k Keeper) OnValidatorModified(ctx sdk.Context, valAddr sdk.ValAddress) {
 	if k.hooks != nil {
-		k.hooks.OnValidatorModified(ctx, address)
+		k.hooks.OnValidatorModified(ctx, valAddr)
 	}
 }
 
-func (k Keeper) OnValidatorRemoved(ctx sdk.Context, address sdk.ValAddress) {
+func (k Keeper) OnValidatorRemoved(ctx sdk.Context, valAddr sdk.ValAddress) {
 	if k.hooks != nil {
-		k.hooks.OnValidatorRemoved(ctx, address)
+		k.hooks.OnValidatorRemoved(ctx, valAddr)
 	}
 }
 
-func (k Keeper) OnValidatorBonded(ctx sdk.Context, address sdk.ConsAddress, operator sdk.ValAddress) {
+func (k Keeper) OnValidatorBonded(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) {
 	if k.hooks != nil {
-		k.hooks.OnValidatorBonded(ctx, address, operator)
+		k.hooks.OnValidatorBonded(ctx, consAddr, valAddr)
 	}
 }
 
-func (k Keeper) OnValidatorBeginUnbonding(ctx sdk.Context, address sdk.ConsAddress, operator sdk.ValAddress) {
+func (k Keeper) OnValidatorPowerDidChange(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) {
 	if k.hooks != nil {
-		k.hooks.OnValidatorBeginUnbonding(ctx, address, operator)
+		k.hooks.OnValidatorPowerDidChange(ctx, consAddr, valAddr)
+	}
+}
+
+func (k Keeper) OnValidatorBeginUnbonding(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) {
+	if k.hooks != nil {
+		k.hooks.OnValidatorBeginUnbonding(ctx, consAddr, valAddr)
 	}
 }
 

x/stake/keeper/slash.go

@@ -181,7 +181,7 @@ func (k Keeper) slashUnbondingDelegation(ctx sdk.Context, unbondingDelegation ty
 
 		// Burn loose tokens
 		// Ref https://github.com/cosmos/cosmos-sdk/pull/1278#discussion_r198657760
-		pool.LooseTokens = pool.LooseTokens.Sub(slashAmount)
+		pool.LooseTokens = pool.LooseTokens.Sub(sdk.NewDecFromInt(unbondingSlashAmount))
 		k.SetPool(ctx, pool)
 	}
 

x/stake/keeper/val_state_change.go

@@ -40,8 +40,8 @@ func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx sdk.Context) (updates []ab
 	for ; iterator.Valid() && count < int(maxValidators); iterator.Next() {
 
 		// fetch the validator
-		operator := sdk.ValAddress(iterator.Value())
+		valAddr := sdk.ValAddress(iterator.Value())
-		validator := k.mustGetValidator(ctx, operator)
+		validator := k.mustGetValidator(ctx, valAddr)
 
 		if validator.Jailed {
 			panic("should never retrieve a jailed validator from the power store")
@@ -67,9 +67,9 @@ func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx sdk.Context) (updates []ab
 		}
 
 		// fetch the old power bytes
-		var operatorBytes [sdk.AddrLen]byte
+		var valAddrBytes [sdk.AddrLen]byte
-		copy(operatorBytes[:], operator[:])
+		copy(valAddrBytes[:], valAddr[:])
-		oldPowerBytes, found := last[operatorBytes]
+		oldPowerBytes, found := last[valAddrBytes]
 
 		// calculate the new power bytes
 		newPower := validator.BondedTokens().RoundInt64()
@@ -78,12 +78,18 @@ func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx sdk.Context) (updates []ab
 		if !found || !bytes.Equal(oldPowerBytes, newPowerBytes) {
 			updates = append(updates, validator.ABCIValidatorUpdate())
 
+			// XXX Assert that the validator had updated its ValidatorDistInfo.FeePoolWithdrawalHeight.
+			// XXX This hook probably shouldn't exist.  Maybe rethink the hook system.
+			if k.hooks != nil {
+				k.hooks.OnValidatorPowerDidChange(ctx, validator.ConsAddress(), valAddr)
+			}
+
 			// set validator power on lookup index.
-			k.SetLastValidatorPower(ctx, operator, sdk.NewInt(newPower))
+			k.SetLastValidatorPower(ctx, valAddr, sdk.NewInt(newPower))
 		}
 
 		// validator still in the validator set, so delete from the copy
-		delete(last, operatorBytes)
+		delete(last, valAddrBytes)
 
 		// keep count
 		count++
@@ -94,10 +100,10 @@ func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx sdk.Context) (updates []ab
 	noLongerBonded := k.sortNoLongerBonded(last)
 
 	// iterate through the sorted no-longer-bonded validators
-	for _, operator := range noLongerBonded {
+	for _, valAddrBytes := range noLongerBonded {
 
 		// fetch the validator
-		validator := k.mustGetValidator(ctx, sdk.ValAddress(operator))
+		validator := k.mustGetValidator(ctx, sdk.ValAddress(valAddrBytes))
 
 		// bonded to unbonding
 		k.bondedToUnbonding(ctx, validator)
@@ -108,7 +114,7 @@ func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx sdk.Context) (updates []ab
 		}
 
 		// delete from the bonded validator index
-		k.DeleteLastValidatorPower(ctx, sdk.ValAddress(operator))
+		k.DeleteLastValidatorPower(ctx, sdk.ValAddress(valAddrBytes))
 
 		// update the validator set
 		updates = append(updates, validator.ABCIValidatorUpdateZero())
@@ -257,11 +263,11 @@ func (k Keeper) getLastValidatorsByAddr(ctx sdk.Context) validatorsByAddr {
 	store := ctx.KVStore(k.storeKey)
 	iterator := sdk.KVStorePrefixIterator(store, LastValidatorPowerKey)
 	for ; iterator.Valid(); iterator.Next() {
-		var operator [sdk.AddrLen]byte
+		var valAddr [sdk.AddrLen]byte
-		copy(operator[:], iterator.Key()[1:])
+		copy(valAddr[:], iterator.Key()[1:])
 		powerBytes := iterator.Value()
-		last[operator] = make([]byte, len(powerBytes))
+		last[valAddr] = make([]byte, len(powerBytes))
-		copy(last[operator][:], powerBytes[:])
+		copy(last[valAddr][:], powerBytes[:])
 	}
 	return last
 }
@@ -272,10 +278,10 @@ func (k Keeper) sortNoLongerBonded(last validatorsByAddr) [][]byte {
 	// sort the map keys for determinism
 	noLongerBonded := make([][]byte, len(last))
 	index := 0
-	for operatorBytes := range last {
+	for valAddrBytes := range last {
-		operator := make([]byte, sdk.AddrLen)
+		valAddr := make([]byte, sdk.AddrLen)
-		copy(operator[:], operatorBytes[:])
+		copy(valAddr[:], valAddrBytes[:])
-		noLongerBonded[index] = operator
+		noLongerBonded[index] = valAddr
 		index++
 	}
 	// sorted by address - order doesn't matter

x/stake/simulation/invariants.go

@@ -68,12 +68,12 @@ func SupplyInvariants(ck bank.Keeper, k stake.Keeper, f auth.FeeCollectionKeeper
 		loose = loose.Add(feePool.CommunityPool.AmountOf("steak"))
 
 		// add validator distribution pool
-		loose = loose.Add(feePool.Pool.AmountOf("steak"))
+		loose = loose.Add(feePool.ValPool.AmountOf("steak"))
 
 		// add validator distribution commission and yet-to-be-withdrawn-by-delegators
 		d.IterateValidatorDistInfos(ctx, func(_ int64, distInfo distribution.ValidatorDistInfo) (stop bool) {
-			loose = loose.Add(distInfo.Pool.AmountOf("steak"))
+			loose = loose.Add(distInfo.ValCommission.AmountOf("steak"))
-			loose = loose.Add(distInfo.PoolCommission.AmountOf("steak"))
+			loose = loose.Add(distInfo.DelPool.AmountOf("steak"))
 			return false
 		})