From 7565ba4c0ca67ee790397cf9cea3f335ae6aa94e Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Mon, 2 Apr 2018 20:37:35 +0200 Subject: [PATCH] fix kicking validators logic --- x/stake/keeper.go | 46 ++++- x/stake/keeper_keys.go | 13 +- x/stake/keeper_test.go | 374 ++++++++++++++++++++++------------------- 3 files changed, 255 insertions(+), 178 deletions(-) diff --git a/x/stake/keeper.go b/x/stake/keeper.go index dd56b94aa4..155d8e04a0 100644 --- a/x/stake/keeper.go +++ b/x/stake/keeper.go @@ -89,6 +89,11 @@ func (k Keeper) setCandidate(ctx sdk.Context, candidate Candidate) { panic(err) } + // if the voting power is the same no need to update any of the other indexes + if oldFound && oldCandidate.Assets.Equal(candidate.Assets) { + return + } + // update the list ordered by voting power if oldFound { store.Delete(GetValidatorKey(address, oldCandidate.Assets, k.cdc)) @@ -96,7 +101,16 @@ func (k Keeper) setCandidate(ctx sdk.Context, candidate Candidate) { store.Set(GetValidatorKey(address, validator.VotingPower, k.cdc), bz) // add to the validators to update list if is already a validator - if store.Get(GetRecentValidatorKey(address)) != nil || k.isNewValidator(ctx, store, address) { + // or is a new validator + setAcc := false + if store.Get(GetRecentValidatorKey(address)) != nil { + setAcc = true + + // want to check in the else statement because inefficient + } else if k.isNewValidator(ctx, store, address) { + setAcc = true + } + if setAcc { store.Set(GetAccUpdateValidatorKey(validator.Address), bz) } return @@ -138,12 +152,18 @@ func (k Keeper) removeCandidate(ctx sdk.Context, address sdk.Address) { func (k Keeper) GetValidators(ctx sdk.Context) (validators []Validator) { store := ctx.KVStore(k.storeKey) - // clear the recent validators store - k.deleteSubSpace(store, RecentValidatorsKey) + // clear the recent validators store, add to the ToKickOut Temp store + iterator := store.Iterator(subspace(RecentValidatorsKey)) + for ; iterator.Valid(); iterator.Next() { + addr := AddrFromKey(iterator.Key()) + store.Set(GetToKickOutValidatorKey(addr), []byte{}) + store.Delete(iterator.Key()) + } + iterator.Close() // add the actual validator power sorted store maxVal := k.GetParams(ctx).MaxValidators - iterator := store.ReverseIterator(subspace(ValidatorsKey)) // largest to smallest + iterator = store.ReverseIterator(subspace(ValidatorsKey)) // largest to smallest validators = make([]Validator, maxVal) i := 0 for ; ; i++ { @@ -159,12 +179,28 @@ func (k Keeper) GetValidators(ctx sdk.Context) (validators []Validator) { } validators[i] = val + // remove from ToKickOut group + store.Delete(GetToKickOutValidatorKey(val.Address)) + // also add to the recent validators group - store.Set(GetRecentValidatorKey(val.Address), bz) + store.Set(GetRecentValidatorKey(val.Address), bz) // XXX should store nothing iterator.Next() } + // add any kicked out validators to the acc change + iterator = store.Iterator(subspace(ToKickOutValidatorsKey)) + for ; iterator.Valid(); iterator.Next() { + addr := AddrFromKey(iterator.Key()) + bz, err := k.cdc.MarshalBinary(Validator{addr, sdk.ZeroRat}) + if err != nil { + panic(err) + } + store.Set(GetAccUpdateValidatorKey(addr), bz) + store.Delete(iterator.Key()) + } + iterator.Close() + return validators[:i] // trim } diff --git a/x/stake/keeper_keys.go b/x/stake/keeper_keys.go index 5c09a47fc4..3b4c77174f 100644 --- a/x/stake/keeper_keys.go +++ b/x/stake/keeper_keys.go @@ -16,8 +16,9 @@ var ( ValidatorsKey = []byte{0x03} // prefix for each key to a validator AccUpdateValidatorsKey = []byte{0x04} // prefix for each key to a validator which is being updated RecentValidatorsKey = []byte{0x05} // prefix for each key to the last updated validator group + ToKickOutValidatorsKey = []byte{0x06} // prefix for each key to the last updated validator group - DelegatorBondKeyPrefix = []byte{0x06} // prefix for each key to a delegator's bond + DelegatorBondKeyPrefix = []byte{0x07} // prefix for each key to a delegator's bond ) const maxDigitsForAccount = 12 // ~220,000,000 atoms created at launch @@ -43,6 +44,16 @@ func GetRecentValidatorKey(addr sdk.Address) []byte { return append(RecentValidatorsKey, addr.Bytes()...) } +// reverse operation of GetRecentValidatorKey +func AddrFromKey(key []byte) sdk.Address { + return key[1:] +} + +// get the key for the accumulated update validators +func GetToKickOutValidatorKey(addr sdk.Address) []byte { + return append(ToKickOutValidatorsKey, addr.Bytes()...) +} + // get the key for delegator bond with candidate func GetDelegatorBondKey(delegatorAddr, candidateAddr sdk.Address, cdc *wire.Codec) []byte { return append(GetDelegatorBondsKey(delegatorAddr, cdc), candidateAddr.Bytes()...) diff --git a/x/stake/keeper_test.go b/x/stake/keeper_test.go index 51a3f5b46b..654c243029 100644 --- a/x/stake/keeper_test.go +++ b/x/stake/keeper_test.go @@ -266,178 +266,6 @@ func TestGetValidators(t *testing.T) { assert.Equal(t, candidates[3].Address, validators[1].Address, "%v", validators) } -// test the mechanism which keeps track of a validator set change -func TestGetAccUpdateValidators(t *testing.T) { - ctx, _, keeper := createTestInput(t, nil, false, 0) - params := defaultParams() - params.MaxValidators = 4 - keeper.setParams(ctx, params) - - amts := []int64{9, 8, 7, 10, 3} - var candidatesIn [5]Candidate - for i, amt := range amts { - candidatesIn[i] = Candidate{ - Address: addrVals[i], - PubKey: pks[i], - Assets: sdk.NewRat(amt), - Liabilities: sdk.NewRat(amt), - } - } - - // test from nothing to something - // candidate set: {} -> {c1, c3} - // validator set: {} -> {c1, c3} - // accUpdate set: {} -> {c1, c3} - acc := keeper.getAccUpdateValidators(ctx) - assert.Equal(t, 0, len(acc)) - keeper.setCandidate(ctx, candidatesIn[1]) - keeper.setCandidate(ctx, candidatesIn[3]) - _ = keeper.GetValidators(ctx) // to init recent validator set - acc = keeper.getAccUpdateValidators(ctx) - require.Equal(t, 2, len(acc)) - candidates := keeper.GetCandidates(ctx, 5) - require.Equal(t, 2, len(candidates)) - assert.Equal(t, candidates[0].validator(), acc[0]) - assert.Equal(t, candidates[1].validator(), acc[1]) - - // test identical - // {c1, c3} -> {c1, c3} - // {c1, c3} -> {c1, c3} - // {c1, c3} -> {c1, c3} - keeper.setCandidate(ctx, candidates[0]) - keeper.setCandidate(ctx, candidates[1]) - acc = keeper.getAccUpdateValidators(ctx) - require.Equal(t, 2, len(acc)) - candidates = keeper.GetCandidates(ctx, 5) - require.Equal(t, 2, len(candidates)) - assert.Equal(t, candidates[0].validator(), acc[0]) - assert.Equal(t, candidates[1].validator(), acc[1]) - - // test single value change - // {c1, c3} -> {c1', c3} - // {c1, c3} -> {c1', c3} - // {c1, c3} -> {c1', c3} - candidates[0].Assets = sdk.NewRat(600) - keeper.setCandidate(ctx, candidates[0]) - acc = keeper.getAccUpdateValidators(ctx) - require.Equal(t, 2, len(acc)) - candidates = keeper.GetCandidates(ctx, 5) - require.Equal(t, 2, len(candidates)) - assert.Equal(t, candidates[0].validator(), acc[0]) - assert.Equal(t, candidates[1].validator(), acc[1]) - - // test multiple value change - // {c1, c3} -> {c1', c3'} - // {c1, c3} -> {c1', c3'} - // {c1, c3} -> {c1', c3'} - candidates[0].Assets = sdk.NewRat(200) - candidates[1].Assets = sdk.NewRat(100) - keeper.setCandidate(ctx, candidates[0]) - keeper.setCandidate(ctx, candidates[1]) - acc = keeper.getAccUpdateValidators(ctx) - require.Equal(t, 2, len(acc)) - candidates = keeper.GetCandidates(ctx, 5) - require.Equal(t, 2, len(candidates)) - require.Equal(t, candidates[0].validator(), acc[0]) - require.Equal(t, candidates[1].validator(), acc[1]) - - // test validtor added at the beginning - // {c1, c3} -> {c0, c1, c3} - // {c1, c3} -> {c0, c1, c3} - // {c1, c3} -> {c0, c1, c3} - keeper.setCandidate(ctx, candidatesIn[0]) - acc = keeper.getAccUpdateValidators(ctx) - require.Equal(t, 3, len(acc)) - candidates = keeper.GetCandidates(ctx, 5) - require.Equal(t, 3, len(candidates)) - assert.Equal(t, candidates[0].validator(), acc[0]) - assert.Equal(t, candidates[1].validator(), acc[1]) - assert.Equal(t, candidates[2].validator(), acc[2]) - - // test validator added at the middle - // {c0, c1, c3} -> {c0, c1, c2, c3] - // {c0, c1, c3} -> {c0, c1, c2, c3} - // {c0, c1, c3} -> {c0, c1, c2, c3} - keeper.setCandidate(ctx, candidatesIn[2]) - acc = keeper.getAccUpdateValidators(ctx) - require.Equal(t, 4, len(acc)) - candidates = keeper.GetCandidates(ctx, 5) - require.Equal(t, 4, len(candidates)) - assert.Equal(t, candidates[0].validator(), acc[0]) - assert.Equal(t, candidates[1].validator(), acc[1]) - assert.Equal(t, candidates[2].validator(), acc[2]) - assert.Equal(t, candidates[3].validator(), acc[3]) - - // test candidate added at the end but not inserted in the valset - // {c0, c1, c2, c3} -> {c0, c1, c2, c3, c4} - // {c0, c1, c2, c3} -> {c0, c1, c2, c3} - // {c0, c1, c2, c3} -> {c0, c1, c2, c3} - keeper.setCandidate(ctx, candidatesIn[4]) - acc = keeper.getAccUpdateValidators(ctx) - require.Equal(t, 4, len(acc)) // max validator number is 4 - candidates = keeper.GetCandidates(ctx, 5) - require.Equal(t, 5, len(candidates)) - assert.Equal(t, candidates[0].validator(), acc[0]) - assert.Equal(t, candidates[1].validator(), acc[1]) - assert.Equal(t, candidates[2].validator(), acc[2]) - assert.Equal(t, candidates[3].validator(), acc[3]) - - // test candidate change its power but still not in the valset - // {c0, c1, c2, c3, c4} -> {c0, c1, c2, c3, c4} - // {c0, c1, c2, c3} -> {c0, c1, c2, c3} - // {c0, c1, c2, c3} -> {c0, c1, c2, c3} - candidatesIn[4].Assets = sdk.NewRat(5) - keeper.setCandidate(ctx, candidatesIn[4]) - acc = keeper.getAccUpdateValidators(ctx) - require.Equal(t, 4, len(acc)) - candidates = keeper.GetCandidates(ctx, 5) - require.Equal(t, 5, len(candidates)) - assert.Equal(t, candidates[0].validator(), acc[0]) - assert.Equal(t, candidates[1].validator(), acc[1]) - assert.Equal(t, candidates[2].validator(), acc[2]) - assert.Equal(t, candidates[3].validator(), acc[3]) - - // test candidate change its power and become a validator(pushing out an existing) - // {c0, c1, c2, c3, c4} -> {c0, c1, c2, c3, c4} - // {c0, c1, c2, c3} -> {c0, c1, c3, c4} - // {c0, c1, c2, c3} -> {c0, c1, c2, c3, c4} - candidates[4].Assets = sdk.NewRat(1000) - keeper.setCandidate(ctx, candidates[4]) - acc = keeper.getAccUpdateValidators(ctx) - require.Equal(t, 5, len(acc)) - candidates = keeper.GetCandidates(ctx, 5) - require.Equal(t, 5, len(candidates)) - assert.Equal(t, candidates[0].validator(), acc[0]) - assert.Equal(t, candidates[1].validator(), acc[1]) - assert.Equal(t, candidates[2].validator(), acc[2]) - assert.Equal(t, candidates[3].validator(), acc[3]) - assert.Equal(t, candidates[4].validator(), acc[4]) - - // test from something to nothing - // {c0, c1, c2, c3, c4} -> {} - // {c0, c1, c3, c4} -> {} - // {c0, c1, c2, c3, c4} -> {c0, c1, c2, c3, c4} - keeper.removeCandidate(ctx, candidates[0].Address) - keeper.removeCandidate(ctx, candidates[1].Address) - keeper.removeCandidate(ctx, candidates[2].Address) - keeper.removeCandidate(ctx, candidates[3].Address) - keeper.removeCandidate(ctx, candidates[4].Address) - acc = keeper.getAccUpdateValidators(ctx) - require.Equal(t, 5, len(acc)) - candidates = keeper.GetCandidates(ctx, 5) - require.Equal(t, 0, len(candidates)) - assert.Equal(t, candidatesIn[0].Address, acc[0].Address) - assert.Equal(t, int64(0), acc[0].VotingPower.Evaluate()) - assert.Equal(t, candidatesIn[1].Address, acc[1].Address) - assert.Equal(t, int64(0), acc[1].VotingPower.Evaluate()) - assert.Equal(t, candidatesIn[2].Address, acc[2].Address) - assert.Equal(t, int64(0), acc[2].VotingPower.Evaluate()) - assert.Equal(t, candidatesIn[3].Address, acc[3].Address) - assert.Equal(t, int64(0), acc[3].VotingPower.Evaluate()) - assert.Equal(t, candidatesIn[4].Address, acc[4].Address) - assert.Equal(t, int64(0), acc[4].VotingPower.Evaluate()) -} - // clear the tracked changes to the validator set func TestClearAccUpdateValidators(t *testing.T) { ctx, _, keeper := createTestInput(t, nil, false, 0) @@ -463,6 +291,208 @@ func TestClearAccUpdateValidators(t *testing.T) { assert.Equal(t, 0, len(acc)) } +// test the mechanism which keeps track of a validator set change +func TestGetAccUpdateValidators(t *testing.T) { + ctx, _, keeper := createTestInput(t, nil, false, 0) + params := defaultParams() + params.MaxValidators = 4 + keeper.setParams(ctx, params) + + // TODO eliminate use of candidatesIn here + // tests could be clearer if they just + // created the candidate at time of use + // and were labelled by power in the comments + // outlining in each test + amts := []int64{10, 11, 12, 13, 1} + var candidatesIn [5]Candidate + for i, amt := range amts { + candidatesIn[i] = Candidate{ + Address: addrs[i], + PubKey: pks[i], + Assets: sdk.NewRat(amt), + Liabilities: sdk.NewRat(amt), + } + } + + // test from nothing to something + // candidate set: {} -> {c1, c3} + // validator set: {} -> {c1, c3} + // accUpdate set: {} -> {c1, c3} + assert.Equal(t, 0, len(keeper.GetCandidates(ctx, 5))) + assert.Equal(t, 0, len(keeper.GetValidators(ctx))) + assert.Equal(t, 0, len(keeper.getAccUpdateValidators(ctx))) + + keeper.setCandidate(ctx, candidatesIn[1]) + keeper.setCandidate(ctx, candidatesIn[3]) + + vals := keeper.GetValidators(ctx) // to init recent validator set + require.Equal(t, 2, len(vals)) + acc := keeper.getAccUpdateValidators(ctx) + require.Equal(t, 2, len(acc)) + candidates := keeper.GetCandidates(ctx, 5) + require.Equal(t, 2, len(candidates)) + assert.Equal(t, candidates[0].validator(), acc[0]) + assert.Equal(t, candidates[1].validator(), acc[1]) + assert.Equal(t, candidates[0].validator(), vals[1]) + assert.Equal(t, candidates[1].validator(), vals[0]) + + // test identical, + // candidate set: {c1, c3} -> {c1, c3} + // accUpdate set: {} -> {} + keeper.clearAccUpdateValidators(ctx) + assert.Equal(t, 2, len(keeper.GetCandidates(ctx, 5))) + assert.Equal(t, 0, len(keeper.getAccUpdateValidators(ctx))) + + keeper.setCandidate(ctx, candidates[0]) + keeper.setCandidate(ctx, candidates[1]) + + require.Equal(t, 2, len(keeper.GetCandidates(ctx, 5))) + assert.Equal(t, 0, len(keeper.getAccUpdateValidators(ctx))) + + // test single value change + // candidate set: {c1, c3} -> {c1', c3} + // accUpdate set: {} -> {c1'} + keeper.clearAccUpdateValidators(ctx) + assert.Equal(t, 2, len(keeper.GetCandidates(ctx, 5))) + assert.Equal(t, 0, len(keeper.getAccUpdateValidators(ctx))) + + candidates[0].Assets = sdk.NewRat(600) + keeper.setCandidate(ctx, candidates[0]) + + candidates = keeper.GetCandidates(ctx, 5) + require.Equal(t, 2, len(candidates)) + assert.True(t, candidates[0].Assets.Equal(sdk.NewRat(600))) + acc = keeper.getAccUpdateValidators(ctx) + require.Equal(t, 1, len(acc)) + assert.Equal(t, candidates[0].validator(), acc[0]) + + // test multiple value change + // candidate set: {c1, c3} -> {c1', c3'} + // accUpdate set: {c1, c3} -> {c1', c3'} + keeper.clearAccUpdateValidators(ctx) + assert.Equal(t, 2, len(keeper.GetCandidates(ctx, 5))) + assert.Equal(t, 0, len(keeper.getAccUpdateValidators(ctx))) + + candidates[0].Assets = sdk.NewRat(200) + candidates[1].Assets = sdk.NewRat(100) + keeper.setCandidate(ctx, candidates[0]) + keeper.setCandidate(ctx, candidates[1]) + + acc = keeper.getAccUpdateValidators(ctx) + require.Equal(t, 2, len(acc)) + candidates = keeper.GetCandidates(ctx, 5) + require.Equal(t, 2, len(candidates)) + require.Equal(t, candidates[0].validator(), acc[0]) + require.Equal(t, candidates[1].validator(), acc[1]) + + // test validtor added at the beginning + // candidate set: {c1, c3} -> {c0, c1, c3} + // accUpdate set: {} -> {c0} + keeper.clearAccUpdateValidators(ctx) + assert.Equal(t, 2, len(keeper.GetCandidates(ctx, 5))) + assert.Equal(t, 0, len(keeper.getAccUpdateValidators(ctx))) + + keeper.setCandidate(ctx, candidatesIn[0]) + acc = keeper.getAccUpdateValidators(ctx) + require.Equal(t, 1, len(acc)) + candidates = keeper.GetCandidates(ctx, 5) + require.Equal(t, 3, len(candidates)) + assert.Equal(t, candidates[0].validator(), acc[0]) + + // test validator added at the middle + // candidate set: {c0, c1, c3} -> {c0, c1, c2, c3] + // accUpdate set: {} -> {c2} + keeper.clearAccUpdateValidators(ctx) + assert.Equal(t, 3, len(keeper.GetCandidates(ctx, 5))) + assert.Equal(t, 0, len(keeper.getAccUpdateValidators(ctx))) + + keeper.setCandidate(ctx, candidatesIn[2]) + acc = keeper.getAccUpdateValidators(ctx) + require.Equal(t, 1, len(acc)) + candidates = keeper.GetCandidates(ctx, 5) + require.Equal(t, 4, len(candidates)) + assert.Equal(t, candidates[2].validator(), acc[0]) + + // test candidate added at the end but not inserted in the valset + // candidate set: {c0, c1, c2, c3} -> {c0, c1, c2, c3, c4} + // validator set: {c0, c1, c2, c3} -> {c0, c1, c2, c3} + // accUpdate set: {} -> {} + keeper.clearAccUpdateValidators(ctx) + assert.Equal(t, 4, len(keeper.GetCandidates(ctx, 5))) + assert.Equal(t, 4, len(keeper.GetValidators(ctx))) + assert.Equal(t, 0, len(keeper.getAccUpdateValidators(ctx))) + + keeper.setCandidate(ctx, candidatesIn[4]) + + assert.Equal(t, 5, len(keeper.GetCandidates(ctx, 5))) + assert.Equal(t, 4, len(keeper.GetValidators(ctx))) + require.Equal(t, 0, len(keeper.getAccUpdateValidators(ctx))) // max validator number is 4 + + // test candidate change its power but still not in the valset + // candidate set: {c0, c1, c2, c3, c4} -> {c0, c1, c2, c3, c4} + // validator set: {c0, c1, c2, c3} -> {c0, c1, c2, c3} + // accUpdate set: {} -> {} + keeper.clearAccUpdateValidators(ctx) + assert.Equal(t, 5, len(keeper.GetCandidates(ctx, 5))) + assert.Equal(t, 4, len(keeper.GetValidators(ctx))) + assert.Equal(t, 0, len(keeper.getAccUpdateValidators(ctx))) + + candidatesIn[4].Assets = sdk.NewRat(1) + keeper.setCandidate(ctx, candidatesIn[4]) + + assert.Equal(t, 5, len(keeper.GetCandidates(ctx, 5))) + assert.Equal(t, 4, len(keeper.GetValidators(ctx))) + require.Equal(t, 0, len(keeper.getAccUpdateValidators(ctx))) // max validator number is 4 + + // test candidate change its power and become a validator(pushing out an existing) + // candidate set: {c0, c1, c2, c3, c4} -> {c0, c1, c2, c3, c4} + // validator set: {c0, c1, c2, c3} -> {c1, c2, c3, c4} + // accUpdate set: {} -> {c0, c4} + keeper.clearAccUpdateValidators(ctx) + assert.Equal(t, 5, len(keeper.GetCandidates(ctx, 5))) + assert.Equal(t, 4, len(keeper.GetValidators(ctx))) + assert.Equal(t, 0, len(keeper.getAccUpdateValidators(ctx))) + + candidatesIn[4].Assets = sdk.NewRat(1000) + keeper.setCandidate(ctx, candidatesIn[4]) + + candidates = keeper.GetCandidates(ctx, 5) + require.Equal(t, 5, len(candidates)) + vals = keeper.GetValidators(ctx) + require.Equal(t, 4, len(vals)) + acc = keeper.getAccUpdateValidators(ctx) + require.Equal(t, 2, len(acc), "%v", acc) + + assert.Equal(t, candidatesIn[0].Address, acc[0].Address) + assert.True(t, acc[0].VotingPower.Equal(sdk.ZeroRat)) + assert.Equal(t, vals[0], acc[1]) + + // test from something to nothing + // candidate set: {c0, c1, c2, c3, c4} -> {} + // validator set: {c0, c1, c3, c4} -> {} + // accUpdate set: {} -> {c0, c1, c2, c3, c4} + keeper.clearAccUpdateValidators(ctx) + keeper.removeCandidate(ctx, candidates[0].Address) + keeper.removeCandidate(ctx, candidates[1].Address) + keeper.removeCandidate(ctx, candidates[2].Address) + keeper.removeCandidate(ctx, candidates[3].Address) + keeper.removeCandidate(ctx, candidates[4].Address) + acc = keeper.getAccUpdateValidators(ctx) + require.Equal(t, 5, len(acc)) + candidates = keeper.GetCandidates(ctx, 5) + require.Equal(t, 0, len(candidates)) + assert.Equal(t, candidatesIn[0].Address, acc[0].Address) + assert.Equal(t, int64(0), acc[0].VotingPower.Evaluate()) + assert.Equal(t, candidatesIn[1].Address, acc[1].Address) + assert.Equal(t, int64(0), acc[1].VotingPower.Evaluate()) + assert.Equal(t, candidatesIn[2].Address, acc[2].Address) + assert.Equal(t, int64(0), acc[2].VotingPower.Evaluate()) + assert.Equal(t, candidatesIn[3].Address, acc[3].Address) + assert.Equal(t, int64(0), acc[3].VotingPower.Evaluate()) + assert.Equal(t, candidatesIn[4].Address, acc[4].Address) + assert.Equal(t, int64(0), acc[4].VotingPower.Evaluate()) +} + // test if is a validator from the last update func TestIsRecentValidator(t *testing.T) { ctx, _, keeper := createTestInput(t, nil, false, 0)