From 65b6bae6b6cd069ab764639a4537e59cfcdac4c6 Mon Sep 17 00:00:00 2001
From: Simon Noetzlin <simon.ntz@gmail.com>
Date: Mon, 13 Nov 2023 17:17:33 +0100
Subject: [PATCH] last changes

---
 tests/integration/misbehaviour.go     | 60 ++++++++++++++++++++++-----
 x/ccv/provider/keeper/misbehaviour.go |  7 +++-
 2 files changed, 54 insertions(+), 13 deletions(-)

diff --git a/tests/integration/misbehaviour.go b/tests/integration/misbehaviour.go
index 0768c4a37a..63b0eafb6d 100644
--- a/tests/integration/misbehaviour.go
+++ b/tests/integration/misbehaviour.go
@@ -149,8 +149,7 @@ func (s *CCVTestSuite) TestGetByzantineValidators() {
 					clientSigners,
 				)
 
-				// create conflicting header with 2/4 validators voting nil
-				clientHeaderWithNilVotes := s.consumerChain.CreateTMClientHeader(
+				clientHeaderWithCorruptedValset := s.consumerChain.CreateTMClientHeader(
 					s.consumerChain.ChainID,
 					int64(clientHeight.RevisionHeight+1),
 					clientHeight,
@@ -161,15 +160,54 @@ func (s *CCVTestSuite) TestGetByzantineValidators() {
 					clientSigners,
 				)
 
-				testutil.CorruptValidatorPubkeyInHeader(clientHeaderWithNilVotes, clientTMValset.Validators[0].Address)
+				// change a validator public key in one the second header
+				testutil.CorruptValidatorPubkeyInHeader(clientHeaderWithCorruptedValset, clientTMValset.Validators[0].Address)
 
 				return &ibctmtypes.Misbehaviour{
 					ClientId: s.path.EndpointA.ClientID,
 					Header1:  clientHeader,
-					Header2:  clientHeaderWithNilVotes,
+					Header2:  clientHeaderWithCorruptedValset,
+				}
+			},
+			[]*tmtypes.Validator{},
+			false,
+		},
+		{
+			"incorrect valset 2 - shouldn't pass",
+			func() *ibctmtypes.Misbehaviour {
+				clientHeader := s.consumerChain.CreateTMClientHeader(
+					s.consumerChain.ChainID,
+					int64(clientHeight.RevisionHeight+1),
+					clientHeight,
+					altTime.Add(time.Minute),
+					clientTMValset,
+					clientTMValset,
+					clientTMValset,
+					clientSigners,
+				)
+
+				clientHeaderWithCorruptedSigs := s.consumerChain.CreateTMClientHeader(
+					s.consumerChain.ChainID,
+					int64(clientHeight.RevisionHeight+1),
+					clientHeight,
+					altTime.Add(time.Hour),
+					clientTMValset,
+					clientTMValset,
+					clientTMValset,
+					clientSigners,
+				)
+
+				// change the valset in the header
+				vs, _ := altValset.ToProto()
+				clientHeader.ValidatorSet.Validators = vs.Validators[:3]
+				clientHeaderWithCorruptedSigs.ValidatorSet.Validators = vs.Validators[:3]
+
+				return &ibctmtypes.Misbehaviour{
+					ClientId: s.path.EndpointA.ClientID,
+					Header1:  clientHeader,
+					Header2:  clientHeaderWithCorruptedSigs,
 				}
 			},
-			// Expect validators who did NOT vote nil
 			[]*tmtypes.Validator{},
 			false,
 		},
@@ -187,8 +225,7 @@ func (s *CCVTestSuite) TestGetByzantineValidators() {
 					clientSigners,
 				)
 
-				// create conflicting header with 2/4 validators voting nil
-				clientHeaderWithNilVotes := s.consumerChain.CreateTMClientHeader(
+				clientHeaderWithCorruptedSigs := s.consumerChain.CreateTMClientHeader(
 					s.consumerChain.ChainID,
 					int64(clientHeight.RevisionHeight+1),
 					clientHeight,
@@ -198,15 +235,16 @@ func (s *CCVTestSuite) TestGetByzantineValidators() {
 					clientTMValset,
 					clientSigners,
 				)
-				testutil.CorruptCommitSigsInHeader(clientHeaderWithNilVotes, clientTMValset.Validators[0].Address)
+
+				// change the signature of one of the validator in the header
+				testutil.CorruptCommitSigsInHeader(clientHeaderWithCorruptedSigs, clientTMValset.Validators[0].Address)
 
 				return &ibctmtypes.Misbehaviour{
 					ClientId: s.path.EndpointA.ClientID,
 					Header1:  clientHeader,
-					Header2:  clientHeaderWithNilVotes,
+					Header2:  clientHeaderWithCorruptedSigs,
 				}
 			},
-			// Expect validators who did NOT vote nil
 			[]*tmtypes.Validator{},
 			false,
 		},
@@ -300,7 +338,7 @@ func (s *CCVTestSuite) TestGetByzantineValidators() {
 				s.Equal(len(tc.expByzantineValidators), len(byzantineValidators))
 
 				// For both lunatic and equivocation attacks, all the validators
-				// who signed both headers and didn't vote nil should be returned
+				// who signed both headers
 				if len(tc.expByzantineValidators) > 0 {
 					expValset := tmtypes.NewValidatorSet(tc.expByzantineValidators)
 					s.NoError(err)
diff --git a/x/ccv/provider/keeper/misbehaviour.go b/x/ccv/provider/keeper/misbehaviour.go
index 06344656a2..f5b63533a0 100644
--- a/x/ccv/provider/keeper/misbehaviour.go
+++ b/x/ccv/provider/keeper/misbehaviour.go
@@ -206,11 +206,14 @@ func verifyLightBlockCommitSig(lightBlock tmtypes.LightBlock, sigIdx int) error
 	sig := lightBlock.Commit.Signatures[sigIdx]
 
 	// get validator
-	_, val := lightBlock.ValidatorSet.GetByAddress(sig.ValidatorAddress)
+	idx, val := lightBlock.ValidatorSet.GetByAddress(sig.ValidatorAddress)
+	if idx == -1 {
+		return fmt.Errorf("incorrect signature: validator address %s isn't part of the validator set", sig.ValidatorAddress.String())
+	}
 
 	// verify validator pubkey corresponds to signature validator address
 	if !bytes.Equal(val.PubKey.Address(), sig.ValidatorAddress) {
-		return fmt.Errorf("validator public key doesn't correspond to signature validator address: %d!= %d", val.PubKey.Address(), sig.ValidatorAddress)
+		return fmt.Errorf("validator public key doesn't correspond to signature validator address: %s!= %s", val.PubKey.Address(), sig.ValidatorAddress)
 	}
 
 	// validate signature