fix: Global min Self Delegation (#134)

* fix testing and protobuf related issues * remove unnecessary changes in legacy migration tests * remove extraneous comments * add test for minselfdelegation * fix: Ensure that MsgCreateValidator's accompanying delegation is at least the provided MinSelfDelegation (#132) * add check to make sure validator object is only created if min delegation threshold is met * fix check location and use standard error type * fix logic for check * add test for new check * update test comments for better context * add checks for global min self delegation and fix corresponding tests * add migration code * fix migration code tests * fix staking client unit tests * tidy up * Apply suggestions from code review Co-authored-by: Dev Ojha <[email protected]> Co-authored-by: Aleksandr Bezobchuk <[email protected]> * apply changes suggested in review * fixed edge case error * lint++ * lint++ * Update x/staking/keeper/msg_server.go * Update x/staking/keeper/msg_server.go * Update x/staking/keeper/msg_server.go Co-authored-by: Aleksandr Bezobchuk <[email protected]> Co-authored-by: Aleksandr Bezobchuk <[email protected]> Co-authored-by: Dev Ojha <[email protected]> Co-authored-by: Aleksandr Bezobchuk <[email protected]>
cosmos · Apr 13, 2022 · 7a06fb2 · 7a06fb2
1 parent 71bb5bd
commit 7a06fb2
Show file tree

Hide file tree

Showing 14 changed files with 967 additions and 712 deletions.
diff --git a/docs/core/proto-docs.md b/docs/core/proto-docs.md
@@ -6510,6 +6510,7 @@ Params defines the parameters for the staking module.
 | `historical_entries` | [uint32](#uint32) |  | historical_entries is the number of historical entries to persist. |
 | `bond_denom` | [string](#string) |  | bond_denom defines the bondable coin denomination. |
 | `min_commission_rate` | [string](#string) |  | min_commission_rate is the chain-wide minimum commission rate that a validator can charge their delegators |
+| `min_self_delegation` | [string](#string) |  | min_self_delegation is the chain-wide minimum self delegation a validator can have |
 
 
 

diff --git a/proto/cosmos/staking/v1beta1/staking.proto b/proto/cosmos/staking/v1beta1/staking.proto
@@ -288,6 +288,12 @@ message Params {
     (gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec",
     (gogoproto.nullable)   = false
   ];
+  // min_self_delegation is the chain-wide minimum amount that a validator has to self delegate
+  string min_self_delegation = 7 [
+    (gogoproto.moretags)   = "yaml:\"min_self_delegation\"",
+    (gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int",
+    (gogoproto.nullable)   = false
+  ];
 }
 
 // DelegationResponse is equivalent to Delegation except that it contains a

diff --git a/x/staking/client/testutil/suite.go b/x/staking/client/testutil/suite.go
@@ -877,12 +877,13 @@ historical_entries: 10000
 max_entries: 7
 max_validators: 100
 min_commission_rate: "0.000000000000000000"
+min_self_delegation: "0"
 unbonding_time: 1814400s`,
 		},
 		{
 			"with json output",
 			[]string{fmt.Sprintf("--%s=json", tmcli.OutputFlag)},
-			`{"unbonding_time":"1814400s","max_validators":100,"max_entries":7,"historical_entries":10000,"bond_denom":"stake","min_commission_rate":"0.000000000000000000"}`,
+			`{"unbonding_time":"1814400s","max_validators":100,"max_entries":7,"historical_entries":10000,"bond_denom":"stake","min_commission_rate":"0.000000000000000000","min_self_delegation":"0"}`,
 		},
 	}
 	for _, tc := range testCases {

diff --git a/x/staking/keeper/migrations.go b/x/staking/keeper/migrations.go
@@ -3,6 +3,7 @@ package keeper
 import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	v043 "github.com/cosmos/cosmos-sdk/x/staking/legacy/v043"
+	v046 "github.com/cosmos/cosmos-sdk/x/staking/legacy/v046"
 )
 
 // Migrator is a struct for handling in-place store migrations.
@@ -19,3 +20,8 @@ func NewMigrator(keeper Keeper) Migrator {
 func (m Migrator) Migrate1to2(ctx sdk.Context) error {
 	return v043.MigrateStore(ctx, m.keeper.storeKey)
 }
+
+// Migrate1to2 migrates from version 2 to 3.
+func (m Migrator) Migrate2to3(ctx sdk.Context) error {
+	return v046.MigrateStore(ctx, m.keeper.paramstore)
+}
diff --git a/x/staking/keeper/msg_server.go b/x/staking/keeper/msg_server.go
@@ -56,6 +56,10 @@ func (k msgServer) CreateValidator(goCtx context.Context, msg *types.MsgCreateVa
 		)
 	}
 
+	if msg.Value.Amount.LT(msg.MinSelfDelegation) {
+		return nil, types.ErrSelfDelegationBelowMinimum
+	}
+
 	if _, err := msg.Description.EnsureLength(); err != nil {
 		return nil, err
 	}
@@ -99,6 +103,13 @@ func (k msgServer) CreateValidator(goCtx context.Context, msg *types.MsgCreateVa
 
 	validator.MinSelfDelegation = msg.MinSelfDelegation
 
+	globalMinSelfDelegation := k.MinSelfDelegation(ctx)
+
+	// minimum validator self delegation must be greater than or equal to global minimum
+	if validator.MinSelfDelegation.LT(globalMinSelfDelegation) {
+		return nil, types.ErrMinSelfDelegationBelowMinimum
+	}
+
 	k.SetValidator(ctx, validator)
 	k.SetValidatorByConsAddr(ctx, validator)
 	k.SetNewValidatorByPowerIndex(ctx, validator)
@@ -163,6 +174,13 @@ func (k msgServer) EditValidator(goCtx context.Context, msg *types.MsgEditValida
 		validator.Commission = commission
 	}
 
+	globalMinSelfDelegation := k.MinSelfDelegation(ctx)
+
+	// minimum validator self delegation must be greater than or equal to global minimum
+	if validator.MinSelfDelegation.LT(globalMinSelfDelegation) {
+		return nil, types.ErrMinSelfDelegationBelowMinimum
+	}
+
 	if msg.MinSelfDelegation != nil {
 		if !msg.MinSelfDelegation.GT(validator.MinSelfDelegation) {
 			return nil, types.ErrMinSelfDelegationDecreased

diff --git a/x/staking/keeper/msg_server_test.go b/x/staking/keeper/msg_server_test.go
@@ -25,6 +25,7 @@ func TestCreateValidatorWithLessThanMinCommission(t *testing.T) {
 	// set min commission rate to non-zero
 	params := app.StakingKeeper.GetParams(ctx)
 	params.MinCommissionRate = sdk.NewDecWithPrec(1, 2)
+	params.MinSelfDelegation = sdk.NewInt(0)
 	app.StakingKeeper.SetParams(ctx, params)
 
 	// create validator with 0% commission
@@ -51,3 +52,85 @@ func TestCreateValidatorWithLessThanMinCommission(t *testing.T) {
 	_, err = sh(ctx.WithBlockHeight(712001), msg2)
 	require.Error(t, err)
 }
+
+func TestCreateValidatorWithLessThanMinSelfDelegation(t *testing.T) {
+	PKS := simapp.CreateTestPubKeys(2)
+	valConsPk1 := PKS[0]
+	valConsPk2 := PKS[1]
+
+	app := simapp.Setup(false)
+	ctx := app.BaseApp.NewContext(false, tmproto.Header{})
+
+	addrs := simapp.AddTestAddrs(app, ctx, 3, sdk.NewInt(1234))
+
+	// set min commission rate to non-zero
+	params := app.StakingKeeper.GetParams(ctx)
+	params.MinCommissionRate = sdk.NewDecWithPrec(1, 2)
+	app.StakingKeeper.SetParams(ctx, params)
+
+	// create two validators with 0% commission and self delegations that are above and below the minimum, respectively
+	msg1, err := stakingtypes.NewMsgCreateValidator(
+		sdk.ValAddress(addrs[0]),
+		valConsPk1,
+		sdk.NewInt64Coin(sdk.DefaultBondDenom, 100),
+		stakingtypes.Description{},
+		stakingtypes.NewCommissionRates(sdk.NewDec(0), sdk.NewDecWithPrec(5, 1), sdk.NewDec(0)),
+		sdk.OneInt())
+	require.NoError(t, err)
+	msg2, err := stakingtypes.NewMsgCreateValidator(
+		sdk.ValAddress(addrs[1]),
+		valConsPk2,
+		sdk.NewInt64Coin(sdk.DefaultBondDenom, 0),
+		stakingtypes.Description{},
+		stakingtypes.NewCommissionRates(sdk.NewDec(0), sdk.NewDecWithPrec(5, 1), sdk.NewDec(0)),
+		sdk.OneInt())
+	require.NoError(t, err)
+
+	sh := staking.NewHandler(app.StakingKeeper)
+	_, err = sh(ctx, msg1)
+	require.NoError(t, err)
+	_, err = sh(ctx, msg2)
+	require.Error(t, err)
+}
+
+func TestCreateValidatorWithLessThanGlobalMinSelfDelegation(t *testing.T) {
+	PKS := simapp.CreateTestPubKeys(2)
+	valConsPk1 := PKS[0]
+	valConsPk2 := PKS[1]
+
+	app := simapp.Setup(false)
+	ctx := app.BaseApp.NewContext(false, tmproto.Header{})
+
+	addrs := simapp.AddTestAddrs(app, ctx, 3, sdk.NewInt(1234))
+
+	// set min commission rate and min self delegation to non-zero
+	params := app.StakingKeeper.GetParams(ctx)
+	params.MinCommissionRate = sdk.NewDecWithPrec(1, 2)
+	params.MinSelfDelegation = sdk.NewInt(5)
+	app.StakingKeeper.SetParams(ctx, params)
+
+	// create two validators with 0% commission and self delegations that are above and below the minimum, respectively
+	msg1, err := stakingtypes.NewMsgCreateValidator(
+		sdk.ValAddress(addrs[0]),
+		valConsPk1,
+		sdk.NewInt64Coin(sdk.DefaultBondDenom, 100),
+		stakingtypes.Description{},
+		stakingtypes.NewCommissionRates(sdk.NewDec(0), sdk.NewDecWithPrec(5, 1), sdk.NewDec(0)),
+		params.MinSelfDelegation)
+	require.NoError(t, err)
+	msg2, err := stakingtypes.NewMsgCreateValidator(
+		sdk.ValAddress(addrs[1]),
+		valConsPk2,
+		sdk.NewInt64Coin(sdk.DefaultBondDenom, 10),
+		stakingtypes.Description{},
+		stakingtypes.NewCommissionRates(sdk.NewDec(0), sdk.NewDecWithPrec(5, 1), sdk.NewDec(0)),
+		// input minimum self delegation that is less than the allowed global minimum
+		sdk.OneInt())
+	require.NoError(t, err)
+
+	sh := staking.NewHandler(app.StakingKeeper)
+	_, err = sh(ctx, msg1)
+	require.NoError(t, err)
+	_, err = sh(ctx, msg2)
+	require.Error(t, err)
+}
diff --git a/x/staking/keeper/params.go b/x/staking/keeper/params.go
@@ -53,6 +53,12 @@ func (k Keeper) MinCommissionRate(ctx sdk.Context) (res sdk.Dec) {
 	return
 }
 
+// MinSelfDelegation - Minimum validator self-delegation
+func (k Keeper) MinSelfDelegation(ctx sdk.Context) (res sdk.Int) {
+	k.paramstore.Get(ctx, types.KeyMinSelfDelegation, &res)
+	return
+}
+
 // Get all parameters as types.Params
 func (k Keeper) GetParams(ctx sdk.Context) types.Params {
 	return types.NewParams(
@@ -62,6 +68,7 @@ func (k Keeper) GetParams(ctx sdk.Context) types.Params {
 		k.HistoricalEntries(ctx),
 		k.BondDenom(ctx),
 		k.MinCommissionRate(ctx),
+		k.MinSelfDelegation(ctx),
 	)
 }
 

diff --git a/x/staking/legacy/v046/store.go b/x/staking/legacy/v046/store.go
@@ -0,0 +1,23 @@
+package v046
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
+	"github.com/cosmos/cosmos-sdk/x/staking/types"
+)
+
+// MigrateStore performs in-place store migrations from v0.43/v0.44 to v0.45.
+// The migration includes:
+//
+// - Setting the MinCommissionRate param in the paramstore
+func MigrateStore(ctx sdk.Context, paramstore paramtypes.Subspace) error {
+	migrateParamsStore(ctx, paramstore)
+
+	return nil
+}
+
+func migrateParamsStore(ctx sdk.Context, paramstore paramtypes.Subspace) {
+	DefaultMinSelfDelegation := sdk.ZeroInt()
+	paramstore.WithKeyTable(types.ParamKeyTable())
+	paramstore.Set(ctx, types.KeyMinSelfDelegation, DefaultMinSelfDelegation)
+}
diff --git a/x/staking/legacy/v046/store_test.go b/x/staking/legacy/v046/store_test.go
@@ -0,0 +1,32 @@
+package v046_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/cosmos/cosmos-sdk/simapp"
+	"github.com/cosmos/cosmos-sdk/testutil"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
+	v046staking "github.com/cosmos/cosmos-sdk/x/staking/legacy/v046"
+	"github.com/cosmos/cosmos-sdk/x/staking/types"
+)
+
+func TestStoreMigration(t *testing.T) {
+	encCfg := simapp.MakeTestEncodingConfig()
+	stakingKey := sdk.NewKVStoreKey("staking")
+	tStakingKey := sdk.NewTransientStoreKey("transient_test")
+	ctx := testutil.DefaultContext(stakingKey, tStakingKey)
+	paramstore := paramtypes.NewSubspace(encCfg.Marshaler, encCfg.Amino, stakingKey, tStakingKey, "staking")
+
+	// Check no params
+	require.False(t, paramstore.Has(ctx, types.KeyMinSelfDelegation))
+
+	// Run migrations.
+	err := v046staking.MigrateStore(ctx, paramstore)
+	require.NoError(t, err)
+
+	// Make sure the new params are set.
+	require.True(t, paramstore.Has(ctx, types.KeyMinSelfDelegation))
+}
diff --git a/x/staking/module.go b/x/staking/module.go
@@ -141,6 +141,7 @@ func (am AppModule) RegisterServices(cfg module.Configurator) {
 
 	m := keeper.NewMigrator(am.keeper)
 	cfg.RegisterMigration(types.ModuleName, 1, m.Migrate1to2)
+	cfg.RegisterMigration(types.ModuleName, 2, m.Migrate2to3)
 }
 
 // InitGenesis performs genesis initialization for the staking module. It returns

diff --git a/x/staking/simulation/genesis.go b/x/staking/simulation/genesis.go
@@ -45,6 +45,7 @@ func RandomizedGenState(simState *module.SimulationState) {
 		maxVals     uint32
 		histEntries uint32
 		minComRate  sdk.Dec
+		minSelfDel  sdk.Int
 	)
 
 	simState.AppParams.GetOrGenerate(
@@ -70,7 +71,7 @@ func RandomizedGenState(simState *module.SimulationState) {
 	// NOTE: the slashing module need to be defined after the staking module on the
 	// NewSimulationManager constructor for this to work
 	simState.UnbondTime = unbondTime
-	params := types.NewParams(simState.UnbondTime, maxVals, 7, histEntries, sdk.DefaultBondDenom, minComRate)
+	params := types.NewParams(simState.UnbondTime, maxVals, 7, histEntries, sdk.DefaultBondDenom, minComRate, minSelfDel)
 
 	// validators & delegations
 	var (

diff --git a/x/staking/types/errors.go b/x/staking/types/errors.go
@@ -50,4 +50,5 @@ var (
 	ErrNoHistoricalInfo                = sdkerrors.Register(ModuleName, 38, "no historical info found")
 	ErrEmptyValidatorPubKey            = sdkerrors.Register(ModuleName, 39, "empty validator public key")
 	ErrCommissionLTMinRate             = sdkerrors.Register(ModuleName, 40, "commission cannot be less than min rate")
+	ErrMinSelfDelegationBelowMinimum   = sdkerrors.Register(ModuleName, 41, "validator's minimum self delegation must be greater than the global minimum self delegation")
 )
diff --git a/x/staking/types/params.go b/x/staking/types/params.go
@@ -37,6 +37,11 @@ var (
 	DefaultMinCommissionRate = sdk.ZeroDec()
 )
 
+var (
+	// DefaultMinSelfDelegation is set to 0
+	DefaultMinSelfDelegation = sdk.ZeroInt()
+)
+
 var (
 	KeyUnbondingTime     = []byte("UnbondingTime")
 	KeyMaxValidators     = []byte("MaxValidators")
@@ -45,6 +50,7 @@ var (
 	KeyHistoricalEntries = []byte("HistoricalEntries")
 	KeyPowerReduction    = []byte("PowerReduction")
 	KeyMinCommissionRate = []byte("MinCommissionRate")
+	KeyMinSelfDelegation = []byte("MinSelfDelegation")
 )
 
 var _ paramtypes.ParamSet = (*Params)(nil)
@@ -55,14 +61,15 @@ func ParamKeyTable() paramtypes.KeyTable {
 }
 
 // NewParams creates a new Params instance
-func NewParams(unbondingTime time.Duration, maxValidators, maxEntries, historicalEntries uint32, bondDenom string, minCommissionRate sdk.Dec) Params {
+func NewParams(unbondingTime time.Duration, maxValidators, maxEntries, historicalEntries uint32, bondDenom string, minCommissionRate sdk.Dec, minSelfDelegation sdk.Int) Params {
 	return Params{
 		UnbondingTime:     unbondingTime,
 		MaxValidators:     maxValidators,
 		MaxEntries:        maxEntries,
 		HistoricalEntries: historicalEntries,
 		BondDenom:         bondDenom,
 		MinCommissionRate: minCommissionRate,
+		MinSelfDelegation: minSelfDelegation,
 	}
 }
 
@@ -75,6 +82,7 @@ func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs {
 		paramtypes.NewParamSetPair(KeyHistoricalEntries, &p.HistoricalEntries, validateHistoricalEntries),
 		paramtypes.NewParamSetPair(KeyBondDenom, &p.BondDenom, validateBondDenom),
 		paramtypes.NewParamSetPair(KeyMinCommissionRate, &p.MinCommissionRate, validateMinCommissionRate),
+		paramtypes.NewParamSetPair(KeyMinSelfDelegation, &p.MinSelfDelegation, validateMinSelfDelegation),
 	}
 }
 
@@ -87,6 +95,7 @@ func DefaultParams() Params {
 		DefaultHistoricalEntries,
 		sdk.DefaultBondDenom,
 		DefaultMinCommissionRate,
+		DefaultMinSelfDelegation,
 	)
 }
 
@@ -235,3 +244,16 @@ func validateMinCommissionRate(i interface{}) error {
 
 	return nil
 }
+
+func validateMinSelfDelegation(i interface{}) error {
+	v, ok := i.(sdk.Int)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if v.IsNegative() {
+		return fmt.Errorf("minimum self delegation cannot be negative: %s", v)
+	}
+
+	return nil
+}