From b92ac313acce45c0c60cfe319f774a4a50f05b02 Mon Sep 17 00:00:00 2001 From: Rigel Date: Fri, 31 Aug 2018 15:21:12 -0400 Subject: [PATCH] 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 | 1 + examples/democoin/mock/validator.go | 5 ++ types/stake.go | 4 ++ x/slashing/errors.go | 14 ++++- x/slashing/handler.go | 15 +++-- x/slashing/handler_test.go | 60 +++++++++++++++++++ x/slashing/test_common.go | 11 +++- x/stake/handler.go | 8 ++- x/stake/handler_test.go | 91 +++++++++++++++++++++++++++++ x/stake/keeper/delegation.go | 3 +- x/stake/keeper/sdk_types.go | 1 + 11 files changed, 200 insertions(+), 13 deletions(-) diff --git a/PENDING.md b/PENDING.md index 832f25e8f..3843be5fd 100644 --- a/PENDING.md +++ b/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. diff --git a/examples/democoin/mock/validator.go b/examples/democoin/mock/validator.go index f937e45dc..9f84786ad 100644 --- a/examples/democoin/mock/validator.go +++ b/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") +} diff --git a/types/stake.go b/types/stake.go index f41125177..978398fa5 100644 --- a/types/stake.go +++ b/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 } //_______________________________________________________________________________ diff --git a/x/slashing/errors.go b/x/slashing/errors.go index 4573d5e14..77cb2d28e 100644 --- a/x/slashing/errors.go +++ b/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") +} diff --git a/x/slashing/handler.go b/x/slashing/handler.go index d79ea73c2..c18587955 100644 --- a/x/slashing/handler.go +++ b/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())) diff --git a/x/slashing/handler_test.go b/x/slashing/handler_test.go index 8e3b719f4..8c522215a 100644 --- a/x/slashing/handler_test.go +++ b/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) +} diff --git a/x/slashing/test_common.go b/x/slashing/test_common.go index 105382378..697411310 100644 --- a/x/slashing/test_common.go +++ b/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}, + } +} diff --git a/x/stake/handler.go b/x/stake/handler.go index c8be6a835..4b478fffd 100644 --- a/x/stake/handler.go +++ b/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, } diff --git a/x/stake/handler_test.go b/x/stake/handler_test.go index f7d01bcfc..2006745d8 100644 --- a/x/stake/handler_test.go +++ b/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) diff --git a/x/stake/keeper/delegation.go b/x/stake/keeper/delegation.go index 9f7a402fc..2c997b463 100644 --- a/x/stake/keeper/delegation.go +++ b/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 } //______________________________________________________________________________________________________ diff --git a/x/stake/keeper/sdk_types.go b/x/stake/keeper/sdk_types.go index 480df701b..e4c7a1c19 100644 --- a/x/stake/keeper/sdk_types.go +++ b/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 }