diff --git a/go.sum b/go.sum index 7cff65a977..1e40900e17 100644 --- a/go.sum +++ b/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= diff --git a/internal/chain/engine.go b/internal/chain/engine.go index 5bf8dc629d..d5866e3887 100644 --- a/internal/chain/engine.go +++ b/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 } } diff --git a/staking/availability/measure.go b/staking/availability/measure.go index faa105f261..a1c48845dd 100644 --- a/staking/availability/measure.go +++ b/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 } diff --git a/staking/availability/measure_test.go b/staking/availability/measure_test.go index 393d19bb4f..b3a26d8172 100644 --- a/staking/availability/measure_test.go +++ b/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) + }) +}