Merge PR #3762: Allow Validator w/ No Self-Delegation to Unjail (Transfers Disabled)

PENDING.md

@@ -19,6 +19,9 @@
 
 ### Gaia
 
+* [\#3760] Allow unjailing when a validator has no self-delegation and during
+disabled transfers.
+
 ### SDK
 
 * [\#3669] Ensure consistency in message naming, codec registration, and JSON

cmd/gaia/app/app.go

@@ -133,6 +133,7 @@ func NewGaiaApp(logger log.Logger, db dbm.DB, traceStore io.Writer, loadLatest b
 		app.keySlashing,
 		&stakingKeeper, app.paramsKeeper.Subspace(slashing.DefaultParamspace),
 		slashing.DefaultCodespace,
+		app.bankKeeper,
 	)
 	app.govKeeper = gov.NewKeeper(
 		app.cdc,

cmd/gaia/app/genesis.go

@@ -188,8 +188,8 @@ func GaiaAppGenState(cdc *codec.Codec, genDoc tmtypes.GenesisDoc, appGenTxs []js
 	for _, acc := range genesisState.Accounts {
 		for _, coin := range acc.Coins {
 			if coin.Denom == genesisState.StakingData.Params.BondDenom {
-				stakingData.Pool.NotBondedTokens = stakingData.Pool.NotBondedTokens.
-					Add(coin.Amount) // increase the supply
+				// increase the supply
+				stakingData.Pool.NotBondedTokens = stakingData.Pool.NotBondedTokens.Add(coin.Amount)
 			}
 		}
 	}

cmd/gaia/cmd/gaiadebug/hack.go

@@ -180,7 +180,7 @@ func NewGaiaApp(logger log.Logger, db dbm.DB, baseAppOptions ...func(*bam.BaseAp
 	// add handlers
 	app.bankKeeper = bank.NewBaseKeeper(app.accountKeeper, app.paramsKeeper.Subspace(bank.DefaultParamspace), bank.DefaultCodespace)
 	app.stakingKeeper = staking.NewKeeper(app.cdc, app.keyStaking, app.tkeyStaking, app.bankKeeper, app.paramsKeeper.Subspace(staking.DefaultParamspace), staking.DefaultCodespace)
-	app.slashingKeeper = slashing.NewKeeper(app.cdc, app.keySlashing, app.stakingKeeper, app.paramsKeeper.Subspace(slashing.DefaultParamspace), slashing.DefaultCodespace)
+	app.slashingKeeper = slashing.NewKeeper(app.cdc, app.keySlashing, app.stakingKeeper, app.paramsKeeper.Subspace(slashing.DefaultParamspace), slashing.DefaultCodespace, app.bankKeeper)
 
 	// register message routes
 	app.Router().

docs/gaia/validators/validator-setup.md

@@ -84,11 +84,17 @@ __Note__: This command automatically store your `gentx` in `~/.gaiad/config/gent
 Consult `gaiad gentx --help` for more information on the flags defaults.
 :::
 
-A `gentx` is a JSON file carrying a self-delegation. All genesis transactions are collected by a `genesis coordinator` and validated against an initial `genesis.json`. Such initial `genesis.json` contains only a list of accounts and their coins. Once the transactions are processed, they are merged in the `genesis.json`'s `gentxs` field.
+A `gentx` is a JSON file carrying a self-delegation. All genesis transactions are
+collected by a `genesis coordinator` and validated against an initial `genesis.json`.
+Such an initial `genesis.json` contains only a list of accounts and their coins.
+Once the transactions are processed, they are merged in the `genesis.json`'s
+`gentxs` field.
 
-### Case 2: The initial stake comes from a delegator's address
+### Case 2: The initial stake comes from (on behalf of) a delegator's address
 
-In this case, you need both the signature of the validator and the delegator. Start by creating an unsigned `create-validator` transaction, and save it in a file called `unsignedValTx`: 
+In this case, you need both the signature of the validator and the delegator.
+Start by creating an unsigned `create-validator` transaction, and save it in a
+file called `unsignedValTx`:
 
 ```bash
 gaiacli tx staking create-validator \
@@ -96,11 +102,11 @@ gaiacli tx staking create-validator \
   --pubkey=$(gaiad tendermint show-validator) \
   --moniker="choose a moniker" \
   --chain-id=<chain_id> \
-  --from=<key_name> \
+  --from=<validator_key> \
   --commission-rate="0.10" \
   --commission-max-rate="0.20" \
   --commission-max-change-rate="0.01" \
-  --address-delegator="address of the delegator" \
+  --address-delegator=<delegator_address> \
   --generate-only \
   > unsignedValTx.json
 ```

x/slashing/app_test.go

@@ -33,10 +33,11 @@ func getMockApp(t *testing.T) (*mock.App, staking.Keeper, Keeper) {
 
 	bankKeeper := bank.NewBaseKeeper(mapp.AccountKeeper, mapp.ParamsKeeper.Subspace(bank.DefaultParamspace), bank.DefaultCodespace)
 	stakingKeeper := staking.NewKeeper(mapp.Cdc, keyStaking, tkeyStaking, bankKeeper, mapp.ParamsKeeper.Subspace(staking.DefaultParamspace), staking.DefaultCodespace)
-	keeper := NewKeeper(mapp.Cdc, keySlashing, stakingKeeper, mapp.ParamsKeeper.Subspace(DefaultParamspace), DefaultCodespace)
+	mbk := mockBankKeeper{true}
+	keeper := NewKeeper(mapp.Cdc, keySlashing, stakingKeeper, mapp.ParamsKeeper.Subspace(DefaultParamspace), DefaultCodespace, mbk)
+
 	mapp.Router().AddRoute(staking.RouterKey, staking.NewHandler(stakingKeeper))
 	mapp.Router().AddRoute(RouterKey, NewHandler(keeper))
-
 	mapp.SetEndBlocker(getEndBlocker(stakingKeeper))
 	mapp.SetInitChainer(getInitChainer(mapp, stakingKeeper))
 

x/slashing/handler.go

@@ -25,14 +25,23 @@ func handleMsgUnjail(ctx sdk.Context, msg MsgUnjail, k Keeper) sdk.Result {
 		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.GetDelegatorShareExRate().Mul(selfDel.GetShares()).TruncateInt().LT(validator.GetMinSelfDelegation()) {
-		return ErrSelfDelegationTooLowToUnjail(k.codespace).Result()
+	// A validator attempting to unjail may only do so if its self-bond amount
+	// is at least their declared min self-delegation. However, during disabled
+	// transfers, we allow validators to unjail if they have no self-delegation.
+	// This is to allow newly created validators during a time when transfers are
+	// disabled to successfully unjail.
+	if k.bk.GetSendEnabled(ctx) {
+		if selfDel == nil {
+			// cannot be unjailed if no self-delegation exists
+			return ErrMissingSelfDelegation(k.codespace).Result()
+		}
+
+		valSelfBond := validator.GetDelegatorShareExRate().Mul(selfDel.GetShares()).TruncateInt()
+		if valSelfBond.LT(validator.GetMinSelfDelegation()) {
+			return ErrSelfDelegationTooLowToUnjail(k.codespace).Result()
+		}
 	}
 
 	// cannot be unjailed if not jailed

x/slashing/handler_test.go

@@ -53,19 +53,56 @@ func TestCannotUnjailUnlessMeetMinSelfDelegation(t *testing.T) {
 
 	undelegateMsg := staking.NewMsgUndelegate(sdk.AccAddress(addr), addr, sdk.OneDec())
 	got = staking.NewHandler(sk)(ctx, undelegateMsg)
-
+	require.True(t, got.IsOK())
 	require.True(t, sk.Validator(ctx, addr).GetJailed())
 
-	// assert non-jailed validator can't be unjailed
+	// assert jailed validator can't be unjailed due to min self-delegation
 	got = slh(ctx, NewMsgUnjail(addr))
 	require.False(t, got.IsOK(), "allowed unjail of validator with less than MinSelfDelegation")
 	require.EqualValues(t, CodeValidatorNotJailed, got.Code)
 	require.EqualValues(t, DefaultCodespace, got.Codespace)
 }
 
+func TestUnjailNoTransferValidator(t *testing.T) {
+	// initial setup
+	ctx, bk, sk, _, keeper := createTestInput(t, DefaultParams())
+	keeper.bk = mockBankKeeper{false}
+
+	slh := NewHandler(keeper)
+	amtInt := int64(100)
+	delAddr := sdk.AccAddress(addrs[0])
+	valAddr, pubKey, amt := addrs[1], pks[1], sdk.TokensFromTendermintPower(amtInt)
+
+	msg := NewTestMsgCreateValidatorOnBehalfOf(delAddr, valAddr, pubKey, amt, amt)
+	got := staking.NewHandler(sk)(ctx, msg)
+	require.True(t, got.IsOK())
+
+	staking.EndBlocker(ctx, sk)
+	require.Equal(
+		t, bk.GetCoins(ctx, delAddr),
+		sdk.Coins{sdk.NewCoin(sk.GetParams(ctx).BondDenom, initCoins.Sub(amt))},
+	)
+	require.Equal(
+		t, bk.GetCoins(ctx, sdk.AccAddress(valAddr)),
+		sdk.Coins{sdk.NewCoin(sk.GetParams(ctx).BondDenom, initCoins)},
+	)
+
+	sk.Jail(ctx, sdk.ConsAddress(valAddr))
+
+	val := sk.Validator(ctx, valAddr)
+	selfDel := keeper.validatorSet.Delegation(ctx, sdk.AccAddress(valAddr), valAddr)
+	require.Nil(t, selfDel)
+	require.True(t, val.GetJailed())
+
+	got = slh(ctx, NewMsgUnjail(valAddr))
+	require.True(t, got.IsOK(), "expected validator to be unjailed")
+
+	val = sk.Validator(ctx, valAddr)
+	require.False(t, val.GetJailed())
+}
+
 func TestJailedValidatorDelegations(t *testing.T) {
 	ctx, _, stakingKeeper, _, slashingKeeper := createTestInput(t, DefaultParams())
-
 	stakingParams := stakingKeeper.GetParams(ctx)
 	stakingParams.UnbondingTime = 0
 	stakingKeeper.SetParams(ctx, stakingParams)
@@ -107,7 +144,7 @@ func TestJailedValidatorDelegations(t *testing.T) {
 	require.True(t, found)
 	require.True(t, validator.GetJailed())
 
-	// verify the validator cannot unjail itself
+	// verify the validator cannot unjail itself (with transfers enabled)
 	got = NewHandler(slashingKeeper)(ctx, NewMsgUnjail(valAddr))
 	require.False(t, got.IsOK(), "expected jailed validator to not be able to unjail, got: %v", got)
 

x/slashing/keeper.go

@@ -17,16 +17,22 @@ type Keeper struct {
 	cdc          *codec.Codec
 	validatorSet sdk.ValidatorSet
 	paramspace   params.Subspace
+	bk           BankKeeper
 
 	// codespace
 	codespace sdk.CodespaceType
 }
 
 // NewKeeper creates a slashing keeper
-func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, vs sdk.ValidatorSet, paramspace params.Subspace, codespace sdk.CodespaceType) Keeper {
+func NewKeeper(
+	cdc *codec.Codec, key sdk.StoreKey, vs sdk.ValidatorSet,
+	paramspace params.Subspace, codespace sdk.CodespaceType, bk BankKeeper,
+) Keeper {
+
 	keeper := Keeper{
 		storeKey:     key,
 		cdc:          cdc,
+		bk:           bk,
 		validatorSet: vs,
 		paramspace:   paramspace.WithKeyTable(ParamKeyTable()),
 		codespace:    codespace,

x/slashing/keeper_contracts.go

@@ -0,0 +1,9 @@
+package slashing
+
+import sdk "github.com/cosmos/cosmos-sdk/types"
+
+// BankKeeper defines the bank keeper interfact contract the slashing module
+// requires. It is needed in order to determine if transfers are enabled.
+type BankKeeper interface {
+	GetSendEnabled(ctx sdk.Context) bool
+}

x/slashing/test_common.go

@@ -39,6 +39,16 @@ var (
 	initCoins = sdk.TokensFromTendermintPower(200)
 )
 
+var _ BankKeeper = (*mockBankKeeper)(nil)
+
+type mockBankKeeper struct {
+	sendEnabled bool
+}
+
+func (mbk mockBankKeeper) GetSendEnabled(_ sdk.Context) bool {
+	return mbk.sendEnabled
+}
+
 func createTestCodec() *codec.Codec {
 	cdc := codec.New()
 	sdk.RegisterCodec(cdc)
@@ -80,14 +90,15 @@ func createTestInput(t *testing.T, defaults Params) (sdk.Context, bank.Keeper, s
 	_, err = staking.InitGenesis(ctx, sk, genesis)
 	require.Nil(t, err)
 
+	initCoins := sdk.NewCoin(sk.GetParams(ctx).BondDenom, initCoins)
 	for _, addr := range addrs {
-		_, _, err = ck.AddCoins(ctx, sdk.AccAddress(addr), sdk.Coins{
-			{sk.GetParams(ctx).BondDenom, initCoins},
-		})
+		_, _, err = ck.AddCoins(ctx, sdk.AccAddress(addr), sdk.Coins{initCoins})
+		require.Nil(t, err)
 	}
-	require.Nil(t, err)
+
+	mbk := mockBankKeeper{true}
 	paramstore := paramsKeeper.Subspace(DefaultParamspace)
-	keeper := NewKeeper(cdc, keySlashing, &sk, paramstore, DefaultCodespace)
+	keeper := NewKeeper(cdc, keySlashing, &sk, paramstore, DefaultCodespace, mbk)
 	sk.SetHooks(keeper.Hooks())
 
 	require.NotPanics(t, func() {
@@ -120,6 +131,17 @@ func NewTestMsgCreateValidator(address sdk.ValAddress, pubKey crypto.PubKey, amt
 	)
 }
 
+func NewTestMsgCreateValidatorOnBehalfOf(
+	delAddr sdk.AccAddress, valAddr sdk.ValAddress, pubKey crypto.PubKey, amt, msb sdk.Int,
+) staking.MsgCreateValidator {
+
+	commission := staking.NewCommissionMsg(sdk.ZeroDec(), sdk.ZeroDec(), sdk.ZeroDec())
+	return staking.NewMsgCreateValidatorOnBehalfOf(
+		delAddr, valAddr, pubKey, sdk.NewCoin(sdk.DefaultBondDenom, amt),
+		staking.Description{}, commission, msb,
+	)
+}
+
 func newTestMsgDelegate(delAddr sdk.AccAddress, valAddr sdk.ValAddress, delAmount sdk.Int) staking.MsgDelegate {
 	amount := sdk.NewCoin(sdk.DefaultBondDenom, delAmount)
 	return staking.NewMsgDelegate(delAddr, valAddr, amount)