Skip to content
This repository has been archived by the owner on Aug 24, 2022. It is now read-only.

Commit

Permalink
Fix race and nil return in precommits/votes (#220)
Browse files Browse the repository at this point in the history
* Bug fux on unit tests

* Fix race

* Bug fix timestamp test

* Fix signing in test
  • Loading branch information
jinmannwong authored Oct 29, 2020
1 parent 15c6dec commit af5f49d
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 18 deletions.
10 changes: 5 additions & 5 deletions consensus/reactor.go
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ OUTER_LOOP:
sleeping = 1
logger.Debug("No votes to send, sleeping", "rs.Height", rs.Height, "prs.Height", prs.Height,
"localPV", rs.Votes.Prevotes(rs.Round).BitArray(), "peerPV", prs.Prevotes,
"localPC", rs.Votes.Precommits(rs.Round).BitArray(), "peerPC", prs.Precommits.BitArray(conR.conS.Validators.Size()))
"localPC", rs.Votes.Precommits(rs.Round).BitArray(), "peerPC", prs.Precommits.BitArray())
} else if sleeping == 2 {
// Continued sleep...
sleeping = 1
Expand Down Expand Up @@ -1208,7 +1208,7 @@ func (ps *PeerState) ensureCatchupCommitRound(height int64, round int, numValida
if round == ps.PRS.Round {
ps.PRS.CatchupCommit = ps.PRS.Precommits
} else {
ps.PRS.CatchupCommit = cstypes.NewPrecommitRecord()
ps.PRS.CatchupCommit = cstypes.NewPrecommitRecord(numValidators)
}
}

Expand All @@ -1228,17 +1228,17 @@ func (ps *PeerState) ensureVoteBitArrays(height int64, numValidators int) {
ps.PRS.Prevotes = bits.NewBitArray(numValidators)
}
if ps.PRS.Precommits == nil {
ps.PRS.Precommits = cstypes.NewPrecommitRecord()
ps.PRS.Precommits = cstypes.NewPrecommitRecord(numValidators)
}
if ps.PRS.CatchupCommit == nil {
ps.PRS.CatchupCommit = cstypes.NewPrecommitRecord()
ps.PRS.CatchupCommit = cstypes.NewPrecommitRecord(numValidators)
}
if ps.PRS.ProposalPOL == nil {
ps.PRS.ProposalPOL = bits.NewBitArray(numValidators)
}
} else if ps.PRS.Height == height+1 {
if ps.PRS.LastCommit == nil {
ps.PRS.LastCommit = cstypes.NewPrecommitRecord()
ps.PRS.LastCommit = cstypes.NewPrecommitRecord(numValidators)
}
}
}
Expand Down
8 changes: 5 additions & 3 deletions consensus/timestamp_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package consensus

import (
"fmt"
"math/rand"
"testing"
"time"
Expand All @@ -27,7 +28,7 @@ func TestReactorConflictingTimestamps(t *testing.T) {
css, cleanup := randConsensusNet(N, "consensus_reactor_test", newMockTickerFunc(true), newCounter)
defer cleanup()

for i := 0; i < 4; i++ {
for i := 0; i < N; i++ {
ticker := NewTimeoutTicker()
ticker.SetLogger(css[i].Logger)
css[i].SetTimeoutTicker(ticker)
Expand Down Expand Up @@ -55,6 +56,7 @@ func TestReactorConflictingTimestamps(t *testing.T) {
for i := 0; i < 10; i++ {
timeoutWaitGroup(t, N, func(j int) {
<-blocksSubs[j].Out()
fmt.Printf("Complete block height %v index %v\n", i, j)
}, css)
}
})
Expand Down Expand Up @@ -95,7 +97,7 @@ func invalidVoteFunc(t *testing.T, height int64, round int, cs *State, sw *p2p.S
PartsHeader: blockPartsHeader,
},
}
cs.privValidator.SignVote(cs.state.ChainID, vote1)
cs.privValidator.SignVote(types.VotePrefix(cs.state.ChainID, cs.Validators.Hash()), vote1)
// vote2
vote2 := &types.Vote{
ValidatorAddress: addr,
Expand All @@ -109,7 +111,7 @@ func invalidVoteFunc(t *testing.T, height int64, round int, cs *State, sw *p2p.S
PartsHeader: blockPartsHeader,
},
}
cs.privValidator.SignVote(cs.state.ChainID, vote2)
cs.privValidator.SignVote(types.VotePrefix(cs.state.ChainID, cs.Validators.Hash()), vote2)
cs.mtx.Unlock()

peers := sw.Peers().List()
Expand Down
12 changes: 10 additions & 2 deletions consensus/types/height_vote_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,11 @@ func (hvs *HeightVoteSet) AddVote(vote *types.Vote, peerID p2p.ID) (added bool,
func (hvs *HeightVoteSet) Prevotes(round int) *types.PrevoteSet {
hvs.mtx.Lock()
defer hvs.mtx.Unlock()
prevotes, ok := hvs.getVoteSet(round, types.PrevoteType).(*types.PrevoteSet)
voteSet := hvs.getVoteSet(round, types.PrevoteType)
if voteSet == nil {
return nil
}
prevotes, ok := voteSet.(*types.PrevoteSet)
if !ok {
panic(fmt.Sprintf("Could not convert HVS.Prevotes to PrevoteSet for round %v", round))
}
Expand All @@ -146,7 +150,11 @@ func (hvs *HeightVoteSet) Prevotes(round int) *types.PrevoteSet {
func (hvs *HeightVoteSet) Precommits(round int) *types.PrecommitSet {
hvs.mtx.Lock()
defer hvs.mtx.Unlock()
precommits, ok := hvs.getVoteSet(round, types.PrecommitType).(*types.PrecommitSet)
voteSet := hvs.getVoteSet(round, types.PrecommitType)
if voteSet == nil {
return nil
}
precommits, ok := voteSet.(*types.PrecommitSet)
if !ok {
panic(fmt.Sprintf("Could not convert HVS.Precommits to PrecommitSet for round %v", round))
}
Expand Down
14 changes: 8 additions & 6 deletions consensus/types/peer_round_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,15 @@ func (prs *PeerRoundState) Unmarshal(bs []byte) error {
// Thread safe map for recording precommits seen by peer

type PrecommitRecord struct {
record map[string]struct{} // All precommits peer has, identified by validator index and timestamp
mtx sync.RWMutex
record map[string]struct{} // All precommits peer has, identified by validator index and timestamp
numVals int
mtx sync.RWMutex
}

func NewPrecommitRecord() *PrecommitRecord {
func NewPrecommitRecord(numVals int) *PrecommitRecord {
return &PrecommitRecord{
record: map[string]struct{}{},
record: map[string]struct{}{},
numVals: numVals,
}
}

Expand All @@ -130,15 +132,15 @@ func (pr *PrecommitRecord) SetHasVote(identifier string) {

// BitArray returns bit array of whether peer has seen a precommit from a certain validator, identified
// by their index
func (pr *PrecommitRecord) BitArray(numValidators int) *bits.BitArray {
func (pr *PrecommitRecord) BitArray() *bits.BitArray {
if pr == nil {
return nil
}

pr.mtx.RLock()
defer pr.mtx.RUnlock()

bitArray := bits.NewBitArray(numValidators)
bitArray := bits.NewBitArray(pr.numVals)
for key := range pr.record {
bitArray.SetIndex(types.PrecommitIdentifierToIndex(key), true)
}
Expand Down
4 changes: 2 additions & 2 deletions types/precommit_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ func (voteSet *PrecommitSet) BitArray() *bits.BitArray {

func (voteSet *PrecommitSet) VotesByBlockID(blockID BlockID) []string {
if voteSet == nil {
return nil
return []string{}
}
voteSet.mtx.Lock()
defer voteSet.mtx.Unlock()
Expand All @@ -344,7 +344,7 @@ func (voteSet *PrecommitSet) VotesByBlockID(blockID BlockID) []string {
}
return votes
}
return nil
return []string{}
}

// GetByIndex returns all precommit votes from a validator index as a map with timestamp
Expand Down

0 comments on commit af5f49d

Please sign in to comment.