Fix review comments.

consensus/consensus.go

@@ -180,12 +180,21 @@ func (consensus *Consensus) GetLeaderPubKey() *bls_cosi.PublicKeyWrapper {
 	defer consensus.pubKeyLock.Unlock()
 	return consensus.LeaderPubKey
 }
+
+func (consensus *Consensus) getLeaderPubKey() *bls_cosi.PublicKeyWrapper {
+	return consensus.LeaderPubKey
+}
+
 func (consensus *Consensus) SetLeaderPubKey(pub *bls_cosi.PublicKeyWrapper) {
 	consensus.pubKeyLock.Lock()
 	consensus.LeaderPubKey = pub
 	consensus.pubKeyLock.Unlock()
 }
 
+func (consensus *Consensus) setLeaderPubKey(pub *bls_cosi.PublicKeyWrapper) {
+	consensus.LeaderPubKey = pub
+}
+
 func (consensus *Consensus) GetPrivateKeys() multibls.PrivateKeys {
 	return consensus.priKey
 }

consensus/consensus_v2.go

@@ -680,46 +680,46 @@ func (consensus *Consensus) commitBlock(blk *types.Block, committedMsg *FBFTMess
 	return nil
 }
 
+// rotateLeader rotates the leader to the next leader in the committee.
+// This function must be called with enabled leader rotation.
 func (consensus *Consensus) rotateLeader(epoch *big.Int) {
-	prev := consensus.GetLeaderPubKey()
+	prev := consensus.getLeaderPubKey()
 	curNumber := consensus.Blockchain.CurrentHeader().Number().Uint64()
 	utils.Logger().Info().Msgf("[Rotating leader] epoch: %v rotation:%v numblocks:%d", epoch.Uint64(), consensus.Blockchain.Config().IsLeaderRotation(epoch), consensus.Blockchain.Config().LeaderRotationBlocksCount)
-	if consensus.Blockchain.Config().IsLeaderRotation(epoch) {
-		leader := consensus.GetLeaderPubKey()
-		for i := 0; i < consensus.Blockchain.Config().LeaderRotationBlocksCount; i++ {
-			header := consensus.Blockchain.GetHeaderByNumber(curNumber - uint64(i))
-			if header == nil {
-				return
-			}
-			// Previous block was epoch block, we should not change leader.
-			if header.Epoch().Uint64() != epoch.Uint64() {
-				return
-			}
-			// Check if the same leader.
-			pub, err := consensus.Blockchain.GetLeaderPubKeyFromCoinbase(header)
-			if err != nil {
-				utils.Logger().Error().Err(err).Msg("Failed to get leader public key from coinbase")
-				return
-			}
-			if !pub.Object.IsEqual(leader.Object) {
-				// Another leader.
-				return
-			}
+	leader := consensus.getLeaderPubKey()
+	for i := 0; i < consensus.Blockchain.Config().LeaderRotationBlocksCount; i++ {
+		header := consensus.Blockchain.GetHeaderByNumber(curNumber - uint64(i))
+		if header == nil {
+			return
 		}
-		// Passed all checks, we can change leader.
-		wasFound, next := consensus.Decider.NthNextHmy(shard.Schedule.InstanceForEpoch(epoch), leader, 1)
-		if !wasFound {
-			utils.Logger().Error().Msg("Failed to get next leader")
+		// Previous epoch, we should not change leader.
+		if header.Epoch().Uint64() != epoch.Uint64() {
 			return
-		} else {
-			consensus.SetLeaderPubKey(next)
 		}
-		if consensus.IsLeader() && !consensus.GetLeaderPubKey().Object.IsEqual(prev.Object) {
-			// leader changed
-			go func() {
-				consensus.ReadySignal <- SyncProposal
-			}()
+		// Check if the same leader.
+		pub, err := consensus.Blockchain.GetLeaderPubKeyFromCoinbase(header)
+		if err != nil {
+			utils.Logger().Error().Err(err).Msg("Failed to get leader public key from coinbase")
+			return
 		}
+		if !pub.Object.IsEqual(leader.Object) {
+			// Another leader.
+			return
+		}
+	}
+	// Passed all checks, we can change leader.
+	wasFound, next := consensus.Decider.NthNextHmy(shard.Schedule.InstanceForEpoch(epoch), leader, 1)
+	if !wasFound {
+		utils.Logger().Error().Msg("Failed to get next leader")
+		return
+	} else {
+		consensus.setLeaderPubKey(next)
+	}
+	if consensus.isLeader() && !consensus.getLeaderPubKey().Object.IsEqual(prev.Object) {
+		// leader changed
+		go func() {
+			consensus.ReadySignal <- SyncProposal
+		}()
 	}
 }
 

internal/params/config.go

@@ -70,6 +70,8 @@ var (
 		CrossShardXferPrecompileEpoch: big.NewInt(1323), // Around Wed 8 Feb 11:30PM UTC
 		AllowlistEpoch:                EpochTBD,
 		FeeCollectEpoch:               EpochTBD,
+		LeaderRotationEpoch:           EpochTBD,
+		LeaderRotationBlocksCount:     64,
 	}
 
 	// TestnetChainConfig contains the chain parameters to run a node on the harmony test network.
@@ -147,6 +149,8 @@ var (
 		SlotsLimitedEpoch:             EpochTBD, // epoch to enable HIP-16
 		CrossShardXferPrecompileEpoch: big.NewInt(1),
 		AllowlistEpoch:                EpochTBD,
+		LeaderRotationEpoch:           EpochTBD,
+		LeaderRotationBlocksCount:     64,
 		FeeCollectEpoch:               EpochTBD,
 	}
 
@@ -187,6 +191,8 @@ var (
 		CrossShardXferPrecompileEpoch: big.NewInt(1),
 		AllowlistEpoch:                EpochTBD,
 		FeeCollectEpoch:               big.NewInt(574),
+		LeaderRotationEpoch:           EpochTBD,
+		LeaderRotationBlocksCount:     64,
 	}
 
 	// StressnetChainConfig contains the chain parameters for the Stress test network.