Reject blocks with committed evidence (#37)

* evidence: NewEvidencePool takes evidenceDB

* evidence: failing TestStoreCommitDuplicate

tendermint/security#35

* GetEvidence -> GetEvidenceInfo

* fix TestStoreCommitDuplicate

* comment in VerifyEvidence

* add check if evidence was already seen
 - modify EventPool interface (EventStore is not known in ApplyBlock):
   - add IsCommitted method to iface
 - add test

* update changelog

* fix TestStoreMark:
 - priority in evidence info gets reset to zero after evidence gets committed

* review comments: simplify EvidencePool.IsCommitted
 - delete obsolete EvidenceStore.IsCommitted

* add simple test for IsCommitted

* update changelog: this is actually breaking (PR number still missing)

* fix TestStoreMark:
 - priority in evidence info gets reset to zero after evidence gets
 committed

* review suggestion: simplify return

CHANGELOG_PENDING.md

@@ -6,8 +6,23 @@ Special thanks to external contributors on this release:
 
 ### BREAKING CHANGES:
 
+* CLI/RPC/Config
+
+* Apps
+
+* Go API
+
+* Blockchain Protocol
+
+ - [types] Reject blocks which contain already committed evidence
+
+* P2P Protocol
+
 ### FEATURES:
 
 ### IMPROVEMENTS:
 
 ### BUG FIXES:
+
+ - [evidence] Do not store evidence which was already marked as committed
+

consensus/reactor_test.go

@@ -211,6 +211,7 @@ func (m *mockEvidencePool) Update(block *types.Block, state sm.State) {
 	}
 	m.height++
 }
+func (m *mockEvidencePool) IsCommitted(types.Evidence) bool { return false }
 
 //------------------------------------
 

evidence/pool.go

@@ -133,6 +133,12 @@ func (evpool *EvidencePool) MarkEvidenceAsCommitted(height int64, evidence []typ
 
 }
 
+// IsCommitted returns true if we have already seen this exact evidence and it is already marked as committed.
+func (evpool *EvidencePool) IsCommitted(evidence types.Evidence) bool {
+	ei := evpool.evidenceStore.getEvidenceInfo(evidence)
+	return ei.Evidence != nil && ei.Committed
+}
+
 func (evpool *EvidencePool) removeEvidence(height, maxAge int64, blockEvidenceMap map[string]struct{}) {
 	for e := evpool.evidenceList.Front(); e != nil; e = e.Next() {
 		ev := e.Value.(types.Evidence)

evidence/pool_test.go

@@ -84,3 +84,24 @@ func TestEvidencePool(t *testing.T) {
 	assert.Nil(t, err)
 	assert.Equal(t, 1, pool.evidenceList.Len())
 }
+
+func TestEvidencePoolIsCommitted(t *testing.T) {
+	// Initialization:
+	valAddr := []byte("validator_address")
+	height := int64(42)
+	stateDB := initializeValidatorState(valAddr, height)
+	evidenceDB := dbm.NewMemDB()
+	pool := NewEvidencePool(stateDB, evidenceDB)
+
+	// evidence not seen yet:
+	evidence := types.NewMockGoodEvidence(height, 0, valAddr)
+	assert.False(t, pool.IsCommitted(evidence))
+
+	// evidence seen but not yet committed:
+	assert.NoError(t, pool.AddEvidence(evidence))
+	assert.False(t, pool.IsCommitted(evidence))
+
+	// evidence seen and committed:
+	pool.MarkEvidenceAsCommitted(height, []types.Evidence{evidence})
+	assert.True(t, pool.IsCommitted(evidence))
+}

evidence/store.go

@@ -167,7 +167,7 @@ func (store *EvidenceStore) AddNewEvidence(evidence types.Evidence, priority int
 func (store *EvidenceStore) MarkEvidenceAsBroadcasted(evidence types.Evidence) {
 	ei := store.getEvidenceInfo(evidence)
 	if ei.Evidence == nil {
-		// nothin to do
+		// nothing to do; we did not store the evidence yet (AddNewEvidence):
 		return
 	}
 	// remove from the outqueue

state/execution.go

@@ -101,7 +101,7 @@ func (blockExec *BlockExecutor) CreateProposalBlock(
 // Validation does not mutate state, but does require historical information from the stateDB,
 // ie. to verify evidence from a validator at an old height.
 func (blockExec *BlockExecutor) ValidateBlock(state State, block *types.Block) error {
-	return validateBlock(blockExec.db, state, block)
+	return validateBlock(blockExec.evpool, blockExec.db, state, block)
 }
 
 // ApplyBlock validates the block against the state, executes it against the app,

state/services.go

@@ -85,6 +85,8 @@ type EvidencePool interface {
 	PendingEvidence(int64) []types.Evidence
 	AddEvidence(types.Evidence) error
 	Update(*types.Block, State)
+	// IsCommitted indicates if this evidence was already marked committed in another block.
+	IsCommitted(types.Evidence) bool
 }
 
 // MockMempool is an empty implementation of a Mempool, useful for testing.
@@ -93,3 +95,4 @@ type MockEvidencePool struct{}
 func (m MockEvidencePool) PendingEvidence(int64) []types.Evidence { return nil }
 func (m MockEvidencePool) AddEvidence(types.Evidence) error       { return nil }
 func (m MockEvidencePool) Update(*types.Block, State)             {}
+func (m MockEvidencePool) IsCommitted(types.Evidence) bool        { return false }

state/validation.go

@@ -13,7 +13,7 @@ import (
 //-----------------------------------------------------
 // Validate block
 
-func validateBlock(stateDB dbm.DB, state State, block *types.Block) error {
+func validateBlock(evidencePool EvidencePool, stateDB dbm.DB, state State, block *types.Block) error {
 	// Validate internal consistency.
 	if err := block.ValidateBasic(); err != nil {
 		return err
@@ -145,6 +145,9 @@ func validateBlock(stateDB dbm.DB, state State, block *types.Block) error {
 		if err := VerifyEvidence(stateDB, state, ev); err != nil {
 			return types.NewErrEvidenceInvalid(ev, err)
 		}
+		if evidencePool != nil && evidencePool.IsCommitted(ev) {
+			return types.NewErrEvidenceInvalid(ev, errors.New("evidence was already committed"))
+		}
 	}
 
 	// NOTE: We can't actually verify it's the right proposer because we dont

state/validation_test.go

@@ -121,6 +121,31 @@ func TestValidateBlockEvidence(t *testing.T) {
 	require.True(t, ok)
 }
 
+// always returns true if asked if any evidence was already committed.
+type mockEvPoolAlwaysCommitted struct{}
+
+func (m mockEvPoolAlwaysCommitted) PendingEvidence(int64) []types.Evidence { return nil }
+func (m mockEvPoolAlwaysCommitted) AddEvidence(types.Evidence) error       { return nil }
+func (m mockEvPoolAlwaysCommitted) Update(*types.Block, State)             {}
+func (m mockEvPoolAlwaysCommitted) IsCommitted(types.Evidence) bool        { return true }
+
+func TestValidateFailBlockOnCommittedEvidence(t *testing.T) {
+	var height int64 = 1
+	state, stateDB := state(1, int(height))
+
+	blockExec := NewBlockExecutor(stateDB, log.TestingLogger(), nil, nil, mockEvPoolAlwaysCommitted{})
+	// A block with a couple pieces of evidence passes.
+	block := makeBlock(state, height)
+	addr, _ := state.Validators.GetByIndex(0)
+	alreadyCommittedEvidence := types.NewMockGoodEvidence(height, 0, addr)
+	block.Evidence.Evidence = []types.Evidence{alreadyCommittedEvidence}
+	block.EvidenceHash = block.Evidence.Hash()
+	err := blockExec.ValidateBlock(state, block)
+
+	require.Error(t, err)
+	require.IsType(t, err, &types.ErrEvidenceInvalid{})
+}
+
 /*
 	TODO(#2589):
 	- test unmarshalling BlockParts that are too big into a Block that