From 87bdc42bf827c659aaa4b3c9c796bc3f8e698a87 Mon Sep 17 00:00:00 2001 From: Ismail Khoffi Date: Sat, 9 Feb 2019 00:30:45 +0100 Subject: [PATCH] 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 | 15 +++++++++++++++ consensus/reactor_test.go | 1 + evidence/pool.go | 6 ++++++ evidence/pool_test.go | 21 +++++++++++++++++++++ evidence/store.go | 2 +- state/execution.go | 2 +- state/services.go | 3 +++ state/validation.go | 5 ++++- state/validation_test.go | 25 +++++++++++++++++++++++++ 9 files changed, 77 insertions(+), 3 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 4548eb1eb..91eed1882 100644 --- a/CHANGELOG_PENDING.md +++ b/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 + diff --git a/consensus/reactor_test.go b/consensus/reactor_test.go index 28e245aec..d35eaf3c0 100644 --- a/consensus/reactor_test.go +++ b/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 } //------------------------------------ diff --git a/evidence/pool.go b/evidence/pool.go index 3df0ec70a..18ccb3344 100644 --- a/evidence/pool.go +++ b/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) diff --git a/evidence/pool_test.go b/evidence/pool_test.go index 677ce78bb..30b20011e 100644 --- a/evidence/pool_test.go +++ b/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)) +} diff --git a/evidence/store.go b/evidence/store.go index 998a763d4..464d6138e 100644 --- a/evidence/store.go +++ b/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 diff --git a/state/execution.go b/state/execution.go index e3668c77a..8ab95839e 100644 --- a/state/execution.go +++ b/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, diff --git a/state/services.go b/state/services.go index 90f0cd019..02c3aa7d1 100644 --- a/state/services.go +++ b/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 } diff --git a/state/validation.go b/state/validation.go index 82c479ac6..3cb0ee8fb 100644 --- a/state/validation.go +++ b/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 diff --git a/state/validation_test.go b/state/validation_test.go index 12aaf6361..a873855a9 100644 --- a/state/validation_test.go +++ b/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