diff --git a/consensus/consensus.go b/consensus/consensus.go index e84661efd..13f2023b3 100644 --- a/consensus/consensus.go +++ b/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: diff --git a/consensus/consensus_test.go b/consensus/consensus_test.go index 9b74a4bdb..0d8adae12 100644 --- a/consensus/consensus_test.go +++ b/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 { diff --git a/consensus/cp.go b/consensus/cp.go index 600eb51c8..9e4fe416b 100644 --- a/consensus/cp.go +++ b/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) } } diff --git a/consensus/cp_decide.go b/consensus/cp_decide.go index 07d82b914..403a7ac41 100644 --- a/consensus/cp_decide.go +++ b/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 { diff --git a/consensus/cp_mainvote.go b/consensus/cp_mainvote.go index 1685c9b86..8a47a47d6 100644 --- a/consensus/cp_mainvote.go +++ b/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 { diff --git a/consensus/log/log_test.go b/consensus/log/log_test.go index 971b960f2..ab5b86da0 100644 --- a/consensus/log/log_test.go +++ b/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())) } diff --git a/types/vote/vote_type.go b/types/vote/vote_type.go index 1760101a0..164d20853 100644 --- a/types/vote/vote_type.go +++ b/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) } }