WIP: use block power throughout distribution (II)

cmd/gaia/app/app.go

@@ -345,6 +345,7 @@ func (h Hooks) OnValidatorBonded(ctx sdk.Context, addr sdk.ConsAddress) {
 	h.sh.OnValidatorBonded(ctx, addr)
 }
 func (h Hooks) OnValidatorBeginUnbonding(ctx sdk.Context, addr sdk.ConsAddress, operator sdk.ValAddress) {
+	h.dh.OnValidatorBeginUnbonding(ctx, addr, operator)
 	h.sh.OnValidatorBeginUnbonding(ctx, addr, operator)
 }
 func (h Hooks) OnDelegationCreated(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) {

types/decimal.go

@@ -247,6 +247,11 @@ func (d Dec) QuoInt(i Int) Dec {
 	return Dec{mul}
 }
 
+// is integer, e.g. decimals are zero.
+func (d Dec) IsInteger() bool {
+	return new(big.Int).Rem(d.Int, precisionReuse).Sign() == 0
+}
+
 func (d Dec) String() string {
 	str := d.ToLeftPaddedWithDecimals(Precision)
 	placement := len(str) - Precision

types/stake.go

@@ -85,9 +85,9 @@ type ValidatorSet interface {
 
 // delegation bond for a delegated proof of stake system
 type Delegation interface {
-	GetDelegator() AccAddress // delegator AccAddress for the bond
-	GetValidator() ValAddress // validator operator address
-	GetShares() Dec           // amount of validator's shares held in this delegation
+	GetDelegatorAddr() AccAddress // delegator AccAddress for the bond
+	GetValidatorAddr() ValAddress // validator operator address
+	GetShares() Dec               // amount of validator's shares held in this delegation
 }
 
 // properties for the set of all delegations for a particular

x/distribution/keeper/delegation.go

@@ -69,25 +69,28 @@ func (k Keeper) RemoveDelegatorWithdrawAddr(ctx sdk.Context, delAddr, withdrawAd
 
 //___________________________________________________________________________________________
 
-// withdraw all the rewards for a single delegation
+// Withdraw all the rewards for a single delegation.
+// NOTE: This gets called "onDelegationSharesModified",
+// meaning any changes to bonded coins.
 func (k Keeper) WithdrawDelegationReward(ctx sdk.Context, delegatorAddr sdk.AccAddress,
-	validatorAddr sdk.ValAddress) sdk.Error {
+	valAddr sdk.ValAddress) sdk.Error {
 
-	if !k.HasDelegationDistInfo(ctx, delegatorAddr, validatorAddr) {
+	if !k.HasDelegationDistInfo(ctx, delegatorAddr, valAddr) {
 		return types.ErrNoDelegationDistInfo(k.codespace)
 	}
 
 	// TODO: Reconcile with duplicate code in getDelegatorRewardsAll.
 	height := ctx.BlockHeight()
-	bondedTokens := k.stakeKeeper.TotalPower(ctx)
+	lastTotalPower := k.stakeKeeper.GetLastTotalPower(ctx)
+	lastValPower := k.stakeKeeper.GetLastValidatorPower(ctx, valAddr)
 	feePool := k.GetFeePool(ctx)
-	delInfo := k.GetDelegationDistInfo(ctx, delegatorAddr, validatorAddr)
-	valInfo := k.GetValidatorDistInfo(ctx, validatorAddr)
-	validator := k.stakeKeeper.Validator(ctx, validatorAddr)
-	delegation := k.stakeKeeper.Delegation(ctx, delegatorAddr, validatorAddr)
+	delInfo := k.GetDelegationDistInfo(ctx, delegatorAddr, valAddr)
+	valInfo := k.GetValidatorDistInfo(ctx, valAddr)
+	validator := k.stakeKeeper.Validator(ctx, valAddr)
+	delegation := k.stakeKeeper.Delegation(ctx, delegatorAddr, valAddr)
 
-	delInfo, valInfo, feePool, withdraw := delInfo.WithdrawRewards(feePool, valInfo, height, bondedTokens,
-		validator.GetPower(), validator.GetDelegatorShares(), delegation.GetShares(), validator.GetCommission())
+	delInfo, valInfo, feePool, withdraw := delInfo.WithdrawRewards(feePool, valInfo, height, lastTotalPower,
+		lastValPower, validator.GetDelegatorShares(), delegation.GetShares(), validator.GetCommission())
 
 	k.SetValidatorDistInfo(ctx, valInfo)
 	k.SetDelegationDistInfo(ctx, delInfo)
@@ -123,20 +126,21 @@ func (k Keeper) WithdrawDelegationRewardsAll(ctx sdk.Context, delegatorAddr sdk.
 func (k Keeper) getDelegatorRewardsAll(ctx sdk.Context, delAddr sdk.AccAddress, height int64) types.DecCoins {
 
 	withdraw := types.DecCoins{}
-	bondedTokens := k.stakeKeeper.TotalPower(ctx)
+	lastTotalPower := k.stakeKeeper.GetLastTotalPower(ctx)
 
 	// iterate over all the delegations
 	// TODO: Reconcile with duplicate code in WithdrawDelegationReward.
 	operationAtDelegation := func(_ int64, del sdk.Delegation) (stop bool) {
 		feePool := k.GetFeePool(ctx)
-		valAddr := del.GetValidator()
+		valAddr := del.GetValidatorAddr()
+		lastValPower := k.stakeKeeper.GetLastValidatorPower(ctx, valAddr)
 		delInfo := k.GetDelegationDistInfo(ctx, delAddr, valAddr)
 		valInfo := k.GetValidatorDistInfo(ctx, valAddr)
 		validator := k.stakeKeeper.Validator(ctx, valAddr)
 		delegation := k.stakeKeeper.Delegation(ctx, delAddr, valAddr)
 
-		delInfo, valInfo, feePool, diWithdraw := delInfo.WithdrawRewards(feePool, valInfo, height, bondedTokens,
-			validator.GetPower(), validator.GetDelegatorShares(), delegation.GetShares(), validator.GetCommission())
+		delInfo, valInfo, feePool, diWithdraw := delInfo.WithdrawRewards(feePool, valInfo, height, lastTotalPower,
+			lastValPower, validator.GetDelegatorShares(), delegation.GetShares(), validator.GetCommission())
 		withdraw = withdraw.Plus(diWithdraw)
 		k.SetFeePool(ctx, feePool)
 		k.SetValidatorDistInfo(ctx, valInfo)

x/distribution/keeper/delegation_test.go

@@ -34,6 +34,8 @@ func TestWithdrawDelegationRewardBasic(t *testing.T) {
 
 	// withdraw delegation
 	ctx = ctx.WithBlockHeight(1)
+	sk.SetLastTotalPower(ctx, sdk.NewDec(10))
+	sk.SetLastValidatorPower(ctx, valOpAddr1, sdk.NewDec(10))
 	keeper.WithdrawDelegationReward(ctx, delAddr1, valOpAddr1)
 	amt = accMapper.GetAccount(ctx, delAddr1).GetCoins().AmountOf(denom)
 
@@ -204,8 +206,6 @@ func TestWithdrawDelegationRewardsAll(t *testing.T) {
 	got = stakeHandler(ctx, msgCreateValidator)
 	require.True(t, got.IsOK(), "expected msg to be ok, got %v", got)
 
-	_ = sk.ApplyAndReturnValidatorSetUpdates(ctx)
-
 	// delegate to all the validators
 	msgDelegate := stake.NewTestMsgDelegate(delAddr1, valOpAddr1, 10)
 	require.True(t, stakeHandler(ctx, msgDelegate).IsOK())
@@ -214,6 +214,9 @@ func TestWithdrawDelegationRewardsAll(t *testing.T) {
 	msgDelegate = stake.NewTestMsgDelegate(delAddr1, valOpAddr3, 30)
 	require.True(t, stakeHandler(ctx, msgDelegate).IsOK())
 
+	// Update sk's LastValidatorPower/LastTotalPowers.
+	_ = sk.ApplyAndReturnValidatorSetUpdates(ctx)
+
 	// 40 tokens left after delegating 60 of them
 	amt := accMapper.GetAccount(ctx, delAddr1).GetCoins().AmountOf(denom)
 	require.Equal(t, int64(40), amt.Int64())

x/distribution/keeper/keeper_test.go

@@ -29,9 +29,9 @@ func TestSetGetFeePool(t *testing.T) {
 	ctx, _, keeper, _, _ := CreateTestInputDefault(t, false, 0)
 
 	fp := types.InitialFeePool()
-	fp.ValAccum.UpdateHeight = 777
+	fp.TotalValAccum.UpdateHeight = 777
 
 	keeper.SetFeePool(ctx, fp)
 	res := keeper.GetFeePool(ctx)
-	require.Equal(t, fp.ValAccum, res.ValAccum)
+	require.Equal(t, fp.TotalValAccum, res.TotalValAccum)
 }

x/distribution/keeper/test_common.go

@@ -114,7 +114,6 @@ func CreateTestInputAdvanced(t *testing.T, isCheckTx bool, initCoins int64,
 	sk := stake.NewKeeper(cdc, keyStake, tkeyStake, ck, pk.Subspace(stake.DefaultParamspace), stake.DefaultCodespace)
 	sk.SetPool(ctx, stake.InitialPool())
 	sk.SetParams(ctx, stake.DefaultParams())
-	sk.InitIntraTxCounter(ctx)
 
 	// fill all the addresses with some coins, set the loose pool tokens simultaneously
 	for _, addr := range addrs {

x/distribution/keeper/validator.go

@@ -50,15 +50,16 @@ func (k Keeper) WithdrawValidatorRewardsAll(ctx sdk.Context, operatorAddr sdk.Va
 	// withdraw self-delegation
 	height := ctx.BlockHeight()
 	validator := k.stakeKeeper.Validator(ctx, operatorAddr)
+	lastValPower := k.stakeKeeper.GetLastValidatorPower(ctx, operatorAddr)
 	accAddr := sdk.AccAddress(operatorAddr.Bytes())
 	withdraw := k.getDelegatorRewardsAll(ctx, accAddr, height)
 
 	// withdrawal validator commission rewards
-	bondedTokens := k.stakeKeeper.TotalPower(ctx)
+	lastTotalPower := k.stakeKeeper.GetLastTotalPower(ctx)
 	valInfo := k.GetValidatorDistInfo(ctx, operatorAddr)
 	feePool := k.GetFeePool(ctx)
-	valInfo, feePool, commission := valInfo.WithdrawCommission(feePool, height, bondedTokens,
-		validator.GetPower(), validator.GetCommission())
+	valInfo, feePool, commission := valInfo.WithdrawCommission(feePool, height, lastTotalPower,
+		lastValPower, validator.GetCommission())
 	withdraw = withdraw.Plus(commission)
 	k.SetValidatorDistInfo(ctx, valInfo)
 

x/distribution/types/delegator_info.go

@@ -21,7 +21,13 @@ func NewDelegationDistInfo(delegatorAddr sdk.AccAddress, valOperatorAddr sdk.Val
 	}
 }
 
-// withdraw rewards from delegator
+// Withdraw rewards from delegator.
+// Among many things, it does:
+// * updates validator info's total del accum.
+// * calls vi.TakeFeePoolRewards, which:
+//   * updates validator info's FeePoolWithdrawalHeight, thus setting accum to 0.
+//   * updates fee pool to latest height and total val accum w/ given totalBonded.
+//   (see comment on TakeFeePoolRewards for more info).
 func (di DelegationDistInfo) WithdrawRewards(fp FeePool, vi ValidatorDistInfo,
 	height int64, totalBonded, vdTokens, totalDelShares, delegatorShares,
 	commissionRate sdk.Dec) (DelegationDistInfo, ValidatorDistInfo, FeePool, DecCoins) {

x/distribution/types/delegator_info_test.go

@@ -33,7 +33,7 @@ func TestWithdrawRewards(t *testing.T) {
 		validatorTokens, validatorDelShares, di1Shares, commissionRate)
 
 	assert.Equal(t, height, di1.WithdrawalHeight)
-	assert.True(sdk.DecEq(t, sdk.NewDec(900), fp.ValAccum.Accum))
+	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(49), vi.Pool[0].Amount))
 	assert.True(sdk.DecEq(t, sdk.NewDec(2), vi.PoolCommission[0].Amount))
@@ -48,7 +48,7 @@ func TestWithdrawRewards(t *testing.T) {
 		validatorTokens, validatorDelShares, di2Shares, commissionRate)
 
 	assert.Equal(t, height, di2.WithdrawalHeight)
-	assert.True(sdk.DecEq(t, sdk.NewDec(1800), fp.ValAccum.Accum))
+	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(49), vi.Pool[0].Amount))
 	assert.True(sdk.DecEq(t, sdk.NewDec(4), vi.PoolCommission[0].Amount))

x/distribution/types/fee_pool.go

@@ -1,7 +1,10 @@
 package types
 
 import (
+	"fmt"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	cmn "github.com/tendermint/tendermint/libs/common"
 )
 
 // total accumulation tracker
@@ -29,25 +32,50 @@ func (ta TotalAccum) UpdateForNewHeight(height int64, accumCreatedPerBlock sdk.D
 	return ta
 }
 
+// update total validator accumulation factor for the new height
+// CONTRACT: height should be greater than the old height
+func (ta TotalAccum) UpdateForNewHeight_DEBUG(height int64, accumCreatedPerBlock sdk.Dec) TotalAccum {
+	blocks := height - ta.UpdateHeight
+	if blocks < 0 {
+		panic("reverse updated for new height")
+	}
+	if !accumCreatedPerBlock.IsZero() && blocks != 0 {
+		fmt.Println(
+			cmn.Blue(
+				fmt.Sprintf("FP Add %v * %v = %v, + %v (old) => %v (new)",
+					accumCreatedPerBlock.String(), sdk.NewInt(blocks),
+					accumCreatedPerBlock.MulInt(sdk.NewInt(blocks)).String(),
+					ta.Accum.String(),
+					ta.Accum.Add(accumCreatedPerBlock.MulInt(sdk.NewInt(blocks))).String(),
+				),
+			),
+		)
+	}
+	ta.Accum = ta.Accum.Add(accumCreatedPerBlock.MulInt(sdk.NewInt(blocks)))
+	ta.UpdateHeight = height
+	return ta
+}
+
 //___________________________________________________________________________________________
 
 // global fee pool for distribution
 type FeePool struct {
-	ValAccum      TotalAccum `json:"val_accum"`      // total valdator accum held by validators
+	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
 	CommunityPool DecCoins   `json:"community_pool"` // pool for community funds yet to be spent
 }
 
 // update total validator accumulation factor
+// NOTE: Do not call this except from ValidatorDistInfo.TakeFeePoolRewards().
 func (f FeePool) UpdateTotalValAccum(height int64, totalBondedTokens sdk.Dec) FeePool {
-	f.ValAccum = f.ValAccum.UpdateForNewHeight(height, totalBondedTokens)
+	f.TotalValAccum = f.TotalValAccum.UpdateForNewHeight_DEBUG(height, totalBondedTokens)
 	return f
 }
 
 // zero fee pool
 func InitialFeePool() FeePool {
 	return FeePool{
-		ValAccum:      NewTotalAccum(0),
+		TotalValAccum: NewTotalAccum(0),
 		Pool:          DecCoins{},
 		CommunityPool: DecCoins{},
 	}

x/distribution/types/fee_pool_test.go

@@ -23,8 +23,8 @@ func TestUpdateTotalValAccum(t *testing.T) {
 	fp := InitialFeePool()
 
 	fp = fp.UpdateTotalValAccum(5, sdk.NewDec(3))
-	require.True(sdk.DecEq(t, sdk.NewDec(15), fp.ValAccum.Accum))
+	require.True(sdk.DecEq(t, sdk.NewDec(15), fp.TotalValAccum.Accum))
 
 	fp = fp.UpdateTotalValAccum(8, sdk.NewDec(2))
-	require.True(sdk.DecEq(t, sdk.NewDec(21), fp.ValAccum.Accum))
+	require.True(sdk.DecEq(t, sdk.NewDec(21), fp.TotalValAccum.Accum))
 }

x/distribution/types/keepers.go

@@ -10,6 +10,8 @@ type StakeKeeper interface {
 	Validator(ctx sdk.Context, valAddr sdk.ValAddress) sdk.Validator
 	ValidatorByConsAddr(ctx sdk.Context, consAddr sdk.ConsAddress) sdk.Validator
 	TotalPower(ctx sdk.Context) sdk.Dec
+	GetLastTotalPower(ctx sdk.Context) sdk.Dec
+	GetLastValidatorPower(ctx sdk.Context, valAddr sdk.ValAddress) sdk.Dec
 }
 
 // expected coin keeper

x/distribution/types/validator_info.go

@@ -1,7 +1,10 @@
 package types
 
 import (
+	"fmt"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	cmn "github.com/tendermint/tendermint/libs/common"
 )
 
 // distribution info for a particular validator
@@ -19,9 +22,9 @@ func NewValidatorDistInfo(operatorAddr sdk.ValAddress, currentHeight int64) Vali
 	return ValidatorDistInfo{
 		OperatorAddr:            operatorAddr,
 		FeePoolWithdrawalHeight: currentHeight,
-		Pool:                    DecCoins{},
-		PoolCommission:          DecCoins{},
-		DelAccum:                NewTotalAccum(currentHeight),
+		Pool:           DecCoins{},
+		PoolCommission: DecCoins{},
+		DelAccum:       NewTotalAccum(currentHeight),
 	}
 }
 
@@ -31,30 +34,52 @@ func (vi ValidatorDistInfo) UpdateTotalDelAccum(height int64, totalDelShares sdk
 	return vi
 }
 
-// move any available accumulated fees in the FeePool to the validator's pool
+// Move any available accumulated fees in the FeePool to the validator's pool.
+// * updates validator info's FeePoolWithdrawalHeight, thus setting accum to 0.
+// * updates fee pool to latest height and total val accum w/ given totalBonded.
+// This is the only way to update the FeePool's validator TotalAccum.
+// NOTE: This algorithm works as long as TakeFeePoolRewards is called after every power change.
+// - called in ValidationDistInfo.WithdrawCommission.
+// - called in DelegationDistInfo.WithdrawRewards.
+// NOTE: When a delegator unbonds, say, onDelegationSharesModified ->
+//       WithdrawDelegationReward -> WithdrawRewards.
 func (vi ValidatorDistInfo) TakeFeePoolRewards(fp FeePool, height int64, totalBonded, vdTokens,
 	commissionRate sdk.Dec) (ValidatorDistInfo, FeePool) {
 
 	fp = fp.UpdateTotalValAccum(height, totalBonded)
 
-	if fp.ValAccum.Accum.IsZero() {
+	if fp.TotalValAccum.Accum.IsZero() {
 		return vi, fp
 	}
 
 	// update the validators pool
 	blocks := height - vi.FeePoolWithdrawalHeight
 	vi.FeePoolWithdrawalHeight = height
 	accum := vdTokens.MulInt(sdk.NewInt(blocks))
-	if accum.GT(fp.ValAccum.Accum) {
+
+	if !accum.IsZero() {
+		fmt.Println(
+			cmn.Red(
+				fmt.Sprintf("FP Sub %v * %v = %v, %v - _ => %v",
+					vdTokens.String(), sdk.NewInt(blocks),
+					accum.String(),
+					fp.TotalValAccum.Accum.String(),
+					fp.TotalValAccum.Accum.Sub(accum).String(),
+				),
+			),
+		)
+	}
+
+	if accum.GT(fp.TotalValAccum.Accum) {
 		panic("individual accum should never be greater than the total")
 	}
-	withdrawalTokens := fp.Pool.MulDec(accum).QuoDec(fp.ValAccum.Accum)
+	withdrawalTokens := fp.Pool.MulDec(accum).QuoDec(fp.TotalValAccum.Accum)
 	remainingTokens := fp.Pool.Minus(withdrawalTokens)
 
 	commission := withdrawalTokens.MulDec(commissionRate)
 	afterCommission := withdrawalTokens.Minus(commission)
 
-	fp.ValAccum.Accum = fp.ValAccum.Accum.Sub(accum)
+	fp.TotalValAccum.Accum = fp.TotalValAccum.Accum.Sub(accum)
 	fp.Pool = remainingTokens
 	vi.PoolCommission = vi.PoolCommission.Plus(commission)
 	vi.Pool = vi.Pool.Plus(afterCommission)