Merge PR #1805: Downtime slashing off-by-one-block fix

* Avoid slashing & revoking no longer stored or already revoked validators for downtime
* Add testcase
* Update PENDING.md

PENDING.md

@@ -0,0 +1,49 @@
+## PENDING
+
+BREAKING CHANGES
+* [baseapp] Msgs are no longer run on CheckTx, removed `ctx.IsCheckTx()`
+* [x/gov] CLI flag changed from `proposalID` to `proposal-id`
+* [x/stake] Fixed the period check for the inflation calculation
+* [x/stake] Inflation doesn't use rationals in calculation (performance boost)
+* [baseapp] NewBaseApp constructor now takes sdk.TxDecoder as argument instead of wire.Codec
+* [x/auth] Default TxDecoder can be found in `x/auth` rather than baseapp
+* \#1606 The following CLI commands have been switched to use `--from`
+  * `gaiacli stake create-validator --address-validator`
+  * `gaiacli stake edit-validator --address-validator`
+  * `gaiacli stake delegate --address-delegator`
+  * `gaiacli stake unbond begin --address-delegator`
+  * `gaiacli stake unbond complete --address-delegator`
+  * `gaiacli stake redelegate begin --address-delegator`
+  * `gaiacli stake redelegate complete --address-delegator`
+  * `gaiacli stake unrevoke [validator-address]`
+  * `gaiacli gov submit-proposal --proposer`
+  * `gaiacli gov deposit --depositer`
+  * `gaiacli gov vote --voter`
+* [x/gov] Added tags sub-package, changed tags to use dash-case 
+
+FEATURES
+* [lcd] Can now query governance proposals by ProposalStatus
+* [x/mock/simulation] Randomized simulation framework
+  * Modules specify invariants and operations, preferably in an x/[module]/simulation package
+  * Modules can test random combinations of their own operations
+  * Applications can integrate operations and invariants from modules together for an integrated simulation
+* [baseapp] Initialize validator set on ResponseInitChain
+* [cosmos-sdk-cli] Added support for cosmos-sdk-cli tool under cosmos-sdk/cmd	
+   * This allows SDK users to initialize a new project repository.
+* [tests] Remotenet commands for AWS (awsnet)
+
+IMPROVEMENTS
+* [baseapp] Allow any alphanumeric character in route
+* [cli] Improve error messages for all txs when the account doesn't exist
+* [tools] Remove `rm -rf vendor/` from `make get_vendor_deps`
+* [x/auth] Recover ErrorOutOfGas panic in order to set sdk.Result attributes correctly
+* [x/stake] Add revoked to human-readable validator 
+* [tests] Add tests to example apps in docs
+* [x/gov] Votes on a proposal can now be queried
+* [x/bank] Unit tests are now table-driven
+* [tests] Fixes ansible scripts to work with AWS too
+
+BUG FIXES
+*  \#1666 Add intra-tx counter to the genesis validators
+*  \#1797 Fix off-by-one error in slashing for downtime
+*  \#1787 Fixed bug where Tally fails due to revoked/unbonding validator

examples/democoin/mock/validator.go

@@ -82,6 +82,11 @@ func (vs *ValidatorSet) Validator(ctx sdk.Context, addr sdk.AccAddress) sdk.Vali
 	return nil
 }
 
+// ValidatorByPubKey implements sdk.ValidatorSet
+func (vs *ValidatorSet) ValidatorByPubKey(ctx sdk.Context, pubkey crypto.PubKey) sdk.Validator {
+	panic("not implemented")
+}
+
 // TotalPower implements sdk.ValidatorSet
 func (vs *ValidatorSet) TotalPower(ctx sdk.Context) sdk.Rat {
 	res := sdk.ZeroRat()

types/stake.go

@@ -65,8 +65,9 @@ type ValidatorSet interface {
 	IterateValidatorsBonded(Context,
 		func(index int64, validator Validator) (stop bool))
 
-	Validator(Context, AccAddress) Validator // get a particular validator by owner AccAddress
-	TotalPower(Context) Rat                  // total power of the validator set
+	Validator(Context, AccAddress) Validator            // get a particular validator by owner AccAddress
+	ValidatorByPubKey(Context, crypto.PubKey) Validator // get a particular validator by signing PubKey
+	TotalPower(Context) Rat                             // total power of the validator set
 
 	// slash the validator and delegators of the validator, specifying offence height, offence power, and slash fraction
 	Slash(Context, crypto.PubKey, int64, int64, Rat)

x/slashing/keeper.go

@@ -61,6 +61,7 @@ func (k Keeper) handleDoubleSign(ctx sdk.Context, pubkey crypto.PubKey, infracti
 }
 
 // handle a validator signature, must be called once per validator per block
+// nolint gocyclo
 func (k Keeper) handleValidatorSignature(ctx sdk.Context, pubkey crypto.PubKey, power int64, signed bool) {
 	logger := ctx.Logger().With("module", "x/slashing")
 	height := ctx.BlockHeight()
@@ -97,11 +98,19 @@ func (k Keeper) handleValidatorSignature(ctx sdk.Context, pubkey crypto.PubKey,
 	}
 	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.validatorSet.Slash(ctx, pubkey, height, power, SlashFractionDowntime)
-		k.validatorSet.Revoke(ctx, pubkey)
-		signInfo.JailedUntil = ctx.BlockHeader().Time + DowntimeUnbondDuration
+		validator := k.validatorSet.ValidatorByPubKey(ctx, pubkey)
+		if validator != nil && !validator.GetRevoked() {
+			// 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.validatorSet.Slash(ctx, pubkey, height, power, SlashFractionDowntime)
+			k.validatorSet.Revoke(ctx, pubkey)
+			signInfo.JailedUntil = ctx.BlockHeader().Time + DowntimeUnbondDuration
+		} else {
+			// Validator was (a) not found or (b) already revoked, don't slash
+			logger.Info(fmt.Sprintf("Validator %s would have been slashed for downtime, but was either not found in store or already revoked",
+				pubkey.Address()))
+		}
 	}
 
 	// Set the updated signing info

x/slashing/keeper_test.go

@@ -197,3 +197,47 @@ func TestHandleNewValidator(t *testing.T) {
 	pool := sk.GetPool(ctx)
 	require.Equal(t, int64(100), pool.BondedTokens.RoundInt64())
 }
+
+// Test a revoked validator being "down" twice
+// Ensure that they're only slashed once
+func TestHandleAlreadyRevoked(t *testing.T) {
+
+	// initial setup
+	ctx, _, sk, keeper := createTestInput(t)
+	amtInt := int64(100)
+	addr, val, amt := addrs[0], pks[0], sdk.NewInt(amtInt)
+	sh := stake.NewHandler(sk)
+	got := sh(ctx, newTestMsgCreateValidator(addr, val, amt))
+	require.True(t, got.IsOK())
+	stake.EndBlocker(ctx, sk)
+
+	// 1000 first blocks OK
+	height := int64(0)
+	for ; height < SignedBlocksWindow; height++ {
+		ctx = ctx.WithBlockHeight(height)
+		keeper.handleValidatorSignature(ctx, val, amtInt, true)
+	}
+
+	// 501 blocks missed
+	for ; height < SignedBlocksWindow+(SignedBlocksWindow-MinSignedPerWindow)+1; height++ {
+		ctx = ctx.WithBlockHeight(height)
+		keeper.handleValidatorSignature(ctx, val, amtInt, false)
+	}
+
+	// validator should have been revoked and slashed
+	validator, _ := sk.GetValidatorByPubKey(ctx, val)
+	require.Equal(t, sdk.Unbonded, validator.GetStatus())
+
+	// validator should have been slashed
+	slashAmt := sdk.NewRat(amtInt).Mul(SlashFractionDowntime).RoundInt64()
+	require.Equal(t, int64(amtInt)-slashAmt, validator.Tokens.RoundInt64()) // TODO replace w/ .GetTokens()
+
+	// another block missed
+	ctx = ctx.WithBlockHeight(height)
+	keeper.handleValidatorSignature(ctx, val, amtInt, false)
+
+	// validator should not have been slashed twice
+	validator, _ = sk.GetValidatorByPubKey(ctx, val)
+	require.Equal(t, int64(amtInt)-slashAmt, validator.Tokens.RoundInt64()) // TODO replace w/ .GetTokens()
+
+}

x/stake/keeper/sdk_types.go

@@ -3,6 +3,8 @@ package keeper
 import (
 	"fmt"
 
+	"github.com/tendermint/tendermint/crypto"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/stake/types"
 )
@@ -57,6 +59,15 @@ func (k Keeper) Validator(ctx sdk.Context, address sdk.AccAddress) sdk.Validator
 	return val
 }
 
+// get the sdk.validator for a particular pubkey
+func (k Keeper) ValidatorByPubKey(ctx sdk.Context, pubkey crypto.PubKey) sdk.Validator {
+	val, found := k.GetValidatorByPubKey(ctx, pubkey)
+	if !found {
+		return nil
+	}
+	return val
+}
+
 // total power from the bond
 func (k Keeper) TotalPower(ctx sdk.Context) sdk.Rat {
 	pool := k.GetPool(ctx)