From 4b0c367afadbab84ca59f1cf23ec5f799d3b791c Mon Sep 17 00:00:00 2001 From: mossid Date: Wed, 28 Mar 2018 20:12:21 +0200 Subject: [PATCH] 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 af2015fe8..8aa539116 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 051994456..5c09a47fc 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 6e7478957..a65789529 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