From 9dc8f43288ae552293b9efda9adc7bce2a4db301 Mon Sep 17 00:00:00 2001 From: Leo Chen Date: Sun, 20 Sep 2020 09:48:03 +0000 Subject: [PATCH] [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 --- consensus/consensus_viewchange_msg.go | 4 ++-- consensus/quorum/quorum.go | 6 +++--- consensus/view_change.go | 16 +++++++++------- consensus/view_change_test.go | 4 ++-- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/consensus/consensus_viewchange_msg.go b/consensus/consensus_viewchange_msg.go index fae0a9eff..00f2d7f9d 100644 --- a/consensus/consensus_viewchange_msg.go +++ b/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[:], }, }, } diff --git a/consensus/quorum/quorum.go b/consensus/quorum/quorum.go index 17b0a5361..a5db25689 100644 --- a/consensus/quorum/quorum.go +++ b/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] } diff --git a/consensus/view_change.go b/consensus/view_change.go index 871b4ea7e..50f37f78f 100644 --- a/consensus/view_change.go +++ b/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 { - wasFound, next := consensus.Decider.NextAfter(consensus.LeaderPubKey) +func (consensus *Consensus) GetNextLeaderKey(viewID uint64) *bls.PublicKeyWrapper { + 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() } diff --git a/consensus/view_change_test.go b/consensus/view_change_test.go index 07880634d..27d3c1dd0 100644 --- a/consensus/view_change_test.go +++ b/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]) }