UpdateMaxCommissionFee 100% coverage.

go.sum

@@ -1431,7 +1431,6 @@ github.com/vmihailenco/msgpack/v5 v5.3.5/go.mod h1:7xyJ9e+0+9SaZT0Wt1RGleJXzli6Q
 github.com/vmihailenco/tagparser v0.1.2/go.mod h1:OeAg3pn3UbLjkWt+rN9oFYB6u/cQgqMEUPoW2WPyhdI=
 github.com/vmihailenco/tagparser/v2 v2.0.0/go.mod h1:Wri+At7QHww0WTrCBeu4J6bNtoV6mEfg5OIWRZA9qds=
 github.com/warpfork/go-wish v0.0.0-20200122115046-b9ea61034e4a h1:G++j5e0OC488te356JvdhaM8YS6nMsjLAYF7JxCv07w=
-github.com/warpfork/go-wish v0.0.0-20200122115046-b9ea61034e4a/go.mod h1:x6AKhvSSexNrVSrViXSHUEbICjmGXhtgABaHIySUSGw=
 github.com/whyrusleeping/go-keyspace v0.0.0-20160322163242-5b898ac5add1 h1:EKhdznlJHPMoKr0XTrX+IlJs1LH3lyx2nfr1dOlZ79k=
 github.com/whyrusleeping/go-keyspace v0.0.0-20160322163242-5b898ac5add1/go.mod h1:8UvriyWtv5Q5EOgjHaSseUEdkQfvwFv1I/In/O2M9gc=
 github.com/whyrusleeping/timecache v0.0.0-20160911033111-cfcb2f1abfee h1:lYbXeSvJi5zk5GLKVuid9TVjS9a0OmLIDKTfoZBL6Ow=

internal/chain/engine.go

@@ -458,7 +458,7 @@ func setElectionEpochAndMinFee(chain engine.ChainReader, header *block.Header, s
 
 	if config.IsMaxRate(newShardState.Epoch) {
 		for _, addr := range chain.ValidatorCandidates() {
-			if _, err := availability.UpdateMaxCommissionFee(newShardState.Epoch, config, state, addr, minRate); err != nil {
+			if err := availability.UpdateMaxCommissionFee(config.IsTopMaxRate(newShardState.Epoch), state, addr, minRate); err != nil {
 				return err
 			}
 		}
@@ -486,10 +486,10 @@ func setElectionEpochAndMinFee(chain engine.ChainReader, header *block.Header, s
 	// for all validators which have MaxRate < minRate + maxChangeRate
 	// set their MaxRate equal to the minRate + MaxChangeRate
 	// this will allow the wrapper.SanityCheck to pass if Rate is set to a value
-	// higher than the the MaxRate by UpdateMinimumCommissionFee above
+	// higher than the MaxRate by UpdateMinimumCommissionFee above
 	if config.IsMaxRate(newShardState.Epoch) && minRateNotZero {
 		for _, addr := range chain.ValidatorCandidates() {
-			if _, err := availability.UpdateMaxCommissionFee(newShardState.Epoch, config, state, addr, minRate); err != nil {
+			if err := availability.UpdateMaxCommissionFee(config.IsTopMaxRate(newShardState.Epoch), state, addr, minRate); err != nil {
 				return err
 			}
 		}

staking/availability/measure.go

@@ -3,11 +3,9 @@ package availability
 import (
 	"math/big"
 
-	"github.com/harmony-one/harmony/core/state"
-
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/harmony-one/harmony/core/state"
 	"github.com/harmony-one/harmony/crypto/bls"
-	"github.com/harmony-one/harmony/internal/params"
 	"github.com/harmony-one/harmony/internal/utils"
 	"github.com/harmony-one/harmony/numeric"
 	"github.com/harmony-one/harmony/shard"
@@ -267,33 +265,36 @@ func UpdateMinimumCommissionFee(
 	return false, nil
 }
 
+type stateValidatorWrapper interface {
+	ValidatorWrapper(addr common.Address, sendOriginal bool, copyDelegations bool) (*staking.ValidatorWrapper, error)
+}
+
 // UpdateMaxCommissionFee makes sure the max-rate is at least higher than the rate + max-rate-change.
-func UpdateMaxCommissionFee(epoch *big.Int, config *params.ChainConfig, state *state.DB, addr common.Address, minRate numeric.Dec) (bool, error) {
+func UpdateMaxCommissionFee(IsTopMaxRate bool, state stateValidatorWrapper, addr common.Address, curRate numeric.Dec) error {
 	utils.Logger().Info().Msg("begin update max commission fee")
 
 	wrapper, err := state.ValidatorWrapper(addr, true, false)
 	if err != nil {
-		return false, err
+		return err
 	}
 
-	minMaxRate := minRate.Add(wrapper.MaxChangeRate)
+	newRate := curRate.Add(wrapper.MaxChangeRate)
 
-	if config.IsTopMaxRate(epoch) {
+	if IsTopMaxRate {
 		hundredPercent := numeric.NewDec(1)
-		if minMaxRate.GT(hundredPercent) {
-			minMaxRate = hundredPercent
+		if newRate.GT(hundredPercent) {
+			newRate = hundredPercent
 		}
 	}
 
-	if wrapper.MaxRate.LT(minMaxRate) {
+	if wrapper.MaxRate.LT(newRate) {
 		utils.Logger().Info().
 			Str("addr", addr.Hex()).
 			Str("old max-rate", wrapper.MaxRate.String()).
-			Str("new max-rate", minMaxRate.String()).
+			Str("new max-rate", newRate.String()).
 			Msg("updating max commission rate")
-		wrapper.MaxRate.SetBytes(minMaxRate.Bytes())
-		return true, nil
+		wrapper.MaxRate.SetBytes(newRate.Bytes())
 	}
 
-	return false, nil
+	return nil
 }

staking/availability/measure_test.go

@@ -7,14 +7,14 @@ import (
 	"reflect"
 	"testing"
 
-	"github.com/harmony-one/harmony/crypto/bls"
-
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/rlp"
+	"github.com/harmony-one/harmony/crypto/bls"
 	"github.com/harmony-one/harmony/numeric"
 	"github.com/harmony-one/harmony/shard"
 	"github.com/harmony-one/harmony/staking/effective"
 	staking "github.com/harmony-one/harmony/staking/types"
+	"github.com/stretchr/testify/require"
 )
 
 func TestBlockSigners(t *testing.T) {
@@ -731,3 +731,60 @@ func makeTestWrapper(addr common.Address, numSigned, numToSign int64) staking.Va
 	val.Counters.NumBlocksSigned = new(big.Int).SetInt64(numSigned)
 	return val
 }
+
+type stateValidatorWrapperImpl struct {
+	v   *staking.ValidatorWrapper
+	err error
+}
+
+func (a stateValidatorWrapperImpl) ValidatorWrapper(addr common.Address, sendOriginal bool, copyDelegations bool) (*staking.ValidatorWrapper, error) {
+	return a.v, a.err
+}
+
+func TestUpdateMaxCommissionFee(t *testing.T) {
+	t.Run("0.6 + 0.6 = 1 (100%)", func(t *testing.T) {
+		v1 := stateValidatorWrapperImpl{
+			v: &staking.ValidatorWrapper{
+				Validator: staking.Validator{
+					Commission: staking.Commission{
+						CommissionRates: staking.CommissionRates{
+							MaxChangeRate: numeric.MustNewDecFromStr("0.6"),
+							MaxRate:       numeric.MustNewDecFromStr("0.95"),
+						},
+					},
+				},
+			},
+		}
+
+		err := UpdateMaxCommissionFee(true, v1, common.Address{}, numeric.MustNewDecFromStr("0.6"))
+		require.NoError(t, err)
+		require.Equal(t, "1.000000000000000000", v1.v.Commission.CommissionRates.MaxRate.String())
+	})
+
+	t.Run("0.07 + 0.5 = 0.57", func(t *testing.T) {
+		v2 := stateValidatorWrapperImpl{
+			v: &staking.ValidatorWrapper{
+				Validator: staking.Validator{
+					Commission: staking.Commission{
+						CommissionRates: staking.CommissionRates{
+							MaxChangeRate: numeric.MustNewDecFromStr("0.5"),
+							MaxRate:       numeric.MustNewDecFromStr("0"),
+						},
+					},
+				},
+			},
+		}
+		err := UpdateMaxCommissionFee(true, v2, common.Address{}, numeric.MustNewDecFromStr("0.07"))
+		require.NoError(t, err)
+		require.Equal(t, "0.570000000000000000", v2.v.Commission.CommissionRates.MaxRate.String())
+	})
+
+	t.Run("error", func(t *testing.T) {
+		s := stateValidatorWrapperImpl{
+			v:   &staking.ValidatorWrapper{},
+			err: errors.New("error"),
+		}
+		err := UpdateMaxCommissionFee(true, s, common.Address{}, numeric.MustNewDecFromStr("0.07"))
+		require.Error(t, err)
+	})
+}