Merge PR #1514: Unbond validators set to zero power

CHANGELOG.md

@@ -100,6 +100,7 @@ IMPROVEMENTS
 
 BUG FIXES
 * [x/slashing] \#1510 Unrevoked validators cannot un-revoke themselves
+* [x/stake] \#1513 Validators slashed to zero power are unbonded and removed from the store
 * [x/stake] \#1567 Validators decreased in power but not unbonded are now updated in Tendermint
 * [gaia] Added self delegation for validators in the genesis creation
 * [lcd] tests now don't depend on raw json text

x/stake/handler_test.go

@@ -638,7 +638,7 @@ func TestBondUnbondRedelegateSlashTwice(t *testing.T) {
 	require.Equal(t, sdk.NewRat(3), delegation.Shares)
 
 	// validator power should have been reduced to zero
-	validator, found = keeper.GetValidator(ctx, valA)
-	require.True(t, found)
-	require.Equal(t, sdk.NewRat(0), validator.GetPower())
+	// ergo validator should have been removed from the store
+	_, found = keeper.GetValidator(ctx, valA)
+	require.False(t, found)
 }

x/stake/keeper/slash.go

@@ -15,8 +15,6 @@ import (
 // CONTRACT:
 //    slashFactor is non-negative
 // CONTRACT:
-//    Validator exists and can be looked up by public key
-// CONTRACT:
 //    Infraction committed equal to or less than an unbonding period in the past,
 //    so all unbonding delegations and redelegations from that height are stored
 // CONTRACT:
@@ -36,7 +34,12 @@ func (k Keeper) Slash(ctx sdk.Context, pubkey crypto.PubKey, infractionHeight in
 
 	validator, found := k.GetValidatorByPubKey(ctx, pubkey)
 	if !found {
-		panic(fmt.Errorf("attempted to slash a nonexistent validator with address %s", pubkey.Address()))
+		// If not found, the validator must have been overslashed and removed - so we don't need to do anything
+		// NOTE:  Correctness dependent on invariant that unbonding delegations / redelegations must also have been completely
+		//        slashed in this case - which we don't explicitly check, but should be true.
+		// Log the slash attempt for future reference (maybe we should tag it too)
+		logger.Error(fmt.Sprintf("WARNING: Ignored attempt to slash a nonexistent validator with address %s, we recommend you investigate immediately", pubkey.Address()))
+		return
 	}
 	ownerAddress := validator.GetOwner()
 
@@ -92,7 +95,11 @@ func (k Keeper) Slash(ctx sdk.Context, pubkey crypto.PubKey, infractionHeight in
 	// update the pool
 	k.SetPool(ctx, pool)
 	// update the validator, possibly kicking it out
-	k.UpdateValidator(ctx, validator)
+	validator = k.UpdateValidator(ctx, validator)
+	// remove validator if it has been reduced to zero shares
+	if validator.PoolShares.Amount.IsZero() {
+		k.RemoveValidator(ctx, validator.Owner)
+	}
 
 	// Log that a slash occurred!
 	logger.Info(fmt.Sprintf("Validator %s slashed by slashFactor %v, removed %v shares and burned %d tokens", pubkey.Address(), slashFactor, sharesToRemove, burned))

x/stake/keeper/slash_test.go

@@ -291,10 +291,10 @@ func TestSlashWithUnbondingDelegation(t *testing.T) {
 	// just 1 bonded token burned again since that's all the validator now has
 	require.Equal(t, int64(10), oldPool.BondedTokens-newPool.BondedTokens)
 	// read updated validator
-	validator, found = keeper.GetValidatorByPubKey(ctx, pk)
-	require.True(t, found)
 	// power decreased by 1 again, validator is out of stake
-	require.Equal(t, sdk.NewRat(0), validator.GetPower())
+	// ergo validator should have been removed from the store
+	_, found = keeper.GetValidatorByPubKey(ctx, pk)
+	require.False(t, found)
 }
 
 // tests Slash at a previous height with a redelegation
@@ -387,16 +387,16 @@ func TestSlashWithRedelegation(t *testing.T) {
 	// four more bonded tokens burned
 	require.Equal(t, int64(16), oldPool.BondedTokens-newPool.BondedTokens)
 	// read updated validator
-	validator, found = keeper.GetValidatorByPubKey(ctx, pk)
-	require.True(t, found)
-	// power decreased by 4, down to 0
-	require.Equal(t, sdk.NewRat(0), validator.GetPower())
+	// validator decreased to zero power, should have been removed from the store
+	_, found = keeper.GetValidatorByPubKey(ctx, pk)
+	require.False(t, found)
 
 	// slash the validator again, by 100%
 	// no stake remains to be slashed
 	ctx = ctx.WithBlockHeight(12)
-	validator, found = keeper.GetValidatorByPubKey(ctx, pk)
-	require.True(t, found)
+	// validator no longer in the store
+	_, found = keeper.GetValidatorByPubKey(ctx, pk)
+	require.False(t, found)
 	keeper.Slash(ctx, pk, 10, 10, sdk.OneRat())
 
 	// read updating redelegation
@@ -409,10 +409,9 @@ func TestSlashWithRedelegation(t *testing.T) {
 	// no more bonded tokens burned
 	require.Equal(t, int64(16), oldPool.BondedTokens-newPool.BondedTokens)
 	// read updated validator
-	validator, found = keeper.GetValidatorByPubKey(ctx, pk)
-	require.True(t, found)
-	// power still zero
-	require.Equal(t, sdk.NewRat(0), validator.GetPower())
+	// power still zero, still not in the store
+	_, found = keeper.GetValidatorByPubKey(ctx, pk)
+	require.False(t, found)
 }
 
 // tests Slash at a previous height with both an unbonding delegation and a redelegation

x/stake/keeper/validator_test.go

@@ -93,6 +93,28 @@ func TestUpdateValidatorByPowerIndex(t *testing.T) {
 	require.True(t, keeper.validatorByPowerIndexExists(ctx, power))
 }
 
+func TestSlashToZeroPowerRemoved(t *testing.T) {
+	// initialize setup
+	ctx, _, keeper := CreateTestInput(t, false, 100)
+	pool := keeper.GetPool(ctx)
+
+	// add a validator
+	validator := types.NewValidator(addrVals[0], PKs[0], types.Description{})
+	validator, pool, _ = validator.AddTokensFromDel(pool, 100)
+	require.Equal(t, sdk.Unbonded, validator.Status())
+	require.Equal(t, int64(100), validator.PoolShares.Tokens(pool).RoundInt64())
+	keeper.SetPool(ctx, pool)
+	keeper.SetValidatorByPubKeyIndex(ctx, validator)
+	validator = keeper.UpdateValidator(ctx, validator)
+	require.Equal(t, int64(100), validator.PoolShares.Tokens(pool).RoundInt64(), "\nvalidator %v\npool %v", validator, pool)
+
+	// slash the validator by 100%
+	keeper.Slash(ctx, PKs[0], 0, 100, sdk.OneRat())
+	// validator should have been deleted
+	_, found := keeper.GetValidator(ctx, addrVals[0])
+	require.False(t, found)
+}
+
 // This function tests UpdateValidator, GetValidator, GetValidatorsBonded, RemoveValidator
 func TestValidatorBasics(t *testing.T) {
 	ctx, _, keeper := CreateTestInput(t, false, 1000)