e2e: clean up generation of evidence (#6904)

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) {

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

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(

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 {

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,

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")
 		})