Merge pull request #3784 from cosmos/revert-3762-bez/3760-allow-unjail-zero-self-bond

Revert "Allow Validator w/ No Self-Delegation to Unjail (Transfers Disabled)"

PENDING.md

@@ -19,9 +19,6 @@
 
 ### 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,7 +133,6 @@ 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 {
-				// increase the supply
+				stakingData.Pool.NotBondedTokens = stakingData.Pool.NotBondedTokens.
-				stakingData.Pool.NotBondedTokens = stakingData.Pool.NotBondedTokens.Add(coin.Amount)
+					Add(coin.Amount) // increase the supply
 			}
 		}
 	}

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.bankKeeper)
+	app.slashingKeeper = slashing.NewKeeper(app.cdc, app.keySlashing, app.stakingKeeper, app.paramsKeeper.Subspace(slashing.DefaultParamspace), slashing.DefaultCodespace)
 
 	// register message routes
 	app.Router().

docs/gaia/validators/validator-setup.md

@@ -84,17 +84,11 @@ __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
+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.
-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 (on behalf of) a delegator's address
+### Case 2: The initial stake comes from a delegator's address
 
-In this case, you need both the signature of the validator and the delegator.
+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`: 
-Start by creating an unsigned `create-validator` transaction, and save it in a
-file called `unsignedValTx`:
 
 ```bash
 gaiacli tx staking create-validator \
@@ -102,11 +96,11 @@ gaiacli tx staking create-validator \
   --pubkey=$(gaiad tendermint show-validator) \
   --moniker="choose a moniker" \
   --chain-id=<chain_id> \
-  --from=<validator_key> \
+  --from=<key_name> \
   --commission-rate="0.10" \
   --commission-max-rate="0.20" \
   --commission-max-change-rate="0.01" \
-  --address-delegator=<delegator_address> \
+  --address-delegator="address of the delegator" \
   --generate-only \
   > unsignedValTx.json
 ```

x/slashing/app_test.go

@@ -33,11 +33,10 @@ 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)
-	mbk := mockBankKeeper{true}
+	keeper := NewKeeper(mapp.Cdc, keySlashing, stakingKeeper, mapp.ParamsKeeper.Subspace(DefaultParamspace), DefaultCodespace)
-	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,23 +25,14 @@ 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()
+	}
 
-	// A validator attempting to unjail may only do so if its self-bond amount
+	if validator.GetDelegatorShareExRate().Mul(selfDel.GetShares()).TruncateInt().LT(validator.GetMinSelfDelegation()) {
-	// is at least their declared min self-delegation. However, during disabled
+		return ErrSelfDelegationTooLowToUnjail(k.codespace).Result()
-	// 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,56 +53,19 @@ 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 jailed validator can't be unjailed due to min self-delegation
+	// assert non-jailed validator can't be unjailed
 	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)
@@ -144,7 +107,7 @@ func TestJailedValidatorDelegations(t *testing.T) {
 	require.True(t, found)
 	require.True(t, validator.GetJailed())
 
-	// verify the validator cannot unjail itself (with transfers enabled)
+	// 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)
 

x/slashing/keeper.go

@@ -17,22 +17,16 @@ 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(
+func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, vs sdk.ValidatorSet, paramspace params.Subspace, codespace sdk.CodespaceType) Keeper {
-	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

@@ -1,9 +0,0 @@
-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,16 +39,6 @@ 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)
@@ -90,15 +80,14 @@ 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{initCoins})
+		_, _, err = ck.AddCoins(ctx, sdk.AccAddress(addr), sdk.Coins{
-		require.Nil(t, err)
+			{sk.GetParams(ctx).BondDenom, initCoins},
+		})
 	}
-
+	require.Nil(t, err)
-	mbk := mockBankKeeper{true}
 	paramstore := paramsKeeper.Subspace(DefaultParamspace)
-	keeper := NewKeeper(cdc, keySlashing, &sk, paramstore, DefaultCodespace, mbk)
+	keeper := NewKeeper(cdc, keySlashing, &sk, paramstore, DefaultCodespace)
 	sk.SetHooks(keeper.Hooks())
 
 	require.NotPanics(t, func() {
@@ -131,17 +120,6 @@ 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)