@ysv reviewed 10 of 10 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @masihyeganeh, @miladz68, and @TxCorpi0x)

app/upgrade/v6/validator_commission.go line 34 at r1 (raw file):

		if validator.Commission.Rate.LT(minCommissionRate) {
			validatorsToUpdate = append(validatorsToUpdate, validator)

why do we need this intermediate slice ?
Can't we make all changes in a single for loop ?

testutil/simapp/simapp.go line 279 at r1 (raw file):

	operator sdk.AccAddress,
	value sdk.Coin,
	commissionRate, maxRate *sdkmath.LegacyDec,

slightly better version IMO is to accept *stakingtypes.CommissionRates and use it or set default values (partially / fully)

integration-tests/upgrade/validator_commission.go line 17 at r1 (raw file):

type validatorCommission struct{}

func (v *validatorCommission) Before(t *testing.T) {

do we have validators with < 5% commission generated in znet in current config ?
Would be nice to test this migration in integration test also, if we have this possibility

Reviewable status: 2 of 10 files reviewed, 3 unresolved discussions (waiting on @masihyeganeh, @miladz68, and @ysv)

app/upgrade/v6/validator_commission.go line 34 at r1 (raw file):

Done

integration-tests/upgrade/validator_commission.go line 17 at r1 (raw file):

They are created by 10% in the genesis, but also added a check for that in the Before step.

testutil/simapp/simapp.go line 279 at r1 (raw file):

Done

@ysv reviewed 8 of 8 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @masihyeganeh and @miladz68)

@miladz68 reviewed 2 of 10 files at r1, 6 of 8 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @masihyeganeh and @TxCorpi0x)

app/upgrade/v6/upgrade.go line 61 at r1 (raw file):

			// Set minimum commission rate to 5% and update validators with lower rates
			minCommissionRate := sdkmath.LegacyNewDecWithPrec(5, 2) // 5 * 10^(-2) = 0.05 = 5%
			if err := SetMinCommissionRate(ctx, stakingKeeper, minCommissionRate); err != nil {

this action seems redundant.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @masihyeganeh and @miladz68)

app/upgrade/v6/upgrade.go line 61 at r1 (raw file):

Is it planned to do it in separate proposal for governance with this?
I think It would be more straightforward to do it in upgrade handler in one shot.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @masihyeganeh and @ysv)

app/upgrade/v6/upgrade.go line 61 at r1 (raw file):

@ysv What do you think?

@ysv reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @masihyeganeh and @miladz68)

app/upgrade/v6/upgrade.go line 61 at r1 (raw file):

this has already been executed as gov proposal on mainnet & testnet
So this action is redundant I agree

Reviewable status: 7 of 10 files reviewed, 1 unresolved discussion (waiting on @masihyeganeh, @miladz68, and @ysv)

app/upgrade/v6/upgrade.go line 61 at r1 (raw file):

Done

@ysv reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @masihyeganeh and @miladz68)

@miladz68 reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @masihyeganeh)