fix!: add check for the height of evidence (backport #2007) (#2011)

fix!: add check for the height of evidence (#2007)

* init commit

* added check, setting, and deleting of the equivocation min height

* update changelog entry

* remove unwwanted changelog entry

---------

Co-authored-by: insumity <karolos@informal.systems>
(cherry picked from commit de9db96)

Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com>

Author: mergify[bot]

.changelog/unreleased/bug-fixes/provider/2007-evidence-min-height-filter.md

@@ -0,0 +1,2 @@
+- Add missing check for the minimum height of evidence in the consumer double-vote handler.
+[#2007](https://github.com/cosmos/interchain-security/pull/2007)

.changelog/unreleased/state-breaking/provider/2007-evidence-min-height-filter.md

@@ -0,0 +1,2 @@
+- Add missing check for the minimum height of evidence in the consumer double-vote handler.
+[#2007](https://github.com/cosmos/interchain-security/pull/2007)

tests/integration/double_vote.go

@@ -86,9 +86,9 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() {
 		s.consumerChain.ChainID,
 	)
 
-	// create a vote using the consumer validator key
-	// with block height that is smaller than the equivocation evidence min height
-	consuVoteOld := testutil.MakeAndSignVote(
+	// create two votes using the consumer validator key that both have
+	// the same block height that is smaller than the equivocation evidence min height
+	consuVoteOld1 := testutil.MakeAndSignVote(
 		blockID1,
 		int64(equivocationEvidenceMinHeight-1),
 		s.consumerCtx().BlockTime(),
@@ -97,6 +97,15 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() {
 		s.consumerChain.ChainID,
 	)
 
+	consuVoteOld2 := testutil.MakeAndSignVote(
+		blockID2,
+		int64(equivocationEvidenceMinHeight-1),
+		s.consumerCtx().BlockTime(),
+		consuValSet,
+		consuSigner,
+		s.consumerChain.ChainID,
+	)
+
 	testCases := []struct {
 		name    string
 		ev      *tmtypes.DuplicateVoteEvidence
@@ -120,8 +129,8 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() {
 		{
 			"evidence is older than equivocation evidence min height - shouldn't pass",
 			&tmtypes.DuplicateVoteEvidence{
-				VoteA:            consuVoteOld,
-				VoteB:            consuBadVote,
+				VoteA:            consuVoteOld1,
+				VoteB:            consuVoteOld2,
 				ValidatorPower:   consuVal.VotingPower,
 				TotalVotingPower: consuVal.VotingPower,
 				Timestamp:        s.consumerCtx().BlockTime(),
@@ -134,7 +143,7 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() {
 			"the votes in the evidence are for different height - shouldn't pass",
 			&tmtypes.DuplicateVoteEvidence{
 				VoteA:            consuVote,
-				VoteB:            consuVoteOld,
+				VoteB:            consuVoteOld1,
 				ValidatorPower:   consuVal.VotingPower,
 				TotalVotingPower: consuVal.VotingPower,
 				Timestamp:        s.consumerCtx().BlockTime(),

tests/integration/misbehaviour.go

@@ -509,7 +509,16 @@ func (s *CCVTestSuite) TestCheckMisbehaviour() {
 					clientTMValset,
 					altSigners,
 				),
-				Header2: clientHeader,
+				Header2: s.consumerChain.CreateTMClientHeader(
+					s.consumerChain.ChainID,
+					int64(equivocationEvidenceMinHeight-1),
+					clientHeight,
+					headerTs,
+					clientTMValset,
+					clientTMValset,
+					clientTMValset,
+					clientSigners,
+				),
 			},
 			false,
 		},

x/ccv/provider/keeper/consumer_equivocation.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/binary"
 	"fmt"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 
 	ibcclienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
 	ibctmtypes "github.com/cosmos/ibc-go/v7/modules/light-clients/07-tendermint"
@@ -35,6 +36,27 @@ func (k Keeper) HandleConsumerDoubleVoting(
 	chainID string,
 	pubkey cryptotypes.PubKey,
 ) error {
+	// check that the evidence is for an ICS consumer chain
+	if _, found := k.GetConsumerClientId(ctx, chainID); !found {
+		return errorsmod.Wrapf(
+			ccvtypes.ErrInvalidDoubleVotingEvidence,
+			"cannot find consumer chain %s",
+			chainID,
+		)
+	}
+
+	// check that the evidence is not too old
+	minHeight := k.GetEquivocationEvidenceMinHeight(ctx, chainID)
+	if uint64(evidence.VoteA.Height) < minHeight {
+		return errorsmod.Wrapf(
+			ccvtypes.ErrInvalidDoubleVotingEvidence,
+			"evidence for consumer chain %s is too old - evidence height (%d), min (%d)",
+			chainID,
+			evidence.VoteA.Height,
+			minHeight,
+		)
+	}
+
 	// verifies the double voting evidence using the consumer chain public key
 	if err := k.VerifyDoubleVotingEvidence(*evidence, chainID, pubkey); err != nil {
 		return err
@@ -269,34 +291,51 @@ func headerToLightBlock(h ibctmtypes.Header) (*tmtypes.LightBlock, error) {
 }
 
 // CheckMisbehaviour checks that headers in the given misbehaviour forms
-// a valid light client attack on a light client that tracks an ICS consumer chain
+// 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 {
+	consumerChainID := misbehaviour.Header1.Header.ChainID
+
 	// check that the misbehaviour is for an ICS consumer chain
-	clientId, found := k.GetConsumerClientId(ctx, misbehaviour.Header1.Header.ChainID)
+	clientId, found := k.GetConsumerClientId(ctx, consumerChainID)
 	if !found {
-		return fmt.Errorf("incorrect misbehaviour with conflicting headers from a non-existent consumer chain: %s", misbehaviour.Header1.Header.ChainID)
+		return fmt.Errorf("incorrect misbehaviour with conflicting headers from a non-existent consumer chain: %s", consumerChainID)
 	} 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,
+			consumerChainID,
 			clientId,
 			misbehaviour.ClientId,
 		)
 	}
 
+	// Check that the headers are at the same height to ensure that
+	// the misbehaviour is for a light client attack and not a time violation,
+	// see ibc-go/modules/light-clients/07-tendermint/types/misbehaviour_handle.go
+	if !misbehaviour.Header1.GetHeight().EQ(misbehaviour.Header2.GetHeight()) {
+		return sdkerrors.Wrap(ibcclienttypes.ErrInvalidMisbehaviour, "headers are not at same height")
+	}
+
+	// Check that the evidence is not too old
+	minHeight := k.GetEquivocationEvidenceMinHeight(ctx, consumerChainID)
+	evidenceHeight := misbehaviour.Header1.GetHeight().GetRevisionHeight()
+	// Note that the revision number is not relevant for checking the age of evidence
+	// as it's already part of the chain ID and the minimum height is mapped to chain IDs
+	if evidenceHeight < minHeight {
+		return errorsmod.Wrapf(
+			ccvtypes.ErrInvalidDoubleVotingEvidence,
+			"evidence for consumer chain %s is too old - evidence height (%d), min (%d)",
+			consumerChainID,
+			evidenceHeight,
+			minHeight,
+		)
+	}
+
 	clientState, found := k.clientKeeper.GetClientState(ctx, clientId)
 	if !found {
 		return errorsmod.Wrapf(ibcclienttypes.ErrClientNotFound, "cannot find client state for client with ID %s", clientId)
 	}
 
 	clientStore := k.clientKeeper.ClientStore(ctx, clientId)
 
-	// Check that the headers are at the same height to ensure that
-	// the misbehaviour is for a light client attack and not a time violation,
-	// see CheckForMisbehaviour in ibc-go/blob/v7.3.0/modules/light-clients/07-tendermint/misbehaviour_handle.go#L73
-	if !misbehaviour.Header1.GetHeight().EQ(misbehaviour.Header2.GetHeight()) {
-		return errorsmod.Wrap(ibcclienttypes.ErrInvalidMisbehaviour, "headers are not at same height")
-	}
-
 	// CheckForMisbehaviour verifies that the headers have different blockID hashes
 	ok := clientState.CheckForMisbehaviour(ctx, k.cdc, clientStore, &misbehaviour)
 	if !ok {

x/ccv/provider/keeper/proposal.go

@@ -58,6 +58,9 @@ func (k Keeper) CreateConsumerClient(ctx sdk.Context, prop *types.ConsumerAdditi
 			fmt.Sprintf("cannot create client for existent consumer chain: %s", chainID))
 	}
 
+	// Set minimum height for equivocation evidence from this consumer chain
+	k.SetEquivocationEvidenceMinHeight(ctx, chainID, prop.InitialHeight.RevisionHeight)
+
 	// Consumers start out with the unbonding period from the consumer addition prop
 	consumerUnbondingPeriod := prop.UnbondingPeriod
 
@@ -219,6 +222,7 @@ func (k Keeper) StopConsumerChain(ctx sdk.Context, chainID string, closeChan boo
 	// Note: this call panics if the key assignment state is invalid
 	k.DeleteKeyAssignments(ctx, chainID)
 	k.DeleteMinimumPowerInTopN(ctx, chainID)
+	k.DeleteEquivocationEvidenceMinHeight(ctx, chainID)
 
 	// close channel and delete the mappings between chain ID and channel ID
 	if channelID, found := k.GetChainToChannel(ctx, chainID); found {