From 926a6160c99108ec3b478c5c8d492bd386bc5548 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Fri, 6 Jul 2018 03:04:01 -0400 Subject: [PATCH] rearrage cliff tests to reveal new problem --- x/stake/handler_test.go | 25 ++++++++++++----------- x/stake/keeper/validator.go | 34 +++++++++++++++++++++++++++----- x/stake/keeper/validator_test.go | 1 + 3 files changed, 44 insertions(+), 16 deletions(-) diff --git a/x/stake/handler_test.go b/x/stake/handler_test.go index a3115462fe..0f79b253d0 100644 --- a/x/stake/handler_test.go +++ b/x/stake/handler_test.go @@ -562,7 +562,7 @@ func TestTransitiveRedelegation(t *testing.T) { func TestCliffValidator(t *testing.T) { ctx, _, keeper := keep.CreateTestInput(t, false, 1000) - validatorAddr, validatorAddr2, validatorAddr3 := keep.Addrs[0], keep.Addrs[1], keep.Addrs[2] + validatorAddr1, validatorAddr2, validatorAddr3 := keep.Addrs[0], keep.Addrs[1], keep.Addrs[2] // make sure that the cliff validator is nil to begin with cliffVal := keeper.GetCliffValidator(ctx) @@ -575,7 +575,7 @@ func TestCliffValidator(t *testing.T) { keeper.SetParams(ctx, params) // add the first validator - msgCreateValidator := newTestMsgCreateValidator(validatorAddr, keep.PKs[0], 30) + msgCreateValidator := newTestMsgCreateValidator(validatorAddr1, keep.PKs[0], 50) got := handleMsgCreateValidator(ctx, msgCreateValidator, keeper) require.True(t, got.IsOK(), "expected no error on runMsgCreateValidator") @@ -584,13 +584,13 @@ func TestCliffValidator(t *testing.T) { require.Equal(t, []byte(nil), cliffVal) // Add the second validator - msgCreateValidator = newTestMsgCreateValidator(validatorAddr2, keep.PKs[1], 50) + msgCreateValidator = newTestMsgCreateValidator(validatorAddr2, keep.PKs[1], 30) got = handleMsgCreateValidator(ctx, msgCreateValidator, keeper) require.True(t, got.IsOK(), "expected no error on runMsgCreateValidator") - // now that we've reached maximum validators, the val-1 should be added to the cliff (top) + // now that we've reached maximum validators, the val-2 should be added to the cliff (top) cliffVal = keeper.GetCliffValidator(ctx) - require.Equal(t, validatorAddr2, cliffVal) + require.Equal(t, validatorAddr2.Bytes(), cliffVal) // add the third validator, which should not make it to being bonded, // so the cliff validator should not change because nobody has been kicked out @@ -599,25 +599,28 @@ func TestCliffValidator(t *testing.T) { require.True(t, got.IsOK(), "expected no error on runMsgCreateValidator") cliffVal = keeper.GetCliffValidator(ctx) - require.Equal(t, validatorAddr2, sdk.Address(cliffVal)) + require.Equal(t, validatorAddr2.Bytes(), cliffVal) - // unbond the first validator - msgBeginUnbonding := NewMsgBeginUnbonding(validatorAddr1, validatorAddr1, sdk.NewRat(30)) + // unbond the second validator + msgBeginUnbonding := NewMsgBeginUnbonding(validatorAddr2, validatorAddr2, sdk.NewRat(30)) got = handleMsgBeginUnbonding(ctx, msgBeginUnbonding, keeper) require.True(t, got.IsOK(), "expected no error on runMsgBeginUnbonding") + vals := keeper.GetValidatorsBonded(ctx) + require.Equal(t, 2, len(vals)) + // now the validator set should be updated, // where val-3 enters the validator set on the cliff cliffVal = keeper.GetCliffValidator(ctx) - require.Equal(t, validatorAddr2, sdk.Address(cliffVal)) + require.Equal(t, validatorAddr3.Bytes(), cliffVal) // unbond the second validator - msgBeginUnbonding = NewMsgBeginUnbonding(validatorAddr2, validatorAddr2, sdk.NewRat(5)) + msgBeginUnbonding = NewMsgBeginUnbonding(validatorAddr1, validatorAddr1, sdk.NewRat(5)) got = handleMsgBeginUnbonding(ctx, msgBeginUnbonding, keeper) require.True(t, got.IsOK(), "expected no error on runMsgBeginUnbonding") // get bonded validators - should just be one - vals := keeper.GetValidatorsBonded(ctx) + vals = keeper.GetValidatorsBonded(ctx) require.Equal(t, 1, len(vals)) // cliff now should be empty diff --git a/x/stake/keeper/validator.go b/x/stake/keeper/validator.go index 109dd4a3cd..2d237e46b5 100644 --- a/x/stake/keeper/validator.go +++ b/x/stake/keeper/validator.go @@ -284,6 +284,7 @@ func (k Keeper) UpdateValidator(ctx sdk.Context, validator types.Validator) type // Optionally also return the validator from a retrieve address if the validator has been bonded func (k Keeper) UpdateBondedValidators(ctx sdk.Context, newValidator types.Validator) (updatedVal types.Validator) { + fmt.Println("wackydebugoutput UpdateBondedValidators 0") store := ctx.KVStore(k.storeKey) @@ -296,14 +297,24 @@ func (k Keeper) UpdateBondedValidators(ctx sdk.Context, bondedValidatorsCount := 0 var validator types.Validator for { + fmt.Println("wackydebugoutput UpdateBondedValidators 1") if !iterator.Valid() || bondedValidatorsCount > int(maxValidators-1) { + fmt.Println("wackydebugoutput UpdateBondedValidators 2") + fmt.Printf("debug bondedValidatorsCount: %v\n", bondedValidatorsCount) + fmt.Printf("debug maxValidators: %v\n", maxValidators) // TODO benchmark if we should read the current power and not write if it's the same if bondedValidatorsCount == int(maxValidators) { // is cliff validator + fmt.Println("wackydebugoutput UpdateBondedValidators 3") k.setCliffValidator(ctx, validator, k.GetPool(ctx)) + } else { + fmt.Println("wackydebugoutput UpdateBondedValidators 4") + k.clearCliffValidator(ctx) } + fmt.Println("wackydebugoutput UpdateBondedValidators 5") break } + fmt.Println("wackydebugoutput UpdateBondedValidators 6") // either retrieve the original validator from the store, or under the // situation that this is the "new validator" just use the validator @@ -311,44 +322,59 @@ func (k Keeper) UpdateBondedValidators(ctx sdk.Context, // store ownerAddr := iterator.Value() if bytes.Equal(ownerAddr, newValidator.Owner) { + fmt.Println("wackydebugoutput UpdateBondedValidators 7") validator = newValidator } else { + fmt.Println("wackydebugoutput UpdateBondedValidators 8") var found bool validator, found = k.GetValidator(ctx, ownerAddr) if !found { + fmt.Println("wackydebugoutput UpdateBondedValidators 9") panic(fmt.Sprintf("validator record not found for address: %v\n", ownerAddr)) } + fmt.Println("wackydebugoutput UpdateBondedValidators 10") } + fmt.Println("wackydebugoutput UpdateBondedValidators 11") // if not previously a validator (and unrevoked), // kick the cliff validator / bond this new validator if validator.Status() != sdk.Bonded && !validator.Revoked { + fmt.Println("wackydebugoutput UpdateBondedValidators 12") kickCliffValidator = true validator = k.bondValidator(ctx, validator) if bytes.Equal(ownerAddr, newValidator.Owner) { + fmt.Println("wackydebugoutput UpdateBondedValidators 13") updatedVal = validator } + fmt.Println("wackydebugoutput UpdateBondedValidators 14") } + fmt.Println("wackydebugoutput UpdateBondedValidators 15") if validator.Revoked && validator.Status() == sdk.Bonded { + fmt.Println("wackydebugoutput UpdateBondedValidators 16") panic(fmt.Sprintf("revoked validator cannot be bonded, address: %v\n", ownerAddr)) - } else { - bondedValidatorsCount++ } + fmt.Println("wackydebugoutput UpdateBondedValidators 18") + bondedValidatorsCount++ iterator.Next() } + fmt.Println("wackydebugoutput UpdateBondedValidators 19") iterator.Close() // perform the actual kicks if oldCliffValidatorAddr != nil && kickCliffValidator { + fmt.Println("wackydebugoutput UpdateBondedValidators 20") validator, found := k.GetValidator(ctx, oldCliffValidatorAddr) if !found { + fmt.Println("wackydebugoutput UpdateBondedValidators 21") panic(fmt.Sprintf("validator record not found for address: %v\n", oldCliffValidatorAddr)) } + fmt.Println("wackydebugoutput UpdateBondedValidators 22") k.unbondValidator(ctx, validator) } + fmt.Println("wackydebugoutput UpdateBondedValidators 23") return } @@ -405,10 +431,8 @@ func (k Keeper) UpdateBondedValidatorsFull(ctx sdk.Context) { if validator.Revoked && validator.Status() == sdk.Bonded { panic(fmt.Sprintf("revoked validator cannot be bonded, address: %v\n", ownerAddr)) - } else { - bondedValidatorsCount++ } - + bondedValidatorsCount++ iterator.Next() } iterator.Close() diff --git a/x/stake/keeper/validator_test.go b/x/stake/keeper/validator_test.go index c4d197a36b..807c16012b 100644 --- a/x/stake/keeper/validator_test.go +++ b/x/stake/keeper/validator_test.go @@ -634,6 +634,7 @@ func TestGetTendermintUpdatesInserted(t *testing.T) { require.Equal(t, validators[4].ABCIValidator(), updates[0]) } +//TODO why is this called NotValidatorCliff? func TestGetTendermintUpdatesNotValidatorCliff(t *testing.T) { ctx, _, keeper := CreateTestInput(t, false, 1000) params := types.DefaultParams()