update byzantine validators extraction

Dockerfile

@@ -36,7 +36,7 @@ FROM informalofftermatt/cometmock:latest as cometmock-builder
 # Get GoRelayer
 FROM informalofftermatt/gorelayer:nogas AS gorelayer-builder
 
-FROM --platform=linux/amd64 fedora:36
+FROM --platform=linux/arm64 fedora:36
 RUN dnf update -y
 RUN dnf install -y which iproute iputils procps-ng vim-minimal tmux net-tools htop jq
 USER root

tests/e2e/main.go

@@ -57,11 +57,11 @@ func main() {
 	}
 
 	testRuns := []testRunWithSteps{
-		{ChangeoverTestRun(), changeoverSteps},
-		{DefaultTestRun(), happyPathSteps},
-		{DemocracyTestRun(true), democracySteps},
-		{DemocracyTestRun(false), rewardDenomConsumerSteps},
-		{SlashThrottleTestRun(), slashThrottleSteps},
+		// {ChangeoverTestRun(), changeoverSteps},
+		// {DefaultTestRun(), happyPathSteps},
+		// {DemocracyTestRun(true), democracySteps},
+		// {DemocracyTestRun(false), rewardDenomConsumerSteps},
+		// {SlashThrottleTestRun(), slashThrottleSteps},
 		{ConsumerMisbehaviourTestRun(), consumerMisbehaviourSteps},
 		{DefaultTestRun(), consumerDoubleSignSteps},
 	}

tests/integration/misbehaviour.go

@@ -95,6 +95,15 @@ func (s *CCVTestSuite) TestGetByzantineValidators() {
 	altSigners[clientTMValset.Validators[1].Address.String()] = clientSigners[clientTMValset.Validators[1].Address.String()]
 	altSigners[clientTMValset.Validators[2].Address.String()] = clientSigners[clientTMValset.Validators[2].Address.String()]
 
+	// maliciousSigner := tmtypes.NewMockPV()
+
+	// // Create a subset of the consumer client validator set
+	// altValset2 := tmtypes.NewValidatorSet(s.consumerChain.Vals.Validators[0:3])
+	// altSigners2 := make(map[string]tmtypes.PrivValidator, 3)
+	// altSigners2[clientTMValset.Validators[0].Address.String()] = clientSigners[clientTMValset.Validators[0].Address.String()]
+	// altSigners2[clientTMValset.Validators[1].Address.String()] = clientSigners[clientTMValset.Validators[1].Address.String()]
+	// altSigners2[clientTMValset.Validators[2].Address.String()] = maliciousSigner
+
 	// create a consumer client header
 	clientHeader := s.consumerChain.CreateTMClientHeader(
 		s.consumerChain.ChainID,
@@ -135,6 +144,81 @@ func (s *CCVTestSuite) TestGetByzantineValidators() {
 			nil,
 			false,
 		},
+		{
+			"incorrect valset - 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,
+				)
+
+				// create conflicting header with 2/4 validators voting nil
+				clientHeaderWithNilVotes := s.consumerChain.CreateTMClientHeader(
+					s.consumerChain.ChainID,
+					int64(clientHeight.RevisionHeight+1),
+					clientHeight,
+					altTime.Add(time.Hour),
+					clientTMValset,
+					clientTMValset,
+					clientTMValset,
+					clientSigners,
+				)
+
+				testutil.CorruptValidatorPubkeyInHeader(clientHeaderWithNilVotes, clientTMValset.Validators[0].Address)
+
+				return &ibctmtypes.Misbehaviour{
+					ClientId: s.path.EndpointA.ClientID,
+					Header1:  clientHeader,
+					Header2:  clientHeaderWithNilVotes,
+				}
+			},
+			// Expect validators who did NOT vote nil
+			[]*tmtypes.Validator{},
+			false,
+		},
+		{
+			"incorrect signatures - 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,
+				)
+
+				// create conflicting header with 2/4 validators voting nil
+				clientHeaderWithNilVotes := s.consumerChain.CreateTMClientHeader(
+					s.consumerChain.ChainID,
+					int64(clientHeight.RevisionHeight+1),
+					clientHeight,
+					altTime.Add(time.Hour),
+					clientTMValset,
+					clientTMValset,
+					clientTMValset,
+					clientSigners,
+				)
+				testutil.CorruptCommitSigsInHeader(clientHeaderWithNilVotes, clientTMValset.Validators[0].Address)
+
+				return &ibctmtypes.Misbehaviour{
+					ClientId: s.path.EndpointA.ClientID,
+					Header1:  clientHeader,
+					Header2:  clientHeaderWithNilVotes,
+				}
+			},
+			// Expect validators who did NOT vote nil
+			[]*tmtypes.Validator{},
+			false,
+		},
 		{
 			"light client attack - lunatic attack",
 			func() *ibctmtypes.Misbehaviour {
@@ -212,43 +296,6 @@ func (s *CCVTestSuite) TestGetByzantineValidators() {
 			[]*tmtypes.Validator{},
 			true,
 		},
-		{
-			"validators who did vote nil should not be returned",
-			func() *ibctmtypes.Misbehaviour {
-				clientHeader := s.consumerChain.CreateTMClientHeader(
-					s.consumerChain.ChainID,
-					int64(clientHeight.RevisionHeight+1),
-					clientHeight,
-					altTime.Add(time.Minute),
-					clientTMValset,
-					clientTMValset,
-					clientTMValset,
-					clientSigners,
-				)
-
-				// create conflicting header with 2/4 validators voting nil
-				clientHeaderWithNilVotes := s.consumerChain.CreateTMClientHeader(
-					s.consumerChain.ChainID,
-					int64(clientHeight.RevisionHeight+1),
-					clientHeight,
-					altTime.Add(time.Hour),
-					clientTMValset,
-					clientTMValset,
-					clientTMValset,
-					clientSigners,
-				)
-				testutil.UpdateHeaderCommitWithNilVotes(clientHeaderWithNilVotes, clientTMValset.Validators[:2])
-
-				return &ibctmtypes.Misbehaviour{
-					ClientId: s.path.EndpointA.ClientID,
-					Header1:  clientHeader,
-					Header2:  clientHeaderWithNilVotes,
-				}
-			},
-			// Expect validators who did NOT vote nil
-			clientTMValset.Validators[2:],
-			true,
-		},
 	}
 
 	for _, tc := range testCases {
@@ -299,10 +346,15 @@ func (s *CCVTestSuite) TestCheckMisbehaviour() {
 
 	// create an alternative validator set using more than 1/3 of the trusted validator set
 	altValset := tmtypes.NewValidatorSet(s.consumerChain.Vals.Validators[0:2])
-	altSigners := make(map[string]tmtypes.PrivValidator, 1)
+	altSigners := make(map[string]tmtypes.PrivValidator, 2)
 	altSigners[clientTMValset.Validators[0].Address.String()] = clientSigners[clientTMValset.Validators[0].Address.String()]
 	altSigners[clientTMValset.Validators[1].Address.String()] = clientSigners[clientTMValset.Validators[1].Address.String()]
 
+	// create an alternative validator set using less 1/3 of the trusted validator set
+	altValset2 := tmtypes.NewValidatorSet(s.consumerChain.Vals.Validators[0:1])
+	altSigners2 := make(map[string]tmtypes.PrivValidator, 1)
+	altSigners2[clientTMValset.Validators[0].Address.String()] = clientSigners[clientTMValset.Validators[0].Address.String()]
+
 	clientHeader := s.consumerChain.CreateTMClientHeader(
 		s.consumerChain.ChainID,
 		int64(clientHeight.RevisionHeight+1),
@@ -315,19 +367,17 @@ func (s *CCVTestSuite) TestCheckMisbehaviour() {
 	)
 
 	// create a conflicting client header with insufficient voting power
-	// by changing 3/4 of its validators votes to nil
-	clientHeaderWithNilVotes := s.consumerChain.CreateTMClientHeader(
+	clientHeader2 := s.consumerChain.CreateTMClientHeader(
 		s.consumerChain.ChainID,
 		int64(clientHeight.RevisionHeight+1),
 		clientHeight,
 		// use a different block time to change the header BlockID
 		headerTs.Add(time.Hour),
+		altValset2,
+		altValset2,
 		clientTMValset,
-		clientTMValset,
-		clientTMValset,
-		clientSigners,
+		altSigners2,
 	)
-	testutil.UpdateHeaderCommitWithNilVotes(clientHeaderWithNilVotes, clientTMValset.Validators[:4])
 
 	testCases := []struct {
 		name         string
@@ -402,7 +452,7 @@ func (s *CCVTestSuite) TestCheckMisbehaviour() {
 			&ibctmtypes.Misbehaviour{
 				ClientId: s.path.EndpointA.ClientID,
 				Header1:  clientHeader,
-				Header2:  clientHeaderWithNilVotes,
+				Header2:  clientHeader2,
 			},
 			false,
 		},

testutil/crypto/evidence.go

@@ -1,11 +1,11 @@
 package crypto
 
 import (
-	"fmt"
 	"time"
 
 	ibctmtypes "github.com/cosmos/ibc-go/v4/modules/light-clients/07-tendermint/types"
 	"github.com/tendermint/tendermint/crypto/tmhash"
+	"github.com/tendermint/tendermint/libs/bytes"
 	tmtypes "github.com/tendermint/tendermint/types"
 )
 
@@ -92,39 +92,43 @@ func MakeAndSignVoteWithForgedValAddress(
 	return vote
 }
 
-// UpdateHeaderCommitWithNilVotes updates the given light client header
-// by changing the commit BlockIDFlag of the given validators to nil
-//
+// CorruptCommitSigsInHeader corrupts the header by changing the value of the commit signature
+// given validator address
 // Note that this method is solely used for testing purposes
-func UpdateHeaderCommitWithNilVotes(header *ibctmtypes.Header, validators []*tmtypes.Validator) {
-	if len(validators) > len(header.ValidatorSet.Validators) {
-		panic(fmt.Sprintf("cannot change more than %d validators votes: got %d",
-			len(header.ValidatorSet.Validators), len(header.ValidatorSet.Validators)))
-	}
-
+func CorruptCommitSigsInHeader(header *ibctmtypes.Header, valAddress bytes.HexBytes) {
 	commit, err := tmtypes.CommitFromProto(header.Commit)
 	if err != nil {
 		panic(err)
 	}
 
+	for idx, sig := range commit.Signatures {
+		if sig.ValidatorAddress.String() == valAddress.String() {
+			sig.Signature = []byte("randomsig")
+			commit.Signatures[idx] = sig
+		}
+	}
+	// update the commit in client the header
+	header.SignedHeader.Commit = commit.ToProto()
+}
+
+// CorruptValidatorPubkeyInHeader corrupts the header by changing the validator pubkey
+// of the given validator address in the validator set
+// Note that this method is solely used for testing purposes
+func CorruptValidatorPubkeyInHeader(header *ibctmtypes.Header, valAddress bytes.HexBytes) {
 	valset, err := tmtypes.ValidatorSetFromProto(header.ValidatorSet)
 	if err != nil {
 		panic(err)
 	}
 
-	for _, v := range validators {
-		// get validator index in valset
-		idx, _ := valset.GetByAddress(v.Address)
-		if idx != -1 {
-			// get validator commit sig
-			s := commit.Signatures[idx]
-			// change BlockIDFlag to nil
-			s.BlockIDFlag = tmtypes.BlockIDFlagNil
-			// update the signatures
-			commit.Signatures[idx] = s
+	for _, v := range valset.Validators {
+		if v.Address.String() == valAddress.String() {
+			v.PubKey = tmtypes.NewMockPV().PrivKey.PubKey()
 		}
 	}
 
-	// update the commit in client the header
-	header.SignedHeader.Commit = commit.ToProto()
+	vs, err := valset.ToProto()
+	if err != nil {
+		panic(err)
+	}
+	header.ValidatorSet = vs
 }

x/ccv/provider/keeper/misbehaviour.go

@@ -101,20 +101,28 @@ func (k Keeper) GetByzantineValidators(ctx sdk.Context, misbehaviour ibctmtypes.
 	// and return the intersection of validators who signed both
 
 	// create a map with the validators' address that signed header1
-	header1Signers := map[string]struct{}{}
-	for _, sign := range lightBlock1.Commit.Signatures {
-		if !sign.ForBlock() {
+	header1Signers := map[string]int{}
+	for idx, sign := range lightBlock1.Commit.Signatures {
+		if sign.Absent() {
 			continue
 		}
-		header1Signers[sign.ValidatorAddress.String()] = struct{}{}
+		header1Signers[sign.ValidatorAddress.String()] = idx
 	}
 
 	// iterate over the header2 signers and check if they signed header1
-	for _, sign := range lightBlock2.Commit.Signatures {
-		if !sign.ForBlock() {
+	for sigIdxHeader2, sign := range lightBlock2.Commit.Signatures {
+		if sign.Absent() {
 			continue
 		}
-		if _, ok := header1Signers[sign.ValidatorAddress.String()]; ok {
+		if sigIdxHeader1, ok := header1Signers[sign.ValidatorAddress.String()]; ok {
+			if err := verifyLightBlockCommitSig(*lightBlock1, sigIdxHeader1); err != nil {
+				return nil, err
+			}
+
+			if err := verifyLightBlockCommitSig(*lightBlock2, sigIdxHeader2); err != nil {
+				return nil, err
+			}
+
 			_, val := lightBlock1.ValidatorSet.GetByAddress(sign.ValidatorAddress)
 			validators = append(validators, val)
 		}
@@ -192,3 +200,24 @@ func headersStateTransitionsAreConflicting(h1, h2 tmtypes.Header) bool {
 		!bytes.Equal(h1.AppHash, h2.AppHash) ||
 		!bytes.Equal(h1.LastResultsHash, h2.LastResultsHash)
 }
+
+func verifyLightBlockCommitSig(lightBlock tmtypes.LightBlock, sigIdx int) error {
+	// get signature
+	sig := lightBlock.Commit.Signatures[sigIdx]
+
+	// get validator
+	_, val := lightBlock.ValidatorSet.GetByAddress(sig.ValidatorAddress)
+
+	// verify validator pubkey correspond 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)
+	}
+
+	// validate signature
+	voteSignBytes := lightBlock.Commit.VoteSignBytes(lightBlock.ChainID, int32(sigIdx))
+	if !val.PubKey.VerifySignature(voteSignBytes, sig.Signature) {
+		return fmt.Errorf("wrong signature (#%d): %X", sigIdx, sig.Signature)
+	}
+
+	return nil
+}