Misbehaviour Followup (#6989)

* cherry-pick first commit and fix conflicts

* fix final issues in client_test

* Apply suggestions from code review

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>

* fix rest of comments

* add validatebasic check and fix tests

* fix godoc

* Update x/ibc/07-tendermint/misbehaviour.go

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>

* address final nits

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
Co-authored-by: Christopher Goes <cwgoes@pluranimity.org>

x/ibc/02-client/keeper/client.go

@@ -132,18 +132,29 @@ func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, misbehaviour ex
 	if !found {
 		return sdkerrors.Wrapf(types.ErrClientNotFound, "cannot check misbehaviour for client with ID %s", misbehaviour.GetClientID())
 	}
+	if err := misbehaviour.ValidateBasic(); err != nil {
+		return sdkerrors.Wrap(err, "IBC misbehaviour failed validate basic")
+	}
 
 	var err error
 	switch e := misbehaviour.(type) {
 	case ibctmtypes.Evidence:
-		// Get ConsensusState at TrustedHeight
+		// Get consensus states at TrustedHeight for each header
-		consensusState, found := k.GetClientConsensusState(ctx, misbehaviour.GetClientID(), e.Header1.TrustedHeight)
+		consensusState1, found := k.GetClientConsensusState(ctx, misbehaviour.GetClientID(), e.Header1.TrustedHeight)
 		if !found {
-			return sdkerrors.Wrapf(types.ErrConsensusStateNotFound, "cannot check misbehaviour for client with ID %s", misbehaviour.GetClientID())
+			return sdkerrors.Wrapf(types.ErrConsensusStateNotFound, "could not find ConsensusState for clientID %s at TrustedHeight (%d) for first header",
+				misbehaviour.GetClientID(), e.Header1.TrustedHeight)
+		}
+		consensusState2, found := k.GetClientConsensusState(ctx, misbehaviour.GetClientID(), e.Header2.TrustedHeight)
+		if !found {
+			return sdkerrors.Wrapf(types.ErrConsensusStateNotFound, "could not find ConsensusState for clientID %s at TrustedHeight (%d) for second header",
+				misbehaviour.GetClientID(), e.Header2.TrustedHeight)
 		}
 
+		// TODO: Retrieve consensusparams from client and not context
+		// Issue #6516: https://github.com/cosmos/cosmos-sdk/issues/6516
 		clientState, err = tendermint.CheckMisbehaviourAndUpdateState(
-			clientState, consensusState, misbehaviour, consensusState.GetHeight(), ctx.BlockTime(), ctx.ConsensusParams(),
+			clientState, consensusState1, consensusState2, misbehaviour, ctx.BlockTime(), ctx.ConsensusParams(),
 		)
 
 	default:

x/ibc/02-client/keeper/client_test.go

@@ -248,8 +248,13 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
 	suite.Require().NoError(err)
 	altVal := tmtypes.NewValidator(altPubKey, 4)
 
+	// Set valSet here with suite.valSet so it doesn't get reset on each testcase
+	valSet := suite.valSet
+	valsHash := valSet.Hash()
+
 	// Create bothValSet with both suite validator and altVal
 	bothValSet := tmtypes.NewValidatorSet(append(suite.valSet.Validators, altVal))
+	bothValsHash := bothValSet.Hash()
 	// Create alternative validator set with only altVal
 	altValSet := tmtypes.NewValidatorSet([]*tmtypes.Validator{altVal})
 
@@ -266,6 +271,9 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
 
 	altSigners := []tmtypes.PrivValidator{altPrivVal}
 
+	// Create valid Misbehaviour by making a duplicate header that signs over different block time
+	altTime := suite.ctx.BlockTime().Add(time.Minute)
+
 	testCases := []struct {
 		name     string
 		evidence ibctmtypes.Evidence
@@ -275,13 +283,13 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
 		{
 			"trusting period misbehavior should pass",
 			ibctmtypes.Evidence{
-				Header1:  ibctmtypes.CreateTestHeader(testClientID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
+				Header1:  ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, altTime, bothValSet, bothValSet, bothSigners),
-				Header2:  ibctmtypes.CreateTestHeader(testClientID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
+				Header2:  ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
-				ChainID:  testClientID,
+				ChainID:  testChainID,
 				ClientID: testClientID,
 			},
 			func() error {
-				suite.consensusState.NextValidatorsHash = bothValSet.Hash()
+				suite.consensusState.NextValidatorsHash = bothValsHash
 				clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs())
 				_, err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState)
 
@@ -292,13 +300,13 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
 		{
 			"misbehavior at later height should pass",
 			ibctmtypes.Evidence{
-				Header1:  ibctmtypes.CreateTestHeader(testClientID, testClientHeight+5, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
+				Header1:  ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight, altTime, bothValSet, valSet, bothSigners),
-				Header2:  ibctmtypes.CreateTestHeader(testClientID, testClientHeight+5, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
+				Header2:  ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight, suite.ctx.BlockTime(), bothValSet, valSet, bothSigners),
-				ChainID:  testClientID,
+				ChainID:  testChainID,
 				ClientID: testClientID,
 			},
 			func() error {
-				suite.consensusState.NextValidatorsHash = bothValSet.Hash()
+				suite.consensusState.NextValidatorsHash = valsHash
 				clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs())
 				_, err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState)
 
@@ -317,6 +325,86 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
 			},
 			true,
 		},
+		{
+			"misbehavior at later height with different trusted heights should pass",
+			ibctmtypes.Evidence{
+				Header1:  ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight, altTime, bothValSet, valSet, bothSigners),
+				Header2:  ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight+3, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
+				ChainID:  testChainID,
+				ClientID: testClientID,
+			},
+			func() error {
+				suite.consensusState.NextValidatorsHash = valsHash
+				clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs())
+				_, err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState)
+
+				// store trusted consensus state for Header2
+				intermediateConsState := &ibctmtypes.ConsensusState{
+					Height:             testClientHeight + 3,
+					Timestamp:          suite.now.Add(time.Minute),
+					NextValidatorsHash: bothValsHash,
+				}
+				suite.keeper.SetClientConsensusState(suite.ctx, testClientID, testClientHeight+3, intermediateConsState)
+
+				clientState.LatestHeight = testClientHeight + 3
+				suite.keeper.SetClientState(suite.ctx, testClientID, clientState)
+
+				return err
+			},
+			true,
+		},
+		{
+			"misbehaviour fails validatebasic",
+			ibctmtypes.Evidence{
+				Header1:  ibctmtypes.CreateTestHeader(testChainID, testClientHeight+1, testClientHeight, altTime, bothValSet, bothValSet, bothSigners),
+				Header2:  ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
+				ChainID:  testChainID,
+				ClientID: testClientID,
+			},
+			func() error {
+				suite.consensusState.NextValidatorsHash = bothValsHash
+				clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs())
+				_, err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState)
+
+				return err
+			},
+			false,
+		},
+		{
+			"trusted ConsensusState1 not found",
+			ibctmtypes.Evidence{
+				Header1:  ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight+3, altTime, bothValSet, bothValSet, bothSigners),
+				Header2:  ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight, suite.ctx.BlockTime(), bothValSet, valSet, bothSigners),
+				ChainID:  testChainID,
+				ClientID: testClientID,
+			},
+			func() error {
+				suite.consensusState.NextValidatorsHash = valsHash
+				clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs())
+				_, err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState)
+				// intermediate consensus state at height + 3 is not created
+				return err
+			},
+			false,
+		},
+		{
+			"trusted ConsensusState2 not found",
+			ibctmtypes.Evidence{
+				Header1:  ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight, altTime, bothValSet, valSet, bothSigners),
+				Header2:  ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight+3, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
+				ChainID:  testChainID,
+				ClientID: testClientID,
+			},
+			func() error {
+				suite.consensusState.NextValidatorsHash = valsHash
+				clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs())
+				_, err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState)
+				// intermediate consensus state at height + 3 is not created
+				return err
+			},
+			false,
+		},
+
 		{
 			"client state not found",
 			ibctmtypes.Evidence{},
@@ -324,41 +412,32 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
 			false,
 		},
 		{
-			"consensus state not found",
+			"client already frozen at earlier height",
 			ibctmtypes.Evidence{
-				Header1:  ibctmtypes.CreateTestHeader(testClientID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
+				Header1:  ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, altTime, bothValSet, bothValSet, bothSigners),
-				Header2:  ibctmtypes.CreateTestHeader(testClientID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
+				Header2:  ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
-				ChainID:  testClientID,
+				ChainID:  testChainID,
 				ClientID: testClientID,
 			},
 			func() error {
-				clientState := &ibctmtypes.ClientState{FrozenHeight: 1, LatestHeight: testClientHeight}
+				suite.consensusState.NextValidatorsHash = bothValsHash
+				clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs())
+				_, err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState)
+
+				clientState.FrozenHeight = 1
 				suite.keeper.SetClientState(suite.ctx, testClientID, clientState)
-				return nil
+
-			},
+				return err
-			false,
-		},
-		{
-			"consensus state not found",
-			ibctmtypes.Evidence{
-				Header1:  ibctmtypes.CreateTestHeader(testClientID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
-				Header2:  ibctmtypes.CreateTestHeader(testClientID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
-				ChainID:  testClientID,
-				ClientID: testClientID,
-			},
-			func() error {
-				clientState := &ibctmtypes.ClientState{FrozenHeight: 1, LatestHeight: testClientHeight}
-				suite.keeper.SetClientState(suite.ctx, testClientID, clientState)
-				return nil
 			},
 			false,
 		},
+
 		{
 			"misbehaviour check failed",
 			ibctmtypes.Evidence{
-				Header1:  ibctmtypes.CreateTestHeader(testClientID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners),
+				Header1:  ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, altTime, bothValSet, bothValSet, bothSigners),
-				Header2:  ibctmtypes.CreateTestHeader(testClientID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), altValSet, bothValSet, altSigners),
+				Header2:  ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), altValSet, bothValSet, altSigners),
-				ChainID:  testClientID,
+				ChainID:  testChainID,
 				ClientID: testClientID,
 			},
 			func() error {
@@ -391,6 +470,8 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
 				clientState, found := suite.keeper.GetClientState(suite.ctx, testClientID)
 				suite.Require().True(found, "valid test case %d failed: %s", i, tc.name)
 				suite.Require().True(clientState.IsFrozen(), "valid test case %d failed: %s", i, tc.name)
+				suite.Require().Equal(uint64(tc.evidence.GetHeight()), clientState.GetFrozenHeight(),
+					"valid test case %d failed: %s. Expected FrozenHeight %d got %d", tc.evidence.GetHeight(), clientState.GetFrozenHeight())
 			} else {
 				suite.Require().Error(err, "invalid test case %d passed: %s", i, tc.name)
 			}

x/ibc/07-tendermint/misbehaviour.go

@@ -1,7 +1,6 @@
 package tendermint
 
 import (
-	"bytes"
 	"time"
 
 	abci "github.com/tendermint/tendermint/abci/types"
@@ -15,13 +14,14 @@ import (
 // CheckMisbehaviourAndUpdateState determines whether or not two conflicting
 // headers at the same height would have convinced the light client.
 //
-// NOTE: assumes provided height is the height at which the consensusState is
+// NOTE: consensusState1 is the trusted consensus state that corresponds to the TrustedHeight
-// stored.
+// of misbehaviour.Header1
+// Similarly, consensusState2 is the trusted consensus state that corresponds
+// to misbehaviour.Header2
 func CheckMisbehaviourAndUpdateState(
 	clientState clientexported.ClientState,
-	consensusState clientexported.ConsensusState,
+	consensusState1, consensusState2 clientexported.ConsensusState,
 	misbehaviour clientexported.Misbehaviour,
-	height uint64, // height at which the consensus state was loaded
 	currentTimestamp time.Time,
 	consensusParams *abci.ConsensusParams,
 ) (clientexported.ClientState, error) {
@@ -38,9 +38,13 @@ func CheckMisbehaviourAndUpdateState(
 			"client is already frozen at earlier height %d than misbehaviour height %d", tmClientState.FrozenHeight, misbehaviour.GetHeight())
 	}
 
-	tmConsensusState, ok := consensusState.(*types.ConsensusState)
+	tmConsensusState1, ok := consensusState1.(*types.ConsensusState)
 	if !ok {
-		return nil, sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "expected type %T, got %T", &types.ConsensusState{}, consensusState)
+		return nil, sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "invalid consensus state type for first header: expected type %T, got %T", &types.ConsensusState{}, consensusState1)
+	}
+	tmConsensusState2, ok := consensusState2.(*types.ConsensusState)
+	if !ok {
+		return nil, sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "invalid consensus state for second header: expected type %T, got %T", &types.ConsensusState{}, consensusState2)
 	}
 
 	tmEvidence, ok := misbehaviour.(types.Evidence)
@@ -48,40 +52,20 @@ func CheckMisbehaviourAndUpdateState(
 		return nil, sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "expected type %T, got %T", misbehaviour, types.Evidence{})
 	}
 
-	if err := checkMisbehaviour(
+	// use earliest height of trusted consensus states to verify ageBlocks
-		tmClientState, tmConsensusState, tmEvidence, height, currentTimestamp, consensusParams,
+	var height uint64
-	); err != nil {
+	if tmConsensusState1.Height < tmConsensusState2.Height {
-		return nil, err
+		height = tmConsensusState1.Height
+	} else {
+		height = tmConsensusState2.Height
 	}
 
-	tmClientState.FrozenHeight = uint64(tmEvidence.GetHeight())
-	return tmClientState, nil
-}
-
-// checkMisbehaviour checks if the evidence provided is a valid light client misbehaviour
-func checkMisbehaviour(
-	clientState *types.ClientState, consensusState *types.ConsensusState, evidence types.Evidence,
-	height uint64, currentTimestamp time.Time, consensusParams *abci.ConsensusParams,
-) error {
 	// calculate the age of the misbehaviour evidence
-	infractionHeight := evidence.GetHeight()
+	infractionHeight := tmEvidence.GetHeight()
-	infractionTime := evidence.GetTime()
+	infractionTime := tmEvidence.GetTime()
 	ageDuration := currentTimestamp.Sub(infractionTime)
 	ageBlocks := uint64(infractionHeight) - height
 
-	// assert that trustedVals is NextValidators of last trusted header
-	// to do this, we check that trustedVals.Hash() == consState.NextValidatorsHash
-	trustedValsetHash1 := evidence.Header1.TrustedValidators.Hash()
-	trustedValsetHash2 := evidence.Header2.TrustedValidators.Hash()
-
-	if !bytes.Equal(consensusState.NextValidatorsHash, trustedValsetHash1) || !bytes.Equal(consensusState.NextValidatorsHash, trustedValsetHash2) {
-		return sdkerrors.Wrapf(
-			types.ErrInvalidValidatorSet,
-			"header's trusted validators %s, does not hash to either consensus state's trusted validator set. Expected: %X, got: TrustedValSet Header1 %X, TrustedValSet Header2 %X",
-			evidence.Header1.TrustedValidators, consensusState.NextValidatorsHash, trustedValsetHash1, trustedValsetHash2,
-		)
-	}
-
 	// Reject misbehaviour if the age is too old. Evidence is considered stale
 	// if the difference in time and number of blocks is greater than the allowed
 	// parameters defined.
@@ -94,47 +78,58 @@ func checkMisbehaviour(
 		consensusParams.Evidence != nil &&
 		ageDuration > consensusParams.Evidence.MaxAgeDuration &&
 		ageBlocks > uint64(consensusParams.Evidence.MaxAgeNumBlocks) {
-		return sdkerrors.Wrapf(clienttypes.ErrInvalidEvidence,
+		return nil, sdkerrors.Wrapf(clienttypes.ErrInvalidEvidence,
 			"age duration (%s) and age blocks (%d) are greater than max consensus params for duration (%s) and block (%d)",
 			ageDuration, ageBlocks, consensusParams.Evidence.MaxAgeDuration, consensusParams.Evidence.MaxAgeNumBlocks,
 		)
 	}
 
-	// check if provided height matches the headers' height
+	// Check the validity of the two conflicting headers against their respective
-	if height > uint64(evidence.GetHeight()) {
+	// trusted consensus states
-		return sdkerrors.Wrapf(
+	// NOTE: header height and commitment root assertions are checked in
-			sdkerrors.ErrInvalidHeight,
+	// evidence.ValidateBasic by the client keeper and msg.ValidateBasic
-			"height > evidence header height (%d > %d)", height, evidence.GetHeight(),
+	// by the base application.
-		)
+	if err := checkMisbehaviourHeader(
+		tmClientState, tmConsensusState1, tmEvidence.Header1, currentTimestamp,
+	); err != nil {
+		return nil, sdkerrors.Wrap(err, "verifying Header1 in Evidence failed")
+	}
+	if err := checkMisbehaviourHeader(
+		tmClientState, tmConsensusState2, tmEvidence.Header2, currentTimestamp,
+	); err != nil {
+		return nil, sdkerrors.Wrap(err, "verifying Header2 in Evidence failed")
 	}
 
-	// NOTE: header height and commitment root assertions are checked with the
+	tmClientState.FrozenHeight = uint64(tmEvidence.GetHeight())
-	// evidence and msg ValidateBasic functions at the AnteHandler level.
+	return tmClientState, nil
+}
+
+// checkMisbehaviourHeader checks that a Header in Misbehaviour is valid evidence given
+// a trusted ConsensusState
+func checkMisbehaviourHeader(
+	clientState *types.ClientState, consState *types.ConsensusState, header types.Header, currentTimestamp time.Time,
+) error {
+	// check the trusted fields for the header against ConsensusState
+	if err := checkTrustedHeader(header, consState); err != nil {
+		return err
+	}
 
 	// assert that the timestamp is not from more than an unbonding period ago
-	if currentTimestamp.Sub(consensusState.Timestamp) >= clientState.UnbondingPeriod {
+	if currentTimestamp.Sub(consState.Timestamp) >= clientState.UnbondingPeriod {
 		return sdkerrors.Wrapf(
 			types.ErrUnbondingPeriodExpired,
 			"current timestamp minus the latest consensus state timestamp is greater than or equal to the unbonding period (%s >= %s)",
-			currentTimestamp.Sub(consensusState.Timestamp), clientState.UnbondingPeriod,
+			currentTimestamp.Sub(consState.Timestamp), clientState.UnbondingPeriod,
 		)
 	}
 
 	// - ValidatorSet must have 2/3 similarity with trusted FromValidatorSet
 	// - ValidatorSets on both headers are valid given the last trusted ValidatorSet
-	if err := evidence.Header1.TrustedValidators.VerifyCommitLightTrusting(
+	if err := header.TrustedValidators.VerifyCommitLightTrusting(
-		evidence.ChainID, evidence.Header1.Commit.BlockID, evidence.Header1.Height,
+		clientState.GetChainID(), header.Commit.BlockID, header.Height,
-		evidence.Header1.Commit, clientState.TrustLevel.ToTendermint(),
+		header.Commit, clientState.TrustLevel.ToTendermint(),
 	); err != nil {
-		return sdkerrors.Wrapf(clienttypes.ErrInvalidEvidence, "validator set in header 1 has too much change from trusted validator set: %v", err)
+		return sdkerrors.Wrapf(clienttypes.ErrInvalidEvidence, "validator set in header has too much change from trusted validator set: %v", err)
 	}
-
-	if err := evidence.Header2.TrustedValidators.VerifyCommitLightTrusting(
-		evidence.ChainID, evidence.Header2.Commit.BlockID, evidence.Header2.Height,
-		evidence.Header2.Commit, clientState.TrustLevel.ToTendermint(),
-	); err != nil {
-		return sdkerrors.Wrapf(clienttypes.ErrInvalidEvidence, "validator set in header 2 has too much change from trusted validator set: %v", err)
-	}
-
 	return nil
 }

x/ibc/07-tendermint/misbehaviour_test.go

@@ -15,7 +15,7 @@ import (
 	commitmenttypes "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment/types"
 )
 
-func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
+func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() {
 	altPrivVal := tmtypes.NewMockPV()
 	altPubKey, err := altPrivVal.GetPubKey()
 	suite.Require().NoError(err)
@@ -45,10 +45,10 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 	testCases := []struct {
 		name            string
 		clientState     clientexported.ClientState
-		consensusState  clientexported.ConsensusState
+		consensusState1 clientexported.ConsensusState
+		consensusState2 clientexported.ConsensusState
 		evidence        clientexported.Misbehaviour
 		consensusParams *abci.ConsensusParams
-		height          uint64
 		timestamp       time.Time
 		expPass         bool
 	}{
@@ -56,6 +56,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 			"valid misbehavior evidence",
 			types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()),
 			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
 			types.Evidence{
 				Header1:  types.CreateTestHeader(chainID, height, height, suite.now, bothValSet, bothValSet, bothSigners),
 				Header2:  types.CreateTestHeader(chainID, height, height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners),
@@ -63,7 +64,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 				ClientID: chainID,
 			},
 			simapp.DefaultConsensusParams,
-			height,
 			suite.now,
 			true,
 		},
@@ -71,6 +71,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 			"valid misbehavior at height greater than last consensusState",
 			types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()),
 			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height-1, bothValsHash),
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height-1, bothValsHash),
 			types.Evidence{
 				Header1:  types.CreateTestHeader(chainID, height, height-1, suite.now, bothValSet, bothValSet, bothSigners),
 				Header2:  types.CreateTestHeader(chainID, height, height-1, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners),
@@ -78,29 +79,74 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 				ClientID: chainID,
 			},
 			simapp.DefaultConsensusParams,
-			height - 1,
+			suite.now,
+			true,
+		},
+		{
+			"valid misbehavior evidence with different trusted heights",
+			types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()),
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height-1, bothValsHash),
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height-3, suite.valsHash),
+			types.Evidence{
+				Header1:  types.CreateTestHeader(chainID, height, height-1, suite.now, bothValSet, bothValSet, bothSigners),
+				Header2:  types.CreateTestHeader(chainID, height, height-3, suite.now.Add(time.Minute), bothValSet, suite.valSet, bothSigners),
+				ChainID:  chainID,
+				ClientID: chainID,
+			},
+			simapp.DefaultConsensusParams,
 			suite.now,
 			true,
 		},
 		{
 			"consensus state's valset hash different from evidence should still pass",
 			types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()),
-			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, suite.valsHash),
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height-1, suite.valsHash),
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height-1, suite.valsHash),
 			types.Evidence{
-				Header1:  types.CreateTestHeader(chainID, height, height, suite.now, bothValSet, suite.valSet, bothSigners),
+				Header1:  types.CreateTestHeader(chainID, height, height-1, suite.now, bothValSet, suite.valSet, bothSigners),
+				Header2:  types.CreateTestHeader(chainID, height, height-1, suite.now.Add(time.Minute), bothValSet, suite.valSet, bothSigners),
+				ChainID:  chainID,
+				ClientID: chainID,
+			},
+			simapp.DefaultConsensusParams,
+			suite.now,
+			true,
+		},
+		{
+			"invalid misbehavior evidence with trusted height different from trusted consensus state",
+			types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()),
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height-1, bothValsHash),
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height-3, suite.valsHash),
+			types.Evidence{
+				Header1:  types.CreateTestHeader(chainID, height, height-1, suite.now, bothValSet, bothValSet, bothSigners),
 				Header2:  types.CreateTestHeader(chainID, height, height, suite.now.Add(time.Minute), bothValSet, suite.valSet, bothSigners),
 				ChainID:  chainID,
 				ClientID: chainID,
 			},
 			simapp.DefaultConsensusParams,
-			height,
 			suite.now,
-			true,
+			false,
+		},
+		{
+			"invalid misbehavior evidence with trusted validators different from trusted consensus state",
+			types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()),
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height-1, bothValsHash),
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height-3, suite.valsHash),
+			types.Evidence{
+				Header1:  types.CreateTestHeader(chainID, height, height-1, suite.now, bothValSet, bothValSet, bothSigners),
+				Header2:  types.CreateTestHeader(chainID, height, height-3, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners),
+				ChainID:  chainID,
+				ClientID: chainID,
+			},
+			simapp.DefaultConsensusParams,
+			suite.now,
+			false,
 		},
 		{
 			"invalid tendermint client state",
 			nil,
 			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
 			types.Evidence{
 				Header1:  types.CreateTestHeader(chainID, height, height, suite.now, bothValSet, bothValSet, bothSigners),
 				Header2:  types.CreateTestHeader(chainID, height, height, suite.now.Add(time.Minute), bothValSet, altValSet, bothSigners),
@@ -108,13 +154,13 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 				ClientID: chainID,
 			},
 			simapp.DefaultConsensusParams,
-			height,
 			suite.now,
 			false,
 		},
 		{
 			"already frozen client state",
-			&types.ClientState{FrozenHeight: 1},
+			types.ClientState{FrozenHeight: 1},
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
 			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
 			types.Evidence{
 				Header1:  types.CreateTestHeader(chainID, height, height, suite.now, bothValSet, bothValSet, bothSigners),
@@ -123,7 +169,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 				ClientID: chainID,
 			},
 			simapp.DefaultConsensusParams,
-			height,
 			suite.now,
 			false,
 		},
@@ -131,6 +176,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 			"invalid tendermint consensus state",
 			types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()),
 			nil,
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
 			types.Evidence{
 				Header1:  types.CreateTestHeader(chainID, height, height, suite.now, bothValSet, bothValSet, bothSigners),
 				Header2:  types.CreateTestHeader(chainID, height, height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners),
@@ -138,7 +184,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 				ClientID: chainID,
 			},
 			simapp.DefaultConsensusParams,
-			height,
 			suite.now,
 			false,
 		},
@@ -146,9 +191,9 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 			"invalid tendermint misbehaviour evidence",
 			types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()),
 			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
 			nil,
 			simapp.DefaultConsensusParams,
-			height,
 			suite.now,
 			false,
 		},
@@ -156,6 +201,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 			"rejected misbehaviour due to expired age",
 			types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()),
 			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
 			types.Evidence{
 				Header1: types.CreateTestHeader(chainID, int64(2*height+uint64(simapp.DefaultConsensusParams.Evidence.MaxAgeNumBlocks)), height,
 					suite.now, bothValSet, bothValSet, bothSigners),
@@ -165,7 +211,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 				ClientID: chainID,
 			},
 			simapp.DefaultConsensusParams,
-			height,
 			suite.now.Add(2 * time.Minute).Add(simapp.DefaultConsensusParams.Evidence.MaxAgeDuration),
 			false,
 		},
@@ -173,6 +218,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 			"provided height > header height",
 			types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()),
 			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
 			types.Evidence{
 				Header1:  types.CreateTestHeader(chainID, height, height-1, suite.now, bothValSet, bothValSet, bothSigners),
 				Header2:  types.CreateTestHeader(chainID, height, height-1, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners),
@@ -180,14 +226,14 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 				ClientID: chainID,
 			},
 			simapp.DefaultConsensusParams,
-			height + 10,
 			suite.now,
 			false,
 		},
 		{
 			"unbonding period expired",
 			types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()),
-			types.NewConsensusState(time.Time{}, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
+			types.ConsensusState{Timestamp: time.Time{}, Root: commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), NextValidatorsHash: bothValsHash},
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
 			types.Evidence{
 				Header1:  types.CreateTestHeader(chainID, height, height, suite.now, bothValSet, bothValSet, bothSigners),
 				Header2:  types.CreateTestHeader(chainID, height, height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners),
@@ -195,7 +241,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 				ClientID: chainID,
 			},
 			simapp.DefaultConsensusParams,
-			height,
 			suite.now,
 			false,
 		},
@@ -203,6 +248,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 			"trusted validators is incorrect for given consensus state",
 			types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()),
 			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
 			types.Evidence{
 				Header1:  types.CreateTestHeader(chainID, height, height, suite.now, bothValSet, suite.valSet, bothSigners),
 				Header2:  types.CreateTestHeader(chainID, height, height, suite.now.Add(time.Minute), bothValSet, suite.valSet, bothSigners),
@@ -210,7 +256,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 				ClientID: chainID,
 			},
 			simapp.DefaultConsensusParams,
-			height,
 			suite.now,
 			false,
 		},
@@ -218,6 +263,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 			"first valset has too much change",
 			types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()),
 			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
 			types.Evidence{
 				Header1:  types.CreateTestHeader(chainID, height, height, suite.now, altValSet, bothValSet, altSigners),
 				Header2:  types.CreateTestHeader(chainID, height, height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners),
@@ -225,7 +271,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 				ClientID: chainID,
 			},
 			simapp.DefaultConsensusParams,
-			height,
 			suite.now,
 			false,
 		},
@@ -233,6 +278,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 			"second valset has too much change",
 			types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()),
 			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
 			types.Evidence{
 				Header1:  types.CreateTestHeader(chainID, height, height, suite.now, bothValSet, bothValSet, bothSigners),
 				Header2:  types.CreateTestHeader(chainID, height, height, suite.now.Add(time.Minute), altValSet, bothValSet, altSigners),
@@ -240,7 +286,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 				ClientID: chainID,
 			},
 			simapp.DefaultConsensusParams,
-			height,
 			suite.now,
 			false,
 		},
@@ -248,6 +293,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 			"both valsets have too much change",
 			types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()),
 			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
+			types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash),
 			types.Evidence{
 				Header1:  types.CreateTestHeader(chainID, height, height, suite.now, altValSet, bothValSet, altSigners),
 				Header2:  types.CreateTestHeader(chainID, height, height, suite.now.Add(time.Minute), altValSet, bothValSet, altSigners),
@@ -255,7 +301,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 				ClientID: chainID,
 			},
 			simapp.DefaultConsensusParams,
-			height,
 			suite.now,
 			false,
 		},
@@ -264,12 +309,14 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() {
 	for i, tc := range testCases {
 		tc := tc
 
-		clientState, err := tendermint.CheckMisbehaviourAndUpdateState(tc.clientState, tc.consensusState, tc.evidence, tc.height, tc.timestamp, tc.consensusParams)
+		clientState, err := tendermint.CheckMisbehaviourAndUpdateState(tc.clientState, tc.consensusState1, tc.consensusState2, tc.evidence, tc.timestamp, tc.consensusParams)
 
 		if tc.expPass {
 			suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.name)
 			suite.Require().NotNil(clientState, "valid test case %d failed: %s", i, tc.name)
 			suite.Require().True(clientState.IsFrozen(), "valid test case %d failed: %s", i, tc.name)
+			suite.Require().Equal(uint64(tc.evidence.GetHeight()), clientState.GetFrozenHeight(),
+				"valid test case %d failed: %s. Expected FrozenHeight %d got %d", tc.evidence.GetHeight(), clientState.GetFrozenHeight())
 		} else {
 			suite.Require().Error(err, "invalid test case %d passed: %s", i, tc.name)
 			suite.Require().Nil(clientState, "invalid test case %d passed: %s", i, tc.name)

x/ibc/07-tendermint/types/evidence.go

@@ -1,7 +1,6 @@
 package types
 
 import (
-	"bytes"
 	"math"
 	"time"
 
@@ -86,17 +85,19 @@ func (ev Evidence) GetTime() time.Time {
 
 // ValidateBasic implements Evidence interface
 func (ev Evidence) ValidateBasic() error {
-	if ev.Header1.TrustedHeight != ev.Header2.TrustedHeight {
+	if ev.Header1.TrustedHeight == 0 {
-		return sdkerrors.Wrapf(ErrInvalidHeaderHeight, "evidence headers must share the same trusted height. got height1: %d, height2: %d",
+		return sdkerrors.Wrap(ErrInvalidHeaderHeight, "evidence Header1 must have non-zero trusted height")
-			ev.Header1.TrustedHeight, ev.Header2.TrustedHeight)
 	}
-	if !bytes.Equal(ev.Header1.TrustedValidators.Hash(), ev.Header2.TrustedValidators.Hash()) {
+	if ev.Header2.TrustedHeight == 0 {
-		return sdkerrors.Wrapf(ErrInvalidValidatorSet, "trusted validators on both submitted headers must be the same. Got valset1: %s, valset2: %s",
+		return sdkerrors.Wrap(ErrInvalidHeaderHeight, "evidence Header2 must have non-zero trusted height")
-			ev.Header1.TrustedValidators, ev.Header2.TrustedValidators)
 	}
 	if ev.Header1.TrustedValidators == nil {
-		return sdkerrors.Wrap(ErrInvalidValidatorSet, "trusted validator set cannot be empty")
+		return sdkerrors.Wrap(ErrInvalidValidatorSet, "trusted validator set in Header1 cannot be empty")
 	}
+	if ev.Header2.TrustedValidators == nil {
+		return sdkerrors.Wrap(ErrInvalidValidatorSet, "trusted validator set in Header2 cannot be empty")
+	}
+
 	if err := host.ClientIdentifierValidator(ev.ClientID); err != nil {
 		return sdkerrors.Wrap(err, "evidence client ID is invalid")
 	}

x/ibc/07-tendermint/types/evidence_test.go

@@ -19,11 +19,11 @@ func (suite *TendermintTestSuite) TestEvidence() {
 		Header1:  suite.header,
 		Header2:  ibctmtypes.CreateTestHeader(chainID, height, height-1, suite.now, suite.valSet, suite.valSet, signers),
 		ChainID:  chainID,
-		ClientID: "gaiamainnet",
+		ClientID: clientID,
 	}
 
 	suite.Require().Equal(ev.ClientType(), clientexported.Tendermint)
-	suite.Require().Equal(ev.GetClientID(), "gaiamainnet")
+	suite.Require().Equal(ev.GetClientID(), clientID)
 	suite.Require().Equal(ev.Route(), "client")
 	suite.Require().Equal(ev.Type(), "client_misbehaviour")
 	suite.Require().Equal(ev.Hash(), tmbytes.HexBytes(tmhash.Sum(suite.aminoCdc.MustMarshalBinaryBare(ev))))
@@ -69,29 +69,62 @@ func (suite *TendermintTestSuite) TestEvidenceValidateBasic() {
 				Header1:  suite.header,
 				Header2:  ibctmtypes.CreateTestHeader(chainID, height, height-1, suite.now.Add(time.Minute), suite.valSet, suite.valSet, signers),
 				ChainID:  chainID,
-				ClientID: "gaiamainnet",
+				ClientID: clientID,
 			},
 			func(ev *ibctmtypes.Evidence) error { return nil },
 			true,
 		},
 		{
-			"trusted heights don't match",
+			"valid evidence with different trusted headers",
 			ibctmtypes.Evidence{
 				Header1:  suite.header,
-				Header2:  ibctmtypes.CreateTestHeader(chainID, height, height-2, suite.now.Add(time.Minute), suite.valSet, suite.valSet, signers),
+				Header2:  ibctmtypes.CreateTestHeader(chainID, height, height-3, suite.now.Add(time.Minute), suite.valSet, bothValSet, signers),
 				ChainID:  chainID,
-				ClientID: "gaiamainnet",
+				ClientID: clientID,
+			},
+			func(ev *ibctmtypes.Evidence) error { return nil },
+			true,
+		},
+		{
+			"trusted height is 0 in Header1",
+			ibctmtypes.Evidence{
+				Header1:  ibctmtypes.CreateTestHeader(chainID, height, 0, suite.now.Add(time.Minute), suite.valSet, suite.valSet, signers),
+				Header2:  suite.header,
+				ChainID:  chainID,
+				ClientID: clientID,
 			},
 			func(ev *ibctmtypes.Evidence) error { return nil },
 			false,
 		},
 		{
-			"trusted valsets don't match",
+			"trusted height is 0 in Header2",
 			ibctmtypes.Evidence{
 				Header1:  suite.header,
-				Header2:  ibctmtypes.CreateTestHeader(chainID, height, height-1, suite.now.Add(time.Minute), suite.valSet, bothValSet, signers),
+				Header2:  ibctmtypes.CreateTestHeader(chainID, height, 0, suite.now.Add(time.Minute), suite.valSet, suite.valSet, signers),
 				ChainID:  chainID,
-				ClientID: "gaiamainnet",
+				ClientID: clientID,
+			},
+			func(ev *ibctmtypes.Evidence) error { return nil },
+			false,
+		},
+		{
+			"trusted valset is nil in Header1",
+			ibctmtypes.Evidence{
+				Header1:  ibctmtypes.CreateTestHeader(chainID, height, height-1, suite.now.Add(time.Minute), suite.valSet, nil, signers),
+				Header2:  suite.header,
+				ChainID:  chainID,
+				ClientID: clientID,
+			},
+			func(ev *ibctmtypes.Evidence) error { return nil },
+			false,
+		},
+		{
+			"trusted valset is nil in Header2",
+			ibctmtypes.Evidence{
+				Header1:  suite.header,
+				Header2:  ibctmtypes.CreateTestHeader(chainID, height, height-1, suite.now.Add(time.Minute), suite.valSet, nil, signers),
+				ChainID:  chainID,
+				ClientID: clientID,
 			},
 			func(ev *ibctmtypes.Evidence) error { return nil },
 			false,
@@ -113,7 +146,7 @@ func (suite *TendermintTestSuite) TestEvidenceValidateBasic() {
 				Header1:  suite.header,
 				Header2:  ibctmtypes.CreateTestHeader("ethermint", height, height-1, suite.now, suite.valSet, suite.valSet, signers),
 				ChainID:  "ethermint",
-				ClientID: "gaiamainnet",
+				ClientID: clientID,
 			},
 			func(ev *ibctmtypes.Evidence) error { return nil },
 			false,
@@ -124,7 +157,7 @@ func (suite *TendermintTestSuite) TestEvidenceValidateBasic() {
 				Header1:  suite.header,
 				Header2:  ibctmtypes.CreateTestHeader("ethermint", height, height-1, suite.now, suite.valSet, suite.valSet, signers),
 				ChainID:  chainID,
-				ClientID: "gaiamainnet",
+				ClientID: clientID,
 			},
 			func(ev *ibctmtypes.Evidence) error { return nil },
 			false,
@@ -135,7 +168,7 @@ func (suite *TendermintTestSuite) TestEvidenceValidateBasic() {
 				Header1:  suite.header,
 				Header2:  ibctmtypes.CreateTestHeader(chainID, 6, 4, suite.now, suite.valSet, suite.valSet, signers),
 				ChainID:  chainID,
-				ClientID: "gaiamainnet",
+				ClientID: clientID,
 			},
 			func(ev *ibctmtypes.Evidence) error { return nil },
 			false,
@@ -146,7 +179,7 @@ func (suite *TendermintTestSuite) TestEvidenceValidateBasic() {
 				Header1:  suite.header,
 				Header2:  suite.header,
 				ChainID:  chainID,
-				ClientID: "gaiamainnet",
+				ClientID: clientID,
 			},
 			func(ev *ibctmtypes.Evidence) error { return nil },
 			false,
@@ -157,7 +190,7 @@ func (suite *TendermintTestSuite) TestEvidenceValidateBasic() {
 				Header1:  ibctmtypes.CreateTestHeader(chainID, height, height-1, suite.now, bothValSet, suite.valSet, bothSigners),
 				Header2:  suite.header,
 				ChainID:  chainID,
-				ClientID: "gaiamainnet",
+				ClientID: clientID,
 			},
 			func(ev *ibctmtypes.Evidence) error {
 				// voteSet contains only altVal which is less than 2/3 of total power (height/1height)
@@ -174,7 +207,7 @@ func (suite *TendermintTestSuite) TestEvidenceValidateBasic() {
 				Header1:  suite.header,
 				Header2:  ibctmtypes.CreateTestHeader(chainID, height, height-1, suite.now, bothValSet, suite.valSet, bothSigners),
 				ChainID:  chainID,
-				ClientID: "gaiamainnet",
+				ClientID: clientID,
 			},
 			func(ev *ibctmtypes.Evidence) error {
 				// voteSet contains only altVal which is less than 2/3 of total power (height/1height)
@@ -191,7 +224,7 @@ func (suite *TendermintTestSuite) TestEvidenceValidateBasic() {
 				Header1:  suite.header,
 				Header2:  ibctmtypes.CreateTestHeader(chainID, height, height-1, suite.now, bothValSet, suite.valSet, bothSigners),
 				ChainID:  chainID,
-				ClientID: "gaiamainnet",
+				ClientID: clientID,
 			},
 			func(ev *ibctmtypes.Evidence) error {
 				ev.Header2.Commit.BlockID = ibctmtypes.MakeBlockID(tmhash.Sum([]byte("other_hash")), 3, tmhash.Sum([]byte("other_partset")))

x/ibc/07-tendermint/types/tendermint_test.go

@@ -18,6 +18,7 @@ import (
 
 const (
 	chainID                      = "gaia"
+	clientID                     = "gaiamainnet"
 	height                       = 4
 	trustingPeriod time.Duration = time.Hour * 24 * 7 * 2
 	ubdPeriod      time.Duration = time.Hour * 24 * 7 * 3

x/ibc/07-tendermint/update.go

@@ -66,12 +66,15 @@ func CheckValidityAndUpdateState(
 	return tmClientState, consensusState, nil
 }
 
-// checkValidity checks if the Tendermint header is valid.
+// checkTrustedHeader checks that consensus state matches trusted fields of Header
-// CONTRACT: consState.Height == header.TrustedHeight
+func checkTrustedHeader(header types.Header, consState *types.ConsensusState) error {
-func checkValidity(
+	if header.TrustedHeight != consState.Height {
-	clientState *types.ClientState, consState *types.ConsensusState,
+		return sdkerrors.Wrapf(
-	header types.Header, currentTimestamp time.Time,
+			types.ErrInvalidHeaderHeight,
-) error {
+			"trusted header height %d does not match consensus state height %d",
+			header.TrustedHeight, consState.Height,
+		)
+	}
 	// assert that trustedVals is NextValidators of last trusted header
 	// to do this, we check that trustedVals.Hash() == consState.NextValidatorsHash
 	tvalHash := header.TrustedValidators.Hash()
@@ -82,30 +85,17 @@ func checkValidity(
 			header.TrustedValidators, consState.NextValidatorsHash, tvalHash,
 		)
 	}
-	// assert trusting period has not yet passed
+	return nil
-	if currentTimestamp.Sub(consState.Timestamp) >= clientState.TrustingPeriod {
+}
-		return sdkerrors.Wrapf(
-			types.ErrTrustingPeriodExpired,
-			"current timestamp minus the consensus state timestamp is greater than or equal to the trusting period (%s >= %s)",
-			currentTimestamp.Sub(consState.Timestamp), clientState.TrustingPeriod,
-		)
-	}
 
-	// assert header timestamp is not past the trusting period
+// checkValidity checks if the Tendermint header is valid.
-	if header.Time.Sub(consState.Timestamp) >= clientState.TrustingPeriod {
+// CONTRACT: consState.Height == header.TrustedHeight
-		return sdkerrors.Wrap(
+func checkValidity(
-			clienttypes.ErrInvalidHeader,
+	clientState *types.ClientState, consState *types.ConsensusState,
-			"header timestamp is beyond trusting period in relation to the consensus state timestamp",
+	header types.Header, currentTimestamp time.Time,
-		)
+) error {
-	}
+	if err := checkTrustedHeader(header, consState); err != nil {
-
+		return err
-	// assert header timestamp is past latest stored consensus state timestamp
-	if header.Time.Unix() <= consState.Timestamp.Unix() {
-		return sdkerrors.Wrapf(
-			clienttypes.ErrInvalidHeader,
-			"header timestamp ≤ consensus state timestamp (%s ≤ %s)",
-			header.Time.UTC(), consState.Timestamp.UTC(),
-		)
 	}
 
 	// assert header height is newer than consensus state
@@ -128,6 +118,10 @@ func checkValidity(
 	}
 
 	// Verify next header with the passed-in trustedVals
+	// - asserts trusting period not passed
+	// - assert header timestamp is not past the trusting period
+	// - assert header timestamp is past latest stored consensus state timestamp
+	// - assert that a TrustLevel proportion of TrustedValidators signed new Commit
 	err := lite.Verify(
 		clientState.GetChainID(), &signedHeader,
 		header.TrustedValidators, &header.SignedHeader, header.ValidatorSet,