feat(x/staking): update validators min commission rate after `MsgUpda…

…teParams` (#19537)

x/staking/CHANGELOG.md

@@ -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.

x/staking/README.md

@@ -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

x/staking/keeper/msg_server.go

@@ -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
 }
 

x/staking/keeper/msg_server_test.go

@@ -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,15 +1051,35 @@ 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",
 			input: &types.MsgUpdateParams{
 				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()
+				}
 			}
 		})
 	}