Skip to content

[wip] slashing: remove validator missed bit array from HandleValidatorSignature #4641

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 20 additions & 31 deletions x/slashing/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,30 +160,17 @@ func (k Keeper) HandleValidatorSignature(ctx sdk.Context, addr crypto.Address, p
panic(fmt.Sprintf("Expected signing info for validator %s but not found", consAddr))
}

// this is a relative index, so it counts blocks the validator *should* have signed
// will use the 0-value default signing info if not present, except for start height
index := signInfo.IndexOffset % k.SignedBlocksWindow(ctx)
signInfo.IndexOffset++

// Update signed block bit array & counter
// This counter just tracks the sum of the bit array
// That way we avoid needing to read/write the whole array each time
previous := k.getValidatorMissedBlockBitArray(ctx, consAddr, index)
missed := !signed
switch {
case !previous && missed:
// Array value has changed from not missed to missed, increment counter
k.setValidatorMissedBlockBitArray(ctx, consAddr, index, true)
signInfo.MissedBlocksCounter++
case previous && !missed:
// Array value has changed from missed to not missed, decrement counter
k.setValidatorMissedBlockBitArray(ctx, consAddr, index, false)
signInfo.MissedBlocksCounter--
default:
// Array value at this index has not changed, no need to update counter
}
signWindow := k.SignedBlocksWindow(ctx)
minSignedPerWindow := k.MinSignedPerWindow(ctx)
maxMissed := signWindow - minSignedPerWindow

if !signed {
// no need to increment the missed blocks counter beyond the sign window
if signInfo.MissedBlocksCounter < signWindow {
signInfo.MissedBlocksCounter++
}
signInfo.LastMissHeight = height

if missed {
ctx.EventManager().EmitEvent(
sdk.NewEvent(
types.EventTypeLiveness,
Expand All @@ -193,20 +180,23 @@ func (k Keeper) HandleValidatorSignature(ctx sdk.Context, addr crypto.Address, p
),
)

logger.Info(fmt.Sprintf("Absent validator %s (%s) at height %d, %d missed, threshold %d", addr, pubkey, height, signInfo.MissedBlocksCounter, k.MinSignedPerWindow(ctx)))
logger.Info(fmt.Sprintf("Absent validator %s (%v) at height %d, %d missed, threshold %d", addr, pubkey, height, signInfo.MissedBlocksCounter, k.MinSignedPerWindow(ctx)))
}

minHeight := signInfo.StartHeight + k.SignedBlocksWindow(ctx)
maxMissed := k.SignedBlocksWindow(ctx) - k.MinSignedPerWindow(ctx)
// Only when the last miss height is past the sign window can we decrement the miss block counter
// This ensures that a missed block is tracked throughout its lifetime in the sign window
if signed && signInfo.MissedBlocksCounter > 0 && (signInfo.LastMissHeight+signWindow) < height {
Copy link
Contributor

@alexanderbez alexanderbez Jul 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to exactly resemble the same logic as the rolling window? What if I miss a block within signed window?

(essentially what @cwgoes stated)

signInfo.MissedBlocksCounter--
}

// if we are past the minimum height and the validator has missed too many blocks, punish them
if height > minHeight && signInfo.MissedBlocksCounter > maxMissed {
// if the validator has missed too many blocks, punish them
if signInfo.MissedBlocksCounter > maxMissed {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be off by one here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept the same logic as before which was also strictly greater-than.

validator := k.sk.ValidatorByConsAddr(ctx, consAddr)
if validator != nil && !validator.IsJailed() {

// Downtime confirmed: slash and jail the validator
logger.Info(fmt.Sprintf("Validator %s past min height of %d and below signed blocks threshold of %d",
sdk.ConsAddress(pubkey.Address()), minHeight, k.MinSignedPerWindow(ctx)))
logger.Info(fmt.Sprintf("Validator %s below signed blocks threshold of %d",
pubkey.Address(), k.MinSignedPerWindow(ctx)))

// We need to retrieve the stake distribution which signed the block, so we subtract ValidatorUpdateDelay from the evidence height,
// and subtract an additional 1 since this is the LastCommit.
Expand All @@ -231,7 +221,6 @@ func (k Keeper) HandleValidatorSignature(ctx sdk.Context, addr crypto.Address, p

// We need to reset the counter & array so that the validator won't be immediately slashed for downtime upon rebonding.
signInfo.MissedBlocksCounter = 0
signInfo.IndexOffset = 0
k.clearValidatorMissedBlockBitArray(ctx, consAddr)
} else {
// Validator was (a) not found or (b) already jailed, don't slash
Expand Down
11 changes: 3 additions & 8 deletions x/slashing/types/signing_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,20 @@ import (
// Signing info for a validator
type ValidatorSigningInfo struct {
Address sdk.ConsAddress `json:"address"` // validator consensus address
StartHeight int64 `json:"start_height"` // height at which validator was first a candidate OR was unjailed
IndexOffset int64 `json:"index_offset"` // index offset into signed block bit array
LastMissHeight int64 `json:"last_miss_height"` // height at which the validator last missed a block
JailedUntil time.Time `json:"jailed_until"` // timestamp validator cannot be unjailed until
Tombstoned bool `json:"tombstoned"` // whether or not a validator has been tombstoned (killed out of validator set)
MissedBlocksCounter int64 `json:"missed_blocks_counter"` // missed blocks counter (to avoid scanning the array every time)
}

// Construct a new `ValidatorSigningInfo` struct
func NewValidatorSigningInfo(
condAddr sdk.ConsAddress, startHeight, indexOffset int64,
condAddr sdk.ConsAddress, indexOffset int64,
jailedUntil time.Time, tombstoned bool, missedBlocksCounter int64,
) ValidatorSigningInfo {

return ValidatorSigningInfo{
Address: condAddr,
StartHeight: startHeight,
IndexOffset: indexOffset,
JailedUntil: jailedUntil,
Tombstoned: tombstoned,
MissedBlocksCounter: missedBlocksCounter,
Expand All @@ -37,11 +34,9 @@ func NewValidatorSigningInfo(
func (i ValidatorSigningInfo) String() string {
return fmt.Sprintf(`Validator Signing Info:
Address: %s
Start Height: %d
Index Offset: %d
Jailed Until: %v
Tombstoned: %t
Missed Blocks Counter: %d`,
i.Address, i.StartHeight, i.IndexOffset, i.JailedUntil,
i.Address, i.JailedUntil,
i.Tombstoned, i.MissedBlocksCounter)
}