Merg PR #2198: Ensure Legacy Validator Delegation Invariants

* Test and allow jailed validator to self-bond

* Implement TestJailedValidatorDelegations

* Restructure TestJailedValidatorDelegations

* Add Delegation to Validator type and update handleMsgUnjail accordingly

* Update ErrMissingSelfDelegation error message

* Update democoin mock validator set impl

* Update pending log

* Add comment to ValidatorSet

* Fix conflicts/errors due to develop merge

PENDING.md

@@ -73,6 +73,7 @@ IMPROVEMENTS
 * Gaia
     * [x/stake] [#2023](https://github.com/cosmos/cosmos-sdk/pull/2023) Terminate iteration loop in `UpdateBondedValidators` and `UpdateBondedValidatorsFull` when the first revoked validator is encountered and perform a sanity check.
     * [x/auth] Signature verification's gas cost now accounts for pubkey type. [#2046](https://github.com/tendermint/tendermint/pull/2046)
+    * [x/stake] [x/slashing] Ensure delegation invariants to jailed validators [#1883](https://github.com/cosmos/cosmos-sdk/issues/1883).
 
 * SDK
     * [tools] Make get_vendor_deps deletes `.vendor-new` directories, in case scratch files are present.

examples/democoin/mock/validator.go

@@ -135,3 +135,8 @@ func (vs *ValidatorSet) Jail(ctx sdk.Context, pubkey crypto.PubKey) {
 func (vs *ValidatorSet) Unjail(ctx sdk.Context, pubkey crypto.PubKey) {
 	panic("not implemented")
 }
+
+// Implements sdk.ValidatorSet
+func (vs *ValidatorSet) Delegation(ctx sdk.Context, addrDel sdk.AccAddress, addrVal sdk.ValAddress) sdk.Delegation {
+	panic("not implemented")
+}

types/stake.go

@@ -75,6 +75,10 @@ type ValidatorSet interface {
 	Slash(Context, crypto.PubKey, int64, int64, Dec)
 	Jail(Context, crypto.PubKey)   // jail a validator
 	Unjail(Context, crypto.PubKey) // unjail a validator
+
+	// Delegation allows for getting a particular delegation for a given validator
+	// and delegator outside the scope of the staking module.
+	Delegation(Context, AccAddress, ValAddress) Delegation
 }
 
 //_______________________________________________________________________________

x/slashing/errors.go

@@ -12,20 +12,28 @@ const (
 	// Default slashing codespace
 	DefaultCodespace sdk.CodespaceType = 10
 
-	CodeInvalidValidator   CodeType = 101
-	CodeValidatorJailed    CodeType = 102
-	CodeValidatorNotJailed CodeType = 103
+	CodeInvalidValidator      CodeType = 101
+	CodeValidatorJailed       CodeType = 102
+	CodeValidatorNotJailed    CodeType = 103
+	CodeMissingSelfDelegation CodeType = 104
 )
 
 func ErrNoValidatorForAddress(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidValidator, "that address is not associated with any known validator")
 }
+
 func ErrBadValidatorAddr(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidValidator, "validator does not exist for that address")
 }
+
 func ErrValidatorJailed(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeValidatorJailed, "validator still jailed, cannot yet be unjailed")
 }
+
 func ErrValidatorNotJailed(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeValidatorNotJailed, "validator not jailed, cannot be unjailed")
 }
+
+func ErrMissingSelfDelegation(codespace sdk.CodespaceType) sdk.Error {
+	return sdk.NewError(codespace, CodeMissingSelfDelegation, "validator has no self-delegation; cannot be unjailed")
+}

x/slashing/handler.go

@@ -19,35 +19,38 @@ func NewHandler(k Keeper) sdk.Handler {
 // Validators must submit a transaction to unjail itself after
 // having been jailed (and thus unbonded) for downtime
 func handleMsgUnjail(ctx sdk.Context, msg MsgUnjail, k Keeper) sdk.Result {
-
-	// Validator must exist
 	validator := k.validatorSet.Validator(ctx, msg.ValidatorAddr)
 	if validator == nil {
 		return ErrNoValidatorForAddress(k.codespace).Result()
 	}
 
+	// cannot be unjailed if no self-delegation exists
+	selfDel := k.validatorSet.Delegation(ctx, sdk.AccAddress(msg.ValidatorAddr), msg.ValidatorAddr)
+	if selfDel == nil {
+		return ErrMissingSelfDelegation(k.codespace).Result()
+	}
+
 	if !validator.GetJailed() {
 		return ErrValidatorNotJailed(k.codespace).Result()
 	}
 
 	addr := sdk.ValAddress(validator.GetPubKey().Address())
 
-	// Signing info must exist
 	info, found := k.getValidatorSigningInfo(ctx, addr)
 	if !found {
 		return ErrNoValidatorForAddress(k.codespace).Result()
 	}
 
-	// Cannot be unjailed until out of jail
+	// cannot be unjailed until out of jail
 	if ctx.BlockHeader().Time.Before(info.JailedUntil) {
 		return ErrValidatorJailed(k.codespace).Result()
 	}
 
-	// Update the starting height (so the validator can't be immediately jailed again)
+	// update the starting height so the validator can't be immediately jailed
+	// again
 	info.StartHeight = ctx.BlockHeight()
 	k.setValidatorSigningInfo(ctx, addr, info)
 
-	// Unjail the validator
 	k.validatorSet.Unjail(ctx, validator.GetPubKey())
 
 	tags := sdk.NewTags("action", []byte("unjail"), "validator", []byte(msg.ValidatorAddr.String()))

x/slashing/handler_test.go

@@ -2,6 +2,7 @@ package slashing
 
 import (
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 
@@ -27,3 +28,62 @@ func TestCannotUnjailUnlessJailed(t *testing.T) {
 	require.False(t, got.IsOK(), "allowed unjail of non-jailed validator")
 	require.Equal(t, sdk.ToABCICode(DefaultCodespace, CodeValidatorNotJailed), got.Code)
 }
+
+func TestJailedValidatorDelegations(t *testing.T) {
+	ctx, _, stakeKeeper, _, slashingKeeper := createTestInput(t)
+
+	stakeParams := stakeKeeper.GetParams(ctx)
+	stakeParams.UnbondingTime = 0
+	stakeKeeper.SetParams(ctx, stakeParams)
+
+	// create a validator
+	amount := int64(10)
+	valAddr, valPubKey, bondAmount := sdk.ValAddress(addrs[0]), pks[0], sdk.NewInt(amount)
+	msgCreateVal := newTestMsgCreateValidator(valAddr, valPubKey, bondAmount)
+	got := stake.NewHandler(stakeKeeper)(ctx, msgCreateVal)
+	require.True(t, got.IsOK(), "expected create validator msg to be ok, got: %v", got)
+
+	// set dummy signing info
+	newInfo := ValidatorSigningInfo{
+		StartHeight:         int64(0),
+		IndexOffset:         int64(0),
+		JailedUntil:         time.Unix(0, 0),
+		SignedBlocksCounter: int64(0),
+	}
+	slashingKeeper.setValidatorSigningInfo(ctx, valAddr, newInfo)
+
+	// delegate tokens to the validator
+	delAddr := addrs[1]
+	msgDelegate := newTestMsgDelegate(delAddr, valAddr, bondAmount)
+	got = stake.NewHandler(stakeKeeper)(ctx, msgDelegate)
+	require.True(t, got.IsOK(), "expected delegation to be ok, got %v", got)
+
+	unbondShares := sdk.NewDec(10)
+
+	// unbond validator total self-delegations (which should jail the validator)
+	msgBeginUnbonding := stake.NewMsgBeginUnbonding(sdk.AccAddress(valAddr), valAddr, unbondShares)
+	got = stake.NewHandler(stakeKeeper)(ctx, msgBeginUnbonding)
+	require.True(t, got.IsOK(), "expected begin unbonding validator msg to be ok, got: %v", got)
+
+	msgCompleteUnbonding := stake.NewMsgCompleteUnbonding(sdk.AccAddress(valAddr), valAddr)
+	got = stake.NewHandler(stakeKeeper)(ctx, msgCompleteUnbonding)
+	require.True(t, got.IsOK(), "expected complete unbonding validator msg to be ok, got: %v", got)
+
+	// verify validator still exists and is jailed
+	validator, found := stakeKeeper.GetValidator(ctx, valAddr)
+	require.True(t, found)
+	require.True(t, validator.GetJailed())
+
+	// verify the validator cannot unjail itself
+	got = NewHandler(slashingKeeper)(ctx, NewMsgUnjail(valAddr))
+	require.False(t, got.IsOK(), "expected jailed validator to not be able to unjail, got: %v", got)
+
+	// self-delegate to validator
+	msgSelfDelegate := newTestMsgDelegate(sdk.AccAddress(valAddr), valAddr, bondAmount)
+	got = stake.NewHandler(stakeKeeper)(ctx, msgSelfDelegate)
+	require.True(t, got.IsOK(), "expected delegation to not be ok, got %v", got)
+
+	// verify the validator can now unjail itself
+	got = NewHandler(slashingKeeper)(ctx, NewMsgUnjail(valAddr))
+	require.True(t, got.IsOK(), "expected jailed validator to be able to unjail, got: %v", got)
+}

x/slashing/test_common.go

@@ -4,6 +4,7 @@ import (
 	"encoding/hex"
 	"os"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 
@@ -61,7 +62,7 @@ func createTestInput(t *testing.T) (sdk.Context, bank.Keeper, stake.Keeper, para
 	ms.MountStoreWithDB(keyParams, sdk.StoreTypeIAVL, db)
 	err := ms.LoadLatestVersion()
 	require.Nil(t, err)
-	ctx := sdk.NewContext(ms, abci.Header{}, false, log.NewTMLogger(os.Stdout))
+	ctx := sdk.NewContext(ms, abci.Header{Time: time.Unix(0, 0)}, false, log.NewTMLogger(os.Stdout))
 	cdc := createTestCodec()
 	accountMapper := auth.NewAccountMapper(cdc, keyAcc, auth.ProtoBaseAccount)
 	ck := bank.NewKeeper(accountMapper)
@@ -108,3 +109,11 @@ func newTestMsgCreateValidator(address sdk.ValAddress, pubKey crypto.PubKey, amt
 		Delegation:    sdk.Coin{"steak", amt},
 	}
 }
+
+func newTestMsgDelegate(delAddr sdk.AccAddress, valAddr sdk.ValAddress, delAmount sdk.Int) stake.MsgDelegate {
+	return stake.MsgDelegate{
+		DelegatorAddr: delAddr,
+		ValidatorAddr: valAddr,
+		Delegation:    sdk.Coin{"steak", delAmount},
+	}
+}

x/stake/handler.go

@@ -1,6 +1,7 @@
 package stake
 
 import (
+	"bytes"
 	"time"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -128,17 +129,19 @@ func handleMsgEditValidator(ctx sdk.Context, msg types.MsgEditValidator, k keepe
 }
 
 func handleMsgDelegate(ctx sdk.Context, msg types.MsgDelegate, k keeper.Keeper) sdk.Result {
-
 	validator, found := k.GetValidator(ctx, msg.ValidatorAddr)
 	if !found {
 		return ErrNoValidatorFound(k.Codespace()).Result()
 	}
+
 	if msg.Delegation.Denom != k.GetParams(ctx).BondDenom {
 		return ErrBadDenom(k.Codespace()).Result()
 	}
-	if validator.Jailed {
+
+	if validator.Jailed && !bytes.Equal(validator.Operator, msg.DelegatorAddr) {
 		return ErrValidatorJailed(k.Codespace()).Result()
 	}
+
 	_, err := k.Delegate(ctx, msg.DelegatorAddr, msg.Delegation, validator, true)
 	if err != nil {
 		return err.Result()
@@ -149,6 +152,7 @@ func handleMsgDelegate(ctx sdk.Context, msg types.MsgDelegate, k keeper.Keeper)
 		tags.Delegator, []byte(msg.DelegatorAddr.String()),
 		tags.DstValidator, []byte(msg.ValidatorAddr.String()),
 	)
+
 	return sdk.Result{
 		Tags: tags,
 	}

x/stake/handler_test.go

@@ -193,6 +193,97 @@ func TestDuplicatesMsgCreateValidatorOnBehalfOf(t *testing.T) {
 	require.False(t, got.IsOK(), "%v", got)
 }
 
+func TestLegacyValidatorDelegations(t *testing.T) {
+	ctx, _, keeper := keep.CreateTestInput(t, false, int64(1000))
+	setInstantUnbondPeriod(keeper, ctx)
+
+	bondAmount := int64(10)
+	valAddr, valPubKey := sdk.ValAddress(keep.Addrs[0]), keep.PKs[0]
+	delAddr := keep.Addrs[1]
+
+	// create validator
+	msgCreateVal := newTestMsgCreateValidator(valAddr, valPubKey, bondAmount)
+	got := handleMsgCreateValidator(ctx, msgCreateVal, keeper)
+	require.True(t, got.IsOK(), "expected create validator msg to be ok, got %v", got)
+
+	// verify the validator exists and has the correct attributes
+	validator, found := keeper.GetValidator(ctx, valAddr)
+	require.True(t, found)
+	require.Equal(t, sdk.Bonded, validator.Status)
+	require.Equal(t, bondAmount, validator.DelegatorShares.RoundInt64())
+	require.Equal(t, bondAmount, validator.BondedTokens().RoundInt64())
+
+	// delegate tokens to the validator
+	msgDelegate := newTestMsgDelegate(delAddr, valAddr, bondAmount)
+	got = handleMsgDelegate(ctx, msgDelegate, keeper)
+	require.True(t, got.IsOK(), "expected delegation to be ok, got %v", got)
+
+	// verify validator bonded shares
+	validator, found = keeper.GetValidator(ctx, valAddr)
+	require.True(t, found)
+	require.Equal(t, bondAmount*2, validator.DelegatorShares.RoundInt64())
+	require.Equal(t, bondAmount*2, validator.BondedTokens().RoundInt64())
+
+	// unbond validator total self-delegations (which should jail the validator)
+	unbondShares := sdk.NewDec(10)
+	msgBeginUnbonding := NewMsgBeginUnbonding(sdk.AccAddress(valAddr), valAddr, unbondShares)
+	msgCompleteUnbonding := NewMsgCompleteUnbonding(sdk.AccAddress(valAddr), valAddr)
+
+	got = handleMsgBeginUnbonding(ctx, msgBeginUnbonding, keeper)
+	require.True(t, got.IsOK(), "expected begin unbonding validator msg to be ok, got %v", got)
+
+	got = handleMsgCompleteUnbonding(ctx, msgCompleteUnbonding, keeper)
+	require.True(t, got.IsOK(), "expected complete unbonding validator msg to be ok, got %v", got)
+
+	// verify the validator record still exists, is jailed, and has correct tokens
+	validator, found = keeper.GetValidator(ctx, valAddr)
+	require.True(t, found)
+	require.True(t, validator.Jailed)
+	require.Equal(t, sdk.NewDec(10), validator.Tokens)
+
+	// verify delegation still exists
+	bond, found := keeper.GetDelegation(ctx, delAddr, valAddr)
+	require.True(t, found)
+	require.Equal(t, bondAmount, bond.Shares.RoundInt64())
+	require.Equal(t, bondAmount, validator.DelegatorShares.RoundInt64())
+
+	// verify a delegator cannot create a new delegation to the now jailed validator
+	msgDelegate = newTestMsgDelegate(delAddr, valAddr, bondAmount)
+	got = handleMsgDelegate(ctx, msgDelegate, keeper)
+	require.False(t, got.IsOK(), "expected delegation to not be ok, got %v", got)
+
+	// verify the validator can still self-delegate
+	msgSelfDelegate := newTestMsgDelegate(sdk.AccAddress(valAddr), valAddr, bondAmount)
+	got = handleMsgDelegate(ctx, msgSelfDelegate, keeper)
+	require.True(t, got.IsOK(), "expected delegation to not be ok, got %v", got)
+
+	// verify validator bonded shares
+	validator, found = keeper.GetValidator(ctx, valAddr)
+	require.True(t, found)
+	require.Equal(t, bondAmount*2, validator.DelegatorShares.RoundInt64())
+	require.Equal(t, bondAmount*2, validator.Tokens.RoundInt64())
+
+	// unjail the validator now that is has non-zero self-delegated shares
+	keeper.Unjail(ctx, valPubKey)
+
+	// verify the validator can now accept delegations
+	msgDelegate = newTestMsgDelegate(delAddr, valAddr, bondAmount)
+	got = handleMsgDelegate(ctx, msgDelegate, keeper)
+	require.True(t, got.IsOK(), "expected delegation to be ok, got %v", got)
+
+	// verify validator bonded shares
+	validator, found = keeper.GetValidator(ctx, valAddr)
+	require.True(t, found)
+	require.Equal(t, bondAmount*3, validator.DelegatorShares.RoundInt64())
+	require.Equal(t, bondAmount*3, validator.Tokens.RoundInt64())
+
+	// verify new delegation
+	bond, found = keeper.GetDelegation(ctx, delAddr, valAddr)
+	require.True(t, found)
+	require.Equal(t, bondAmount*2, bond.Shares.RoundInt64())
+	require.Equal(t, bondAmount*3, validator.DelegatorShares.RoundInt64())
+}
+
 func TestIncrementsMsgDelegate(t *testing.T) {
 	initBond := int64(1000)
 	ctx, accMapper, keeper := keep.CreateTestInput(t, false, initBond)

x/stake/keeper/delegation.go

@@ -288,6 +288,7 @@ func (k Keeper) unbond(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValA
 		if bytes.Equal(delegation.DelegatorAddr, validator.Operator) && validator.Jailed == false {
 			validator.Jailed = true
 		}
+
 		k.RemoveDelegation(ctx, delegation)
 	} else {
 		// Update height
@@ -307,7 +308,7 @@ func (k Keeper) unbond(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValA
 		k.RemoveValidator(ctx, validator.Operator)
 	}
 
-	return
+	return amount, nil
 }
 
 //______________________________________________________________________________________________________

x/stake/keeper/sdk_types.go

@@ -91,6 +91,7 @@ func (k Keeper) Delegation(ctx sdk.Context, addrDel sdk.AccAddress, addrVal sdk.
 	if !ok {
 		return nil
 	}
+
 	return bond
 }