From 379848eab7967186e13c7e50b35e512749e8f4f9 Mon Sep 17 00:00:00 2001
From: Callum Waters <cmwaters19@gmail.com>
Date: Mon, 6 Apr 2020 08:04:24 +0200
Subject: [PATCH] lite2: Prevent falsely returned double voting error (#4620)

* prevent faulty double voting error

* create test

* clean tests

* clean tests

Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
 types/validator_set.go      | 10 ++++----
 types/validator_set_test.go | 46 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/types/validator_set.go b/types/validator_set.go
index 1a17d1e83..f3e4627db 100644
--- a/types/validator_set.go
+++ b/types/validator_set.go
@@ -778,12 +778,12 @@ func (vals *ValidatorSet) VerifyCommitTrusting(chainID string, blockID BlockID,
 		// check for each vote if its validator is already known.
 		valIdx, val := vals.GetByAddress(commitSig.ValidatorAddress)
 
-		if firstIndex, ok := seenVals[valIdx]; ok { // double vote
-			secondIndex := idx
-			return errors.Errorf("double vote from %v (%d and %d)", val, firstIndex, secondIndex)
-		}
-
 		if val != nil {
+			// check for double vote of validator on the same commit
+			if firstIndex, ok := seenVals[valIdx]; ok {
+				secondIndex := idx
+				return errors.Errorf("double vote from %v (%d and %d)", val, firstIndex, secondIndex)
+			}
 			seenVals[valIdx] = idx
 
 			// Validate signature.
diff --git a/types/validator_set_test.go b/types/validator_set_test.go
index e5e972f6f..bc12b69c2 100644
--- a/types/validator_set_test.go
+++ b/types/validator_set_test.go
@@ -3,11 +3,13 @@ package types
 import (
 	"bytes"
 	"fmt"
+	"github.com/stretchr/testify/require"
 	"math"
 	"sort"
 	"strings"
 	"testing"
 	"testing/quick"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 
@@ -308,7 +310,7 @@ func randPubKey() crypto.PubKey {
 func randValidator(totalVotingPower int64) *Validator {
 	// this modulo limits the ProposerPriority/VotingPower to stay in the
 	// bounds of MaxTotalVotingPower minus the already existing voting power:
-	val := NewValidator(randPubKey(), int64(tmrand.Uint64()%uint64((MaxTotalVotingPower-totalVotingPower))))
+	val := NewValidator(randPubKey(), int64(tmrand.Uint64()%uint64(MaxTotalVotingPower-totalVotingPower)))
 	val.ProposerPriority = tmrand.Int64() % (MaxTotalVotingPower - totalVotingPower)
 	return val
 }
@@ -1324,6 +1326,48 @@ func TestValSetUpdateOverflowRelated(t *testing.T) {
 	}
 }
 
+func TestVerifyCommitTrusting(t *testing.T) {
+	var (
+		blockID                       = makeBlockIDRandom()
+		voteSet, originalValset, vals = randVoteSet(1, 1, PrecommitType, 6, 1)
+		commit, err                   = MakeCommit(blockID, 1, 1, voteSet, vals, time.Now())
+		newValSet, _                  = RandValidatorSet(2, 1)
+	)
+	require.NoError(t, err)
+
+	testCases := []struct {
+		valSet *ValidatorSet
+		err    bool
+	}{
+		// good
+		0: {
+			valSet: originalValset,
+			err:    false,
+		},
+		// bad - no overlap between validator sets
+		1: {
+			valSet: newValSet,
+			err:    true,
+		},
+		// good - first two are different but the rest of the same -> >1/3
+		2: {
+			valSet: NewValidatorSet(append(newValSet.Validators, originalValset.Validators...)),
+			err:    false,
+		},
+	}
+
+	for _, tc := range testCases {
+		err = tc.valSet.VerifyCommitTrusting("test_chain_id", blockID, commit.Height, commit,
+			tmmath.Fraction{Numerator: 1, Denominator: 3})
+		if tc.err {
+			assert.Error(t, err)
+		} else {
+			assert.NoError(t, err)
+		}
+	}
+
+}
+
 //---------------------
 // Sort validators by priority and address
 type validatorsByPriority []*Validator