fix(consensus): refcator strong termination for change-proposer phase (…

…#1643)

consensus/consensus.go

@@ -186,7 +186,7 @@ func (cs *consensus) moveToNewHeight() {
 func (cs *consensus) scheduleTimeout(duration time.Duration, height uint32, round int16, target tickerTarget) {
 	ticker := &ticker{duration, height, round, target}
 	timer := time.NewTimer(duration)
-	cs.logger.Debug("new timer scheduled ⏱️", "duration", duration, "height", height, "round", round, "target", target)
+	cs.logger.Trace("new timer scheduled ⏱️", "duration", duration, "height", height, "round", round, "target", target)
 
 	go func() {
 		<-timer.C
@@ -508,14 +508,13 @@ func (cs *consensus) HandleQueryVote(height uint32, round int16) *vote.Vote {
 	votes := []*vote.Vote{}
 	switch {
 	case round < cs.round:
-		// A validator requests votes for past rounds.
-		// Sending cp:decide for the last round helps them advance to the current round.
-		vs := cs.log.CPDecidedVoteSet(cs.round - 1)
+		// Past round: Only broadcast cp:decided votes
+		vs := cs.log.CPDecidedVoteSet(round)
 		votes = append(votes, vs.AllVotes()...)
 
 	case round == cs.round:
 		// Current round
-		m := cs.log.RoundMessages(cs.round)
+		m := cs.log.RoundMessages(round)
 		votes = append(votes, m.AllVotes()...)
 
 	case round > cs.round:

consensus/consensus_test.go

@@ -638,11 +638,11 @@ func TestHandleQueryVote(t *testing.T) {
 	td.enterNextRound(td.consP)
 	td.addPrepareVote(td.consP, td.RandHash(), height, 2, tIndexY)
 
-	t.Run("Query vote for round 0: should send the decided vote for the round 1", func(t *testing.T) {
+	t.Run("Query vote for round 0: should send the decided vote for the round 0", func(t *testing.T) {
 		rndVote := td.consP.HandleQueryVote(height, 0)
 		assert.Equal(t, vote.VoteTypeCPDecided, rndVote.Type())
 		assert.Equal(t, height, rndVote.Height())
-		assert.Equal(t, int16(1), rndVote.Round())
+		assert.Equal(t, int16(0), rndVote.Round())
 	})
 
 	t.Run("Query vote for round 1: should send the decided vote for the round 1", func(t *testing.T) {
@@ -652,7 +652,7 @@ func TestHandleQueryVote(t *testing.T) {
 		assert.Equal(t, int16(1), rndVote.Round())
 	})
 
-	t.Run("Query vote for round 2: should send the prepare vote for the current round", func(t *testing.T) {
+	t.Run("Query vote for round 2: should send the prepare vote for the round 2", func(t *testing.T) {
 		rndVote := td.consP.HandleQueryVote(height, 2)
 		assert.Equal(t, vote.VoteTypePrepare, rndVote.Type())
 		assert.Equal(t, height, rndVote.Height())
@@ -837,10 +837,10 @@ func TestCases(t *testing.T) {
 		description string
 	}{
 		{1697898884837384019, 2, "1/3+ cp:PRE-VOTE in Prepare step"},
-		{1694848907840926239, 2, "1/3+ cp:PRE-VOTE in Precommit step"},
-		{1732698646319341342, 1, "Conflicting cp:PRE-VOTE in cp_round 0"},
-		{1732698786369716238, 1, "Conflicting cp:PRE-VOTE in cp_round 1"},
-		{1732702222972506364, 1, "consX & consY: Change Proposer, consB & consP: Committed block"},
+		{1734526933123806220, 1, "1/3+ cp:PRE-VOTE in Precommit step"},
+		{1734526832618973590, 1, "Conflicting cp:PRE-VOTE in cp_round=0"},
+		{1734527064850322674, 2, "Conflicting cp:PRE-VOTE in cp_round=1"},
+		{1734526579569939721, 1, "consP & consB: Change Proposer, consX & consY: Commit (2 block announces)"},
 	}
 
 	for no, tt := range tests {

consensus/cp.go

@@ -310,22 +310,21 @@ func (cp *changeProposer) checkJust(vte *vote.Vote) error {
 	}
 }
 
-func (cp *changeProposer) cpStrongTermination() {
-	cpDecided := cp.log.CPDecidedVoteSet(cp.round)
-	if cpDecided.HasAnyVoteFor(cp.cpRound, vote.CPValueNo) {
+func (cp *changeProposer) cpStrongTermination(round, cpRound int16) {
+	cpDecided := cp.log.CPDecidedVoteSet(round)
+	if cpDecided.HasAnyVoteFor(cpRound, vote.CPValueNo) {
+		cp.round = round
 		cp.cpDecided = 0
 
-		roundProposal := cp.log.RoundProposal(cp.round)
+		roundProposal := cp.log.RoundProposal(round)
 		if roundProposal == nil {
 			cp.queryProposal()
 		}
 		cp.enterNewState(cp.prepareState)
-	} else if cpDecided.HasAnyVoteFor(cp.cpRound, vote.CPValueYes) {
-		cp.round++
+	} else if cpDecided.HasAnyVoteFor(cpRound, vote.CPValueYes) {
+		cp.round = round + 1
 		cp.cpDecided = 1
-		cp.enterNewState(cp.proposeState)
 
-		// Check if there is any decided vote for the next round.
-		cp.cpStrongTermination()
+		cp.enterNewState(cp.proposeState)
 	}
 }

consensus/cp_decide.go

@@ -26,6 +26,7 @@ func (s *cpDecideState) decide() {
 				QCert: cert,
 			}
 			s.signAddCPDecidedVote(hash.UndefHash, s.cpRound, vote.CPValueYes, just)
+			s.cpStrongTermination(s.round, s.cpRound)
 		} else if cpMainVotes.HasQuorumVotesFor(s.cpRound, vote.CPValueNo) {
 			// decided for no and proceeds to the next round
 			s.logger.Info("binary agreement decided", "value", 0, "round", s.cpRound)
@@ -36,6 +37,7 @@ func (s *cpDecideState) decide() {
 				QCert: cert,
 			}
 			s.signAddCPDecidedVote(*s.cpWeakValidity, s.cpRound, vote.CPValueNo, just)
+			s.cpStrongTermination(s.round, s.cpRound)
 		} else {
 			// conflicting votes
 			s.logger.Debug("conflicting main votes", "round", s.cpRound)
@@ -45,12 +47,14 @@ func (s *cpDecideState) decide() {
 	}
 }
 
-func (s *cpDecideState) onAddVote(v *vote.Vote) {
-	if v.Type() == vote.VoteTypeCPMainVote {
+func (s *cpDecideState) onAddVote(vte *vote.Vote) {
+	if vte.Type() == vote.VoteTypeCPMainVote {
 		s.decide()
 	}
 
-	s.cpStrongTermination()
+	if vte.IsCPVote() {
+		s.cpStrongTermination(vte.Round(), vte.CPRound())
+	}
 }
 
 func (*cpDecideState) name() string {

consensus/cp_mainvote.go

@@ -86,12 +86,14 @@ func (s *cpMainVoteState) detectByzantineProposal() {
 	}
 }
 
-func (s *cpMainVoteState) onAddVote(v *vote.Vote) {
-	if v.Type() == vote.VoteTypeCPPreVote {
+func (s *cpMainVoteState) onAddVote(vte *vote.Vote) {
+	if vte.Type() == vote.VoteTypeCPPreVote {
 		s.decide()
 	}
 
-	s.cpStrongTermination()
+	if vte.IsCPVote() {
+		s.cpStrongTermination(vte.Round(), vte.CPRound())
+	}
 }
 
 func (*cpMainVoteState) name() string {

consensus/log/log_test.go

@@ -64,14 +64,14 @@ func TestAddInvalidVoteType(t *testing.T) {
 	log := NewLog()
 	log.MoveToNewHeight(cmt.Validators())
 
-	data, _ := hex.DecodeString("A701050218320301045820BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB" +
+	data, _ := hex.DecodeString("A7010F0218320301045820BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB" +
 		"055501AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA06f607f6")
 	invVote := new(vote.Vote)
 	err := invVote.UnmarshalCBOR(data)
 	assert.NoError(t, err)
 
 	added, err := log.AddVote(invVote)
-	assert.Error(t, err)
+	assert.ErrorContains(t, err, "unexpected vote type: 15")
 	assert.False(t, added)
 	assert.False(t, log.HasVote(invVote.Hash()))
 }

types/vote/vote_type.go

@@ -1,5 +1,7 @@
 package vote
 
+import "fmt"
+
 type Type int
 
 const (
@@ -33,6 +35,6 @@ func (t Type) String() string {
 	case VoteTypeCPDecided:
 		return "DECIDED"
 	default:
-		return ("invalid vote type")
+		return fmt.Sprintf("%d", t)
 	}
 }