Skip to content

Commit

Permalink
Merge pull request #1 from b00f/fix/send-decide-vote
Browse files Browse the repository at this point in the history
refactor(consensus): refcator strong termination for change-proposer phase
  • Loading branch information
Ja7ad authored Dec 18, 2024
2 parents 6402581 + 5a7866c commit 27d3dd3
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 26 deletions.
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++ {
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 @@ func (t Type) String() string {
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
}
}

0 comments on commit 27d3dd3

Please sign in to comment.