Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions consensus/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
16 changes: 8 additions & 8 deletions consensus/consensus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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())
Expand Down Expand Up @@ -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 {
Expand All @@ -862,7 +862,7 @@ func TestCases(t *testing.T) {
}

func TestFaulty(t *testing.T) {
for i := 0; i < 10; i++ {
for i := 0; i < 100; i++ {
b00f marked this conversation as resolved.
Show resolved Hide resolved
td := setup(t)
td.commitBlockForAllStates(t)

Expand Down
17 changes: 8 additions & 9 deletions consensus/cp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
10 changes: 7 additions & 3 deletions consensus/cp_decide.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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 {
Expand Down
8 changes: 5 additions & 3 deletions consensus/cp_mainvote.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions consensus/log/log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
}
Expand Down
4 changes: 3 additions & 1 deletion types/vote/vote_type.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package vote

import "fmt"

type Type int

const (
Expand Down Expand Up @@ -33,6 +35,6 @@
case VoteTypeCPDecided:
return "DECIDED"
default:
return ("invalid vote type")
return fmt.Sprintf("%d", t)

Check warning on line 38 in types/vote/vote_type.go

View check run for this annotation

Codecov / codecov/patch

types/vote/vote_type.go#L38

Added line #L38 was not covered by tests
}
}
Loading