Testcase fixes, 'make format', negative pre-genesis block

client/rpc/validators.go

@@ -11,10 +11,10 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/client/context"
+	"github.com/cosmos/cosmos-sdk/client/utils"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/spf13/viper"
 	tmtypes "github.com/tendermint/tendermint/types"
-	"github.com/cosmos/cosmos-sdk/client/utils"
 )
 
 // TODO these next two functions feel kinda hacky based on their placement

x/slashing/keeper.go

@@ -56,23 +56,26 @@ func (k Keeper) handleDoubleSign(ctx sdk.Context, addr crypto.Address, infractio
 	// 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(), infractionHeight, age, maxEvidenceAge))
 
-	// Cap the amount slashed to the penalty for the worst infraction
-	// within the slashing period when this infraction was committed
-	fraction := k.SlashFractionDoubleSign(ctx)
-	revisedFraction := k.capBySlashingPeriod(ctx, consAddr, fraction, infractionHeight)
-	logger.Info(fmt.Sprintf("Fraction slashed capped by slashing period from %v to %v", fraction, revisedFraction))
-
 	// We need to retrieve the stake distribution which signed the block, so we subtract ValidatorUpdateDelay from the evidence height.
 	// Note that this *can* result in a "distributionHeight" of -1,
 	// i.e. at the end of the pre-genesis block (none) = at the beginning of the genesis block.
 	// That's fine since this is just used to filter unbonding delegations & redelegations.
 	distributionHeight := infractionHeight - ValidatorUpdateDelay
 
+	// Cap the amount slashed to the penalty for the worst infraction
+	// within the slashing period when this infraction was committed
+	fraction := k.SlashFractionDoubleSign(ctx)
+	revisedFraction := k.capBySlashingPeriod(ctx, consAddr, fraction, distributionHeight)
+	logger.Info(fmt.Sprintf("Fraction slashed capped by slashing period from %v to %v", fraction, revisedFraction))
+
 	// Slash validator
 	k.validatorSet.Slash(ctx, consAddr, distributionHeight, power, revisedFraction)
 
-	// Jail validator
+	// Jail validator if not already jailed
+	validator := k.validatorSet.ValidatorByConsAddr(ctx, consAddr)
+	if !validator.GetJailed() {
 		k.validatorSet.Jail(ctx, consAddr)
+	}
 
 	// Set validator jail duration
 	signInfo, found := k.getValidatorSigningInfo(ctx, consAddr)

x/slashing/keeper_test.go

@@ -74,6 +74,7 @@ func TestSlashingPeriodCap(t *testing.T) {
 	got := stake.NewHandler(sk)(ctx, newTestMsgCreateValidator(operatorAddr, valConsPubKey, amt))
 	require.True(t, got.IsOK())
 	validatorUpdates := stake.EndBlocker(ctx, sk)
+	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1)
 	keeper.AddValidators(ctx, validatorUpdates)
 	require.Equal(t, ck.GetCoins(ctx, sdk.AccAddress(operatorAddr)), sdk.Coins{{sk.GetParams(ctx).BondDenom, initCoins.Sub(amt)}})
 	require.True(t, sdk.NewDecFromInt(amt).Equal(sk.Validator(ctx, operatorAddr).GetPower()))
@@ -82,23 +83,7 @@ func TestSlashingPeriodCap(t *testing.T) {
 	keeper.handleValidatorSignature(ctx, valConsAddr, amtInt, true)
 
 	// double sign less than max age
-	keeper.handleDoubleSign(ctx, valConsAddr, 0, time.Unix(0, 0), amtInt)
+	keeper.handleDoubleSign(ctx, valConsAddr, 1, time.Unix(0, 0), amtInt)
-	// should be jailed
-	require.True(t, sk.Validator(ctx, operatorAddr).GetJailed())
-	// end block
-	stake.EndBlocker(ctx, sk)
-	// update block height
-	ctx = ctx.WithBlockHeight(int64(1))
-	// unjail to measure power
-	sk.Unjail(ctx, sdk.ConsAddress(valConsAddr))
-	// end block
-	stake.EndBlocker(ctx, sk)
-	// power should be reduced
-	expectedPower := sdk.NewDecFromInt(amt).Mul(sdk.NewDec(19).Quo(sdk.NewDec(20)))
-	require.Equal(t, expectedPower, sk.Validator(ctx, operatorAddr).GetPower())
-
-	// double sign again, same slashing period
-	keeper.handleDoubleSign(ctx, valConsAddr, 0, time.Unix(0, 0), amtInt)
 	// should be jailed
 	require.True(t, sk.Validator(ctx, operatorAddr).GetJailed())
 	// end block
@@ -109,12 +94,28 @@ func TestSlashingPeriodCap(t *testing.T) {
 	sk.Unjail(ctx, sdk.ConsAddress(valConsAddr))
 	// end block
 	stake.EndBlocker(ctx, sk)
+	// power should be reduced
+	expectedPower := sdk.NewDecFromInt(amt).Mul(sdk.NewDec(19).Quo(sdk.NewDec(20)))
+	require.Equal(t, expectedPower, sk.Validator(ctx, operatorAddr).GetPower())
+
+	// double sign again, same slashing period
+	keeper.handleDoubleSign(ctx, valConsAddr, 1, time.Unix(0, 0), amtInt)
+	// should be jailed
+	require.True(t, sk.Validator(ctx, operatorAddr).GetJailed())
+	// end block
+	stake.EndBlocker(ctx, sk)
+	// update block height
+	ctx = ctx.WithBlockHeight(int64(3))
+	// unjail to measure power
+	sk.Unjail(ctx, sdk.ConsAddress(valConsAddr))
+	// end block
+	stake.EndBlocker(ctx, sk)
 	// power should be equal, no more should have been slashed
 	expectedPower = sdk.NewDecFromInt(amt).Mul(sdk.NewDec(19).Quo(sdk.NewDec(20)))
 	require.Equal(t, expectedPower, sk.Validator(ctx, operatorAddr).GetPower())
 
 	// double sign again, new slashing period
-	keeper.handleDoubleSign(ctx, valConsAddr, 2, time.Unix(0, 0), amtInt)
+	keeper.handleDoubleSign(ctx, valConsAddr, 3, time.Unix(0, 0), amtInt)
 	// should be jailed
 	require.True(t, sk.Validator(ctx, operatorAddr).GetJailed())
 	// unjail to measure power
@@ -210,8 +211,16 @@ func TestHandleAbsentValidator(t *testing.T) {
 	stake.EndBlocker(ctx, sk)
 
 	// validator should not have been slashed any more, since it was already jailed
+	validator, _ = sk.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(val))
 	require.Equal(t, int64(99), validator.GetTokens().RoundInt64())
 
+	// 502nd block *double signed* (oh no!)
+	keeper.handleDoubleSign(ctx, val.Address(), height, ctx.BlockHeader().Time, amtInt)
+
+	// validator should have been slashed
+	validator, _ = sk.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(val))
+	require.Equal(t, int64(94), validator.GetTokens().RoundInt64())
+
 	// unrevocation should fail prior to jail expiration
 	got = slh(ctx, NewMsgUnjail(addr))
 	require.False(t, got.IsOK())
@@ -231,7 +240,7 @@ func TestHandleAbsentValidator(t *testing.T) {
 	// validator should have been slashed
 	pool = sk.GetPool(ctx)
 	slashAmt := sdk.NewDec(amtInt).Mul(keeper.SlashFractionDowntime(ctx)).RoundInt64()
-	require.Equal(t, amtInt-slashAmt, pool.BondedTokens.RoundInt64())
+	require.Equal(t, amtInt-slashAmt-5, pool.BondedTokens.RoundInt64())
 
 	// validator start height should have been changed
 	info, found = keeper.getValidatorSigningInfo(ctx, sdk.ConsAddress(val.Address()))

x/stake/genesis.go

@@ -16,6 +16,12 @@ import (
 // the bonded validators.
 // Returns final validator set after applying all declaration and delegations
 func InitGenesis(ctx sdk.Context, keeper Keeper, data types.GenesisState) (res []abci.ValidatorUpdate, err error) {
+
+	// We need to pretend to be "one block before genesis" so that e.g. slashing periods
+	// are correctly initialized for the validator set one-block offset - the first TM block
+	// is at height 0, so state updates applied from genesis.json are in block -1.
+	ctx = ctx.WithBlockHeight(-1)
+
 	keeper.SetPool(ctx, data.Pool)
 	keeper.SetParams(ctx, data.Params)
 	keeper.InitIntraTxCounter(ctx)