Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Min commission upgrade #10529

Merged
merged 16 commits into from
Nov 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/bank) [\#9832] (https://github.com/cosmos/cosmos-sdk/pull/9832) Account balance is stored as `sdk.Int` rather than `sdk.Coin`.
* (x/bank) [\#9890] (https://github.com/cosmos/cosmos-sdk/pull/9890) Remove duplicate denom from denom metadata key.
* (x/upgrade) [\#10189](https://github.com/cosmos/cosmos-sdk/issues/10189) Removed potential sources of non-determinism in upgrades
* [\#10393](https://github.com/cosmos/cosmos-sdk/pull/10422) Add `MinCommissionRate` param to `x/staking` module.

### Deprecated

Expand Down
1 change: 1 addition & 0 deletions docs/core/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -8405,6 +8405,7 @@ Params defines the parameters for the staking module.
| `max_entries` | [uint32](#uint32) | | max_entries is the max entries for either unbonding delegation or redelegation (per pair/trio). |
| `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 |



Expand Down
6 changes: 6 additions & 0 deletions proto/cosmos/staking/v1beta1/staking.proto
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,12 @@ message Params {
uint32 historical_entries = 4;
// bond_denom defines the bondable coin denomination.
string bond_denom = 5;
// min_commission_rate is the chain-wide minimum commission rate that a validator can charge their delegators
string min_commission_rate = 6 [
(gogoproto.moretags) = "yaml:\"min_commission_rate\"",
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec",
(gogoproto.nullable) = false
];
}

// DelegationResponse is equivalent to Delegation except that it contains a
Expand Down
3 changes: 2 additions & 1 deletion x/staking/client/testutil/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -885,12 +885,13 @@ func (s *IntegrationTestSuite) TestGetCmdQueryParams() {
historical_entries: 10000
max_entries: 7
max_validators: 100
min_commission_rate: "0.000000000000000000"
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"}`,
`{"unbonding_time":"1814400s","max_validators":100,"max_entries":7,"historical_entries":10000,"bond_denom":"stake","min_commission_rate":"0.000000000000000000"}`,
},
}
for _, tc := range testCases {
Expand Down
2 changes: 1 addition & 1 deletion x/staking/keeper/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,5 @@ func (m Migrator) Migrate1to2(ctx sdk.Context) error {

// Migrate2to3 migrates x/staking state from consensus version 2 to 3.
func (m Migrator) Migrate2to3(ctx sdk.Context) error {
return v045.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc)
return v045.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc, m.keeper.paramstore)
}
5 changes: 5 additions & 0 deletions x/staking/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ func (k msgServer) CreateValidator(goCtx context.Context, msg *types.MsgCreateVa
return nil, err
}

if msg.Commission.Rate.LT(k.MinCommissionRate(ctx)) {
return nil, sdkerrors.Wrapf(types.ErrCommissionLTMinRate, "cannot set validator commission to less than minimum rate of %s", k.MinCommissionRate(ctx))
}

// check to see if the pubkey or sender has been registered before
if _, found := k.GetValidator(ctx, valAddr); found {
return nil, types.ErrValidatorOwnerExists
Expand Down Expand Up @@ -74,6 +78,7 @@ func (k msgServer) CreateValidator(goCtx context.Context, msg *types.MsgCreateVa
if err != nil {
return nil, err
}

commission := types.NewCommissionWithTime(
msg.Commission.Rate, msg.Commission.MaxRate,
msg.Commission.MaxChangeRate, ctx.BlockHeader().Time,
Expand Down
9 changes: 8 additions & 1 deletion x/staking/keeper/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,21 @@ func (k Keeper) PowerReduction(ctx sdk.Context) sdk.Int {
return sdk.DefaultPowerReduction
}

// Get all parameteras as types.Params
// MinCommissionRate - Minimum validator commission rate
func (k Keeper) MinCommissionRate(ctx sdk.Context) (res sdk.Dec) {
k.paramstore.Get(ctx, types.KeyMinCommissionRate, &res)
return
}

// Get all parameters as types.Params
func (k Keeper) GetParams(ctx sdk.Context) types.Params {
return types.NewParams(
k.UnbondingTime(ctx),
k.MaxValidators(ctx),
k.MaxEntries(ctx),
k.HistoricalEntries(ctx),
k.BondDenom(ctx),
k.MinCommissionRate(ctx),
)
}

Expand Down
4 changes: 4 additions & 0 deletions x/staking/keeper/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ func (k Keeper) UpdateValidatorCommission(ctx sdk.Context,
return commission, err
}

if newRate.LT(k.MinCommissionRate(ctx)) {
return commission, fmt.Errorf("cannot set validator commission to less than minimum rate of %s", k.MinCommissionRate(ctx))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use SDK error

}

commission.Rate = newRate
commission.UpdateTime = blockTime

Expand Down
6 changes: 6 additions & 0 deletions x/staking/keeper/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1052,6 +1052,11 @@ func TestUpdateValidatorCommission(t *testing.T) {
app, ctx, _, addrVals := bootstrapValidatorTest(t, 1000, 20)
ctx = ctx.WithBlockHeader(tmproto.Header{Time: time.Now().UTC()})

// Set MinCommissionRate to 0.05
params := app.StakingKeeper.GetParams(ctx)
params.MinCommissionRate = sdk.NewDecWithPrec(5, 2)
app.StakingKeeper.SetParams(ctx, params)

commission1 := types.NewCommissionWithTime(
sdk.NewDecWithPrec(1, 1), sdk.NewDecWithPrec(3, 1),
sdk.NewDecWithPrec(1, 1), time.Now().UTC().Add(time.Duration(-1)*time.Hour),
Expand All @@ -1076,6 +1081,7 @@ func TestUpdateValidatorCommission(t *testing.T) {
{val2, sdk.NewDecWithPrec(-1, 1), true},
{val2, sdk.NewDecWithPrec(4, 1), true},
{val2, sdk.NewDecWithPrec(3, 1), true},
{val2, sdk.NewDecWithPrec(1, 2), true},
{val2, sdk.NewDecWithPrec(2, 1), false},
}

Expand Down
12 changes: 11 additions & 1 deletion x/staking/migrations/v045/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,29 @@ import (
"github.com/cosmos/cosmos-sdk/store/prefix"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
v040staking "github.com/cosmos/cosmos-sdk/x/staking/migrations/v040"
"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:
//
// - Removing delegations that have a zero share or token amount.
func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec) error {
// - Setting the MinCommissionRate param in the paramstore
func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec, paramstore paramtypes.Subspace) error {
store := ctx.KVStore(storeKey)

migrateParamsStore(ctx, paramstore)

return purgeDelegations(store, cdc)
}

func migrateParamsStore(ctx sdk.Context, paramstore paramtypes.Subspace) {
paramstore.WithKeyTable(types.ParamKeyTable())
paramstore.Set(ctx, types.KeyMinCommissionRate, types.DefaultMinCommissionRate)
}

func purgeDelegations(store sdk.KVStore, cdc codec.BinaryCodec) error {
prefixDelStore := prefix.NewStore(store, v040staking.DelegationKey)

Expand Down
32 changes: 32 additions & 0 deletions x/staking/migrations/v045/store_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package v045_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"
v045staking "github.com/cosmos/cosmos-sdk/x/staking/migrations/v045"
"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.Codec, encCfg.Amino, stakingKey, tStakingKey, "staking")

// Check no params
require.False(t, paramstore.Has(ctx, types.KeyMinCommissionRate))

// Run migrations.
err := v045staking.MigrateStore(ctx, stakingKey, encCfg.Codec, paramstore)
require.NoError(t, err)

// Make sure the new params are set.
require.True(t, paramstore.Has(ctx, types.KeyMinCommissionRate))
}
9 changes: 5 additions & 4 deletions x/staking/simulation/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ func getHistEntries(r *rand.Rand) uint32 {
func RandomizedGenState(simState *module.SimulationState) {
// params
var (
unbondTime time.Duration
maxVals uint32
histEntries uint32
unbondTime time.Duration
maxVals uint32
histEntries uint32
minCommissionRate sdk.Dec
)

simState.AppParams.GetOrGenerate(
Expand All @@ -63,7 +64,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)
params := types.NewParams(simState.UnbondTime, maxVals, 7, histEntries, sdk.DefaultBondDenom, minCommissionRate)

// validators & delegations
var (
Expand Down
16 changes: 8 additions & 8 deletions x/staking/spec/08_params.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ order: 8

The staking module contains the following parameters:

| Key | Type | Example |
|-------------------|------------------|-------------------|
| UnbondingTime | string (time ns) | "259200000000000" |
| MaxValidators | uint16 | 100 |
| KeyMaxEntries | uint16 | 7 |
| HistoricalEntries | uint16 | 3 |
| BondDenom | string | "stake" |
| PowerReduction | string | "1000000" |
| Key | Type | Example |
|-------------------|------------------|------------------------|
| UnbondingTime | string (time ns) | "259200000000000" |
| MaxValidators | uint16 | 100 |
| KeyMaxEntries | uint16 | 7 |
| HistoricalEntries | uint16 | 3 |
| BondDenom | string | "stake" |
| MinCommissionRate | string | "0.000000000000000000" |
1 change: 1 addition & 0 deletions x/staking/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,5 @@ var (
ErrInvalidHistoricalInfo = sdkerrors.Register(ModuleName, 37, "invalid historical info")
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")
)
31 changes: 30 additions & 1 deletion x/staking/types/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,18 @@ const (
DefaultHistoricalEntries uint32 = 10000
)

var (
// DefaultMinCommissionRate is set to 0%
DefaultMinCommissionRate = sdk.ZeroDec()
)

var (
KeyUnbondingTime = []byte("UnbondingTime")
KeyMaxValidators = []byte("MaxValidators")
KeyMaxEntries = []byte("MaxEntries")
KeyBondDenom = []byte("BondDenom")
KeyHistoricalEntries = []byte("HistoricalEntries")
KeyMinCommissionRate = []byte("MinCommissionRate")
)

var _ paramtypes.ParamSet = (*Params)(nil)
Expand All @@ -48,13 +54,14 @@ func ParamKeyTable() paramtypes.KeyTable {
}

// NewParams creates a new Params instance
func NewParams(unbondingTime time.Duration, maxValidators, maxEntries, historicalEntries uint32, bondDenom string) Params {
func NewParams(unbondingTime time.Duration, maxValidators, maxEntries, historicalEntries uint32, bondDenom string, minCommissionRate sdk.Dec) Params {
return Params{
UnbondingTime: unbondingTime,
MaxValidators: maxValidators,
MaxEntries: maxEntries,
HistoricalEntries: historicalEntries,
BondDenom: bondDenom,
MinCommissionRate: minCommissionRate,
}
}

Expand All @@ -66,6 +73,7 @@ func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs {
paramtypes.NewParamSetPair(KeyMaxEntries, &p.MaxEntries, validateMaxEntries),
paramtypes.NewParamSetPair(KeyHistoricalEntries, &p.HistoricalEntries, validateHistoricalEntries),
paramtypes.NewParamSetPair(KeyBondDenom, &p.BondDenom, validateBondDenom),
paramtypes.NewParamSetPair(KeyMinCommissionRate, &p.MinCommissionRate, validateMinCommissionRate),
}
}

Expand All @@ -77,6 +85,7 @@ func DefaultParams() Params {
DefaultMaxEntries,
DefaultHistoricalEntries,
sdk.DefaultBondDenom,
DefaultMinCommissionRate,
)
}

Expand Down Expand Up @@ -124,6 +133,10 @@ func (p Params) Validate() error {
return err
}

if err := validateMinCommissionRate(p.MinCommissionRate); err != nil {
return err
}

return nil
}

Expand Down Expand Up @@ -204,3 +217,19 @@ func ValidatePowerReduction(i interface{}) error {

return nil
}

func validateMinCommissionRate(i interface{}) error {
v, ok := i.(sdk.Dec)
if !ok {
return fmt.Errorf("invalid parameter type: %T", i)
}

if v.IsNegative() {
return fmt.Errorf("minimum commission rate cannot be negative: %s", v)
}
if v.GT(sdk.OneDec()) {
return fmt.Errorf("minimum commission rate cannot be greater than 100%%: %s", v)
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved
}

return nil
}
15 changes: 15 additions & 0 deletions x/staking/types/params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/stretchr/testify/require"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/staking/types"
)

Expand All @@ -21,3 +22,17 @@ func TestParamsEqual(t *testing.T) {
ok = p1.Equal(p2)
require.False(t, ok)
}

func Test_validateParams(t *testing.T) {
params := types.DefaultParams()

// default params have no error
require.NoError(t, params.Validate())

// validate mincommision
params.MinCommissionRate = sdk.NewDec(-1)
require.Error(t, params.Validate())

params.MinCommissionRate = sdk.NewDec(2)
require.Error(t, params.Validate())
}
Loading