Add some explanatory comments

x/slashing/handler.go

@@ -16,19 +16,25 @@ func NewHandler(k Keeper) sdk.Handler {
 	}
 }
 
+// Validators must submit a transaction to unrevoke themselves after
+// having been revoked (and thus unbonded) for downtime
 func handleMsgUnrevoke(ctx sdk.Context, msg MsgUnrevoke, k Keeper) sdk.Result {
+
+	// Validator must exist
 	validator := k.stakeKeeper.Validator(ctx, msg.ValidatorAddr)
 	if validator == nil {
 		return ErrNoValidatorForAddress(k.codespace).Result()
 	}
 
 	addr := validator.GetPubKey().Address()
 
+	// Signing info must exist
 	info, found := k.getValidatorSigningInfo(ctx, addr)
 	if !found {
 		return ErrNoValidatorForAddress(k.codespace).Result()
 	}
 
+	// Cannot be unrevoked until out of jail
 	if ctx.BlockHeader().Time < info.JailedUntil {
 		return ErrValidatorJailed(k.codespace).Result()
 	}

x/slashing/keeper.go

@@ -34,10 +34,14 @@ func NewKeeper(cdc *wire.Codec, key sdk.StoreKey, sk stake.Keeper, codespace sdk
 func (k Keeper) handleDoubleSign(ctx sdk.Context, height int64, timestamp int64, pubkey crypto.PubKey) {
 	logger := ctx.Logger().With("module", "x/slashing")
 	age := ctx.BlockHeader().Time - timestamp
+
+	// Double sign too old
 	if age > MaxEvidenceAge {
 		logger.Info(fmt.Sprintf("Ignored double sign from %s at height %d, age of %d past max age of %d", pubkey.Address(), height, age, MaxEvidenceAge))
 		return
 	}
+
+	// Double sign confirmed
 	logger.Info(fmt.Sprintf("Confirmed double sign from %s at height %d, age of %d less than max age of %d", pubkey.Address(), height, age, MaxEvidenceAge))
 	k.stakeKeeper.Slash(ctx, pubkey, height, SlashFractionDoubleSign)
 }
@@ -50,9 +54,13 @@ func (k Keeper) handleValidatorSignature(ctx sdk.Context, pubkey crypto.PubKey,
 		logger.Info(fmt.Sprintf("Absent validator %s at height %d", pubkey.Address(), height))
 	}
 	address := pubkey.Address()
+
+	// Local index, so counts blocks validator *should* have signed
 	signInfo, _ := k.getValidatorSigningInfo(ctx, address)
 	index := signInfo.IndexOffset % SignedBlocksWindow
 	signInfo.IndexOffset++
+
+	// Update signed block bit array & counter
 	previous := k.getValidatorSigningBitArray(ctx, address, index)
 	if previous && !signed {
 		k.setValidatorSigningBitArray(ctx, address, index, false)
@@ -63,8 +71,10 @@ func (k Keeper) handleValidatorSignature(ctx sdk.Context, pubkey crypto.PubKey,
 		signInfo.SignedBlocksCounter++
 		k.setValidatorSigningInfo(ctx, address, signInfo)
 	}
+
 	minHeight := signInfo.StartHeight + SignedBlocksWindow
 	if height > minHeight && signInfo.SignedBlocksCounter < MinSignedPerWindow {
+		// Downtime confirmed, slash, revoke, and jail the validator
 		logger.Info(fmt.Sprintf("Validator %s past min height of %d and below signed blocks threshold of %d", pubkey.Address(), minHeight, MinSignedPerWindow))
 		k.stakeKeeper.Slash(ctx, pubkey, height, SlashFractionDowntime)
 		k.stakeKeeper.Revoke(ctx, pubkey)

x/slashing/keeper_test.go

@@ -39,7 +39,9 @@ func TestHandleAbsentValidator(t *testing.T) {
 	info, found := keeper.getValidatorSigningInfo(ctx, val.Address())
 	require.False(t, found)
 	require.Equal(t, int64(0), info.StartHeight)
+	require.Equal(t, int64(0), info.IndexOffset)
 	require.Equal(t, int64(0), info.SignedBlocksCounter)
+	require.Equal(t, int64(0), info.JailedUntil)
 	height := int64(0)
 	// 1000 blocks OK
 	for ; height < 1000; height++ {
@@ -59,7 +61,7 @@ func TestHandleAbsentValidator(t *testing.T) {
 	require.True(t, found)
 	require.Equal(t, int64(0), info.StartHeight)
 	require.Equal(t, SignedBlocksWindow-50, info.SignedBlocksCounter)
-	// should be bonded still
+	// validator should be bonded still
 	validator := sk.ValidatorByPubKey(ctx, val)
 	require.Equal(t, sdk.Bonded, validator.GetStatus())
 	pool := sk.GetPool(ctx)
@@ -71,31 +73,34 @@ func TestHandleAbsentValidator(t *testing.T) {
 	require.True(t, found)
 	require.Equal(t, int64(0), info.StartHeight)
 	require.Equal(t, SignedBlocksWindow-51, info.SignedBlocksCounter)
-	// should have been revoked
+	// validator should have been revoked
 	validator = sk.ValidatorByPubKey(ctx, val)
 	require.Equal(t, sdk.Unbonded, validator.GetStatus())
+	// unrevocation should fail prior to jail expiration
 	got = slh(ctx, NewMsgUnrevoke(addr))
-	require.False(t, got.IsOK()) // should fail prior to jail expiration
+	require.False(t, got.IsOK())
+	// unrevocation should succeed after jail expiration
 	ctx = ctx.WithBlockHeader(abci.Header{Time: int64(86400 * 2)})
 	got = slh(ctx, NewMsgUnrevoke(addr))
-	require.True(t, got.IsOK()) // should succeed after jail expiration
+	require.True(t, got.IsOK())
+	// validator should be rebonded now
 	validator = sk.ValidatorByPubKey(ctx, val)
 	require.Equal(t, sdk.Bonded, validator.GetStatus())
-	// should have been slashed
+	// validator should have been slashed
 	pool = sk.GetPool(ctx)
 	require.Equal(t, int64(99), pool.BondedTokens)
-	// start height should have been changed
+	// validator start height should have been changed
 	info, found = keeper.getValidatorSigningInfo(ctx, val.Address())
 	require.True(t, found)
 	require.Equal(t, height, info.StartHeight)
 	require.Equal(t, SignedBlocksWindow-51, info.SignedBlocksCounter)
-	// should not be immediately revoked again
+	// validator should not be immediately revoked again
 	height++
 	ctx = ctx.WithBlockHeight(height)
 	keeper.handleValidatorSignature(ctx, val, false)
 	validator = sk.ValidatorByPubKey(ctx, val)
 	require.Equal(t, sdk.Bonded, validator.GetStatus())
-	// should be revoked again after 100 blocks
+	// validator should be revoked again after 100 unsigned blocks
 	nextHeight := height + 100
 	for ; height <= nextHeight; height++ {
 		ctx = ctx.WithBlockHeight(height)

x/slashing/tick.go

@@ -12,9 +12,12 @@ import (
 
 func NewBeginBlocker(sk Keeper) sdk.BeginBlocker {
 	return func(ctx sdk.Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock {
+		// Tag the height
 		heightBytes := make([]byte, 8)
 		binary.LittleEndian.PutUint64(heightBytes, uint64(req.Header.Height))
 		tags := sdk.NewTags("height", heightBytes)
+
+		// Deal with any equivocation evidence
 		for _, evidence := range req.ByzantineValidators {
 			var pk crypto.PubKey
 			sk.cdc.MustUnmarshalBinary(evidence.PubKey, &pk)
@@ -25,18 +28,25 @@ func NewBeginBlocker(sk Keeper) sdk.BeginBlocker {
 				ctx.Logger().With("module", "x/slashing").Error(fmt.Sprintf("Ignored unknown evidence type: %s", string(evidence.Type)))
 			}
 		}
+
+		// Figure out which validators were absent
 		absent := make(map[string]bool)
 		for _, pubkey := range req.AbsentValidators {
 			var pk crypto.PubKey
 			sk.cdc.MustUnmarshalBinary(pubkey, &pk)
 			absent[string(pk.Bytes())] = true
 		}
+
+		// Iterate over all the validators which *should* have signed this block
 		sk.stakeKeeper.IterateValidatorsBonded(ctx, func(_ int64, validator sdk.Validator) (stop bool) {
 			pubkey := validator.GetPubKey()
 			sk.handleValidatorSignature(ctx, pubkey, !absent[string(pubkey.Bytes())])
 			return false
 		})
-		// TODO Add some more tags so clients can track slashing
+
+		// Return the begin block response
+		// TODO Return something composable, so other modules can also have BeginBlockers
+		// TODO Add some more tags so clients can track slashing events
 		return abci.ResponseBeginBlock{
 			Tags: tags.ToKVPairs(),
 		}

x/stake/keeper.go

@@ -799,7 +799,8 @@ func (k Keeper) Revoke(ctx sdk.Context, pubkey crypto.PubKey) {
 	logger := ctx.Logger().With("module", "x/stake")
 	val, found := k.GetValidatorByPubKey(ctx, pubkey)
 	if !found {
-		ctx.Logger().Info("Validator with pubkey %s not found, cannot force unbond", pubkey)
+		// TODO Should we panic?
+		ctx.Logger().Info("Validator with pubkey %s not found, cannot revoke", pubkey)
 		return
 	}
 	val.Revoked = true
@@ -813,7 +814,8 @@ func (k Keeper) Unrevoke(ctx sdk.Context, pubkey crypto.PubKey) {
 	logger := ctx.Logger().With("module", "x/stake")
 	val, found := k.GetValidatorByPubKey(ctx, pubkey)
 	if !found {
-		ctx.Logger().Info("Validator with pubkey %s not found, cannot force unbond", pubkey)
+		// TODO Should we panic?
+		ctx.Logger().Info("Validator with pubkey %s not found, cannot unrevoke", pubkey)
 		return
 	}
 	val.Revoked = false