diff --git a/core/staking_verifier.go b/core/staking_verifier.go index 541d26f4e..7de6cdd39 100644 --- a/core/staking_verifier.go +++ b/core/staking_verifier.go @@ -171,11 +171,20 @@ func VerifyAndEditValidatorFromMsg( return nil, errCommissionRateChangeTooHigh } - if chainContext.Config().IsMinCommissionRate(epoch) && newRate.LT(availability.MinCommissionRate) { + minRate := availability.MinCommissionRate( + chainContext.Config().IsMinCommissionRate(epoch), + chainContext.Config().IsHIP30(epoch), + ) + if newRate.LT(minRate) { firstEpoch := stateDB.GetValidatorFirstElectionEpoch(msg.ValidatorAddress) promoPeriod := chainContext.Config().MinCommissionPromoPeriod.Int64() if firstEpoch.Uint64() != 0 && big.NewInt(0).Sub(epoch, firstEpoch).Int64() >= promoPeriod { - return nil, errCommissionRateChangeTooLow + return nil, + errors.Errorf( + "%s %d%%", + errCommissionRateChangeTooLowT, + minRate.MulInt64(100).Int64(), + ) } } diff --git a/core/staking_verifier_test.go b/core/staking_verifier_test.go index 01d24cc8d..1a33f9848 100644 --- a/core/staking_verifier_test.go +++ b/core/staking_verifier_test.go @@ -676,7 +676,7 @@ func TestVerifyAndEditValidatorFromMsg(t *testing.T) { return msg }(), - expErr: errCommissionRateChangeTooLow, + expErr: errCommissionRateChangeTooLowT, }, { // 15: Rate is ok within the promo period diff --git a/core/state_transition.go b/core/state_transition.go index 9684812cb..93e7f2322 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -40,7 +40,7 @@ var ( errNoDelegationToUndelegate = errors.New("no delegation to undelegate") errCommissionRateChangeTooFast = errors.New("change on commission rate can not be more than max change rate within the same epoch") errCommissionRateChangeTooHigh = errors.New("commission rate can not be higher than maximum commission rate") - errCommissionRateChangeTooLow = errors.New("commission rate can not be lower than min rate of 5%") + errCommissionRateChangeTooLowT = errors.New("commission rate can not be lower than min rate of ") errNoRewardsToCollect = errors.New("no rewards to collect") errNegativeAmount = errors.New("amount can not be negative") errDupIdentity = errors.New("validator identity exists") diff --git a/internal/chain/engine.go b/internal/chain/engine.go index 71bb0d305..18096c157 100644 --- a/internal/chain/engine.go +++ b/internal/chain/engine.go @@ -7,6 +7,7 @@ import ( "time" "github.com/harmony-one/harmony/internal/params" + "github.com/harmony-one/harmony/numeric" bls2 "github.com/harmony-one/bls/ffi/go/bls" blsvrf "github.com/harmony-one/harmony/crypto/vrf/bls" @@ -285,7 +286,7 @@ func (e *engineImpl) Finalize( // depends on the old LastEpochInCommittee startTime = time.Now() - if err := setElectionEpochAndMinFee(header, state, chain.Config()); err != nil { + if err := setElectionEpochAndMinFee(chain, header, state, chain.Config()); err != nil { return nil, nil, err } utils.Logger().Debug().Int64("elapsed time", time.Now().Sub(startTime).Milliseconds()).Msg("SetElectionEpochAndMinFee") @@ -390,12 +391,23 @@ func IsCommitteeSelectionBlock(chain engine.ChainReader, header *block.Header) b return isBeaconChain && header.IsLastBlockInEpoch() && inPreStakingEra } -func setElectionEpochAndMinFee(header *block.Header, state *state.DB, config *params.ChainConfig) error { +func setElectionEpochAndMinFee(chain engine.ChainReader, header *block.Header, state *state.DB, config *params.ChainConfig) error { newShardState, err := header.GetShardState() if err != nil { const msg = "[Finalize] failed to read shard state" return errors.New(msg) } + // these 2 should be created outside of loop to optimize + minRate := availability.MinCommissionRate( + config.IsMinCommissionRate(newShardState.Epoch), + config.IsHIP30(newShardState.Epoch), + ) + minRateNotZero := !minRate.Equal(numeric.ZeroDec()) + // elected validators have their fee updated, if required to do so + isElected := make( + map[common.Address]struct{}, + len(newShardState.StakedValidators().Addrs), + ) for _, addr := range newShardState.StakedValidators().Addrs { wrapper, err := state.ValidatorWrapper(addr, true, false) if err != nil { @@ -405,14 +417,32 @@ func setElectionEpochAndMinFee(header *block.Header, state *state.DB, config *pa } // Set last epoch in committee wrapper.LastEpochInCommittee = newShardState.Epoch - - if config.IsMinCommissionRate(newShardState.Epoch) { - // Set first election epoch + if minRateNotZero { + // Set first election epoch (applies only if previously unset) state.SetValidatorFirstElectionEpoch(addr, newShardState.Epoch) - // Update minimum commission fee - if err := availability.UpdateMinimumCommissionFee( - newShardState.Epoch, state, addr, config.MinCommissionPromoPeriod.Int64(), + if _, err := availability.UpdateMinimumCommissionFee( + newShardState.Epoch, state, addr, minRate, + config.MinCommissionPromoPeriod.Uint64(), + ); err != nil { + return err + } + } + isElected[addr] = struct{}{} + } + // due to a bug in the old implementation of the minimum fee, + // unelected validators did not have their fee updated even + // when the protocol required them to do so. here we fix it, + // but only after the HIP-30 hard fork is effective. + if config.IsHIP30(newShardState.Epoch) { + for _, addr := range chain.ValidatorCandidates() { + // skip elected validator + if _, ok := isElected[addr]; ok { + continue + } + if _, err := availability.UpdateMinimumCommissionFee( + newShardState.Epoch, state, addr, minRate, + config.MinCommissionPromoPeriod.Uint64(), ); err != nil { return err } diff --git a/staking/availability/measure.go b/staking/availability/measure.go index 680092aa0..881baa855 100644 --- a/staking/availability/measure.go +++ b/staking/availability/measure.go @@ -18,11 +18,25 @@ import ( var ( measure = numeric.NewDec(2).Quo(numeric.NewDec(3)) - MinCommissionRate = numeric.MustNewDecFromStr("0.05") + minCommissionRateEra1 = numeric.MustNewDecFromStr("0.05") + minCommissionRateEra2 = numeric.MustNewDecFromStr("0.07") // ErrDivByZero .. ErrDivByZero = errors.New("toSign of availability cannot be 0, mistake in protocol") ) +// Returns the minimum commission rate between the two options. +// The later rate supersedes the earlier rate. +// If neither is applicable, returns 0. +func MinCommissionRate(era1, era2 bool) numeric.Dec { + if era2 { + return minCommissionRateEra2 + } + if era1 { + return minCommissionRateEra1 + } + return numeric.ZeroDec() +} + // BlockSigners .. func BlockSigners( bitmap []byte, parentCommittee *shard.Committee, @@ -217,33 +231,39 @@ func ComputeAndMutateEPOSStatus( return nil } -// UpdateMinimumCommissionFee update the validator commission fee to the minimum 5% -// if the validator has a lower commission rate and 100 epochs have passed after -// the validator was first elected. +// UpdateMinimumCommissionFee update the validator commission fee to the minRate +// if the validator has a lower commission rate and promoPeriod epochs have passed after +// the validator was first elected. It returns true if the commission was updated func UpdateMinimumCommissionFee( electionEpoch *big.Int, state *state.DB, addr common.Address, - promoPeriod int64, -) error { + minRate numeric.Dec, + promoPeriod uint64, +) (bool, error) { utils.Logger().Info().Msg("begin update min commission fee") wrapper, err := state.ValidatorWrapper(addr, true, false) if err != nil { - return err + return false, err } firstElectionEpoch := state.GetValidatorFirstElectionEpoch(addr) - if firstElectionEpoch.Uint64() != 0 && big.NewInt(0).Sub(electionEpoch, firstElectionEpoch).Int64() >= int64(promoPeriod) { - if wrapper.Rate.LT(MinCommissionRate) { + // convert all to uint64 for easy maths + // this can take decades of time without overflowing + first := firstElectionEpoch.Uint64() + election := electionEpoch.Uint64() + if first != 0 && election-first >= promoPeriod && election >= first { + if wrapper.Rate.LT(minRate) { utils.Logger().Info(). Str("addr", addr.Hex()). Str("old rate", wrapper.Rate.String()). Str("firstElectionEpoch", firstElectionEpoch.String()). Msg("updating min commission rate") - wrapper.Rate.SetBytes(MinCommissionRate.Bytes()) + wrapper.Rate.SetBytes(minRate.Bytes()) + return true, nil } } - return nil + return false, nil }