[viewchange] fix getNextLeader

next leader can be any leader depending on the gap of the view ID

do not change current pubkey unless leader change succeeded

Signed-off-by: Leo Chen <leo@harmony.one>

consensus/consensus_viewchange_msg.go

@@ -13,7 +13,7 @@ import (
 )
 
 // construct the view change message
-func (consensus *Consensus) constructViewChangeMessage(priKey *bls.PrivateKeyWrapper) []byte {
+func (consensus *Consensus) constructViewChangeMessage(priKey *bls.PrivateKeyWrapper, nextLeaderPubKey *bls.PublicKeyWrapper) []byte {
 	message := &msg_pb.Message{
 		ServiceType: msg_pb.ServiceType_CONSENSUS,
 		Type:        msg_pb.MessageType_VIEWCHANGE,
@@ -23,7 +23,7 @@ func (consensus *Consensus) constructViewChangeMessage(priKey *bls.PrivateKeyWra
 				BlockNum:     consensus.blockNum,
 				ShardId:      consensus.ShardID,
 				SenderPubkey: priKey.Pub.Bytes[:],
-				LeaderPubkey: consensus.LeaderPubKey.Bytes[:],
+				LeaderPubkey: nextLeaderPubKey.Bytes[:],
 			},
 		},
 	}

consensus/quorum/quorum.go

@@ -72,7 +72,7 @@ type ParticipantTracker interface {
 	Participants() multibls.PublicKeys
 	IndexOf(bls.SerializedPublicKey) int
 	ParticipantsCount() int64
-	NextAfter(*bls.PublicKeyWrapper) (bool, *bls.PublicKeyWrapper)
+	NextAfter(*bls.PublicKeyWrapper, int) (bool, *bls.PublicKeyWrapper)
 	UpdateParticipants(pubKeys []bls.PublicKeyWrapper)
 }
 
@@ -187,14 +187,14 @@ func (s *cIdentities) IndexOf(pubKey bls.SerializedPublicKey) int {
 	return -1
 }
 
-func (s *cIdentities) NextAfter(pubKey *bls.PublicKeyWrapper) (bool, *bls.PublicKeyWrapper) {
+func (s *cIdentities) NextAfter(pubKey *bls.PublicKeyWrapper, next int) (bool, *bls.PublicKeyWrapper) {
 	found := false
 
 	idx := s.IndexOf(pubKey.Bytes)
 	if idx != -1 {
 		found = true
 	}
-	idx = (idx + 1) % int(s.ParticipantsCount())
+	idx = (idx + next) % int(s.ParticipantsCount())
 	return found, &s.publicKeys[idx]
 }
 

consensus/view_change.go

@@ -120,8 +120,12 @@ func (consensus *Consensus) switchPhase(desired FBFTPhase, override bool) {
 }
 
 // GetNextLeaderKey uniquely determine who is the leader for given viewID
-func (consensus *Consensus) GetNextLeaderKey() *bls.PublicKeyWrapper {
+func (consensus *Consensus) GetNextLeaderKey(viewID uint64) *bls.PublicKeyWrapper {
-	wasFound, next := consensus.Decider.NextAfter(consensus.LeaderPubKey)
+	gap := 1
+	if viewID > consensus.GetCurViewID() {
+		gap = int(viewID - consensus.GetCurViewID())
+	}
+	wasFound, next := consensus.Decider.NextAfter(consensus.LeaderPubKey, gap)
 	if !wasFound {
 		consensus.getLogger().Warn().
 			Str("key", consensus.LeaderPubKey.Bytes.Hex()).
@@ -163,20 +167,20 @@ func (consensus *Consensus) startViewChange(viewID uint64) {
 	consensus.consensusTimeout[timeoutBootstrap].Stop()
 	consensus.current.SetMode(ViewChanging)
 	consensus.SetViewChangingID(viewID)
-	consensus.LeaderPubKey = consensus.GetNextLeaderKey()
+	nextLeaderPubKey := consensus.GetNextLeaderKey(viewID)
 
 	duration := consensus.current.GetViewChangeDuraion()
 	consensus.getLogger().Warn().
 		Uint64("viewChangingID", consensus.GetViewChangingID()).
 		Dur("timeoutDuration", duration).
-		Str("NextLeader", consensus.LeaderPubKey.Bytes.Hex()).
+		Str("NextLeader", nextLeaderPubKey.Bytes.Hex()).
 		Msg("[startViewChange]")
 
 	for _, key := range consensus.priKey {
 		if !consensus.IsValidatorInCommittee(key.Pub.Bytes) {
 			continue
 		}
-		msgToSend := consensus.constructViewChangeMessage(&key)
+		msgToSend := consensus.constructViewChangeMessage(&key, nextLeaderPubKey)
 		consensus.host.SendMessageToGroups([]nodeconfig.GroupID{
 			nodeconfig.NewGroupIDByShardID(nodeconfig.ShardID(consensus.ShardID)),
 		},
@@ -680,8 +684,6 @@ func (consensus *Consensus) onNewView(msg *msg_pb.Message) {
 	consensus.getLogger().Info().
 		Str("newLeaderKey", consensus.LeaderPubKey.Bytes.Hex()).
 		Msg("new leader changed")
-	consensus.getLogger().Info().
-		Msg("validator start consensus timer and stop view change timer")
 	consensus.consensusTimeout[timeoutConsensus].Start()
 	consensus.consensusTimeout[timeoutViewChange].Stop()
 }

consensus/view_change_test.go

@@ -111,7 +111,7 @@ func TestGetNextLeaderKeyShouldFailForStandardGeneratedConsensus(t *testing.T) {
 
 	// The below results in: "panic: runtime error: integer divide by zero"
 	// This happens because there's no check for if there are any participants or not in https://github.com/harmony-one/harmony/blob/main/consensus/quorum/quorum.go#L188-L197
-	assert.Panics(t, func() { consensus.GetNextLeaderKey() })
+	assert.Panics(t, func() { consensus.GetNextLeaderKey(uint64(1)) })
 }
 
 func TestGetNextLeaderKeyShouldSucceed(t *testing.T) {
@@ -139,7 +139,7 @@ func TestGetNextLeaderKeyShouldSucceed(t *testing.T) {
 	assert.Equal(t, keyCount, consensus.Decider.ParticipantsCount())
 
 	consensus.LeaderPubKey = &wrappedBLSKeys[0]
-	nextKey := consensus.GetNextLeaderKey()
+	nextKey := consensus.GetNextLeaderKey(uint64(1))
 
 	assert.Equal(t, nextKey, &wrappedBLSKeys[1])
 }