From 9247b0fbd23659232202696f2aca312d038cb087 Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Thu, 31 Dec 2015 15:02:38 -0800 Subject: [PATCH] Fix HeightVoteSet bug where first catchup vote doesn't get added --- consensus/height_vote_set.go | 3 +- consensus/height_vote_set_test.go | 48 +++++++++++++++++++++++++++++++ types/validator.go | 21 ++++++++++++++ types/validator_set.go | 16 +++++++++++ types/vote_set.go | 21 -------------- types/vote_set_test.go | 12 ++------ 6 files changed, 89 insertions(+), 32 deletions(-) create mode 100644 consensus/height_vote_set_test.go diff --git a/consensus/height_vote_set.go b/consensus/height_vote_set.go index 3ee69391..0d78e199 100644 --- a/consensus/height_vote_set.go +++ b/consensus/height_vote_set.go @@ -95,14 +95,15 @@ func (hvs *HeightVoteSet) AddByIndex(valIndex int, vote *types.Vote, peerKey str if voteSet == nil { if _, ok := hvs.peerCatchupRounds[peerKey]; !ok { hvs.addRound(vote.Round) + voteSet = hvs.getVoteSet(vote.Round, vote.Type) hvs.peerCatchupRounds[peerKey] = vote.Round } else { // Peer has sent a vote that does not match our round, // for more than one round. Bad peer! // TODO punish peer. log.Warn("Deal with peer giving votes from unwanted rounds") + return } - return } added, address, err = voteSet.AddByIndex(valIndex, vote) return diff --git a/consensus/height_vote_set_test.go b/consensus/height_vote_set_test.go new file mode 100644 index 00000000..19257282 --- /dev/null +++ b/consensus/height_vote_set_test.go @@ -0,0 +1,48 @@ +package consensus + +import ( + _ "github.com/tendermint/tendermint/config/tendermint_test" + "github.com/tendermint/tendermint/types" + + "testing" +) + +func TestPeerCatchupRounds(t *testing.T) { + valSet, privVals := types.RandValidatorSet(10, 1) + + hvs := NewHeightVoteSet(1, valSet) + + vote999_0 := makeVoteHR(t, 1, 999, privVals[0]) + added, _, err := hvs.AddByIndex(0, vote999_0, "peer1") + if !added || err != nil { + t.Error("Expected to successfully add vote from peer", added, err) + } + + vote1000_0 := makeVoteHR(t, 1, 1000, privVals[0]) + added, _, err = hvs.AddByIndex(0, vote1000_0, "peer1") + if added { + t.Error("Expected to *not* add vote from peer, too many catchup rounds.") + } + + added, _, err = hvs.AddByIndex(0, vote1000_0, "peer2") + if !added || err != nil { + t.Error("Expected to successfully add vote from another peer") + } + +} + +func makeVoteHR(t *testing.T, height, round int, privVal *types.PrivValidator) *types.Vote { + vote := &types.Vote{ + Height: height, + Round: round, + Type: types.VoteTypePrecommit, + BlockHash: []byte("fakehash"), + } + chainID := config.GetString("chain_id") + err := privVal.SignVote(chainID, vote) + if err != nil { + t.Fatalf("Error signing vote: %v", err) + return nil + } + return vote +} diff --git a/types/validator.go b/types/validator.go index 944ce9d9..699114f2 100644 --- a/types/validator.go +++ b/types/validator.go @@ -83,3 +83,24 @@ func (vc validatorCodec) Compare(o1 interface{}, o2 interface{}) int { PanicSanity("ValidatorCodec.Compare not implemented") return 0 } + +//-------------------------------------------------------------------------------- +// For testing... + +func RandValidator(randPower bool, minPower int64) (*Validator, *PrivValidator) { + privVal := GenPrivValidator() + _, tempFilePath := Tempfile("priv_validator_") + privVal.SetFile(tempFilePath) + votePower := minPower + if randPower { + votePower += int64(RandUint32()) + } + val := &Validator{ + Address: privVal.Address, + PubKey: privVal.PubKey, + LastCommitHeight: 0, + VotingPower: votePower, + Accum: 0, + } + return val, privVal +} diff --git a/types/validator_set.go b/types/validator_set.go index b0017819..a56265bf 100644 --- a/types/validator_set.go +++ b/types/validator_set.go @@ -309,3 +309,19 @@ type accumComparable int64 func (ac accumComparable) Less(o interface{}) bool { return int64(ac) > int64(o.(accumComparable)) } + +//---------------------------------------- +// For testing + +func RandValidatorSet(numValidators int, votingPower int64) (*ValidatorSet, []*PrivValidator) { + vals := make([]*Validator, numValidators) + privValidators := make([]*PrivValidator, numValidators) + for i := 0; i < numValidators; i++ { + val, privValidator := RandValidator(false, votingPower) + vals[i] = val + privValidators[i] = privValidator + } + valSet := NewValidatorSet(vals) + sort.Sort(PrivValidatorsByAddress(privValidators)) + return valSet, privValidators +} diff --git a/types/vote_set.go b/types/vote_set.go index 470aaf59..44ee5345 100644 --- a/types/vote_set.go +++ b/types/vote_set.go @@ -300,24 +300,3 @@ func (voteSet *VoteSet) MakeValidation() *Validation { Precommits: precommits, } } - -//-------------------------------------------------------------------------------- -// For testing... - -func RandValidator(randPower bool, minPower int64) (*Validator, *PrivValidator) { - privVal := GenPrivValidator() - _, tempFilePath := Tempfile("priv_validator_") - privVal.SetFile(tempFilePath) - votePower := minPower - if randPower { - votePower += int64(RandUint32()) - } - val := &Validator{ - Address: privVal.Address, - PubKey: privVal.PubKey, - LastCommitHeight: 0, - VotingPower: votePower, - Accum: 0, - } - return val, privVal -} diff --git a/types/vote_set_test.go b/types/vote_set_test.go index 92686c91..6bbf0544 100644 --- a/types/vote_set_test.go +++ b/types/vote_set_test.go @@ -2,7 +2,6 @@ package types import ( "bytes" - "sort" . "github.com/tendermint/go-common" . "github.com/tendermint/go-common/test" @@ -11,16 +10,9 @@ import ( "testing" ) +// Move it out? func randVoteSet(height int, round int, type_ byte, numValidators int, votingPower int64) (*VoteSet, *ValidatorSet, []*PrivValidator) { - vals := make([]*Validator, numValidators) - privValidators := make([]*PrivValidator, numValidators) - for i := 0; i < numValidators; i++ { - val, privValidator := RandValidator(false, votingPower) - vals[i] = val - privValidators[i] = privValidator - } - valSet := NewValidatorSet(vals) - sort.Sort(PrivValidatorsByAddress(privValidators)) + valSet, privValidators := RandValidatorSet(numValidators, votingPower) return NewVoteSet(height, round, type_, valSet), valSet, privValidators }