fix: update StartHeight signing info in AfterValidatorBonded hook when validator re-bonds

x/slashing/keeper/hooks.go

@@ -11,19 +11,22 @@ import (
 
 func (k Keeper) AfterValidatorBonded(ctx sdk.Context, address sdk.ConsAddress, _ sdk.ValAddress) error {
 	// Update the signing info start height or create a new signing info
-	_, found := k.GetValidatorSigningInfo(ctx, address)
-	if !found {
-		signingInfo := types.NewValidatorSigningInfo(
+	signingInfo, found := k.GetValidatorSigningInfo(ctx, address)
+	if found {
+		signingInfo.StartHeight = ctx.BlockHeight()
+	} else {
+		signingInfo = types.NewValidatorSigningInfo(
 			address,
 			ctx.BlockHeight(),
 			0,
 			time.Unix(0, 0),
 			false,
 			0,
 		)
-		k.SetValidatorSigningInfo(ctx, address, signingInfo)
 	}
 
+	k.SetValidatorSigningInfo(ctx, address, signingInfo)
+
 	return nil
 }
 
@@ -74,17 +77,27 @@ func (h Hooks) AfterValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) er
 func (h Hooks) AfterValidatorBeginUnbonding(_ sdk.Context, _ sdk.ConsAddress, _ sdk.ValAddress) error {
 	return nil
 }
-func (h Hooks) BeforeValidatorModified(_ sdk.Context, _ sdk.ValAddress) error { return nil }
+
+func (h Hooks) BeforeValidatorModified(_ sdk.Context, _ sdk.ValAddress) error {
+	return nil
+}
+
 func (h Hooks) BeforeDelegationCreated(_ sdk.Context, _ sdk.AccAddress, _ sdk.ValAddress) error {
 	return nil
 }
+
 func (h Hooks) BeforeDelegationSharesModified(_ sdk.Context, _ sdk.AccAddress, _ sdk.ValAddress) error {
 	return nil
 }
+
 func (h Hooks) BeforeDelegationRemoved(_ sdk.Context, _ sdk.AccAddress, _ sdk.ValAddress) error {
 	return nil
 }
+
 func (h Hooks) AfterDelegationModified(_ sdk.Context, _ sdk.AccAddress, _ sdk.ValAddress) error {
 	return nil
 }
-func (h Hooks) BeforeValidatorSlashed(_ sdk.Context, _ sdk.ValAddress, _ sdk.Dec) error { return nil }
+
+func (h Hooks) BeforeValidatorSlashed(_ sdk.Context, _ sdk.ValAddress, _ sdk.Dec) error {
+	return nil
+}

x/slashing/keeper/keeper_test.go

@@ -200,7 +200,9 @@ func TestValidatorDippingInAndOut(t *testing.T) {
 	valAddr := sdk.ValAddress(addr)
 
 	tstaking.CreateValidatorWithValPower(valAddr, val, power, true)
-	staking.EndBlocker(ctx, app.StakingKeeper)
+	validatorUpdates := staking.EndBlocker(ctx, app.StakingKeeper)
+	require.Equal(t, 2, len(validatorUpdates))
+	tstaking.CheckValidator(valAddr, stakingtypes.Bonded, false)
 
 	// 100 first blocks OK
 	height := int64(0)
@@ -210,22 +212,23 @@ func TestValidatorDippingInAndOut(t *testing.T) {
 	}
 
 	// kick first validator out of validator set
-	tstaking.CreateValidatorWithValPower(sdk.ValAddress(pks[1].Address()), pks[1], 101, true)
-	validatorUpdates := staking.EndBlocker(ctx, app.StakingKeeper)
+	tstaking.CreateValidatorWithValPower(sdk.ValAddress(pks[1].Address()), pks[1], power+1, true)
+	validatorUpdates = staking.EndBlocker(ctx, app.StakingKeeper)
 	require.Equal(t, 2, len(validatorUpdates))
+	tstaking.CheckValidator(sdk.ValAddress(pks[1].Address()), stakingtypes.Bonded, false)
 	tstaking.CheckValidator(valAddr, stakingtypes.Unbonding, false)
 
 	// 600 more blocks happened
-	height = 700
+	height = height + 600
 	ctx = ctx.WithBlockHeight(height)
 
 	// validator added back in
-	tstaking.DelegateWithPower(sdk.AccAddress(pks[2].Address()), sdk.ValAddress(pks[0].Address()), 50)
+	tstaking.DelegateWithPower(sdk.AccAddress(pks[2].Address()), valAddr, 50)
 
 	validatorUpdates = staking.EndBlocker(ctx, app.StakingKeeper)
 	require.Equal(t, 2, len(validatorUpdates))
 	tstaking.CheckValidator(valAddr, stakingtypes.Bonded, false)
-	newPower := int64(150)
+	newPower := power + 50
 
 	// validator misses a block
 	app.SlashingKeeper.HandleValidatorSignature(ctx, val.Address(), newPower, false)
@@ -234,9 +237,9 @@ func TestValidatorDippingInAndOut(t *testing.T) {
 	// shouldn't be jailed/kicked yet
 	tstaking.CheckValidator(valAddr, stakingtypes.Bonded, false)
 
-	// validator misses 500 more blocks, 501 total
-	latest := height
-	for ; height < latest+500; height++ {
+	// validator misses an additional 500 more blocks, after the cooling off period of SignedBlockWindow (here 1000 blocks).
+	latest := app.SlashingKeeper.SignedBlocksWindow(ctx) + height
+	for ; height < latest+app.SlashingKeeper.MinSignedPerWindow(ctx); height++ {
 		ctx = ctx.WithBlockHeight(height)
 		app.SlashingKeeper.HandleValidatorSignature(ctx, val.Address(), newPower, false)
 	}
@@ -248,30 +251,31 @@ func TestValidatorDippingInAndOut(t *testing.T) {
 	// check all the signing information
 	signInfo, found := app.SlashingKeeper.GetValidatorSigningInfo(ctx, consAddr)
 	require.True(t, found)
-	require.Equal(t, int64(0), signInfo.MissedBlocksCounter)
-	require.Equal(t, int64(0), signInfo.IndexOffset)
-	// array should be cleared
-	for offset := int64(0); offset < app.SlashingKeeper.SignedBlocksWindow(ctx); offset++ {
-		missed := app.SlashingKeeper.GetValidatorMissedBlockBitArray(ctx, consAddr, offset)
-		require.False(t, missed)
-	}
+	require.Equal(t, int64(700), signInfo.StartHeight)
+	require.Equal(t, int64(499), signInfo.MissedBlocksCounter)
+	require.Equal(t, int64(499), signInfo.IndexOffset)
 
 	// some blocks pass
 	height = int64(5000)
 	ctx = ctx.WithBlockHeight(height)
 
 	// validator rejoins and starts signing again
 	app.StakingKeeper.Unjail(ctx, consAddr)
+
 	app.SlashingKeeper.HandleValidatorSignature(ctx, val.Address(), newPower, true)
-	height++
 
 	// validator should not be kicked since we reset counter/array when it was jailed
 	staking.EndBlocker(ctx, app.StakingKeeper)
 	tstaking.CheckValidator(valAddr, stakingtypes.Bonded, false)
 
-	// validator misses 501 blocks
-	latest = height
-	for ; height < latest+501; height++ {
+	// check start height is correctly set
+	signInfo, found = app.SlashingKeeper.GetValidatorSigningInfo(ctx, consAddr)
+	require.True(t, found)
+	require.Equal(t, height, signInfo.StartHeight)
+
+	// validator misses 501 blocks after SignedBlockWindow period (1000 blocks)
+	latest = app.SlashingKeeper.SignedBlocksWindow(ctx) + height
+	for ; height < latest+app.SlashingKeeper.MinSignedPerWindow(ctx); height++ {
 		ctx = ctx.WithBlockHeight(height)
 		app.SlashingKeeper.HandleValidatorSignature(ctx, val.Address(), newPower, false)
 	}

x/slashing/spec/01_concepts.md

@@ -43,16 +43,14 @@ _V<sub>u</sub>_ : validator unbonded
 
 ### Single Double Sign Infraction
 
-<----------------->
-[----------C<sub>1</sub>----D<sub>1</sub>,V<sub>u</sub>-----]
+\[----------C<sub>1</sub>----D<sub>1</sub>,V<sub>u</sub>-----\]
 
 A single infraction is committed then later discovered, at which point the
 validator is unbonded and slashed at the full amount for the infraction.
 
 ### Multiple Double Sign Infractions
 
-<--------------------------->
-[----------C<sub>1</sub>--C<sub>2</sub>---C<sub>3</sub>---D<sub>1</sub>,D<sub>2</sub>,D<sub>3</sub>V<sub>u</sub>-----]
+\[----------C<sub>1</sub>--C<sub>2</sub>---C<sub>3</sub>---D<sub>1</sub>,D<sub>2</sub>,D<sub>3</sub>V<sub>u</sub>-----\]
 
 Multiple infractions are committed and then later discovered, at which point the
 validator is jailed and slashed for only one infraction. Because the validator

x/slashing/spec/05_hooks.md

@@ -21,6 +21,8 @@ The following hooks impact the slashing state:
 Upon successful first-time bonding of a new validator, we create a new `ValidatorSigningInfo` structure for the
 now-bonded validator, which `StartHeight` of the current block.
 
+If the validator was out of the validator set and gets bonded again, its new bonded height is set.
+
 ```go
 onValidatorBonded(address sdk.ValAddress)
 
@@ -32,7 +34,10 @@ onValidatorBonded(address sdk.ValAddress)
       JailedUntil         : time.Unix(0, 0),
       Tombstone           : false,
       MissedBloskCounter  : 0
+    } else {
+      signingInfo.StartHeight = CurrentHeight
     }
+
     setValidatorSigningInfo(signingInfo)
   }