diff --git a/tests/integration/misbehaviour.go b/tests/integration/misbehaviour.go
index 766fa41e5b..306fb06ec4 100644
--- a/tests/integration/misbehaviour.go
+++ b/tests/integration/misbehaviour.go
@@ -5,6 +5,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
ibctmtypes "github.com/cosmos/ibc-go/v4/modules/light-clients/07-tendermint/types"
+ testutil "github.com/cosmos/interchain-security/v2/testutil/crypto"
"github.com/cosmos/interchain-security/v2/x/ccv/provider/types"
tmtypes "github.com/tendermint/tendermint/types"
)
@@ -211,6 +212,43 @@ 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 {
@@ -223,21 +261,17 @@ func (s *CCVTestSuite) TestGetByzantineValidators() {
s.NoError(err)
s.Equal(len(tc.expByzantineValidators), len(byzantineValidators))
- // For both lunatic and equivocation attacks all the validators
- // who signed the bad header (Header2) should be in returned in the evidence
+ // For both lunatic and equivocation attacks, all the validators
+ // who signed both headers and didn't vote nil should be returned
if len(tc.expByzantineValidators) > 0 {
- equivocatingVals := tc.getMisbehaviour().Header2.ValidatorSet
- s.Equal(len(equivocatingVals.Validators), len(byzantineValidators))
-
- vs, err := tmtypes.ValidatorSetFromProto(equivocatingVals)
+ expValset := tmtypes.NewValidatorSet(tc.expByzantineValidators)
s.NoError(err)
for _, v := range tc.expByzantineValidators {
- idx, _ := vs.GetByAddress(v.Address)
+ idx, _ := expValset.GetByAddress(v.Address)
s.True(idx >= 0)
}
}
-
} else {
s.Error(err)
}
@@ -268,27 +302,53 @@ func (s *CCVTestSuite) TestCheckMisbehaviour() {
altSigners := make(map[string]tmtypes.PrivValidator, 1)
altSigners[clientTMValset.Validators[0].Address.String()] = clientSigners[clientTMValset.Validators[0].Address.String()]
altSigners[clientTMValset.Validators[1].Address.String()] = clientSigners[clientTMValset.Validators[1].Address.String()]
+
+ clientHeader := s.consumerChain.CreateTMClientHeader(
+ s.consumerChain.ChainID,
+ int64(clientHeight.RevisionHeight+1),
+ clientHeight,
+ headerTs,
+ clientTMValset,
+ clientTMValset,
+ clientTMValset,
+ clientSigners,
+ )
+
+ // create a conflicting client header with insufficient voting power
+ // by changing 3/4 of its validators votes to nil
+ clientHeaderWithNilVotes := 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),
+ clientTMValset,
+ clientTMValset,
+ clientTMValset,
+ clientSigners,
+ )
+ testutil.UpdateHeaderCommitWithNilVotes(clientHeaderWithNilVotes, clientTMValset.Validators[:4])
+
testCases := []struct {
name string
misbehaviour *ibctmtypes.Misbehaviour
expPass bool
}{
{
- "client state not found - shouldn't pass",
+ "identical headers - shouldn't pass",
&ibctmtypes.Misbehaviour{
- ClientId: "clientID",
+ ClientId: s.path.EndpointA.ClientID,
+ Header1: clientHeader,
+ Header2: clientHeader,
+ },
+ false,
+ },
+ {
+ "misbehaviour isn't for a consumer chain - shouldn't pass",
+ &ibctmtypes.Misbehaviour{
+ ClientId: s.path.EndpointA.ClientID,
Header1: s.consumerChain.CreateTMClientHeader(
- s.consumerChain.ChainID,
- int64(clientHeight.RevisionHeight+1),
- clientHeight,
- headerTs,
- clientTMValset,
- clientTMValset,
- clientTMValset,
- clientSigners,
- ),
- Header2: s.consumerChain.CreateTMClientHeader(
- s.consumerChain.ChainID,
+ "aChainID",
int64(clientHeight.RevisionHeight+1),
clientHeight,
headerTs,
@@ -297,13 +357,15 @@ func (s *CCVTestSuite) TestCheckMisbehaviour() {
clientTMValset,
altSigners,
),
+ Header2: clientHeader,
},
false,
},
{
- "invalid misbehaviour with empty header1 - shouldn't pass",
+ "client ID doesn't correspond to the client ID of consumer chain - shouldn't pass",
&ibctmtypes.Misbehaviour{
- Header1: &ibctmtypes.Header{},
+ ClientId: "clientID",
+ Header1: clientHeader,
Header2: s.consumerChain.CreateTMClientHeader(
s.consumerChain.ChainID,
int64(clientHeight.RevisionHeight+1),
@@ -321,16 +383,7 @@ func (s *CCVTestSuite) TestCheckMisbehaviour() {
"invalid misbehaviour with different header height - shouldn't pass",
&ibctmtypes.Misbehaviour{
ClientId: s.path.EndpointA.ClientID,
- Header1: s.consumerChain.CreateTMClientHeader(
- s.consumerChain.ChainID,
- int64(clientHeight.RevisionHeight+1),
- clientHeight,
- headerTs,
- clientTMValset,
- clientTMValset,
- clientTMValset,
- clientSigners,
- ),
+ Header1: clientHeader,
Header2: s.consumerChain.CreateTMClientHeader(
s.consumerChain.ChainID,
int64(clientHeight.RevisionHeight+2),
@@ -345,49 +398,20 @@ func (s *CCVTestSuite) TestCheckMisbehaviour() {
false,
},
{
- "valid misbehaviour - should pass",
+ "one header of the misbehaviour has insufficient voting power - shouldn't pass",
&ibctmtypes.Misbehaviour{
ClientId: s.path.EndpointA.ClientID,
- Header1: s.consumerChain.CreateTMClientHeader(
- s.consumerChain.ChainID,
- int64(clientHeight.RevisionHeight+1),
- clientHeight,
- headerTs,
- clientTMValset,
- clientTMValset,
- clientTMValset,
- clientSigners,
- ),
- // create header using a different validator set
- Header2: s.consumerChain.CreateTMClientHeader(
- s.consumerChain.ChainID,
- int64(clientHeight.RevisionHeight+1),
- clientHeight,
- headerTs,
- altValset,
- altValset,
- clientTMValset,
- altSigners,
- ),
+ Header1: clientHeader,
+ Header2: clientHeaderWithNilVotes,
},
- true,
+ false,
},
{
- "valid misbehaviour with already frozen client - should pass",
+ "valid misbehaviour - should pass",
&ibctmtypes.Misbehaviour{
ClientId: s.path.EndpointA.ClientID,
- Header1: s.consumerChain.CreateTMClientHeader(
- s.consumerChain.ChainID,
- int64(clientHeight.RevisionHeight+1),
- clientHeight,
- headerTs,
- clientTMValset,
- clientTMValset,
- clientTMValset,
- clientSigners,
- ),
- // the resulting Header2 will have a different BlockID
- // than Header1 since doesn't share the same valset and signers
+ Header1: clientHeader,
+ // create header using a different validator set
Header2: s.consumerChain.CreateTMClientHeader(
s.consumerChain.ChainID,
int64(clientHeight.RevisionHeight+1),
diff --git a/testutil/crypto/evidence.go b/testutil/crypto/evidence.go
index 3f75010fa1..1a2c2cbc7e 100644
--- a/testutil/crypto/evidence.go
+++ b/testutil/crypto/evidence.go
@@ -1,8 +1,10 @@
package crypto
import (
+ "fmt"
"time"
+ ibctmtypes "github.com/cosmos/ibc-go/v4/modules/light-clients/07-tendermint/types"
"github.com/tendermint/tendermint/crypto/tmhash"
tmtypes "github.com/tendermint/tendermint/types"
)
@@ -89,3 +91,40 @@ func MakeAndSignVoteWithForgedValAddress(
vote.Signature = v.Signature
return vote
}
+
+// UpdateHeaderCommitWithNilVotes updates the given light client header
+// by changing the commit BlockIDFlag of the given validators to nil
+//
+// 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)))
+ }
+
+ commit, err := tmtypes.CommitFromProto(header.Commit)
+ if err != nil {
+ panic(err)
+ }
+
+ 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
+ }
+ }
+
+ // update the commit in client the header
+ header.SignedHeader.Commit = commit.ToProto()
+}
diff --git a/x/ccv/provider/keeper/misbehaviour.go b/x/ccv/provider/keeper/misbehaviour.go
index c9749a1245..7307284655 100644
--- a/x/ccv/provider/keeper/misbehaviour.go
+++ b/x/ccv/provider/keeper/misbehaviour.go
@@ -103,7 +103,7 @@ func (k Keeper) GetByzantineValidators(ctx sdk.Context, misbehaviour ibctmtypes.
// create a map with the validators' address that signed header1
header1Signers := map[string]struct{}{}
for _, sign := range lightBlock1.Commit.Signatures {
- if sign.Absent() {
+ if !sign.ForBlock() {
continue
}
header1Signers[sign.ValidatorAddress.String()] = struct{}{}
@@ -111,7 +111,7 @@ func (k Keeper) GetByzantineValidators(ctx sdk.Context, misbehaviour ibctmtypes.
// iterate over the header2 signers and check if they signed header1
for _, sign := range lightBlock2.Commit.Signatures {
- if sign.Absent() {
+ if !sign.ForBlock() {
continue
}
if _, ok := header1Signers[sign.ValidatorAddress.String()]; ok {
@@ -142,9 +142,21 @@ func headerToLightBlock(h ibctmtypes.Header) (*tmtypes.LightBlock, error) {
}
// CheckMisbehaviour checks that headers in the given misbehaviour forms
-// a valid light client attack and that the corresponding light client isn't expired
+// a valid light client attack from an ICS consumer chain and that the light client isn't expired
func (k Keeper) CheckMisbehaviour(ctx sdk.Context, misbehaviour ibctmtypes.Misbehaviour) error {
- clientState, found := k.clientKeeper.GetClientState(ctx, misbehaviour.GetClientID())
+ // check that the misbehaviour is for an ICS consumer chain
+ clientId, found := k.GetConsumerClientId(ctx, misbehaviour.Header1.Header.ChainID)
+ if !found {
+ return fmt.Errorf("incorrect misbehaviour with conflicting headers from a non-existent consumer chain: %s", misbehaviour.Header1.Header.ChainID)
+ } else if misbehaviour.ClientId != clientId {
+ return fmt.Errorf("incorrect misbehaviour: expected client ID for consumer chain %s is %s got %s",
+ misbehaviour.Header1.Header.ChainID,
+ clientId,
+ misbehaviour.ClientId,
+ )
+ }
+
+ clientState, found := k.clientKeeper.GetClientState(ctx, clientId)
if !found {
return sdkerrors.Wrapf(ibcclienttypes.ErrClientNotFound, "cannot check misbehaviour for client with ID %s", misbehaviour.GetClientID())
}