Update edit validator logic to handle new commission rate

types/decimal.go

@@ -174,6 +174,7 @@ func NewDecFromStr(str string) (d Dec, err Error) {
 
 //______________________________________________________________________________________________
 //nolint
+func (d Dec) IsNil() bool       { return d.Int == nil }                 // is decimal nil
 func (d Dec) IsZero() bool      { return (d.Int).Sign() == 0 }          // is equal to zero
 func (d Dec) Equal(d2 Dec) bool { return (d.Int).Cmp(d2.Int) == 0 }     // equal decimals
 func (d Dec) GT(d2 Dec) bool    { return (d.Int).Cmp(d2.Int) > 0 }      // greater than

x/stake/client/cli/flags.go

@@ -34,6 +34,7 @@ var (
 	fsShares            = flag.NewFlagSet("", flag.ContinueOnError)
 	fsDescriptionCreate = flag.NewFlagSet("", flag.ContinueOnError)
 	fsCommissionCreate  = flag.NewFlagSet("", flag.ContinueOnError)
+	fsCommissionUpdate  = flag.NewFlagSet("", flag.ContinueOnError)
 	fsDescriptionEdit   = flag.NewFlagSet("", flag.ContinueOnError)
 	fsValidator         = flag.NewFlagSet("", flag.ContinueOnError)
 	fsDelegator         = flag.NewFlagSet("", flag.ContinueOnError)
@@ -49,6 +50,7 @@ func init() {
 	fsDescriptionCreate.String(FlagIdentity, "", "optional identity signature (ex. UPort or Keybase)")
 	fsDescriptionCreate.String(FlagWebsite, "", "optional website")
 	fsDescriptionCreate.String(FlagDetails, "", "optional details")
+	fsCommissionUpdate.String(FlagCommissionRate, "", "The new commission rate percentage")
 	fsCommissionCreate.String(FlagCommissionRate, "", "The initial commission rate percentage")
 	fsCommissionCreate.String(FlagCommissionMaxRate, "", "The maximum commission rate percentage")
 	fsCommissionCreate.String(FlagCommissionMaxChangeRate, "", "The maximum commission change rate percentage (per day)")

x/stake/client/cli/tx.go

@@ -131,17 +131,31 @@ func GetCmdEditValidator(cdc *codec.Codec) *cobra.Command {
 				Details:  viper.GetString(FlagDetails),
 			}
 
-			msg := stake.NewMsgEditValidator(sdk.ValAddress(valAddr), description)
+			var newRate *sdk.Dec
+
+			commissionRate := viper.GetString(FlagCommissionRate)
+			if commissionRate != "" {
+				rate, err := sdk.NewDecFromStr(commissionRate)
+				if err != nil {
+					return fmt.Errorf("invalid new commission rate: %v", err)
+				}
+
+				newRate = &rate
+			}
+
+			msg := stake.NewMsgEditValidator(sdk.ValAddress(valAddr), description, newRate)
 
 			if cliCtx.GenerateOnly {
 				return utils.PrintUnsignedStdTx(txBldr, cliCtx, []sdk.Msg{msg})
 			}
+
 			// build and sign the transaction, then broadcast to Tendermint
 			return utils.SendTx(txBldr, cliCtx, []sdk.Msg{msg})
 		},
 	}
 
 	cmd.Flags().AddFlagSet(fsDescriptionEdit)
+	cmd.Flags().AddFlagSet(fsCommissionUpdate)
 
 	return cmd
 }

x/stake/handler.go

@@ -107,7 +107,6 @@ func handleMsgCreateValidator(ctx sdk.Context, msg types.MsgCreateValidator, k k
 }
 
 func handleMsgEditValidator(ctx sdk.Context, msg types.MsgEditValidator, k keeper.Keeper) sdk.Result {
-
 	// validator must already be registered
 	validator, found := k.GetValidator(ctx, msg.ValidatorAddr)
 	if !found {
@@ -119,17 +118,26 @@ func handleMsgEditValidator(ctx sdk.Context, msg types.MsgEditValidator, k keepe
 	if err != nil {
 		return err.Result()
 	}
+
 	validator.Description = description
 
+	if msg.CommissionRate != nil {
+		if err := k.UpdateValidatorCommission(ctx, validator, *msg.CommissionRate); err != nil {
+			return err.Result()
+		}
+	}
+
 	// We don't need to run through all the power update logic within k.UpdateValidator
 	// We just need to override the entry in state, since only the description has changed.
 	k.SetValidator(ctx, validator)
+
 	tags := sdk.NewTags(
 		tags.Action, tags.ActionEditValidator,
 		tags.DstValidator, []byte(msg.ValidatorAddr.String()),
 		tags.Moniker, []byte(description.Moniker),
 		tags.Identity, []byte(description.Identity),
 	)
+
 	return sdk.Result{
 		Tags: tags,
 	}

x/stake/keeper/validator_test.go

@@ -3,9 +3,11 @@ package keeper
 import (
 	"fmt"
 	"testing"
+	"time"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/stake/types"
+	abci "github.com/tendermint/tendermint/abci/types"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -1043,3 +1045,71 @@ func TestGetValidTendermintUpdatesBondTransition(t *testing.T) {
 	clearTendermintUpdates(ctx, keeper)
 	require.Equal(t, 0, len(keeper.GetValidTendermintUpdates(ctx)))
 }
+
+func TestUpdateValidatorCommission(t *testing.T) {
+	ctx, _, keeper := CreateTestInput(t, false, 1000)
+	ctx = ctx.WithBlockHeader(abci.Header{Time: time.Now().UTC()})
+
+	commission := types.NewCommission(sdk.NewDecWithPrec(1, 1), sdk.NewDecWithPrec(3, 1), sdk.NewDecWithPrec(1, 1))
+
+	val1 := types.NewValidator(addrVals[0], PKs[0], types.Description{})
+	val2 := types.NewValidator(addrVals[1], PKs[1], types.Description{})
+
+	val1, _ = val1.SetInitialCommission(commission, time.Now().UTC().Add(time.Duration(-1)*time.Hour))
+	val2, _ = val1.SetInitialCommission(commission, time.Unix(0, 0).UTC())
+
+	testCases := []struct {
+		validator   types.Validator
+		newRate     sdk.Dec
+		expectedErr bool
+	}{
+		{
+			validator:   val1,
+			newRate:     sdk.ZeroDec(),
+			expectedErr: true,
+		},
+		{
+			validator:   val2,
+			newRate:     sdk.NewDecWithPrec(-1, 1),
+			expectedErr: true,
+		},
+		{
+			validator:   val2,
+			newRate:     sdk.NewDecWithPrec(4, 1),
+			expectedErr: true,
+		},
+		{
+			validator:   val2,
+			newRate:     sdk.NewDecWithPrec(3, 1),
+			expectedErr: true,
+		},
+		{
+			validator:   val2,
+			newRate:     sdk.NewDecWithPrec(2, 1),
+			expectedErr: false,
+		},
+	}
+
+	for i, tc := range testCases {
+		err := keeper.UpdateValidatorCommission(ctx, tc.validator, tc.newRate)
+
+		if tc.expectedErr {
+			require.Error(t, err, "expected error for test case #%d with rate: %s", i, tc.newRate)
+		} else {
+			val, found := keeper.GetValidator(ctx, tc.validator.OperatorAddr)
+
+			require.True(t, found,
+				"expected to find validator for test case #%d with rate: %s", i, tc.newRate,
+			)
+			require.NoError(t, err,
+				"unexpected error for test case #%d with rate: %s", i, tc.newRate,
+			)
+			require.Equal(t, tc.newRate, val.Commission.Rate,
+				"expected new validator commission rate for test case #%d with rate: %s", i, tc.newRate,
+			)
+			require.Equal(t, ctx.BlockHeader().Time, val.CommissionUpdateTime,
+				"expected new validator commission update time for test case #%d with rate: %s", i, tc.newRate,
+			)
+		}
+	}
+}

x/stake/types/msg.go

@@ -113,12 +113,20 @@ func (msg MsgCreateValidator) ValidateBasic() sdk.Error {
 type MsgEditValidator struct {
 	Description
 	ValidatorAddr sdk.ValAddress `json:"address"`
+
+	// We pass a reference to the new commission rate as it's not mandatory to
+	// update. If updated, the deserialized rate will be zero with no way to
+	// distinguish if an update was intended.
+	//
+	// REF: #2373
+	CommissionRate *sdk.Dec `json:"commission_rate"`
 }
 
-func NewMsgEditValidator(valAddr sdk.ValAddress, description Description) MsgEditValidator {
+func NewMsgEditValidator(valAddr sdk.ValAddress, description Description, newRate *sdk.Dec) MsgEditValidator {
 	return MsgEditValidator{
-		Description:   description,
-		ValidatorAddr: valAddr,
+		Description:    description,
+		CommissionRate: newRate,
+		ValidatorAddr:  valAddr,
 	}
 }
 
@@ -149,10 +157,11 @@ func (msg MsgEditValidator) ValidateBasic() sdk.Error {
 	if msg.ValidatorAddr == nil {
 		return sdk.NewError(DefaultCodespace, CodeInvalidInput, "nil validator address")
 	}
-	empty := Description{}
-	if msg.Description == empty {
+
+	if msg.Description == (Description{}) {
 		return sdk.NewError(DefaultCodespace, CodeInvalidInput, "transaction must include some information to modify")
 	}
+
 	return nil
 }
 

x/stake/types/msg_test.go

@@ -64,7 +64,9 @@ func TestMsgEditValidator(t *testing.T) {
 
 	for _, tc := range tests {
 		description := NewDescription(tc.moniker, tc.identity, tc.website, tc.details)
-		msg := NewMsgEditValidator(tc.validatorAddr, description)
+		newRate := sdk.ZeroDec()
+
+		msg := NewMsgEditValidator(tc.validatorAddr, description, newRate)
 		if tc.expectPass {
 			require.Nil(t, msg.ValidateBasic(), "test: %v", tc.name)
 		} else {