From 4d18b7feec441926ae437489325765e1782fa4bf Mon Sep 17 00:00:00 2001 From: Rongjian Lan Date: Mon, 2 Nov 2020 18:48:51 -0800 Subject: [PATCH] Revert "Fix view change stuck issue" --- consensus/consensus_service.go | 10 ++-------- consensus/consensus_v2.go | 2 +- consensus/construct_test.go | 10 ++++------ consensus/quorum/one-node-one-vote.go | 2 +- consensus/quorum/one-node-staked-vote.go | 2 +- consensus/quorum/quorom_test.go | 14 +++++++------- consensus/quorum/quorum.go | 6 ++---- consensus/validator.go | 19 ++++++++----------- consensus/view_change.go | 5 +++-- 9 files changed, 29 insertions(+), 41 deletions(-) diff --git a/consensus/consensus_service.go b/consensus/consensus_service.go index b3ede1404..17a220de0 100644 --- a/consensus/consensus_service.go +++ b/consensus/consensus_service.go @@ -421,12 +421,6 @@ func (consensus *Consensus) UpdateConsensusInformation() Mode { Msg("[UpdateConsensusInformation] changing committee") // take care of possible leader change during the epoch - // TODO: in a very rare case, when a M1 view change happened, the block contains coinbase for last leader - // but the new leader is actually recognized by most of the nodes. At this time, if a node sync to this - // exact block and set its leader, it will set with the failed leader as in the coinbase of the block. - // This is a very rare case scenario and not likely to cause any issue in mainnet. But we need to think about - // a solution to take care of this case because the coinbase of the latest block doesn't really represent the - // the real current leader in case of M1 view change. if !curHeader.IsLastBlockInEpoch() && curHeader.Number().Uint64() != 0 { leaderPubKey, err := consensus.getLeaderPubKeyFromCoinbase(curHeader) if err != nil || leaderPubKey == nil { @@ -579,9 +573,9 @@ func (consensus *Consensus) selfCommit(payload []byte) error { continue } - if _, err := consensus.Decider.AddNewVote( + if _, err := consensus.Decider.SubmitVote( quorum.Commit, - []*bls_cosi.PublicKeyWrapper{key.Pub}, + []bls.SerializedPublicKey{key.Pub.Bytes}, key.Pri.SignHash(commitPayload), common.BytesToHash(consensus.blockHash[:]), block.NumberU64(), diff --git a/consensus/consensus_v2.go b/consensus/consensus_v2.go index ea1d1b9a0..6360d77a0 100644 --- a/consensus/consensus_v2.go +++ b/consensus/consensus_v2.go @@ -517,7 +517,7 @@ func (consensus *Consensus) commitBlock(blk *types.Block, committedMsg *FBFTMess } atomic.AddUint64(&consensus.blockNum, 1) - consensus.SetViewIDs(committedMsg.ViewID + 1) + consensus.SetCurBlockViewID(committedMsg.ViewID + 1) consensus.LeaderPubKey = committedMsg.SenderPubkeys[0] // Update consensus keys at last so the change of leader status doesn't mess up normal flow if blk.IsLastBlockInEpoch() { diff --git a/consensus/construct_test.go b/consensus/construct_test.go index 27d1e82f4..24b45c11a 100644 --- a/consensus/construct_test.go +++ b/consensus/construct_test.go @@ -71,21 +71,19 @@ func TestConstructPreparedMessage(test *testing.T) { message := "test string" leaderKey := bls.SerializedPublicKey{} leaderKey.FromLibBLSPublicKey(leaderPubKey) - leaderKeyWrapper := bls.PublicKeyWrapper{Object: leaderPubKey, Bytes: leaderKey} validatorKey := bls.SerializedPublicKey{} validatorKey.FromLibBLSPublicKey(validatorPubKey) - validatorKeyWrapper := bls.PublicKeyWrapper{Object: validatorPubKey, Bytes: validatorKey} - consensus.Decider.AddNewVote( + consensus.Decider.SubmitVote( quorum.Prepare, - []*bls.PublicKeyWrapper{&leaderKeyWrapper}, + []bls.SerializedPublicKey{leaderKey}, leaderPriKey.Sign(message), common.BytesToHash(consensus.blockHash[:]), consensus.blockNum, consensus.GetCurBlockViewID(), ) - if _, err := consensus.Decider.AddNewVote( + if _, err := consensus.Decider.SubmitVote( quorum.Prepare, - []*bls.PublicKeyWrapper{&validatorKeyWrapper}, + []bls.SerializedPublicKey{validatorKey}, validatorPriKey.Sign(message), common.BytesToHash(consensus.blockHash[:]), consensus.blockNum, diff --git a/consensus/quorum/one-node-one-vote.go b/consensus/quorum/one-node-one-vote.go index 7455d6e72..45db9bcd3 100644 --- a/consensus/quorum/one-node-one-vote.go +++ b/consensus/quorum/one-node-one-vote.go @@ -36,7 +36,7 @@ func (v *uniformVoteWeight) AddNewVote( for i, pubKey := range pubKeys { pubKeysBytes[i] = pubKey.Bytes } - return v.submitVote(p, pubKeysBytes, sig, headerHash, height, viewID) + return v.SubmitVote(p, pubKeysBytes, sig, headerHash, height, viewID) } // IsQuorumAchieved .. diff --git a/consensus/quorum/one-node-staked-vote.go b/consensus/quorum/one-node-staked-vote.go index 0f65257c9..8023b03d8 100644 --- a/consensus/quorum/one-node-staked-vote.go +++ b/consensus/quorum/one-node-staked-vote.go @@ -82,7 +82,7 @@ func (v *stakedVoteWeight) AddNewVote( pubKeysBytes[i] = pubKey.Bytes } - ballet, err := v.submitVote(p, pubKeysBytes, sig, headerHash, height, viewID) + ballet, err := v.SubmitVote(p, pubKeysBytes, sig, headerHash, height, viewID) if err != nil { return ballet, err diff --git a/consensus/quorum/quorom_test.go b/consensus/quorum/quorom_test.go index 73767ea25..d6b91a5bf 100644 --- a/consensus/quorum/quorom_test.go +++ b/consensus/quorum/quorom_test.go @@ -88,7 +88,7 @@ func TestSubmitVote(test *testing.T) { decider.UpdateParticipants([]bls.PublicKeyWrapper{pubKeyWrapper1, pubKeyWrapper2}) - if _, err := decider.submitVote( + if _, err := decider.SubmitVote( Prepare, []bls.SerializedPublicKey{pubKeyWrapper1.Bytes}, blsPriKey1.Sign(message), @@ -99,7 +99,7 @@ func TestSubmitVote(test *testing.T) { test.Log(err) } - if _, err := decider.submitVote( + if _, err := decider.SubmitVote( Prepare, []bls.SerializedPublicKey{pubKeyWrapper2.Bytes}, blsPriKey2.Sign(message), @@ -110,7 +110,7 @@ func TestSubmitVote(test *testing.T) { test.Log(err) } if decider.SignersCount(Prepare) != 2 { - test.Fatal("submitVote failed") + test.Fatal("SubmitVote failed") } aggSig := &bls_core.Sign{} @@ -145,7 +145,7 @@ func TestSubmitVoteAggregateSig(test *testing.T) { decider.UpdateParticipants([]bls.PublicKeyWrapper{pubKeyWrapper1, pubKeyWrapper2}) - decider.submitVote( + decider.SubmitVote( Prepare, []bls.SerializedPublicKey{pubKeyWrapper1.Bytes}, blsPriKey1.SignHash(blockHash[:]), @@ -160,7 +160,7 @@ func TestSubmitVoteAggregateSig(test *testing.T) { aggSig.Add(s) } } - if _, err := decider.submitVote( + if _, err := decider.SubmitVote( Prepare, []bls.SerializedPublicKey{pubKeyWrapper2.Bytes, pubKeyWrapper3.Bytes}, aggSig, @@ -172,7 +172,7 @@ func TestSubmitVoteAggregateSig(test *testing.T) { } if decider.SignersCount(Prepare) != 3 { - test.Fatal("submitVote failed") + test.Fatal("SubmitVote failed") } aggSig.Add(blsPriKey1.SignHash(blockHash[:])) @@ -180,7 +180,7 @@ func TestSubmitVoteAggregateSig(test *testing.T) { test.Fatal("AggregateVotes failed") } - if _, err := decider.submitVote( + if _, err := decider.SubmitVote( Prepare, []bls.SerializedPublicKey{pubKeyWrapper2.Bytes}, aggSig, diff --git a/consensus/quorum/quorum.go b/consensus/quorum/quorum.go index 674ea96c0..f41d47ec1 100644 --- a/consensus/quorum/quorum.go +++ b/consensus/quorum/quorum.go @@ -81,8 +81,7 @@ type ParticipantTracker interface { // SignatoryTracker .. type SignatoryTracker interface { ParticipantTracker - // This func shouldn't be called directly from outside of quorum. Use AddNewVote instead. - submitVote( + SubmitVote( p Phase, pubkeys []bls.SerializedPublicKey, sig *bls_core.Sign, headerHash common.Hash, height, viewID uint64, @@ -119,7 +118,6 @@ type Decider interface { DependencyInjectionWriter SetVoters(subCommittee *shard.Committee, epoch *big.Int) (*TallyResult, error) Policy() Policy - // Add new vote will add the signature in the memory and increase the cumulative voting power AddNewVote( p Phase, pubkeys []*bls_cosi.PublicKeyWrapper, sig *bls_core.Sign, headerHash common.Hash, @@ -275,7 +273,7 @@ func (s *cIdentities) SignersCount(p Phase) int64 { } } -func (s *cIdentities) submitVote( +func (s *cIdentities) SubmitVote( p Phase, pubkeys []bls.SerializedPublicKey, sig *bls_core.Sign, headerHash common.Hash, height, viewID uint64, diff --git a/consensus/validator.go b/consensus/validator.go index 3837ddda4..3dd3f03f8 100644 --- a/consensus/validator.go +++ b/consensus/validator.go @@ -114,6 +114,10 @@ func (consensus *Consensus) onPrepared(msg *msg_pb.Message) { Msg("Wrong BlockNum Received, ignoring!") return } + if recvMsg.BlockNum > consensus.blockNum { + consensus.getLogger().Warn().Msgf("[OnPrepared] low consensus block number. Spin sync") + consensus.spinUpStateSync() + } // check validity of prepared signature blockHash := recvMsg.BlockHash @@ -149,12 +153,6 @@ func (consensus *Consensus) onPrepared(msg *msg_pb.Message) { if !consensus.onPreparedSanityChecks(&blockObj, recvMsg) { return } - - if recvMsg.BlockNum > consensus.blockNum { - consensus.getLogger().Warn().Msgf("[OnPrepared] low consensus block number. Spin sync") - consensus.spinUpStateSync() - } - consensus.mutex.Lock() defer consensus.mutex.Unlock() @@ -238,6 +236,10 @@ func (consensus *Consensus) onCommitted(msg *msg_pb.Message) { if !consensus.isRightBlockNumCheck(recvMsg) { return } + if recvMsg.BlockNum > consensus.blockNum { + consensus.getLogger().Info().Msg("[OnCommitted] low consensus block number. Spin up state sync") + consensus.spinUpStateSync() + } aggSig, mask, err := consensus.ReadSignatureBitmapPayload(recvMsg.Payload, 0) if err != nil { @@ -270,11 +272,6 @@ func (consensus *Consensus) onCommitted(msg *msg_pb.Message) { consensus.FBFTLog.AddMessage(recvMsg) - if recvMsg.BlockNum > consensus.blockNum { - consensus.getLogger().Info().Msg("[OnCommitted] low consensus block number. Spin up state sync") - consensus.spinUpStateSync() - } - consensus.mutex.Lock() defer consensus.mutex.Unlock() diff --git a/consensus/view_change.go b/consensus/view_change.go index 3ef66eb9d..37df69712 100644 --- a/consensus/view_change.go +++ b/consensus/view_change.go @@ -131,8 +131,9 @@ func (consensus *Consensus) getNextViewID() (uint64, time.Duration) { if curTimestamp <= blockTimestamp { return consensus.fallbackNextViewID() } - // diff only increases - diff := uint64((curTimestamp - blockTimestamp) / viewChangeTimeout) + totalNode := consensus.Decider.ParticipantsCount() + // diff is at least 1, and it won't exceed the totalNode + diff := uint64(((curTimestamp - blockTimestamp) / viewChangeTimeout) % int64(totalNode)) nextViewID := diff + consensus.GetCurBlockViewID() consensus.getLogger().Info().