diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 7c76703f1..1da463e37 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -66,3 +66,5 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - [privval] \#5638 Increase read/write timeout to 5s and calculate ping interval based on it (@JoeKash) - [blockchain/v1] [\#5701](https://github.com/tendermint/tendermint/pull/5701) Handle peers without blocks (@melekes) - [blockchain/v1] \#5711 Fix deadlock (@melekes) +- [evidence] \#5890 Add a buffer to evidence from consensus to avoid broadcasting and proposing evidence before the +height of such an evidence has finished (@cmwaters) \ No newline at end of file diff --git a/evidence/pool.go b/evidence/pool.go index cf425d988..6cb47069f 100644 --- a/evidence/pool.go +++ b/evidence/pool.go @@ -43,6 +43,10 @@ type Pool struct { mtx sync.Mutex // latest state state sm.State + // evidence from consensus if buffered to this slice, awaiting until the next height + // before being flushed to the pool. This prevents broadcasting and proposing of + // evidence before the height with which the evidence happened is finished. + consensusBuffer []types.Evidence pruningHeight int64 pruningTime time.Time @@ -57,12 +61,13 @@ func NewPool(logger log.Logger, evidenceDB dbm.DB, stateDB sm.Store, blockStore } pool := &Pool{ - stateDB: stateDB, - blockStore: blockStore, - state: state, - logger: logger, - evidenceStore: evidenceDB, - evidenceList: clist.New(), + stateDB: stateDB, + blockStore: blockStore, + state: state, + logger: logger, + evidenceStore: evidenceDB, + evidenceList: clist.New(), + consensusBuffer: make([]types.Evidence, 0), } // If pending evidence already in db, in event of prior failure, then check @@ -115,7 +120,14 @@ func (evpool *Pool) Update(state sm.State, ev types.EvidenceList) { "last_block_time", state.LastBlockTime, ) - evpool.updateState(state) + evpool.mtx.Lock() + // flush awaiting evidence from consensus into pool + evpool.flushConsensusBuffer() + // update state + evpool.state = state + evpool.mtx.Unlock() + + // move committed evidence out from the pending pool and into the committed pool evpool.markEvidenceAsCommitted(ev) // Prune pending evidence when it has expired. This also updates when the next @@ -170,14 +182,14 @@ func (evpool *Pool) AddEvidenceFromConsensus(ev types.Evidence) error { return nil } - if err := evpool.addPendingEvidence(ev); err != nil { - return fmt.Errorf("failed to add evidence to pending list: %w", err) - } + // add evidence to a buffer which will pass the evidence to the pool at the following height. + // This avoids the issue of some nodes verifying and proposing evidence at a height where the + // block hasn't been committed on cause others to potentially fail. + evpool.mtx.Lock() + defer evpool.mtx.Unlock() + evpool.consensusBuffer = append(evpool.consensusBuffer, ev) + evpool.logger.Info("received new evidence of byzantine behavior from consensus", "evidence", ev) - // add evidence to be gossiped with peers - evpool.evidenceList.PushBack(ev) - - evpool.logger.Info("verified new evidence of byzantine behavior", "evidence", ev) return nil } @@ -520,10 +532,19 @@ func (evpool *Pool) removeEvidenceFromList( } } -func (evpool *Pool) updateState(state sm.State) { - evpool.mtx.Lock() - defer evpool.mtx.Unlock() - evpool.state = state +// flushConsensusBuffer moves the evidence produced from consensus into the evidence pool +// and list so that it can be broadcasted and proposed +func (evpool *Pool) flushConsensusBuffer() { + for _, ev := range evpool.consensusBuffer { + if err := evpool.addPendingEvidence(ev); err != nil { + evpool.logger.Error("failed to flush evidence from consensus buffer to pending list: %w", err) + continue + } + + evpool.evidenceList.PushBack(ev) + } + // reset consensus buffer + evpool.consensusBuffer = make([]types.Evidence, 0) } func bytesToEv(evBytes []byte) (types.Evidence, error) { diff --git a/evidence/pool_test.go b/evidence/pool_test.go index f9b540ac0..a246219d5 100644 --- a/evidence/pool_test.go +++ b/evidence/pool_test.go @@ -143,13 +143,31 @@ func TestAddEvidenceFromConsensus(t *testing.T) { ev := types.NewMockDuplicateVoteEvidenceWithValidator(height, defaultEvidenceTime, val, evidenceChainID) require.NoError(t, pool.AddEvidenceFromConsensus(ev)) + + // evidence from consensus should not be added immediately but reside in the consensus buffer + evList, evSize := pool.PendingEvidence(defaultEvidenceMaxBytes) + require.Empty(t, evList) + require.Zero(t, evSize) + next := pool.EvidenceFront() - require.Equal(t, ev, next.Value.(types.Evidence)) + require.Nil(t, next) + + // move to next height and update state and evidence pool + state := pool.State() + state.LastBlockHeight++ + pool.Update(state, []types.Evidence{}) + + // should be able to retrieve evidence from pool + evList, _ = pool.PendingEvidence(defaultEvidenceMaxBytes) + require.Equal(t, []types.Evidence{ev}, evList) // shouldn't be able to submit the same evidence twice require.NoError(t, pool.AddEvidenceFromConsensus(ev)) - evs, _ := pool.PendingEvidence(defaultEvidenceMaxBytes) - require.Equal(t, 1, len(evs)) + state = pool.State() + state.LastBlockHeight++ + pool.Update(state, []types.Evidence{}) + evList2, _ := pool.PendingEvidence(defaultEvidenceMaxBytes) + require.Equal(t, evList, evList2) } func TestEvidencePoolUpdate(t *testing.T) {