From 52bf9ef9fde809118089d625d0308a7b79dfd43a Mon Sep 17 00:00:00 2001 From: frog power 4000 Date: Fri, 1 Mar 2019 19:04:37 -0500 Subject: [PATCH] Revert "Merge PR #3762: Allow Validator w/ No Self-Delegation to Unjail (Transfers Disabled)" This reverts commit c2aecb8b0ef439f5861458d6d29d5f7e10bd44f3. --- PENDING.md | 3 -- cmd/gaia/app/app.go | 1 - cmd/gaia/app/genesis.go | 4 +-- cmd/gaia/cmd/gaiadebug/hack.go | 2 +- docs/gaia/validators/validator-setup.md | 16 +++------ x/slashing/app_test.go | 5 ++- x/slashing/handler.go | 21 ++++-------- x/slashing/handler_test.go | 45 +++---------------------- x/slashing/keeper.go | 8 +---- x/slashing/keeper_contracts.go | 9 ----- x/slashing/test_common.go | 32 +++--------------- 11 files changed, 26 insertions(+), 120 deletions(-) delete mode 100644 x/slashing/keeper_contracts.go diff --git a/PENDING.md b/PENDING.md index b1e9760b1..c6e5fbde1 100644 --- a/PENDING.md +++ b/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 diff --git a/cmd/gaia/app/app.go b/cmd/gaia/app/app.go index 3efc36fd8..d5b031272 100644 --- a/cmd/gaia/app/app.go +++ b/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, diff --git a/cmd/gaia/app/genesis.go b/cmd/gaia/app/genesis.go index ba48769b4..74e396962 100644 --- a/cmd/gaia/app/genesis.go +++ b/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.Add(coin.Amount) + stakingData.Pool.NotBondedTokens = stakingData.Pool.NotBondedTokens. + Add(coin.Amount) // increase the supply } } } diff --git a/cmd/gaia/cmd/gaiadebug/hack.go b/cmd/gaia/cmd/gaiadebug/hack.go index 9a0dc1d63..e7a89cea7 100644 --- a/cmd/gaia/cmd/gaiadebug/hack.go +++ b/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(). diff --git a/docs/gaia/validators/validator-setup.md b/docs/gaia/validators/validator-setup.md index ca129ded9..cac1508d0 100644 --- a/docs/gaia/validators/validator-setup.md +++ b/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 -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. +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. -### 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. -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 \ @@ -102,11 +96,11 @@ gaiacli tx staking create-validator \ --pubkey=$(gaiad tendermint show-validator) \ --moniker="choose a moniker" \ --chain-id= \ - --from= \ + --from= \ --commission-rate="0.10" \ --commission-max-rate="0.20" \ --commission-max-change-rate="0.01" \ - --address-delegator= \ + --address-delegator="address of the delegator" \ --generate-only \ > unsignedValTx.json ``` diff --git a/x/slashing/app_test.go b/x/slashing/app_test.go index b2d0acde8..6f15e4aee 100644 --- a/x/slashing/app_test.go +++ b/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, mbk) - + keeper := NewKeeper(mapp.Cdc, keySlashing, stakingKeeper, mapp.ParamsKeeper.Subspace(DefaultParamspace), DefaultCodespace) mapp.Router().AddRoute(staking.RouterKey, staking.NewHandler(stakingKeeper)) mapp.Router().AddRoute(RouterKey, NewHandler(keeper)) + mapp.SetEndBlocker(getEndBlocker(stakingKeeper)) mapp.SetInitChainer(getInitChainer(mapp, stakingKeeper)) diff --git a/x/slashing/handler.go b/x/slashing/handler.go index 41a54ec1d..d9ae9f2d8 100644 --- a/x/slashing/handler.go +++ b/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 - // 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() - } + if validator.GetDelegatorShareExRate().Mul(selfDel.GetShares()).TruncateInt().LT(validator.GetMinSelfDelegation()) { + return ErrSelfDelegationTooLowToUnjail(k.codespace).Result() } // cannot be unjailed if not jailed diff --git a/x/slashing/handler_test.go b/x/slashing/handler_test.go index fdf9e8c3f..07cf45001 100644 --- a/x/slashing/handler_test.go +++ b/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) diff --git a/x/slashing/keeper.go b/x/slashing/keeper.go index c66d5f02d..8b8360492 100644 --- a/x/slashing/keeper.go +++ b/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( - cdc *codec.Codec, key sdk.StoreKey, vs sdk.ValidatorSet, - paramspace params.Subspace, codespace sdk.CodespaceType, bk BankKeeper, -) Keeper { - +func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, vs sdk.ValidatorSet, paramspace params.Subspace, codespace sdk.CodespaceType) Keeper { keeper := Keeper{ storeKey: key, cdc: cdc, - bk: bk, validatorSet: vs, paramspace: paramspace.WithKeyTable(ParamKeyTable()), codespace: codespace, diff --git a/x/slashing/keeper_contracts.go b/x/slashing/keeper_contracts.go deleted file mode 100644 index 15abdaf27..000000000 --- a/x/slashing/keeper_contracts.go +++ /dev/null @@ -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 -} diff --git a/x/slashing/test_common.go b/x/slashing/test_common.go index 9658855ba..13279d614 100644 --- a/x/slashing/test_common.go +++ b/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}) - require.Nil(t, err) + _, _, err = ck.AddCoins(ctx, sdk.AccAddress(addr), sdk.Coins{ + {sk.GetParams(ctx).BondDenom, initCoins}, + }) } - - mbk := mockBankKeeper{true} + require.Nil(t, err) 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)