evidence: minor correction to potential amnesia ev validate basic (#5151)

ValidateBasic() for PotentialAmnesiaEvidence checks that the rounds of the two votes are different and does not check Vote Type

ValidateBasic() now also ensures that the first block is not a nil block (else the validator hasn't actually locked onto a block)

state/validation_test.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/tendermint/tendermint/crypto"
 	"github.com/tendermint/tendermint/libs/bytes"
+	tmrand "github.com/tendermint/tendermint/libs/rand"
 	"github.com/tendermint/tendermint/proto/tendermint/version"
 
 	"github.com/tendermint/tendermint/crypto/ed25519"
@@ -360,13 +361,22 @@ func TestValidateDuplicateEvidenceShouldFail(t *testing.T) {
 	assert.Error(t, err)
 }
 
-var blockID = types.BlockID{
+var (
-	Hash: []byte("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
+	blockID = types.BlockID{
-	PartSetHeader: types.PartSetHeader{
+		Hash: tmrand.Bytes(tmhash.Size),
-		Total: 1,
+		PartSetHeader: types.PartSetHeader{
-		Hash:  []byte("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
+			Total: 1,
-	},
+			Hash:  tmrand.Bytes(tmhash.Size),
-}
+		},
+	}
+	differentBlockID = types.BlockID{
+		Hash: tmrand.Bytes(tmhash.Size),
+		PartSetHeader: types.PartSetHeader{
+			Total: 1,
+			Hash:  tmrand.Bytes(tmhash.Size),
+		},
+	}
+)
 
 func TestValidateUnseenAmnesiaEvidence(t *testing.T) {
 	var height int64 = 1
@@ -377,7 +387,7 @@ func TestValidateUnseenAmnesiaEvidence(t *testing.T) {
 	err := vals[val.Address.String()].SignVote(chainID, vA)
 	voteA.Signature = vA.Signature
 	require.NoError(t, err)
-	voteB := makeVote(height, 2, 0, addr, types.BlockID{})
+	voteB := makeVote(height, 2, 0, addr, differentBlockID)
 	vB := voteB.ToProto()
 	err = vals[val.Address.String()].SignVote(chainID, vB)
 	voteB.Signature = vB.Signature
@@ -426,7 +436,7 @@ func TestValidatePrimedAmnesiaEvidence(t *testing.T) {
 	err := vals[val.Address.String()].SignVote(chainID, vA)
 	require.NoError(t, err)
 	voteA.Signature = vA.Signature
-	voteB := makeVote(height, 2, 0, addr, types.BlockID{})
+	voteB := makeVote(height, 2, 0, addr, differentBlockID)
 	vB := voteB.ToProto()
 	err = vals[val.Address.String()].SignVote(chainID, vB)
 	voteB.Signature = vB.Signature

types/block_test.go

@@ -849,3 +849,18 @@ func TestSignedHeaderProtoBuf(t *testing.T) {
 		}
 	}
 }
+
+func TestBlockIDEquals(t *testing.T) {
+	var (
+		blockID          = makeBlockID([]byte("hash"), 2, []byte("part_set_hash"))
+		blockIDDuplicate = makeBlockID([]byte("hash"), 2, []byte("part_set_hash"))
+		blockIDDifferent = makeBlockID([]byte("different_hash"), 2, []byte("part_set_hash"))
+		blockIDEmpty     = BlockID{}
+	)
+
+	assert.True(t, blockID.Equals(blockIDDuplicate))
+	assert.False(t, blockID.Equals(blockIDDifferent))
+	assert.False(t, blockID.Equals(blockIDEmpty))
+	assert.True(t, blockIDEmpty.Equals(blockIDEmpty))
+	assert.False(t, blockIDEmpty.Equals(blockIDDifferent))
+}

types/evidence.go

@@ -1041,7 +1041,7 @@ func LunaticValidatorEvidenceFromProto(pb *tmproto.LunaticValidatorEvidence) (*L
 // in the same height. PotentialAmnesiaEvidence can then evolve into AmnesiaEvidence if the indicted validator
 // is incapable of providing the proof of lock change that validates voting twice in the allotted trial period.
 // Heightstamp is used for each node to keep a track of how much time has passed so as to know when the trial period
-// is finished and is set when the node first receives the evidence.
+// is finished and is set when the node first receives the evidence. Votes are ordered based on their timestamp
 type PotentialAmnesiaEvidence struct {
 	VoteA *Vote `json:"vote_a"`
 	VoteB *Vote `json:"vote_b"`
@@ -1148,10 +1148,14 @@ func (e *PotentialAmnesiaEvidence) ValidateBasic() error {
 	}
 
 	// H/S must be the same
-	if e.VoteA.Height != e.VoteB.Height ||
+	if e.VoteA.Height != e.VoteB.Height {
-		e.VoteA.Type != e.VoteB.Type {
+		return fmt.Errorf("heights do not match: %d vs %d",
-		return fmt.Errorf("h/s do not match: %d/%v vs %d/%v",
+			e.VoteA.Height, e.VoteB.Height)
-			e.VoteA.Height, e.VoteA.Type, e.VoteB.Height, e.VoteB.Type)
+	}
+
+	if e.VoteA.Round == e.VoteB.Round {
+		return fmt.Errorf("votes must be for different rounds: %d",
+			e.VoteA.Round)
 	}
 
 	// Enforce that vote A came before vote B
@@ -1178,6 +1182,10 @@ func (e *PotentialAmnesiaEvidence) ValidateBasic() error {
 		)
 	}
 
+	if e.VoteA.BlockID.IsZero() {
+		return errors.New("first vote is for a nil block - voter hasn't locked on a block")
+	}
+
 	// BlockIDs must be different
 	if e.VoteA.BlockID.Equals(e.VoteB.BlockID) {
 		return fmt.Errorf(

types/evidence_test.go

@@ -375,6 +375,7 @@ func TestPotentialAmnesiaEvidence(t *testing.T) {
 
 	var (
 		val      = NewMockPV()
+		val2     = NewMockPV()
 		blockID  = makeBlockID(tmhash.Sum([]byte("blockhash")), math.MaxInt32, tmhash.Sum([]byte("partshash")))
 		blockID2 = makeBlockID(tmhash.Sum([]byte("blockhash2")), math.MaxInt32, tmhash.Sum([]byte("partshash")))
 		vote1    = makeVote(t, val, chainID, 0, height, 0, 2, blockID, defaultVoteTime)
@@ -409,14 +410,7 @@ func TestPotentialAmnesiaEvidence(t *testing.T) {
 	assert.True(t, ev.Equal(ev2))
 	assert.Equal(t, ev.Hash(), ev2.Hash())
 
-	ev3 := &PotentialAmnesiaEvidence{
+	ev3 := NewPotentialAmnesiaEvidence(vote2, vote1)
-		VoteA: vote2,
-		VoteB: vote1,
-	}
-
-	assert.Error(t, ev3.ValidateBasic())
-
-	ev3 = NewPotentialAmnesiaEvidence(vote2, vote1)
 	assert.True(t, ev3.Equal(ev))
 
 	ev4 := &PotentialAmnesiaEvidence{
@@ -428,6 +422,37 @@ func TestPotentialAmnesiaEvidence(t *testing.T) {
 	assert.NotEqual(t, ev.Hash(), ev4.Hash())
 	assert.False(t, ev.Equal(ev4))
 
+	// bad evidence
+	badEv := []*PotentialAmnesiaEvidence{
+		// first vote is for a later time than the second vote
+		{
+			VoteA: vote2,
+			VoteB: vote1,
+		},
+
+		// votes are for the same round
+		{
+			VoteA: vote1,
+			VoteB: makeVote(t, val, chainID, 0, height, 0, 2, blockID2, defaultVoteTime.Add(1*time.Second)),
+		},
+
+		// first vote was for a nil block - not locked
+		{
+			VoteA: makeVote(t, val, chainID, 0, height, 0, 2, BlockID{}, defaultVoteTime.Add(1*time.Second)),
+			VoteB: vote2,
+		},
+
+		// second vote is from a different validator
+		{
+			VoteA: vote1,
+			VoteB: makeVote(t, val2, chainID, 0, height, 1, 2, blockID2, defaultVoteTime.Add(1*time.Second)),
+		},
+	}
+
+	for _, ev := range badEv {
+		assert.Error(t, ev.ValidateBasic())
+	}
+
 }
 
 func TestProofOfLockChange(t *testing.T) {