From 8fe651ba30dac26dd3ebf092f2623dfb4b7ba851 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Tue, 7 Sep 2021 12:20:43 +0200 Subject: [PATCH] e2e: clean up generation of evidence (#6904) --- internal/evidence/pool.go | 17 ++++++++++++----- internal/evidence/reactor_test.go | 3 ++- rpc/client/evidence_test.go | 6 ++++-- test/e2e/runner/evidence.go | 27 ++++++++++++++++++++------- types/evidence.go | 29 +++++++++++++++++++---------- types/evidence_test.go | 3 ++- 6 files changed, 59 insertions(+), 26 deletions(-) diff --git a/internal/evidence/pool.go b/internal/evidence/pool.go index fa722f4f1..2a48fe032 100644 --- a/internal/evidence/pool.go +++ b/internal/evidence/pool.go @@ -516,10 +516,13 @@ func (evpool *Pool) processConsensusBuffer(state sm.State) { // Check the height of the conflicting votes and fetch the corresponding time and validator set // to produce the valid evidence - var dve *types.DuplicateVoteEvidence + var ( + dve *types.DuplicateVoteEvidence + err error + ) switch { case voteSet.VoteA.Height == state.LastBlockHeight: - dve = types.NewDuplicateVoteEvidence( + dve, err = types.NewDuplicateVoteEvidence( voteSet.VoteA, voteSet.VoteB, state.LastBlockTime, @@ -527,8 +530,8 @@ func (evpool *Pool) processConsensusBuffer(state sm.State) { ) case voteSet.VoteA.Height < state.LastBlockHeight: - valSet, err := evpool.stateDB.LoadValidators(voteSet.VoteA.Height) - if err != nil { + valSet, dbErr := evpool.stateDB.LoadValidators(voteSet.VoteA.Height) + if dbErr != nil { evpool.logger.Error("failed to load validator set for conflicting votes", "height", voteSet.VoteA.Height, "err", err) continue @@ -538,7 +541,7 @@ func (evpool *Pool) processConsensusBuffer(state sm.State) { evpool.logger.Error("failed to load block time for conflicting votes", "height", voteSet.VoteA.Height) continue } - dve = types.NewDuplicateVoteEvidence( + dve, err = types.NewDuplicateVoteEvidence( voteSet.VoteA, voteSet.VoteB, blockMeta.Header.Time, @@ -554,6 +557,10 @@ func (evpool *Pool) processConsensusBuffer(state sm.State) { "state.LastBlockHeight", state.LastBlockHeight) continue } + if err != nil { + evpool.logger.Error("error in generating evidence from votes", "err", err) + continue + } // check if we already have this evidence if evpool.isPending(dve) { diff --git a/internal/evidence/reactor_test.go b/internal/evidence/reactor_test.go index b098eb373..1cf995731 100644 --- a/internal/evidence/reactor_test.go +++ b/internal/evidence/reactor_test.go @@ -534,12 +534,13 @@ func TestEvidenceListSerialization(t *testing.T) { valSet := types.NewValidatorSet([]*types.Validator{val}) - dupl := types.NewDuplicateVoteEvidence( + dupl, err := types.NewDuplicateVoteEvidence( exampleVote(1), exampleVote(2), defaultEvidenceTime, valSet, ) + require.NoError(t, err) testCases := map[string]struct { evidenceList []types.Evidence diff --git a/rpc/client/evidence_test.go b/rpc/client/evidence_test.go index 0ff158e56..5626b7f48 100644 --- a/rpc/client/evidence_test.go +++ b/rpc/client/evidence_test.go @@ -29,7 +29,7 @@ var defaultTestTime = time.Date(2018, 10, 10, 8, 20, 13, 695936996, time.UTC) func newEvidence(t *testing.T, val *privval.FilePV, vote *types.Vote, vote2 *types.Vote, chainID string) *types.DuplicateVoteEvidence { - + t.Helper() var err error v := vote.ToProto() @@ -44,7 +44,9 @@ func newEvidence(t *testing.T, val *privval.FilePV, validator := types.NewValidator(val.Key.PubKey, 10) valSet := types.NewValidatorSet([]*types.Validator{validator}) - return types.NewDuplicateVoteEvidence(vote, vote2, defaultTestTime, valSet) + ev, err := types.NewDuplicateVoteEvidence(vote, vote2, defaultTestTime, valSet) + require.NoError(t, err) + return ev } func makeEvidences( diff --git a/test/e2e/runner/evidence.go b/test/e2e/runner/evidence.go index 35646ccdb..9249d7958 100644 --- a/test/e2e/runner/evidence.go +++ b/test/e2e/runner/evidence.go @@ -197,10 +197,10 @@ func generateDuplicateVoteEvidence( chainID string, time time.Time, ) (*types.DuplicateVoteEvidence, error) { - // nolint:gosec // G404: Use of weak random number generator - privVal := privVals[rand.Intn(len(privVals))] - - valIdx, _ := vals.GetByAddress(privVal.PrivKey.PubKey().Address()) + privVal, valIdx, err := getRandomValidatorIndex(privVals, vals) + if err != nil { + return nil, err + } voteA, err := factory.MakeVote(privVal, chainID, valIdx, height, 0, 2, makeRandomBlockID(), time) if err != nil { return nil, err @@ -209,14 +209,27 @@ func generateDuplicateVoteEvidence( if err != nil { return nil, err } - ev := types.NewDuplicateVoteEvidence(voteA, voteB, time, vals) - if ev == nil { - return nil, fmt.Errorf("could not generate evidence a=%v b=%v vals=%v", voteA, voteB, vals) + ev, err := types.NewDuplicateVoteEvidence(voteA, voteB, time, vals) + if err != nil { + return nil, fmt.Errorf("could not generate evidence: %w", err) } return ev, nil } +// getRandomValidatorIndex picks a random validator from a slice of mock PrivVals that's +// also part of the validator set, returning the PrivVal and its index in the validator set +func getRandomValidatorIndex(privVals []types.MockPV, vals *types.ValidatorSet) (types.MockPV, int32, error) { + for _, idx := range rand.Perm(len(privVals)) { + pv := privVals[idx] + valIdx, _ := vals.GetByAddress(pv.PrivKey.PubKey().Address()) + if valIdx >= 0 { + return pv, valIdx, nil + } + } + return types.MockPV{}, -1, errors.New("no private validator found in validator set") +} + func readPrivKey(keyFilePath string) (crypto.PrivKey, error) { keyJSONBytes, err := ioutil.ReadFile(keyFilePath) if err != nil { diff --git a/types/evidence.go b/types/evidence.go index 40ff85e5e..330850ea3 100644 --- a/types/evidence.go +++ b/types/evidence.go @@ -46,15 +46,20 @@ type DuplicateVoteEvidence struct { var _ Evidence = &DuplicateVoteEvidence{} // NewDuplicateVoteEvidence creates DuplicateVoteEvidence with right ordering given -// two conflicting votes. If one of the votes is nil, evidence returned is nil as well -func NewDuplicateVoteEvidence(vote1, vote2 *Vote, blockTime time.Time, valSet *ValidatorSet) *DuplicateVoteEvidence { +// two conflicting votes. If either of the votes is nil, the val set is nil or the voter is +// not in the val set, an error is returned +func NewDuplicateVoteEvidence(vote1, vote2 *Vote, blockTime time.Time, valSet *ValidatorSet, +) (*DuplicateVoteEvidence, error) { var voteA, voteB *Vote - if vote1 == nil || vote2 == nil || valSet == nil { - return nil + if vote1 == nil || vote2 == nil { + return nil, errors.New("missing vote") + } + if valSet == nil { + return nil, errors.New("missing validator set") } idx, val := valSet.GetByAddress(vote1.ValidatorAddress) if idx == -1 { - return nil + return nil, errors.New("validator not in validator set") } if strings.Compare(vote1.BlockID.Key(), vote2.BlockID.Key()) == -1 { @@ -70,7 +75,7 @@ func NewDuplicateVoteEvidence(vote1, vote2 *Vote, blockTime time.Time, valSet *V TotalVotingPower: valSet.TotalVotingPower(), ValidatorPower: val.VotingPower, Timestamp: blockTime, - } + }, nil } // ABCI returns the application relevant representation of the evidence @@ -92,7 +97,7 @@ func (dve *DuplicateVoteEvidence) Bytes() []byte { pbe := dve.ToProto() bz, err := pbe.Marshal() if err != nil { - panic(err) + panic("marshaling duplicate vote evidence to bytes: " + err.Error()) } return bz @@ -260,11 +265,11 @@ func (l *LightClientAttackEvidence) ABCI() []abci.Evidence { func (l *LightClientAttackEvidence) Bytes() []byte { pbe, err := l.ToProto() if err != nil { - panic(err) + panic("converting light client attack evidence to proto: " + err.Error()) } bz, err := pbe.Marshal() if err != nil { - panic(err) + panic("marshaling light client attack evidence to bytes: " + err.Error()) } return bz } @@ -684,7 +689,11 @@ func NewMockDuplicateVoteEvidenceWithValidator(height int64, time time.Time, vB := voteB.ToProto() _ = pv.SignVote(context.Background(), chainID, vB) voteB.Signature = vB.Signature - return NewDuplicateVoteEvidence(voteA, voteB, time, NewValidatorSet([]*Validator{val})) + ev, err := NewDuplicateVoteEvidence(voteA, voteB, time, NewValidatorSet([]*Validator{val})) + if err != nil { + panic("constructing mock duplicate vote evidence: " + err.Error()) + } + return ev } func makeMockVote(height int64, round, index int32, addr Address, diff --git a/types/evidence_test.go b/types/evidence_test.go index 9d54797e4..5110bcb1d 100644 --- a/types/evidence_test.go +++ b/types/evidence_test.go @@ -85,7 +85,8 @@ func TestDuplicateVoteEvidenceValidation(t *testing.T) { vote1 := makeVote(t, val, chainID, math.MaxInt32, math.MaxInt64, math.MaxInt32, 0x02, blockID, defaultVoteTime) vote2 := makeVote(t, val, chainID, math.MaxInt32, math.MaxInt64, math.MaxInt32, 0x02, blockID2, defaultVoteTime) valSet := NewValidatorSet([]*Validator{val.ExtractIntoValidator(10)}) - ev := NewDuplicateVoteEvidence(vote1, vote2, defaultVoteTime, valSet) + ev, err := NewDuplicateVoteEvidence(vote1, vote2, defaultVoteTime, valSet) + require.NoError(t, err) tc.malleateEvidence(ev) assert.Equal(t, tc.expectErr, ev.ValidateBasic() != nil, "Validate Basic had an unexpected result") })