From 0fa90ad61466f3627707e3a2099b67ec9f394544 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 18 May 2022 16:08:05 +0200 Subject: [PATCH] fix: update `StartHeight` signing info in `AfterValidatorBonded` hook when validator re-bonds (#11973) --- x/slashing/keeper/hooks.go | 25 +++++++++++++----- x/slashing/keeper/keeper_test.go | 44 +++++++++++++++++--------------- x/slashing/spec/01_concepts.md | 6 ++--- x/slashing/spec/05_hooks.md | 5 ++++ 4 files changed, 50 insertions(+), 30 deletions(-) diff --git a/x/slashing/keeper/hooks.go b/x/slashing/keeper/hooks.go index 09d1afe02fac..a306f76c210d 100644 --- a/x/slashing/keeper/hooks.go +++ b/x/slashing/keeper/hooks.go @@ -11,9 +11,11 @@ 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, @@ -21,9 +23,10 @@ func (k Keeper) AfterValidatorBonded(ctx sdk.Context, address sdk.ConsAddress, _ 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 +} diff --git a/x/slashing/keeper/keeper_test.go b/x/slashing/keeper/keeper_test.go index 7385c1f4d7e2..08502fa89c94 100644 --- a/x/slashing/keeper/keeper_test.go +++ b/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,13 +251,9 @@ 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) @@ -262,16 +261,21 @@ func TestValidatorDippingInAndOut(t *testing.T) { // 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) } diff --git a/x/slashing/spec/01_concepts.md b/x/slashing/spec/01_concepts.md index 9505706f90d3..ea7c6b319bae 100644 --- a/x/slashing/spec/01_concepts.md +++ b/x/slashing/spec/01_concepts.md @@ -43,16 +43,14 @@ _Vu_ : validator unbonded ### Single Double Sign Infraction -<-----------------> -[----------C1----D1,Vu-----] +\[----------C1----D1,Vu-----\] 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 -<---------------------------> -[----------C1--C2---C3---D1,D2,D3Vu-----] +\[----------C1--C2---C3---D1,D2,D3Vu-----\] Multiple infractions are committed and then later discovered, at which point the validator is jailed and slashed for only one infraction. Because the validator diff --git a/x/slashing/spec/05_hooks.md b/x/slashing/spec/05_hooks.md index d1234e58ee05..a839689429a8 100644 --- a/x/slashing/spec/05_hooks.md +++ b/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) }