some comments and cleanup

consensus/state.go

@@ -1237,10 +1237,6 @@ func (cs *ConsensusState) finalizeCommit(height int64) {
 
 	fail.Fail() // XXX
 
-	// TODO: remove included evidence
-	// and persist remaining evidence
-	// ... is this the right spot? need to ensure we never lose evidence
-
 	// NewHeightStep!
 	cs.updateToState(stateCopy)
 
@@ -1333,7 +1329,7 @@ func (cs *ConsensusState) tryAddVote(vote *types.Vote, peerKey string) error {
 	_, err := cs.addVote(vote, peerKey)
 	if err != nil {
 		// If the vote height is off, we'll just ignore it,
-		// But if it's a conflicting sig, broadcast evidence tx for slashing.
+		// But if it's a conflicting sig, add it to the cs.evpool.
 		// If it's otherwise invalid, punish peer.
 		if err == ErrVoteHeightMismatch {
 			return err

evidence/evidence_pool.go

@@ -6,15 +6,14 @@ import (
 	"github.com/tendermint/tendermint/types"
 )
 
-const cacheSize = 100000
-
-// EvidencePool maintains a set of valid uncommitted evidence.
+// EvidencePool maintains a pool of valid evidence
+// in an EvidenceStore.
 type EvidencePool struct {
 	config *EvidencePoolConfig
 	logger log.Logger
 
-	evidenceStore   *EvidenceStore
-	newEvidenceChan chan types.Evidence
+	evidenceStore *EvidenceStore
+	evidenceChan  chan types.Evidence
 }
 
 type EvidencePoolConfig struct {
@@ -22,10 +21,10 @@ type EvidencePoolConfig struct {
 
 func NewEvidencePool(config *EvidencePoolConfig, evidenceStore *EvidenceStore) *EvidencePool {
 	evpool := &EvidencePool{
-		config:          config,
-		logger:          log.NewNopLogger(),
-		evidenceStore:   evidenceStore,
-		newEvidenceChan: make(chan types.Evidence),
+		config:        config,
+		logger:        log.NewNopLogger(),
+		evidenceStore: evidenceStore,
+		evidenceChan:  make(chan types.Evidence),
 	}
 	return evpool
 }
@@ -35,9 +34,9 @@ func (evpool *EvidencePool) SetLogger(l log.Logger) {
 	evpool.logger = l
 }
 
-// NewEvidenceChan returns a channel on which new evidence is sent.
-func (evpool *EvidencePool) NewEvidenceChan() chan types.Evidence {
-	return evpool.newEvidenceChan
+// EvidenceChan returns an unbuffered channel on which new evidence can be received.
+func (evpool *EvidencePool) EvidenceChan() chan types.Evidence {
+	return evpool.evidenceChan
 }
 
 // PriorityEvidence returns the priority evidence.
@@ -51,6 +50,7 @@ func (evpool *EvidencePool) PendingEvidence() []types.Evidence {
 }
 
 // AddEvidence checks the evidence is valid and adds it to the pool.
+// Blocks on the EvidenceChan.
 func (evpool *EvidencePool) AddEvidence(evidence types.Evidence) (err error) {
 	added, err := evpool.evidenceStore.AddNewEvidence(evidence)
 	if err != nil {
@@ -62,7 +62,7 @@ func (evpool *EvidencePool) AddEvidence(evidence types.Evidence) (err error) {
 
 	evpool.logger.Info("Verified new evidence of byzantine behaviour", "evidence", evidence)
 
-	evpool.newEvidenceChan <- evidence
+	evpool.evidenceChan <- evidence
 	return nil
 }
 

evidence/reactor.go

@@ -111,12 +111,13 @@ func (evR *EvidencePoolReactor) SetEventSwitch(evsw types.EventSwitch) {
 	evR.evsw = evsw
 }
 
-// broadcast new evidence to all peers
+// broadcast new evidence to all peers.
+// broadcasts must be non-blocking so routine is always available to read off EvidenceChan.
 func (evR *EvidencePoolReactor) broadcastRoutine() {
 	ticker := time.NewTicker(time.Second * broadcastEvidenceIntervalS)
 	for {
 		select {
-		case evidence := <-evR.evpool.NewEvidenceChan():
+		case evidence := <-evR.evpool.EvidenceChan():
 			// broadcast some new evidence
 			msg := EvidenceMessage{[]types.Evidence{evidence}}
 			evR.Switch.Broadcast(EvidencePoolChannel, struct{ EvidencePoolMessage }{msg})

evidence/store.go

@@ -9,9 +9,12 @@ import (
 )
 
 /*
+Schema for indexing evidence:
+
 "evidence-lookup"/<evidence-height>/<evidence-hash> -> evidence struct
 "evidence-outqueue"/<index>/<evidence-height>/<evidence-hash> -> nil
 "evidence-pending"/<evidence-height>/evidence-hash> -> nil
+
 */
 
 var nullValue = []byte{0}
@@ -23,21 +26,25 @@ type evidenceInfo struct {
 }
 
 const (
-	baseKeyLookup   = "evidence-lookup"
-	baseKeyOutqueue = "evidence-outqueue"
-	baseKeyPending  = "evidence-pending"
+	baseKeyLookup   = "evidence-lookup"   // all evidence
+	baseKeyOutqueue = "evidence-outqueue" // not-yet broadcast
+	baseKeyPending  = "evidence-pending"  // broadcast but not committed
 )
 
 func keyLookup(evidence types.Evidence) []byte {
-	return []byte(fmt.Sprintf("%s/%d/%X", baseKeyLookup, evidence.Height(), evidence.Hash()))
+	return _key(baseKeyLookup, evidence)
 }
 
 func keyOutqueue(evidence types.Evidence) []byte {
-	return []byte(fmt.Sprintf("%s/%d/%X", baseKeyOutqueue, evidence.Height(), evidence.Hash()))
+	return _key(baseKeyOutqueue, evidence)
 }
 
 func keyPending(evidence types.Evidence) []byte {
-	return []byte(fmt.Sprintf("%s/%d/%X", baseKeyPending, evidence.Height(), evidence.Hash()))
+	return _key(baseKeyPending, evidence)
+}
+
+func _key(key string, evidence types.Evidence) []byte {
+	return []byte(fmt.Sprintf("%s/%d/%X", key, evidence.Height(), evidence.Hash()))
 }
 
 // EvidenceStore stores all the evidence we've seen, including
@@ -47,14 +54,15 @@ type EvidenceStore struct {
 	chainID string
 	db      dbm.DB
 
+	// so we can verify evidence was from a real validator
 	historicalValidators types.HistoricalValidators
 }
 
-func NewEvidenceStore(chainID string, db dbm.DB) *EvidenceStore {
+func NewEvidenceStore(chainID string, db dbm.DB, vals types.HistoricalValidators) *EvidenceStore {
 	return &EvidenceStore{
-		chainID: chainID,
-		db:      db,
-		// TODO historicalValidators
+		chainID:              chainID,
+		db:                   db,
+		historicalValidators: vals,
 	}
 }
 
@@ -111,6 +119,7 @@ func (store *EvidenceStore) AddNewEvidence(evidence types.Evidence) (bool, error
 	eiBytes := wire.BinaryBytes(ei)
 
 	// add it to the store
+	key = keyLookup(evidence)
 	store.db.Set(key, eiBytes)
 
 	key = keyOutqueue(evidence)
@@ -128,8 +137,11 @@ func (store *EvidenceStore) MarkEvidenceAsBroadcasted(evidence types.Evidence) {
 	store.db.Delete(key)
 }
 
-// MarkEvidenceAsPending removes evidence from pending and sets the state to committed.
+// MarkEvidenceAsPending removes evidence from pending and outqueue and sets the state to committed.
 func (store *EvidenceStore) MarkEvidenceAsCommitted(evidence types.Evidence) {
+	// if its committed, its been broadcast
+	store.MarkEvidenceAsBroadcasted(evidence)
+
 	key := keyPending(evidence)
 	store.db.Delete(key)