From ad82e971aec716c15cc6866bf4b74fd115b58516 Mon Sep 17 00:00:00 2001 From: Kevin Davis Date: Fri, 4 Oct 2019 13:55:49 -0400 Subject: [PATCH] address review comments --- app/app.go | 2 + x/validator-vesting/abci.go | 43 ++++------ x/validator-vesting/abci_test.go | 52 ++++++++++-- x/validator-vesting/internal/keeper/keeper.go | 24 ++---- .../internal/keeper/keeper_test.go | 55 +++++++------ .../internal/keeper/test_common.go | 4 +- .../types/validator_vesting_account.go | 70 ++++++++++------ .../types/validator_vesting_account_test.go | 82 +++++++++---------- x/validator-vesting/spec/03_begin_block.md | 8 +- 9 files changed, 191 insertions(+), 149 deletions(-) diff --git a/app/app.go b/app/app.go index 1826378c..be60f667 100644 --- a/app/app.go +++ b/app/app.go @@ -17,6 +17,7 @@ import ( "github.com/cosmos/cosmos-sdk/types/module" "github.com/cosmos/cosmos-sdk/version" "github.com/cosmos/cosmos-sdk/x/auth" + "github.com/cosmos/cosmos-sdk/x/auth/vesting" "github.com/cosmos/cosmos-sdk/x/bank" "github.com/cosmos/cosmos-sdk/x/crisis" distr "github.com/cosmos/cosmos-sdk/x/distribution" @@ -293,6 +294,7 @@ func MakeCodec() *codec.Codec { var cdc = codec.New() ModuleBasics.RegisterCodec(cdc) + vesting.RegisterCodec(cdc) sdk.RegisterCodec(cdc) codec.RegisterCrypto(cdc) codec.RegisterEvidences(cdc) diff --git a/x/validator-vesting/abci.go b/x/validator-vesting/abci.go index 0300354f..b959d4ef 100644 --- a/x/validator-vesting/abci.go +++ b/x/validator-vesting/abci.go @@ -23,30 +23,25 @@ func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, k keeper.Keeper) voteInfos = req.LastCommitInfo.GetVotes() validatorVestingKeys := k.GetAllAccountKeys(ctx) for _, key := range validatorVestingKeys { - acc := k.GetAccountFromAuthKeeper(ctx, key[1:]) - if voteInfos.ContainsValidatorAddress(acc.ValidatorAddress) { - vote := voteInfos.MustFilterByValidatorAddress(acc.ValidatorAddress) - if !vote.SignedLastBlock { - // if the validator explicitly missed signing the block, increment the missing sign count - k.UpdateMissingSignCount(ctx, acc.GetAddress(), true) - } else { - k.UpdateMissingSignCount(ctx, acc.GetAddress(), false) - } - } else { - // if the validator was not a voting member of the validator set, increment the missing sign count + acc := k.GetAccountFromAuthKeeper(ctx, key) + vote, found := voteInfos.FilterByValidatorAddress(acc.ValidatorAddress) + if !found || !vote.SignedLastBlock { + // if the validator was not found or explicitly didn't sign, increment the missing sign count k.UpdateMissingSignCount(ctx, acc.GetAddress(), true) + } else { + k.UpdateMissingSignCount(ctx, acc.GetAddress(), false) } // check if a period ended in the last block - endTimes := k.GetPeriodEndTimes(ctx, key[1:]) + endTimes := k.GetPeriodEndTimes(ctx, key) for i, t := range endTimes { if currentBlockTime.Unix() >= t && previousBlockTime.Unix() < t { - k.UpdateVestedCoinsProgress(ctx, key[1:], i) + k.UpdateVestedCoinsProgress(ctx, key, i) } } // handle any new/remaining debt on the account - k.HandleVestingDebt(ctx, key[1:], currentBlockTime) + k.HandleVestingDebt(ctx, key, currentBlockTime) } k.SetPreviousBlockTime(ctx, currentBlockTime) } @@ -54,24 +49,14 @@ func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, k keeper.Keeper) // VoteInfos an array of abci.VoteInfo type VoteInfos []abci.VoteInfo -// ContainsValidatorAddress returns true if the input validator address is found in the VoteInfos array -func (vis VoteInfos) ContainsValidatorAddress(consAddress sdk.ConsAddress) bool { - for _, vi := range vis { - votingAddress := sdk.ConsAddress(vi.Validator.Address) - if bytes.Equal(consAddress, votingAddress) { - return true - } - } - return false -} - -// MustFilterByValidatorAddress returns the VoteInfo that has a validator address matching the input validator address -func (vis VoteInfos) MustFilterByValidatorAddress(consAddress sdk.ConsAddress) abci.VoteInfo { +// FilterByValidatorAddress returns the VoteInfo of the validator address matching the input validator address +// and a boolean for if the address was found. +func (vis VoteInfos) FilterByValidatorAddress(consAddress sdk.ConsAddress) (abci.VoteInfo, bool) { for i, vi := range vis { votingAddress := sdk.ConsAddress(vi.Validator.Address) if bytes.Equal(consAddress, votingAddress) { - return vis[i] + return vis[i], true } } - panic("validator address not found") + return abci.VoteInfo{}, false } diff --git a/x/validator-vesting/abci_test.go b/x/validator-vesting/abci_test.go index 918d2566..f63f4c13 100644 --- a/x/validator-vesting/abci_test.go +++ b/x/validator-vesting/abci_test.go @@ -12,6 +12,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/staking" stakingexported "github.com/cosmos/cosmos-sdk/x/staking/exported" "github.com/kava-labs/kava/x/validator-vesting/internal/keeper" + "github.com/kava-labs/kava/x/validator-vesting/internal/types" ) func TestBeginBlockerSignedBlock(t *testing.T) { @@ -72,7 +73,7 @@ func TestBeginBlockerSignedBlock(t *testing.T) { height++ blockTime = addHour(blockTime) vva = vvk.GetAccountFromAuthKeeper(ctx, vva.Address) - require.Equal(t, []int64{0, 1}, vva.MissingSignCount) + require.Equal(t, types.CurrentPeriodProgress{0, 1}, vva.CurrentPeriodProgress) header = abci.Header{Height: height, Time: addHour(blockTime)} @@ -91,7 +92,7 @@ func TestBeginBlockerSignedBlock(t *testing.T) { height++ blockTime = addHour(blockTime) vva = vvk.GetAccountFromAuthKeeper(ctx, vva.Address) - require.Equal(t, []int64{1, 2}, vva.MissingSignCount) + require.Equal(t, types.CurrentPeriodProgress{1, 2}, vva.CurrentPeriodProgress) // mark the validator as being absent req = abci.RequestBeginBlock{ @@ -108,7 +109,7 @@ func TestBeginBlockerSignedBlock(t *testing.T) { height++ blockTime = addHour(blockTime) vva = vvk.GetAccountFromAuthKeeper(ctx, vva.Address) - require.Equal(t, []int64{2, 3}, vva.MissingSignCount) + require.Equal(t, types.CurrentPeriodProgress{2, 3}, vva.CurrentPeriodProgress) } func TestBeginBlockerSuccessfulPeriod(t *testing.T) { @@ -158,14 +159,14 @@ func TestBeginBlockerSuccessfulPeriod(t *testing.T) { if height == 11 { vva = vvk.GetAccountFromAuthKeeper(ctx, vva.Address) // require that missing sign count is set back to zero after the period increments. - require.Equal(t, []int64{0, 0}, vva.MissingSignCount) + require.Equal(t, types.CurrentPeriodProgress{0, 0}, vva.CurrentPeriodProgress) } } vva = vvk.GetAccountFromAuthKeeper(ctx, vva.Address) // t.Log(vva.MarshalYAML()) - require.Equal(t, [][]int{[]int{1, 1}, []int{0, 0}, []int{0, 0}}, vva.VestingPeriodProgress) + require.Equal(t, []types.VestingProgress{types.VestingProgress{true, true}, types.VestingProgress{false, false}, types.VestingProgress{false, false}}, vva.VestingPeriodProgress) } func TestBeginBlockerUnsuccessfulPeriod(t *testing.T) { @@ -175,12 +176,15 @@ func TestBeginBlockerUnsuccessfulPeriod(t *testing.T) { numBlocks := int64(12) addHour := func(t time.Time) time.Time { return t.Add(1 * time.Hour) } - ctx, ak, _, stakingKeeper, _, vvk := keeper.CreateTestInput(t, false, 1000) + ctx, ak, _, stakingKeeper, supplyKeeper, vvk := keeper.CreateTestInput(t, false, 1000) + + initialSupply := supplyKeeper.GetSupply(ctx).GetTotal() keeper.CreateValidators(ctx, stakingKeeper, []int64{5, 5, 5}) vva := keeper.ValidatorVestingDelegatorTestAccount(now) ak.SetAccount(ctx, vva) + // delegate all coins delTokens := sdk.TokensFromConsensusPower(60) vvk.SetValidatorVestingAccountKey(ctx, vva.Address) @@ -223,7 +227,7 @@ func TestBeginBlockerUnsuccessfulPeriod(t *testing.T) { vva = vvk.GetAccountFromAuthKeeper(ctx, vva.Address) // check that the period was unsucessful - require.Equal(t, [][]int{[]int{1, 0}, []int{0, 0}, []int{0, 0}}, vva.VestingPeriodProgress) + require.Equal(t, []types.VestingProgress{types.VestingProgress{true, false}, types.VestingProgress{false, false}, types.VestingProgress{false, false}}, vva.VestingPeriodProgress) // check that there is debt after the period. require.Equal(t, sdk.Coins{sdk.NewInt64Coin("stake", 30000000)}, vva.DebtAfterFailedVesting) @@ -234,4 +238,38 @@ func TestBeginBlockerUnsuccessfulPeriod(t *testing.T) { }) // require that all delegations were unbonded require.Equal(t, 0, delegations) + + // complete the unbonding period + header := abci.Header{Height: height, Time: blockTime.Add(time.Hour * 2)} + req := abci.RequestBeginBlock{ + Header: header, + LastCommitInfo: abci.LastCommitInfo{ + Votes: []abci.VoteInfo{{ + Validator: val, + SignedLastBlock: false, + }}, + }, + } + ctx = ctx.WithBlockHeader(header) + BeginBlocker(ctx, req, vvk) + _ = staking.EndBlocker(ctx, stakingKeeper) + + header = abci.Header{Height: height, Time: blockTime.Add(time.Hour * 2)} + req = abci.RequestBeginBlock{ + Header: header, + LastCommitInfo: abci.LastCommitInfo{ + Votes: []abci.VoteInfo{{ + Validator: val, + SignedLastBlock: false, + }}, + }, + } + ctx = ctx.WithBlockHeader(header) + BeginBlocker(ctx, req, vvk) + vva = vvk.GetAccountFromAuthKeeper(ctx, vva.Address) + // require that debt has reset to zero and coins balance is reduced by period 1 amount. + require.Equal(t, vva.GetCoins(), sdk.Coins{sdk.NewInt64Coin("stake", 30000000)}) + require.Equal(t, sdk.Coins(nil), vva.DebtAfterFailedVesting) + // require that the supply has decreased by period 1 amount + require.Equal(t, initialSupply.Sub(vva.VestingPeriods[0].Amount), supplyKeeper.GetSupply(ctx).GetTotal()) } diff --git a/x/validator-vesting/internal/keeper/keeper.go b/x/validator-vesting/internal/keeper/keeper.go index f1cf48b1..dd7e380c 100644 --- a/x/validator-vesting/internal/keeper/keeper.go +++ b/x/validator-vesting/internal/keeper/keeper.go @@ -83,7 +83,7 @@ func (k Keeper) IterateAccountKeys(ctx sdk.Context, cb func(accountKey []byte) ( func (k Keeper) GetAllAccountKeys(ctx sdk.Context) (keys [][]byte) { k.IterateAccountKeys(ctx, func(key []byte) (stop bool) { - keys = append(keys, key) + keys = append(keys, key[1:]) return false }) return keys @@ -103,39 +103,33 @@ func (k Keeper) GetAccountFromAuthKeeper(ctx sdk.Context, addr sdk.AccAddress) * func (k Keeper) UpdateMissingSignCount(ctx sdk.Context, addr sdk.AccAddress, missedBlock bool) { vv := k.GetAccountFromAuthKeeper(ctx, addr) if missedBlock { - vv.MissingSignCount[0]++ + vv.CurrentPeriodProgress.MissedBlocks++ } - vv.MissingSignCount[1]++ + vv.CurrentPeriodProgress.TotalBlocks++ k.ak.SetAccount(ctx, vv) } // UpdateVestedCoinsProgress sets the VestingPeriodProgress variable (0 = coins did not vest for the period, 1 = coins did vest for the period) for the given address and period. If coins did not vest, those coins are added to DebtAfterFailedVesting. Finally, MissingSignCount is reset to [0,0], representing that the next period has started and no blocks have been missed. func (k Keeper) UpdateVestedCoinsProgress(ctx sdk.Context, addr sdk.AccAddress, period int) { vv := k.GetAccountFromAuthKeeper(ctx, addr) - - threshold := sdk.NewDec(vv.SigningThreshold) - blocksMissed := sdk.NewDec(vv.MissingSignCount[0]) - blockCount := sdk.NewDec(vv.MissingSignCount[1]) var successfulVest bool - if blockCount.IsZero() { + if sdk.NewDec(vv.CurrentPeriodProgress.TotalBlocks).IsZero() { successfulVest = true } else { - blocksSigned := blockCount.Sub(blocksMissed) - percentageBlocksSigned := blocksSigned.Quo(blockCount).Mul(sdk.NewDec(100)) - successfulVest = percentageBlocksSigned.GTE(threshold) + successfulVest = vv.CurrentPeriodProgress.SignedPercetageIsOverThreshold(vv.SigningThreshold) } if successfulVest { - vv.VestingPeriodProgress[period][1] = 1 + vv.VestingPeriodProgress[period].VestingSuccessful = true } else { - vv.VestingPeriodProgress[period][1] = 0 + vv.VestingPeriodProgress[period].VestingSuccessful = false notVestedTokens := vv.VestingPeriods[period].Amount // add the tokens that did not vest to DebtAfterFailedVesting vv.DebtAfterFailedVesting = vv.DebtAfterFailedVesting.Add(notVestedTokens) } - vv.VestingPeriodProgress[period][0] = 1 + vv.VestingPeriodProgress[period].PeriodComplete = true // reset the number of missed blocks and total number of blocks in the period to zero - vv.MissingSignCount = []int64{0, 0} + vv.CurrentPeriodProgress = types.CurrentPeriodProgress{0, 0} k.ak.SetAccount(ctx, vv) } diff --git a/x/validator-vesting/internal/keeper/keeper_test.go b/x/validator-vesting/internal/keeper/keeper_test.go index 792d7a70..19d11010 100644 --- a/x/validator-vesting/internal/keeper/keeper_test.go +++ b/x/validator-vesting/internal/keeper/keeper_test.go @@ -11,6 +11,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/staking" stakingexported "github.com/cosmos/cosmos-sdk/x/staking/exported" + "github.com/kava-labs/kava/x/validator-vesting/internal/types" ) func TestGetSetValidatorVestingAccounts(t *testing.T) { @@ -36,7 +37,7 @@ func TestGetSetValidatorVestingAccounts(t *testing.T) { keys := keeper.GetAllAccountKeys(ctx) require.Equal(t, 1, len(keys)) for _, k := range keys { - require.NotPanics(t, func() { keeper.GetAccountFromAuthKeeper(ctx, k[1:]) }) + require.NotPanics(t, func() { keeper.GetAccountFromAuthKeeper(ctx, k) }) } vvAccounts := ValidatorVestingTestAccounts(10) @@ -50,7 +51,7 @@ func TestGetSetValidatorVestingAccounts(t *testing.T) { var ikeys [][]byte keeper.IterateAccountKeys(ctx, func(accountKey []byte) bool { - if bytes.Equal(accountKey, keys[0]) { + if bytes.Equal(accountKey[1:], keys[0]) { ikeys = append(ikeys, accountKey) return true } @@ -109,17 +110,17 @@ func TestSetMissingSignCount(t *testing.T) { ak.SetAccount(ctx, vva) // require empty array after ValidatorVestingAccount is initialized - require.Equal(t, []int64{0, 0}, vva.MissingSignCount) + require.Equal(t, types.CurrentPeriodProgress{0, 0}, vva.CurrentPeriodProgress) // validator signs a block keeper.UpdateMissingSignCount(ctx, vva.Address, false) vva = keeper.GetAccountFromAuthKeeper(ctx, vva.Address) - require.Equal(t, []int64{0, 1}, vva.MissingSignCount) + require.Equal(t, types.CurrentPeriodProgress{0, 1}, vva.CurrentPeriodProgress) // validator misses a block keeper.UpdateMissingSignCount(ctx, vva.Address, true) vva = keeper.GetAccountFromAuthKeeper(ctx, vva.Address) - require.Equal(t, []int64{1, 2}, vva.MissingSignCount) + require.Equal(t, types.CurrentPeriodProgress{1, 2}, vva.CurrentPeriodProgress) } @@ -132,56 +133,56 @@ func TestUpdateVestedCoinsProgress(t *testing.T) { ak.SetAccount(ctx, vva) // require all vesting period tracking variables to be zero after validator vesting account is initialized - require.Equal(t, [][]int{{0, 0}, {0, 0}, {0, 0}}, vva.VestingPeriodProgress) + require.Equal(t, []types.VestingProgress{types.VestingProgress{false, false}, types.VestingProgress{false, false}, types.VestingProgress{false, false}}, vva.VestingPeriodProgress) // period 0 passes with all blocks signed - vva.MissingSignCount[0] = 0 - vva.MissingSignCount[1] = 100 + vva.CurrentPeriodProgress.MissedBlocks = 0 + vva.CurrentPeriodProgress.TotalBlocks = 100 ak.SetAccount(ctx, vva) vva = keeper.GetAccountFromAuthKeeper(ctx, vva.Address) keeper.UpdateVestedCoinsProgress(ctx, vva.Address, 0) vva = keeper.GetAccountFromAuthKeeper(ctx, vva.Address) // require that debt is zero require.Equal(t, sdk.Coins(nil), vva.DebtAfterFailedVesting) - // require that the first vesting progress variable is 1 - require.Equal(t, [][]int{{1, 1}, {0, 0}, {0, 0}}, vva.VestingPeriodProgress) + // require that the first vesting progress variable is successful + require.Equal(t, []types.VestingProgress{types.VestingProgress{true, true}, types.VestingProgress{false, false}, types.VestingProgress{false, false}}, vva.VestingPeriodProgress) // require that the missing block counter has reset - require.Equal(t, []int64{0, 0}, vva.MissingSignCount) + require.Equal(t, types.CurrentPeriodProgress{0, 0}, vva.CurrentPeriodProgress) vva = ValidatorVestingTestAccount() ak.SetAccount(ctx, vva) // period 0 passes with no blocks signed // this is an edge case that shouldn't happen, // the vest is considered successful in this case. - vva.MissingSignCount[0] = 0 - vva.MissingSignCount[1] = 0 + vva.CurrentPeriodProgress.MissedBlocks = 0 + vva.CurrentPeriodProgress.TotalBlocks = 0 ak.SetAccount(ctx, vva) vva = keeper.GetAccountFromAuthKeeper(ctx, vva.Address) keeper.UpdateVestedCoinsProgress(ctx, vva.Address, 0) vva = keeper.GetAccountFromAuthKeeper(ctx, vva.Address) // require that debt is zero require.Equal(t, sdk.Coins(nil), vva.DebtAfterFailedVesting) - // require that the first vesting progress variable is 1 - require.Equal(t, [][]int{{1, 1}, {0, 0}, {0, 0}}, vva.VestingPeriodProgress) + // require that the first vesting progress variable is successful + require.Equal(t, []types.VestingProgress{types.VestingProgress{true, true}, types.VestingProgress{false, false}, types.VestingProgress{false, false}}, vva.VestingPeriodProgress) // require that the missing block counter has reset - require.Equal(t, []int64{0, 0}, vva.MissingSignCount) + require.Equal(t, types.CurrentPeriodProgress{0, 0}, vva.CurrentPeriodProgress) vva = ValidatorVestingTestAccount() ak.SetAccount(ctx, vva) // period 0 passes with 50% of blocks signed (below threshold) - vva.MissingSignCount[0] = 50 - vva.MissingSignCount[1] = 100 + vva.CurrentPeriodProgress.MissedBlocks = 50 + vva.CurrentPeriodProgress.TotalBlocks = 100 ak.SetAccount(ctx, vva) keeper.UpdateVestedCoinsProgress(ctx, vva.Address, 0) vva = keeper.GetAccountFromAuthKeeper(ctx, vva.Address) // require that period 1 coins have become debt require.Equal(t, sdk.NewCoins(sdk.NewInt64Coin(feeDenom, 500), sdk.NewInt64Coin(stakeDenom, 50)), vva.DebtAfterFailedVesting) - // require that the first vesting progress variable is {1,0} - require.Equal(t, [][]int{{1, 0}, {0, 0}, {0, 0}}, vva.VestingPeriodProgress) + // require that the first vesting progress variable is {true, false} + require.Equal(t, []types.VestingProgress{types.VestingProgress{true, false}, types.VestingProgress{false, false}, types.VestingProgress{false, false}}, vva.VestingPeriodProgress) // require that the missing block counter has reset - require.Equal(t, []int64{0, 0}, vva.MissingSignCount) + require.Equal(t, types.CurrentPeriodProgress{0, 0}, vva.CurrentPeriodProgress) } func TestHandleVestingDebtNoDebt(t *testing.T) { @@ -240,8 +241,8 @@ func TestHandleVestingDebtForcedUnbond(t *testing.T) { require.Equal(t, 1, delegations) // period 0 passes and the threshold is not met - vva.MissingSignCount[0] = 50 - vva.MissingSignCount[1] = 100 + vva.CurrentPeriodProgress.MissedBlocks = 50 + vva.CurrentPeriodProgress.TotalBlocks = 100 ak.SetAccount(ctx, vva) keeper.UpdateVestedCoinsProgress(ctx, vva.Address, 0) vva = keeper.GetAccountFromAuthKeeper(ctx, vva.Address) @@ -277,8 +278,8 @@ func TestHandleVestingDebtBurn(t *testing.T) { _ = staking.EndBlocker(ctx, stakingKeeper) // period 0 passes and the threshold is not met - vva.MissingSignCount[0] = 50 - vva.MissingSignCount[1] = 100 + vva.CurrentPeriodProgress.MissedBlocks = 50 + vva.CurrentPeriodProgress.TotalBlocks = 100 ak.SetAccount(ctx, vva) keeper.UpdateVestedCoinsProgress(ctx, vva.Address, 0) vva = keeper.GetAccountFromAuthKeeper(ctx, vva.Address) @@ -320,8 +321,8 @@ func TestHandleVestingDebtReturn(t *testing.T) { _ = staking.EndBlocker(ctx, stakingKeeper) // period 0 passes and the threshold is not met - vva.MissingSignCount[0] = 50 - vva.MissingSignCount[1] = 100 + vva.CurrentPeriodProgress.MissedBlocks = 50 + vva.CurrentPeriodProgress.TotalBlocks = 100 ak.SetAccount(ctx, vva) keeper.UpdateVestedCoinsProgress(ctx, vva.Address, 0) vva = keeper.GetAccountFromAuthKeeper(ctx, vva.Address) diff --git a/x/validator-vesting/internal/keeper/test_common.go b/x/validator-vesting/internal/keeper/test_common.go index 5ae88d3c..8c630329 100644 --- a/x/validator-vesting/internal/keeper/test_common.go +++ b/x/validator-vesting/internal/keeper/test_common.go @@ -122,6 +122,8 @@ func CreateTestInput(t *testing.T, isCheckTx bool, initPower int64) (sdk.Context pk := params.NewKeeper(cdc, keyParams, tkeyParams, params.DefaultCodespace) + stakingParams := staking.NewParams(time.Hour, 100, uint16(7), sdk.DefaultBondDenom) + accountKeeper := auth.NewAccountKeeper(cdc, keyAcc, pk.Subspace(auth.DefaultParamspace), auth.ProtoBaseAccount) bankKeeper := bank.NewBaseKeeper(accountKeeper, pk.Subspace(bank.DefaultParamspace), bank.DefaultCodespace, blacklistedAddrs) maccPerms := map[string][]string{ @@ -133,7 +135,7 @@ func CreateTestInput(t *testing.T, isCheckTx bool, initPower int64) (sdk.Context supplyKeeper := supply.NewKeeper(cdc, keySupply, accountKeeper, bankKeeper, maccPerms) stakingKeeper := staking.NewKeeper(cdc, keyStaking, supplyKeeper, pk.Subspace(staking.DefaultParamspace), staking.DefaultCodespace) - stakingKeeper.SetParams(ctx, staking.DefaultParams()) + stakingKeeper.SetParams(ctx, stakingParams) keeper := NewKeeper(cdc, keyValidatorVesting, accountKeeper, bankKeeper, supplyKeeper, stakingKeeper) diff --git a/x/validator-vesting/internal/types/validator_vesting_account.go b/x/validator-vesting/internal/types/validator_vesting_account.go index fb39111f..bf7cce24 100644 --- a/x/validator-vesting/internal/types/validator_vesting_account.go +++ b/x/validator-vesting/internal/types/validator_vesting_account.go @@ -23,6 +23,30 @@ func init() { authtypes.RegisterAccountTypeCodec(&ValidatorVestingAccount{}, "cosmos-sdk/ValidatorVestingAccount") } +type VestingProgress struct { + PeriodComplete bool `json:"period_complete" yaml:"period_complete"` + VestingSuccessful bool `json:"vesting_successful" yaml:"vesting_successful"` +} + +type CurrentPeriodProgress struct { + MissedBlocks int64 `json:"missed_blocks" yaml:"missed_blocks"` + TotalBlocks int64 `json:"total_blocks" yaml:"total_blocks` +} + +func (cpp CurrentPeriodProgress) GetSignedPercentage() sdk.Dec { + blocksSigned := cpp.TotalBlocks - cpp.MissedBlocks + // signed_percentage = blocksSigned/TotalBlocks * 100 + signedPercentage := sdk.NewDec(blocksSigned).Quo( + sdk.NewDec(cpp.TotalBlocks)).Mul( + sdk.NewDec(100)) + return signedPercentage +} + +func (cpp CurrentPeriodProgress) SignedPercetageIsOverThreshold(threshold int64) bool { + signedPercentage := cpp.GetSignedPercentage() + return signedPercentage.GTE(sdk.NewDec(threshold)) +} + // ValidatorVestingAccount implements the VestingAccount interface. It // conditionally vests by unlocking coins during each specified period, provided // that the validator address has validated at least **SigningThreshold** blocks during @@ -32,12 +56,12 @@ func init() { // the coins are returned to the return address, or burned if the return address is null. type ValidatorVestingAccount struct { *vestingtypes.PeriodicVestingAccount - ValidatorAddress sdk.ConsAddress `json:"validator_address" yaml:"validator_address"` - ReturnAddress sdk.AccAddress `json:"return_address" yaml:"return_address"` - SigningThreshold int64 `json:"signing_threshold" yaml:"signing_threshold"` - MissingSignCount []int64 `json:"missing_sign_count" yaml:"missing_sign_count"` - VestingPeriodProgress [][]int `json:"vesting_period_progress" yaml:"vesting_period_progress"` - DebtAfterFailedVesting sdk.Coins `json:"debt_after_failed_vesting" yaml:"debt_after_failed_vesting"` + ValidatorAddress sdk.ConsAddress `json:"validator_address" yaml:"validator_address"` + ReturnAddress sdk.AccAddress `json:"return_address" yaml:"return_address"` + SigningThreshold int64 `json:"signing_threshold" yaml:"signing_threshold"` + CurrentPeriodProgress CurrentPeriodProgress `json:"missing_sign_count" yaml:"missing_sign_count"` + VestingPeriodProgress []VestingProgress `json:"vesting_period_progress" yaml:"vesting_period_progress"` + DebtAfterFailedVesting sdk.Coins `json:"debt_after_failed_vesting" yaml:"debt_after_failed_vesting"` } // NewValidatorVestingAccountRaw creates a new ValidatorVestingAccount object from BaseVestingAccount @@ -49,9 +73,9 @@ func NewValidatorVestingAccountRaw(bva *vestingtypes.BaseVestingAccount, StartTime: startTime, VestingPeriods: periods, } - var vestingPeriodProgress = make([][]int, len(periods)) - for i := range vestingPeriodProgress { - vestingPeriodProgress[i] = make([]int, 2) + var vestingPeriodProgress []VestingProgress + for i := 0; i < len(periods); i++ { + vestingPeriodProgress = append(vestingPeriodProgress, VestingProgress{false, false}) } return &ValidatorVestingAccount{ @@ -59,7 +83,7 @@ func NewValidatorVestingAccountRaw(bva *vestingtypes.BaseVestingAccount, ValidatorAddress: validatorAddress, ReturnAddress: returnAddress, SigningThreshold: signingThreshold, - MissingSignCount: []int64{0, 0}, + CurrentPeriodProgress: CurrentPeriodProgress{0, 0}, VestingPeriodProgress: vestingPeriodProgress, DebtAfterFailedVesting: sdk.NewCoins(), } @@ -82,21 +106,19 @@ func NewValidatorVestingAccount(baseAcc *authtypes.BaseAccount, startTime int64, StartTime: startTime, VestingPeriods: periods, } - var vestingPeriodProgress = make([][]int, len(periods)) - for i := range vestingPeriodProgress { - vestingPeriodProgress[i] = make([]int, 2) + var vestingPeriodProgress []VestingProgress + for i := 0; i < len(periods); i++ { + vestingPeriodProgress = append(vestingPeriodProgress, VestingProgress{false, false}) } - debt := sdk.NewCoins() - return &ValidatorVestingAccount{ PeriodicVestingAccount: pva, ValidatorAddress: validatorAddress, ReturnAddress: returnAddress, SigningThreshold: signingThreshold, - MissingSignCount: []int64{0, 0}, + CurrentPeriodProgress: CurrentPeriodProgress{0, 0}, VestingPeriodProgress: vestingPeriodProgress, - DebtAfterFailedVesting: debt, + DebtAfterFailedVesting: sdk.NewCoins(), } } @@ -111,8 +133,7 @@ func (vva ValidatorVestingAccount) GetVestedCoins(blockTime time.Time) sdk.Coins for i := 0; i < numberPeriods; i++ { x := blockTime.Unix() - currentPeriodStartTime if x >= vva.VestingPeriods[i].Length { - vestingComplete := vva.VestingPeriodProgress[i][0] == 1 - if vestingComplete { + if vva.VestingPeriodProgress[i].PeriodComplete { vestedCoins = vestedCoins.Add(vva.VestingPeriods[i].Amount) } currentPeriodStartTime += vva.VestingPeriods[i].Length @@ -129,9 +150,8 @@ func (vva ValidatorVestingAccount) GetFailedVestedCoins() sdk.Coins { var failedVestedCoins sdk.Coins numberPeriods := len(vva.VestingPeriods) for i := 0; i < numberPeriods; i++ { - if vva.VestingPeriodProgress[i][0] == 1 { - vestedFailure := vva.VestingPeriodProgress[i][1] == 0 - if vestedFailure { + if vva.VestingPeriodProgress[i].PeriodComplete { + if !vva.VestingPeriodProgress[i].VestingSuccessful { failedVestedCoins = failedVestedCoins.Add(vva.VestingPeriods[i].Amount) } } else { @@ -198,8 +218,8 @@ func (vva ValidatorVestingAccount) MarshalYAML() (interface{}, error) { ValidatorAddress sdk.ConsAddress ReturnAddress sdk.AccAddress SigningThreshold int64 - MissingSignCount []int64 - VestingPeriodProgress [][]int + CurrentPeriodProgress CurrentPeriodProgress + VestingPeriodProgress []VestingProgress DebtAfterFailedVesting sdk.Coins }{ Address: vva.Address, @@ -216,7 +236,7 @@ func (vva ValidatorVestingAccount) MarshalYAML() (interface{}, error) { ValidatorAddress: vva.ValidatorAddress, ReturnAddress: vva.ReturnAddress, SigningThreshold: vva.SigningThreshold, - MissingSignCount: vva.MissingSignCount, + CurrentPeriodProgress: vva.CurrentPeriodProgress, VestingPeriodProgress: vva.VestingPeriodProgress, DebtAfterFailedVesting: vva.DebtAfterFailedVesting, }) diff --git a/x/validator-vesting/internal/types/validator_vesting_account_test.go b/x/validator-vesting/internal/types/validator_vesting_account_test.go index b5e7db4a..2d5a1f51 100644 --- a/x/validator-vesting/internal/types/validator_vesting_account_test.go +++ b/x/validator-vesting/internal/types/validator_vesting_account_test.go @@ -68,51 +68,51 @@ func TestGetVestedCoinsValidatorVestingAcc(t *testing.T) { require.Nil(t, vestedCoins) // require 50% of coins vested after successful period 1 vesting - vva.VestingPeriodProgress[0] = []int{1, 1} + vva.VestingPeriodProgress[0] = VestingProgress{true, true} vestedCoins = vva.GetVestedCoins(now.Add(12 * time.Hour)) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(feeDenom, 500), sdk.NewInt64Coin(stakeDenom, 50)}, vestedCoins) // require 50% of coins vested after unsuccessful period 1 vesting // NOTE: There is a fairly important semantic distinction here. It seems tempting to say that a failed vesting period should mean that 'GetVestedCoins' should not return those coins. While the point of a validator vesting account is to 'seize' or 'burn' unsuccessfully vested coins, they do in fact vest and become spendable. The intuition is that they have to be spendable in order for the bank keeper to allow us to send/burn them. If they were not vested, then a validator vesting account that failed all of it's vesting periods would never return/burn the coins because it would never have a spendable balance by which to do so. They way we prevent them from being spent in a way other than return/burn is by sending them in the BeginBlock and thus beating any other transfers that would otherwise occur. - vva.VestingPeriodProgress[0] = []int{1, 0} + vva.VestingPeriodProgress[0] = VestingProgress{true, false} vestedCoins = vva.GetVestedCoins(now.Add(12 * time.Hour)) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(feeDenom, 500), sdk.NewInt64Coin(stakeDenom, 50)}, vestedCoins) // require period 2 coins don't vest until period is over - vva.VestingPeriodProgress[0] = []int{1, 1} + vva.VestingPeriodProgress[0] = VestingProgress{true, true} // even if the vesting period was somehow successful, should still only return 50% of coins as vested, since the second vesting period hasn't completed. - vva.VestingPeriodProgress[1] = []int{1, 1} + vva.VestingPeriodProgress[1] = VestingProgress{true, true} vestedCoins = vva.GetVestedCoins(now.Add(15 * time.Hour)) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(feeDenom, 500), sdk.NewInt64Coin(stakeDenom, 50)}, vestedCoins) // require 75% of coins vested after successful period 2 - vva.VestingPeriodProgress[0] = []int{1, 1} - vva.VestingPeriodProgress[1] = []int{1, 1} + vva.VestingPeriodProgress[0] = VestingProgress{true, true} + vva.VestingPeriodProgress[1] = VestingProgress{true, true} vestedCoins = vva.GetVestedCoins(now.Add(18 * time.Hour)) require.Equal(t, sdk.Coins{ sdk.NewInt64Coin(feeDenom, 750), sdk.NewInt64Coin(stakeDenom, 75)}, vestedCoins) // require 75% of coins vested after successful period 1 and unsuccessful period 2. - vva.VestingPeriodProgress[0] = []int{1, 1} - vva.VestingPeriodProgress[1] = []int{1, 0} + vva.VestingPeriodProgress[0] = VestingProgress{true, true} + vva.VestingPeriodProgress[1] = VestingProgress{true, false} vestedCoins = vva.GetVestedCoins(now.Add(18 * time.Hour)) require.Equal(t, sdk.Coins{ sdk.NewInt64Coin(feeDenom, 750), sdk.NewInt64Coin(stakeDenom, 75)}, vestedCoins) // require 100% of coins vested after all periods complete successfully - vva.VestingPeriodProgress[0] = []int{1, 1} - vva.VestingPeriodProgress[1] = []int{1, 1} - vva.VestingPeriodProgress[2] = []int{1, 1} + vva.VestingPeriodProgress[0] = VestingProgress{true, true} + vva.VestingPeriodProgress[1] = VestingProgress{true, true} + vva.VestingPeriodProgress[2] = VestingProgress{true, true} vestedCoins = vva.GetVestedCoins(now.Add(48 * time.Hour)) require.Equal(t, origCoins, vestedCoins) // require 100% of coins vested after all periods complete unsuccessfully - vva.VestingPeriodProgress[0] = []int{1, 0} - vva.VestingPeriodProgress[1] = []int{1, 0} - vva.VestingPeriodProgress[2] = []int{1, 0} + vva.VestingPeriodProgress[0] = VestingProgress{true, false} + vva.VestingPeriodProgress[1] = VestingProgress{true, false} + vva.VestingPeriodProgress[2] = VestingProgress{true, false} vestedCoins = vva.GetVestedCoins(now.Add(48 * time.Hour)) require.Equal(t, origCoins, vestedCoins) @@ -143,48 +143,48 @@ func TestGetVestingCoinsValidatorVestingAcc(t *testing.T) { require.Equal(t, origCoins, vestingCoins) // require 50% of coins vesting after successful period 1 vesting - vva.VestingPeriodProgress[0] = []int{1, 1} + vva.VestingPeriodProgress[0] = VestingProgress{true, true} vestingCoins = vva.GetVestingCoins(now.Add(12 * time.Hour)) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(feeDenom, 500), sdk.NewInt64Coin(stakeDenom, 50)}, vestingCoins) // require 50% of coins vesting after unsuccessful period 1 vesting - vva.VestingPeriodProgress[0] = []int{1, 0} + vva.VestingPeriodProgress[0] = VestingProgress{true, false} vestingCoins = vva.GetVestingCoins(now.Add(12 * time.Hour)) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(feeDenom, 500), sdk.NewInt64Coin(stakeDenom, 50)}, vestingCoins) // require period 2 coins still vesting until period is over - vva.VestingPeriodProgress[0] = []int{1, 1} + vva.VestingPeriodProgress[0] = VestingProgress{true, true} // should never happen, but still won't affect vesting balance - vva.VestingPeriodProgress[1] = []int{1, 1} + vva.VestingPeriodProgress[1] = VestingProgress{true, true} vestingCoins = vva.GetVestingCoins(now.Add(15 * time.Hour)) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(feeDenom, 500), sdk.NewInt64Coin(stakeDenom, 50)}, vestingCoins) // require 25% of coins vesting after successful period 2 - vva.VestingPeriodProgress[0] = []int{1, 1} - vva.VestingPeriodProgress[1] = []int{1, 1} + vva.VestingPeriodProgress[0] = VestingProgress{true, true} + vva.VestingPeriodProgress[1] = VestingProgress{true, true} vestingCoins = vva.GetVestingCoins(now.Add(18 * time.Hour)) require.Equal(t, sdk.Coins{ sdk.NewInt64Coin(feeDenom, 250), sdk.NewInt64Coin(stakeDenom, 25)}, vestingCoins) // require 25% of coins vesting after successful period 1 and unsuccessful period 2 - vva.VestingPeriodProgress[0] = []int{1, 1} - vva.VestingPeriodProgress[1] = []int{1, 1} + vva.VestingPeriodProgress[0] = VestingProgress{true, true} + vva.VestingPeriodProgress[1] = VestingProgress{true, false} vestingCoins = vva.GetVestingCoins(now.Add(18 * time.Hour)) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(feeDenom, 250), sdk.NewInt64Coin(stakeDenom, 25)}, vestingCoins) // require no coins vesting after all periods complete successfully - vva.VestingPeriodProgress[0] = []int{1, 1} - vva.VestingPeriodProgress[1] = []int{1, 1} - vva.VestingPeriodProgress[2] = []int{1, 1} + vva.VestingPeriodProgress[0] = VestingProgress{true, true} + vva.VestingPeriodProgress[1] = VestingProgress{true, true} + vva.VestingPeriodProgress[2] = VestingProgress{true, true} vestingCoins = vva.GetVestingCoins(now.Add(48 * time.Hour)) require.Nil(t, vestingCoins) // require no coins vesting after all periods complete unsuccessfully - vva.VestingPeriodProgress[0] = []int{1, 0} - vva.VestingPeriodProgress[1] = []int{1, 0} - vva.VestingPeriodProgress[2] = []int{1, 0} + vva.VestingPeriodProgress[0] = VestingProgress{true, false} + vva.VestingPeriodProgress[1] = VestingProgress{true, false} + vva.VestingPeriodProgress[2] = VestingProgress{true, false} vestingCoins = vva.GetVestingCoins(now.Add(48 * time.Hour)) require.Nil(t, vestingCoins) @@ -211,12 +211,12 @@ func TestSpendableCoinsValidatorVestingAccount(t *testing.T) { require.Nil(t, spendableCoins) // require that all vested coins (50%) are spendable when period 1 completes successfully - vva.VestingPeriodProgress[0] = []int{1, 1} + vva.VestingPeriodProgress[0] = VestingProgress{true, true} spendableCoins = vva.SpendableCoins(now.Add(12 * time.Hour)) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(feeDenom, 500), sdk.NewInt64Coin(stakeDenom, 50)}, spendableCoins) // require that 50% of coins are spendable after period 1 completes unsuccessfully. See note above. The reason the coins are still 'spendable' is that we need to be able to transfer the coins to the return address/burn them. Making them not spendable means that it would be impossible to recover the debt for a validator vesting account for which all periods failed. - vva.VestingPeriodProgress[0] = []int{1, 0} + vva.VestingPeriodProgress[0] = VestingProgress{true, false} spendableCoins = vva.SpendableCoins(now.Add(12 * time.Hour)) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(feeDenom, 500), sdk.NewInt64Coin(stakeDenom, 50)}, spendableCoins) @@ -225,7 +225,7 @@ func TestSpendableCoinsValidatorVestingAccount(t *testing.T) { vva.SetCoins(vva.GetCoins().Add(recvAmt)) // require that all vested coins (50%) are spendable plus any received after period 1 completes successfully - vva.VestingPeriodProgress[0] = []int{1, 1} + vva.VestingPeriodProgress[0] = VestingProgress{true, true} spendableCoins = vva.SpendableCoins(now.Add(12 * time.Hour)) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(feeDenom, 500), sdk.NewInt64Coin(stakeDenom, 100)}, spendableCoins) @@ -253,14 +253,14 @@ func TestGetFailedVestedCoins(t *testing.T) { bacc.SetCoins(origCoins) vva := NewValidatorVestingAccount(&bacc, now.Unix(), periods, testConsAddr, nil, 90) - vva.VestingPeriodProgress[0] = []int{1, 0} + vva.VestingPeriodProgress[0] = VestingProgress{true, false} // require that period 1 coins are failed if the period completed unsucessfully. require.Equal(t, sdk.Coins{sdk.NewInt64Coin(feeDenom, 500), sdk.NewInt64Coin(stakeDenom, 50)}, vva.GetFailedVestedCoins(), ) - vva.VestingPeriodProgress[0] = []int{1, 1} + vva.VestingPeriodProgress[0] = VestingProgress{true, true} require.Equal(t, sdk.Coins(nil), vva.GetFailedVestedCoins(), @@ -290,9 +290,9 @@ func TestTrackDelegationValidatorVestingAcc(t *testing.T) { // all periods pass successfully bacc.SetCoins(origCoins) vva = NewValidatorVestingAccount(&bacc, now.Unix(), periods, testConsAddr, nil, 90) - vva.VestingPeriodProgress[0] = []int{1, 1} - vva.VestingPeriodProgress[1] = []int{1, 1} - vva.VestingPeriodProgress[2] = []int{1, 1} + vva.VestingPeriodProgress[0] = VestingProgress{true, true} + vva.VestingPeriodProgress[1] = VestingProgress{true, true} + vva.VestingPeriodProgress[2] = VestingProgress{true, true} vva.TrackDelegation(now.Add(48*time.Hour), origCoins) // require all delegated coins are free require.Equal(t, origCoins, vva.DelegatedFree) @@ -305,7 +305,7 @@ func TestTrackDelegationValidatorVestingAcc(t *testing.T) { require.Equal(t, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}, vva.DelegatedVesting) require.Nil(t, vva.DelegatedFree) - vva.VestingPeriodProgress[0] = []int{1, 1} + vva.VestingPeriodProgress[0] = VestingProgress{true, true} vva.TrackDelegation(now.Add(12*time.Hour), sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}, vva.DelegatedVesting) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}, vva.DelegatedFree) @@ -345,9 +345,9 @@ func TestTrackUndelegationPeriodicVestingAcc(t *testing.T) { // require the ability to delegate all coins after they have successfully vested bacc.SetCoins(origCoins) vva = NewValidatorVestingAccount(&bacc, now.Unix(), periods, testConsAddr, nil, 90) - vva.VestingPeriodProgress[0] = []int{1, 1} - vva.VestingPeriodProgress[1] = []int{1, 1} - vva.VestingPeriodProgress[2] = []int{1, 1} + vva.VestingPeriodProgress[0] = VestingProgress{true, true} + vva.VestingPeriodProgress[1] = VestingProgress{true, true} + vva.VestingPeriodProgress[2] = VestingProgress{true, true} vva.TrackDelegation(now.Add(24*time.Hour), origCoins) vva.TrackUndelegation(origCoins) require.Nil(t, vva.DelegatedFree) @@ -364,7 +364,7 @@ func TestTrackUndelegationPeriodicVestingAcc(t *testing.T) { // successfuly vest period 1 and delegate to two validators vva = NewValidatorVestingAccount(&bacc, now.Unix(), periods, testConsAddr, nil, 90) - vva.VestingPeriodProgress[0] = []int{1, 1} + vva.VestingPeriodProgress[0] = VestingProgress{true, true} vva.TrackDelegation(now.Add(12*time.Hour), sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}) vva.TrackDelegation(now.Add(12*time.Hour), sdk.Coins{sdk.NewInt64Coin(stakeDenom, 50)}) diff --git a/x/validator-vesting/spec/03_begin_block.md b/x/validator-vesting/spec/03_begin_block.md index b9528c51..967bb1ca 100644 --- a/x/validator-vesting/spec/03_begin_block.md +++ b/x/validator-vesting/spec/03_begin_block.md @@ -14,7 +14,7 @@ func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, k keeper.Keeper) voteInfos = req.LastCommitInfo.GetVotes() validatorVestingKeys := k.GetAllAccountKeys(ctx) for _, key := range validatorVestingKeys { - acc := k.GetAccountFromAuthKeeper(ctx, key[1:]) + acc := k.GetAccountFromAuthKeeper(ctx, key) if voteInfos.ContainsValidatorAddress(acc.ValidatorAddress) { vote := voteInfos.MustFilterByValidatorAddress(acc.ValidatorAddress) if !vote.SignedLastBlock { @@ -29,15 +29,15 @@ func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, k keeper.Keeper) } // check if a period ended in the last block - endTimes := k.GetPeriodEndTimes(ctx, key[1:]) + endTimes := k.GetPeriodEndTimes(ctx, key) for i, t := range endTimes { if currentBlockTime.Unix() >= t && previousBlockTime.Unix() < t { - k.UpdateVestedCoinsProgress(ctx, key[1:], i) + k.UpdateVestedCoinsProgress(ctx, key, i) } } // handle any new/remaining debt on the account - k.HandleVestingDebt(ctx, key[1:], currentBlockTime) + k.HandleVestingDebt(ctx, key, currentBlockTime) } k.SetPreviousBlockTime(ctx, currentBlockTime) }