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

Fix race and nil return in precommits/votes #220

Merged
merged 4 commits into from
Oct 29, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
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