diff --git a/PENDING.md b/PENDING.md index e0b211ad3102..b6e159544afb 100644 --- a/PENDING.md +++ b/PENDING.md @@ -84,6 +84,7 @@ IMPROVEMENTS for tx size in the ante handler. * [\#3454](https://github.com/cosmos/cosmos-sdk/pull/3454) Add `--jail-whitelist` to `gaiad export` to enable testing of complex exports * [\#3424](https://github.com/cosmos/cosmos-sdk/issues/3424) Allow generation of gentxs with empty memo field. + * \#3507 General cleanup, removal of unnecessary struct fields, undelegation bugfix, and comment clarification in x/staking and x/slashing * SDK * [\#2605] x/params add subkey accessing diff --git a/client/lcd/lcd_test.go b/client/lcd/lcd_test.go index 98f55dc3c638..ff9da7ace97e 100644 --- a/client/lcd/lcd_test.go +++ b/client/lcd/lcd_test.go @@ -646,7 +646,6 @@ func TestBonding(t *testing.T) { redelegation := getRedelegations(t, port, addr, operAddrs[0], operAddrs[1]) require.Len(t, redelegation, 1) require.Len(t, redelegation[0].Entries, 1) - require.Equal(t, rdTokens, redelegation[0].Entries[0].Balance.Amount) delegatorUbds := getDelegatorUnbondingDelegations(t, port, addr) require.Len(t, delegatorUbds, 1) @@ -656,7 +655,6 @@ func TestBonding(t *testing.T) { delegatorReds := getRedelegations(t, port, addr, nil, nil) require.Len(t, delegatorReds, 1) require.Len(t, delegatorReds[0].Entries, 1) - require.Equal(t, rdTokens, delegatorReds[0].Entries[0].Balance.Amount) validatorUbds := getValidatorUnbondingDelegations(t, port, operAddrs[0]) require.Len(t, validatorUbds, 1) @@ -666,7 +664,6 @@ func TestBonding(t *testing.T) { validatorReds := getRedelegations(t, port, nil, operAddrs[0], nil) require.Len(t, validatorReds, 1) require.Len(t, validatorReds[0].Entries, 1) - require.Equal(t, rdTokens, validatorReds[0].Entries[0].Balance.Amount) // TODO Undonding status not currently implemented // require.Equal(t, sdk.Unbonding, bondedValidators[0].Status) diff --git a/cmd/gaia/app/app.go b/cmd/gaia/app/app.go index 2d66bd9342df..2910f54cc7be 100644 --- a/cmd/gaia/app/app.go +++ b/cmd/gaia/app/app.go @@ -355,10 +355,6 @@ func (h StakingHooks) AfterValidatorBonded(ctx sdk.Context, consAddr sdk.ConsAdd h.dh.AfterValidatorBonded(ctx, consAddr, valAddr) h.sh.AfterValidatorBonded(ctx, consAddr, valAddr) } -func (h StakingHooks) AfterValidatorPowerDidChange(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) { - h.dh.AfterValidatorPowerDidChange(ctx, consAddr, valAddr) - h.sh.AfterValidatorPowerDidChange(ctx, consAddr, valAddr) -} func (h StakingHooks) AfterValidatorBeginUnbonding(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) { h.dh.AfterValidatorBeginUnbonding(ctx, consAddr, valAddr) h.sh.AfterValidatorBeginUnbonding(ctx, consAddr, valAddr) diff --git a/cmd/gaia/app/export.go b/cmd/gaia/app/export.go index efc9c2051451..3c3df5e14989 100644 --- a/cmd/gaia/app/export.go +++ b/cmd/gaia/app/export.go @@ -150,7 +150,6 @@ func (app *GaiaApp) prepForZeroHeightGenesis(ctx sdk.Context, jailWhiteList []st panic("expected validator, not found") } - validator.BondHeight = 0 validator.UnbondingHeight = 0 valConsAddrs = append(valConsAddrs, validator.ConsAddress()) if applyWhiteList && !whiteListMap[addr.String()] { diff --git a/docs/spec/staking/hooks.md b/docs/spec/staking/hooks.md index 85fcc69cc4ce..e3c87d7c57fc 100644 --- a/docs/spec/staking/hooks.md +++ b/docs/spec/staking/hooks.md @@ -15,8 +15,6 @@ following hooks can registered with staking: - called when a validator is bonded - `AfterValidatorBeginUnbonding(Context, ConsAddress, ValAddress)` - called when a validator begins unbonding - - `AfterValidatorPowerDidChange(Context, ConsAddress, ValAddress)` - - called at EndBlock when a validator's power is changed - `BeforeDelegationCreated(Context, AccAddress, ValAddress)` - called when a delegation is created - `BeforeDelegationSharesModified(Context, AccAddress, ValAddress)` diff --git a/docs/spec/staking/state.md b/docs/spec/staking/state.md index cd49b7326393..dbe408cd5789 100644 --- a/docs/spec/staking/state.md +++ b/docs/spec/staking/state.md @@ -187,7 +187,6 @@ type RedelegationEntry struct { CompletionTime time.Time // unix time for redelegation completion InitialBalance sdk.Coin // initial balance when redelegation started Balance sdk.Coin // current balance (current value held in destination validator) - SharesSrc sdk.Dec // amount of source-validator shares removed by redelegation SharesDst sdk.Dec // amount of destination-validator shares created by redelegation } ``` diff --git a/types/int.go b/types/int.go index 5c4ff42450a3..33654c48abfa 100644 --- a/types/int.go +++ b/types/int.go @@ -18,6 +18,8 @@ func gt(i *big.Int, i2 *big.Int) bool { return i.Cmp(i2) == 1 } func lt(i *big.Int, i2 *big.Int) bool { return i.Cmp(i2) == -1 } +func lte(i *big.Int, i2 *big.Int) bool { return i.Cmp(i2) <= 0 } + func add(i *big.Int, i2 *big.Int) *big.Int { return new(big.Int).Add(i, i2) } func sub(i *big.Int, i2 *big.Int) *big.Int { return new(big.Int).Sub(i, i2) } @@ -191,6 +193,11 @@ func (i Int) LT(i2 Int) bool { return lt(i.i, i2.i) } +// LTE returns true if first Int is less than or equal to second +func (i Int) LTE(i2 Int) bool { + return lte(i.i, i2.i) +} + // Add adds Int from another func (i Int) Add(i2 Int) (res Int) { res = Int{add(i.i, i2.i)} diff --git a/types/stake.go b/types/stake.go index cd3e442a6300..05b3c4c19973 100644 --- a/types/stake.go +++ b/types/stake.go @@ -47,7 +47,6 @@ type Validator interface { GetTendermintPower() int64 // validation power in tendermint GetCommission() Dec // validator commission rate GetDelegatorShares() Dec // total outstanding delegator shares - GetBondHeight() int64 // height in which the validator became active GetDelegatorShareExRate() Dec // tokens per delegator share exchange rate } @@ -123,7 +122,6 @@ type StakingHooks interface { AfterValidatorBonded(ctx Context, consAddr ConsAddress, valAddr ValAddress) // Must be called when a validator is bonded AfterValidatorBeginUnbonding(ctx Context, consAddr ConsAddress, valAddr ValAddress) // Must be called when a validator begins unbonding - AfterValidatorPowerDidChange(ctx Context, consAddr ConsAddress, valAddr ValAddress) // Called at EndBlock when a validator's power did change BeforeDelegationCreated(ctx Context, delAddr AccAddress, valAddr ValAddress) // Must be called when a delegation is created BeforeDelegationSharesModified(ctx Context, delAddr AccAddress, valAddr ValAddress) // Must be called when a delegation's shares are modified diff --git a/x/distribution/keeper/hooks.go b/x/distribution/keeper/hooks.go index 3973b36ede6a..49d09b317e0e 100644 --- a/x/distribution/keeper/hooks.go +++ b/x/distribution/keeper/hooks.go @@ -82,8 +82,6 @@ func (h Hooks) AfterValidatorBeginUnbonding(ctx sdk.Context, _ sdk.ConsAddress, } func (h Hooks) AfterValidatorBonded(ctx sdk.Context, _ sdk.ConsAddress, valAddr sdk.ValAddress) { } -func (h Hooks) AfterValidatorPowerDidChange(ctx sdk.Context, _ sdk.ConsAddress, valAddr sdk.ValAddress) { -} func (h Hooks) BeforeValidatorSlashed(ctx sdk.Context, valAddr sdk.ValAddress, fraction sdk.Dec) { // record the slash event h.k.updateValidatorSlashFraction(ctx, valAddr, fraction) diff --git a/x/slashing/client/module_client.go b/x/slashing/client/module_client.go index 0ce05df6407f..aca581f2e8a8 100644 --- a/x/slashing/client/module_client.go +++ b/x/slashing/client/module_client.go @@ -5,6 +5,7 @@ import ( amino "github.com/tendermint/go-amino" "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/cosmos-sdk/x/slashing" "github.com/cosmos/cosmos-sdk/x/slashing/client/cli" ) @@ -22,7 +23,7 @@ func NewModuleClient(storeKey string, cdc *amino.Codec) ModuleClient { func (mc ModuleClient) GetQueryCmd() *cobra.Command { // Group slashing queries under a subcommand slashingQueryCmd := &cobra.Command{ - Use: "slashing", + Use: slashing.ModuleName, Short: "Querying commands for the slashing module", } @@ -40,7 +41,7 @@ func (mc ModuleClient) GetQueryCmd() *cobra.Command { // GetTxCmd returns the transaction commands for this module func (mc ModuleClient) GetTxCmd() *cobra.Command { slashingTxCmd := &cobra.Command{ - Use: "slashing", + Use: slashing.ModuleName, Short: "Slashing transactions subcommands", } diff --git a/x/slashing/errors.go b/x/slashing/errors.go index 0fa523c256ce..81c8e180470b 100644 --- a/x/slashing/errors.go +++ b/x/slashing/errors.go @@ -10,7 +10,7 @@ type CodeType = sdk.CodeType const ( // Default slashing codespace - DefaultCodespace sdk.CodespaceType = "SLASH" + DefaultCodespace sdk.CodespaceType = ModuleName CodeInvalidValidator CodeType = 101 CodeValidatorJailed CodeType = 102 diff --git a/x/slashing/genesis.go b/x/slashing/genesis.go index c3672c3d698b..8a6cfdee6217 100644 --- a/x/slashing/genesis.go +++ b/x/slashing/genesis.go @@ -21,7 +21,7 @@ type MissedBlock struct { Missed bool `json:"missed"` } -// HubDefaultGenesisState - default GenesisState used by Cosmos Hub +// DefaultGenesisState - default GenesisState used by Cosmos Hub func DefaultGenesisState() GenesisState { return GenesisState{ Params: DefaultParams(), diff --git a/x/slashing/handler.go b/x/slashing/handler.go index d0a26ff765df..201cc68ee700 100644 --- a/x/slashing/handler.go +++ b/x/slashing/handler.go @@ -31,6 +31,7 @@ func handleMsgUnjail(ctx sdk.Context, msg MsgUnjail, k Keeper) sdk.Result { return ErrMissingSelfDelegation(k.codespace).Result() } + // cannot be unjailed if not jailed if !validator.GetJailed() { return ErrValidatorNotJailed(k.codespace).Result() } @@ -42,6 +43,7 @@ func handleMsgUnjail(ctx sdk.Context, msg MsgUnjail, k Keeper) sdk.Result { return ErrNoValidatorForAddress(k.codespace).Result() } + // cannot be unjailed if tombstoned if info.Tombstoned { return ErrValidatorJailed(k.codespace).Result() } diff --git a/x/slashing/hooks.go b/x/slashing/hooks.go index 3a4a38f2a721..9908e8d7ff10 100644 --- a/x/slashing/hooks.go +++ b/x/slashing/hooks.go @@ -66,7 +66,6 @@ func (h Hooks) AfterValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) { // nolint - unused hooks func (h Hooks) AfterValidatorBeginUnbonding(_ sdk.Context, _ sdk.ConsAddress, _ sdk.ValAddress) {} -func (h Hooks) AfterValidatorPowerDidChange(_ sdk.Context, _ sdk.ConsAddress, _ sdk.ValAddress) {} func (h Hooks) BeforeValidatorModified(_ sdk.Context, _ sdk.ValAddress) {} func (h Hooks) BeforeDelegationCreated(_ sdk.Context, _ sdk.AccAddress, _ sdk.ValAddress) {} func (h Hooks) BeforeDelegationSharesModified(_ sdk.Context, _ sdk.AccAddress, _ sdk.ValAddress) {} diff --git a/x/slashing/keeper.go b/x/slashing/keeper.go index bf7c5a18b1d1..036d2c6080d4 100644 --- a/x/slashing/keeper.go +++ b/x/slashing/keeper.go @@ -39,15 +39,19 @@ func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, vs sdk.ValidatorSet, paramspa // power: power of the double-signing validator at the height of infraction func (k Keeper) handleDoubleSign(ctx sdk.Context, addr crypto.Address, infractionHeight int64, timestamp time.Time, power int64) { logger := ctx.Logger().With("module", "x/slashing") + + // calculate the age of the evidence time := ctx.BlockHeader().Time age := time.Sub(timestamp) + + // fetch the validator public key consAddr := sdk.ConsAddress(addr) pubkey, err := k.getPubkey(ctx, addr) if err != nil { panic(fmt.Sprintf("Validator consensus-address %v not found", consAddr)) } - // Reject evidence if the double is too old + // Reject evidence if the double-sign is too old if age > k.MaxEvidenceAge(ctx) { logger.Info(fmt.Sprintf("Ignored double sign from %s at height %d, age of %d past max age of %d", pubkey.Address(), infractionHeight, age, k.MaxEvidenceAge(ctx))) @@ -62,18 +66,20 @@ func (k Keeper) handleDoubleSign(ctx sdk.Context, addr crypto.Address, infractio // Tendermint might break this assumption at some point. return } + + // fetch the validator signing info signInfo, found := k.getValidatorSigningInfo(ctx, consAddr) if !found { panic(fmt.Sprintf("Expected signing info for validator %s but not found", consAddr)) } - // Validator is already tombstoned + // validator is already tombstoned if signInfo.Tombstoned { logger.Info(fmt.Sprintf("Ignored double sign from %s at height %d, validator already tombstoned", pubkey.Address(), infractionHeight)) return } - // Double sign confirmed + // double sign confirmed logger.Info(fmt.Sprintf("Confirmed double sign from %s at height %d, age of %d", pubkey.Address(), infractionHeight, age)) // We need to retrieve the stake distribution which signed the block, so we subtract ValidatorUpdateDelay from the evidence height. @@ -98,7 +104,7 @@ func (k Keeper) handleDoubleSign(ctx sdk.Context, addr crypto.Address, infractio k.validatorSet.Jail(ctx, consAddr) } - // Set slashed so far to total slash + // Set tombstoned to be true signInfo.Tombstoned = true // Set jailed until to be forever (max time) @@ -118,12 +124,15 @@ func (k Keeper) handleValidatorSignature(ctx sdk.Context, addr crypto.Address, p if err != nil { panic(fmt.Sprintf("Validator consensus-address %v not found", consAddr)) } - // Local index, so counts blocks validator *should* have signed - // Will use the 0-value default signing info if not present, except for start height + + // fetch signing info signInfo, found := k.getValidatorSigningInfo(ctx, consAddr) if !found { panic(fmt.Sprintf("Expected signing info for validator %s but not found", consAddr)) } + + // this is a relative index, so it counts blocks the validator *should* have signed + // will use the 0-value default signing info if not present, except for start height index := signInfo.IndexOffset % k.SignedBlocksWindow(ctx) signInfo.IndexOffset++ @@ -148,14 +157,19 @@ func (k Keeper) handleValidatorSignature(ctx sdk.Context, addr crypto.Address, p if missed { logger.Info(fmt.Sprintf("Absent validator %s at height %d, %d missed, threshold %d", addr, height, signInfo.MissedBlocksCounter, k.MinSignedPerWindow(ctx))) } + minHeight := signInfo.StartHeight + k.SignedBlocksWindow(ctx) maxMissed := k.SignedBlocksWindow(ctx) - k.MinSignedPerWindow(ctx) + + // if we are past the minimum height and the validator has missed too many blocks, punish them if height > minHeight && signInfo.MissedBlocksCounter > maxMissed { validator := k.validatorSet.ValidatorByConsAddr(ctx, consAddr) if validator != nil && !validator.GetJailed() { + // Downtime confirmed: slash and jail the validator logger.Info(fmt.Sprintf("Validator %s past min height of %d and below signed blocks threshold of %d", pubkey.Address(), minHeight, k.MinSignedPerWindow(ctx))) + // We need to retrieve the stake distribution which signed the block, so we subtract ValidatorUpdateDelay from the evidence height, // and subtract an additional 1 since this is the LastCommit. // Note that this *can* result in a negative "distributionHeight" up to -ValidatorUpdateDelay-1, @@ -165,6 +179,7 @@ func (k Keeper) handleValidatorSignature(ctx sdk.Context, addr crypto.Address, p k.validatorSet.Slash(ctx, consAddr, distributionHeight, power, k.SlashFractionDowntime(ctx)) k.validatorSet.Jail(ctx, consAddr) signInfo.JailedUntil = ctx.BlockHeader().Time.Add(k.DowntimeJailDuration(ctx)) + // We need to reset the counter & array so that the validator won't be immediately slashed for downtime upon rebonding. signInfo.MissedBlocksCounter = 0 signInfo.IndexOffset = 0 diff --git a/x/slashing/keys.go b/x/slashing/keys.go index 5e5b08a7f4d7..c2f491508c6e 100644 --- a/x/slashing/keys.go +++ b/x/slashing/keys.go @@ -8,14 +8,17 @@ import ( ) const ( + // ModuleName is the name of the module + ModuleName = "slashing" + // StoreKey is the store key string for slashing - StoreKey = "slashing" + StoreKey = ModuleName // RouterKey is the message route for slashing - RouterKey = "slashing" + RouterKey = ModuleName // QuerierRoute is the querier route for slashing - QuerierRoute = "slashing" + QuerierRoute = ModuleName ) // key prefix bytes diff --git a/x/slashing/params.go b/x/slashing/params.go index e5d68fbcd68a..439de35ea201 100644 --- a/x/slashing/params.go +++ b/x/slashing/params.go @@ -10,15 +10,21 @@ import ( // Default parameter namespace const ( - DefaultParamspace = "slashing" + DefaultParamspace = ModuleName + DefaultMaxEvidenceAge time.Duration = 60 * 2 * time.Second + DefaultSignedBlocksWindow int64 = 100 + DefaultDowntimeJailDuration time.Duration = 60 * 10 * time.Second ) // The Double Sign Jail period ends at Max Time supported by Amino (Dec 31, 9999 - 23:59:59 GMT) var ( - DoubleSignJailEndTime = time.Unix(253402300799, 0) + DoubleSignJailEndTime = time.Unix(253402300799, 0) + DefaultMinSignedPerWindow sdk.Dec = sdk.NewDecWithPrec(5, 1) + DefaultSlashFractionDoubleSign sdk.Dec = sdk.NewDec(1).Quo(sdk.NewDec(20)) + DefaultSlashFractionDowntime sdk.Dec = sdk.NewDec(1).Quo(sdk.NewDec(100)) ) -// Parameter store key +// Parameter store keys var ( KeyMaxEvidenceAge = []byte("MaxEvidenceAge") KeySignedBlocksWindow = []byte("SignedBlocksWindow") @@ -56,7 +62,7 @@ func (p Params) String() string { p.SlashFractionDowntime) } -// Implements params.ParamStruct +// Implements params.ParamSet func (p *Params) ParamSetPairs() params.ParamSetPairs { return params.ParamSetPairs{ {KeyMaxEvidenceAge, &p.MaxEvidenceAge}, @@ -68,31 +74,19 @@ func (p *Params) ParamSetPairs() params.ParamSetPairs { } } -// Default parameters used by Cosmos Hub +// Default parameters for this module func DefaultParams() Params { return Params{ - // defaultMaxEvidenceAge = 60 * 60 * 24 * 7 * 3 - // TODO Temporarily set to 2 minutes for testnets. - MaxEvidenceAge: 60 * 2 * time.Second, - - // TODO Temporarily set to 100 blocks for testnets - SignedBlocksWindow: 100, - - // TODO Temporarily set to 10 minutes for testnets - DowntimeJailDuration: 60 * 10 * time.Second, - - // CONTRACT must be less than 1 - // TODO enforce this contract https://github.com/cosmos/cosmos-sdk/issues/3474 - MinSignedPerWindow: sdk.NewDecWithPrec(5, 1), - - SlashFractionDoubleSign: sdk.NewDec(1).QuoInt64(20), - - SlashFractionDowntime: sdk.NewDec(1).QuoInt64(100), + MaxEvidenceAge: DefaultMaxEvidenceAge, + SignedBlocksWindow: DefaultSignedBlocksWindow, + MinSignedPerWindow: DefaultMinSignedPerWindow, + DowntimeJailDuration: DefaultDowntimeJailDuration, + SlashFractionDoubleSign: DefaultSlashFractionDoubleSign, + SlashFractionDowntime: DefaultSlashFractionDowntime, } } -// MaxEvidenceAge - Max age for evidence - 21 days (3 weeks) -// MaxEvidenceAge = 60 * 60 * 24 * 7 * 3 +// MaxEvidenceAge - max age for evidence func (k Keeper) MaxEvidenceAge(ctx sdk.Context) (res time.Duration) { k.paramspace.Get(ctx, KeyMaxEvidenceAge, &res) return @@ -104,7 +98,7 @@ func (k Keeper) SignedBlocksWindow(ctx sdk.Context) (res int64) { return } -// Downtime slashing threshold - default 50% of the SignedBlocksWindow +// Downtime slashing threshold func (k Keeper) MinSignedPerWindow(ctx sdk.Context) int64 { var minSignedPerWindow sdk.Dec k.paramspace.Get(ctx, KeyMinSignedPerWindow, &minSignedPerWindow) @@ -121,13 +115,13 @@ func (k Keeper) DowntimeJailDuration(ctx sdk.Context) (res time.Duration) { return } -// SlashFractionDoubleSign - currently default 5% +// SlashFractionDoubleSign func (k Keeper) SlashFractionDoubleSign(ctx sdk.Context) (res sdk.Dec) { k.paramspace.Get(ctx, KeySlashFractionDoubleSign, &res) return } -// SlashFractionDowntime - currently default 1% +// SlashFractionDowntime func (k Keeper) SlashFractionDowntime(ctx sdk.Context) (res sdk.Dec) { k.paramspace.Get(ctx, KeySlashFractionDowntime, &res) return diff --git a/x/slashing/tick.go b/x/slashing/tick.go index 15fb9cd18f57..bd779973f1ca 100644 --- a/x/slashing/tick.go +++ b/x/slashing/tick.go @@ -11,7 +11,8 @@ import ( // slashing begin block functionality func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, sk Keeper) sdk.Tags { - // Iterate over all the validators which *should* have signed this block + + // Iterate over all the validators which *should* have signed this block // store whether or not they have actually signed it and slash/unbond any // which have missed too many blocks in a row (downtime slashing) for _, voteInfo := range req.LastCommitInfo.GetVotes() { diff --git a/x/staking/client/module_client.go b/x/staking/client/module_client.go index ae2c8e7e80e3..754e8c5c8500 100644 --- a/x/staking/client/module_client.go +++ b/x/staking/client/module_client.go @@ -6,6 +6,7 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/x/staking/client/cli" + "github.com/cosmos/cosmos-sdk/x/staking/types" ) // ModuleClient exports all client functionality from this module @@ -21,7 +22,7 @@ func NewModuleClient(storeKey string, cdc *amino.Codec) ModuleClient { // GetQueryCmd returns the cli query commands for this module func (mc ModuleClient) GetQueryCmd() *cobra.Command { stakingQueryCmd := &cobra.Command{ - Use: "staking", + Use: types.ModuleName, Short: "Querying commands for the staking module", } stakingQueryCmd.AddCommand(client.GetCommands( @@ -46,7 +47,7 @@ func (mc ModuleClient) GetQueryCmd() *cobra.Command { // GetTxCmd returns the transaction commands for this module func (mc ModuleClient) GetTxCmd() *cobra.Command { stakingTxCmd := &cobra.Command{ - Use: "staking", + Use: types.ModuleName, Short: "Staking transaction subcommands", } diff --git a/x/staking/genesis.go b/x/staking/genesis.go index 40df1a2f4630..8940378f18a8 100644 --- a/x/staking/genesis.go +++ b/x/staking/genesis.go @@ -34,6 +34,8 @@ func InitGenesis(ctx sdk.Context, keeper Keeper, data types.GenesisState) (res [ // Manually set indices for the first time keeper.SetValidatorByConsAddr(ctx, validator) keeper.SetValidatorByPowerIndex(ctx, validator) + + // Call the creation hook if not exported if !data.Exported { keeper.AfterValidatorCreated(ctx, validator.OperatorAddr) } @@ -45,10 +47,12 @@ func InitGenesis(ctx sdk.Context, keeper Keeper, data types.GenesisState) (res [ } for _, delegation := range data.Bonds { + // Call the before-creation hook if not exported if !data.Exported { keeper.BeforeDelegationCreated(ctx, delegation.DelegatorAddr, delegation.ValidatorAddr) } keeper.SetDelegation(ctx, delegation) + // Call the after-modification hook if not exported if !data.Exported { keeper.AfterDelegationModified(ctx, delegation.DelegatorAddr, delegation.ValidatorAddr) } @@ -147,7 +151,7 @@ func ValidateGenesis(data types.GenesisState) error { if err != nil { return err } - err = validateParams(data.Params) + err = data.Params.Validate() if err != nil { return err } @@ -155,23 +159,16 @@ func ValidateGenesis(data types.GenesisState) error { return nil } -func validateParams(params types.Params) error { - if params.BondDenom == "" { - return fmt.Errorf("staking parameter BondDenom can't be an empty string") - } - return nil -} - func validateGenesisStateValidators(validators []types.Validator) (err error) { addrMap := make(map[string]bool, len(validators)) for i := 0; i < len(validators); i++ { val := validators[i] strKey := string(val.ConsPubKey.Bytes()) if _, ok := addrMap[strKey]; ok { - return fmt.Errorf("duplicate validator in genesis state: moniker %v, Address %v", val.Description.Moniker, val.ConsAddress()) + return fmt.Errorf("duplicate validator in genesis state: moniker %v, address %v", val.Description.Moniker, val.ConsAddress()) } if val.Jailed && val.Status == sdk.Bonded { - return fmt.Errorf("validator is bonded and jailed in genesis state: moniker %v, Address %v", val.Description.Moniker, val.ConsAddress()) + return fmt.Errorf("validator is bonded and jailed in genesis state: moniker %v, address %v", val.Description.Moniker, val.ConsAddress()) } if val.DelegatorShares.IsZero() && val.Status != sdk.Unbonding { return fmt.Errorf("bonded/unbonded genesis validator cannot have zero delegator shares, validator: %v", val) diff --git a/x/staking/handler.go b/x/staking/handler.go index 3bf5ddf05d22..76dee0eacc3e 100644 --- a/x/staking/handler.go +++ b/x/staking/handler.go @@ -1,7 +1,6 @@ package staking import ( - "bytes" "time" abci "github.com/tendermint/tendermint/abci/types" @@ -87,20 +86,16 @@ func EndBlocker(ctx sdk.Context, k keeper.Keeper) ([]abci.ValidatorUpdate, sdk.T return validatorUpdates, resTags } -//_____________________________________________________________________ - // These functions assume everything has been authenticated, // now we just perform action and save func handleMsgCreateValidator(ctx sdk.Context, msg types.MsgCreateValidator, k keeper.Keeper) sdk.Result { // check to see if the pubkey or sender has been registered before - _, found := k.GetValidator(ctx, msg.ValidatorAddr) - if found { + if _, found := k.GetValidator(ctx, msg.ValidatorAddr); found { return ErrValidatorOwnerExists(k.Codespace()).Result() } - _, found = k.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(msg.PubKey)) - if found { + if _, found := k.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(msg.PubKey)); found { return ErrValidatorPubKeyExists(k.Codespace()).Result() } @@ -135,6 +130,7 @@ func handleMsgCreateValidator(ctx sdk.Context, msg types.MsgCreateValidator, k k k.SetValidatorByConsAddr(ctx, validator) k.SetNewValidatorByPowerIndex(ctx, validator) + // call the after-creation hook k.AfterValidatorCreated(ctx, validator.OperatorAddr) // move coins from the msg.Address account to a (self-delegation) delegator account @@ -176,7 +172,9 @@ func handleMsgEditValidator(ctx sdk.Context, msg types.MsgEditValidator, k keepe return err.Result() } + // call the before-modification hook since we're about to update the commission k.BeforeValidatorModified(ctx, msg.ValidatorAddr) + validator.Commission = commission } @@ -203,10 +201,6 @@ func handleMsgDelegate(ctx sdk.Context, msg types.MsgDelegate, k keeper.Keeper) return ErrBadDenom(k.Codespace()).Result() } - if validator.Jailed && !bytes.Equal(validator.OperatorAddr, msg.DelegatorAddr) { - return ErrValidatorJailed(k.Codespace()).Result() - } - _, err := k.Delegate(ctx, msg.DelegatorAddr, msg.Value, validator, true) if err != nil { return err.Result() diff --git a/x/staking/handler_test.go b/x/staking/handler_test.go index e9692e7fff53..7b8778a87e90 100644 --- a/x/staking/handler_test.go +++ b/x/staking/handler_test.go @@ -270,15 +270,10 @@ func TestLegacyValidatorDelegations(t *testing.T) { require.Equal(t, bondAmount, bond.Shares.RoundInt()) require.Equal(t, bondAmount, validator.DelegatorShares.RoundInt()) - // 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) + require.True(t, got.IsOK(), "expected delegation to be ok, got %v", got) // verify validator bonded shares validator, found = keeper.GetValidator(ctx, valAddr) @@ -613,10 +608,6 @@ func TestJailValidator(t *testing.T) { require.True(t, found) require.True(t, validator.Jailed, "%v", validator) - // test that this address cannot yet be bonded too because is jailed - got = handleMsgDelegate(ctx, msgDelegate, keeper) - require.False(t, got.IsOK(), "expected error, got %v", got) - // test that the delegator can still withdraw their bonds msgUndelegateDelegator := NewMsgUndelegate(delegatorAddr, validatorAddr, sdk.NewDec(10)) got = handleMsgUndelegate(ctx, msgUndelegateDelegator, keeper) @@ -1190,7 +1181,6 @@ func TestBondUnbondRedelegateSlashTwice(t *testing.T) { redelegation, found := keeper.GetRedelegation(ctx, del, valA, valB) require.True(t, found) require.Len(t, redelegation.Entries, 1) - require.Equal(t, rdTokens.DivRaw(2), redelegation.Entries[0].Balance.Amount) // destination delegation should have been slashed by half delegation, found = keeper.GetDelegation(ctx, del, valB) @@ -1216,7 +1206,6 @@ func TestBondUnbondRedelegateSlashTwice(t *testing.T) { redelegation, found = keeper.GetRedelegation(ctx, del, valA, valB) require.True(t, found) require.Len(t, redelegation.Entries, 1) - require.Equal(t, rdTokens.DivRaw(2), redelegation.Entries[0].Balance.Amount) // destination delegation should be unchanged delegation, found = keeper.GetDelegation(ctx, del, valB) diff --git a/x/staking/keeper/_store.md b/x/staking/keeper/_store.md deleted file mode 100644 index f6430c312b90..000000000000 --- a/x/staking/keeper/_store.md +++ /dev/null @@ -1,43 +0,0 @@ -# Stores - -This document provided a bit more insight as to the purpose of several related -prefixed areas of the staking store which are accessed in `x/staking/keeper.go`. - -# IAVL Store - -## Validators - - Prefix Key Space: ValidatorsKey - - Key/Sort: Validator Operator Address - - Value: Validator Object - - Contains: All Validator records independent of being bonded or not - - Used For: Retrieve validator from operator address, general validator retrieval - -## Validators By Power - - Prefix Key Space: ValidatorsByPowerKey - - Key/Sort: Validator Power (equivalent bonded shares) then Block - Height then Transaction Order - - Value: Validator Operator Address - - Contains: All Validator records independent of being bonded or not - - Used For: Determining who the top validators are whom should be bonded - -## Validators Cliff Power - - Prefix Key Space: ValidatorCliffKey - - Key/Sort: single-record - - Value: Validator Power Key (as above store) - - Contains: The cliff validator (ex. 100th validator) power - - Used For: Efficient updates to validator status - -## Validators Bonded - - Prefix Key Space: ValidatorsBondedKey - - Key/Sort: Validator PubKey Address (NOTE same as Tendermint) - - Value: Validator Operator Address - - Contains: Only currently bonded Validators - - Used For: Retrieving the list of all currently bonded validators when updating - for a new validator entering the validator set we may want to loop - through this set to determine who we've kicked out. - retrieving validator by tendermint index - -# Transient Store - -The transient store persists between transations but not between blocks - diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index 162d87aa4d6c..e6874d7c272a 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -72,22 +72,21 @@ func (k Keeper) GetDelegatorDelegations(ctx sdk.Context, delegator sdk.AccAddres return delegations[:i] // trim if the array length < maxRetrieve } -// set the delegation +// set a delegation func (k Keeper) SetDelegation(ctx sdk.Context, delegation types.Delegation) { store := ctx.KVStore(k.storeKey) b := types.MustMarshalDelegation(k.cdc, delegation) store.Set(GetDelegationKey(delegation.DelegatorAddr, delegation.ValidatorAddr), b) } -// remove a delegation from store +// remove a delegation func (k Keeper) RemoveDelegation(ctx sdk.Context, delegation types.Delegation) { + // TODO: Consider calling hooks outside of the store wrapper functions, it's unobvious. k.BeforeDelegationRemoved(ctx, delegation.DelegatorAddr, delegation.ValidatorAddr) store := ctx.KVStore(k.storeKey) store.Delete(GetDelegationKey(delegation.DelegatorAddr, delegation.ValidatorAddr)) } -//_____________________________________________________________________________________ - // return a given amount of all the delegator unbonding-delegations func (k Keeper) GetUnbondingDelegations(ctx sdk.Context, delegator sdk.AccAddress, maxRetrieve uint16) (unbondingDelegations []types.UnbondingDelegation) { @@ -153,7 +152,7 @@ func (k Keeper) IterateUnbondingDelegations(ctx sdk.Context, fn func(index int64 } } -// HasMaxUnbondingDelegationEntries - unbonding delegation has maximum number of entries +// HasMaxUnbondingDelegationEntries - check if unbonding delegation has maximum number of entries func (k Keeper) HasMaxUnbondingDelegationEntries(ctx sdk.Context, delegatorAddr sdk.AccAddress, validatorAddr sdk.ValAddress) bool { @@ -197,7 +196,6 @@ func (k Keeper) SetUnbondingDelegationEntry(ctx sdk.Context, return ubd } -//________________________________________________ // unbonding delegation queue timeslice operations // gets a specific unbonding queue timeslice. A timeslice is a slice of DVPairs @@ -258,8 +256,6 @@ func (k Keeper) DequeueAllMatureUBDQueue(ctx sdk.Context, return matureUnbonds } -//_____________________________________________________________________________________ - // return a given amount of all the delegator redelegations func (k Keeper) GetRedelegations(ctx sdk.Context, delegator sdk.AccAddress, maxRetrieve uint16) (redelegations []types.Redelegation) { @@ -356,11 +352,10 @@ func (k Keeper) SetRedelegationEntry(ctx sdk.Context, red, found := k.GetRedelegation(ctx, delegatorAddr, validatorSrcAddr, validatorDstAddr) if found { - red.AddEntry(creationHeight, minTime, balance, sharesSrc, sharesDst) + red.AddEntry(creationHeight, minTime, balance, sharesDst) } else { red = types.NewRedelegation(delegatorAddr, validatorSrcAddr, - validatorDstAddr, creationHeight, minTime, balance, sharesSrc, - sharesDst) + validatorDstAddr, creationHeight, minTime, balance, sharesDst) } k.SetRedelegation(ctx, red) return red @@ -390,7 +385,6 @@ func (k Keeper) RemoveRedelegation(ctx sdk.Context, red types.Redelegation) { store.Delete(GetREDByValDstIndexKey(red.DelegatorAddr, red.ValidatorSrcAddr, red.ValidatorDstAddr)) } -//________________________________________________ // redelegation queue timeslice operations // Gets a specific redelegation queue timeslice. A timeslice is a slice of DVVTriplets corresponding to redelegations @@ -448,20 +442,18 @@ func (k Keeper) DequeueAllMatureRedelegationQueue(ctx sdk.Context, currTime time return matureRedelegations } -//_____________________________________________________________________________________ - // Perform a delegation, set/update everything necessary within the store. func (k Keeper) Delegate(ctx sdk.Context, delAddr sdk.AccAddress, bondAmt sdk.Coin, validator types.Validator, subtractAccount bool) (newShares sdk.Dec, err sdk.Error) { // In some situations, the exchange rate becomes invalid, e.g. if - // validator loses all tokens due to slashing. In this case, + // Validator loses all tokens due to slashing. In this case, // make all future delegations invalid. if validator.DelegatorShareExRate().IsZero() { return sdk.ZeroDec(), types.ErrDelegatorShareExRateInvalid(k.Codespace()) } - // Get or create the delegator delegation + // Get or create the delegation object delegation, found := k.GetDelegation(ctx, delAddr, validator.OperatorAddr) if !found { delegation = types.Delegation{ @@ -490,24 +482,27 @@ func (k Keeper) Delegate(ctx sdk.Context, delAddr sdk.AccAddress, bondAmt sdk.Co // Update delegation delegation.Shares = delegation.Shares.Add(newShares) k.SetDelegation(ctx, delegation) + + // Call the after-modification hook k.AfterDelegationModified(ctx, delegation.DelegatorAddr, delegation.ValidatorAddr) return newShares, nil } -// unbond the the delegation return +// unbond a particular delegation and perform associated store operations func (k Keeper) unbond(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress, shares sdk.Dec) (amount sdk.Int, err sdk.Error) { - // check if delegation has any shares in it unbond + // check if a delegation object exists in the store delegation, found := k.GetDelegation(ctx, delAddr, valAddr) if !found { return amount, types.ErrNoDelegatorForAddress(k.Codespace()) } + // call the before-delegation-modified hook k.BeforeDelegationSharesModified(ctx, delAddr, valAddr) - // retrieve the amount to remove + // ensure that we have enough shares to remove if delegation.Shares.LT(shares) { return amount, types.ErrNotEnoughDelegationShares(k.Codespace(), delegation.Shares.String()) } @@ -518,23 +513,20 @@ func (k Keeper) unbond(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValA return amount, types.ErrNoValidatorFound(k.Codespace()) } - // subtract shares from delegator + // subtract shares from delegation delegation.Shares = delegation.Shares.Sub(shares) - // remove the delegation + // update or remove the delegation if delegation.Shares.IsZero() { - - // if the delegation is the operator of the validator then - // trigger a jail validator + // if the delegator is the operator of the validator then jail the validator if bytes.Equal(delegation.DelegatorAddr, validator.OperatorAddr) && !validator.Jailed { k.jailValidator(ctx, validator) validator = k.mustGetValidator(ctx, validator.OperatorAddr) } - k.RemoveDelegation(ctx, delegation) } else { - // update the delegation k.SetDelegation(ctx, delegation) + // call the after delegation modification hook k.AfterDelegationModified(ctx, delegation.DelegatorAddr, delegation.ValidatorAddr) } @@ -549,8 +541,6 @@ func (k Keeper) unbond(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValA return amount, nil } -//______________________________________________________________________________________________________ - // get info for begin functions: completionTime and CreationHeight func (k Keeper) getBeginInfo(ctx sdk.Context, valSrcAddr sdk.ValAddress) ( completionTime time.Time, height int64, completeNow bool) { @@ -558,6 +548,7 @@ func (k Keeper) getBeginInfo(ctx sdk.Context, valSrcAddr sdk.ValAddress) ( validator, found := k.GetValidator(ctx, valSrcAddr) switch { + // TODO: when would the validator not be found? case !found || validator.Status == sdk.Bonded: // the longest wait - just unbonding period from now @@ -578,7 +569,7 @@ func (k Keeper) getBeginInfo(ctx sdk.Context, valSrcAddr sdk.ValAddress) ( } } -// begin unbonding an unbonding record +// begin unbonding part or all of a delegation func (k Keeper) Undelegate(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress, sharesAmount sdk.Dec) (completionTime time.Time, sdkErr sdk.Error) { @@ -633,9 +624,12 @@ func (k Keeper) CompleteUnbonding(ctx sdk.Context, delAddr sdk.AccAddress, ubd.RemoveEntry(int64(i)) i-- - _, _, err := k.bankKeeper.AddCoins(ctx, ubd.DelegatorAddr, sdk.Coins{entry.Balance}) - if err != nil { - return err + // track undelegation only when remaining or truncated shares are non-zero + if !entry.Balance.IsZero() { + _, err := k.bankKeeper.UndelegateCoins(ctx, ubd.DelegatorAddr, sdk.Coins{entry.Balance}) + if err != nil { + return err + } } } } diff --git a/x/staking/keeper/delegation_test.go b/x/staking/keeper/delegation_test.go index 10e34a81e816..49e8ccc722fa 100644 --- a/x/staking/keeper/delegation_test.go +++ b/x/staking/keeper/delegation_test.go @@ -506,7 +506,7 @@ func TestGetRedelegationsFromValidator(t *testing.T) { rd := types.NewRedelegation(addrDels[0], addrVals[0], addrVals[1], 0, time.Unix(0, 0), sdk.NewInt64Coin(types.DefaultBondDenom, 5), - sdk.NewDec(5), sdk.NewDec(5)) + sdk.NewDec(5)) // set and retrieve a record keeper.SetRedelegation(ctx, rd) @@ -530,7 +530,7 @@ func TestRedelegation(t *testing.T) { rd := types.NewRedelegation(addrDels[0], addrVals[0], addrVals[1], 0, time.Unix(0, 0), sdk.NewInt64Coin(types.DefaultBondDenom, 5), - sdk.NewDec(5), sdk.NewDec(5)) + sdk.NewDec(5)) // test shouldn't have and redelegations has := keeper.HasReceivingRedelegation(ctx, addrDels[0], addrVals[1]) @@ -558,7 +558,6 @@ func TestRedelegation(t *testing.T) { require.True(t, has) // modify a records, save, and retrieve - rd.Entries[0].SharesSrc = sdk.NewDec(21) rd.Entries[0].SharesDst = sdk.NewDec(21) keeper.SetRedelegation(ctx, rd) @@ -786,7 +785,6 @@ func TestRedelegateFromUnbondingValidator(t *testing.T) { ubd, found := keeper.GetRedelegation(ctx, addrDels[0], addrVals[0], addrVals[1]) require.True(t, found) require.Len(t, ubd.Entries, 1) - require.True(t, ubd.Entries[0].Balance.IsEqual(sdk.NewCoin(params.BondDenom, redelegateTokens))) assert.Equal(t, blockHeight, ubd.Entries[0].CreationHeight) assert.True(t, blockTime.Add(params.UnbondingTime).Equal(ubd.Entries[0].CompletionTime)) } diff --git a/x/staking/keeper/hooks.go b/x/staking/keeper/hooks.go index 4ed0a724c54d..e41106b4af81 100644 --- a/x/staking/keeper/hooks.go +++ b/x/staking/keeper/hooks.go @@ -30,12 +30,6 @@ func (k Keeper) AfterValidatorBonded(ctx sdk.Context, consAddr sdk.ConsAddress, } } -func (k Keeper) AfterValidatorPowerDidChange(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) { - if k.hooks != nil { - k.hooks.AfterValidatorPowerDidChange(ctx, consAddr, valAddr) - } -} - func (k Keeper) AfterValidatorBeginUnbonding(ctx sdk.Context, consAddr sdk.ConsAddress, valAddr sdk.ValAddress) { if k.hooks != nil { k.hooks.AfterValidatorBeginUnbonding(ctx, consAddr, valAddr) diff --git a/x/staking/keeper/keeper.go b/x/staking/keeper/keeper.go index 73f71a8bfec3..8984d88b7a9a 100644 --- a/x/staking/keeper/keeper.go +++ b/x/staking/keeper/keeper.go @@ -52,16 +52,12 @@ func (k *Keeper) SetHooks(sh sdk.StakingHooks) *Keeper { return k } -//_________________________________________________________________________ - // return the codespace func (k Keeper) Codespace() sdk.CodespaceType { return k.codespace } -//_______________________________________________________________________ - -// load the pool +// get the pool func (k Keeper) GetPool(ctx sdk.Context) (pool types.Pool) { store := ctx.KVStore(k.storeKey) b := store.Get(PoolKey) @@ -79,8 +75,6 @@ func (k Keeper) SetPool(ctx sdk.Context, pool types.Pool) { store.Set(PoolKey, b) } -//_______________________________________________________________________ - // Load the last total validator power. func (k Keeper) GetLastTotalPower(ctx sdk.Context) (power sdk.Int) { store := ctx.KVStore(k.storeKey) @@ -99,9 +93,7 @@ func (k Keeper) SetLastTotalPower(ctx sdk.Context, power sdk.Int) { store.Set(LastTotalPowerKey, b) } -//_______________________________________________________________________ - -// Load the last validator power. +// Get the last validator power. // Returns zero if the operator was not a validator last block. func (k Keeper) GetLastValidatorPower(ctx sdk.Context, operator sdk.ValAddress) (power sdk.Int) { store := ctx.KVStore(k.storeKey) diff --git a/x/staking/keeper/key.go b/x/staking/keeper/key.go index 6dbb053cf688..ab4eebbffa89 100644 --- a/x/staking/keeper/key.go +++ b/x/staking/keeper/key.go @@ -13,9 +13,7 @@ import ( //nolint var ( // Keys for store prefixes - // TODO DEPRECATED: delete in next release and reorder keys - // ParamKey = []byte{0x00} // key for parameters relating to stake - PoolKey = []byte{0x01} // key for the staking pools + PoolKey = []byte{0x00} // key for the staking pools // Last* values are const during a block. LastValidatorPowerKey = []byte{0x11} // prefix for each key to a validator index, for bonded validators @@ -37,8 +35,6 @@ var ( ValidatorQueueKey = []byte{0x43} // prefix for the timestamps in validator queue ) -const maxDigitsForAccount = 12 // ~220,000,000 atoms created at launch - // gets the key for the validator with address // VALUE: staking/types.Validator func GetValidatorKey(operatorAddr sdk.ValAddress) []byte { diff --git a/x/staking/keeper/params.go b/x/staking/keeper/params.go index a0f1acaccd06..dede56b2c19d 100644 --- a/x/staking/keeper/params.go +++ b/x/staking/keeper/params.go @@ -10,7 +10,7 @@ import ( // Default parameter namespace const ( - DefaultParamspace = "staking" + DefaultParamspace = types.ModuleName ) // ParamTable for staking module diff --git a/x/staking/keeper/query_utils.go b/x/staking/keeper/query_utils.go index 872059f1108a..9de7f627f491 100644 --- a/x/staking/keeper/query_utils.go +++ b/x/staking/keeper/query_utils.go @@ -12,7 +12,7 @@ func (k Keeper) GetDelegatorValidators(ctx sdk.Context, delegatorAddr sdk.AccAdd store := ctx.KVStore(k.storeKey) delegatorPrefixKey := GetDelegationsKey(delegatorAddr) - iterator := sdk.KVStorePrefixIterator(store, delegatorPrefixKey) //smallest to largest + iterator := sdk.KVStorePrefixIterator(store, delegatorPrefixKey) // smallest to largest defer iterator.Close() i := 0 @@ -71,7 +71,7 @@ func (k Keeper) GetAllUnbondingDelegations(ctx sdk.Context, delegator sdk.AccAdd store := ctx.KVStore(k.storeKey) delegatorPrefixKey := GetUBDsKey(delegator) - iterator := sdk.KVStorePrefixIterator(store, delegatorPrefixKey) //smallest to largest + iterator := sdk.KVStorePrefixIterator(store, delegatorPrefixKey) // smallest to largest defer iterator.Close() i := 0 @@ -87,7 +87,7 @@ func (k Keeper) GetAllUnbondingDelegations(ctx sdk.Context, delegator sdk.AccAdd func (k Keeper) GetAllRedelegations(ctx sdk.Context, delegator sdk.AccAddress, srcValAddress, dstValAddress sdk.ValAddress) (redelegations []types.Redelegation) { store := ctx.KVStore(k.storeKey) delegatorPrefixKey := GetREDsKey(delegator) - iterator := sdk.KVStorePrefixIterator(store, delegatorPrefixKey) //smallest to largest + iterator := sdk.KVStorePrefixIterator(store, delegatorPrefixKey) // smallest to largest defer iterator.Close() srcValFilter := !(srcValAddress.Empty() || srcValAddress == nil) diff --git a/x/staking/keeper/sdk_types.go b/x/staking/keeper/sdk_types.go index 8c5bad161288..ad52c8b25ec6 100644 --- a/x/staking/keeper/sdk_types.go +++ b/x/staking/keeper/sdk_types.go @@ -106,8 +106,6 @@ func (k Keeper) InflateSupply(ctx sdk.Context, newTokens sdk.Int) { k.SetPool(ctx, pool) } -//__________________________________________________________________________ - // Implements DelegationSet var _ sdk.DelegationSet = Keeper{} @@ -133,7 +131,7 @@ func (k Keeper) IterateDelegations(ctx sdk.Context, delAddr sdk.AccAddress, store := ctx.KVStore(k.storeKey) delegatorPrefixKey := GetDelegationsKey(delAddr) - iterator := sdk.KVStorePrefixIterator(store, delegatorPrefixKey) //smallest to largest + iterator := sdk.KVStorePrefixIterator(store, delegatorPrefixKey) // smallest to largest defer iterator.Close() for i := int64(0); iterator.Valid(); iterator.Next() { del := types.MustUnmarshalDelegation(k.cdc, iterator.Value()) diff --git a/x/staking/keeper/slash.go b/x/staking/keeper/slash.go index 100f32674146..98f21b486cc0 100644 --- a/x/staking/keeper/slash.go +++ b/x/staking/keeper/slash.go @@ -9,17 +9,17 @@ import ( // Slash a validator for an infraction committed at a known height // Find the contributing stake at that height and burn the specified slashFactor -// of it, updating unbonding delegation & redelegations appropriately +// of it, updating unbonding delegations & redelegations appropriately // // CONTRACT: // slashFactor is non-negative // CONTRACT: -// Infraction committed equal to or less than an unbonding period in the past, +// Infraction was committed equal to or less than an unbonding period in the past, // so all unbonding delegations and redelegations from that height are stored // CONTRACT: // Slash will not slash unbonded validators (for the above reason) // CONTRACT: -// Infraction committed at the current height or at a past height, +// Infraction was committed at the current height or at a past height, // not at a height in the future func (k Keeper) Slash(ctx sdk.Context, consAddr sdk.ConsAddress, infractionHeight int64, power int64, slashFactor sdk.Dec) { logger := ctx.Logger().With("module", "x/staking") @@ -34,7 +34,6 @@ func (k Keeper) Slash(ctx sdk.Context, consAddr sdk.ConsAddress, infractionHeigh slashAmount := slashAmountDec.TruncateInt() // ref https://github.com/cosmos/cosmos-sdk/issues/1348 - // ref https://github.com/cosmos/cosmos-sdk/issues/1471 validator, found := k.GetValidatorByConsAddr(ctx, consAddr) if !found { @@ -48,12 +47,14 @@ func (k Keeper) Slash(ctx sdk.Context, consAddr sdk.ConsAddress, infractionHeigh return } - // should not be slashing unbonded + // should not be slashing an unbonded validator if validator.Status == sdk.Unbonded { panic(fmt.Sprintf("should not be slashing unbonded validator: %s", validator.GetOperator())) } operatorAddress := validator.GetOperator() + + // call the before-modification hook k.BeforeValidatorModified(ctx, operatorAddress) // we need to calculate the *effective* slash fraction for distribution @@ -63,6 +64,7 @@ func (k Keeper) Slash(ctx sdk.Context, consAddr sdk.ConsAddress, infractionHeigh if effectiveFraction.GT(sdk.OneDec()) { effectiveFraction = sdk.OneDec() } + // call the before-slashed hook k.BeforeValidatorSlashed(ctx, operatorAddress, effectiveFraction) } @@ -115,6 +117,7 @@ func (k Keeper) Slash(ctx sdk.Context, consAddr sdk.ConsAddress, infractionHeigh // Deduct from validator's bonded tokens and update the validator. // The deducted tokens are returned to pool.NotBondedTokens. + // TODO: Move the token accounting outside of `RemoveValidatorTokens` so it is less confusing validator = k.RemoveValidatorTokens(ctx, validator, tokensToBurn) pool := k.GetPool(ctx) // Burn the slashed tokens, which are now loose. @@ -216,7 +219,7 @@ func (k Keeper) slashRedelegation(ctx sdk.Context, validator types.Validator, re totalSlashAmount = sdk.ZeroInt() // perform slashing on all entries within the redelegation - for i, entry := range redelegation.Entries { + for _, entry := range redelegation.Entries { // If redelegation started before this height, stake didn't contribute to infraction if entry.CreationHeight < infractionHeight { @@ -233,19 +236,6 @@ func (k Keeper) slashRedelegation(ctx sdk.Context, validator types.Validator, re slashAmount := slashAmountDec.TruncateInt() totalSlashAmount = totalSlashAmount.Add(slashAmount) - // Don't slash more tokens than held - // Possible since the redelegation may already - // have been slashed, and slash amounts are calculated - // according to stake held at time of infraction - redelegationSlashAmount := sdk.MinInt(slashAmount, entry.Balance.Amount) - - // Update entry if necessary - if !redelegationSlashAmount.IsZero() { - entry.Balance.Amount = entry.Balance.Amount.Sub(redelegationSlashAmount) - redelegation.Entries[i] = entry - k.SetRedelegation(ctx, redelegation) - } - // Unbond from target validator sharesToUnbond := slashFactor.Mul(entry.SharesDst) if sharesToUnbond.IsZero() { diff --git a/x/staking/keeper/slash_test.go b/x/staking/keeper/slash_test.go index fb5beb15d83c..6457617308d4 100644 --- a/x/staking/keeper/slash_test.go +++ b/x/staking/keeper/slash_test.go @@ -115,8 +115,7 @@ func TestSlashRedelegation(t *testing.T) { // set a redelegation with an expiration timestamp beyond which the // redelegation shouldn't be slashed rd := types.NewRedelegation(addrDels[0], addrVals[0], addrVals[1], 0, - time.Unix(5, 0), sdk.NewInt64Coin(params.BondDenom, 10), sdk.NewDec(10), - sdk.NewDec(10)) + time.Unix(5, 0), sdk.NewInt64Coin(params.BondDenom, 10), sdk.NewDec(10)) keeper.SetRedelegation(ctx, rd) @@ -157,9 +156,6 @@ func TestSlashRedelegation(t *testing.T) { // initialbalance unchanged require.Equal(t, sdk.NewInt64Coin(params.BondDenom, 10), rd.Entries[0].InitialBalance) - // balance decreased - require.Equal(t, sdk.NewInt64Coin(params.BondDenom, 5), rd.Entries[0].Balance) - // shares decreased del, found = keeper.GetDelegation(ctx, addrDels[0], addrVals[1]) require.True(t, found) @@ -348,8 +344,7 @@ func TestSlashWithRedelegation(t *testing.T) { // set a redelegation rdTokens := types.TokensFromTendermintPower(6) rd := types.NewRedelegation(addrDels[0], addrVals[0], addrVals[1], 11, - time.Unix(0, 0), sdk.NewCoin(params.BondDenom, rdTokens), sdk.NewDecFromInt(rdTokens), - sdk.NewDecFromInt(rdTokens)) + time.Unix(0, 0), sdk.NewCoin(params.BondDenom, rdTokens), sdk.NewDecFromInt(rdTokens)) keeper.SetRedelegation(ctx, rd) // set the associated delegation @@ -372,8 +367,6 @@ func TestSlashWithRedelegation(t *testing.T) { rd, found = keeper.GetRedelegation(ctx, addrDels[0], addrVals[0], addrVals[1]) require.True(t, found) require.Len(t, rd.Entries, 1) - // balance decreased - require.Equal(t, types.TokensFromTendermintPower(3), rd.Entries[0].Balance.Amount) // read updated pool newPool := keeper.GetPool(ctx) // bonded tokens burned @@ -397,8 +390,6 @@ func TestSlashWithRedelegation(t *testing.T) { rd, found = keeper.GetRedelegation(ctx, addrDels[0], addrVals[0], addrVals[1]) require.True(t, found) require.Len(t, rd.Entries, 1) - // balance decreased, now zero - require.Equal(t, sdk.NewInt(0), rd.Entries[0].Balance.Amount) // read updated pool newPool = keeper.GetPool(ctx) // seven bonded tokens burned @@ -419,8 +410,6 @@ func TestSlashWithRedelegation(t *testing.T) { rd, found = keeper.GetRedelegation(ctx, addrDels[0], addrVals[0], addrVals[1]) require.True(t, found) require.Len(t, rd.Entries, 1) - // balance still zero - require.Equal(t, sdk.NewInt(0), rd.Entries[0].Balance.Amount) // read updated pool newPool = keeper.GetPool(ctx) // four more bonded tokens burned @@ -444,8 +433,6 @@ func TestSlashWithRedelegation(t *testing.T) { rd, found = keeper.GetRedelegation(ctx, addrDels[0], addrVals[0], addrVals[1]) require.True(t, found) require.Len(t, rd.Entries, 1) - // balance still zero - require.Equal(t, sdk.NewInt(0), rd.Entries[0].Balance.Amount) // read updated pool newPool = keeper.GetPool(ctx) // no more bonded tokens burned @@ -466,7 +453,7 @@ func TestSlashBoth(t *testing.T) { rdATokens := types.TokensFromTendermintPower(6) rdA := types.NewRedelegation(addrDels[0], addrVals[0], addrVals[1], 11, time.Unix(0, 0), sdk.NewCoin(params.BondDenom, rdATokens), - sdk.NewDecFromInt(rdATokens), sdk.NewDecFromInt(rdATokens)) + sdk.NewDecFromInt(rdATokens)) keeper.SetRedelegation(ctx, rdA) // set the associated delegation @@ -492,8 +479,6 @@ func TestSlashBoth(t *testing.T) { rdA, found = keeper.GetRedelegation(ctx, addrDels[0], addrVals[0], addrVals[1]) require.True(t, found) require.Len(t, rdA.Entries, 1) - // balance decreased - require.Equal(t, types.TokensFromTendermintPower(3), rdA.Entries[0].Balance.Amount) // read updated pool newPool := keeper.GetPool(ctx) // not-bonded tokens burned diff --git a/x/staking/keeper/test_common.go b/x/staking/keeper/test_common.go index d01f307b7b67..9036bddd751f 100644 --- a/x/staking/keeper/test_common.go +++ b/x/staking/keeper/test_common.go @@ -49,7 +49,7 @@ var ( // intended to be used with require/assert: require.True(ValEq(...)) func ValEq(t *testing.T, exp, got types.Validator) (*testing.T, bool, string, types.Validator, types.Validator) { - return t, exp.Equal(got), "expected:\t%v\ngot:\t\t%v", exp, got + return t, exp.TestEquivalent(got), "expected:\t%v\ngot:\t\t%v", exp, got } //_______________________________________________________________________________________ diff --git a/x/staking/keeper/val_state_change.go b/x/staking/keeper/val_state_change.go index f6399abb8bec..aa3609d331ef 100644 --- a/x/staking/keeper/val_state_change.go +++ b/x/staking/keeper/val_state_change.go @@ -37,8 +37,7 @@ func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx sdk.Context) (updates []ab // Iterate over validators, highest power to lowest. iterator := sdk.KVStoreReversePrefixIterator(store, ValidatorsByPowerIndexKey) defer iterator.Close() - count := 0 - for ; iterator.Valid() && count < int(maxValidators); iterator.Next() { + for count := 0; iterator.Valid() && count < int(maxValidators); iterator.Next() { // fetch the validator valAddr := sdk.ValAddress(iterator.Value()) @@ -74,17 +73,12 @@ func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx sdk.Context) (updates []ab // calculate the new power bytes newPower := validator.TendermintPower() newPowerBytes := k.cdc.MustMarshalBinaryLengthPrefixed(newPower) + // update the validator set if power has changed if !found || !bytes.Equal(oldPowerBytes, newPowerBytes) { updates = append(updates, validator.ABCIValidatorUpdate()) - // Assert that the validator had updated its ValidatorDistInfo.FeePoolWithdrawalHeight. - // This hook is extremely useful, otherwise lazy accum bugs will be difficult to solve. - if k.hooks != nil { - k.hooks.AfterValidatorPowerDidChange(ctx, validator.ConsAddress(), valAddr) - } - - // set validator power on lookup index. + // set validator power on lookup index k.SetLastValidatorPower(ctx, valAddr, newPower) } @@ -97,7 +91,7 @@ func (k Keeper) ApplyAndReturnValidatorSetUpdates(ctx sdk.Context) (updates []ab } // sort the no-longer-bonded validators - noLongerBonded := k.sortNoLongerBonded(last) + noLongerBonded := sortNoLongerBonded(last) // iterate through the sorted no-longer-bonded validators for _, valAddrBytes := range noLongerBonded { @@ -179,10 +173,9 @@ func (k Keeper) unjailValidator(ctx sdk.Context, validator types.Validator) { // perform all the store operations for when a validator status becomes bonded func (k Keeper) bondValidator(ctx sdk.Context, validator types.Validator) types.Validator { + // delete the validator by power index, as the key will change k.DeleteValidatorByPowerIndex(ctx, validator) - validator.BondHeight = ctx.BlockHeight() - // set the status pool := k.GetPool(ctx) validator, pool = validator.UpdateStatus(pool, sdk.Bonded) @@ -203,11 +196,12 @@ func (k Keeper) bondValidator(ctx sdk.Context, validator types.Validator) types. return validator } -// perform all the store operations for when a validator status begins unbonding +// perform all the store operations for when a validator begins unbonding func (k Keeper) beginUnbondingValidator(ctx sdk.Context, validator types.Validator) types.Validator { params := k.GetParams(ctx) + // delete the validator by power index, as the key will change k.DeleteValidatorByPowerIndex(ctx, validator) // sanity check @@ -220,6 +214,7 @@ func (k Keeper) beginUnbondingValidator(ctx sdk.Context, validator types.Validat validator, pool = validator.UpdateStatus(pool, sdk.Unbonding) k.SetPool(ctx, pool) + // set the unbonding completion time and completion height appropriately validator.UnbondingCompletionTime = ctx.BlockHeader().Time.Add(params.UnbondingTime) validator.UnbondingHeight = ctx.BlockHeader().Height @@ -256,9 +251,12 @@ func (k Keeper) getLastValidatorsByAddr(ctx sdk.Context) validatorsByAddr { store := ctx.KVStore(k.storeKey) iterator := sdk.KVStorePrefixIterator(store, LastValidatorPowerKey) defer iterator.Close() + // iterate over the last validator set index for ; iterator.Valid(); iterator.Next() { var valAddr [sdk.AddrLen]byte + // extract the validator address from the key (prefix is 1-byte) copy(valAddr[:], iterator.Key()[1:]) + // power bytes is just the value powerBytes := iterator.Value() last[valAddr] = make([]byte, len(powerBytes)) copy(last[valAddr][:], powerBytes[:]) @@ -268,7 +266,7 @@ func (k Keeper) getLastValidatorsByAddr(ctx sdk.Context) validatorsByAddr { // given a map of remaining validators to previous bonded power // returns the list of validators to be unbonded, sorted by operator address -func (k Keeper) sortNoLongerBonded(last validatorsByAddr) [][]byte { +func sortNoLongerBonded(last validatorsByAddr) [][]byte { // sort the map keys for determinism noLongerBonded := make([][]byte, len(last)) index := 0 @@ -280,6 +278,7 @@ func (k Keeper) sortNoLongerBonded(last validatorsByAddr) [][]byte { } // sorted by address - order doesn't matter sort.SliceStable(noLongerBonded, func(i, j int) bool { + // -1 means strictly less than return bytes.Compare(noLongerBonded[i], noLongerBonded[j]) == -1 }) return noLongerBonded diff --git a/x/staking/keeper/validator.go b/x/staking/keeper/validator.go index 8e6330cff39b..f559b9e86e80 100644 --- a/x/staking/keeper/validator.go +++ b/x/staking/keeper/validator.go @@ -84,8 +84,6 @@ func (k Keeper) mustGetValidatorByConsAddr(ctx sdk.Context, consAddr sdk.ConsAdd return validator } -//___________________________________________________________________________ - // set the main record holding validator details func (k Keeper) SetValidator(ctx sdk.Context, validator types.Validator) { store := ctx.KVStore(k.storeKey) @@ -122,8 +120,6 @@ func (k Keeper) SetNewValidatorByPowerIndex(ctx sdk.Context, validator types.Val store.Set(GetValidatorsByPowerIndexKey(validator), validator.OperatorAddr) } -//___________________________________________________________________________ - // Update the tokens of an existing validator, update the validators power index key func (k Keeper) AddValidatorTokensAndShares(ctx sdk.Context, validator types.Validator, tokensToAdd sdk.Int) (valOut types.Validator, addedShares sdk.Dec) { @@ -190,16 +186,14 @@ func (k Keeper) RemoveValidator(ctx sdk.Context, address sdk.ValAddress) { if !found { return } + if validator.Status != sdk.Unbonded { panic("Cannot call RemoveValidator on bonded or unbonding validators") } - // if any tokens remain, remove from pool (burning the tokens). - // this happens if shares are zero but tokens are not. - // TODO: Remove once https://github.com/cosmos/cosmos-sdk/pull/2958 is merged - pool := k.GetPool(ctx) - pool.NotBondedTokens = pool.NotBondedTokens.Sub(validator.Tokens) - k.SetPool(ctx, pool) + if validator.Tokens.GT(sdk.ZeroInt()) { + panic("validator being removed should never have positive tokens") + } // delete the old validator record store := ctx.KVStore(k.storeKey) @@ -214,7 +208,6 @@ func (k Keeper) RemoveValidator(ctx sdk.Context, address sdk.ValAddress) { } -//___________________________________________________________________________ // get groups of validators // get the set of all validators with no limits, used during genesis dump @@ -392,7 +385,7 @@ func (k Keeper) UnbondAllMatureValidatorQueue(ctx sdk.Context) { for _, valAddr := range timeslice { val, found := k.GetValidator(ctx, valAddr) if !found { - continue + panic("validator in the unbonding queue was not found") } if val.GetStatus() != sdk.Unbonding { panic("unexpected validator in unbonding queue, status was not unbonding") diff --git a/x/staking/keeper/validator_test.go b/x/staking/keeper/validator_test.go index 13b7a0836d19..9f0a01959c28 100644 --- a/x/staking/keeper/validator_test.go +++ b/x/staking/keeper/validator_test.go @@ -132,7 +132,6 @@ func TestUpdateBondedValidatorsDecreaseCliff(t *testing.T) { for i := 0; i < len(validators); i++ { moniker := fmt.Sprintf("val#%d", int64(i)) val := types.NewValidator(sdk.ValAddress(Addrs[i]), PKs[i], types.Description{Moniker: moniker}) - val.BondHeight = int64(i) delTokens := types.TokensFromTendermintPower(int64((i + 1) * 10)) val, pool, _ = val.AddTokensFromDel(pool, delTokens) @@ -282,7 +281,8 @@ func TestValidatorBasics(t *testing.T) { // remove a record validators[1].Status = sdk.Unbonded // First must set to Unbonded. - keeper.SetValidator(ctx, validators[1]) // ... + validators[1].Tokens = sdk.ZeroInt() // ...remove all tokens + keeper.SetValidator(ctx, validators[1]) // ...set the validator keeper.RemoveValidator(ctx, validators[1].OperatorAddr) // Now it can be removed. _, found = keeper.GetValidator(ctx, addrVals[1]) require.False(t, found) @@ -341,8 +341,6 @@ func GetValidatorSortingUnmixed(t *testing.T) { require.Equal(t, len(resValidators), n) assert.True(ValEq(t, validators[3], resValidators[0])) assert.True(ValEq(t, validators[4], resValidators[1])) - require.Equal(t, int64(0), resValidators[0].BondHeight, "%v", resValidators) - require.Equal(t, int64(0), resValidators[1].BondHeight, "%v", resValidators) // no change in voting power - no change in sort ctx = ctx.WithBlockHeight(20) @@ -511,9 +509,8 @@ func TestGetValidatorsEdgeCases(t *testing.T) { require.Equal(t, nMax, uint16(len(resValidators))) assert.True(ValEq(t, validators[0], resValidators[0])) assert.True(ValEq(t, validators[2], resValidators[1])) - validator, exists := keeper.GetValidator(ctx, validators[3].OperatorAddr) - require.Equal(t, exists, true) - require.Equal(t, int64(40), validator.BondHeight) + _, exists := keeper.GetValidator(ctx, validators[3].OperatorAddr) + require.True(t, exists) } func TestValidatorBondHeight(t *testing.T) { diff --git a/x/staking/types/commission.go b/x/staking/types/commission.go index 4baeea799c6b..89c3560394b0 100644 --- a/x/staking/types/commission.go +++ b/x/staking/types/commission.go @@ -7,21 +7,23 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) -// Commission defines a commission parameters for a given validator. -type Commission struct { - Rate sdk.Dec `json:"rate"` // the commission rate charged to delegators - MaxRate sdk.Dec `json:"max_rate"` // maximum commission rate which this validator can ever charge - MaxChangeRate sdk.Dec `json:"max_change_rate"` // maximum daily increase of the validator commission - UpdateTime time.Time `json:"update_time"` // the last time the commission rate was changed -} +type ( + // Commission defines a commission parameters for a given validator. + Commission struct { + Rate sdk.Dec `json:"rate"` // the commission rate charged to delegators, as a fraction + MaxRate sdk.Dec `json:"max_rate"` // maximum commission rate which this validator can ever charge, as a fraction + MaxChangeRate sdk.Dec `json:"max_change_rate"` // maximum daily increase of the validator commission, as a fraction + UpdateTime time.Time `json:"update_time"` // the last time the commission rate was changed + } -// CommissionMsg defines a commission message to be used for creating a -// validator. -type CommissionMsg struct { - Rate sdk.Dec `json:"rate"` // the commission rate charged to delegators - MaxRate sdk.Dec `json:"max_rate"` // maximum commission rate which validator can ever charge - MaxChangeRate sdk.Dec `json:"max_change_rate"` // maximum daily increase of the validator commission -} + // CommissionMsg defines a commission message to be used for creating a + // validator. + CommissionMsg struct { + Rate sdk.Dec `json:"rate"` // the commission rate charged to delegators, as a fraction + MaxRate sdk.Dec `json:"max_rate"` // maximum commission rate which validator can ever charge, as a fraction + MaxChangeRate sdk.Dec `json:"max_change_rate"` // maximum daily increase of the validator commission, as a fraction + } +) // NewCommissionMsg returns an initialized validator commission message. func NewCommissionMsg(rate, maxRate, maxChangeRate sdk.Dec) CommissionMsg { @@ -78,7 +80,7 @@ func (c Commission) Validate() sdk.Error { return ErrCommissionNegative(DefaultCodespace) case c.MaxRate.GT(sdk.OneDec()): - // max rate cannot be greater than 100% + // max rate cannot be greater than 1 return ErrCommissionHuge(DefaultCodespace) case c.Rate.LT(sdk.ZeroDec()): @@ -117,6 +119,7 @@ func (c Commission) ValidateNewRate(newRate sdk.Dec, blockTime time.Time) sdk.Er // new rate cannot be greater than the max rate return ErrCommissionGTMaxRate(DefaultCodespace) + // TODO: why do we need an absolute value, do we care if the validator decreases their rate rapidly? case newRate.Sub(c.Rate).Abs().GT(c.MaxChangeRate): // new rate % points change cannot be greater than the max change rate return ErrCommissionGTMaxChangeRate(DefaultCodespace) diff --git a/x/staking/types/delegation.go b/x/staking/types/delegation.go index e46f89624633..8ffe319123d2 100644 --- a/x/staking/types/delegation.go +++ b/x/staking/types/delegation.go @@ -27,11 +27,9 @@ type DVVTriplet struct { ValidatorDstAddr sdk.ValAddress } -//_______________________________________________________________________ - -// Delegation represents the bond with tokens held by an account. It is +// Delegation represents the bond with tokens held by an account. It is // owned by one delegator, and is associated with the voting power of one -// pubKey. +// validator. type Delegation struct { DelegatorAddr sdk.AccAddress `json:"delegator_addr"` ValidatorAddr sdk.ValAddress `json:"validator_addr"` @@ -103,10 +101,8 @@ func (d Delegations) String() (out string) { return strings.TrimSpace(out) } -//________________________________________________________________________ - -// UnbondingDelegation reflects a delegation's passive unbonding queue. -// it may hold multiple entries between the same delegator/validator +// UnbondingDelegation stores all of a single delegator's unbonding bonds +// for a single validator in an time-ordered list type UnbondingDelegation struct { DelegatorAddr sdk.AccAddress `json:"delegator_addr"` // delegator ValidatorAddr sdk.ValAddress `json:"validator_addr"` // validator unbonding from operator addr @@ -116,7 +112,7 @@ type UnbondingDelegation struct { // UnbondingDelegationEntry - entry to an UnbondingDelegation type UnbondingDelegationEntry struct { CreationHeight int64 `json:"creation_height"` // height which the unbonding took place - CompletionTime time.Time `json:"completion_time"` // unix time for unbonding completion + CompletionTime time.Time `json:"completion_time"` // time at which the unbonding delegation will complete InitialBalance sdk.Coin `json:"initial_balance"` // atoms initially scheduled to receive at completion Balance sdk.Coin `json:"balance"` // atoms to receive at completion } @@ -185,6 +181,7 @@ func UnmarshalUBD(cdc *codec.Codec, value []byte) (ubd UnbondingDelegation, err } // nolint +// inefficient but only used in testing func (d UnbondingDelegation) Equal(d2 UnbondingDelegation) bool { bz1 := MsgCdc.MustMarshalBinaryLengthPrefixed(&d) bz2 := MsgCdc.MustMarshalBinaryLengthPrefixed(&d2) @@ -217,7 +214,9 @@ func (ubds UnbondingDelegations) String() (out string) { return strings.TrimSpace(out) } -// Redelegation reflects a delegation's passive re-delegation queue. +// Redelegation contains the list of a particular delegator's +// redelegating bonds from a particular source validator to a +// particular destination validator type Redelegation struct { DelegatorAddr sdk.AccAddress `json:"delegator_addr"` // delegator ValidatorSrcAddr sdk.ValAddress `json:"validator_src_addr"` // validator redelegation source operator addr @@ -226,12 +225,11 @@ type Redelegation struct { } // RedelegationEntry - entry to a Redelegation +// TODO: Why do we need to store the initial balance as `sdk.Coin` instead of just the amount type RedelegationEntry struct { - CreationHeight int64 `json:"creation_height"` // height which the redelegation took place - CompletionTime time.Time `json:"completion_time"` // unix time for redelegation completion + CreationHeight int64 `json:"creation_height"` // height at which the redelegation took place + CompletionTime time.Time `json:"completion_time"` // time at which the redelegation will complete InitialBalance sdk.Coin `json:"initial_balance"` // initial balance when redelegation started - Balance sdk.Coin `json:"balance"` // current balance (current value held in destination validator) - SharesSrc sdk.Dec `json:"shares_src"` // amount of source-validator shares removed by redelegation SharesDst sdk.Dec `json:"shares_dst"` // amount of destination-validator shares created by redelegation } @@ -239,10 +237,10 @@ type RedelegationEntry struct { func NewRedelegation(delegatorAddr sdk.AccAddress, validatorSrcAddr, validatorDstAddr sdk.ValAddress, creationHeight int64, minTime time.Time, balance sdk.Coin, - sharesSrc, sharesDst sdk.Dec) Redelegation { + sharesDst sdk.Dec) Redelegation { entry := NewRedelegationEntry(creationHeight, - minTime, balance, sharesSrc, sharesDst) + minTime, balance, sharesDst) return Redelegation{ DelegatorAddr: delegatorAddr, @@ -255,14 +253,12 @@ func NewRedelegation(delegatorAddr sdk.AccAddress, validatorSrcAddr, // NewRedelegation - create a new redelegation object func NewRedelegationEntry(creationHeight int64, completionTime time.Time, balance sdk.Coin, - sharesSrc, sharesDst sdk.Dec) RedelegationEntry { + sharesDst sdk.Dec) RedelegationEntry { return RedelegationEntry{ CreationHeight: creationHeight, CompletionTime: completionTime, InitialBalance: balance, - Balance: balance, - SharesSrc: sharesSrc, SharesDst: sharesDst, } } @@ -275,9 +271,9 @@ func (e RedelegationEntry) IsMature(currentTime time.Time) bool { // AddEntry - append entry to the unbonding delegation func (d *Redelegation) AddEntry(creationHeight int64, minTime time.Time, balance sdk.Coin, - sharesSrc, sharesDst sdk.Dec) { + sharesDst sdk.Dec) { - entry := NewRedelegationEntry(creationHeight, minTime, balance, sharesSrc, sharesDst) + entry := NewRedelegationEntry(creationHeight, minTime, balance, sharesDst) d.Entries = append(d.Entries, entry) } @@ -307,6 +303,7 @@ func UnmarshalRED(cdc *codec.Codec, value []byte) (red Redelegation, err error) } // nolint +// inefficient but only used in tests func (d Redelegation) Equal(d2 Redelegation) bool { bz1 := MsgCdc.MustMarshalBinaryLengthPrefixed(&d) bz2 := MsgCdc.MustMarshalBinaryLengthPrefixed(&d2) @@ -324,9 +321,8 @@ func (d Redelegation) String() string { out += fmt.Sprintf(` Redelegation %d: Creation height: %v Min time to unbond (unix): %v - Source shares: %s Dest Shares: %s`, i, entry.CreationHeight, - entry.CompletionTime, entry.SharesSrc, entry.SharesDst) + entry.CompletionTime, entry.SharesDst) } return out } diff --git a/x/staking/types/delegation_test.go b/x/staking/types/delegation_test.go index a2a637685a06..ce3b9936a2e2 100644 --- a/x/staking/types/delegation_test.go +++ b/x/staking/types/delegation_test.go @@ -53,16 +53,15 @@ func TestUnbondingDelegationString(t *testing.T) { func TestRedelegationEqual(t *testing.T) { r1 := NewRedelegation(sdk.AccAddress(addr1), addr2, addr3, 0, time.Unix(0, 0), sdk.NewInt64Coin(DefaultBondDenom, 0), - sdk.NewDec(0), sdk.NewDec(0)) + sdk.NewDec(0)) r2 := NewRedelegation(sdk.AccAddress(addr1), addr2, addr3, 0, time.Unix(0, 0), sdk.NewInt64Coin(DefaultBondDenom, 0), - sdk.NewDec(0), sdk.NewDec(0)) + sdk.NewDec(0)) ok := r1.Equal(r2) require.True(t, ok) r2.Entries[0].SharesDst = sdk.NewDec(10) - r2.Entries[0].SharesSrc = sdk.NewDec(20) r2.Entries[0].CompletionTime = time.Unix(20*20*2, 0) ok = r1.Equal(r2) @@ -72,7 +71,7 @@ func TestRedelegationEqual(t *testing.T) { func TestRedelegationString(t *testing.T) { r := NewRedelegation(sdk.AccAddress(addr1), addr2, addr3, 0, time.Unix(0, 0), sdk.NewInt64Coin(DefaultBondDenom, 0), - sdk.NewDec(10), sdk.NewDec(20)) + sdk.NewDec(10)) require.NotEmpty(t, r.String()) } diff --git a/x/staking/types/errors.go b/x/staking/types/errors.go index 337c9d043452..37a80f24c908 100644 --- a/x/staking/types/errors.go +++ b/x/staking/types/errors.go @@ -12,7 +12,7 @@ import ( type CodeType = sdk.CodeType const ( - DefaultCodespace sdk.CodespaceType = "staking" + DefaultCodespace sdk.CodespaceType = ModuleName CodeInvalidValidator CodeType = 101 CodeInvalidDelegation CodeType = 102 diff --git a/x/staking/types/keys.go b/x/staking/types/keys.go index aa1b92d46def..bba59695c05c 100644 --- a/x/staking/types/keys.go +++ b/x/staking/types/keys.go @@ -1,15 +1,18 @@ package types const ( + // ModuleName is the name of the staking module + ModuleName = "staking" + // StoreKey is the string store representation - StoreKey = "staking" + StoreKey = ModuleName // TStoreKey is the string transient store representation - TStoreKey = "transient_staking" + TStoreKey = "transient_" + ModuleName // QuerierRoute is the querier route for the staking module - QuerierRoute = "staking" + QuerierRoute = ModuleName // RouterKey is the msg router key for the staking module - RouterKey = "staking" + RouterKey = ModuleName ) diff --git a/x/staking/types/msg.go b/x/staking/types/msg.go index 9ef062104675..2437d3ea3806 100644 --- a/x/staking/types/msg.go +++ b/x/staking/types/msg.go @@ -21,6 +21,7 @@ var ( //______________________________________________________________________ // MsgCreateValidator - struct for bonding transactions +// TODO: Why does this need to contain a denomination in `Value` type MsgCreateValidator struct { Description Description `json:"description"` Commission CommissionMsg `json:"commission"` @@ -117,13 +118,14 @@ func (msg MsgCreateValidator) GetSignBytes() []byte { // quick validity check func (msg MsgCreateValidator) ValidateBasic() sdk.Error { + // note that unmarshaling from bech32 ensures either empty or valid if msg.DelegatorAddr == nil { return ErrNilDelegatorAddr(DefaultCodespace) } if msg.ValidatorAddr == nil { return ErrNilValidatorAddr(DefaultCodespace) } - if !(msg.Value.Amount.GT(sdk.ZeroInt())) { + if msg.Value.Amount.LTE(sdk.ZeroInt()) { return ErrBadDelegationAmount(DefaultCodespace) } if msg.Description == (Description{}) { @@ -136,8 +138,6 @@ func (msg MsgCreateValidator) ValidateBasic() sdk.Error { return nil } -//______________________________________________________________________ - // MsgEditValidator - struct for editing a validator type MsgEditValidator struct { Description @@ -182,12 +182,17 @@ func (msg MsgEditValidator) ValidateBasic() sdk.Error { return sdk.NewError(DefaultCodespace, CodeInvalidInput, "transaction must include some information to modify") } + if msg.CommissionRate != nil { + if msg.CommissionRate.GT(sdk.OneDec()) || msg.CommissionRate.LT(sdk.ZeroDec()) { + return sdk.NewError(DefaultCodespace, CodeInvalidInput, "commission rate must be between 0 and 1, inclusive") + } + } + return nil } -//______________________________________________________________________ - // MsgDelegate - struct for bonding transactions +// TODO: Why do we need to store the denomination in `Value` type MsgDelegate struct { DelegatorAddr sdk.AccAddress `json:"delegator_addr"` ValidatorAddr sdk.ValAddress `json:"validator_addr"` @@ -223,7 +228,7 @@ func (msg MsgDelegate) ValidateBasic() sdk.Error { if msg.ValidatorAddr == nil { return ErrNilValidatorAddr(DefaultCodespace) } - if !(msg.Value.Amount.GT(sdk.ZeroInt())) { + if msg.Value.Amount.LTE(sdk.ZeroInt()) { return ErrBadDelegationAmount(DefaultCodespace) } return nil @@ -280,8 +285,6 @@ func (msg MsgBeginRedelegate) ValidateBasic() sdk.Error { return nil } -//______________________________________________________________________ - // MsgUndelegate - struct for unbonding transactions type MsgUndelegate struct { DelegatorAddr sdk.AccAddress `json:"delegator_addr"` diff --git a/x/staking/types/params.go b/x/staking/types/params.go index 182abfce62d5..759efab52ccf 100644 --- a/x/staking/types/params.go +++ b/x/staking/types/params.go @@ -10,14 +10,22 @@ import ( ) const ( - // defaultUnbondingTime reflects three weeks in seconds as the default + // DefaultUnbondingTime reflects three weeks in seconds as the default // unbonding time. - defaultUnbondingTime time.Duration = 60 * 60 * 24 * 3 * time.Second + // TODO: Justify our choice of default here. + DefaultUnbondingTime time.Duration = time.Second * 60 * 60 * 24 * 3 + + // Default maximum number of bonded validators + DefaultMaxValidators uint16 = 100 + + // Default maximum entries in a UBD/RED pair + DefaultMaxEntries uint16 = 7 // Delay, in blocks, between when validator updates are returned to Tendermint and when they are applied // For example, if this is 0, the validator set at the end of a block will sign the next block, or // if this is 1, the validator set at the end of a block will sign the block after the next. // Constant as this should not change without a hard fork. + // TODO: Link to some Tendermint docs, this is very unobvious. ValidatorUpdateDelay int64 = 1 // Default bondable coin denomination @@ -37,9 +45,10 @@ var _ params.ParamSet = (*Params)(nil) // Params defines the high level settings for staking type Params struct { UnbondingTime time.Duration `json:"unbonding_time"` // time duration of unbonding - MaxValidators uint16 `json:"max_validators"` // maximum number of validators + MaxValidators uint16 `json:"max_validators"` // maximum number of validators (max uint16 = 65535) MaxEntries uint16 `json:"max_entries"` // max entries for either unbonding delegation or redelegation (per pair/trio) - BondDenom string `json:"bond_denom"` // bondable coin denomination + // note: we need to be a bit careful about potential overflow here, since this is user-determined + BondDenom string `json:"bond_denom"` // bondable coin denomination } func NewParams(unbondingTime time.Duration, maxValidators, maxEntries uint16, @@ -64,6 +73,7 @@ func (p *Params) ParamSetPairs() params.ParamSetPairs { } // Equal returns a boolean determining if two Param types are identical. +// TODO: This is slower than comparing struct fields directly func (p Params) Equal(p2 Params) bool { bz1 := MsgCdc.MustMarshalBinaryLengthPrefixed(&p) bz2 := MsgCdc.MustMarshalBinaryLengthPrefixed(&p2) @@ -72,7 +82,7 @@ func (p Params) Equal(p2 Params) bool { // DefaultParams returns a default set of parameters. func DefaultParams() Params { - return NewParams(defaultUnbondingTime, 100, 7, DefaultBondDenom) + return NewParams(DefaultUnbondingTime, DefaultMaxValidators, DefaultMaxEntries, DefaultBondDenom) } // String returns a human readable string representation of the parameters. @@ -102,3 +112,14 @@ func UnmarshalParams(cdc *codec.Codec, value []byte) (params Params, err error) } return } + +// validate a set of params +func (p Params) Validate() error { + if p.BondDenom == "" { + return fmt.Errorf("staking parameter BondDenom can't be an empty string") + } + if p.MaxValidators == 0 { + return fmt.Errorf("staking parameter MaxValidators must be a positive integer") + } + return nil +} diff --git a/x/staking/types/pool.go b/x/staking/types/pool.go index 840e4f609e65..6f108e8b36be 100644 --- a/x/staking/types/pool.go +++ b/x/staking/types/pool.go @@ -8,13 +8,14 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) -// Pool - dynamic parameters of the current state +// Pool - tracking bonded and not-bonded token supply of the bond denomination type Pool struct { - NotBondedTokens sdk.Int `json:"not_bonded_tokens"` // tokens which are not bonded in a validator - BondedTokens sdk.Int `json:"bonded_tokens"` // reserve of bonded tokens + NotBondedTokens sdk.Int `json:"not_bonded_tokens"` // tokens which are not bonded to a validator (unbonded or unbonding) + BondedTokens sdk.Int `json:"bonded_tokens"` // tokens which are currently bonded to a validator } // nolint +// TODO: This is slower than comparing struct fields directly func (p Pool) Equal(p2 Pool) bool { bz1 := MsgCdc.MustMarshalBinaryLengthPrefixed(&p) bz2 := MsgCdc.MustMarshalBinaryLengthPrefixed(&p2) @@ -29,16 +30,12 @@ func InitialPool() Pool { } } -//____________________________________________________________________ - // Sum total of all staking tokens in the pool func (p Pool) TokenSupply() sdk.Int { return p.NotBondedTokens.Add(p.BondedTokens) } -//____________________________________________________________________ - -// get the bond ratio of the global state +// Get the fraction of the staking token which is currently bonded func (p Pool) BondedRatio() sdk.Dec { supply := p.TokenSupply() if supply.IsPositive() { @@ -48,8 +45,6 @@ func (p Pool) BondedRatio() sdk.Dec { return sdk.ZeroDec() } -//_______________________________________________________________________ - func (p Pool) notBondedTokensToBonded(bondedTokens sdk.Int) Pool { p.BondedTokens = p.BondedTokens.Add(bondedTokens) p.NotBondedTokens = p.NotBondedTokens.Sub(bondedTokens) diff --git a/x/staking/types/validator.go b/x/staking/types/validator.go index 03f253d2cd71..21145081f237 100644 --- a/x/staking/types/validator.go +++ b/x/staking/types/validator.go @@ -15,29 +15,33 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) +// nolint +const ( + // TODO: Why can't we just have one string description which can be JSON by convention + MaxMonikerLength = 70 + MaxIdentityLength = 3000 + MaxWebsiteLength = 140 + MaxDetailsLength = 280 +) + // Validator defines the total amount of bond shares and their exchange rate to -// coins. Accumulation of interest is modelled as an in increase in the -// exchange rate, and slashing as a decrease. When coins are delegated to this -// validator, the validator is credited with a Delegation whose number of -// bond shares is based on the amount of coins delegated divided by the current -// exchange rate. Voting power can be calculated as total bonds multiplied by -// exchange rate. +// coins. Slashing results in a decrease in the exchange rate, allowing correct +// calculation of future undelegations without iterating over delegators. +// When coins are delegated to this validator, the validator is credited with a +// delegation whose number of bond shares is based on the amount of coins delegated +// divided by the current exchange rate. Voting power can be calculated as total +// bonded shares multiplied by exchange rate. type Validator struct { - OperatorAddr sdk.ValAddress `json:"operator_address"` // address of the validator's operator; bech encoded in JSON - ConsPubKey crypto.PubKey `json:"consensus_pubkey"` // the consensus public key of the validator; bech encoded in JSON - Jailed bool `json:"jailed"` // has the validator been jailed from bonded status? - - Status sdk.BondStatus `json:"status"` // validator status (bonded/unbonding/unbonded) - Tokens sdk.Int `json:"tokens"` // delegated tokens (incl. self-delegation) - DelegatorShares sdk.Dec `json:"delegator_shares"` // total shares issued to a validator's delegators - - Description Description `json:"description"` // description terms for the validator - BondHeight int64 `json:"bond_height"` // earliest height as a bonded validator - - UnbondingHeight int64 `json:"unbonding_height"` // if unbonding, height at which this validator has begun unbonding - UnbondingCompletionTime time.Time `json:"unbonding_time"` // if unbonding, min time for the validator to complete unbonding - - Commission Commission `json:"commission"` // commission parameters + OperatorAddr sdk.ValAddress `json:"operator_address"` // address of the validator's operator; bech encoded in JSON + ConsPubKey crypto.PubKey `json:"consensus_pubkey"` // the consensus public key of the validator; bech encoded in JSON + Jailed bool `json:"jailed"` // has the validator been jailed from bonded status? + Status sdk.BondStatus `json:"status"` // validator status (bonded/unbonding/unbonded) + Tokens sdk.Int `json:"tokens"` // delegated tokens (incl. self-delegation) + DelegatorShares sdk.Dec `json:"delegator_shares"` // total shares issued to a validator's delegators + Description Description `json:"description"` // description terms for the validator + UnbondingHeight int64 `json:"unbonding_height"` // if unbonding, height at which this validator has begun unbonding + UnbondingCompletionTime time.Time `json:"unbonding_time"` // if unbonding, min time for the validator to complete unbonding + Commission Commission `json:"commission"` // commission parameters } // Validators is a collection of Validator @@ -60,7 +64,6 @@ func NewValidator(operator sdk.ValAddress, pubKey crypto.PubKey, description Des Tokens: sdk.ZeroInt(), DelegatorShares: sdk.ZeroDec(), Description: description, - BondHeight: int64(0), UnbondingHeight: int64(0), UnbondingCompletionTime: time.Unix(0, 0).UTC(), Commission: NewCommission(sdk.ZeroDec(), sdk.ZeroDec(), sdk.ZeroDec()), @@ -101,34 +104,26 @@ func (v Validator) String() string { Tokens: %s Delegator Shares: %s Description: %s - Bond Height: %d Unbonding Height: %d Unbonding Completion Time: %v Commission: %s`, v.OperatorAddr, bechConsPubKey, v.Jailed, sdk.BondStatusToString(v.Status), v.Tokens, - v.DelegatorShares, v.Description, v.BondHeight, + v.DelegatorShares, v.Description, v.UnbondingHeight, v.UnbondingCompletionTime, v.Commission) } -//___________________________________________________________________ - // this is a helper struct used for JSON de- and encoding only type bechValidator struct { - OperatorAddr sdk.ValAddress `json:"operator_address"` // the bech32 address of the validator's operator - ConsPubKey string `json:"consensus_pubkey"` // the bech32 consensus public key of the validator - Jailed bool `json:"jailed"` // has the validator been jailed from bonded status? - - Status sdk.BondStatus `json:"status"` // validator status (bonded/unbonding/unbonded) - Tokens sdk.Int `json:"tokens"` // delegated tokens (incl. self-delegation) - DelegatorShares sdk.Dec `json:"delegator_shares"` // total shares issued to a validator's delegators - - Description Description `json:"description"` // description terms for the validator - BondHeight int64 `json:"bond_height"` // earliest height as a bonded validator - - UnbondingHeight int64 `json:"unbonding_height"` // if unbonding, height at which this validator has begun unbonding - UnbondingCompletionTime time.Time `json:"unbonding_time"` // if unbonding, min time for the validator to complete unbonding - - Commission Commission `json:"commission"` // commission parameters + OperatorAddr sdk.ValAddress `json:"operator_address"` // the bech32 address of the validator's operator + ConsPubKey string `json:"consensus_pubkey"` // the bech32 consensus public key of the validator + Jailed bool `json:"jailed"` // has the validator been jailed from bonded status? + Status sdk.BondStatus `json:"status"` // validator status (bonded/unbonding/unbonded) + Tokens sdk.Int `json:"tokens"` // delegated tokens (incl. self-delegation) + DelegatorShares sdk.Dec `json:"delegator_shares"` // total shares issued to a validator's delegators + Description Description `json:"description"` // description terms for the validator + UnbondingHeight int64 `json:"unbonding_height"` // if unbonding, height at which this validator has begun unbonding + UnbondingCompletionTime time.Time `json:"unbonding_time"` // if unbonding, min time for the validator to complete unbonding + Commission Commission `json:"commission"` // commission parameters } // MarshalJSON marshals the validator to JSON using Bech32 @@ -146,7 +141,6 @@ func (v Validator) MarshalJSON() ([]byte, error) { Tokens: v.Tokens, DelegatorShares: v.DelegatorShares, Description: v.Description, - BondHeight: v.BondHeight, UnbondingHeight: v.UnbondingHeight, UnbondingCompletionTime: v.UnbondingCompletionTime, Commission: v.Commission, @@ -171,7 +165,6 @@ func (v *Validator) UnmarshalJSON(data []byte) error { Status: bv.Status, DelegatorShares: bv.DelegatorShares, Description: bv.Description, - BondHeight: bv.BondHeight, UnbondingHeight: bv.UnbondingHeight, UnbondingCompletionTime: bv.UnbondingCompletionTime, Commission: bv.Commission, @@ -179,10 +172,8 @@ func (v *Validator) UnmarshalJSON(data []byte) error { return nil } -//___________________________________________________________________ - // only the vitals -func (v Validator) Equal(v2 Validator) bool { +func (v Validator) TestEquivalent(v2 Validator) bool { return v.ConsPubKey.Equals(v2.ConsPubKey) && bytes.Equal(v.OperatorAddr, v2.OperatorAddr) && v.Status.Equal(v2.Status) && @@ -244,23 +235,23 @@ func (d Description) UpdateDescription(d2 Description) (Description, sdk.Error) // EnsureLength ensures the length of a validator's description. func (d Description) EnsureLength() (Description, sdk.Error) { - if len(d.Moniker) > 70 { - return d, ErrDescriptionLength(DefaultCodespace, "moniker", len(d.Moniker), 70) + if len(d.Moniker) > MaxMonikerLength { + return d, ErrDescriptionLength(DefaultCodespace, "moniker", len(d.Moniker), MaxMonikerLength) } - if len(d.Identity) > 3000 { - return d, ErrDescriptionLength(DefaultCodespace, "identity", len(d.Identity), 3000) + if len(d.Identity) > MaxIdentityLength { + return d, ErrDescriptionLength(DefaultCodespace, "identity", len(d.Identity), MaxIdentityLength) } - if len(d.Website) > 140 { - return d, ErrDescriptionLength(DefaultCodespace, "website", len(d.Website), 140) + if len(d.Website) > MaxWebsiteLength { + return d, ErrDescriptionLength(DefaultCodespace, "website", len(d.Website), MaxWebsiteLength) } - if len(d.Details) > 280 { - return d, ErrDescriptionLength(DefaultCodespace, "details", len(d.Details), 280) + if len(d.Details) > MaxDetailsLength { + return d, ErrDescriptionLength(DefaultCodespace, "details", len(d.Details), MaxDetailsLength) } return d, nil } -// ABCIValidatorUpdate returns an abci.ValidatorUpdate from a staked validator type +// ABCIValidatorUpdate returns an abci.ValidatorUpdate from a staking validator type // with the full validator power func (v Validator) ABCIValidatorUpdate() abci.ValidatorUpdate { return abci.ValidatorUpdate{ @@ -269,7 +260,7 @@ func (v Validator) ABCIValidatorUpdate() abci.ValidatorUpdate { } } -// ABCIValidatorUpdateZero returns an abci.ValidatorUpdate from a staked validator type +// ABCIValidatorUpdateZero returns an abci.ValidatorUpdate from a staking validator type // with zero power used for validator updates. func (v Validator) ABCIValidatorUpdateZero() abci.ValidatorUpdate { return abci.ValidatorUpdate{ @@ -322,6 +313,7 @@ func (v Validator) RemoveTokens(pool Pool, tokens sdk.Int) (Validator, Pool) { panic(fmt.Sprintf("should not happen: only have %v tokens, trying to remove %v", v.Tokens, tokens)) } v.Tokens = v.Tokens.Sub(tokens) + // TODO: It is not obvious from the name of the function that this will happen. Either justify or move outside. if v.Status == sdk.Bonded { pool = pool.bondedTokensToNotBonded(tokens) } @@ -339,9 +331,8 @@ func (v Validator) SetInitialCommission(commission Commission) (Validator, sdk.E return v, nil } -//_________________________________________________________________________________________________________ - // AddTokensFromDel adds tokens to a validator +// CONTRACT: Tokens are assumed to have come from not-bonded pool. func (v Validator) AddTokensFromDel(pool Pool, amount sdk.Int) (Validator, Pool, sdk.Dec) { // bondedShare/delegatedShare @@ -364,6 +355,7 @@ func (v Validator) AddTokensFromDel(pool Pool, amount sdk.Int) (Validator, Pool, // RemoveDelShares removes delegator shares from a validator. // NOTE: because token fractions are left in the valiadator, // the exchange rate of future shares of this validator can increase. +// CONTRACT: Tokens are assumed to move to the not-bonded pool. func (v Validator) RemoveDelShares(pool Pool, delShares sdk.Dec) (Validator, Pool, sdk.Int) { remainingShares := v.DelegatorShares.Sub(delShares) @@ -396,6 +388,7 @@ func (v Validator) RemoveDelShares(pool Pool, delShares sdk.Dec) (Validator, Poo // UNITS: tokens/delegator-shares func (v Validator) DelegatorShareExRate() sdk.Dec { if v.DelegatorShares.IsZero() { + // the first delegation to a validator sets the exchange rate to one return sdk.OneDec() } return sdk.NewDecFromInt(v.Tokens).Quo(v.DelegatorShares) @@ -437,8 +430,6 @@ func TokensFromTendermintPower(power int64) sdk.Int { return sdk.NewInt(power).Mul(powerReduction) } -//______________________________________________________________________ - // ensure fulfills the sdk validator types var _ sdk.Validator = Validator{} @@ -454,5 +445,4 @@ func (v Validator) GetBondedTokens() sdk.Int { return v.BondedTokens() } func (v Validator) GetTendermintPower() int64 { return v.TendermintPower() } func (v Validator) GetCommission() sdk.Dec { return v.Commission.Rate } func (v Validator) GetDelegatorShares() sdk.Dec { return v.DelegatorShares } -func (v Validator) GetBondHeight() int64 { return v.BondHeight } func (v Validator) GetDelegatorShareExRate() sdk.Dec { return v.DelegatorShareExRate() } diff --git a/x/staking/types/validator_test.go b/x/staking/types/validator_test.go index 2beba8e671d4..09641ee2a5d3 100644 --- a/x/staking/types/validator_test.go +++ b/x/staking/types/validator_test.go @@ -12,16 +12,16 @@ import ( tmtypes "github.com/tendermint/tendermint/types" ) -func TestValidatorEqual(t *testing.T) { +func TestValidatorTestEquivalent(t *testing.T) { val1 := NewValidator(addr1, pk1, Description{}) val2 := NewValidator(addr1, pk1, Description{}) - ok := val1.Equal(val2) + ok := val1.TestEquivalent(val2) require.True(t, ok) val2 = NewValidator(addr2, pk2, Description{}) - ok = val1.Equal(val2) + ok = val1.TestEquivalent(val2) require.False(t, ok) }