feat(x/staking): update validators min commission rate after MsgUpdateParams (#19537)

This commit is contained in:
Julien Robert 2024-02-23 18:17:01 +01:00 committed by GitHub
parent 72e4134710
commit 4b73e31b00
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 80 additions and 10 deletions

View File

@ -27,6 +27,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Features
* [#19537](https://github.com/cosmos/cosmos-sdk/pull/19537) Changing `MinCommissionRate` in `MsgUpdateParams` now updates the minimum commission rate for all validators.
### Improvements
* [#19277](https://github.com/cosmos/cosmos-sdk/pull/19277) Hooks calls on `SetUnbondingDelegationEntry`, `SetRedelegationEntry`, `Slash` and `RemoveValidator` returns errors instead of logging just like other hooks calls.

View File

@ -776,6 +776,7 @@ When this message is processed the following actions occur:
The `MsgUpdateParams` update the staking module parameters.
The params are updated through a governance proposal where the signer is the gov module account address.
When the `MinCommissionRate` is updated, all validators with a lower (max) commission rate than `MinCommissionRate` will be updated to `MinCommissionRate`.
```protobuf reference
https://github.com/cosmos/cosmos-sdk/blob/v0.47.0-rc1/proto/cosmos/staking/v1beta1/tx.proto#L182-L195
@ -1037,6 +1038,10 @@ The staking module contains the following parameters:
| KeyRotationFee | sdk.Coin | "1000000stake" |
| MaxConsPubkeyRotations | int | 1 |
:::warning
Manually updating the `MinCommissionRate` parameter will not affect the commission rate of the existing validators. It will only affect the commission rate of the new validators. Update the parameter with `MsgUpdateParams` to affect the commission rate of the existing validators as well.
:::
## Client
### CLI

View File

@ -3,6 +3,7 @@ package keeper
import (
"context"
"errors"
"fmt"
"slices"
"strconv"
"time"
@ -598,11 +599,43 @@ func (k msgServer) UpdateParams(ctx context.Context, msg *types.MsgUpdateParams)
return nil, err
}
// get previous staking params
previousParams, err := k.Params.Get(ctx)
if err != nil {
return nil, err
}
// store params
if err := k.Params.Set(ctx, msg.Params); err != nil {
return nil, err
}
// when min commission rate is updated, we need to update the commission rate of all validators
if !previousParams.MinCommissionRate.Equal(msg.Params.MinCommissionRate) {
minRate := msg.Params.MinCommissionRate
vals, err := k.GetAllValidators(ctx)
if err != nil {
return nil, err
}
for _, val := range vals {
// set the commission rate to min rate
if val.Commission.CommissionRates.Rate.LT(minRate) {
val.Commission.CommissionRates.Rate = minRate
// set the max rate to minRate if it is less than min rate
if val.Commission.CommissionRates.MaxRate.LT(minRate) {
val.Commission.CommissionRates.MaxRate = minRate
}
val.Commission.UpdateTime = k.environment.HeaderService.GetHeaderInfo(ctx).Time
if err := k.SetValidator(ctx, val); err != nil {
return nil, fmt.Errorf("failed to set validator after MinCommissionRate param change: %w", err)
}
}
}
}
return &types.MsgUpdateParamsResponse{}, nil
}

View File

@ -1027,10 +1027,22 @@ func (s *KeeperTestSuite) TestMsgUpdateParams() {
ctx, keeper, msgServer := s.ctx, s.stakingKeeper, s.msgServer
require := s.Require()
// create validator to test commission rate
pk := ed25519.GenPrivKey().PubKey()
require.NotNil(pk)
comm := types.NewCommissionRates(math.LegacyNewDec(0), math.LegacyNewDec(0), math.LegacyNewDec(0))
s.bankKeeper.EXPECT().DelegateCoinsFromAccountToModule(gomock.Any(), Addr, types.NotBondedPoolName, gomock.Any()).AnyTimes()
msg, err := types.NewMsgCreateValidator(ValAddr.String(), pk, sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(10)), types.Description{Moniker: "NewVal"}, comm, math.OneInt())
require.NoError(err)
_, err = msgServer.CreateValidator(ctx, msg)
require.NoError(err)
paramsWithUpdatedMinCommissionRate := types.DefaultParams()
paramsWithUpdatedMinCommissionRate.MinCommissionRate = math.LegacyNewDecWithPrec(5, 2)
testCases := []struct {
name string
input *types.MsgUpdateParams
expErr bool
postCheck func()
expErrMsg string
}{
{
@ -1039,7 +1051,28 @@ func (s *KeeperTestSuite) TestMsgUpdateParams() {
Authority: keeper.GetAuthority(),
Params: types.DefaultParams(),
},
expErr: false,
postCheck: func() {
// verify that the commission isn't changed
vals, err := keeper.GetAllValidators(ctx)
require.NoError(err)
require.Len(vals, 1)
require.True(vals[0].Commission.Rate.Equal(comm.Rate))
require.True(vals[0].Commission.MaxRate.GTE(comm.MaxRate))
},
},
{
name: "valid params with updated min commission rate",
input: &types.MsgUpdateParams{
Authority: keeper.GetAuthority(),
Params: paramsWithUpdatedMinCommissionRate,
},
postCheck: func() {
vals, err := keeper.GetAllValidators(ctx)
require.NoError(err)
require.Len(vals, 1)
require.True(vals[0].Commission.Rate.GTE(paramsWithUpdatedMinCommissionRate.MinCommissionRate))
require.True(vals[0].Commission.MaxRate.GTE(paramsWithUpdatedMinCommissionRate.MinCommissionRate))
},
},
{
name: "invalid authority",
@ -1047,7 +1080,6 @@ func (s *KeeperTestSuite) TestMsgUpdateParams() {
Authority: "invalid",
Params: types.DefaultParams(),
},
expErr: true,
expErrMsg: "invalid authority",
},
{
@ -1063,7 +1095,6 @@ func (s *KeeperTestSuite) TestMsgUpdateParams() {
BondDenom: types.BondStatusBonded,
},
},
expErr: true,
expErrMsg: "minimum commission rate cannot be negative",
},
{
@ -1079,7 +1110,6 @@ func (s *KeeperTestSuite) TestMsgUpdateParams() {
BondDenom: types.BondStatusBonded,
},
},
expErr: true,
expErrMsg: "minimum commission rate cannot be greater than 100%",
},
{
@ -1095,7 +1125,6 @@ func (s *KeeperTestSuite) TestMsgUpdateParams() {
BondDenom: "",
},
},
expErr: true,
expErrMsg: "bond denom cannot be blank",
},
{
@ -1111,7 +1140,6 @@ func (s *KeeperTestSuite) TestMsgUpdateParams() {
BondDenom: types.BondStatusBonded,
},
},
expErr: true,
expErrMsg: "max validators must be positive",
},
{
@ -1127,7 +1155,6 @@ func (s *KeeperTestSuite) TestMsgUpdateParams() {
BondDenom: types.BondStatusBonded,
},
},
expErr: true,
expErrMsg: "max entries must be positive",
},
{
@ -1143,7 +1170,6 @@ func (s *KeeperTestSuite) TestMsgUpdateParams() {
BondDenom: "denom",
},
},
expErr: true,
expErrMsg: "unbonding time must be positive",
},
}
@ -1152,11 +1178,15 @@ func (s *KeeperTestSuite) TestMsgUpdateParams() {
tc := tc
s.T().Run(tc.name, func(t *testing.T) {
_, err := msgServer.UpdateParams(ctx, tc.input)
if tc.expErr {
if tc.expErrMsg != "" {
require.Error(err)
require.Contains(err.Error(), tc.expErrMsg)
} else {
require.NoError(err)
if tc.postCheck != nil {
tc.postCheck()
}
}
})
}