evidence: fix bug with hashes

evidence/pool.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
-	"sort"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -203,9 +202,11 @@ func (evpool *Pool) CheckEvidence(evList types.EvidenceList) error {
 	hashes := make([][]byte, len(evList))
 	for idx, ev := range evList {
 
-		ok := evpool.fastCheck(ev)
+		_, isLightEv := ev.(*types.LightClientAttackEvidence)
 
-		if !ok {
+		// We must verify light client attack evidence regardless because there could be a
+		// different conflicting block with the same hash.
+		if isLightEv || !evpool.isPending(ev) {
 			// check that the evidence isn't already committed
 			if evpool.isCommitted(ev) {
 				return &types.ErrInvalidEvidence{Evidence: ev, Reason: errors.New("evidence was already committed")}
@@ -222,7 +223,7 @@ func (evpool *Pool) CheckEvidence(evList types.EvidenceList) error {
 				evpool.logger.Error("failed to add evidence to pending list", "err", err, "evidence", ev)
 			}
 
-			evpool.logger.Info("verified new evidence of byzantine behavior", "evidence", ev)
+			evpool.logger.Info("check evidence: verified evidence of byzantine behavior", "evidence", ev)
 		}
 
 		// check for duplicate evidence. We cache hashes so we don't have to work them out again.
@@ -260,78 +261,6 @@ func (evpool *Pool) State() sm.State {
 	return evpool.state
 }
 
-// fastCheck leverages the fact that the evidence pool may have already verified
-// the evidence to see if it can quickly conclude that the evidence is already
-// valid.
-func (evpool *Pool) fastCheck(ev types.Evidence) bool {
-	if lcae, ok := ev.(*types.LightClientAttackEvidence); ok {
-		key := keyPending(ev)
-		evBytes, err := evpool.evidenceStore.Get(key)
-		if evBytes == nil { // the evidence is not in the nodes pending list
-			return false
-		}
-
-		if err != nil {
-			evpool.logger.Error("failed to load light client attack evidence", "err", err, "key(height/hash)", key)
-			return false
-		}
-
-		var trustedPb tmproto.LightClientAttackEvidence
-
-		if err = trustedPb.Unmarshal(evBytes); err != nil {
-			evpool.logger.Error(
-				"failed to convert light client attack evidence from bytes",
-				"key(height/hash)", key,
-				"err", err,
-			)
-			return false
-		}
-
-		trustedEv, err := types.LightClientAttackEvidenceFromProto(&trustedPb)
-		if err != nil {
-			evpool.logger.Error(
-				"failed to convert light client attack evidence from protobuf",
-				"key(height/hash)", key,
-				"err", err,
-			)
-			return false
-		}
-
-		// Ensure that all the byzantine validators that the evidence pool has match
-		// the byzantine validators in this evidence.
-		if trustedEv.ByzantineValidators == nil && lcae.ByzantineValidators != nil {
-			return false
-		}
-
-		if len(trustedEv.ByzantineValidators) != len(lcae.ByzantineValidators) {
-			return false
-		}
-
-		byzValsCopy := make([]*types.Validator, len(lcae.ByzantineValidators))
-		for i, v := range lcae.ByzantineValidators {
-			byzValsCopy[i] = v.Copy()
-		}
-
-		// ensure that both validator arrays are in the same order
-		sort.Sort(types.ValidatorsByVotingPower(byzValsCopy))
-
-		for idx, val := range trustedEv.ByzantineValidators {
-			if !bytes.Equal(byzValsCopy[idx].Address, val.Address) {
-				return false
-			}
-			if byzValsCopy[idx].VotingPower != val.VotingPower {
-				return false
-			}
-		}
-
-		return true
-	}
-
-	// For all other evidence the evidence pool just checks if it is already in
-	// the pending db.
-	return evpool.isPending(ev)
-}
-
 // IsExpired checks whether evidence or a polc is expired by checking whether a height and time is older
 // than set by the evidence consensus parameters
 func (evpool *Pool) isExpired(height int64, time time.Time) bool {
@@ -396,7 +325,7 @@ func (evpool *Pool) markEvidenceAsCommitted(evidence types.EvidenceList, height
 	for _, ev := range evidence {
 		if evpool.isPending(ev) {
 			if err := batch.Delete(keyPending(ev)); err != nil {
-				evpool.logger.Error("failed to batch pending evidence", "err", err)
+				evpool.logger.Error("failed to batch delete pending evidence", "err", err)
 			}
 			blockEvidenceMap[evMapKey(ev)] = struct{}{}
 		}
@@ -546,7 +475,7 @@ func (evpool *Pool) batchExpiredPendingEvidence(batch dbm.Batch) (int64, time.Ti
 
 		// else add to the batch
 		if err := batch.Delete(iter.Key()); err != nil {
-			evpool.logger.Error("failed to batch evidence", "err", err, "ev", ev)
+			evpool.logger.Error("failed to batch delete evidence", "err", err, "ev", ev)
 			continue
 		}
 

evidence/pool_test.go

@@ -14,7 +14,6 @@ import (
 	"github.com/tendermint/tendermint/evidence"
 	"github.com/tendermint/tendermint/evidence/mocks"
 	"github.com/tendermint/tendermint/libs/log"
-	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 	sm "github.com/tendermint/tendermint/state"
 	smmocks "github.com/tendermint/tendermint/state/mocks"
 	"github.com/tendermint/tendermint/store"
@@ -166,6 +165,9 @@ func TestReportConflictingVotes(t *testing.T) {
 	// should be able to retrieve evidence from pool
 	evList, _ = pool.PendingEvidence(defaultEvidenceMaxBytes)
 	require.Equal(t, []types.Evidence{ev}, evList)
+
+	next = pool.EvidenceFront()
+	require.NotNil(t, next)
 }
 
 func TestEvidencePoolUpdate(t *testing.T) {
@@ -262,84 +264,61 @@ func TestVerifyDuplicatedEvidenceFails(t *testing.T) {
 
 // check that valid light client evidence is correctly validated and stored in
 // evidence pool
-func TestCheckEvidenceWithLightClientAttack(t *testing.T) {
+func TestLightClientAttackEvidenceLifecycle(t *testing.T) {
 	var (
-		nValidators          = 5
-		validatorPower int64 = 10
-		height         int64 = 10
+		height       int64 = 100
+		commonHeight int64 = 90
 	)
 
-	conflictingVals, conflictingPrivVals := types.RandValidatorSet(nValidators, validatorPower)
-	trustedHeader := makeHeaderRandom(height)
-	trustedHeader.Time = defaultEvidenceTime
-
-	conflictingHeader := makeHeaderRandom(height)
-	conflictingHeader.ValidatorsHash = conflictingVals.Hash()
-
-	trustedHeader.ValidatorsHash = conflictingHeader.ValidatorsHash
-	trustedHeader.NextValidatorsHash = conflictingHeader.NextValidatorsHash
-	trustedHeader.ConsensusHash = conflictingHeader.ConsensusHash
-	trustedHeader.AppHash = conflictingHeader.AppHash
-	trustedHeader.LastResultsHash = conflictingHeader.LastResultsHash
-
-	// For simplicity we are simulating a duplicate vote attack where all the
-	// validators in the conflictingVals set voted twice.
-	blockID := makeBlockID(conflictingHeader.Hash(), 1000, []byte("partshash"))
-	voteSet := types.NewVoteSet(evidenceChainID, height, 1, tmproto.SignedMsgType(2), conflictingVals)
-
-	commit, err := types.MakeCommit(blockID, height, 1, voteSet, conflictingPrivVals, defaultEvidenceTime)
-	require.NoError(t, err)
-
-	ev := &types.LightClientAttackEvidence{
-		ConflictingBlock: &types.LightBlock{
-			SignedHeader: &types.SignedHeader{
-				Header: conflictingHeader,
-				Commit: commit,
-			},
-			ValidatorSet: conflictingVals,
-		},
-		CommonHeight:        10,
-		TotalVotingPower:    int64(nValidators) * validatorPower,
-		ByzantineValidators: conflictingVals.Validators,
-		Timestamp:           defaultEvidenceTime,
-	}
-
-	trustedBlockID := makeBlockID(trustedHeader.Hash(), 1000, []byte("partshash"))
-	trustedVoteSet := types.NewVoteSet(evidenceChainID, height, 1, tmproto.SignedMsgType(2), conflictingVals)
-	trustedCommit, err := types.MakeCommit(
-		trustedBlockID,
-		height,
-		1,
-		trustedVoteSet,
-		conflictingPrivVals,
-		defaultEvidenceTime,
-	)
-	require.NoError(t, err)
+	ev, trusted, common := makeLunaticEvidence(t, height, commonHeight,
+		10, 5, 5, defaultEvidenceTime, defaultEvidenceTime.Add(1*time.Hour))
 
 	state := sm.State{
-		LastBlockTime:   defaultEvidenceTime.Add(1 * time.Minute),
-		LastBlockHeight: 11,
+		LastBlockTime:   defaultEvidenceTime.Add(2 * time.Hour),
+		LastBlockHeight: 110,
 		ConsensusParams: *types.DefaultConsensusParams(),
 	}
 
 	stateStore := &smmocks.Store{}
-	stateStore.On("LoadValidators", height).Return(conflictingVals, nil)
+	stateStore.On("LoadValidators", height).Return(trusted.ValidatorSet, nil)
+	stateStore.On("LoadValidators", commonHeight).Return(common.ValidatorSet, nil)
 	stateStore.On("Load").Return(state, nil)
 
 	blockStore := &mocks.BlockStore{}
-	blockStore.On("LoadBlockMeta", height).Return(&types.BlockMeta{Header: *trustedHeader})
-	blockStore.On("LoadBlockCommit", height).Return(trustedCommit)
+	blockStore.On("LoadBlockMeta", height).Return(&types.BlockMeta{Header: *trusted.Header})
+	blockStore.On("LoadBlockMeta", commonHeight).Return(&types.BlockMeta{Header: *common.Header})
+	blockStore.On("LoadBlockCommit", height).Return(trusted.Commit)
+	blockStore.On("LoadBlockCommit", commonHeight).Return(common.Commit)
 
 	pool, err := evidence.NewPool(log.TestingLogger(), dbm.NewMemDB(), stateStore, blockStore)
 	require.NoError(t, err)
 
+	hash := ev.Hash()
+
 	require.NoError(t, pool.AddEvidence(ev))
-	require.NoError(t, pool.CheckEvidence(types.EvidenceList{ev}))
+	require.NoError(t, pool.AddEvidence(ev))
+
+	pendingEv, _ := pool.PendingEvidence(state.ConsensusParams.Evidence.MaxBytes)
+	require.Equal(t, 1, len(pendingEv))
+	require.Equal(t, ev, pendingEv[0])
+
+	require.NoError(t, pool.CheckEvidence(pendingEv))
+	require.Equal(t, ev, pendingEv[0])
 
-	// Take away the last signature -> there are less validators then what we have detected,
-	// hence this should fail.
-	commit.Signatures = append(commit.Signatures[:nValidators-1], types.NewCommitSigAbsent())
+	state.LastBlockHeight++
+	state.LastBlockTime = state.LastBlockTime.Add(1 * time.Minute)
+	pool.Update(state, pendingEv)
+	require.Equal(t, hash, pendingEv[0].Hash())
+
+	remaindingEv, _ := pool.PendingEvidence(state.ConsensusParams.Evidence.MaxBytes)
+	require.Empty(t, remaindingEv)
+
+	// evidence is already committed so it shouldn't pass
 	require.Error(t, pool.CheckEvidence(types.EvidenceList{ev}))
+	require.NoError(t, pool.AddEvidence(ev))
+
+	remaindingEv, _ = pool.PendingEvidence(state.ConsensusParams.Evidence.MaxBytes)
+	require.Empty(t, remaindingEv)
 }
 
 // Tests that restarting the evidence pool after a potential failure will recover the
@@ -389,7 +368,7 @@ func TestRecoverPendingEvidence(t *testing.T) {
 			Evidence: types.EvidenceParams{
 				MaxAgeNumBlocks: 20,
 				MaxAgeDuration:  20 * time.Minute,
-				MaxBytes:        1000,
+				MaxBytes:        defaultEvidenceMaxBytes,
 			},
 		},
 	}, nil)