diff --git a/x/stake/keeper.go b/x/stake/keeper.go index 40b209ae7..0a882ac83 100644 --- a/x/stake/keeper.go +++ b/x/stake/keeper.go @@ -74,10 +74,15 @@ func (k Keeper) setValidator(ctx sdk.Context, validator Validator) { // retreive the old validator record oldValidator, oldFound := k.GetValidator(ctx, address) - // if found, copy the old block height and counter - if oldFound { + // if found and already a bonded, copy the old block height and counter, else set them + if oldFound && k.IsValidator(ctx, oldValidator.PubKey) { validator.BondHeight = oldValidator.BondHeight validator.BondIntraTxCounter = oldValidator.BondIntraTxCounter + } else { + validator.ValidatorBondHeight = ctx.BlockHeight() + counter := k.getIntraTxCounter(ctx) + validator.ValidatorBondCounter = counter + k.setIntraTxCounter(ctx, counter+1) } // marshal the validator record and add to the state @@ -90,33 +95,21 @@ func (k Keeper) setValidator(ctx sdk.Context, validator Validator) { return } - // if this validator wasn't just bonded then update the height and counter - if oldValidator.Status != sdk.Bonded { - validator.BondHeight = ctx.BlockHeight() - counter := k.getIntraTxCounter(ctx) - validator.BondIntraTxCounter = counter - k.setIntraTxCounter(ctx, counter+1) - } - // delete the old record in the power ordered list store.Delete(GetValidatorsBondedByPowerKey(oldValidator)) } - // set the new validator record - bz = k.cdc.MustMarshalBinary(validator) - store.Set(GetValidatorKey(address), bz) - // update the list ordered by voting power bzVal := k.cdc.MustMarshalBinary(validator) store.Set(GetValidatorsBondedByPowerKey(validator), bzVal) // add to the validators to update list if is already a validator - if store.Get(GetValidatorsBondedBondedKey(validator.PubKey)) != nil { + if store.Get(GetValidatorsBondedKey(validator.PubKey)) != nil { bzAbci := k.cdc.MustMarshalBinary(validator.abciValidator(k.cdc)) store.Set(GetValidatorsTendermintUpdatesKey(address), bzAbci) // also update the current validator store - store.Set(GetValidatorsBondedBondedKey(validator.PubKey), bzVal) + store.Set(GetValidatorsBondedKey(validator.PubKey), bzVal) return } @@ -140,12 +133,12 @@ func (k Keeper) removeValidator(ctx sdk.Context, address sdk.Address) { // delete from current and power weighted validator groups if the validator // exists and add validator with zero power to the validator updates - if store.Get(GetValidatorsBondedBondedKey(validator.PubKey)) == nil { + if store.Get(GetValidatorsBondedKey(validator.PubKey)) == nil { return } bz := k.cdc.MustMarshalBinary(validator.abciValidatorZero(k.cdc)) store.Set(GetValidatorsTendermintUpdatesKey(address), bz) - store.Delete(GetValidatorsBondedBondedKey(validator.PubKey)) + store.Delete(GetValidatorsBondedKey(validator.PubKey)) } //___________________________________________________________________________ @@ -237,7 +230,7 @@ func (k Keeper) addNewValidatorOrNot(ctx sdk.Context, store sdk.KVStore, address toKickOut[string(validator.Address)] = nil // also add to the current validators group - store.Set(GetValidatorsBondedBondedKey(validator.PubKey), bz) + store.Set(GetValidatorsBondedKey(validator.PubKey), bz) // MOST IMPORTANTLY, add to the accumulated changes if this is the modified validator if bytes.Equal(address, validator.Address) { @@ -380,7 +373,11 @@ func (k Keeper) setParams(ctx sdk.Context, params Params) { store := ctx.KVStore(k.storeKey) b := k.cdc.MustMarshalBinary(params) store.Set(ParamKey, b) - k.params = Params{} // clear the cache + // if max validator count changes, must recalculate validator set + if k.params.MaxValidators != params.MaxValidators { + k.addNewValidatorOrNot(ctx, store, sdk.Address{}) + } + k.params = params // update the cache } //_______________________________________________________________________ diff --git a/x/stake/keeper_keys.go b/x/stake/keeper_keys.go index 75efac977..6d828b37e 100644 --- a/x/stake/keeper_keys.go +++ b/x/stake/keeper_keys.go @@ -52,7 +52,7 @@ func GetValidatorsTendermintUpdatesKey(addr sdk.Address) []byte { } // get the key for the current validator group, ordered like tendermint -func GetValidatorsBondedBondedKey(pk crypto.PubKey) []byte { +func GetValidatorsBondedKey(pk crypto.PubKey) []byte { addr := pk.Address() return append(ValidatorsBondedKey, addr.Bytes()...) } diff --git a/x/stake/keeper_test.go b/x/stake/keeper_test.go index da27b663c..011f12926 100644 --- a/x/stake/keeper_test.go +++ b/x/stake/keeper_test.go @@ -221,50 +221,47 @@ func TestGetValidatorsBonded(t *testing.T) { validators = keeper.GetValidatorsBondedByPower(ctx) require.Equal(t, len(validators), n) assert.Equal(t, sdk.NewRat(300), validators[0].Power, "%v", validators) - assert.Equal(t, validators[3].Address, validators[0].Address, "%v", validators) + assert.Equal(t, candidates[3].Address, validators[0].Address, "%v", validators) + assert.Equal(t, candidates[4].Address, validators[1].Address, "%v", validators) - // XXX FIX TEST // test equal voting power, different age validators[3].BondedShares = sdk.NewRat(200) ctx = ctx.WithBlockHeight(10) keeper.setValidator(ctx, validators[3]) validators = keeper.GetValidatorsBondedByPower(ctx) require.Equal(t, len(validators), n) - //assert.Equal(t, sdk.NewRat(200), validators[0].Power, "%v", validators) - //assert.Equal(t, sdk.NewRat(200), validators[1].Power, "%v", validators) - //assert.Equal(t, validators[3].Address, validators[0].Address, "%v", validators) - //assert.Equal(t, validators[4].Address, validators[1].Address, "%v", validators) - //assert.Equal(t, int64(0), validators[0].Height, "%v", validators) - //assert.Equal(t, int64(0), validators[1].Height, "%v", validators) + assert.Equal(t, sdk.NewRat(200), validators[0].Power, "%v", validators) + assert.Equal(t, sdk.NewRat(200), validators[1].Power, "%v", validators) + assert.Equal(t, candidates[3].Address, validators[0].Address, "%v", validators) + assert.Equal(t, candidates[4].Address, validators[1].Address, "%v", validators) + assert.Equal(t, int64(0), validators[0].Height, "%v", validators) + assert.Equal(t, int64(0), validators[1].Height, "%v", validators) - // XXX FIX TEST // no change in voting power - no change in sort ctx = ctx.WithBlockHeight(20) keeper.setValidator(ctx, validators[4]) validators = keeper.GetValidatorsBondedByPower(ctx) require.Equal(t, len(validators), n) - //assert.Equal(t, validators[3].Address, validators[0].Address, "%v", validators) - //assert.Equal(t, validators[4].Address, validators[1].Address, "%v", validators) + assert.Equal(t, candidates[3].Address, validators[0].Address, "%v", validators) + assert.Equal(t, candidates[4].Address, validators[1].Address, "%v", validators) - // XXX FIX TEST - // change in voting power of both validators, both still in v-set, no age change - validators[3].BondedShares = sdk.NewRat(300) - validators[4].BondedShares = sdk.NewRat(300) - keeper.setValidator(ctx, validators[3]) - validators = keeper.GetValidatorsBondedByPower(ctx) + // change in voting power of both candidates, both still in v-set, no age change + candidates[3].BondedShares = sdk.NewRat(300) + candidates[4].BondedShares = sdk.NewRat(300) + keeper.setCandidate(ctx, candidates[3]) + validators = keeper.getValidatorsOrdered(ctx) require.Equal(t, len(validators), n) ctx = ctx.WithBlockHeight(30) keeper.setValidator(ctx, validators[4]) validators = keeper.GetValidatorsBondedByPower(ctx) require.Equal(t, len(validators), n, "%v", validators) - //assert.Equal(t, validators[3].Address, validators[0].Address, "%v", validators) - //assert.Equal(t, validators[4].Address, validators[1].Address, "%v", validators) + assert.Equal(t, candidates[3].Address, validators[0].Address, "%v", validators) + assert.Equal(t, candidates[4].Address, validators[1].Address, "%v", validators) } // TODO seperate out into multiple tests -/* XXX FIX THESE TESTS -func TestGetValidatorsBondedEdgeCases(t *testing.T) { +func TestGetValidatorsEdgeCases(t *testing.T) { ctx, _, keeper := createTestInput(t, false, 0) // now 2 max validators @@ -272,8 +269,8 @@ func TestGetValidatorsBondedEdgeCases(t *testing.T) { params.MaxValidators = 2 keeper.setParams(ctx, params) - // initialize some validators into the state - amts := []int64{0, 100, 1, 400, 200} + // initialize some candidates into the state + amts := []int64{0, 100, 400, 400, 200} n := len(amts) var validators [5]Validator for i, amt := range amts { @@ -287,50 +284,49 @@ func TestGetValidatorsBondedEdgeCases(t *testing.T) { keeper.setValidator(ctx, validators[0]) validators := keeper.GetValidatorsBondedByPower(ctx) require.Equal(t, uint16(len(validators)), params.MaxValidators) - require.Equal(t, validators[0].Address, validators[0].Address, "%v", validators) - // validator 3 was set before validator 4 - require.Equal(t, validators[3].Address, validators[1].Address, "%v", validators) + require.Equal(t, candidates[0].Address, validators[0].Address, "%v", validators) + // candidate 3 was set before candidate 4 + require.Equal(t, candidates[2].Address, validators[1].Address, "%v", validators) - //A validator which leaves the validator set due to a decrease in voting power, - //then increases to the original voting power, does not get its spot back in the - //case of a tie. - - //ref https://github.com/cosmos/cosmos-sdk/issues/582#issuecomment-380757108 - validators[4].BondedShares = sdk.NewRat(301) - keeper.setValidator(ctx, validators[4]) - validators = keeper.GetValidatorsBondedByPower(ctx) + // A candidate which leaves the validator set due to a decrease in voting power, + // then increases to the original voting power, does not get its spot back in the + // case of a tie. + // ref https://github.com/cosmos/cosmos-sdk/issues/582#issuecomment-380757108 + candidates[3].BondedShares = sdk.NewRat(401) + keeper.setCandidate(ctx, candidates[3]) + validators = keeper.getValidatorsOrdered(ctx) require.Equal(t, uint16(len(validators)), params.MaxValidators) - require.Equal(t, validators[0].Address, validators[0].Address, "%v", validators) - require.Equal(t, validators[4].Address, validators[1].Address, "%v", validators) + require.Equal(t, candidates[0].Address, validators[0].Address, "%v", validators) + require.Equal(t, candidates[3].Address, validators[1].Address, "%v", validators) ctx = ctx.WithBlockHeight(40) - // validator 4 kicked out temporarily - validators[4].BondedShares = sdk.NewRat(200) - keeper.setValidator(ctx, validators[4]) - validators = keeper.GetValidatorsBondedByPower(ctx) + // candidate 3 kicked out temporarily + candidates[3].BondedShares = sdk.NewRat(200) + keeper.setCandidate(ctx, candidates[3]) + validators = keeper.getValidatorsOrdered(ctx) require.Equal(t, uint16(len(validators)), params.MaxValidators) - require.Equal(t, validators[0].Address, validators[0].Address, "%v", validators) - require.Equal(t, validators[3].Address, validators[1].Address, "%v", validators) - // validator 4 does not get spot back - validators[4].BondedShares = sdk.NewRat(300) - keeper.setValidator(ctx, validators[4]) - validators = keeper.GetValidatorsBondedByPower(ctx) + require.Equal(t, candidates[0].Address, validators[0].Address, "%v", validators) + require.Equal(t, candidates[2].Address, validators[1].Address, "%v", validators) + // candidate 4 does not get spot back + candidates[3].BondedShares = sdk.NewRat(400) + keeper.setCandidate(ctx, candidates[3]) + validators = keeper.getValidatorsOrdered(ctx) require.Equal(t, uint16(len(validators)), params.MaxValidators) - require.Equal(t, validators[0].Address, validators[0].Address, "%v", validators) - require.Equal(t, validators[3].Address, validators[1].Address, "%v", validators) - validator, exists := keeper.GetValidator(ctx, validators[4].Address) + require.Equal(t, candidates[0].Address, validators[0].Address, "%v", validators) + require.Equal(t, candidates[2].Address, validators[1].Address, "%v", validators) + candidate, exists := keeper.GetCandidate(ctx, candidates[3].Address) require.Equal(t, exists, true) require.Equal(t, validator.BondHeight, int64(40)) - //If two validators both increase to the same voting power in the same block, - //the one with the first transaction should take precedence (become a validator). - //ref https://github.com/cosmos/cosmos-sdk/issues/582#issuecomment-381250392 - validators[0].BondedShares = sdk.NewRat(2000) - keeper.setValidator(ctx, validators[0]) - validators[1].BondedShares = sdk.NewRat(1000) - validators[2].BondedShares = sdk.NewRat(1000) - keeper.setValidator(ctx, validators[1]) - keeper.setValidator(ctx, validators[2]) - validators = keeper.GetValidatorsBondedByPower(ctx) + // If two candidates both increase to the same voting power in the same block, + // the one with the first transaction should take precedence (become a validator). + // ref https://github.com/cosmos/cosmos-sdk/issues/582#issuecomment-381250392 + candidates[0].BondedShares = sdk.NewRat(2000) + keeper.setCandidate(ctx, candidates[0]) + candidates[1].BondedShares = sdk.NewRat(1000) + candidates[2].BondedShares = sdk.NewRat(1000) + keeper.setCandidate(ctx, candidates[1]) + keeper.setCandidate(ctx, candidates[2]) + validators = keeper.getValidatorsOrdered(ctx) require.Equal(t, uint16(len(validators)), params.MaxValidators) require.Equal(t, validators[0].Address, validators[0].Address, "%v", validators) require.Equal(t, validators[1].Address, validators[1].Address, "%v", validators) @@ -380,7 +376,6 @@ func TestGetValidatorsBondedEdgeCases(t *testing.T) { assert.Equal(t, sdk.NewRat(300), validators[1].Power, "%v", validators) assert.Equal(t, validators[3].Address, validators[1].Address, "%v", validators) } -*/ // clear the tracked changes to the validator set func TestClearValidatorsTendermintUpdates(t *testing.T) {