From 4947333e676f2770149bfea68d42342ff09d47a5 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Tue, 20 Oct 2020 18:47:40 +0200 Subject: [PATCH] evidence: don't gossip consensus evidence too soon (#5528) and don't return errors on seeing the same evidence twice --- consensus/state.go | 7 +++++-- evidence/pool.go | 9 ++++++--- evidence/pool_test.go | 13 +++++++------ evidence/reactor.go | 2 +- evidence/reactor_test.go | 2 +- 5 files changed, 20 insertions(+), 13 deletions(-) diff --git a/consensus/state.go b/consensus/state.go index bed855e6c..939752734 100644 --- a/consensus/state.go +++ b/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 { diff --git a/evidence/pool.go b/evidence/pool.go index 87ae46e3d..74d956019 100644 --- a/evidence/pool.go +++ b/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) diff --git a/evidence/pool_test.go b/evidence/pool_test.go index 40949ef89..9dc657221 100644 --- a/evidence/pool_test.go +++ b/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) { diff --git a/evidence/reactor.go b/evidence/reactor.go index a984008ba..aa2ce6ed2 100644 --- a/evidence/reactor.go +++ b/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 diff --git a/evidence/reactor_test.go b/evidence/reactor_test.go index 8b05ab0fd..2980a692d 100644 --- a/evidence/reactor_test.go +++ b/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()