From 076011a0f6e8fc92d3d739d7d79f8a2d889fa4c8 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Mon, 22 Oct 2018 22:17:52 -0400 Subject: [PATCH 01/24] add GetAccum for validator and fee pool --- x/distribution/keeper/keeper.go | 11 +++++++++++ x/distribution/keeper/validator.go | 16 ++++++++++++++++ x/distribution/types/fee_pool.go | 17 ++++++++++++++++- x/distribution/types/validator_info.go | 15 ++++++++++----- 4 files changed, 53 insertions(+), 6 deletions(-) diff --git a/x/distribution/keeper/keeper.go b/x/distribution/keeper/keeper.go index 0ccf76ca63db..3919a18aedf7 100644 --- a/x/distribution/keeper/keeper.go +++ b/x/distribution/keeper/keeper.go @@ -55,6 +55,17 @@ func (k Keeper) SetFeePool(ctx sdk.Context, feePool types.FeePool) { store.Set(FeePoolKey, b) } +// get the total validator accum for the ctx height +// in the fee pool +func (k Keeper) GetFeePoolValAccum(ctx sdk.Context) sdk.Dec { + + // withdraw self-delegation + height := ctx.BlockHeight() + totalPower := k.stakeKeeper.GetLastTotalPower(ctx) + fp := k.GetFeePool(ctx) + return fp.GetTotalValAccum(height, totalPower) +} + //______________________________________________________________________ // set the proposer public key for this block diff --git a/x/distribution/keeper/validator.go b/x/distribution/keeper/validator.go index a71249d6b4ce..db3d1a9f0e36 100644 --- a/x/distribution/keeper/validator.go +++ b/x/distribution/keeper/validator.go @@ -40,6 +40,22 @@ func (k Keeper) RemoveValidatorDistInfo(ctx sdk.Context, valAddr sdk.ValAddress) store.Delete(GetValidatorDistInfoKey(valAddr)) } +// Get the calculated accum of a validator at the current block +// without affecting the state. +func (k Keeper) GetValidatorAccum(ctx sdk.Context, operatorAddr sdk.ValAddress) (sdk.Dec, sdk.Error) { + if !k.HasValidatorDistInfo(ctx, operatorAddr) { + return sdk.Dec{}, types.ErrNoValidatorDistInfo(k.codespace) + } + + // withdraw self-delegation + height := ctx.BlockHeight() + lastValPower := k.stakeKeeper.GetLastValidatorPower(ctx, operatorAddr) + valInfo := k.GetValidatorDistInfo(ctx, operatorAddr) + accum := valInfo.GetAccum(height, lastValPower) + + return accum, nil +} + // withdrawal all the validator rewards including the commission func (k Keeper) WithdrawValidatorRewardsAll(ctx sdk.Context, operatorAddr sdk.ValAddress) sdk.Error { diff --git a/x/distribution/types/fee_pool.go b/x/distribution/types/fee_pool.go index ae1d72cc0117..def6c6e75f2f 100644 --- a/x/distribution/types/fee_pool.go +++ b/x/distribution/types/fee_pool.go @@ -17,7 +17,7 @@ func NewTotalAccum(height int64) TotalAccum { } } -// update total validator accumulation factor for the new height +// update total accumulation factor for the new height // CONTRACT: height should be greater than the old height func (ta TotalAccum) UpdateForNewHeight(height int64, accumCreatedPerBlock sdk.Dec) TotalAccum { blocks := height - ta.UpdateHeight @@ -29,6 +29,16 @@ func (ta TotalAccum) UpdateForNewHeight(height int64, accumCreatedPerBlock sdk.D return ta } +// get total accumulation factor for the given height +// CONTRACT: height should be greater than the old height +func (ta TotalAccum) GetAccum(height int64, accumCreatedPerBlock sdk.Dec) sdk.Dec { + blocks := height - ta.UpdateHeight + if blocks < 0 { + panic("reverse updated for new height") + } + return ta.Accum.Add(accumCreatedPerBlock.MulInt(sdk.NewInt(blocks))) +} + //___________________________________________________________________________________________ // global fee pool for distribution @@ -45,6 +55,11 @@ func (f FeePool) UpdateTotalValAccum(height int64, totalBondedTokens sdk.Dec) Fe return f } +// get the total validator accum for the fee pool without modifying the state +func (f FeePool) GetTotalValAccum(height int64, totalBondedTokens sdk.Dec) sdk.Dec { + return f.TotalValAccum.GetAccum(height, totalBondedTokens) +} + // zero fee pool func InitialFeePool() FeePool { return FeePool{ diff --git a/x/distribution/types/validator_info.go b/x/distribution/types/validator_info.go index 5fc86efcd5ea..3caea5d8469b 100644 --- a/x/distribution/types/validator_info.go +++ b/x/distribution/types/validator_info.go @@ -19,9 +19,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,6 +31,12 @@ func (vi ValidatorDistInfo) UpdateTotalDelAccum(height int64, totalDelShares sdk return vi } +// Get the calculated accum of this validator at the provided height +func (vi ValidatorDistInfo) GetAccum(height int64, vdTokens sdk.Dec) sdk.Dec { + blocks := height - vi.FeePoolWithdrawalHeight + return vdTokens.MulInt(sdk.NewInt(blocks)) +} + // 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 @@ -50,9 +56,8 @@ func (vi ValidatorDistInfo) TakeFeePoolRewards(fp FeePool, height int64, totalBo } // update the validators pool - blocks := height - vi.FeePoolWithdrawalHeight + accum := vi.GetAccum(height, vdTokens) vi.FeePoolWithdrawalHeight = height - accum := vdTokens.MulInt(sdk.NewInt(blocks)) if accum.GT(fp.TotalValAccum.Accum) { panic("individual accum should never be greater than the total") From 6755ec73d2f9bbd10461011e82117a10ccf82ede Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Wed, 24 Oct 2018 19:20:52 -0400 Subject: [PATCH 02/24] refactor accum --- x/distribution/keeper/delegation.go | 73 ++++++++++++------------ x/distribution/types/delegator_info.go | 27 ++++++++- x/distribution/types/fee_pool.go | 37 ------------ x/distribution/types/fee_pool_test.go | 11 ---- x/distribution/types/total_accum.go | 40 +++++++++++++ x/distribution/types/total_accum_test.go | 19 ++++++ x/distribution/types/validator_info.go | 46 ++++++++++++++- 7 files changed, 162 insertions(+), 91 deletions(-) create mode 100644 x/distribution/types/total_accum.go create mode 100644 x/distribution/types/total_accum_test.go diff --git a/x/distribution/keeper/delegation.go b/x/distribution/keeper/delegation.go index c3a6cfa54e68..3c71e8784084 100644 --- a/x/distribution/keeper/delegation.go +++ b/x/distribution/keeper/delegation.go @@ -79,21 +79,12 @@ func (k Keeper) WithdrawDelegationReward(ctx sdk.Context, delegatorAddr sdk.AccA return types.ErrNoDelegationDistInfo(k.codespace) } - // TODO: Reconcile with duplicate code in getDelegatorRewardsAll. - height := ctx.BlockHeight() - lastTotalPower := sdk.NewDecFromInt(k.stakeKeeper.GetLastTotalPower(ctx)) - lastValPower := k.stakeKeeper.GetLastValidatorPower(ctx, valAddr) - feePool := k.GetFeePool(ctx) - 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, lastTotalPower, - lastValPower, validator.GetDelegatorShares(), delegation.GetShares(), validator.GetCommission()) + feePool, valInfo, delInfo, withdraw := + withdrawDelegatorReward(ctx, delAddr, height, lastTotalPower) k.SetValidatorDistInfo(ctx, valInfo) k.SetDelegationDistInfo(ctx, delInfo) + withdrawAddr := k.GetDelegatorWithdrawAddr(ctx, delegatorAddr) coinsToAdd, change := withdraw.TruncateDecimal() feePool.CommunityPool = feePool.CommunityPool.Plus(change) @@ -110,10 +101,26 @@ func (k Keeper) WithdrawDelegationReward(ctx sdk.Context, delegatorAddr sdk.AccA // return all rewards for all delegations of a delegator func (k Keeper) WithdrawDelegationRewardsAll(ctx sdk.Context, delegatorAddr sdk.AccAddress) { height := ctx.BlockHeight() - withdraw := k.getDelegatorRewardsAll(ctx, delegatorAddr, height) - feePool := k.GetFeePool(ctx) + + // iterate over all the delegations + // TODO: Reconcile with duplicate code in WithdrawDelegationReward. + withdraw := types.DecCoins{} + lastTotalPower := sdk.NewDecFromInt(k.stakeKeeper.GetLastTotalPower(ctx)) + operationAtDelegation := func(_ int64, del sdk.Delegation) (stop bool) { + feePool, valInfo, delInfo, diWithdraw := + withdrawDelegatorReward(ctx, delAddr, height, lastTotalPower) + + withdraw = withdraw.Plus(diWithdraw) + k.SetFeePool(ctx, feePool) + k.SetValidatorDistInfo(ctx, valInfo) + k.SetDelegationDistInfo(ctx, delInfo) + return false + } + k.stakeKeeper.IterateDelegations(ctx, delAddr, operationAtDelegation) + withdrawAddr := k.GetDelegatorWithdrawAddr(ctx, delegatorAddr) coinsToAdd, change := withdraw.TruncateDecimal() + feePool := k.GetFeePool(ctx) feePool.CommunityPool = feePool.CommunityPool.Plus(change) k.SetFeePool(ctx, feePool) _, _, err := k.bankKeeper.AddCoins(ctx, withdrawAddr, coinsToAdd) @@ -123,30 +130,20 @@ func (k Keeper) WithdrawDelegationRewardsAll(ctx sdk.Context, delegatorAddr sdk. } // return all rewards for all delegations of a delegator -func (k Keeper) getDelegatorRewardsAll(ctx sdk.Context, delAddr sdk.AccAddress, height int64) types.DecCoins { +func (k Keeper) withdrawDelegatorReward(ctx sdk.Context, delAddr sdk.AccAddress, + height int64, lastTotalPower sdk.Dec) ( + feePool, ValidatorDistInfo, DelegatorDistInfo, types.DecCoins) { - withdraw := types.DecCoins{} - lastTotalPower := sdk.NewDecFromInt(k.stakeKeeper.GetLastTotalPower(ctx)) + feePool := k.GetFeePool(ctx) + 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) - // 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.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, lastTotalPower, - lastValPower, validator.GetDelegatorShares(), delegation.GetShares(), validator.GetCommission()) - withdraw = withdraw.Plus(diWithdraw) - k.SetFeePool(ctx, feePool) - k.SetValidatorDistInfo(ctx, valInfo) - k.SetDelegationDistInfo(ctx, delInfo) - return false - } - k.stakeKeeper.IterateDelegations(ctx, delAddr, operationAtDelegation) - return withdraw + delInfo, valInfo, feePool, withdraw := delInfo.WithdrawRewards(feePool, valInfo, height, lastTotalPower, + lastValPower, validator.GetDelegatorShares(), delegation.GetShares(), validator.GetCommission()) + + return feePool, valInfo, delInfo, withdraw } diff --git a/x/distribution/types/delegator_info.go b/x/distribution/types/delegator_info.go index 7c2eaef76221..14e1a788c8f6 100644 --- a/x/distribution/types/delegator_info.go +++ b/x/distribution/types/delegator_info.go @@ -21,6 +21,12 @@ func NewDelegationDistInfo(delegatorAddr sdk.AccAddress, valOperatorAddr sdk.Val } } +// Get the calculated accum of this delegator at the provided height +func (di DelegationDistInfo) GetDelAccum(height int64, delegatorShares sdk.Dec) sdk.Dec { + blocks := height - di.WithdrawalHeight + return delegatorShares.MulInt(sdk.NewInt(blocks)) +} + // Withdraw rewards from delegator. // Among many things, it does: // * updates validator info's total del accum @@ -40,9 +46,8 @@ func (di DelegationDistInfo) WithdrawRewards(fp FeePool, vi ValidatorDistInfo, vi, fp = vi.TakeFeePoolRewards(fp, height, totalBonded, vdTokens, commissionRate) - blocks := height - di.WithdrawalHeight + accum := di.GetDelAccum(height, delegatorShares) di.WithdrawalHeight = height - accum := delegatorShares.MulInt(sdk.NewInt(blocks)) withdrawalTokens := vi.Pool.MulDec(accum).QuoDec(vi.DelAccum.Accum) remainingTokens := vi.Pool.Minus(withdrawalTokens) @@ -51,3 +56,21 @@ func (di DelegationDistInfo) WithdrawRewards(fp FeePool, vi ValidatorDistInfo, return di, vi, fp, withdrawalTokens } + +// Estimate the delegators rewards at this current state, +// the estimated rewards are subject to fluctuation +func (di DelegationDistInfo) EstimateRewards(fp FeePool, vi ValidatorDistInfo, + height int64, totalBonded, vdTokens, totalDelShares, delegatorShares, + commissionRate sdk.Dec) DecCoins { + + totalDelAccum = GetTotalDelAccum(height, totalDelShares) + + if vi.DelAccum.Accum.IsZero() { + return DecCoins{} + } + + rewards = vi.EstimatePoolRewards(fp, height, totalBonded, vdTokens, commissionRate) + accum := di.GetDelAccum(height, delegatorShares) + estimatedTokens := rewards.MulDec(accum).QuoDec(totalDelAccum) + return estimatedTokens +} diff --git a/x/distribution/types/fee_pool.go b/x/distribution/types/fee_pool.go index def6c6e75f2f..1f109b6278d1 100644 --- a/x/distribution/types/fee_pool.go +++ b/x/distribution/types/fee_pool.go @@ -4,43 +4,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) -// total accumulation tracker -type TotalAccum struct { - UpdateHeight int64 `json:"update_height"` - Accum sdk.Dec `json:"accum"` -} - -func NewTotalAccum(height int64) TotalAccum { - return TotalAccum{ - UpdateHeight: height, - Accum: sdk.ZeroDec(), - } -} - -// update total accumulation factor for the new height -// CONTRACT: height should be greater than the old height -func (ta TotalAccum) UpdateForNewHeight(height int64, accumCreatedPerBlock sdk.Dec) TotalAccum { - blocks := height - ta.UpdateHeight - if blocks < 0 { - panic("reverse updated for new height") - } - ta.Accum = ta.Accum.Add(accumCreatedPerBlock.MulInt(sdk.NewInt(blocks))) - ta.UpdateHeight = height - return ta -} - -// get total accumulation factor for the given height -// CONTRACT: height should be greater than the old height -func (ta TotalAccum) GetAccum(height int64, accumCreatedPerBlock sdk.Dec) sdk.Dec { - blocks := height - ta.UpdateHeight - if blocks < 0 { - panic("reverse updated for new height") - } - return ta.Accum.Add(accumCreatedPerBlock.MulInt(sdk.NewInt(blocks))) -} - -//___________________________________________________________________________________________ - // global fee pool for distribution type FeePool struct { TotalValAccum TotalAccum `json:"val_accum"` // total valdator accum held by validators diff --git a/x/distribution/types/fee_pool_test.go b/x/distribution/types/fee_pool_test.go index 478ec7539baa..73bda52fabf0 100644 --- a/x/distribution/types/fee_pool_test.go +++ b/x/distribution/types/fee_pool_test.go @@ -7,17 +7,6 @@ import ( "github.com/stretchr/testify/require" ) -func TestTotalAccumUpdateForNewHeight(t *testing.T) { - - ta := NewTotalAccum(0) - - ta = ta.UpdateForNewHeight(5, sdk.NewDec(3)) - require.True(sdk.DecEq(t, sdk.NewDec(15), ta.Accum)) - - ta = ta.UpdateForNewHeight(8, sdk.NewDec(2)) - require.True(sdk.DecEq(t, sdk.NewDec(21), ta.Accum)) -} - func TestUpdateTotalValAccum(t *testing.T) { fp := InitialFeePool() diff --git a/x/distribution/types/total_accum.go b/x/distribution/types/total_accum.go new file mode 100644 index 000000000000..0915847a5ef6 --- /dev/null +++ b/x/distribution/types/total_accum.go @@ -0,0 +1,40 @@ +package types + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" +) + +// total accumulation tracker +type TotalAccum struct { + UpdateHeight int64 `json:"update_height"` + Accum sdk.Dec `json:"accum"` +} + +func NewTotalAccum(height int64) TotalAccum { + return TotalAccum{ + UpdateHeight: height, + Accum: sdk.ZeroDec(), + } +} + +// update total accumulation factor for the new height +// CONTRACT: height should be greater than the old height +func (ta TotalAccum) UpdateForNewHeight(height int64, accumCreatedPerBlock sdk.Dec) TotalAccum { + blocks := height - ta.UpdateHeight + if blocks < 0 { + panic("reverse updated for new height") + } + ta.Accum = ta.Accum.Add(accumCreatedPerBlock.MulInt(sdk.NewInt(blocks))) + ta.UpdateHeight = height + return ta +} + +// get total accumulation factor for the given height +// CONTRACT: height should be greater than the old height +func (ta TotalAccum) GetAccum(height int64, accumCreatedPerBlock sdk.Dec) sdk.Dec { + blocks := height - ta.UpdateHeight + if blocks < 0 { + panic("reverse updated for new height") + } + return ta.Accum.Add(accumCreatedPerBlock.MulInt(sdk.NewInt(blocks))) +} diff --git a/x/distribution/types/total_accum_test.go b/x/distribution/types/total_accum_test.go new file mode 100644 index 000000000000..81f80a154a0e --- /dev/null +++ b/x/distribution/types/total_accum_test.go @@ -0,0 +1,19 @@ +package types + +import ( + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/require" +) + +func TestTotalAccumUpdateForNewHeight(t *testing.T) { + + ta := NewTotalAccum(0) + + ta = ta.UpdateForNewHeight(5, sdk.NewDec(3)) + require.True(sdk.DecEq(t, sdk.NewDec(15), ta.Accum)) + + ta = ta.UpdateForNewHeight(8, sdk.NewDec(2)) + require.True(sdk.DecEq(t, sdk.NewDec(21), ta.Accum)) +} diff --git a/x/distribution/types/validator_info.go b/x/distribution/types/validator_info.go index 3caea5d8469b..b45238bee4f7 100644 --- a/x/distribution/types/validator_info.go +++ b/x/distribution/types/validator_info.go @@ -31,8 +31,13 @@ func (vi ValidatorDistInfo) UpdateTotalDelAccum(height int64, totalDelShares sdk return vi } -// Get the calculated accum of this validator at the provided height -func (vi ValidatorDistInfo) GetAccum(height int64, vdTokens sdk.Dec) sdk.Dec { +// Get the total delegator accum within this validator at the provided height +func (vi ValidatorDistInfo) GetTotalDelAccum(height int64, totalDelShares sdk.Dec) sdk.Dec { + return vi.DelAccum.GetAccum(height, totalDelShares) +} + +// Get the validator accum at the provided height +func (vi ValidatorDistInfo) GetValAccum(height int64, vdTokens sdk.Dec) sdk.Dec { blocks := height - vi.FeePoolWithdrawalHeight return vdTokens.MulInt(sdk.NewInt(blocks)) } @@ -56,7 +61,7 @@ func (vi ValidatorDistInfo) TakeFeePoolRewards(fp FeePool, height int64, totalBo } // update the validators pool - accum := vi.GetAccum(height, vdTokens) + accum := vi.GetValAccum(height, vdTokens) vi.FeePoolWithdrawalHeight = height if accum.GT(fp.TotalValAccum.Accum) { @@ -87,3 +92,38 @@ func (vi ValidatorDistInfo) WithdrawCommission(fp FeePool, height int64, return vi, fp, withdrawalTokens } + +// Estimate the validator's pool rewards at this current state, +// the estimated rewards are subject to fluctuation +func (vi ValidatorDistInfo) EstimatePoolRewards(fp FeePool, height int64, + totalBonded, vdTokens, commissionRate sdk.Dec) sdk.Coins { + + totalValAccum = fp.GetTotalValAccum(height, totalBonded) + valAccum := vi.GetValAccum(height, vdTokens) + + if accum.GT(fp.TotalValAccum.Accum) { + panic("individual accum should never be greater than the total") + } + withdrawalTokens := fp.Pool.MulDec(valAccum).QuoDec(totalValAccum) + commission := withdrawalTokens.MulDec(commissionRate) + afterCommission := withdrawalTokens.Minus(commission) + pool := vi.Pool.Plus(afterCommission) + return pool +} + +// Estimate the validator's commission pool rewards at this current state, +// the estimated rewards are subject to fluctuation +func (vi ValidatorDistInfo) EstimateCommissionRewards(fp FeePool, height int64, + totalBonded, vdTokens, commissionRate sdk.Dec) sdk.Coins { + + totalValAccum = fp.GetTotalValAccum(height, totalBonded) + valAccum := vi.GetValAccum(height, vdTokens) + + if accum.GT(fp.TotalValAccum.Accum) { + panic("individual accum should never be greater than the total") + } + withdrawalTokens := fp.Pool.MulDec(valAccum).QuoDec(totalValAccum) + commission := withdrawalTokens.MulDec(commissionRate) + commissionPool := vi.PoolCommission.Plus(commission) + return commissionPool +} From 25cbbd503c771d7fe9f0a85d098774cf401f4f12 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Thu, 25 Oct 2018 04:51:37 -0400 Subject: [PATCH 03/24] refactoring working --- x/distribution/keeper/delegation.go | 87 ++++++++++++++++++++------ x/distribution/keeper/keeper.go | 17 +++++ x/distribution/keeper/test_common.go | 21 +++++++ x/distribution/keeper/validator.go | 43 ++++++------- x/distribution/types/delegator_info.go | 28 ++++----- x/distribution/types/validator_info.go | 63 +++++++++++++------ 6 files changed, 185 insertions(+), 74 deletions(-) diff --git a/x/distribution/keeper/delegation.go b/x/distribution/keeper/delegation.go index 3c71e8784084..fb37dd06f1a5 100644 --- a/x/distribution/keeper/delegation.go +++ b/x/distribution/keeper/delegation.go @@ -69,7 +69,44 @@ func (k Keeper) RemoveDelegatorWithdrawAddr(ctx sdk.Context, delAddr, withdrawAd //___________________________________________________________________________________________ -// Withdraw all the rewards for a single delegation +// return all rewards for a delegation +func (k Keeper) withdrawDelegationReward(ctx sdk.Context, delAddr sdk.AccAddress, + height int64, lastTotalPower sdk.Dec) ( + feePool, ValidatorDistInfo, DelegatorDistInfo, types.DecCoins) { + + valAddr := del.GetValidatorAddr() + wc := k.GetWithdrawContext(ctx, valAddr) + + delInfo := k.GetDelegationDistInfo(ctx, delAddr, valAddr) + validator := k.stakeKeeper.Validator(ctx, valAddr) + delegation := k.stakeKeeper.Delegation(ctx, delAddr, valAddr) + + delInfo, valInfo, feePool, withdraw := delInfo.WithdrawRewards(wc, + validator.GetDelegatorShares(), delegation.GetShares()) + + return feePool, valInfo, delInfo, withdraw +} + +// estimate all rewards for all delegations of a delegator +func (k Keeper) estimateDelegationReward(ctx sdk.Context, delAddr sdk.AccAddress, + height int64, lastTotalPower sdk.Dec) types.DecCoins { + + valAddr := del.GetValidatorAddr() + wc := k.GetWithdrawContext(ctx, valAddr) + + delInfo := k.GetDelegationDistInfo(ctx, delAddr, valAddr) + validator := k.stakeKeeper.Validator(ctx, valAddr) + delegation := k.stakeKeeper.Delegation(ctx, delAddr, valAddr) + + estimation := delInfo.EstimateRewards(wc, + validator.GetDelegatorShares(), delegation.GetShares()) + + return estimation +} + +//___________________________________________________________________________________________ + +// withdraw all 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, @@ -80,7 +117,7 @@ func (k Keeper) WithdrawDelegationReward(ctx sdk.Context, delegatorAddr sdk.AccA } feePool, valInfo, delInfo, withdraw := - withdrawDelegatorReward(ctx, delAddr, height, lastTotalPower) + withdrawDelegationReward(ctx, delAddr, height, lastTotalPower) k.SetValidatorDistInfo(ctx, valInfo) k.SetDelegationDistInfo(ctx, delInfo) @@ -96,6 +133,17 @@ func (k Keeper) WithdrawDelegationReward(ctx sdk.Context, delegatorAddr sdk.AccA return nil } +// estimate rewards for a single delegation +func (k Keeper) EstimateDelegationReward(ctx sdk.Context, delegatorAddr sdk.AccAddress, + valAddr sdk.ValAddress) (sdk.Coins, sdk.Error) { + + if !k.HasDelegationDistInfo(ctx, delegatorAddr, valAddr) { + return types.ErrNoDelegationDistInfo(k.codespace) + } + estCoins := estimateDelegationReward(ctx, delAddr, height, lastTotalPower) + return estCoins.TruncateDecimal() +} + //___________________________________________________________________________________________ // return all rewards for all delegations of a delegator @@ -103,13 +151,12 @@ func (k Keeper) WithdrawDelegationRewardsAll(ctx sdk.Context, delegatorAddr sdk. height := ctx.BlockHeight() // iterate over all the delegations - // TODO: Reconcile with duplicate code in WithdrawDelegationReward. withdraw := types.DecCoins{} lastTotalPower := sdk.NewDecFromInt(k.stakeKeeper.GetLastTotalPower(ctx)) operationAtDelegation := func(_ int64, del sdk.Delegation) (stop bool) { - feePool, valInfo, delInfo, diWithdraw := - withdrawDelegatorReward(ctx, delAddr, height, lastTotalPower) + feePool, valInfo, delInfo, diWithdraw := + withdrawDelegationReward(ctx, delAddr, height, lastTotalPower) withdraw = withdraw.Plus(diWithdraw) k.SetFeePool(ctx, feePool) k.SetValidatorDistInfo(ctx, valInfo) @@ -129,21 +176,21 @@ func (k Keeper) WithdrawDelegationRewardsAll(ctx sdk.Context, delegatorAddr sdk. } } -// return all rewards for all delegations of a delegator -func (k Keeper) withdrawDelegatorReward(ctx sdk.Context, delAddr sdk.AccAddress, - height int64, lastTotalPower sdk.Dec) ( - feePool, ValidatorDistInfo, DelegatorDistInfo, types.DecCoins) { +// estimate all rewards for all delegations of a delegator +func (k Keeper) EstimateDelegationRewardsAll(ctx sdk.Context, + delegatorAddr sdk.AccAddress) sdk.Coins { - feePool := k.GetFeePool(ctx) - 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, withdraw := delInfo.WithdrawRewards(feePool, valInfo, height, lastTotalPower, - lastValPower, validator.GetDelegatorShares(), delegation.GetShares(), validator.GetCommission()) + height := ctx.BlockHeight() - return feePool, valInfo, delInfo, withdraw + // iterate over all the delegations + total := types.DecCoins{} + lastTotalPower := sdk.NewDecFromInt(k.stakeKeeper.GetLastTotalPower(ctx)) + operationAtDelegation := func(_ int64, del sdk.Delegation) (stop bool) { + est := estimateDelegationReward(ctx, delAddr, height, lastTotalPower) + total = total.Plus(est) + return false + } + k.stakeKeeper.IterateDelegations(ctx, delAddr, operationAtDelegation) + estCoins, _ := total.TruncateDecimal() + return estCoins } diff --git a/x/distribution/keeper/keeper.go b/x/distribution/keeper/keeper.go index 3919a18aedf7..4480b9e70f8b 100644 --- a/x/distribution/keeper/keeper.go +++ b/x/distribution/keeper/keeper.go @@ -88,6 +88,23 @@ func (k Keeper) SetPreviousProposerConsAddr(ctx sdk.Context, consAddr sdk.ConsAd store.Set(ProposerKey, b) } +//______________________________________________________________________ + +// get context required for withdraw operations +func (k Keeper) GetWithdrawContext(ctx sdk.Context, + operatorAddr sdk.ValAddr) types.WithdrawContext { + + feePool := k.GetFeePool(ctx) + height := ctx.BlockHeight() + validator := k.stakeKeeper.Validator(ctx, operatorAddr) + lastValPower := k.stakeKeeper.GetLastValidatorPower(ctx, operatorAddr) + lastTotalPower := sdk.NewDecFromInt(k.stakeKeeper.GetLastTotalPower(ctx)) + + return types.NewWithdrawContext( + feePool, height, lastTotalPower, lastValPower, + validator.GetCommission()) +} + //______________________________________________________________________ // PARAM STORE diff --git a/x/distribution/keeper/test_common.go b/x/distribution/keeper/test_common.go index 58658ab8e0ac..b1bd4ae4e05c 100644 --- a/x/distribution/keeper/test_common.go +++ b/x/distribution/keeper/test_common.go @@ -158,3 +158,24 @@ func (fck DummyFeeCollectionKeeper) SetCollectedFees(in sdk.Coins) { func (fck DummyFeeCollectionKeeper) ClearCollectedFees(_ sdk.Context) { heldFees = sdk.Coins{} } + +//__________________________________________________________________________________ +// used in simulation + +// iterate over all the validator distribution infos (inefficient, just used to check invariants) +func (k Keeper) IterateValidatorDistInfos(ctx sdk.Context, + fn func(index int64, distInfo types.ValidatorDistInfo) (stop bool)) { + + store := ctx.KVStore(k.storeKey) + iter := sdk.KVStorePrefixIterator(store, ValidatorDistInfoKey) + defer iter.Close() + index := int64(0) + for ; iter.Valid(); iter.Next() { + var vdi types.ValidatorDistInfo + k.cdc.MustUnmarshalBinary(iter.Value(), &vdi) + if fn(index, vdi) { + return + } + index++ + } +} diff --git a/x/distribution/keeper/validator.go b/x/distribution/keeper/validator.go index db3d1a9f0e36..4e40cf117ee6 100644 --- a/x/distribution/keeper/validator.go +++ b/x/distribution/keeper/validator.go @@ -62,20 +62,15 @@ func (k Keeper) WithdrawValidatorRewardsAll(ctx sdk.Context, operatorAddr sdk.Va if !k.HasValidatorDistInfo(ctx, operatorAddr) { return types.ErrNoValidatorDistInfo(k.codespace) } + wc := k.GetWithdrawContext(ctx, operatorAddr) // 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) + withdraw := k.withdrawDelegatorRewardsAll(ctx, accAddr, wc.Height) // withdrawal validator commission rewards - lastTotalPower := sdk.NewDecFromInt(k.stakeKeeper.GetLastTotalPower(ctx)) valInfo := k.GetValidatorDistInfo(ctx, operatorAddr) - feePool := k.GetFeePool(ctx) - valInfo, feePool, commission := valInfo.WithdrawCommission(feePool, height, lastTotalPower, - lastValPower, validator.GetCommission()) + valInfo, feePool, commission := valInfo.WithdrawCommission(wb) withdraw = withdraw.Plus(commission) k.SetValidatorDistInfo(ctx, valInfo) @@ -91,18 +86,24 @@ func (k Keeper) WithdrawValidatorRewardsAll(ctx sdk.Context, operatorAddr sdk.Va return nil } -// iterate over all the validator distribution infos (inefficient, just used to check invariants) -func (k Keeper) IterateValidatorDistInfos(ctx sdk.Context, fn func(index int64, distInfo types.ValidatorDistInfo) (stop bool)) { - store := ctx.KVStore(k.storeKey) - iter := sdk.KVStorePrefixIterator(store, ValidatorDistInfoKey) - defer iter.Close() - index := int64(0) - for ; iter.Valid(); iter.Next() { - var vdi types.ValidatorDistInfo - k.cdc.MustUnmarshalBinary(iter.Value(), &vdi) - if fn(index, vdi) { - return - } - index++ +// estimate all the validator rewards including the commission +// note: all estimations are subject to flucuation +func (k Keeper) EstimateValidatorRewardsAll(ctx sdk.Context, operatorAddr sdk.ValAddress) sdk.Error { + + if !k.HasValidatorDistInfo(ctx, operatorAddr) { + return types.ErrNoValidatorDistInfo(k.codespace) } + wc := k.GetWithdrawContext(ctx, operatorAddr) + + // withdraw self-delegation + accAddr := sdk.AccAddress(operatorAddr.Bytes()) + withdraw := k.EstimateDelegatorRewardsAll(ctx, accAddr, wc.Height) + + // withdrawal validator commission rewards + valInfo := k.GetValidatorDistInfo(ctx, operatorAddr) + + commission := valInfo.EstimateCommission(wc) + withdraw = withdraw.Plus(commission) + truncated, _ := withdraw.TruncateDecimal() + return truncated } diff --git a/x/distribution/types/delegator_info.go b/x/distribution/types/delegator_info.go index 14e1a788c8f6..615e611a7371 100644 --- a/x/distribution/types/delegator_info.go +++ b/x/distribution/types/delegator_info.go @@ -34,20 +34,21 @@ func (di DelegationDistInfo) GetDelAccum(height int64, delegatorShares sdk.Dec) // * 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) { +func (di DelegationDistInfo) WithdrawRewards(wc WithdrawContext, vi ValidatorDistInfo, + totalDelShares, delegatorShares sdk.Dec) ( + DelegationDistInfo, ValidatorDistInfo, FeePool, DecCoins) { - vi = vi.UpdateTotalDelAccum(height, totalDelShares) + fp := wc.FeePool + vi = vi.UpdateTotalDelAccum(wc.Height, totalDelShares) if vi.DelAccum.Accum.IsZero() { return di, vi, fp, DecCoins{} } - vi, fp = vi.TakeFeePoolRewards(fp, height, totalBonded, vdTokens, commissionRate) + vi, fp = vi.TakeFeePoolRewards(wc) - accum := di.GetDelAccum(height, delegatorShares) - di.WithdrawalHeight = height + accum := di.GetDelAccum(wc.Height, delegatorShares) + di.WithdrawalHeight = wc.Height withdrawalTokens := vi.Pool.MulDec(accum).QuoDec(vi.DelAccum.Accum) remainingTokens := vi.Pool.Minus(withdrawalTokens) @@ -58,19 +59,18 @@ func (di DelegationDistInfo) WithdrawRewards(fp FeePool, vi ValidatorDistInfo, } // Estimate the delegators rewards at this current state, -// the estimated rewards are subject to fluctuation -func (di DelegationDistInfo) EstimateRewards(fp FeePool, vi ValidatorDistInfo, - height int64, totalBonded, vdTokens, totalDelShares, delegatorShares, - commissionRate sdk.Dec) DecCoins { +// note: all estimations are subject to flucuation +func (di DelegationDistInfo) EstimateRewards(wc WithdrawContext, vi ValidatorDistInfo, + totalDelShares, delegatorShares sdk.Dec) DecCoins { - totalDelAccum = GetTotalDelAccum(height, totalDelShares) + totalDelAccum := vi.GetTotalDelAccum(wc.Height, totalDelShares) if vi.DelAccum.Accum.IsZero() { return DecCoins{} } - rewards = vi.EstimatePoolRewards(fp, height, totalBonded, vdTokens, commissionRate) - accum := di.GetDelAccum(height, delegatorShares) + rewards := vi.EstimatePoolRewards(wc) + accum := di.GetDelAccum(wc.Height, delegatorShares) estimatedTokens := rewards.MulDec(accum).QuoDec(totalDelAccum) return estimatedTokens } diff --git a/x/distribution/types/validator_info.go b/x/distribution/types/validator_info.go index b45238bee4f7..166fda6f20f9 100644 --- a/x/distribution/types/validator_info.go +++ b/x/distribution/types/validator_info.go @@ -4,6 +4,29 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) +// common parameters used in withdraws from validators +type WithdrawContext struct { + FeePool FeePool + Height int64 // block height + TotalBonded sdk.Dec // total bonded tokens in the network + ValTokens sdk.Dec // validator's bonded tokens + CommissionRate sdk.Dec // validator commission rate +} + +func NewWithdrawContext(feePool FeePool, height int64, totalBonded, + valTokens, commissionRate sdk.Dec) WithdrawContext { + + return WithdrawContext{ + fp: feePool, + height: height, + totalBonded: totalBonded, + vdTokens: vdTokens, + commissionRate: commissionRate, + } +} + +//_____________________________________________________________________________ + // distribution info for a particular validator type ValidatorDistInfo struct { OperatorAddr sdk.ValAddress `json:"operator_addr"` @@ -51,10 +74,10 @@ func (vi ValidatorDistInfo) GetValAccum(height int64, vdTokens sdk.Dec) sdk.Dec // - 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) { +func (vi ValidatorDistInfo) TakeFeePoolRewards(wc WithdrawContext) ( + ValidatorDistInfo, FeePool) { - fp = fp.UpdateTotalValAccum(height, totalBonded) + fp := wc.FeePool.UpdateTotalValAccum(height, totalBonded) if fp.TotalValAccum.Accum.IsZero() { return vi, fp @@ -64,13 +87,13 @@ func (vi ValidatorDistInfo) TakeFeePoolRewards(fp FeePool, height int64, totalBo accum := vi.GetValAccum(height, vdTokens) vi.FeePoolWithdrawalHeight = height - if accum.GT(fp.TotalValAccum.Accum) { + if accum.GT(wc.fp.TotalValAccum.Accum) { panic("individual accum should never be greater than the total") } withdrawalTokens := fp.Pool.MulDec(accum).QuoDec(fp.TotalValAccum.Accum) remainingTokens := fp.Pool.Minus(withdrawalTokens) - commission := withdrawalTokens.MulDec(commissionRate) + commission := withdrawalTokens.MulDec(wc.CommissionRate) afterCommission := withdrawalTokens.Minus(commission) fp.TotalValAccum.Accum = fp.TotalValAccum.Accum.Sub(accum) @@ -82,10 +105,10 @@ func (vi ValidatorDistInfo) TakeFeePoolRewards(fp FeePool, height int64, totalBo } // withdraw commission rewards -func (vi ValidatorDistInfo) WithdrawCommission(fp FeePool, height int64, - totalBonded, vdTokens, commissionRate sdk.Dec) (vio ValidatorDistInfo, fpo FeePool, withdrawn DecCoins) { +func (vi ValidatorDistInfo) WithdrawCommission(wc WithdrawContext) ( + vio ValidatorDistInfo, fpo FeePool, withdrawn DecCoins) { - vi, fp = vi.TakeFeePoolRewards(fp, height, totalBonded, vdTokens, commissionRate) + vi, fp = vi.TakeFeePoolRewards(wc) withdrawalTokens := vi.PoolCommission vi.PoolCommission = DecCoins{} // zero @@ -95,17 +118,18 @@ func (vi ValidatorDistInfo) WithdrawCommission(fp FeePool, height int64, // Estimate the validator's pool rewards at this current state, // the estimated rewards are subject to fluctuation -func (vi ValidatorDistInfo) EstimatePoolRewards(fp FeePool, height int64, - totalBonded, vdTokens, commissionRate sdk.Dec) sdk.Coins { +func (vi ValidatorDistInfo) EstimatePoolRewards( + wc WithdrawContext) sdk.Coins { - totalValAccum = fp.GetTotalValAccum(height, totalBonded) - valAccum := vi.GetValAccum(height, vdTokens) + fp := wc.FeePool + totalValAccum = fp.GetTotalValAccum(wc.height, wc.TotalBonded) + valAccum := vi.GetValAccum(wc.height, wc.ValTokens) if accum.GT(fp.TotalValAccum.Accum) { panic("individual accum should never be greater than the total") } withdrawalTokens := fp.Pool.MulDec(valAccum).QuoDec(totalValAccum) - commission := withdrawalTokens.MulDec(commissionRate) + commission := withdrawalTokens.MulDec(wc.CommissionRate) afterCommission := withdrawalTokens.Minus(commission) pool := vi.Pool.Plus(afterCommission) return pool @@ -113,17 +137,18 @@ func (vi ValidatorDistInfo) EstimatePoolRewards(fp FeePool, height int64, // Estimate the validator's commission pool rewards at this current state, // the estimated rewards are subject to fluctuation -func (vi ValidatorDistInfo) EstimateCommissionRewards(fp FeePool, height int64, - totalBonded, vdTokens, commissionRate sdk.Dec) sdk.Coins { +func (vi ValidatorDistInfo) EstimateCommissionRewards( + wc WithdrawContext) sdk.Coins { - totalValAccum = fp.GetTotalValAccum(height, totalBonded) - valAccum := vi.GetValAccum(height, vdTokens) + fp := wc.FeePool + totalValAccum = fp.GetTotalValAccum(wc.Height, wc.TotalBonded) + valAccum := vi.GetValAccum(wc.Height, wc.ValTokens) if accum.GT(fp.TotalValAccum.Accum) { panic("individual accum should never be greater than the total") } - withdrawalTokens := fp.Pool.MulDec(valAccum).QuoDec(totalValAccum) - commission := withdrawalTokens.MulDec(commissionRate) + withdrawalTokens := fp.Pool.MulDec(valAccum).QuoDec(wc.TotalValAccum) + commission := withdrawalTokens.MulDec(wc.CommissionRate) commissionPool := vi.PoolCommission.Plus(commission) return commissionPool } From d93a44f90abb2eefe9b2566253432b3758ac5f62 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Thu, 25 Oct 2018 13:07:30 -0400 Subject: [PATCH 04/24] refactor WIP --- x/distribution/keeper/delegation.go | 91 ++++++++++++++------------ x/distribution/keeper/keeper.go | 6 +- x/distribution/keeper/validator.go | 16 ++--- x/distribution/types/validator_info.go | 46 ++++++------- 4 files changed, 80 insertions(+), 79 deletions(-) diff --git a/x/distribution/keeper/delegation.go b/x/distribution/keeper/delegation.go index fb37dd06f1a5..1ba82c9c6c65 100644 --- a/x/distribution/keeper/delegation.go +++ b/x/distribution/keeper/delegation.go @@ -70,18 +70,18 @@ func (k Keeper) RemoveDelegatorWithdrawAddr(ctx sdk.Context, delAddr, withdrawAd //___________________________________________________________________________________________ // return all rewards for a delegation -func (k Keeper) withdrawDelegationReward(ctx sdk.Context, delAddr sdk.AccAddress, - height int64, lastTotalPower sdk.Dec) ( - feePool, ValidatorDistInfo, DelegatorDistInfo, types.DecCoins) { +func (k Keeper) withdrawDelegationReward(ctx sdk.Context, + delAddr sdk.AccAddress, valAddr sdk.ValAddress) (types.FeePool, + types.ValidatorDistInfo, types.DelegationDistInfo, types.DecCoins) { - valAddr := del.GetValidatorAddr() wc := k.GetWithdrawContext(ctx, valAddr) + valInfo := k.GetValidatorDistInfo(ctx, valAddr) delInfo := k.GetDelegationDistInfo(ctx, delAddr, valAddr) validator := k.stakeKeeper.Validator(ctx, valAddr) delegation := k.stakeKeeper.Delegation(ctx, delAddr, valAddr) - delInfo, valInfo, feePool, withdraw := delInfo.WithdrawRewards(wc, + delInfo, valInfo, feePool, withdraw := delInfo.WithdrawRewards(wc, valInfo, validator.GetDelegatorShares(), delegation.GetShares()) return feePool, valInfo, delInfo, withdraw @@ -89,16 +89,16 @@ func (k Keeper) withdrawDelegationReward(ctx sdk.Context, delAddr sdk.AccAddress // estimate all rewards for all delegations of a delegator func (k Keeper) estimateDelegationReward(ctx sdk.Context, delAddr sdk.AccAddress, - height int64, lastTotalPower sdk.Dec) types.DecCoins { + valAddr sdk.ValAddress) types.DecCoins { - valAddr := del.GetValidatorAddr() wc := k.GetWithdrawContext(ctx, valAddr) + valInfo := k.GetValidatorDistInfo(ctx, valAddr) delInfo := k.GetDelegationDistInfo(ctx, delAddr, valAddr) validator := k.stakeKeeper.Validator(ctx, valAddr) delegation := k.stakeKeeper.Delegation(ctx, delAddr, valAddr) - estimation := delInfo.EstimateRewards(wc, + estimation := delInfo.EstimateRewards(wc, valInfo, validator.GetDelegatorShares(), delegation.GetShares()) return estimation @@ -109,54 +109,71 @@ func (k Keeper) estimateDelegationReward(ctx sdk.Context, delAddr sdk.AccAddress // withdraw all 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, +func (k Keeper) CompleteWithdrawal(ctx sdk.Context, feePool types.FeePool, + delAddr sdk.AccAddress, amount types.DecCoins) { + + withdrawAddr := k.GetDelegatorWithdrawAddr(ctx, delAddr) + coinsToAdd, change := amount.TruncateDecimal() + feePool.CommunityPool = feePool.CommunityPool.Plus(change) + k.SetFeePool(ctx, feePool) + _, _, err := k.bankKeeper.AddCoins(ctx, withdrawAddr, coinsToAdd) + if err != nil { + panic(err) + } +} + +//___________________________________________________________________________________________ + +// withdraw all rewards for a single delegation +// NOTE: This gets called "onDelegationSharesModified", +// meaning any changes to bonded coins +func (k Keeper) WithdrawDelegationReward(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) sdk.Error { - if !k.HasDelegationDistInfo(ctx, delegatorAddr, valAddr) { + if !k.HasDelegationDistInfo(ctx, delAddr, valAddr) { return types.ErrNoDelegationDistInfo(k.codespace) } feePool, valInfo, delInfo, withdraw := - withdrawDelegationReward(ctx, delAddr, height, lastTotalPower) + k.withdrawDelegationReward(ctx, delAddr, valAddr) k.SetValidatorDistInfo(ctx, valInfo) k.SetDelegationDistInfo(ctx, delInfo) - - withdrawAddr := k.GetDelegatorWithdrawAddr(ctx, delegatorAddr) - coinsToAdd, change := withdraw.TruncateDecimal() - feePool.CommunityPool = feePool.CommunityPool.Plus(change) - k.SetFeePool(ctx, feePool) - _, _, err := k.bankKeeper.AddCoins(ctx, withdrawAddr, coinsToAdd) - if err != nil { - panic(err) - } + k.CompleteWithdrawal(ctx, feePool, delAddr, withdraw) return nil } // estimate rewards for a single delegation -func (k Keeper) EstimateDelegationReward(ctx sdk.Context, delegatorAddr sdk.AccAddress, +func (k Keeper) EstimateDelegationReward(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) (sdk.Coins, sdk.Error) { - if !k.HasDelegationDistInfo(ctx, delegatorAddr, valAddr) { - return types.ErrNoDelegationDistInfo(k.codespace) + if !k.HasDelegationDistInfo(ctx, delAddr, valAddr) { + return sdk.Coins{}, types.ErrNoDelegationDistInfo(k.codespace) } - estCoins := estimateDelegationReward(ctx, delAddr, height, lastTotalPower) - return estCoins.TruncateDecimal() + estCoins := k.estimateDelegationReward(ctx, delAddr, valAddr) + trucate, _ := estCoins.TruncateDecimal() + return trucate, nil } //___________________________________________________________________________________________ // return all rewards for all delegations of a delegator -func (k Keeper) WithdrawDelegationRewardsAll(ctx sdk.Context, delegatorAddr sdk.AccAddress) { - height := ctx.BlockHeight() +func (k Keeper) WithdrawDelegationRewardsAll(ctx sdk.Context, delAddr sdk.AccAddress) { + withdraw := k.withdrawDelegationRewardsAll(ctx, delAddr) + feePool := k.GetFeePool(ctx) + k.CompleteWithdrawal(ctx, feePool, delAddr, withdraw) +} + +func (k Keeper) withdrawDelegationRewardsAll(ctx sdk.Context, + delAddr sdk.AccAddress) types.DecCoins { // iterate over all the delegations withdraw := types.DecCoins{} - lastTotalPower := sdk.NewDecFromInt(k.stakeKeeper.GetLastTotalPower(ctx)) operationAtDelegation := func(_ int64, del sdk.Delegation) (stop bool) { + valAddr := del.GetValidatorAddr() feePool, valInfo, delInfo, diWithdraw := - withdrawDelegationReward(ctx, delAddr, height, lastTotalPower) + k.withdrawDelegationReward(ctx, delAddr, valAddr) withdraw = withdraw.Plus(diWithdraw) k.SetFeePool(ctx, feePool) k.SetValidatorDistInfo(ctx, valInfo) @@ -164,21 +181,12 @@ func (k Keeper) WithdrawDelegationRewardsAll(ctx sdk.Context, delegatorAddr sdk. return false } k.stakeKeeper.IterateDelegations(ctx, delAddr, operationAtDelegation) - - withdrawAddr := k.GetDelegatorWithdrawAddr(ctx, delegatorAddr) - coinsToAdd, change := withdraw.TruncateDecimal() - feePool := k.GetFeePool(ctx) - feePool.CommunityPool = feePool.CommunityPool.Plus(change) - k.SetFeePool(ctx, feePool) - _, _, err := k.bankKeeper.AddCoins(ctx, withdrawAddr, coinsToAdd) - if err != nil { - panic(err) - } + return withdraw } // estimate all rewards for all delegations of a delegator func (k Keeper) EstimateDelegationRewardsAll(ctx sdk.Context, - delegatorAddr sdk.AccAddress) sdk.Coins { + delAddr sdk.AccAddress) sdk.Coins { height := ctx.BlockHeight() @@ -186,7 +194,8 @@ func (k Keeper) EstimateDelegationRewardsAll(ctx sdk.Context, total := types.DecCoins{} lastTotalPower := sdk.NewDecFromInt(k.stakeKeeper.GetLastTotalPower(ctx)) operationAtDelegation := func(_ int64, del sdk.Delegation) (stop bool) { - est := estimateDelegationReward(ctx, delAddr, height, lastTotalPower) + valAddr := del.GetValidatorAddr() + est := k.estimateDelegationReward(ctx, delAddr, valAddr) total = total.Plus(est) return false } diff --git a/x/distribution/keeper/keeper.go b/x/distribution/keeper/keeper.go index 4480b9e70f8b..73477c474e3b 100644 --- a/x/distribution/keeper/keeper.go +++ b/x/distribution/keeper/keeper.go @@ -92,12 +92,12 @@ func (k Keeper) SetPreviousProposerConsAddr(ctx sdk.Context, consAddr sdk.ConsAd // get context required for withdraw operations func (k Keeper) GetWithdrawContext(ctx sdk.Context, - operatorAddr sdk.ValAddr) types.WithdrawContext { + valOperatorAddr sdk.ValAddress) types.WithdrawContext { feePool := k.GetFeePool(ctx) height := ctx.BlockHeight() - validator := k.stakeKeeper.Validator(ctx, operatorAddr) - lastValPower := k.stakeKeeper.GetLastValidatorPower(ctx, operatorAddr) + validator := k.stakeKeeper.Validator(ctx, valOperatorAddr) + lastValPower := k.stakeKeeper.GetLastValidatorPower(ctx, valOperatorAddr) lastTotalPower := sdk.NewDecFromInt(k.stakeKeeper.GetLastTotalPower(ctx)) return types.NewWithdrawContext( diff --git a/x/distribution/keeper/validator.go b/x/distribution/keeper/validator.go index 4e40cf117ee6..f27301794d91 100644 --- a/x/distribution/keeper/validator.go +++ b/x/distribution/keeper/validator.go @@ -62,27 +62,19 @@ func (k Keeper) WithdrawValidatorRewardsAll(ctx sdk.Context, operatorAddr sdk.Va if !k.HasValidatorDistInfo(ctx, operatorAddr) { return types.ErrNoValidatorDistInfo(k.codespace) } - wc := k.GetWithdrawContext(ctx, operatorAddr) // withdraw self-delegation accAddr := sdk.AccAddress(operatorAddr.Bytes()) - withdraw := k.withdrawDelegatorRewardsAll(ctx, accAddr, wc.Height) + withdraw := k.withdrawDelegationRewardsAll(ctx, accAddr) // withdrawal validator commission rewards valInfo := k.GetValidatorDistInfo(ctx, operatorAddr) - valInfo, feePool, commission := valInfo.WithdrawCommission(wb) + wc := k.GetWithdrawContext(ctx, operatorAddr) + valInfo, feePool, commission := valInfo.WithdrawCommission(wc) withdraw = withdraw.Plus(commission) k.SetValidatorDistInfo(ctx, valInfo) - withdrawAddr := k.GetDelegatorWithdrawAddr(ctx, accAddr) - truncated, change := withdraw.TruncateDecimal() - feePool.CommunityPool = feePool.CommunityPool.Plus(change) - k.SetFeePool(ctx, feePool) - _, _, err := k.bankKeeper.AddCoins(ctx, withdrawAddr, truncated) - if err != nil { - panic(err) - } - + k.CompleteWithdrawal(ctx, feePool, accAddr, withdraw) return nil } diff --git a/x/distribution/types/validator_info.go b/x/distribution/types/validator_info.go index 166fda6f20f9..705f44102523 100644 --- a/x/distribution/types/validator_info.go +++ b/x/distribution/types/validator_info.go @@ -8,20 +8,20 @@ import ( type WithdrawContext struct { FeePool FeePool Height int64 // block height - TotalBonded sdk.Dec // total bonded tokens in the network - ValTokens sdk.Dec // validator's bonded tokens + TotalPower sdk.Dec // total bonded tokens in the network + ValPower sdk.Dec // validator's bonded tokens CommissionRate sdk.Dec // validator commission rate } -func NewWithdrawContext(feePool FeePool, height int64, totalBonded, - valTokens, commissionRate sdk.Dec) WithdrawContext { +func NewWithdrawContext(feePool FeePool, height int64, totalPower, + valPower, commissionRate sdk.Dec) WithdrawContext { return WithdrawContext{ - fp: feePool, - height: height, - totalBonded: totalBonded, - vdTokens: vdTokens, - commissionRate: commissionRate, + FeePool: feePool, + Height: height, + TotalPower: totalPower, + ValPower: valPower, + CommissionRate: commissionRate, } } @@ -77,17 +77,17 @@ func (vi ValidatorDistInfo) GetValAccum(height int64, vdTokens sdk.Dec) sdk.Dec func (vi ValidatorDistInfo) TakeFeePoolRewards(wc WithdrawContext) ( ValidatorDistInfo, FeePool) { - fp := wc.FeePool.UpdateTotalValAccum(height, totalBonded) + fp := wc.FeePool.UpdateTotalValAccum(wc.Height, wc.TotalPower) if fp.TotalValAccum.Accum.IsZero() { return vi, fp } // update the validators pool - accum := vi.GetValAccum(height, vdTokens) - vi.FeePoolWithdrawalHeight = height + accum := vi.GetValAccum(wc.Height, wc.ValPower) + vi.FeePoolWithdrawalHeight = wc.Height - if accum.GT(wc.fp.TotalValAccum.Accum) { + 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) @@ -108,7 +108,7 @@ func (vi ValidatorDistInfo) TakeFeePoolRewards(wc WithdrawContext) ( func (vi ValidatorDistInfo) WithdrawCommission(wc WithdrawContext) ( vio ValidatorDistInfo, fpo FeePool, withdrawn DecCoins) { - vi, fp = vi.TakeFeePoolRewards(wc) + vi, fp := vi.TakeFeePoolRewards(wc) withdrawalTokens := vi.PoolCommission vi.PoolCommission = DecCoins{} // zero @@ -119,13 +119,13 @@ func (vi ValidatorDistInfo) WithdrawCommission(wc WithdrawContext) ( // Estimate the validator's pool rewards at this current state, // the estimated rewards are subject to fluctuation func (vi ValidatorDistInfo) EstimatePoolRewards( - wc WithdrawContext) sdk.Coins { + wc WithdrawContext) DecCoins { fp := wc.FeePool - totalValAccum = fp.GetTotalValAccum(wc.height, wc.TotalBonded) - valAccum := vi.GetValAccum(wc.height, wc.ValTokens) + totalValAccum := fp.GetTotalValAccum(wc.Height, wc.TotalPower) + valAccum := vi.GetValAccum(wc.Height, wc.ValPower) - if accum.GT(fp.TotalValAccum.Accum) { + if valAccum.GT(totalValAccum) { panic("individual accum should never be greater than the total") } withdrawalTokens := fp.Pool.MulDec(valAccum).QuoDec(totalValAccum) @@ -138,16 +138,16 @@ func (vi ValidatorDistInfo) EstimatePoolRewards( // Estimate the validator's commission pool rewards at this current state, // the estimated rewards are subject to fluctuation func (vi ValidatorDistInfo) EstimateCommissionRewards( - wc WithdrawContext) sdk.Coins { + wc WithdrawContext) DecCoins { fp := wc.FeePool - totalValAccum = fp.GetTotalValAccum(wc.Height, wc.TotalBonded) - valAccum := vi.GetValAccum(wc.Height, wc.ValTokens) + totalValAccum := fp.GetTotalValAccum(wc.Height, wc.TotalPower) + valAccum := vi.GetValAccum(wc.Height, wc.ValPower) - if accum.GT(fp.TotalValAccum.Accum) { + if valAccum.GT(totalValAccum) { panic("individual accum should never be greater than the total") } - withdrawalTokens := fp.Pool.MulDec(valAccum).QuoDec(wc.TotalValAccum) + withdrawalTokens := fp.Pool.MulDec(valAccum).QuoDec(totalValAccum) commission := withdrawalTokens.MulDec(wc.CommissionRate) commissionPool := vi.PoolCommission.Plus(commission) return commissionPool From 631215a767c5c441a5832df184bdb2ae701693fb Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Thu, 25 Oct 2018 13:25:52 -0400 Subject: [PATCH 05/24] Estimate -> Current, wip refactor --- x/distribution/keeper/delegation.go | 18 +++++++++--------- x/distribution/keeper/validator.go | 13 ++++++------- x/distribution/types/delegator_info.go | 11 +++++------ x/distribution/types/validator_info.go | 10 ++++------ 4 files changed, 24 insertions(+), 28 deletions(-) diff --git a/x/distribution/keeper/delegation.go b/x/distribution/keeper/delegation.go index 1ba82c9c6c65..ba9f9015a0ea 100644 --- a/x/distribution/keeper/delegation.go +++ b/x/distribution/keeper/delegation.go @@ -87,8 +87,8 @@ func (k Keeper) withdrawDelegationReward(ctx sdk.Context, return feePool, valInfo, delInfo, withdraw } -// estimate all rewards for all delegations of a delegator -func (k Keeper) estimateDelegationReward(ctx sdk.Context, delAddr sdk.AccAddress, +// get all rewards for all delegations of a delegator +func (k Keeper) currentDelegationReward(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) types.DecCoins { wc := k.GetWithdrawContext(ctx, valAddr) @@ -98,7 +98,7 @@ func (k Keeper) estimateDelegationReward(ctx sdk.Context, delAddr sdk.AccAddress validator := k.stakeKeeper.Validator(ctx, valAddr) delegation := k.stakeKeeper.Delegation(ctx, delAddr, valAddr) - estimation := delInfo.EstimateRewards(wc, valInfo, + estimation := delInfo.CurrentRewards(wc, valInfo, validator.GetDelegatorShares(), delegation.GetShares()) return estimation @@ -143,14 +143,14 @@ func (k Keeper) WithdrawDelegationReward(ctx sdk.Context, delAddr sdk.AccAddress return nil } -// estimate rewards for a single delegation -func (k Keeper) EstimateDelegationReward(ctx sdk.Context, delAddr sdk.AccAddress, +// current rewards for a single delegation +func (k Keeper) CurrentDelegationReward(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) (sdk.Coins, sdk.Error) { if !k.HasDelegationDistInfo(ctx, delAddr, valAddr) { return sdk.Coins{}, types.ErrNoDelegationDistInfo(k.codespace) } - estCoins := k.estimateDelegationReward(ctx, delAddr, valAddr) + estCoins := k.currentDelegationReward(ctx, delAddr, valAddr) trucate, _ := estCoins.TruncateDecimal() return trucate, nil } @@ -184,8 +184,8 @@ func (k Keeper) withdrawDelegationRewardsAll(ctx sdk.Context, return withdraw } -// estimate all rewards for all delegations of a delegator -func (k Keeper) EstimateDelegationRewardsAll(ctx sdk.Context, +// get all rewards for all delegations of a delegator +func (k Keeper) CurrentDelegationRewardsAll(ctx sdk.Context, delAddr sdk.AccAddress) sdk.Coins { height := ctx.BlockHeight() @@ -195,7 +195,7 @@ func (k Keeper) EstimateDelegationRewardsAll(ctx sdk.Context, lastTotalPower := sdk.NewDecFromInt(k.stakeKeeper.GetLastTotalPower(ctx)) operationAtDelegation := func(_ int64, del sdk.Delegation) (stop bool) { valAddr := del.GetValidatorAddr() - est := k.estimateDelegationReward(ctx, delAddr, valAddr) + est := k.currentDelegationReward(ctx, delAddr, valAddr) total = total.Plus(est) return false } diff --git a/x/distribution/keeper/validator.go b/x/distribution/keeper/validator.go index f27301794d91..150cc3bd10a8 100644 --- a/x/distribution/keeper/validator.go +++ b/x/distribution/keeper/validator.go @@ -51,7 +51,7 @@ func (k Keeper) GetValidatorAccum(ctx sdk.Context, operatorAddr sdk.ValAddress) height := ctx.BlockHeight() lastValPower := k.stakeKeeper.GetLastValidatorPower(ctx, operatorAddr) valInfo := k.GetValidatorDistInfo(ctx, operatorAddr) - accum := valInfo.GetAccum(height, lastValPower) + accum := valInfo.GetValAccum(height, lastValPower) return accum, nil } @@ -78,23 +78,22 @@ func (k Keeper) WithdrawValidatorRewardsAll(ctx sdk.Context, operatorAddr sdk.Va return nil } -// estimate all the validator rewards including the commission -// note: all estimations are subject to flucuation -func (k Keeper) EstimateValidatorRewardsAll(ctx sdk.Context, operatorAddr sdk.ValAddress) sdk.Error { +// get all the validator rewards including the commission +func (k Keeper) CurrentValidatorRewardsAll(ctx sdk.Context, operatorAddr sdk.ValAddress) sdk.Error { if !k.HasValidatorDistInfo(ctx, operatorAddr) { return types.ErrNoValidatorDistInfo(k.codespace) } - wc := k.GetWithdrawContext(ctx, operatorAddr) // withdraw self-delegation accAddr := sdk.AccAddress(operatorAddr.Bytes()) - withdraw := k.EstimateDelegatorRewardsAll(ctx, accAddr, wc.Height) + withdraw := k.CurrentDelegationRewardsAll(ctx, accAddr) // withdrawal validator commission rewards valInfo := k.GetValidatorDistInfo(ctx, operatorAddr) - commission := valInfo.EstimateCommission(wc) + wc := k.GetWithdrawContext(ctx, operatorAddr) + commission := valInfo.CurrentCommissionRewards(wc) withdraw = withdraw.Plus(commission) truncated, _ := withdraw.TruncateDecimal() return truncated diff --git a/x/distribution/types/delegator_info.go b/x/distribution/types/delegator_info.go index 615e611a7371..66f386a94521 100644 --- a/x/distribution/types/delegator_info.go +++ b/x/distribution/types/delegator_info.go @@ -58,9 +58,8 @@ func (di DelegationDistInfo) WithdrawRewards(wc WithdrawContext, vi ValidatorDis return di, vi, fp, withdrawalTokens } -// Estimate the delegators rewards at this current state, -// note: all estimations are subject to flucuation -func (di DelegationDistInfo) EstimateRewards(wc WithdrawContext, vi ValidatorDistInfo, +// get the delegators rewards at this current state, +func (di DelegationDistInfo) CurrentRewards(wc WithdrawContext, vi ValidatorDistInfo, totalDelShares, delegatorShares sdk.Dec) DecCoins { totalDelAccum := vi.GetTotalDelAccum(wc.Height, totalDelShares) @@ -69,8 +68,8 @@ func (di DelegationDistInfo) EstimateRewards(wc WithdrawContext, vi ValidatorDis return DecCoins{} } - rewards := vi.EstimatePoolRewards(wc) + rewards := vi.CurrentPoolRewards(wc) accum := di.GetDelAccum(wc.Height, delegatorShares) - estimatedTokens := rewards.MulDec(accum).QuoDec(totalDelAccum) - return estimatedTokens + tokens := rewards.MulDec(accum).QuoDec(totalDelAccum) + return tokens } diff --git a/x/distribution/types/validator_info.go b/x/distribution/types/validator_info.go index 705f44102523..933ea98b1fa1 100644 --- a/x/distribution/types/validator_info.go +++ b/x/distribution/types/validator_info.go @@ -116,9 +116,8 @@ func (vi ValidatorDistInfo) WithdrawCommission(wc WithdrawContext) ( return vi, fp, withdrawalTokens } -// Estimate the validator's pool rewards at this current state, -// the estimated rewards are subject to fluctuation -func (vi ValidatorDistInfo) EstimatePoolRewards( +// get the validator's pool rewards at this current state, +func (vi ValidatorDistInfo) CurrentPoolRewards( wc WithdrawContext) DecCoins { fp := wc.FeePool @@ -135,9 +134,8 @@ func (vi ValidatorDistInfo) EstimatePoolRewards( return pool } -// Estimate the validator's commission pool rewards at this current state, -// the estimated rewards are subject to fluctuation -func (vi ValidatorDistInfo) EstimateCommissionRewards( +// get the validator's commission pool rewards at this current state, +func (vi ValidatorDistInfo) CurrentCommissionRewards( wc WithdrawContext) DecCoins { fp := wc.FeePool From eda3463cf2cdddc5771e57ab134fefbf6d95d1cd Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Thu, 25 Oct 2018 13:50:00 -0400 Subject: [PATCH 06/24] core code compiling --- x/distribution/keeper/delegation.go | 14 +++++--------- x/distribution/keeper/keeper.go | 2 +- x/distribution/keeper/validator.go | 8 ++++---- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/x/distribution/keeper/delegation.go b/x/distribution/keeper/delegation.go index ba9f9015a0ea..d133d4e9112f 100644 --- a/x/distribution/keeper/delegation.go +++ b/x/distribution/keeper/delegation.go @@ -109,7 +109,7 @@ func (k Keeper) currentDelegationReward(ctx sdk.Context, delAddr sdk.AccAddress, // withdraw all rewards for a single delegation // NOTE: This gets called "onDelegationSharesModified", // meaning any changes to bonded coins -func (k Keeper) CompleteWithdrawal(ctx sdk.Context, feePool types.FeePool, +func (k Keeper) WithdrawToDelegator(ctx sdk.Context, feePool types.FeePool, delAddr sdk.AccAddress, amount types.DecCoins) { withdrawAddr := k.GetDelegatorWithdrawAddr(ctx, delAddr) @@ -139,7 +139,7 @@ func (k Keeper) WithdrawDelegationReward(ctx sdk.Context, delAddr sdk.AccAddress k.SetValidatorDistInfo(ctx, valInfo) k.SetDelegationDistInfo(ctx, delInfo) - k.CompleteWithdrawal(ctx, feePool, delAddr, withdraw) + k.WithdrawToDelegator(ctx, feePool, delAddr, withdraw) return nil } @@ -161,7 +161,7 @@ func (k Keeper) CurrentDelegationReward(ctx sdk.Context, delAddr sdk.AccAddress, func (k Keeper) WithdrawDelegationRewardsAll(ctx sdk.Context, delAddr sdk.AccAddress) { withdraw := k.withdrawDelegationRewardsAll(ctx, delAddr) feePool := k.GetFeePool(ctx) - k.CompleteWithdrawal(ctx, feePool, delAddr, withdraw) + k.WithdrawToDelegator(ctx, feePool, delAddr, withdraw) } func (k Keeper) withdrawDelegationRewardsAll(ctx sdk.Context, @@ -186,13 +186,10 @@ func (k Keeper) withdrawDelegationRewardsAll(ctx sdk.Context, // get all rewards for all delegations of a delegator func (k Keeper) CurrentDelegationRewardsAll(ctx sdk.Context, - delAddr sdk.AccAddress) sdk.Coins { - - height := ctx.BlockHeight() + delAddr sdk.AccAddress) types.DecCoins { // iterate over all the delegations total := types.DecCoins{} - lastTotalPower := sdk.NewDecFromInt(k.stakeKeeper.GetLastTotalPower(ctx)) operationAtDelegation := func(_ int64, del sdk.Delegation) (stop bool) { valAddr := del.GetValidatorAddr() est := k.currentDelegationReward(ctx, delAddr, valAddr) @@ -200,6 +197,5 @@ func (k Keeper) CurrentDelegationRewardsAll(ctx sdk.Context, return false } k.stakeKeeper.IterateDelegations(ctx, delAddr, operationAtDelegation) - estCoins, _ := total.TruncateDecimal() - return estCoins + return total } diff --git a/x/distribution/keeper/keeper.go b/x/distribution/keeper/keeper.go index 73477c474e3b..ab299a9b8d43 100644 --- a/x/distribution/keeper/keeper.go +++ b/x/distribution/keeper/keeper.go @@ -61,7 +61,7 @@ func (k Keeper) GetFeePoolValAccum(ctx sdk.Context) sdk.Dec { // withdraw self-delegation height := ctx.BlockHeight() - totalPower := k.stakeKeeper.GetLastTotalPower(ctx) + totalPower := sdk.NewDecFromInt(k.stakeKeeper.GetLastTotalPower(ctx)) fp := k.GetFeePool(ctx) return fp.GetTotalValAccum(height, totalPower) } diff --git a/x/distribution/keeper/validator.go b/x/distribution/keeper/validator.go index 150cc3bd10a8..cf6c1c11d9dc 100644 --- a/x/distribution/keeper/validator.go +++ b/x/distribution/keeper/validator.go @@ -74,15 +74,15 @@ func (k Keeper) WithdrawValidatorRewardsAll(ctx sdk.Context, operatorAddr sdk.Va withdraw = withdraw.Plus(commission) k.SetValidatorDistInfo(ctx, valInfo) - k.CompleteWithdrawal(ctx, feePool, accAddr, withdraw) + k.WithdrawToDelegator(ctx, feePool, accAddr, withdraw) return nil } // get all the validator rewards including the commission -func (k Keeper) CurrentValidatorRewardsAll(ctx sdk.Context, operatorAddr sdk.ValAddress) sdk.Error { +func (k Keeper) CurrentValidatorRewardsAll(ctx sdk.Context, operatorAddr sdk.ValAddress) (sdk.Coins, sdk.Error) { if !k.HasValidatorDistInfo(ctx, operatorAddr) { - return types.ErrNoValidatorDistInfo(k.codespace) + return sdk.Coins{}, types.ErrNoValidatorDistInfo(k.codespace) } // withdraw self-delegation @@ -96,5 +96,5 @@ func (k Keeper) CurrentValidatorRewardsAll(ctx sdk.Context, operatorAddr sdk.Val commission := valInfo.CurrentCommissionRewards(wc) withdraw = withdraw.Plus(commission) truncated, _ := withdraw.TruncateDecimal() - return truncated + return truncated, nil } From dc2e43016a7ab885376688988b6b52a2044cb4dc Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Thu, 25 Oct 2018 13:57:59 -0400 Subject: [PATCH 07/24] tests compiling/passing --- x/distribution/types/delegator_info_test.go | 12 ++++++++---- x/distribution/types/validator_info_test.go | 15 ++++++++++----- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/x/distribution/types/delegator_info_test.go b/x/distribution/types/delegator_info_test.go index 4af7f0a8f122..1c1511d7314a 100644 --- a/x/distribution/types/delegator_info_test.go +++ b/x/distribution/types/delegator_info_test.go @@ -29,8 +29,10 @@ func TestWithdrawRewards(t *testing.T) { fp.Pool = DecCoins{NewDecCoin("stake", 1000)} // withdraw rewards - di1, vi, fp, rewardRecv1 := di1.WithdrawRewards(fp, vi, height, totalBondedTokens, - validatorTokens, validatorDelShares, di1Shares, commissionRate) + wc := NewWithdrawContext(fp, height, + totalBondedTokens, validatorTokens, commissionRate) + di1, vi, fp, rewardRecv1 := di1.WithdrawRewards(wc, vi, + validatorDelShares, di1Shares) assert.Equal(t, height, di1.WithdrawalHeight) assert.True(sdk.DecEq(t, sdk.NewDec(900), fp.TotalValAccum.Accum)) @@ -44,8 +46,10 @@ func TestWithdrawRewards(t *testing.T) { fp.Pool[0].Amount = fp.Pool[0].Amount.Add(sdk.NewDec(1000)) // withdraw rewards - di2, vi, fp, rewardRecv2 := di2.WithdrawRewards(fp, vi, height, totalBondedTokens, - validatorTokens, validatorDelShares, di2Shares, commissionRate) + wc = NewWithdrawContext(fp, height, + totalBondedTokens, validatorTokens, commissionRate) + di2, vi, fp, rewardRecv2 := di2.WithdrawRewards(wc, vi, + validatorDelShares, di2Shares) assert.Equal(t, height, di2.WithdrawalHeight) assert.True(sdk.DecEq(t, sdk.NewDec(1800), fp.TotalValAccum.Accum)) diff --git a/x/distribution/types/validator_info_test.go b/x/distribution/types/validator_info_test.go index 9d1e39fa6a06..95feaba06063 100644 --- a/x/distribution/types/validator_info_test.go +++ b/x/distribution/types/validator_info_test.go @@ -28,13 +28,15 @@ func TestTakeFeePoolRewards(t *testing.T) { height = 10 fp.Pool = DecCoins{NewDecCoin("stake", 1000)} - vi1, fp = vi1.TakeFeePoolRewards(fp, height, totalBondedTokens, validatorTokens1, commissionRate1) + vi1, fp = vi1.TakeFeePoolRewards(NewWithdrawContext( + 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(100-2), vi1.Pool[0].Amount)) assert.True(sdk.DecEq(t, sdk.NewDec(2), vi1.PoolCommission[0].Amount)) - vi2, fp = vi2.TakeFeePoolRewards(fp, height, totalBondedTokens, validatorTokens2, commissionRate2) + vi2, fp = vi2.TakeFeePoolRewards(NewWithdrawContext( + 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(400-12), vi2.Pool[0].Amount)) @@ -44,7 +46,8 @@ func TestTakeFeePoolRewards(t *testing.T) { height = 20 fp.Pool[0].Amount = fp.Pool[0].Amount.Add(sdk.NewDec(1000)) - vi3, fp = vi3.TakeFeePoolRewards(fp, height, totalBondedTokens, validatorTokens3, commissionRate3) + vi3, fp = vi3.TakeFeePoolRewards(NewWithdrawContext( + 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(1000-40), vi3.Pool[0].Amount)) @@ -66,7 +69,8 @@ func TestWithdrawCommission(t *testing.T) { fp.Pool = DecCoins{NewDecCoin("stake", 1000)} // for a more fun staring condition, have an non-withdraw update - vi, fp = vi.TakeFeePoolRewards(fp, height, totalBondedTokens, validatorTokens, commissionRate) + vi, fp = vi.TakeFeePoolRewards(NewWithdrawContext( + 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(100-2), vi.Pool[0].Amount)) @@ -76,7 +80,8 @@ func TestWithdrawCommission(t *testing.T) { height = 20 fp.Pool[0].Amount = fp.Pool[0].Amount.Add(sdk.NewDec(1000)) - vi, fp, commissionRecv := vi.WithdrawCommission(fp, height, totalBondedTokens, validatorTokens, commissionRate) + vi, fp, commissionRecv := vi.WithdrawCommission(NewWithdrawContext( + 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(200-4), vi.Pool[0].Amount)) From 4b2c76e27dcd88ed4b6c0d94125fc166c3c83257 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Thu, 25 Oct 2018 15:39:45 -0400 Subject: [PATCH 08/24] wip val accum invar --- cmd/gaia/app/sim_test.go | 4 +- x/distribution/simulation/invariants.go | 56 +++++++++++++++++++++ x/mock/simulation/random_simulate_blocks.go | 10 ++-- x/mock/simulation/types.go | 2 +- x/mock/simulation/util.go | 7 ++- 5 files changed, 70 insertions(+), 9 deletions(-) create mode 100644 x/distribution/simulation/invariants.go diff --git a/cmd/gaia/app/sim_test.go b/cmd/gaia/app/sim_test.go index 6b1acbb8bfd9..0f56c5ef49e1 100644 --- a/cmd/gaia/app/sim_test.go +++ b/cmd/gaia/app/sim_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" + abci "github.com/tendermint/tendermint/abci/types" dbm "github.com/tendermint/tendermint/libs/db" "github.com/tendermint/tendermint/libs/log" @@ -123,10 +124,11 @@ func testAndRunTxs(app *GaiaApp) []simulation.WeightedOperation { } } -func invariants(app *GaiaApp) []simulation.Invariant { +func invariants(app *GaiaApp, header abci.Header) []simulation.Invariant { return []simulation.Invariant{ banksim.NonnegativeBalanceInvariant(app.accountKeeper), govsim.AllInvariants(), + distrsim.AllInvariants(app.distrKeeper, header), stakesim.AllInvariants(app.bankKeeper, app.stakeKeeper, app.feeCollectionKeeper, app.distrKeeper, app.accountKeeper), slashingsim.AllInvariants(), } diff --git a/x/distribution/simulation/invariants.go b/x/distribution/simulation/invariants.go new file mode 100644 index 000000000000..277a6a2ff21a --- /dev/null +++ b/x/distribution/simulation/invariants.go @@ -0,0 +1,56 @@ +package simulation + +import ( + "fmt" + + "github.com/cosmos/cosmos-sdk/baseapp" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/distribution" + "github.com/cosmos/cosmos-sdk/x/mock/simulation" + abci "github.com/tendermint/tendermint/abci/types" +) + +// AllInvariants runs all invariants of the distribution module +// Currently: total supply, positive power +func AllInvariants( + d distribution.Keeper, height int64) simulation.Invariant { + + return func(app *baseapp.BaseApp) error { + err := ValAccumInvariants(ck, k, f, d, am)(app) + if err != nil { + return err + } + } +} + +// SupplyInvariants checks that the total supply reflects all held loose tokens, bonded tokens, and unbonding delegations +// nolint: unparam +func ValAccumInvariants(d distribution.Keeper, header abci.Header) simulation.Invariant { + + return func(app *baseapp.BaseApp) error { + ctx := app.NewContext(false, header) + + valAccum := int64(0) + k.IterateValidators(ctx, func(_ int64, validator sdk.Validator) bool { + switch validator.GetStatus() { + case sdk.Bonded: + bonded = bonded.Add(validator.GetPower()) + case sdk.Unbonding: + loose = loose.Add(validator.GetTokens()) + case sdk.Unbonded: + loose = loose.Add(validator.GetTokens()) + } + return false + }) + + //totalBondedTokens := + totalAccum := d.GetFeePool(ctx).GetTotalValAccum(ctx.GetHeight(), totalBondedTokens) + + if totalAccum != valAccum { + fmt.Errorf("validator accum invariance: \n\tfee pool totalAccum: %v"+ + "\n\tvalidator accum \t%v\n", totalAccum, valAccum) + } + + return nil + } +} diff --git a/x/mock/simulation/random_simulate_blocks.go b/x/mock/simulation/random_simulate_blocks.go index 4c9b91d722da..2506af377b68 100644 --- a/x/mock/simulation/random_simulate_blocks.go +++ b/x/mock/simulation/random_simulate_blocks.go @@ -138,7 +138,7 @@ func SimulateFromSeed(tb testing.TB, app *baseapp.BaseApp, if testingMode { // Make sure invariants hold at beginning of block - assertAllInvariants(t, app, invariants, "BeginBlock", displayLogs) + assertAllInvariants(t, app, header, invariants, "BeginBlock", displayLogs) } ctx := app.NewContext(false, header) @@ -150,7 +150,7 @@ func SimulateFromSeed(tb testing.TB, app *baseapp.BaseApp, numQueuedTimeOpsRan := runQueuedTimeOperations(timeOperationQueue, header.Time, tb, r, app, ctx, accs, logWriter, displayLogs, event) if testingMode && onOperation { // Make sure invariants hold at end of queued operations - assertAllInvariants(t, app, invariants, "QueuedOperations", displayLogs) + assertAllInvariants(t, app, header, invariants, "QueuedOperations", displayLogs) } thisBlockSize = thisBlockSize - numQueuedOpsRan - numQueuedTimeOpsRan @@ -159,7 +159,7 @@ func SimulateFromSeed(tb testing.TB, app *baseapp.BaseApp, opCount += operations + numQueuedOpsRan + numQueuedTimeOpsRan if testingMode { // Make sure invariants hold at end of block - assertAllInvariants(t, app, invariants, "StandardOperations", displayLogs) + assertAllInvariants(t, app, header, invariants, "StandardOperations", displayLogs) } res := app.EndBlock(abci.RequestEndBlock{}) @@ -170,7 +170,7 @@ func SimulateFromSeed(tb testing.TB, app *baseapp.BaseApp, if testingMode { // Make sure invariants hold at end of block - assertAllInvariants(t, app, invariants, "EndBlock", displayLogs) + assertAllInvariants(t, app, header, invariants, "EndBlock", displayLogs) } if commit { app.Commit() @@ -230,7 +230,7 @@ func createBlockSimulator(testingMode bool, tb testing.TB, t *testing.T, event f queueOperations(operationQueue, timeOperationQueue, futureOps) if testingMode { if onOperation { - assertAllInvariants(t, app, invariants, fmt.Sprintf("operation: %v", logUpdate), displayLogs) + assertAllInvariants(t, app, header, invariants, fmt.Sprintf("operation: %v", logUpdate), displayLogs) } if opCount%50 == 0 { fmt.Printf("\rSimulating... block %d/%d, operation %d/%d. ", header.Height, totalNumBlocks, opCount, blocksize) diff --git a/x/mock/simulation/types.go b/x/mock/simulation/types.go index 302ebcbd7d7d..c582e4ae39ac 100644 --- a/x/mock/simulation/types.go +++ b/x/mock/simulation/types.go @@ -33,7 +33,7 @@ type ( // If the invariant has been broken, it should return an error // containing a descriptive message about what happened. // The simulator will then halt and print the logs. - Invariant func(app *baseapp.BaseApp) error + Invariant func(app *baseapp.BaseApp, header abci.Header) error // Account contains a privkey, pubkey, address tuple // eventually more useful data can be placed in here. diff --git a/x/mock/simulation/util.go b/x/mock/simulation/util.go index 93c0a5be097e..2e2f40a82436 100644 --- a/x/mock/simulation/util.go +++ b/x/mock/simulation/util.go @@ -9,6 +9,7 @@ import ( "testing" "time" + abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto/ed25519" "github.com/tendermint/tendermint/crypto/secp256k1" @@ -102,9 +103,11 @@ func addLogMessage(testingmode bool, blockLogBuilders []*strings.Builder, height } // assertAllInvariants asserts a list of provided invariants against application state -func assertAllInvariants(t *testing.T, app *baseapp.BaseApp, invariants []Invariant, where string, displayLogs func()) { +func assertAllInvariants(t *testing.T, header abci.Header, app *baseapp.BaseApp, + invariants []Invariant, where string, displayLogs func()) { + for i := 0; i < len(invariants); i++ { - err := invariants[i](app) + err := invariants[i](app, header) if err != nil { fmt.Printf("Invariants broken after %s\n", where) fmt.Println(err.Error()) From d4a1cf61da7b5601c349d8c4a496e9f0e60ae863 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Thu, 25 Oct 2018 17:31:24 -0400 Subject: [PATCH 09/24] add header to dim iterator --- cmd/gaia/app/sim_test.go | 8 +++--- x/bank/simulation/invariants.go | 4 +-- x/distribution/alias.go | 7 +++-- x/distribution/simulation/invariants.go | 38 +++++++++++-------------- x/distribution/types/validator_info.go | 4 +-- x/gov/simulation/invariants.go | 3 +- x/mock/simulation/types.go | 5 ++-- x/mock/simulation/util.go | 2 +- x/slashing/simulation/invariants.go | 5 ++-- x/stake/simulation/invariants.go | 22 ++++++++------ 10 files changed, 51 insertions(+), 47 deletions(-) diff --git a/cmd/gaia/app/sim_test.go b/cmd/gaia/app/sim_test.go index 0f56c5ef49e1..384afbf6dbbd 100644 --- a/cmd/gaia/app/sim_test.go +++ b/cmd/gaia/app/sim_test.go @@ -10,7 +10,6 @@ import ( "github.com/stretchr/testify/require" - abci "github.com/tendermint/tendermint/abci/types" dbm "github.com/tendermint/tendermint/libs/db" "github.com/tendermint/tendermint/libs/log" @@ -124,12 +123,13 @@ func testAndRunTxs(app *GaiaApp) []simulation.WeightedOperation { } } -func invariants(app *GaiaApp, header abci.Header) []simulation.Invariant { +func invariants(app *GaiaApp) []simulation.Invariant { return []simulation.Invariant{ banksim.NonnegativeBalanceInvariant(app.accountKeeper), govsim.AllInvariants(), - distrsim.AllInvariants(app.distrKeeper, header), - stakesim.AllInvariants(app.bankKeeper, app.stakeKeeper, app.feeCollectionKeeper, app.distrKeeper, app.accountKeeper), + distrsim.AllInvariants(app.distrKeeper, app.stakeKeeper), + stakesim.AllInvariants(app.bankKeeper, app.stakeKeeper, + app.feeCollectionKeeper, app.distrKeeper, app.accountKeeper), slashingsim.AllInvariants(), } } diff --git a/x/bank/simulation/invariants.go b/x/bank/simulation/invariants.go index f0cd5ba63ce0..6574407a4c77 100644 --- a/x/bank/simulation/invariants.go +++ b/x/bank/simulation/invariants.go @@ -14,7 +14,7 @@ import ( // NonnegativeBalanceInvariant checks that all accounts in the application have non-negative balances func NonnegativeBalanceInvariant(mapper auth.AccountKeeper) simulation.Invariant { - return func(app *baseapp.BaseApp) error { + return func(app *baseapp.BaseApp, _ abci.Header) error { ctx := app.NewContext(false, abci.Header{}) accts := mock.GetAllAccounts(mapper, ctx) for _, acc := range accts { @@ -32,7 +32,7 @@ func NonnegativeBalanceInvariant(mapper auth.AccountKeeper) simulation.Invariant // TotalCoinsInvariant checks that the sum of the coins across all accounts // is what is expected func TotalCoinsInvariant(mapper auth.AccountKeeper, totalSupplyFn func() sdk.Coins) simulation.Invariant { - return func(app *baseapp.BaseApp) error { + return func(app *baseapp.BaseApp, _ abci.Header) error { ctx := app.NewContext(false, abci.Header{}) totalCoins := sdk.Coins{} diff --git a/x/distribution/alias.go b/x/distribution/alias.go index 5a7cbce4d53c..f293857abeb9 100644 --- a/x/distribution/alias.go +++ b/x/distribution/alias.go @@ -23,6 +23,11 @@ type ( MsgWithdrawValidatorRewardsAll = types.MsgWithdrawValidatorRewardsAll GenesisState = types.GenesisState + + // expected keepers + StakeKeeper = types.StakeKeeper + BankKeeper = types.BankKeeper + FeeCollectionKeeper = types.FeeCollectionKeeper ) var ( @@ -62,9 +67,7 @@ var ( ErrNilDelegatorAddr = types.ErrNilDelegatorAddr ErrNilWithdrawAddr = types.ErrNilWithdrawAddr ErrNilValidatorAddr = types.ErrNilValidatorAddr -) -var ( ActionModifyWithdrawAddress = tags.ActionModifyWithdrawAddress ActionWithdrawDelegatorRewardsAll = tags.ActionWithdrawDelegatorRewardsAll ActionWithdrawDelegatorReward = tags.ActionWithdrawDelegatorReward diff --git a/x/distribution/simulation/invariants.go b/x/distribution/simulation/invariants.go index 277a6a2ff21a..fe2748af6958 100644 --- a/x/distribution/simulation/invariants.go +++ b/x/distribution/simulation/invariants.go @@ -5,50 +5,44 @@ import ( "github.com/cosmos/cosmos-sdk/baseapp" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/distribution" + distr "github.com/cosmos/cosmos-sdk/x/distribution" "github.com/cosmos/cosmos-sdk/x/mock/simulation" abci "github.com/tendermint/tendermint/abci/types" ) // AllInvariants runs all invariants of the distribution module // Currently: total supply, positive power -func AllInvariants( - d distribution.Keeper, height int64) simulation.Invariant { +func AllInvariants(d distr.Keeper, sk distr.StakeKeeper) simulation.Invariant { - return func(app *baseapp.BaseApp) error { - err := ValAccumInvariants(ck, k, f, d, am)(app) + return func(app *baseapp.BaseApp, header abci.Header) error { + err := ValAccumInvariants(d, sk)(app, header) if err != nil { return err } + return nil } } -// SupplyInvariants checks that the total supply reflects all held loose tokens, bonded tokens, and unbonding delegations -// nolint: unparam -func ValAccumInvariants(d distribution.Keeper, header abci.Header) simulation.Invariant { +// ValAccumInvariants checks that the fee pool accum == sum all validators' accum +func ValAccumInvariants(k distr.Keeper, sk distr.StakeKeeper) simulation.Invariant { - return func(app *baseapp.BaseApp) error { + return func(app *baseapp.BaseApp, header abci.Header) error { ctx := app.NewContext(false, header) + height := ctx.BlockHeight() - valAccum := int64(0) - k.IterateValidators(ctx, func(_ int64, validator sdk.Validator) bool { - switch validator.GetStatus() { - case sdk.Bonded: - bonded = bonded.Add(validator.GetPower()) - case sdk.Unbonding: - loose = loose.Add(validator.GetTokens()) - case sdk.Unbonded: - loose = loose.Add(validator.GetTokens()) - } + valAccum := sdk.ZeroDec() + k.IterateValidatorDistInfos(ctx, func(_ int64, vdi distr.ValidatorDistInfo) bool { + lastValPower := sk.GetLastValidatorPower(ctx, vdi.OperatorAddr) + valAccum = valAccum.Add(vdi.GetValAccum(height, lastValPower)) return false }) - //totalBondedTokens := - totalAccum := d.GetFeePool(ctx).GetTotalValAccum(ctx.GetHeight(), totalBondedTokens) + lastTotalPower := sdk.NewDecFromInt(sk.GetLastTotalPower(ctx)) + totalAccum := k.GetFeePool(ctx).GetTotalValAccum(height, lastTotalPower) if totalAccum != valAccum { fmt.Errorf("validator accum invariance: \n\tfee pool totalAccum: %v"+ - "\n\tvalidator accum \t%v\n", totalAccum, valAccum) + "\n\tvalidator accum \t%v\n", totalAccum.String(), valAccum.String()) } return nil diff --git a/x/distribution/types/validator_info.go b/x/distribution/types/validator_info.go index 933ea98b1fa1..23ce33565710 100644 --- a/x/distribution/types/validator_info.go +++ b/x/distribution/types/validator_info.go @@ -60,9 +60,9 @@ func (vi ValidatorDistInfo) GetTotalDelAccum(height int64, totalDelShares sdk.De } // Get the validator accum at the provided height -func (vi ValidatorDistInfo) GetValAccum(height int64, vdTokens sdk.Dec) sdk.Dec { +func (vi ValidatorDistInfo) GetValAccum(height int64, valTokens sdk.Dec) sdk.Dec { blocks := height - vi.FeePoolWithdrawalHeight - return vdTokens.MulInt(sdk.NewInt(blocks)) + return valTokens.MulInt(sdk.NewInt(blocks)) } // Move any available accumulated fees in the FeePool to the validator's pool diff --git a/x/gov/simulation/invariants.go b/x/gov/simulation/invariants.go index 6d5f41918474..3135ac80cb43 100644 --- a/x/gov/simulation/invariants.go +++ b/x/gov/simulation/invariants.go @@ -3,11 +3,12 @@ package simulation import ( "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/x/mock/simulation" + abci "github.com/tendermint/tendermint/abci/types" ) // AllInvariants tests all governance invariants func AllInvariants() simulation.Invariant { - return func(app *baseapp.BaseApp) error { + return func(app *baseapp.BaseApp, _ abci.Header) error { // TODO Add some invariants! // Checking proposal queues, no passed-but-unexecuted proposals, etc. return nil diff --git a/x/mock/simulation/types.go b/x/mock/simulation/types.go index c582e4ae39ac..0a7c989a5c08 100644 --- a/x/mock/simulation/types.go +++ b/x/mock/simulation/types.go @@ -68,13 +68,14 @@ type ( } ) +// TODO remove? not being called anywhere // PeriodicInvariant returns an Invariant function closure that asserts // a given invariant if the mock application's last block modulo the given // period is congruent to the given offset. func PeriodicInvariant(invariant Invariant, period int, offset int) Invariant { - return func(app *baseapp.BaseApp) error { + return func(app *baseapp.BaseApp, header abci.Header) error { if int(app.LastBlockHeight())%period == offset { - return invariant(app) + return invariant(app, header) } return nil } diff --git a/x/mock/simulation/util.go b/x/mock/simulation/util.go index 2e2f40a82436..c408c5328c40 100644 --- a/x/mock/simulation/util.go +++ b/x/mock/simulation/util.go @@ -103,7 +103,7 @@ func addLogMessage(testingmode bool, blockLogBuilders []*strings.Builder, height } // assertAllInvariants asserts a list of provided invariants against application state -func assertAllInvariants(t *testing.T, header abci.Header, app *baseapp.BaseApp, +func assertAllInvariants(t *testing.T, app *baseapp.BaseApp, header abci.Header, invariants []Invariant, where string, displayLogs func()) { for i := 0; i < len(invariants); i++ { diff --git a/x/slashing/simulation/invariants.go b/x/slashing/simulation/invariants.go index 637a8064bc7b..5ad2d65ad2f0 100644 --- a/x/slashing/simulation/invariants.go +++ b/x/slashing/simulation/invariants.go @@ -3,12 +3,13 @@ package simulation import ( "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/x/mock/simulation" + abci "github.com/tendermint/tendermint/abci/types" ) +// TODO Any invariants to check here? // AllInvariants tests all slashing invariants func AllInvariants() simulation.Invariant { - return func(app *baseapp.BaseApp) error { - // TODO Any invariants to check here? + return func(_ *baseapp.BaseApp, _ abci.Header) error { return nil } } diff --git a/x/stake/simulation/invariants.go b/x/stake/simulation/invariants.go index f8d9a5227aac..4549199cc1b2 100644 --- a/x/stake/simulation/invariants.go +++ b/x/stake/simulation/invariants.go @@ -15,25 +15,29 @@ import ( // AllInvariants runs all invariants of the stake module. // Currently: total supply, positive power -func AllInvariants(ck bank.Keeper, k stake.Keeper, f auth.FeeCollectionKeeper, d distribution.Keeper, am auth.AccountKeeper) simulation.Invariant { - return func(app *baseapp.BaseApp) error { - err := SupplyInvariants(ck, k, f, d, am)(app) +func AllInvariants(ck bank.Keeper, k stake.Keeper, + f auth.FeeCollectionKeeper, d distribution.Keeper, + am auth.AccountKeeper) simulation.Invariant { + + return func(app *baseapp.BaseApp, header abci.Header) error { + err := SupplyInvariants(ck, k, f, d, am)(app, header) if err != nil { return err } - err = PositivePowerInvariant(k)(app) + err = PositivePowerInvariant(k)(app, header) if err != nil { return err } - err = ValidatorSetInvariant(k)(app) + err = ValidatorSetInvariant(k)(app, header) return err } } // SupplyInvariants checks that the total supply reflects all held loose tokens, bonded tokens, and unbonding delegations // nolint: unparam -func SupplyInvariants(ck bank.Keeper, k stake.Keeper, f auth.FeeCollectionKeeper, d distribution.Keeper, am auth.AccountKeeper) simulation.Invariant { - return func(app *baseapp.BaseApp) error { +func SupplyInvariants(ck bank.Keeper, k stake.Keeper, + f auth.FeeCollectionKeeper, d distribution.Keeper, am auth.AccountKeeper) simulation.Invariant { + return func(app *baseapp.BaseApp, _ abci.Header) error { ctx := app.NewContext(false, abci.Header{}) pool := k.GetPool(ctx) @@ -93,7 +97,7 @@ func SupplyInvariants(ck bank.Keeper, k stake.Keeper, f auth.FeeCollectionKeeper // PositivePowerInvariant checks that all stored validators have > 0 power func PositivePowerInvariant(k stake.Keeper) simulation.Invariant { - return func(app *baseapp.BaseApp) error { + return func(app *baseapp.BaseApp, _ abci.Header) error { ctx := app.NewContext(false, abci.Header{}) var err error k.IterateValidatorsBonded(ctx, func(_ int64, validator sdk.Validator) bool { @@ -109,7 +113,7 @@ func PositivePowerInvariant(k stake.Keeper) simulation.Invariant { // ValidatorSetInvariant checks equivalence of Tendermint validator set and SDK validator set func ValidatorSetInvariant(k stake.Keeper) simulation.Invariant { - return func(app *baseapp.BaseApp) error { + return func(app *baseapp.BaseApp, _ abci.Header) error { // TODO return nil } From bad97f2d3ab6d59f525e2a2ea527662c1fd5cfe4 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Thu, 25 Oct 2018 17:46:14 -0400 Subject: [PATCH 10/24] fix invarience output --- x/distribution/simulation/invariants.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/distribution/simulation/invariants.go b/x/distribution/simulation/invariants.go index fe2748af6958..340a337af2de 100644 --- a/x/distribution/simulation/invariants.go +++ b/x/distribution/simulation/invariants.go @@ -40,8 +40,8 @@ func ValAccumInvariants(k distr.Keeper, sk distr.StakeKeeper) simulation.Invaria lastTotalPower := sdk.NewDecFromInt(sk.GetLastTotalPower(ctx)) totalAccum := k.GetFeePool(ctx).GetTotalValAccum(height, lastTotalPower) - if totalAccum != valAccum { - fmt.Errorf("validator accum invariance: \n\tfee pool totalAccum: %v"+ + if !totalAccum.Equal(valAccum) { + return fmt.Errorf("validator accum invariance: \n\tfee pool totalAccum: %v"+ "\n\tvalidator accum \t%v\n", totalAccum.String(), valAccum.String()) } From e23054f0b32fe4796df2cb7a74d991e97d139658 Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Thu, 25 Oct 2018 16:34:02 -0700 Subject: [PATCH 11/24] Rename Pool -> DelRewards; PoolCommission -> ValCommision --- x/distribution/keeper/allocation.go | 4 ++-- x/distribution/keeper/hooks.go | 4 ++-- x/distribution/types/delegator_info.go | 6 +++--- x/distribution/types/delegator_info_test.go | 8 +++---- x/distribution/types/validator_info.go | 24 ++++++++++----------- x/distribution/types/validator_info_test.go | 20 ++++++++--------- x/stake/simulation/invariants.go | 4 ++-- 7 files changed, 35 insertions(+), 35 deletions(-) diff --git a/x/distribution/keeper/allocation.go b/x/distribution/keeper/allocation.go index 294c273930ff..4fd71efb403f 100644 --- a/x/distribution/keeper/allocation.go +++ b/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.Pool = proposerDist.Pool.Plus(remaining) + proposerDist.ValCommission = proposerDist.ValCommission.Plus(commission) + proposerDist.DelRewards = proposerDist.DelRewards.Plus(remaining) // allocate community funding communityTax := k.GetCommunityTax(ctx) diff --git a/x/distribution/keeper/hooks.go b/x/distribution/keeper/hooks.go index 2f9b5ee54882..0b528ceb15be 100644 --- a/x/distribution/keeper/hooks.go +++ b/x/distribution/keeper/hooks.go @@ -12,9 +12,9 @@ func (k Keeper) onValidatorCreated(ctx sdk.Context, addr sdk.ValAddress) { vdi := types.ValidatorDistInfo{ OperatorAddr: addr, FeePoolWithdrawalHeight: height, - Pool: types.DecCoins{}, - PoolCommission: types.DecCoins{}, DelAccum: types.NewTotalAccum(height), + DelRewards: types.DecCoins{}, + ValCommission: types.DecCoins{}, } k.SetValidatorDistInfo(ctx, vdi) } diff --git a/x/distribution/types/delegator_info.go b/x/distribution/types/delegator_info.go index 7c2eaef76221..c4a6b168394c 100644 --- a/x/distribution/types/delegator_info.go +++ b/x/distribution/types/delegator_info.go @@ -43,10 +43,10 @@ func (di DelegationDistInfo) WithdrawRewards(fp FeePool, vi ValidatorDistInfo, blocks := height - di.WithdrawalHeight di.WithdrawalHeight = height accum := delegatorShares.MulInt(sdk.NewInt(blocks)) - withdrawalTokens := vi.Pool.MulDec(accum).QuoDec(vi.DelAccum.Accum) - remainingTokens := vi.Pool.Minus(withdrawalTokens) + withdrawalTokens := vi.DelRewards.MulDec(accum).QuoDec(vi.DelAccum.Accum) + remDelRewards := vi.DelRewards.Minus(withdrawalTokens) - vi.Pool = remainingTokens + vi.DelRewards = remDelRewards vi.DelAccum.Accum = vi.DelAccum.Accum.Sub(accum) return di, vi, fp, withdrawalTokens diff --git a/x/distribution/types/delegator_info_test.go b/x/distribution/types/delegator_info_test.go index 4af7f0a8f122..9a8b1310b18e 100644 --- a/x/distribution/types/delegator_info_test.go +++ b/x/distribution/types/delegator_info_test.go @@ -35,8 +35,8 @@ func TestWithdrawRewards(t *testing.T) { assert.Equal(t, height, di1.WithdrawalHeight) 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)) + assert.True(sdk.DecEq(t, sdk.NewDec(49), vi.DelRewards[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 @@ -50,7 +50,7 @@ func TestWithdrawRewards(t *testing.T) { assert.Equal(t, height, di2.WithdrawalHeight) 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)) + assert.True(sdk.DecEq(t, sdk.NewDec(49), vi.DelRewards[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)) } diff --git a/x/distribution/types/validator_info.go b/x/distribution/types/validator_info.go index 5fc86efcd5ea..5987672428d4 100644 --- a/x/distribution/types/validator_info.go +++ b/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 - 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) + FeePoolWithdrawalHeight int64 `json:"global_withdrawal_height"` // last height this validator withdrew from the global pool - DelAccum TotalAccum `json:"del_accum"` // total proposer pool accumulation factor held by delegators + DelAccum TotalAccum `json:"del_accum"` // total accumulation factor held by delegators + DelRewards DecCoins `json:"del_rewards"` // 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{}, - PoolCommission: DecCoins{}, + DelRewards: DecCoins{}, DelAccum: NewTotalAccum(currentHeight), + ValCommission: DecCoins{}, } } @@ -58,15 +58,15 @@ func (vi ValidatorDistInfo) TakeFeePoolRewards(fp FeePool, height int64, totalBo panic("individual accum should never be greater than the total") } withdrawalTokens := fp.Pool.MulDec(accum).QuoDec(fp.TotalValAccum.Accum) - remainingTokens := fp.Pool.Minus(withdrawalTokens) + remPool := fp.Pool.Minus(withdrawalTokens) commission := withdrawalTokens.MulDec(commissionRate) afterCommission := withdrawalTokens.Minus(commission) fp.TotalValAccum.Accum = fp.TotalValAccum.Accum.Sub(accum) - fp.Pool = remainingTokens - vi.PoolCommission = vi.PoolCommission.Plus(commission) - vi.Pool = vi.Pool.Plus(afterCommission) + fp.Pool = remPool + vi.ValCommission = vi.ValCommission.Plus(commission) + vi.DelRewards = vi.DelRewards.Plus(afterCommission) return vi, fp } @@ -77,8 +77,8 @@ func (vi ValidatorDistInfo) WithdrawCommission(fp FeePool, height int64, vi, fp = vi.TakeFeePoolRewards(fp, height, totalBonded, vdTokens, commissionRate) - withdrawalTokens := vi.PoolCommission - vi.PoolCommission = DecCoins{} // zero + withdrawalTokens := vi.ValCommission + vi.ValCommission = DecCoins{} // zero return vi, fp, withdrawalTokens } diff --git a/x/distribution/types/validator_info_test.go b/x/distribution/types/validator_info_test.go index 9d1e39fa6a06..15b7c0b5e497 100644 --- a/x/distribution/types/validator_info_test.go +++ b/x/distribution/types/validator_info_test.go @@ -31,14 +31,14 @@ func TestTakeFeePoolRewards(t *testing.T) { 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(100-2), vi1.Pool[0].Amount)) - assert.True(sdk.DecEq(t, sdk.NewDec(2), vi1.PoolCommission[0].Amount)) + assert.True(sdk.DecEq(t, sdk.NewDec(100-2), vi1.DelRewards[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(400-12), vi2.Pool[0].Amount)) - assert.True(sdk.DecEq(t, vi2.PoolCommission[0].Amount, sdk.NewDec(12))) + assert.True(sdk.DecEq(t, sdk.NewDec(400-12), vi2.DelRewards[0].Amount)) + assert.True(sdk.DecEq(t, vi2.ValCommission[0].Amount, sdk.NewDec(12))) // add more blocks and inflation height = 20 @@ -47,8 +47,8 @@ func TestTakeFeePoolRewards(t *testing.T) { 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(1000-40), vi3.Pool[0].Amount)) - assert.True(sdk.DecEq(t, vi3.PoolCommission[0].Amount, sdk.NewDec(40))) + assert.True(sdk.DecEq(t, sdk.NewDec(1000-40), vi3.DelRewards[0].Amount)) + assert.True(sdk.DecEq(t, vi3.ValCommission[0].Amount, sdk.NewDec(40))) } func TestWithdrawCommission(t *testing.T) { @@ -69,8 +69,8 @@ func TestWithdrawCommission(t *testing.T) { 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(100-2), vi.Pool[0].Amount)) - assert.True(sdk.DecEq(t, sdk.NewDec(2), vi.PoolCommission[0].Amount)) + assert.True(sdk.DecEq(t, sdk.NewDec(100-2), vi.DelRewards[0].Amount)) + assert.True(sdk.DecEq(t, sdk.NewDec(2), vi.ValCommission[0].Amount)) // add more blocks and inflation height = 20 @@ -79,7 +79,7 @@ func TestWithdrawCommission(t *testing.T) { 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(200-4), vi.Pool[0].Amount)) - assert.Zero(t, len(vi.PoolCommission)) + assert.True(sdk.DecEq(t, sdk.NewDec(200-4), vi.DelRewards[0].Amount)) + assert.Zero(t, len(vi.ValCommission)) assert.True(sdk.DecEq(t, sdk.NewDec(4), commissionRecv[0].Amount)) } diff --git a/x/stake/simulation/invariants.go b/x/stake/simulation/invariants.go index f8d9a5227aac..419b3b167a88 100644 --- a/x/stake/simulation/invariants.go +++ b/x/stake/simulation/invariants.go @@ -72,8 +72,8 @@ func SupplyInvariants(ck bank.Keeper, k stake.Keeper, f auth.FeeCollectionKeeper // 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.PoolCommission.AmountOf("steak")) + loose = loose.Add(distInfo.ValCommission.AmountOf("steak")) + loose = loose.Add(distInfo.DelRewards.AmountOf("steak")) return false }) From 93d5437bb666573d9ebecdd9abd71e1c585d8156 Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Thu, 25 Oct 2018 17:07:23 -0700 Subject: [PATCH 12/24] FeePool.Pool -> FeePool.ValPool --- x/distribution/keeper/allocation.go | 4 +-- x/distribution/keeper/allocation_test.go | 14 +++++------ x/distribution/keeper/hooks.go | 2 +- x/distribution/types/delegator_info.go | 6 ++--- x/distribution/types/delegator_info_test.go | 12 ++++----- x/distribution/types/fee_pool.go | 4 +-- x/distribution/types/validator_info.go | 12 ++++----- x/distribution/types/validator_info_test.go | 28 ++++++++++----------- x/stake/simulation/invariants.go | 4 +-- 9 files changed, 43 insertions(+), 43 deletions(-) diff --git a/x/distribution/keeper/allocation.go b/x/distribution/keeper/allocation.go index 4fd71efb403f..abe6b32dfb47 100644 --- a/x/distribution/keeper/allocation.go +++ b/x/distribution/keeper/allocation.go @@ -28,7 +28,7 @@ func (k Keeper) AllocateTokens(ctx sdk.Context, percentVotes sdk.Dec, proposer s commission := proposerReward.MulDec(proposerValidator.GetCommission()) remaining := proposerReward.Minus(commission) proposerDist.ValCommission = proposerDist.ValCommission.Plus(commission) - proposerDist.DelRewards = proposerDist.DelRewards.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) diff --git a/x/distribution/keeper/allocation_test.go b/x/distribution/keeper/allocation_test.go index 18dfe78e57c8..7d05e82b1ca3 100644 --- a/x/distribution/keeper/allocation_test.go +++ b/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.True(sdk.DecEq(t, expRes, feePool.Pool[0].Amount)) + require.Equal(t, 1, len(feePool.ValPool)) + 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.True(sdk.DecEq(t, expRes, feePool.Pool[0].Amount)) + require.Equal(t, 1, len(feePool.ValPool)) + 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.True(sdk.DecEq(t, expRes, feePool.Pool[0].Amount)) + require.Equal(t, 1, len(feePool.ValPool)) + require.True(sdk.DecEq(t, expRes, feePool.ValPool[0].Amount)) } diff --git a/x/distribution/keeper/hooks.go b/x/distribution/keeper/hooks.go index 0b528ceb15be..08cf8a7fb1fc 100644 --- a/x/distribution/keeper/hooks.go +++ b/x/distribution/keeper/hooks.go @@ -13,7 +13,7 @@ func (k Keeper) onValidatorCreated(ctx sdk.Context, addr sdk.ValAddress) { OperatorAddr: addr, FeePoolWithdrawalHeight: height, DelAccum: types.NewTotalAccum(height), - DelRewards: types.DecCoins{}, + DelPool: types.DecCoins{}, ValCommission: types.DecCoins{}, } k.SetValidatorDistInfo(ctx, vdi) diff --git a/x/distribution/types/delegator_info.go b/x/distribution/types/delegator_info.go index c4a6b168394c..aa9adfddfb81 100644 --- a/x/distribution/types/delegator_info.go +++ b/x/distribution/types/delegator_info.go @@ -43,10 +43,10 @@ func (di DelegationDistInfo) WithdrawRewards(fp FeePool, vi ValidatorDistInfo, blocks := height - di.WithdrawalHeight di.WithdrawalHeight = height accum := delegatorShares.MulInt(sdk.NewInt(blocks)) - withdrawalTokens := vi.DelRewards.MulDec(accum).QuoDec(vi.DelAccum.Accum) - remDelRewards := vi.DelRewards.Minus(withdrawalTokens) + withdrawalTokens := vi.DelPool.MulDec(accum).QuoDec(vi.DelAccum.Accum) + remDelPool := vi.DelPool.Minus(withdrawalTokens) - vi.DelRewards = remDelRewards + vi.DelPool = remDelPool vi.DelAccum.Accum = vi.DelAccum.Accum.Sub(accum) return di, vi, fp, withdrawalTokens diff --git a/x/distribution/types/delegator_info_test.go b/x/distribution/types/delegator_info_test.go index 9a8b1310b18e..240b4e1a65ee 100644 --- a/x/distribution/types/delegator_info_test.go +++ b/x/distribution/types/delegator_info_test.go @@ -26,7 +26,7 @@ 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, @@ -34,14 +34,14 @@ func TestWithdrawRewards(t *testing.T) { assert.Equal(t, height, di1.WithdrawalHeight) 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.DelRewards[0].Amount)) + assert.True(sdk.DecEq(t, sdk.NewDec(900), fp.ValPool[0].Amount)) + assert.True(sdk.DecEq(t, sdk.NewDec(49), vi.DelPool[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, @@ -49,8 +49,8 @@ func TestWithdrawRewards(t *testing.T) { assert.Equal(t, height, di2.WithdrawalHeight) 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.DelRewards[0].Amount)) + assert.True(sdk.DecEq(t, sdk.NewDec(1800), fp.ValPool[0].Amount)) + assert.True(sdk.DecEq(t, sdk.NewDec(49), vi.DelPool[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)) } diff --git a/x/distribution/types/fee_pool.go b/x/distribution/types/fee_pool.go index ae1d72cc0117..4dce113160d5 100644 --- a/x/distribution/types/fee_pool.go +++ b/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{}, } } diff --git a/x/distribution/types/validator_info.go b/x/distribution/types/validator_info.go index 5987672428d4..25f9f30bad57 100644 --- a/x/distribution/types/validator_info.go +++ b/x/distribution/types/validator_info.go @@ -11,7 +11,7 @@ type ValidatorDistInfo struct { FeePoolWithdrawalHeight int64 `json:"global_withdrawal_height"` // last height this validator withdrew from the global pool DelAccum TotalAccum `json:"del_accum"` // total accumulation factor held by delegators - DelRewards DecCoins `json:"del_rewards"` // rewards owed to delegators, commission has already been charged (includes proposer reward) + 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) } @@ -19,7 +19,7 @@ func NewValidatorDistInfo(operatorAddr sdk.ValAddress, currentHeight int64) Vali return ValidatorDistInfo{ OperatorAddr: operatorAddr, FeePoolWithdrawalHeight: currentHeight, - DelRewards: DecCoins{}, + DelPool: DecCoins{}, DelAccum: NewTotalAccum(currentHeight), ValCommission: DecCoins{}, } @@ -57,16 +57,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) - remPool := fp.Pool.Minus(withdrawalTokens) + withdrawalTokens := fp.ValPool.MulDec(accum).QuoDec(fp.TotalValAccum.Accum) + remValPool := fp.ValPool.Minus(withdrawalTokens) commission := withdrawalTokens.MulDec(commissionRate) afterCommission := withdrawalTokens.Minus(commission) fp.TotalValAccum.Accum = fp.TotalValAccum.Accum.Sub(accum) - fp.Pool = remPool + fp.ValPool = remValPool vi.ValCommission = vi.ValCommission.Plus(commission) - vi.DelRewards = vi.DelRewards.Plus(afterCommission) + vi.DelPool = vi.DelPool.Plus(afterCommission) return vi, fp } diff --git a/x/distribution/types/validator_info_test.go b/x/distribution/types/validator_info_test.go index 15b7c0b5e497..f4f6da49b909 100644 --- a/x/distribution/types/validator_info_test.go +++ b/x/distribution/types/validator_info_test.go @@ -26,28 +26,28 @@ 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(100-2), vi1.DelRewards[0].Amount)) + assert.True(sdk.DecEq(t, sdk.NewDec(900), fp.ValPool[0].Amount)) + assert.True(sdk.DecEq(t, sdk.NewDec(100-2), vi1.DelPool[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(400-12), vi2.DelRewards[0].Amount)) + assert.True(sdk.DecEq(t, sdk.NewDec(500), fp.ValPool[0].Amount)) + assert.True(sdk.DecEq(t, sdk.NewDec(400-12), vi2.DelPool[0].Amount)) 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(1000-40), vi3.DelRewards[0].Amount)) + assert.True(sdk.DecEq(t, sdk.NewDec(500), fp.ValPool[0].Amount)) + assert.True(sdk.DecEq(t, sdk.NewDec(1000-40), vi3.DelPool[0].Amount)) assert.True(sdk.DecEq(t, vi3.ValCommission[0].Amount, sdk.NewDec(40))) } @@ -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(100-2), vi.DelRewards[0].Amount)) + assert.True(sdk.DecEq(t, sdk.NewDec(900), fp.ValPool[0].Amount)) + assert.True(sdk.DecEq(t, sdk.NewDec(100-2), vi.DelPool[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(200-4), vi.DelRewards[0].Amount)) + assert.True(sdk.DecEq(t, sdk.NewDec(1800), fp.ValPool[0].Amount)) + assert.True(sdk.DecEq(t, sdk.NewDec(200-4), vi.DelPool[0].Amount)) assert.Zero(t, len(vi.ValCommission)) assert.True(sdk.DecEq(t, sdk.NewDec(4), commissionRecv[0].Amount)) } diff --git a/x/stake/simulation/invariants.go b/x/stake/simulation/invariants.go index 419b3b167a88..fcb883af98c4 100644 --- a/x/stake/simulation/invariants.go +++ b/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.ValCommission.AmountOf("steak")) - loose = loose.Add(distInfo.DelRewards.AmountOf("steak")) + loose = loose.Add(distInfo.DelPool.AmountOf("steak")) return false }) From 33521b032faa70bd40e42b6336f795b99ba237f7 Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Thu, 25 Oct 2018 17:14:23 -0700 Subject: [PATCH 13/24] WithdrawalHeight->DelPoolWithdrawalHeight --- x/distribution/keeper/hooks.go | 8 ++++---- x/distribution/types/delegator_info.go | 16 ++++++++-------- x/distribution/types/delegator_info_test.go | 4 ++-- x/distribution/types/validator_info.go | 2 +- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/x/distribution/keeper/hooks.go b/x/distribution/keeper/hooks.go index 08cf8a7fb1fc..c1d758e2b42f 100644 --- a/x/distribution/keeper/hooks.go +++ b/x/distribution/keeper/hooks.go @@ -13,7 +13,7 @@ func (k Keeper) onValidatorCreated(ctx sdk.Context, addr sdk.ValAddress) { OperatorAddr: addr, FeePoolWithdrawalHeight: height, DelAccum: types.NewTotalAccum(height), - DelPool: types.DecCoins{}, + DelPool: types.DecCoins{}, ValCommission: types.DecCoins{}, } k.SetValidatorDistInfo(ctx, vdi) @@ -41,9 +41,9 @@ func (k Keeper) onDelegationCreated(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) { ddi := types.DelegationDistInfo{ - DelegatorAddr: delAddr, - ValOperatorAddr: valAddr, - WithdrawalHeight: ctx.BlockHeight(), + DelegatorAddr: delAddr, + ValOperatorAddr: valAddr, + DelPoolWithdrawalHeight: ctx.BlockHeight(), } k.SetDelegationDistInfo(ctx, ddi) } diff --git a/x/distribution/types/delegator_info.go b/x/distribution/types/delegator_info.go index aa9adfddfb81..7af8c95c0f66 100644 --- a/x/distribution/types/delegator_info.go +++ b/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"` - ValOperatorAddr sdk.ValAddress `json:"val_operator_addr"` - WithdrawalHeight int64 `json:"withdrawal_height"` // last time this delegation withdrew rewards + DelegatorAddr sdk.AccAddress `json:"delegator_addr"` + ValOperatorAddr sdk.ValAddress `json:"val_operator_addr"` + 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, - ValOperatorAddr: valOperatorAddr, - WithdrawalHeight: currentHeight, + DelegatorAddr: delegatorAddr, + ValOperatorAddr: valOperatorAddr, + DelPoolWithdrawalHeight: currentHeight, } } @@ -40,8 +40,8 @@ func (di DelegationDistInfo) WithdrawRewards(fp FeePool, vi ValidatorDistInfo, vi, fp = vi.TakeFeePoolRewards(fp, height, totalBonded, vdTokens, commissionRate) - blocks := height - di.WithdrawalHeight - di.WithdrawalHeight = height + blocks := height - di.DelPoolWithdrawalHeight + di.DelPoolWithdrawalHeight = height accum := delegatorShares.MulInt(sdk.NewInt(blocks)) withdrawalTokens := vi.DelPool.MulDec(accum).QuoDec(vi.DelAccum.Accum) remDelPool := vi.DelPool.Minus(withdrawalTokens) diff --git a/x/distribution/types/delegator_info_test.go b/x/distribution/types/delegator_info_test.go index 240b4e1a65ee..76b9f048d58c 100644 --- a/x/distribution/types/delegator_info_test.go +++ b/x/distribution/types/delegator_info_test.go @@ -32,7 +32,7 @@ func TestWithdrawRewards(t *testing.T) { 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.ValPool[0].Amount)) assert.True(sdk.DecEq(t, sdk.NewDec(49), vi.DelPool[0].Amount)) @@ -47,7 +47,7 @@ func TestWithdrawRewards(t *testing.T) { 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.ValPool[0].Amount)) assert.True(sdk.DecEq(t, sdk.NewDec(49), vi.DelPool[0].Amount)) diff --git a/x/distribution/types/validator_info.go b/x/distribution/types/validator_info.go index 25f9f30bad57..04c9e7292d12 100644 --- a/x/distribution/types/validator_info.go +++ b/x/distribution/types/validator_info.go @@ -8,7 +8,7 @@ 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 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) From 1143c93dff96223902789c277eac193b69d867b3 Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Thu, 25 Oct 2018 17:41:57 -0700 Subject: [PATCH 14/24] OnValidatorBeginUnbonding --- cmd/gaia/app/app.go | 28 +++++++++++--------- types/stake.go | 11 ++++---- x/distribution/keeper/hooks.go | 40 ++++++++++++++++------------ x/slashing/hooks.go | 10 ++++--- x/stake/keeper/hooks.go | 26 +++++++++++------- x/stake/keeper/val_state_change.go | 42 +++++++++++++++++------------- 6 files changed, 92 insertions(+), 65 deletions(-) diff --git a/cmd/gaia/app/app.go b/cmd/gaia/app/app.go index 33c606105e2d..21ef855f026a 100644 --- a/cmd/gaia/app/app.go +++ b/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) { - h.dh.OnValidatorCreated(ctx, addr) +func (h Hooks) OnValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) { + h.dh.OnValidatorCreated(ctx, valAddr) } -func (h Hooks) OnValidatorModified(ctx sdk.Context, addr sdk.ValAddress) { - h.dh.OnValidatorModified(ctx, addr) +func (h Hooks) OnValidatorModified(ctx sdk.Context, valAddr sdk.ValAddress) { + h.dh.OnValidatorModified(ctx, valAddr) } -func (h Hooks) OnValidatorRemoved(ctx sdk.Context, addr sdk.ValAddress) { - h.dh.OnValidatorRemoved(ctx, addr) +func (h Hooks) OnValidatorRemoved(ctx sdk.Context, valAddr sdk.ValAddress) { + h.dh.OnValidatorRemoved(ctx, valAddr) } -func (h Hooks) OnValidatorBonded(ctx sdk.Context, addr sdk.ConsAddress, operator sdk.ValAddress) { - h.dh.OnValidatorBonded(ctx, addr, operator) - h.sh.OnValidatorBonded(ctx, addr, operator) +func (h Hooks) OnValidatorBonded(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) { + h.dh.OnValidatorBonded(ctx, consAddr, valAddr) + h.sh.OnValidatorBonded(ctx, consAddr, valAddr) } -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) OnValidatorPowerDidChange(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) { + h.dh.OnValidatorPowerDidChange(ctx, consAddr, valAddr) + 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) diff --git a/types/stake.go b/types/stake.go index 319dd470bbf8..f5d3a4aae0ae 100644 --- a/types/stake.go +++ b/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 - OnValidatorModified(ctx Context, address ValAddress) // Must be called when a validator's state changes - OnValidatorRemoved(ctx Context, address ValAddress) // Must be called when a validator is deleted + OnValidatorCreated(ctx Context, valAddr ValAddress) // Must be called when a validator is created + OnValidatorModified(ctx Context, valAddr ValAddress) // Must be called when a validator's state changes + 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 - OnValidatorBeginUnbonding(ctx Context, address ConsAddress, operator ValAddress) // Must be called when a validator begins unbonding + OnValidatorBonded(ctx Context, consAddr ConsAddress, valAddr ValAddress) // Must be called when a validator is bonded + 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 diff --git a/x/distribution/keeper/hooks.go b/x/distribution/keeper/hooks.go index c1d758e2b42f..526d2fc10c5d 100644 --- a/x/distribution/keeper/hooks.go +++ b/x/distribution/keeper/hooks.go @@ -6,11 +6,11 @@ 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, DelAccum: types.NewTotalAccum(height), DelPool: types.DecCoins{}, @@ -20,18 +20,23 @@ func (k Keeper) onValidatorCreated(ctx sdk.Context, addr sdk.ValAddress) { } // Withdrawal 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) { + +} + // Withdrawal all validator distribution rewards and cleanup the distribution record -func (k Keeper) onValidatorRemoved(ctx sdk.Context, addr sdk.ValAddress) { - k.RemoveValidatorDistInfo(ctx, addr) +func (k Keeper) onValidatorRemoved(ctx sdk.Context, valAddr sdk.ValAddress) { + k.RemoveValidatorDistInfo(ctx, valAddr) } //_________________________________________________________________________________________ @@ -77,14 +82,14 @@ var _ sdk.StakingHooks = Hooks{} func (k Keeper) Hooks() Hooks { return Hooks{k} } // nolint -func (h Hooks) OnValidatorCreated(ctx sdk.Context, addr sdk.ValAddress) { - h.k.onValidatorCreated(ctx, addr) +func (h Hooks) OnValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) { + h.k.onValidatorCreated(ctx, valAddr) } -func (h Hooks) OnValidatorModified(ctx sdk.Context, addr sdk.ValAddress) { - h.k.onValidatorModified(ctx, addr) +func (h Hooks) OnValidatorModified(ctx sdk.Context, valAddr sdk.ValAddress) { + h.k.onValidatorModified(ctx, valAddr) } -func (h Hooks) OnValidatorRemoved(ctx sdk.Context, addr sdk.ValAddress) { - h.k.onValidatorRemoved(ctx, addr) +func (h Hooks) OnValidatorRemoved(ctx sdk.Context, valAddr sdk.ValAddress) { + 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 +102,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) { - h.k.onValidatorModified(ctx, addr) +func (h Hooks) OnValidatorBeginUnbonding(ctx sdk.Context, _ sdk.ConsAddress, valAddr sdk.ValAddress) { + h.k.onValidatorModified(ctx, valAddr) +} +func (h Hooks) OnValidatorBonded(ctx sdk.Context, _ sdk.ConsAddress, valAddr sdk.ValAddress) { + h.k.onValidatorModified(ctx, valAddr) } -func (h Hooks) OnValidatorBonded(ctx sdk.Context, _ sdk.ConsAddress, addr sdk.ValAddress) { - h.k.onValidatorModified(ctx, addr) +func (h Hooks) OnValidatorPowerDidChange(ctx sdk.Context, _ sdk.ConsAddress, valAddr sdk.ValAddress) { + h.k.onValidatorPowerDidChange(ctx, valAddr) } diff --git a/x/slashing/hooks.go b/x/slashing/hooks.go index ee794e203ad1..10c16d1993e2 100644 --- a/x/slashing/hooks.go +++ b/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) { - h.k.onValidatorBonded(ctx, address, operator) +func (h Hooks) OnValidatorBonded(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) { + h.k.onValidatorBonded(ctx, consAddr, valAddr) } // Implements sdk.ValidatorHooks -func (h Hooks) OnValidatorBeginUnbonding(ctx sdk.Context, address sdk.ConsAddress, operator sdk.ValAddress) { - h.k.onValidatorBeginUnbonding(ctx, address, operator) +func (h Hooks) OnValidatorBeginUnbonding(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) { + 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) {} diff --git a/x/stake/keeper/hooks.go b/x/stake/keeper/hooks.go index f8c0a6e07eb0..4a8496ddd2a1 100644 --- a/x/stake/keeper/hooks.go +++ b/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) } } diff --git a/x/stake/keeper/val_state_change.go b/x/stake/keeper/val_state_change.go index 6cc2ff1b901b..ea057c0fddcf 100644 --- a/x/stake/keeper/val_state_change.go +++ b/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()) - validator := k.mustGetValidator(ctx, operator) + valAddr := sdk.ValAddress(iterator.Value()) + 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 - copy(operatorBytes[:], operator[:]) - oldPowerBytes, found := last[operatorBytes] + var valAddrBytes [sdk.AddrLen]byte + copy(valAddrBytes[:], valAddr[:]) + 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 - copy(operator[:], iterator.Key()[1:]) + var valAddr [sdk.AddrLen]byte + copy(valAddr[:], iterator.Key()[1:]) powerBytes := iterator.Value() - last[operator] = make([]byte, len(powerBytes)) - copy(last[operator][:], powerBytes[:]) + last[valAddr] = make([]byte, len(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 { - operator := make([]byte, sdk.AddrLen) - copy(operator[:], operatorBytes[:]) - noLongerBonded[index] = operator + for valAddrBytes := range last { + valAddr := make([]byte, sdk.AddrLen) + copy(valAddr[:], valAddrBytes[:]) + noLongerBonded[index] = valAddr index++ } // sorted by address - order doesn't matter From c17f42595d85f0e750dc224f7282ff68fe19c60d Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Thu, 25 Oct 2018 17:49:07 -0700 Subject: [PATCH 15/24] Caught the bug's tail --- Makefile | 2 +- x/distribution/keeper/hooks.go | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index ec02f1402ef0..0699e897dc7c 100644 --- a/Makefile +++ b/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=1 -v -timeout 24h test_sim_gaia_multi_seed: @echo "Running multi-seed Gaia simulation. This may take awhile!" diff --git a/x/distribution/keeper/hooks.go b/x/distribution/keeper/hooks.go index 526d2fc10c5d..72e30bd1061b 100644 --- a/x/distribution/keeper/hooks.go +++ b/x/distribution/keeper/hooks.go @@ -1,6 +1,8 @@ package keeper import ( + "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/distribution/types" ) @@ -31,7 +33,11 @@ func (k Keeper) onValidatorModified(ctx sdk.Context, valAddr sdk.ValAddress) { // 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() { + fmt.Println(vi.OperatorAddr.String(), vi.FeePoolWithdrawalHeight, ctx.BlockHeight()) + panic("DID NOT UPDATE") + } } // Withdrawal all validator distribution rewards and cleanup the distribution record From ce0528ff12690a22bc9a8bba8fdda426a35f5747 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Thu, 25 Oct 2018 21:13:08 -0400 Subject: [PATCH 16/24] unbonding fix --- x/distribution/keeper/hooks.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/x/distribution/keeper/hooks.go b/x/distribution/keeper/hooks.go index 10ef25a195de..9f1d45c1e5a1 100644 --- a/x/distribution/keeper/hooks.go +++ b/x/distribution/keeper/hooks.go @@ -14,13 +14,20 @@ func (k Keeper) onValidatorCreated(ctx sdk.Context, addr sdk.ValAddress) { vdi := types.ValidatorDistInfo{ OperatorAddr: addr, FeePoolWithdrawalHeight: height, - Pool: types.DecCoins{}, - PoolCommission: types.DecCoins{}, - DelAccum: types.NewTotalAccum(height), + Pool: types.DecCoins{}, + PoolCommission: types.DecCoins{}, + DelAccum: types.NewTotalAccum(height), } k.SetValidatorDistInfo(ctx, vdi) } +// Reset the height to the current height when bonded +func (k Keeper) onValidatorBonded(ctx sdk.Context, addr sdk.ValAddress) { + vdi := k.GetValidatorDistInfo(ctx, addr) + vdi.FeePoolWithdrawalHeight = ctx.BlockHeight() + k.SetValidatorDistInfo(ctx, vdi) +} + // Withdrawal all validator rewards func (k Keeper) onValidatorModified(ctx sdk.Context, addr sdk.ValAddress) { // This doesn't need to be run at genesis @@ -104,5 +111,5 @@ func (h Hooks) OnValidatorBeginUnbonding(ctx sdk.Context, _ sdk.ConsAddress, add h.k.onValidatorModified(ctx, addr) } func (h Hooks) OnValidatorBonded(ctx sdk.Context, _ sdk.ConsAddress, addr sdk.ValAddress) { - h.k.onValidatorModified(ctx, addr) + h.k.onValidatorBonded(ctx, addr) } From 79be65283e227227dbeefdc3361f097eacd8a0dd Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Thu, 25 Oct 2018 21:51:20 -0400 Subject: [PATCH 17/24] output cleanup --- x/stake/simulation/invariants.go | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/x/stake/simulation/invariants.go b/x/stake/simulation/invariants.go index 4549199cc1b2..7e339267adeb 100644 --- a/x/stake/simulation/invariants.go +++ b/x/stake/simulation/invariants.go @@ -75,20 +75,25 @@ func SupplyInvariants(ck bank.Keeper, k stake.Keeper, loose = loose.Add(feePool.Pool.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.PoolCommission.AmountOf("steak")) - return false - }) - - // Loose tokens should equal coin supply plus unbonding delegations plus tokens on unbonded validators + d.IterateValidatorDistInfos(ctx, + func(_ int64, distInfo distribution.ValidatorDistInfo) (stop bool) { + loose = loose.Add(distInfo.Pool.AmountOf("steak")) + loose = loose.Add(distInfo.PoolCommission.AmountOf("steak")) + return false + }, + ) + + // Loose tokens should equal coin supply plus unbonding delegations + // plus tokens on unbonded validators if !pool.LooseTokens.Equal(loose) { - return fmt.Errorf("expected loose tokens to equal total steak held by accounts - pool.LooseTokens: %v, sum of account tokens: %v", pool.LooseTokens, loose) + return fmt.Errorf("loose token invariance:\n\tpool.LooseTokens: %v"+ + "\n\tsum of account tokens: %v", pool.LooseTokens, loose) } // Bonded tokens should equal sum of tokens with bonded validators if !pool.BondedTokens.Equal(bonded) { - return fmt.Errorf("expected bonded tokens to equal total steak held by bonded validators - pool.BondedTokens: %v, sum of bonded validator tokens: %v", pool.BondedTokens, bonded) + return fmt.Errorf("bonded token invariance:\n\tpool.BondedTokens: %v"+ + "\n\tsum of account tokens: %v", pool.BondedTokens, bonded) } return nil From b97a07644b1614e768f5379acf0d03e4d22f6053 Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Thu, 25 Oct 2018 20:42:58 -0700 Subject: [PATCH 18/24] Update vi.FeePoolWithdrawalHeight upon bonding --- x/distribution/keeper/hooks.go | 7 ++----- x/distribution/types/validator_info.go | 1 + 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/x/distribution/keeper/hooks.go b/x/distribution/keeper/hooks.go index 72e30bd1061b..4374c72569f7 100644 --- a/x/distribution/keeper/hooks.go +++ b/x/distribution/keeper/hooks.go @@ -1,8 +1,6 @@ package keeper import ( - "fmt" - sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/distribution/types" ) @@ -21,7 +19,7 @@ func (k Keeper) onValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) { k.SetValidatorDistInfo(ctx, vdi) } -// Withdrawal all validator rewards +// Withdraw all validator rewards func (k Keeper) onValidatorModified(ctx sdk.Context, valAddr sdk.ValAddress) { // This doesn't need to be run at genesis if ctx.BlockHeight() > 0 { @@ -35,8 +33,7 @@ func (k Keeper) onValidatorModified(ctx sdk.Context, valAddr sdk.ValAddress) { func (k Keeper) onValidatorPowerDidChange(ctx sdk.Context, valAddr sdk.ValAddress) { vi := k.GetValidatorDistInfo(ctx, valAddr) if vi.FeePoolWithdrawalHeight != ctx.BlockHeight() { - fmt.Println(vi.OperatorAddr.String(), vi.FeePoolWithdrawalHeight, ctx.BlockHeight()) - panic("DID NOT UPDATE") + panic("expected validator dist info FeePoolWithdrawalHeight to be updated, but was not.") } } diff --git a/x/distribution/types/validator_info.go b/x/distribution/types/validator_info.go index 04c9e7292d12..d85f610d0bb5 100644 --- a/x/distribution/types/validator_info.go +++ b/x/distribution/types/validator_info.go @@ -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 } From aba5c95e0c6b112d11717ffb9e45a2182cd3adbd Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Fri, 26 Oct 2018 01:47:18 -0400 Subject: [PATCH 19/24] use jae fix instead --- x/distribution/keeper/hooks.go | 13 +++++++------ x/distribution/types/validator_info.go | 1 + 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/x/distribution/keeper/hooks.go b/x/distribution/keeper/hooks.go index 9f1d45c1e5a1..bb884eb58aff 100644 --- a/x/distribution/keeper/hooks.go +++ b/x/distribution/keeper/hooks.go @@ -22,11 +22,11 @@ func (k Keeper) onValidatorCreated(ctx sdk.Context, addr sdk.ValAddress) { } // Reset the height to the current height when bonded -func (k Keeper) onValidatorBonded(ctx sdk.Context, addr sdk.ValAddress) { - vdi := k.GetValidatorDistInfo(ctx, addr) - vdi.FeePoolWithdrawalHeight = ctx.BlockHeight() - k.SetValidatorDistInfo(ctx, vdi) -} +//func (k Keeper) onValidatorBonded(ctx sdk.Context, addr sdk.ValAddress) { +//vdi := k.GetValidatorDistInfo(ctx, addr) +//vdi.FeePoolWithdrawalHeight = ctx.BlockHeight() +//k.SetValidatorDistInfo(ctx, vdi) +//} // Withdrawal all validator rewards func (k Keeper) onValidatorModified(ctx sdk.Context, addr sdk.ValAddress) { @@ -111,5 +111,6 @@ func (h Hooks) OnValidatorBeginUnbonding(ctx sdk.Context, _ sdk.ConsAddress, add h.k.onValidatorModified(ctx, addr) } func (h Hooks) OnValidatorBonded(ctx sdk.Context, _ sdk.ConsAddress, addr sdk.ValAddress) { - h.k.onValidatorBonded(ctx, addr) + //h.k.onValidatorBonded(ctx, addr) + h.k.onValidatorModified(ctx, addr) } diff --git a/x/distribution/types/validator_info.go b/x/distribution/types/validator_info.go index 23ce33565710..6f112b5f4722 100644 --- a/x/distribution/types/validator_info.go +++ b/x/distribution/types/validator_info.go @@ -80,6 +80,7 @@ func (vi ValidatorDistInfo) TakeFeePoolRewards(wc WithdrawContext) ( fp := wc.FeePool.UpdateTotalValAccum(wc.Height, wc.TotalPower) if fp.TotalValAccum.Accum.IsZero() { + vi.FeePoolWithdrawalHeight = wc.Height return vi, fp } From 99efde2b525a5efa8a22a0814ef4d1469b35540f Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Fri, 26 Oct 2018 00:25:26 -0700 Subject: [PATCH 20/24] Fix staking slashUnbondingDelegation bug; fixes simulator failure #9 --- Makefile | 2 +- x/stake/keeper/slash.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 0699e897dc7c..b36ef61d4d28 100644 --- a/Makefile +++ b/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 -SimulationSeed=1 -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!" diff --git a/x/stake/keeper/slash.go b/x/stake/keeper/slash.go index 294ae9a9fd82..5879428f2bec 100644 --- a/x/stake/keeper/slash.go +++ b/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) } From a62dcad63233c85de82fcc23b1078037502a8a74 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Fri, 26 Oct 2018 04:08:32 -0400 Subject: [PATCH 21/24] compile fixes for jae merge --- x/distribution/keeper/hooks.go | 3 --- x/distribution/keeper/keeper.go | 2 +- x/distribution/keeper/validator.go | 2 +- x/distribution/types/validator_info.go | 8 ++++---- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/x/distribution/keeper/hooks.go b/x/distribution/keeper/hooks.go index f25e3145ace3..4374c72569f7 100644 --- a/x/distribution/keeper/hooks.go +++ b/x/distribution/keeper/hooks.go @@ -111,9 +111,6 @@ func (h Hooks) OnValidatorBeginUnbonding(ctx sdk.Context, _ sdk.ConsAddress, val func (h Hooks) OnValidatorBonded(ctx sdk.Context, _ sdk.ConsAddress, valAddr sdk.ValAddress) { h.k.onValidatorModified(ctx, valAddr) } -func (h Hooks) OnValidatorBonded(ctx sdk.Context, _ sdk.ConsAddress, addr sdk.ValAddress) { - h.k.onValidatorModified(ctx, addr) -} func (h Hooks) OnValidatorPowerDidChange(ctx sdk.Context, _ sdk.ConsAddress, valAddr sdk.ValAddress) { h.k.onValidatorPowerDidChange(ctx, valAddr) } diff --git a/x/distribution/keeper/keeper.go b/x/distribution/keeper/keeper.go index ab299a9b8d43..d167e0efe275 100644 --- a/x/distribution/keeper/keeper.go +++ b/x/distribution/keeper/keeper.go @@ -101,7 +101,7 @@ func (k Keeper) GetWithdrawContext(ctx sdk.Context, lastTotalPower := sdk.NewDecFromInt(k.stakeKeeper.GetLastTotalPower(ctx)) return types.NewWithdrawContext( - feePool, height, lastTotalPower, lastValPower, + feePool, height, lastTotalPower, sdk.NewDecFromInt(lastValPower), validator.GetCommission()) } diff --git a/x/distribution/keeper/validator.go b/x/distribution/keeper/validator.go index cf6c1c11d9dc..d267d66b2247 100644 --- a/x/distribution/keeper/validator.go +++ b/x/distribution/keeper/validator.go @@ -51,7 +51,7 @@ func (k Keeper) GetValidatorAccum(ctx sdk.Context, operatorAddr sdk.ValAddress) height := ctx.BlockHeight() lastValPower := k.stakeKeeper.GetLastValidatorPower(ctx, operatorAddr) valInfo := k.GetValidatorDistInfo(ctx, operatorAddr) - accum := valInfo.GetValAccum(height, lastValPower) + accum := valInfo.GetValAccum(height, sdk.NewDecFromInt(lastValPower)) return accum, nil } diff --git a/x/distribution/types/validator_info.go b/x/distribution/types/validator_info.go index 868812b90ca0..d7739359f2fa 100644 --- a/x/distribution/types/validator_info.go +++ b/x/distribution/types/validator_info.go @@ -128,10 +128,10 @@ func (vi ValidatorDistInfo) CurrentPoolRewards( if valAccum.GT(totalValAccum) { panic("individual accum should never be greater than the total") } - withdrawalTokens := fp.Pool.MulDec(valAccum).QuoDec(totalValAccum) + withdrawalTokens := fp.ValPool.MulDec(valAccum).QuoDec(totalValAccum) commission := withdrawalTokens.MulDec(wc.CommissionRate) afterCommission := withdrawalTokens.Minus(commission) - pool := vi.Pool.Plus(afterCommission) + pool := vi.DelPool.Plus(afterCommission) return pool } @@ -146,8 +146,8 @@ func (vi ValidatorDistInfo) CurrentCommissionRewards( if valAccum.GT(totalValAccum) { panic("individual accum should never be greater than the total") } - withdrawalTokens := fp.Pool.MulDec(valAccum).QuoDec(totalValAccum) + withdrawalTokens := fp.ValPool.MulDec(valAccum).QuoDec(totalValAccum) commission := withdrawalTokens.MulDec(wc.CommissionRate) - commissionPool := vi.PoolCommission.Plus(commission) + commissionPool := vi.ValCommission.Plus(commission) return commissionPool } From 306d1b2fb04b491dcdc693d08893b24db5b6b699 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Fri, 26 Oct 2018 04:12:01 -0400 Subject: [PATCH 22/24] ... --- x/distribution/simulation/invariants.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/distribution/simulation/invariants.go b/x/distribution/simulation/invariants.go index 340a337af2de..45c015cf07dd 100644 --- a/x/distribution/simulation/invariants.go +++ b/x/distribution/simulation/invariants.go @@ -33,7 +33,7 @@ func ValAccumInvariants(k distr.Keeper, sk distr.StakeKeeper) simulation.Invaria valAccum := sdk.ZeroDec() k.IterateValidatorDistInfos(ctx, func(_ int64, vdi distr.ValidatorDistInfo) bool { lastValPower := sk.GetLastValidatorPower(ctx, vdi.OperatorAddr) - valAccum = valAccum.Add(vdi.GetValAccum(height, lastValPower)) + valAccum = valAccum.Add(vdi.GetValAccum(height, sdk.NewDecFromInt(lastValPower))) return false }) From 78b8c2de9712a5a845452120913c4fe06573ba8f Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Fri, 26 Oct 2018 04:16:12 -0400 Subject: [PATCH 23/24] changelog --- PENDING.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/PENDING.md b/PENDING.md index 4d87301f0d5b..f315af561e7e 100644 --- a/PENDING.md +++ b/PENDING.md @@ -35,6 +35,7 @@ IMPROVEMENTS * Gaia * SDK + - #2573 [x/distribution] add accum invariance * Tendermint @@ -48,5 +49,7 @@ BUG FIXES * Gaia * SDK + - #2573 [x/distribution] accum invariance bugfix + - #2573 [x/slashing] unbonding-delegation slashing invariance bugfix * Tendermint From 0d1adfe158dd5f43e4ea3f008068b6a00103ea7e Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Fri, 26 Oct 2018 04:24:18 -0400 Subject: [PATCH 24/24] address @jawkwon comment --- x/distribution/keeper/hooks.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/x/distribution/keeper/hooks.go b/x/distribution/keeper/hooks.go index 4374c72569f7..efd2c0610413 100644 --- a/x/distribution/keeper/hooks.go +++ b/x/distribution/keeper/hooks.go @@ -29,6 +29,15 @@ func (k Keeper) onValidatorModified(ctx sdk.Context, valAddr sdk.ValAddress) { } } +// Withdraw all validator rewards +func (k Keeper) onValidatorBonded(ctx sdk.Context, valAddr sdk.ValAddress) { + lastPower := k.stakeKeeper.GetLastValidatorPower(ctx, valAddr) + if !lastPower.Equal(sdk.ZeroInt()) { + panic("expected last power to be 0 for validator entering bonded state") + } + k.onValidatorModified(ctx, valAddr) +} + // XXX Consider removing this after debugging. func (k Keeper) onValidatorPowerDidChange(ctx sdk.Context, valAddr sdk.ValAddress) { vi := k.GetValidatorDistInfo(ctx, valAddr) @@ -109,7 +118,7 @@ func (h Hooks) OnValidatorBeginUnbonding(ctx sdk.Context, _ sdk.ConsAddress, val h.k.onValidatorModified(ctx, valAddr) } func (h Hooks) OnValidatorBonded(ctx sdk.Context, _ sdk.ConsAddress, valAddr sdk.ValAddress) { - h.k.onValidatorModified(ctx, valAddr) + h.k.onValidatorBonded(ctx, valAddr) } func (h Hooks) OnValidatorPowerDidChange(ctx sdk.Context, _ sdk.ConsAddress, valAddr sdk.ValAddress) { h.k.onValidatorPowerDidChange(ctx, valAddr)