From 4b0c367afadbab84ca59f1cf23ec5f799d3b791c Mon Sep 17 00:00:00 2001 From: mossid Date: Wed, 28 Mar 2018 20:12:21 +0200 Subject: [PATCH 1/8] keeper bugfixes, bit a pair programin w joon in progress in progress --- x/stake/keeper.go | 46 +++++++++++++++-- x/stake/keeper_keys.go | 4 +- x/stake/keeper_test.go | 111 ++++++++++++++++++++++++++++++++++++++--- 3 files changed, 147 insertions(+), 14 deletions(-) diff --git a/x/stake/keeper.go b/x/stake/keeper.go index af2015fe81..8aa539116f 100644 --- a/x/stake/keeper.go +++ b/x/stake/keeper.go @@ -1,6 +1,8 @@ package stake import ( + "bytes" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/wire" "github.com/cosmos/cosmos-sdk/x/bank" @@ -94,11 +96,20 @@ 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 { - return + updateAcc := false + if store.Get(GetRecentValidatorKey(address)) != nil { + updateAcc = true } - store.Set(GetAccUpdateValidatorKey(validator.Address), bz) + // test if this is a new validator + if k.isNewValidator(ctx, store, address) { + updateAcc = true + } + + if updateAcc { + store.Set(GetAccUpdateValidatorKey(validator.Address), bz) + } + return } func (k Keeper) removeCandidate(ctx sdk.Context, address sdk.Address) { @@ -141,7 +152,7 @@ func (k Keeper) GetValidators(ctx sdk.Context) (validators []Validator) { // add the actual validator power sorted store maxVal := k.GetParams(ctx).MaxValidators - iterator := store.ReverseIterator(subspace(ValidatorsKey)) //smallest to largest + iterator := store.ReverseIterator(subspace(ValidatorsKey)) // largest to smallest validators = make([]Validator, maxVal) i := 0 for ; ; i++ { @@ -166,6 +177,33 @@ func (k Keeper) GetValidators(ctx sdk.Context) (validators []Validator) { return validators[:i] // trim } +// TODO this is madly inefficient because need to call every time we set a candidate +// Should use something better than an iterator maybe? +// Used to determine if something has just been added to the actual validator set +func (k Keeper) isNewValidator(ctx sdk.Context, store sdk.KVStore, address sdk.Address) bool { + // add the actual validator power sorted store + maxVal := k.GetParams(ctx).MaxValidators + iterator := store.ReverseIterator(subspace(ValidatorsKey)) // largest to smallest + for i := 0; ; i++ { + if !iterator.Valid() || i > int(maxVal-1) { + iterator.Close() + break + } + bz := iterator.Value() + var val Validator + err := k.cdc.UnmarshalBinary(bz, &val) + if err != nil { + panic(err) + } + if bytes.Equal(val.Address, address) { + return true + } + iterator.Next() + } + + return false +} + // Is the address provided a part of the most recently saved validator group? func (k Keeper) IsRecentValidator(ctx sdk.Context, address sdk.Address) bool { store := ctx.KVStore(k.storeKey) diff --git a/x/stake/keeper_keys.go b/x/stake/keeper_keys.go index 051994456e..5c09a47fc4 100644 --- a/x/stake/keeper_keys.go +++ b/x/stake/keeper_keys.go @@ -15,9 +15,9 @@ var ( CandidatesKey = []byte{0x02} // prefix for each key to a candidate 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{0x04} // prefix for each key to the last updated validator group + RecentValidatorsKey = []byte{0x05} // prefix for each key to the last updated validator group - DelegatorBondKeyPrefix = []byte{0x05} // prefix for each key to a delegator's bond + DelegatorBondKeyPrefix = []byte{0x06} // prefix for each key to a delegator's bond ) const maxDigitsForAccount = 12 // ~220,000,000 atoms created at launch diff --git a/x/stake/keeper_test.go b/x/stake/keeper_test.go index 6e7478957f..a657895290 100644 --- a/x/stake/keeper_test.go +++ b/x/stake/keeper_test.go @@ -2,6 +2,7 @@ package stake import ( "bytes" + "fmt" "testing" sdk "github.com/cosmos/cosmos-sdk/types" @@ -262,21 +263,115 @@ func TestGetValidators(t *testing.T) { // TODO // test the mechanism which keeps track of a validator set change func TestGetAccUpdateValidators(t *testing.T) { + ctx, _, keeper := createTestInput(t, nil, false, 0) + + validatorsEqual := func(t *testing.T, expected []Validator, actual []Validator) { + require.Equal(t, len(expected), len(actual)) + for i := 0; i < len(expected); i++ { + assert.Equal(t, expected[i], actual[i]) + } + } + + amts := []int64{100, 300} + genCandidates := func(amts []int64) ([]Candidate, []Validator) { + candidates := make([]Candidate, len(amts)) + validators := make([]Validator, len(amts)) + for i := 0; i < len(amts); i++ { + c := Candidate{ + Status: Unbonded, + PubKey: pks[i], + Address: addrs[i], + Assets: sdk.NewRat(amts[i]), + Liabilities: sdk.NewRat(amts[i]), + } + candidates[i] = c + validators[i] = c.validator() + } + return candidates, validators + } + + candidates, validators := genCandidates(amts) + //TODO // test from nothing to something - // test from something to nothing + acc := keeper.getAccUpdateValidators(ctx) + assert.Equal(t, 0, len(acc)) + keeper.setCandidate(ctx, candidates[0]) + keeper.setCandidate(ctx, candidates[1]) + //_ = keeper.GetValidators(ctx) // to init recent validator set + acc = keeper.getAccUpdateValidators(ctx) + validatorsEqual(t, validators, acc) + // test identical - // test single value change - // test multiple value change - // test validator added at the beginning - // test validator added in the middle - // test validator added at the end - // test multiple validators removed + keeper.setCandidate(ctx, candidates[0]) + keeper.setCandidate(ctx, candidates[1]) + acc = keeper.getAccUpdateValidators(ctx) + validatorsEqual(t, validators, acc) + + acc = keeper.getAccUpdateValidators(ctx) + fmt.Printf("%+v\n", acc) + + // test from something to nothing + keeper.removeCandidate(ctx, candidates[0].Address) + keeper.removeCandidate(ctx, candidates[1].Address) + acc = keeper.getAccUpdateValidators(ctx) + fmt.Printf("%+v\n", acc) + assert.Equal(t, 2, len(acc)) + assert.Equal(t, validators[0].Address, acc[0].Address) + assert.Equal(t, 0, acc[0].VotingPower.Evaluate()) + assert.Equal(t, validators[1].Address, acc[1].Address) + assert.Equal(t, 0, acc[1].VotingPower.Evaluate()) + + //// test single value change + //amts[0] = 600 + //candidates, validators = genCandidates(amts) + //setCandidates(ctx, candidates) + //acc = keeper.getAccUpdateValidators(ctx) + //validatorsEqual(t, validators, acc) + + //// test multiple value change + //amts[0] = 200 + //amts[1] = 0 + //candidates, validators = genCandidates(amts) + //setCandidates(ctx, candidates) + //acc = keeper.getAccUpdateValidators(ctx) + //validatorsEqual(t, validators, acc) + + //// test validator added at the beginning + //// test validator added in the middle + //// test validator added at the end + //amts = append(amts, 100) + //candidates, validators = genCandidates(amts) + //setCandidates(ctx, candidates) + //acc = keeper.getAccUpdateValidators(ctx) + //validatorsEqual(t, validators, acc) + + //// test multiple validators removed } // clear the tracked changes to the validator set func TestClearAccUpdateValidators(t *testing.T) { - //TODO + ctx, _, keeper := createTestInput(t, nil, false, 0) + + amts := []int64{0, 400} + candidates := make([]Candidate, len(amts)) + for i, amt := range amts { + c := Candidate{ + Status: Unbonded, + PubKey: pks[i], + Address: addrs[i], + Assets: sdk.NewRat(amt), + Liabilities: sdk.NewRat(amt), + } + candidates[i] = c + keeper.setCandidate(ctx, c) + } + + acc := keeper.getAccUpdateValidators(ctx) + assert.Equal(t, len(amts), len(acc)) + keeper.clearAccUpdateValidators(ctx) + acc = keeper.getAccUpdateValidators(ctx) + assert.Equal(t, 0, len(acc)) } // test if is a validator from the last update From 67a943d9dfc438b897b143a9bf884dafc4ba0aea Mon Sep 17 00:00:00 2001 From: mossid Date: Thu, 29 Mar 2018 19:37:04 +0200 Subject: [PATCH 2/8] write test for keeper --- x/stake/keeper_test.go | 85 ++++++++++++++++++++++++++---------------- 1 file changed, 52 insertions(+), 33 deletions(-) diff --git a/x/stake/keeper_test.go b/x/stake/keeper_test.go index a657895290..a78fd1b521 100644 --- a/x/stake/keeper_test.go +++ b/x/stake/keeper_test.go @@ -2,7 +2,6 @@ package stake import ( "bytes" - "fmt" "testing" sdk "github.com/cosmos/cosmos-sdk/types" @@ -31,14 +30,14 @@ var ( candidate2 = Candidate{ Address: addrVal2, PubKey: pk2, - Assets: sdk.NewRat(9), - Liabilities: sdk.NewRat(9), + Assets: sdk.NewRat(8), + Liabilities: sdk.NewRat(8), } candidate3 = Candidate{ Address: addrVal3, PubKey: pk3, - Assets: sdk.NewRat(9), - Liabilities: sdk.NewRat(9), + Assets: sdk.NewRat(7), + Liabilities: sdk.NewRat(7), } ) @@ -298,7 +297,7 @@ func TestGetAccUpdateValidators(t *testing.T) { assert.Equal(t, 0, len(acc)) keeper.setCandidate(ctx, candidates[0]) keeper.setCandidate(ctx, candidates[1]) - //_ = keeper.GetValidators(ctx) // to init recent validator set + _ = keeper.GetValidators(ctx) // to init recent validator set acc = keeper.getAccUpdateValidators(ctx) validatorsEqual(t, validators, acc) @@ -309,44 +308,46 @@ func TestGetAccUpdateValidators(t *testing.T) { validatorsEqual(t, validators, acc) acc = keeper.getAccUpdateValidators(ctx) - fmt.Printf("%+v\n", acc) // test from something to nothing keeper.removeCandidate(ctx, candidates[0].Address) keeper.removeCandidate(ctx, candidates[1].Address) acc = keeper.getAccUpdateValidators(ctx) - fmt.Printf("%+v\n", acc) assert.Equal(t, 2, len(acc)) assert.Equal(t, validators[0].Address, acc[0].Address) - assert.Equal(t, 0, acc[0].VotingPower.Evaluate()) + assert.Equal(t, int64(0), acc[0].VotingPower.Evaluate()) assert.Equal(t, validators[1].Address, acc[1].Address) - assert.Equal(t, 0, acc[1].VotingPower.Evaluate()) + assert.Equal(t, int64(0), acc[1].VotingPower.Evaluate()) - //// test single value change - //amts[0] = 600 - //candidates, validators = genCandidates(amts) - //setCandidates(ctx, candidates) - //acc = keeper.getAccUpdateValidators(ctx) - //validatorsEqual(t, validators, acc) + // test single value change + amts[0] = 600 + candidates, validators = genCandidates(amts) + keeper.setCandidate(ctx, candidates[0]) + keeper.setCandidate(ctx, candidates[1]) + acc = keeper.getAccUpdateValidators(ctx) + validatorsEqual(t, validators, acc) - //// test multiple value change - //amts[0] = 200 - //amts[1] = 0 - //candidates, validators = genCandidates(amts) - //setCandidates(ctx, candidates) - //acc = keeper.getAccUpdateValidators(ctx) - //validatorsEqual(t, validators, acc) + // test multiple value change + amts[0] = 200 + amts[1] = 0 + candidates, validators = genCandidates(amts) + keeper.setCandidate(ctx, candidates[0]) + keeper.setCandidate(ctx, candidates[1]) + acc = keeper.getAccUpdateValidators(ctx) + validatorsEqual(t, validators, acc) - //// test validator added at the beginning - //// test validator added in the middle - //// test validator added at the end - //amts = append(amts, 100) - //candidates, validators = genCandidates(amts) - //setCandidates(ctx, candidates) - //acc = keeper.getAccUpdateValidators(ctx) - //validatorsEqual(t, validators, acc) + // test validator added at the beginning + // test validator added in the middle + // test validator added at the end + amts = append(amts, 100) + candidates, validators = genCandidates(amts) + keeper.setCandidate(ctx, candidates[0]) + keeper.setCandidate(ctx, candidates[1]) + keeper.setCandidate(ctx, candidates[2]) + acc = keeper.getAccUpdateValidators(ctx) + validatorsEqual(t, validators, acc) - //// test multiple validators removed + // test multiple validators removed } // clear the tracked changes to the validator set @@ -376,14 +377,32 @@ func TestClearAccUpdateValidators(t *testing.T) { // test if is a validator from the last update func TestIsRecentValidator(t *testing.T) { - //TODO + ctx, _, keeper := createTestInput(t, nil, false, 0) // test that an empty validator set doesn't have any validators + validators := keeper.GetValidators(ctx) + assert.Equal(t, 0, len(validators)) + // get the validators for the first time + keeper.setCandidate(ctx, candidate1) + keeper.setCandidate(ctx, candidate2) + validators = keeper.GetValidators(ctx) + require.Equal(t, 2, len(validators)) + assert.Equal(t, candidate1.validator(), validators[0]) + assert.Equal(t, candidate2.validator(), validators[1]) + // test a basic retrieve of something that should be a recent validator + assert.True(t, keeper.IsRecentValidator(ctx, candidate1.Address)) + assert.True(t, keeper.IsRecentValidator(ctx, candidate2.Address)) + // test a basic retrieve of something that should not be a recent validator + assert.False(t, keeper.IsRecentValidator(ctx, candidate3.Address)) + // remove that validator, but don't retrieve the recent validator group + keeper.removeCandidate(ctx, candidate1.Address) + // test that removed validator is not considered a recent validator + assert.False(t, keeper.IsRecentValidator(ctx, candidate1.Address)) } func TestParams(t *testing.T) { From 77e73334b7373e5f90ea5956018a2adb6edfddae Mon Sep 17 00:00:00 2001 From: mossid Date: Thu, 29 Mar 2018 20:23:23 +0200 Subject: [PATCH 3/8] add test for inserting validator at the beginning/middle --- x/stake/keeper_test.go | 77 ++++++++++++++++++++++++++++-------------- 1 file changed, 51 insertions(+), 26 deletions(-) diff --git a/x/stake/keeper_test.go b/x/stake/keeper_test.go index a78fd1b521..d02871af33 100644 --- a/x/stake/keeper_test.go +++ b/x/stake/keeper_test.go @@ -17,9 +17,13 @@ var ( addrVal1 = addrs[2] addrVal2 = addrs[3] addrVal3 = addrs[4] + addrVal4 = addrs[5] + addrVal5 = addrs[6] pk1 = crypto.GenPrivKeyEd25519().PubKey() pk2 = crypto.GenPrivKeyEd25519().PubKey() pk3 = crypto.GenPrivKeyEd25519().PubKey() + pk4 = crypto.GenPrivKeyEd25519().PubKey() + pk5 = crypto.GenPrivKeyEd25519().PubKey() candidate1 = Candidate{ Address: addrVal1, @@ -39,6 +43,18 @@ var ( Assets: sdk.NewRat(7), Liabilities: sdk.NewRat(7), } + candidate4 = Candidate{ + Address: addrVal4, + PubKey: pk4, + Assets: sdk.NewRat(10), + Liabilities: sdk.NewRat(10), + } + candidate5 = Candidate{ + Address: addrVal5, + PubKey: pk5, + Assets: sdk.NewRat(6), + Liabilities: sdk.NewRat(6), + } ) // This function tests GetCandidate, GetCandidates, setCandidate, removeCandidate @@ -271,25 +287,17 @@ func TestGetAccUpdateValidators(t *testing.T) { } } - amts := []int64{100, 300} - genCandidates := func(amts []int64) ([]Candidate, []Validator) { - candidates := make([]Candidate, len(amts)) - validators := make([]Validator, len(amts)) - for i := 0; i < len(amts); i++ { - c := Candidate{ - Status: Unbonded, - PubKey: pks[i], - Address: addrs[i], - Assets: sdk.NewRat(amts[i]), - Liabilities: sdk.NewRat(amts[i]), - } - candidates[i] = c + genValidators := func(candidates []Candidate) []Validator { + validators := make([]Validator, len(candidates)) + for i, c := range candidates { validators[i] = c.validator() } - return candidates, validators + + return validators } - candidates, validators := genCandidates(amts) + candidates := []Candidate{candidate2, candidate4} + validators := genValidators(candidates) //TODO // test from nothing to something @@ -320,41 +328,58 @@ func TestGetAccUpdateValidators(t *testing.T) { assert.Equal(t, int64(0), acc[1].VotingPower.Evaluate()) // test single value change - amts[0] = 600 - candidates, validators = genCandidates(amts) + candidates[0].Assets = sdk.NewRat(600) + validators = genValidators(candidates) keeper.setCandidate(ctx, candidates[0]) keeper.setCandidate(ctx, candidates[1]) acc = keeper.getAccUpdateValidators(ctx) validatorsEqual(t, validators, acc) // test multiple value change - amts[0] = 200 - amts[1] = 0 - candidates, validators = genCandidates(amts) + candidates[0].Assets = sdk.NewRat(200) + candidates[1].Assets = sdk.NewRat(0) + validators = genValidators(candidates) keeper.setCandidate(ctx, candidates[0]) keeper.setCandidate(ctx, candidates[1]) acc = keeper.getAccUpdateValidators(ctx) validatorsEqual(t, validators, acc) // test validator added at the beginning - // test validator added in the middle - // test validator added at the end - amts = append(amts, 100) - candidates, validators = genCandidates(amts) + candidates = append([]Candidate{candidate1}, candidates...) + validators = genValidators(candidates) keeper.setCandidate(ctx, candidates[0]) keeper.setCandidate(ctx, candidates[1]) keeper.setCandidate(ctx, candidates[2]) acc = keeper.getAccUpdateValidators(ctx) validatorsEqual(t, validators, acc) - // test multiple validators removed + // test validator added at the middle + candidates = []Candidate{candidates[0], candidates[1], candidate3, candidates[2]} + validators = genValidators(candidates) + keeper.setCandidate(ctx, candidates[0]) + keeper.setCandidate(ctx, candidates[1]) + keeper.setCandidate(ctx, candidates[2]) + keeper.setCandidate(ctx, candidates[3]) + acc = keeper.getAccUpdateValidators(ctx) + validatorsEqual(t, validators, acc) + + // test validator added at the end + candidates = append(candidates, candidate5) + validators = genValidators(candidates) + keeper.setCandidate(ctx, candidates[0]) + keeper.setCandidate(ctx, candidates[1]) + keeper.setCandidate(ctx, candidates[2]) + keeper.setCandidate(ctx, candidates[3]) + keeper.setCandidate(ctx, candidates[4]) + acc = keeper.getAccUpdateValidators(ctx) + validatorsEqual(t, validators, acc) } // clear the tracked changes to the validator set func TestClearAccUpdateValidators(t *testing.T) { ctx, _, keeper := createTestInput(t, nil, false, 0) - amts := []int64{0, 400} + amts := []int64{100, 400, 200} candidates := make([]Candidate, len(amts)) for i, amt := range amts { c := Candidate{ From 1c079199e86238f37fb3dce0e7886bb3fa622abe Mon Sep 17 00:00:00 2001 From: mossid Date: Thu, 29 Mar 2018 20:29:54 +0200 Subject: [PATCH 4/8] remove some TODO tags --- x/stake/keeper_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/x/stake/keeper_test.go b/x/stake/keeper_test.go index d02871af33..c77985be71 100644 --- a/x/stake/keeper_test.go +++ b/x/stake/keeper_test.go @@ -275,7 +275,6 @@ func TestGetValidators(t *testing.T) { assert.Equal(t, candidates[3].Address, validators[1].Address, "%v", validators) } -// TODO // test the mechanism which keeps track of a validator set change func TestGetAccUpdateValidators(t *testing.T) { ctx, _, keeper := createTestInput(t, nil, false, 0) @@ -299,7 +298,6 @@ func TestGetAccUpdateValidators(t *testing.T) { candidates := []Candidate{candidate2, candidate4} validators := genValidators(candidates) - //TODO // test from nothing to something acc := keeper.getAccUpdateValidators(ctx) assert.Equal(t, 0, len(acc)) From daf5fb9a13aa243757efd28843a7d079ddb1451e Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Fri, 30 Mar 2018 20:23:16 +0200 Subject: [PATCH 5/8] change use of global candidates in progress in progress done --- x/stake/keeper.go | 15 +- x/stake/keeper_test.go | 382 +++++++++++++++++++++++++---------------- 2 files changed, 238 insertions(+), 159 deletions(-) diff --git a/x/stake/keeper.go b/x/stake/keeper.go index 8aa539116f..dd56b94aa4 100644 --- a/x/stake/keeper.go +++ b/x/stake/keeper.go @@ -96,17 +96,7 @@ 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 - updateAcc := false - if store.Get(GetRecentValidatorKey(address)) != nil { - updateAcc = true - } - - // test if this is a new validator - if k.isNewValidator(ctx, store, address) { - updateAcc = true - } - - if updateAcc { + if store.Get(GetRecentValidatorKey(address)) != nil || k.isNewValidator(ctx, store, address) { store.Set(GetAccUpdateValidatorKey(validator.Address), bz) } return @@ -126,13 +116,14 @@ func (k Keeper) removeCandidate(ctx sdk.Context, address sdk.Address) { // delete from recent and power weighted validator groups if the validator // exists and add validator with zero power to the validator updates - if store.Get(GetRecentValidatorKey(address)) == nil { + if store.Get(GetRecentValidatorKey(address)) == nil && !k.isNewValidator(ctx, store, address) { return } bz, err := k.cdc.MarshalBinary(Validator{address, sdk.ZeroRat}) if err != nil { panic(err) } + store.Set(GetAccUpdateValidatorKey(address), bz) store.Delete(GetRecentValidatorKey(address)) store.Delete(GetValidatorKey(address, oldCandidate.Assets, k.cdc)) diff --git a/x/stake/keeper_test.go b/x/stake/keeper_test.go index c77985be71..f5b1d038f6 100644 --- a/x/stake/keeper_test.go +++ b/x/stake/keeper_test.go @@ -5,55 +5,22 @@ import ( "testing" sdk "github.com/cosmos/cosmos-sdk/types" - crypto "github.com/tendermint/go-crypto" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) var ( - addrDel1 = addrs[0] - addrDel2 = addrs[1] - addrVal1 = addrs[2] - addrVal2 = addrs[3] - addrVal3 = addrs[4] - addrVal4 = addrs[5] - addrVal5 = addrs[6] - pk1 = crypto.GenPrivKeyEd25519().PubKey() - pk2 = crypto.GenPrivKeyEd25519().PubKey() - pk3 = crypto.GenPrivKeyEd25519().PubKey() - pk4 = crypto.GenPrivKeyEd25519().PubKey() - pk5 = crypto.GenPrivKeyEd25519().PubKey() - - candidate1 = Candidate{ - Address: addrVal1, - PubKey: pk1, - Assets: sdk.NewRat(9), - Liabilities: sdk.NewRat(9), + addrDels = []sdk.Address{ + addrs[0], + addrs[1], } - candidate2 = Candidate{ - Address: addrVal2, - PubKey: pk2, - Assets: sdk.NewRat(8), - Liabilities: sdk.NewRat(8), - } - candidate3 = Candidate{ - Address: addrVal3, - PubKey: pk3, - Assets: sdk.NewRat(7), - Liabilities: sdk.NewRat(7), - } - candidate4 = Candidate{ - Address: addrVal4, - PubKey: pk4, - Assets: sdk.NewRat(10), - Liabilities: sdk.NewRat(10), - } - candidate5 = Candidate{ - Address: addrVal5, - PubKey: pk5, - Assets: sdk.NewRat(6), - Liabilities: sdk.NewRat(6), + addrVals = []sdk.Address{ + addrs[2], + addrs[3], + addrs[4], + addrs[5], + addrs[6], } ) @@ -61,6 +28,18 @@ var ( func TestCandidate(t *testing.T) { ctx, _, keeper := createTestInput(t, nil, false, 0) + //construct the candidates + var candidates [3]Candidate + amts := []int64{9, 8, 7} + for i, amt := range amts { + candidates[i] = Candidate{ + Address: addrVals[i], + PubKey: pks[i], + Assets: sdk.NewRat(amt), + Liabilities: sdk.NewRat(amt), + } + } + candidatesEqual := func(c1, c2 Candidate) bool { return c1.Status == c2.Status && c1.PubKey.Equals(c2.PubKey) && @@ -71,47 +50,47 @@ func TestCandidate(t *testing.T) { } // check the empty keeper first - _, found := keeper.GetCandidate(ctx, addrVal1) + _, found := keeper.GetCandidate(ctx, addrVals[0]) assert.False(t, found) resCands := keeper.GetCandidates(ctx, 100) assert.Zero(t, len(resCands)) // set and retrieve a record - keeper.setCandidate(ctx, candidate1) - resCand, found := keeper.GetCandidate(ctx, addrVal1) + keeper.setCandidate(ctx, candidates[0]) + resCand, found := keeper.GetCandidate(ctx, addrVals[0]) require.True(t, found) - assert.True(t, candidatesEqual(candidate1, resCand), "%v \n %v", resCand, candidate1) + assert.True(t, candidatesEqual(candidates[0], resCand), "%v \n %v", resCand, candidates[0]) // modify a records, save, and retrieve - candidate1.Liabilities = sdk.NewRat(99) - keeper.setCandidate(ctx, candidate1) - resCand, found = keeper.GetCandidate(ctx, addrVal1) + candidates[0].Liabilities = sdk.NewRat(99) + keeper.setCandidate(ctx, candidates[0]) + resCand, found = keeper.GetCandidate(ctx, addrVals[0]) require.True(t, found) - assert.True(t, candidatesEqual(candidate1, resCand)) + assert.True(t, candidatesEqual(candidates[0], resCand)) // also test that the address has been added to address list resCands = keeper.GetCandidates(ctx, 100) require.Equal(t, 1, len(resCands)) - assert.Equal(t, addrVal1, resCands[0].Address) + assert.Equal(t, addrVals[0], resCands[0].Address) // add other candidates - keeper.setCandidate(ctx, candidate2) - keeper.setCandidate(ctx, candidate3) - resCand, found = keeper.GetCandidate(ctx, addrVal2) + keeper.setCandidate(ctx, candidates[1]) + keeper.setCandidate(ctx, candidates[2]) + resCand, found = keeper.GetCandidate(ctx, addrVals[1]) require.True(t, found) - assert.True(t, candidatesEqual(candidate2, resCand), "%v \n %v", resCand, candidate2) - resCand, found = keeper.GetCandidate(ctx, addrVal3) + assert.True(t, candidatesEqual(candidates[1], resCand), "%v \n %v", resCand, candidates[1]) + resCand, found = keeper.GetCandidate(ctx, addrVals[2]) require.True(t, found) - assert.True(t, candidatesEqual(candidate3, resCand), "%v \n %v", resCand, candidate3) + assert.True(t, candidatesEqual(candidates[2], resCand), "%v \n %v", resCand, candidates[2]) resCands = keeper.GetCandidates(ctx, 100) require.Equal(t, 3, len(resCands)) - assert.True(t, candidatesEqual(candidate1, resCands[0]), "%v \n %v", resCands[0], candidate1) - assert.True(t, candidatesEqual(candidate2, resCands[1]), "%v \n %v", resCands[1], candidate2) - assert.True(t, candidatesEqual(candidate3, resCands[2]), "%v \n %v", resCands[2], candidate3) + assert.True(t, candidatesEqual(candidates[0], resCands[0]), "%v \n %v", resCands[0], candidates[0]) + assert.True(t, candidatesEqual(candidates[1], resCands[1]), "%v \n %v", resCands[1], candidates[1]) + assert.True(t, candidatesEqual(candidates[2], resCands[2]), "%v \n %v", resCands[2], candidates[2]) // remove a record - keeper.removeCandidate(ctx, candidate2.Address) - _, found = keeper.GetCandidate(ctx, addrVal2) + keeper.removeCandidate(ctx, candidates[1].Address) + _, found = keeper.GetCandidate(ctx, addrVals[1]) assert.False(t, found) } @@ -119,12 +98,24 @@ func TestCandidate(t *testing.T) { func TestBond(t *testing.T) { ctx, _, keeper := createTestInput(t, nil, false, 0) - // first add a candidate1 to delegate too - keeper.setCandidate(ctx, candidate1) + //construct the candidates + amts := []int64{9, 8, 7} + var candidates [3]Candidate + for i, amt := range amts { + candidates[i] = Candidate{ + Address: addrVals[i], + PubKey: pks[i], + Assets: sdk.NewRat(amt), + Liabilities: sdk.NewRat(amt), + } + } + + // first add a candidates[0] to delegate too + keeper.setCandidate(ctx, candidates[0]) bond1to1 := DelegatorBond{ - DelegatorAddr: addrDel1, - CandidateAddr: addrVal1, + DelegatorAddr: addrDels[0], + CandidateAddr: addrVals[0], Shares: sdk.NewRat(9), } @@ -135,30 +126,30 @@ func TestBond(t *testing.T) { } // check the empty keeper first - _, found := keeper.getDelegatorBond(ctx, addrDel1, addrVal1) + _, found := keeper.getDelegatorBond(ctx, addrDels[0], addrVals[0]) assert.False(t, found) // set and retrieve a record keeper.setDelegatorBond(ctx, bond1to1) - resBond, found := keeper.getDelegatorBond(ctx, addrDel1, addrVal1) + resBond, found := keeper.getDelegatorBond(ctx, addrDels[0], addrVals[0]) assert.True(t, found) assert.True(t, bondsEqual(bond1to1, resBond)) // modify a records, save, and retrieve bond1to1.Shares = sdk.NewRat(99) keeper.setDelegatorBond(ctx, bond1to1) - resBond, found = keeper.getDelegatorBond(ctx, addrDel1, addrVal1) + resBond, found = keeper.getDelegatorBond(ctx, addrDels[0], addrVals[0]) assert.True(t, found) assert.True(t, bondsEqual(bond1to1, resBond)) // add some more records - keeper.setCandidate(ctx, candidate2) - keeper.setCandidate(ctx, candidate3) - bond1to2 := DelegatorBond{addrDel1, addrVal2, sdk.NewRat(9)} - bond1to3 := DelegatorBond{addrDel1, addrVal3, sdk.NewRat(9)} - bond2to1 := DelegatorBond{addrDel2, addrVal1, sdk.NewRat(9)} - bond2to2 := DelegatorBond{addrDel2, addrVal2, sdk.NewRat(9)} - bond2to3 := DelegatorBond{addrDel2, addrVal3, sdk.NewRat(9)} + keeper.setCandidate(ctx, candidates[1]) + keeper.setCandidate(ctx, candidates[2]) + bond1to2 := DelegatorBond{addrDels[0], addrVals[1], sdk.NewRat(9)} + bond1to3 := DelegatorBond{addrDels[0], addrVals[2], sdk.NewRat(9)} + bond2to1 := DelegatorBond{addrDels[1], addrVals[0], sdk.NewRat(9)} + bond2to2 := DelegatorBond{addrDels[1], addrVals[1], sdk.NewRat(9)} + bond2to3 := DelegatorBond{addrDels[1], addrVals[2], sdk.NewRat(9)} keeper.setDelegatorBond(ctx, bond1to2) keeper.setDelegatorBond(ctx, bond1to3) keeper.setDelegatorBond(ctx, bond2to1) @@ -166,16 +157,16 @@ func TestBond(t *testing.T) { keeper.setDelegatorBond(ctx, bond2to3) // test all bond retrieve capabilities - resBonds := keeper.getDelegatorBonds(ctx, addrDel1, 5) + resBonds := keeper.getDelegatorBonds(ctx, addrDels[0], 5) require.Equal(t, 3, len(resBonds)) assert.True(t, bondsEqual(bond1to1, resBonds[0])) assert.True(t, bondsEqual(bond1to2, resBonds[1])) assert.True(t, bondsEqual(bond1to3, resBonds[2])) - resBonds = keeper.getDelegatorBonds(ctx, addrDel1, 3) + resBonds = keeper.getDelegatorBonds(ctx, addrDels[0], 3) require.Equal(t, 3, len(resBonds)) - resBonds = keeper.getDelegatorBonds(ctx, addrDel1, 2) + resBonds = keeper.getDelegatorBonds(ctx, addrDels[0], 2) require.Equal(t, 2, len(resBonds)) - resBonds = keeper.getDelegatorBonds(ctx, addrDel2, 5) + resBonds = keeper.getDelegatorBonds(ctx, addrDels[1], 5) require.Equal(t, 3, len(resBonds)) assert.True(t, bondsEqual(bond2to1, resBonds[0])) assert.True(t, bondsEqual(bond2to2, resBonds[1])) @@ -183,9 +174,9 @@ func TestBond(t *testing.T) { // delete a record keeper.removeDelegatorBond(ctx, bond2to3) - _, found = keeper.getDelegatorBond(ctx, addrDel2, addrVal3) + _, found = keeper.getDelegatorBond(ctx, addrDels[1], addrVals[2]) assert.False(t, found) - resBonds = keeper.getDelegatorBonds(ctx, addrDel2, 5) + resBonds = keeper.getDelegatorBonds(ctx, addrDels[1], 5) require.Equal(t, 2, len(resBonds)) assert.True(t, bondsEqual(bond2to1, resBonds[0])) assert.True(t, bondsEqual(bond2to2, resBonds[1])) @@ -193,11 +184,11 @@ func TestBond(t *testing.T) { // delete all the records from delegator 2 keeper.removeDelegatorBond(ctx, bond2to1) keeper.removeDelegatorBond(ctx, bond2to2) - _, found = keeper.getDelegatorBond(ctx, addrDel2, addrVal1) + _, found = keeper.getDelegatorBond(ctx, addrDels[1], addrVals[0]) assert.False(t, found) - _, found = keeper.getDelegatorBond(ctx, addrDel2, addrVal2) + _, found = keeper.getDelegatorBond(ctx, addrDels[1], addrVals[1]) assert.False(t, found) - resBonds = keeper.getDelegatorBonds(ctx, addrDel2, 5) + resBonds = keeper.getDelegatorBonds(ctx, addrDels[1], 5) require.Equal(t, 0, len(resBonds)) } @@ -209,14 +200,14 @@ func TestGetValidators(t *testing.T) { // initialize some candidates into the state amts := []int64{0, 100, 1, 400, 200} n := len(amts) - candidates := make([]Candidate, n) - for i := 0; i < n; i++ { + var candidates [5]Candidate + for i, amt := range amts { c := Candidate{ Status: Unbonded, PubKey: pks[i], Address: addrs[i], - Assets: sdk.NewRat(amts[i]), - Liabilities: sdk.NewRat(amts[i]), + Assets: sdk.NewRat(amt), + Liabilities: sdk.NewRat(amt), } keeper.setCandidate(ctx, c) candidates[i] = c @@ -278,99 +269,185 @@ func TestGetValidators(t *testing.T) { // 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) - validatorsEqual := func(t *testing.T, expected []Validator, actual []Validator) { - require.Equal(t, len(expected), len(actual)) - for i := 0; i < len(expected); i++ { - assert.Equal(t, expected[i], actual[i]) + 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), } } - genValidators := func(candidates []Candidate) []Validator { - validators := make([]Validator, len(candidates)) - for i, c := range candidates { - validators[i] = c.validator() - } - - return validators - } - - candidates := []Candidate{candidate2, candidate4} - validators := genValidators(candidates) - // 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, candidates[0]) - keeper.setCandidate(ctx, candidates[1]) + keeper.setCandidate(ctx, candidatesIn[1]) + keeper.setCandidate(ctx, candidatesIn[3]) _ = keeper.GetValidators(ctx) // to init recent validator set acc = keeper.getAccUpdateValidators(ctx) - validatorsEqual(t, validators, acc) + 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) - validatorsEqual(t, validators, acc) - - acc = keeper.getAccUpdateValidators(ctx) - - // test from something to nothing - keeper.removeCandidate(ctx, candidates[0].Address) - keeper.removeCandidate(ctx, candidates[1].Address) - acc = keeper.getAccUpdateValidators(ctx) - assert.Equal(t, 2, len(acc)) - assert.Equal(t, validators[0].Address, acc[0].Address) - assert.Equal(t, int64(0), acc[0].VotingPower.Evaluate()) - assert.Equal(t, validators[1].Address, acc[1].Address) - assert.Equal(t, int64(0), acc[1].VotingPower.Evaluate()) + 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) - validators = genValidators(candidates) keeper.setCandidate(ctx, candidates[0]) - keeper.setCandidate(ctx, candidates[1]) acc = keeper.getAccUpdateValidators(ctx) - validatorsEqual(t, validators, acc) + 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(0) - validators = genValidators(candidates) + candidates[1].Assets = sdk.NewRat(100) keeper.setCandidate(ctx, candidates[0]) keeper.setCandidate(ctx, candidates[1]) acc = keeper.getAccUpdateValidators(ctx) - validatorsEqual(t, validators, acc) + 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 validator added at the beginning - candidates = append([]Candidate{candidate1}, candidates...) - validators = genValidators(candidates) + // test validtor added at the beginning + // {c1, c3} -> {c0, c1, c3} + // {c1, c3} -> {c0, c1, c3} + // {c1, c3} -> {c0, c1, c3} + candidates = append([]Candidate{candidatesIn[0]}, candidates...) keeper.setCandidate(ctx, candidates[0]) keeper.setCandidate(ctx, candidates[1]) keeper.setCandidate(ctx, candidates[2]) acc = keeper.getAccUpdateValidators(ctx) - validatorsEqual(t, validators, acc) + 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 - candidates = []Candidate{candidates[0], candidates[1], candidate3, candidates[2]} - validators = genValidators(candidates) + // {c0, c1, c3} -> {c0, c1, c2, c3] + // {c0, c1, c3} -> {c0, c1, c2, c3} + // {c0, c1, c3} -> {c0, c1, c2, c3} + candidates = []Candidate{candidates[0], candidates[1], candidatesIn[2], candidates[2]} keeper.setCandidate(ctx, candidates[0]) keeper.setCandidate(ctx, candidates[1]) keeper.setCandidate(ctx, candidates[2]) keeper.setCandidate(ctx, candidates[3]) acc = keeper.getAccUpdateValidators(ctx) - validatorsEqual(t, validators, acc) + 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 validator added at the end - candidates = append(candidates, candidate5) - validators = genValidators(candidates) + // test candidate(not validator) added at the end + // {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} + candidates = append(candidates, candidatesIn[4]) keeper.setCandidate(ctx, candidates[0]) keeper.setCandidate(ctx, candidates[1]) keeper.setCandidate(ctx, candidates[2]) keeper.setCandidate(ctx, candidates[3]) keeper.setCandidate(ctx, candidates[4]) acc = keeper.getAccUpdateValidators(ctx) - validatorsEqual(t, validators, acc) + 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(not validator) 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} + candidates[4].Assets = sdk.NewRat(5) + keeper.setCandidate(ctx, candidates[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 @@ -402,30 +479,41 @@ func TestClearAccUpdateValidators(t *testing.T) { func TestIsRecentValidator(t *testing.T) { ctx, _, keeper := createTestInput(t, nil, false, 0) + amts := []int64{9, 8, 7, 10, 6} + 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 that an empty validator set doesn't have any validators validators := keeper.GetValidators(ctx) assert.Equal(t, 0, len(validators)) // get the validators for the first time - keeper.setCandidate(ctx, candidate1) - keeper.setCandidate(ctx, candidate2) + keeper.setCandidate(ctx, candidatesIn[0]) + keeper.setCandidate(ctx, candidatesIn[1]) validators = keeper.GetValidators(ctx) require.Equal(t, 2, len(validators)) - assert.Equal(t, candidate1.validator(), validators[0]) - assert.Equal(t, candidate2.validator(), validators[1]) + assert.Equal(t, candidatesIn[0].validator(), validators[0]) + assert.Equal(t, candidatesIn[1].validator(), validators[1]) // test a basic retrieve of something that should be a recent validator - assert.True(t, keeper.IsRecentValidator(ctx, candidate1.Address)) - assert.True(t, keeper.IsRecentValidator(ctx, candidate2.Address)) + assert.True(t, keeper.IsRecentValidator(ctx, candidatesIn[0].Address)) + assert.True(t, keeper.IsRecentValidator(ctx, candidatesIn[1].Address)) // test a basic retrieve of something that should not be a recent validator - assert.False(t, keeper.IsRecentValidator(ctx, candidate3.Address)) + assert.False(t, keeper.IsRecentValidator(ctx, candidatesIn[2].Address)) // remove that validator, but don't retrieve the recent validator group - keeper.removeCandidate(ctx, candidate1.Address) + keeper.removeCandidate(ctx, candidatesIn[0].Address) // test that removed validator is not considered a recent validator - assert.False(t, keeper.IsRecentValidator(ctx, candidate1.Address)) + assert.False(t, keeper.IsRecentValidator(ctx, candidatesIn[0].Address)) } func TestParams(t *testing.T) { From 0fa0491d0f81c0dea77be6f93d1adad079afcaff Mon Sep 17 00:00:00 2001 From: mossid Date: Sat, 31 Mar 2018 21:39:38 +0200 Subject: [PATCH 6/8] remove some unnecessary setCandidates --- x/stake/keeper_test.go | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/x/stake/keeper_test.go b/x/stake/keeper_test.go index f5b1d038f6..51a3f5b46b 100644 --- a/x/stake/keeper_test.go +++ b/x/stake/keeper_test.go @@ -345,10 +345,7 @@ func TestGetAccUpdateValidators(t *testing.T) { // {c1, c3} -> {c0, c1, c3} // {c1, c3} -> {c0, c1, c3} // {c1, c3} -> {c0, c1, c3} - candidates = append([]Candidate{candidatesIn[0]}, candidates...) - keeper.setCandidate(ctx, candidates[0]) - keeper.setCandidate(ctx, candidates[1]) - keeper.setCandidate(ctx, candidates[2]) + keeper.setCandidate(ctx, candidatesIn[0]) acc = keeper.getAccUpdateValidators(ctx) require.Equal(t, 3, len(acc)) candidates = keeper.GetCandidates(ctx, 5) @@ -361,11 +358,7 @@ func TestGetAccUpdateValidators(t *testing.T) { // {c0, c1, c3} -> {c0, c1, c2, c3] // {c0, c1, c3} -> {c0, c1, c2, c3} // {c0, c1, c3} -> {c0, c1, c2, c3} - candidates = []Candidate{candidates[0], candidates[1], candidatesIn[2], candidates[2]} - keeper.setCandidate(ctx, candidates[0]) - keeper.setCandidate(ctx, candidates[1]) - keeper.setCandidate(ctx, candidates[2]) - keeper.setCandidate(ctx, candidates[3]) + keeper.setCandidate(ctx, candidatesIn[2]) acc = keeper.getAccUpdateValidators(ctx) require.Equal(t, 4, len(acc)) candidates = keeper.GetCandidates(ctx, 5) @@ -375,16 +368,11 @@ func TestGetAccUpdateValidators(t *testing.T) { assert.Equal(t, candidates[2].validator(), acc[2]) assert.Equal(t, candidates[3].validator(), acc[3]) - // test candidate(not validator) added at the end + // 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} - candidates = append(candidates, candidatesIn[4]) - keeper.setCandidate(ctx, candidates[0]) - keeper.setCandidate(ctx, candidates[1]) - keeper.setCandidate(ctx, candidates[2]) - keeper.setCandidate(ctx, candidates[3]) - keeper.setCandidate(ctx, candidates[4]) + 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) @@ -394,12 +382,12 @@ func TestGetAccUpdateValidators(t *testing.T) { assert.Equal(t, candidates[2].validator(), acc[2]) assert.Equal(t, candidates[3].validator(), acc[3]) - // test candidate(not validator) change its power but still not in the valset + // 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} - candidates[4].Assets = sdk.NewRat(5) - keeper.setCandidate(ctx, candidates[4]) + 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) From 7565ba4c0ca67ee790397cf9cea3f335ae6aa94e Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Mon, 2 Apr 2018 20:37:35 +0200 Subject: [PATCH 7/8] 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) From a6d587b870c117150b5ef0c39fb605876abb2d4d Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Mon, 2 Apr 2018 22:18:54 +0200 Subject: [PATCH 8/8] fix remove candidate keeper logic --- x/stake/keeper.go | 5 ++--- x/stake/keeper_test.go | 46 +++++++++++++++++++++++++----------------- 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/x/stake/keeper.go b/x/stake/keeper.go index 155d8e04a0..d96bbe3d4b 100644 --- a/x/stake/keeper.go +++ b/x/stake/keeper.go @@ -127,20 +127,19 @@ func (k Keeper) removeCandidate(ctx sdk.Context, address sdk.Address) { // delete the old candidate record store := ctx.KVStore(k.storeKey) store.Delete(GetCandidateKey(address)) + store.Delete(GetValidatorKey(address, oldCandidate.Assets, k.cdc)) // delete from recent and power weighted validator groups if the validator // exists and add validator with zero power to the validator updates - if store.Get(GetRecentValidatorKey(address)) == nil && !k.isNewValidator(ctx, store, address) { + if store.Get(GetRecentValidatorKey(address)) == nil { return } bz, err := k.cdc.MarshalBinary(Validator{address, sdk.ZeroRat}) if err != nil { panic(err) } - store.Set(GetAccUpdateValidatorKey(address), bz) store.Delete(GetRecentValidatorKey(address)) - store.Delete(GetValidatorKey(address, oldCandidate.Assets, k.cdc)) } //___________________________________________________________________________ diff --git a/x/stake/keeper_test.go b/x/stake/keeper_test.go index 654c243029..17260cc0fd 100644 --- a/x/stake/keeper_test.go +++ b/x/stake/keeper_test.go @@ -444,7 +444,7 @@ func TestGetAccUpdateValidators(t *testing.T) { 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) + // 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} @@ -460,37 +460,47 @@ func TestGetAccUpdateValidators(t *testing.T) { require.Equal(t, 5, len(candidates)) vals = keeper.GetValidators(ctx) require.Equal(t, 4, len(vals)) + assert.Equal(t, candidatesIn[1].Address, vals[1].Address) + assert.Equal(t, candidatesIn[2].Address, vals[3].Address) + assert.Equal(t, candidatesIn[3].Address, vals[2].Address) + assert.Equal(t, candidatesIn[4].Address, vals[0].Address) + 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, int64(0), acc[0].VotingPower.Evaluate()) 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} + // candidate set: {c0, c1, c2, c3, c4} -> {} + // validator set: {c1, c2, c3, c4} -> {} + // accUpdate set: {} -> {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)) + 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))) + + keeper.removeCandidate(ctx, candidatesIn[0].Address) + keeper.removeCandidate(ctx, candidatesIn[1].Address) + keeper.removeCandidate(ctx, candidatesIn[2].Address) + keeper.removeCandidate(ctx, candidatesIn[3].Address) + keeper.removeCandidate(ctx, candidatesIn[4].Address) + + vals = keeper.GetValidators(ctx) + assert.Equal(t, 0, len(vals), "%v", vals) candidates = keeper.GetCandidates(ctx, 5) require.Equal(t, 0, len(candidates)) - assert.Equal(t, candidatesIn[0].Address, acc[0].Address) + acc = keeper.getAccUpdateValidators(ctx) + require.Equal(t, 4, len(acc)) + assert.Equal(t, candidatesIn[1].Address, acc[0].Address) + assert.Equal(t, candidatesIn[2].Address, acc[1].Address) + assert.Equal(t, candidatesIn[3].Address, acc[2].Address) + assert.Equal(t, candidatesIn[4].Address, acc[3].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