evidence: don't gossip consensus evidence too soon (#5528)

and don't return errors on seeing the same evidence twice

consensus/state.go

@@ -1871,10 +1871,13 @@ func (cs *State) tryAddVote(vote *types.Vote, peerID p2p.ID) (bool, error) {
 			} else {
 				timestamp = sm.MedianTime(cs.LastCommit.MakeCommit(), cs.LastValidators)
 			}
-			evidenceErr := cs.evpool.AddEvidenceFromConsensus(
-				types.NewDuplicateVoteEvidence(voteErr.VoteA, voteErr.VoteB), timestamp, cs.Validators)
+			evidence := types.NewDuplicateVoteEvidence(voteErr.VoteA, voteErr.VoteB)
+			evidenceErr := cs.evpool.AddEvidenceFromConsensus(evidence, timestamp, cs.Validators)
+
 			if evidenceErr != nil {
 				cs.Logger.Error("Failed to add evidence to the evidence pool", "err", evidenceErr)
+			} else {
+				cs.Logger.Debug("Added evidence to the evidence pool", "evidence", evidence)
 			}
 			return added, err
 		} else if err == types.ErrVoteNonDeterministicSignature {

evidence/pool.go

@@ -122,7 +122,8 @@ func (evpool *Pool) AddEvidence(ev types.Evidence) error {
 
 	// We have already verified this piece of evidence - no need to do it again
 	if evpool.isPending(ev) {
-		return errors.New("evidence already verified and added")
+		evpool.logger.Info("Evidence already pending, ignoring this one", "ev", ev)
+		return nil
 	}
 
 	// 1) Verify against state.
@@ -152,8 +153,10 @@ func (evpool *Pool) AddEvidenceFromConsensus(ev types.Evidence, time time.Time,
 		totalPower int64
 	)
 
+	// we already have this evidence, log this but don't return an error.
 	if evpool.isPending(ev) {
-		return errors.New("evidence already verified and added") // we already have this evidence
+		evpool.logger.Info("Evidence already pending, ignoring this one", "ev", ev)
+		return nil
 	}
 
 	switch ev := ev.(type) {
@@ -175,7 +178,7 @@ func (evpool *Pool) AddEvidenceFromConsensus(ev types.Evidence, time time.Time,
 	if err := evpool.addPendingEvidence(evInfo); err != nil {
 		return fmt.Errorf("can't add evidence to pending list: %w", err)
 	}
-
+	// add evidence to be gossiped with peers
 	evpool.evidenceList.PushBack(ev)
 
 	evpool.logger.Info("Verified new evidence of byzantine behavior", "evidence", ev)

evidence/pool_test.go

@@ -88,7 +88,7 @@ func TestEvidencePoolBasic(t *testing.T) {
 	assert.Equal(t, int64(357), size) // check that the size of the single evidence in bytes is correct
 
 	// shouldn't be able to add evidence twice
-	assert.Error(t, pool.AddEvidence(ev))
+	assert.NoError(t, pool.AddEvidence(ev))
 	evs, _ = pool.PendingEvidence(defaultEvidenceMaxBytes)
 	assert.Equal(t, 1, len(evs))
 
@@ -147,17 +147,18 @@ func TestAddEvidenceFromConsensus(t *testing.T) {
 	var height int64 = 10
 	pool, val := defaultTestPool(height)
 	ev := types.NewMockDuplicateVoteEvidenceWithValidator(height, defaultEvidenceTime, val, evidenceChainID)
-	err := pool.AddEvidenceFromConsensus(ev, defaultEvidenceTime,
-		types.NewValidatorSet([]*types.Validator{val.ExtractIntoValidator(2)}))
+	valSet := types.NewValidatorSet([]*types.Validator{val.ExtractIntoValidator(2)})
+	err := pool.AddEvidenceFromConsensus(ev, defaultEvidenceTime, valSet)
 	assert.NoError(t, err)
 	next := pool.EvidenceFront()
 	assert.Equal(t, ev, next.Value.(types.Evidence))
+
 	// shouldn't be able to submit the same evidence twice
 	err = pool.AddEvidenceFromConsensus(ev, defaultEvidenceTime.Add(-1*time.Second),
 		types.NewValidatorSet([]*types.Validator{val.ExtractIntoValidator(3)}))
-	if assert.Error(t, err) {
-		assert.Equal(t, "evidence already verified and added", err.Error())
-	}
+	assert.NoError(t, err)
+	evs, _ := pool.PendingEvidence(defaultEvidenceMaxBytes)
+	assert.Equal(t, 1, len(evs))
 }
 
 func TestEvidencePoolUpdate(t *testing.T) {

evidence/reactor.go

@@ -177,7 +177,7 @@ func (evR Reactor) checkSendEvidenceMessage(
 		ageNumBlocks = peerHeight - evHeight
 	)
 
-	if peerHeight < evHeight { // peer is behind. sleep while he catches up
+	if peerHeight <= evHeight { // peer is behind. sleep while he catches up
 		return nil, true
 	} else if ageNumBlocks > params.MaxAgeNumBlocks { // evidence is too old relative to the peer, skip
 

evidence/reactor_test.go

@@ -215,7 +215,7 @@ func TestReactorSelectiveBroadcast(t *testing.T) {
 	evList := sendEvidence(t, pools[0], val, numEvidence)
 
 	// only ones less than the peers height should make it through
-	waitForEvidence(t, evList[:numEvidence/2], pools[1:2])
+	waitForEvidence(t, evList[:numEvidence/2-1], pools[1:2])
 
 	// peers should still be connected
 	peers := reactors[1].Switch.Peers().List()