diff --git a/PENDING.md b/PENDING.md index 3843be5fd5..832f25e8f4 100644 --- a/PENDING.md +++ b/PENDING.md @@ -73,7 +73,6 @@ 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. diff --git a/examples/democoin/mock/validator.go b/examples/democoin/mock/validator.go index 3e552d44ad..f937e45dcc 100644 --- a/examples/democoin/mock/validator.go +++ b/examples/democoin/mock/validator.go @@ -135,8 +135,3 @@ 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, addrVal sdk.AccAddress) sdk.Delegation { - panic("not implemented") -} diff --git a/types/stake.go b/types/stake.go index 71386959e4..f411251775 100644 --- a/types/stake.go +++ b/types/stake.go @@ -75,10 +75,6 @@ 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, AccAddress) Delegation } //_______________________________________________________________________________ diff --git a/x/slashing/errors.go b/x/slashing/errors.go index 77cb2d28e3..4573d5e145 100644 --- a/x/slashing/errors.go +++ b/x/slashing/errors.go @@ -12,28 +12,20 @@ const ( // Default slashing codespace DefaultCodespace sdk.CodespaceType = 10 - CodeInvalidValidator CodeType = 101 - CodeValidatorJailed CodeType = 102 - CodeValidatorNotJailed CodeType = 103 - CodeMissingSelfDelegation CodeType = 104 + CodeInvalidValidator CodeType = 101 + CodeValidatorJailed CodeType = 102 + CodeValidatorNotJailed CodeType = 103 ) 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") -} diff --git a/x/slashing/handler.go b/x/slashing/handler.go index 94cfa41ff7..d79ea73c2d 100644 --- a/x/slashing/handler.go +++ b/x/slashing/handler.go @@ -19,38 +19,35 @@ 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, 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())) diff --git a/x/slashing/handler_test.go b/x/slashing/handler_test.go index cb764ec696..8e3b719f49 100644 --- a/x/slashing/handler_test.go +++ b/x/slashing/handler_test.go @@ -2,7 +2,6 @@ package slashing import ( "testing" - "time" "github.com/stretchr/testify/require" @@ -28,62 +27,3 @@ 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 := 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, sdk.ValAddress(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(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(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(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) -} diff --git a/x/slashing/test_common.go b/x/slashing/test_common.go index d051a8cbc5..1053823786 100644 --- a/x/slashing/test_common.go +++ b/x/slashing/test_common.go @@ -4,7 +4,6 @@ import ( "encoding/hex" "os" "testing" - "time" "github.com/stretchr/testify/require" @@ -62,7 +61,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{Time: time.Unix(0, 0)}, false, log.NewTMLogger(os.Stdout)) + ctx := sdk.NewContext(ms, abci.Header{}, false, log.NewTMLogger(os.Stdout)) cdc := createTestCodec() accountMapper := auth.NewAccountMapper(cdc, keyAcc, auth.ProtoBaseAccount) ck := bank.NewKeeper(accountMapper) @@ -109,11 +108,3 @@ func newTestMsgCreateValidator(address sdk.ValAddress, pubKey crypto.PubKey, amt Delegation: sdk.Coin{"steak", amt}, } } - -func newTestMsgDelegate(delAddr, valAddr sdk.AccAddress, delAmount sdk.Int) stake.MsgDelegate { - return stake.MsgDelegate{ - DelegatorAddr: delAddr, - ValidatorAddr: valAddr, - Delegation: sdk.Coin{"steak", delAmount}, - } -} diff --git a/x/stake/handler.go b/x/stake/handler.go index 4b478fffd7..c8be6a835a 100644 --- a/x/stake/handler.go +++ b/x/stake/handler.go @@ -1,7 +1,6 @@ package stake import ( - "bytes" "time" sdk "github.com/cosmos/cosmos-sdk/types" @@ -129,19 +128,17 @@ 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 && !bytes.Equal(validator.Operator, msg.DelegatorAddr) { + if validator.Jailed { return ErrValidatorJailed(k.Codespace()).Result() } - _, err := k.Delegate(ctx, msg.DelegatorAddr, msg.Delegation, validator, true) if err != nil { return err.Result() @@ -152,7 +149,6 @@ 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, } diff --git a/x/stake/handler_test.go b/x/stake/handler_test.go index a5b70e054d..f7d01bcfc7 100644 --- a/x/stake/handler_test.go +++ b/x/stake/handler_test.go @@ -193,97 +193,6 @@ 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 := 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(valAddr, valAddr, unbondShares) - msgCompleteUnbonding := NewMsgCompleteUnbonding(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(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) diff --git a/x/stake/keeper/delegation.go b/x/stake/keeper/delegation.go index 2c997b4633..9f7a402fc2 100644 --- a/x/stake/keeper/delegation.go +++ b/x/stake/keeper/delegation.go @@ -288,7 +288,6 @@ 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 @@ -308,7 +307,7 @@ func (k Keeper) unbond(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValA k.RemoveValidator(ctx, validator.Operator) } - return amount, nil + return } //______________________________________________________________________________________________________