diff --git a/modules/light-clients/07-tendermint/types/misbehaviour_handle.go b/modules/light-clients/07-tendermint/types/misbehaviour_handle.go index 54d53c69b1b..932829aa56b 100644 --- a/modules/light-clients/07-tendermint/types/misbehaviour_handle.go +++ b/modules/light-clients/07-tendermint/types/misbehaviour_handle.go @@ -32,39 +32,81 @@ func (cs ClientState) CheckMisbehaviourAndUpdateState( return nil, sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "expected type %T, got %T", misbehaviour, &Misbehaviour{}) } - // The status of the client is checked in 02-client + if err := cs.VerifyClientMessage(ctx, clientStore, cdc, tmMisbehaviour); err != nil { + return nil, err + } + + cs.FrozenHeight = FrozenHeight + + return &cs, nil +} + +// verifyMisbehaviour determines whether or not two conflicting +// headers at the same height would have convinced the light client. +// +// NOTE: consensusState1 is the trusted consensus state that corresponds to the TrustedHeight +// of misbehaviour.Header1 +// Similarly, consensusState2 is the trusted consensus state that corresponds +// to misbehaviour.Header2 +// Misbehaviour sets frozen height to {0, 1} since it is only used as a boolean value (zero or non-zero). +func (cs *ClientState) verifyMisbehaviour(ctx sdk.Context, clientStore sdk.KVStore, cdc codec.BinaryCodec, misbehaviour *Misbehaviour) error { // if heights are equal check that this is valid misbehaviour of a fork // otherwise if heights are unequal check that this is valid misbehavior of BFT time violation - if tmMisbehaviour.Header1.GetHeight().EQ(tmMisbehaviour.Header2.GetHeight()) { - blockID1, err := tmtypes.BlockIDFromProto(&tmMisbehaviour.Header1.SignedHeader.Commit.BlockID) + if misbehaviour.Header1.GetHeight().EQ(misbehaviour.Header2.GetHeight()) { + blockID1, err := tmtypes.BlockIDFromProto(&misbehaviour.Header1.SignedHeader.Commit.BlockID) if err != nil { - return nil, sdkerrors.Wrap(err, "invalid block ID from header 1 in misbehaviour") + return sdkerrors.Wrap(err, "invalid block ID from header 1 in misbehaviour") } - blockID2, err := tmtypes.BlockIDFromProto(&tmMisbehaviour.Header2.SignedHeader.Commit.BlockID) + + blockID2, err := tmtypes.BlockIDFromProto(&misbehaviour.Header2.SignedHeader.Commit.BlockID) if err != nil { - return nil, sdkerrors.Wrap(err, "invalid block ID from header 2 in misbehaviour") + return sdkerrors.Wrap(err, "invalid block ID from header 2 in misbehaviour") } // Ensure that Commit Hashes are different if bytes.Equal(blockID1.Hash, blockID2.Hash) { - return nil, sdkerrors.Wrap(clienttypes.ErrInvalidMisbehaviour, "headers block hashes are equal") + return sdkerrors.Wrap(clienttypes.ErrInvalidMisbehaviour, "headers block hashes are equal") } + } else { // Header1 is at greater height than Header2, therefore Header1 time must be less than or equal to // Header2 time in order to be valid misbehaviour (violation of monotonic time). - if tmMisbehaviour.Header1.SignedHeader.Header.Time.After(tmMisbehaviour.Header2.SignedHeader.Header.Time) { - return nil, sdkerrors.Wrap(clienttypes.ErrInvalidMisbehaviour, "headers are not at same height and are monotonically increasing") + if misbehaviour.Header1.SignedHeader.Header.Time.After(misbehaviour.Header2.SignedHeader.Header.Time) { + return sdkerrors.Wrap(clienttypes.ErrInvalidMisbehaviour, "headers are not at same height and are monotonically increasing") } } - if err := cs.VerifyClientMessage(ctx, clientStore, cdc, misbehaviour); err != nil { - return nil, err + // Regardless of the type of misbehaviour, ensure that both headers are valid and would have been accepted by light-client + + // Retrieve trusted consensus states for each Header in misbehaviour + tmConsensusState1, err := GetConsensusState(clientStore, cdc, misbehaviour.Header1.TrustedHeight) + if err != nil { + return sdkerrors.Wrapf(err, "could not get trusted consensus state from clientStore for Header1 at TrustedHeight: %s", misbehaviour.Header1.TrustedHeight) } - cs.FrozenHeight = FrozenHeight + tmConsensusState2, err := GetConsensusState(clientStore, cdc, misbehaviour.Header2.TrustedHeight) + if err != nil { + return sdkerrors.Wrapf(err, "could not get trusted consensus state from clientStore for Header2 at TrustedHeight: %s", misbehaviour.Header2.TrustedHeight) + } - return &cs, nil + // Check the validity of the two conflicting headers against their respective + // trusted consensus states + // NOTE: header height and commitment root assertions are checked in + // misbehaviour.ValidateBasic by the client keeper and msg.ValidateBasic + // by the base application. + if err := checkMisbehaviourHeader( + cs, tmConsensusState1, misbehaviour.Header1, ctx.BlockTime(), + ); err != nil { + return sdkerrors.Wrap(err, "verifying Header1 in Misbehaviour failed") + } + if err := checkMisbehaviourHeader( + cs, tmConsensusState2, misbehaviour.Header2, ctx.BlockTime(), + ); err != nil { + return sdkerrors.Wrap(err, "verifying Header2 in Misbehaviour failed") + } + + return nil } // checkMisbehaviourHeader checks that a Header in Misbehaviour is valid misbehaviour given @@ -72,7 +114,6 @@ func (cs ClientState) CheckMisbehaviourAndUpdateState( func checkMisbehaviourHeader( clientState *ClientState, consState *ConsensusState, header *Header, currentTimestamp time.Time, ) error { - tmTrustedValset, err := tmtypes.ValidatorSetFromProto(header.TrustedValidators) if err != nil { return sdkerrors.Wrap(err, "trusted validator set is not tendermint validator set type") diff --git a/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go b/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go index ef92046b9a8..4657bae4d34 100644 --- a/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go +++ b/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go @@ -10,7 +10,9 @@ import ( clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" commitmenttypes "github.com/cosmos/ibc-go/v3/modules/core/23-commitment/types" "github.com/cosmos/ibc-go/v3/modules/core/exported" + smtypes "github.com/cosmos/ibc-go/v3/modules/light-clients/06-solomachine/types" "github.com/cosmos/ibc-go/v3/modules/light-clients/07-tendermint/types" + ibctesting "github.com/cosmos/ibc-go/v3/testing" ibctestingmock "github.com/cosmos/ibc-go/v3/testing/mock" ) @@ -423,3 +425,371 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { }) } } + +func (suite *TendermintTestSuite) TestVerifyMisbehaviour() { + // Setup different validators and signers for testing different types of updates + altPrivVal := ibctestingmock.NewPV() + altPubKey, err := altPrivVal.GetPubKey() + suite.Require().NoError(err) + + // create modified heights to use for test-cases + altVal := tmtypes.NewValidator(altPubKey, 100) + + // Create alternative validator set with only altVal, invalid update (too much change in valSet) + altValSet := tmtypes.NewValidatorSet([]*tmtypes.Validator{altVal}) + altSigners := getAltSigners(altVal, altPrivVal) + + var ( + path *ibctesting.Path + misbehaviour exported.ClientMessage + ) + + testCases := []struct { + name string + malleate func() + expPass bool + }{ + { + "valid fork misbehaviour", func() { + trustedHeight := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + trustedVals, found := suite.chainB.GetValsAtHeight(int64(trustedHeight.RevisionHeight) + 1) + suite.Require().True(found) + + err := path.EndpointA.UpdateClient() + suite.Require().NoError(err) + + height := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + misbehaviour = &types.Misbehaviour{ + Header1: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, int64(height.RevisionHeight), trustedHeight, suite.chainB.CurrentHeader.Time.Add(time.Minute), suite.chainB.Vals, suite.chainB.NextVals, trustedVals, suite.chainB.Signers), + Header2: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, int64(height.RevisionHeight), trustedHeight, suite.chainB.CurrentHeader.Time, suite.chainB.Vals, suite.chainB.NextVals, trustedVals, suite.chainB.Signers), + } + }, + true, + }, + { + "valid time misbehaviour", func() { + trustedHeight := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + trustedVals, found := suite.chainB.GetValsAtHeight(int64(trustedHeight.RevisionHeight) + 1) + suite.Require().True(found) + + misbehaviour = &types.Misbehaviour{ + Header1: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, suite.chainB.CurrentHeader.Height+3, trustedHeight, suite.chainB.CurrentHeader.Time, suite.chainB.Vals, suite.chainB.NextVals, trustedVals, suite.chainB.Signers), + Header2: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, suite.chainB.CurrentHeader.Height, trustedHeight, suite.chainB.CurrentHeader.Time, suite.chainB.Vals, suite.chainB.NextVals, trustedVals, suite.chainB.Signers), + } + }, + true, + }, + { + "valid time misbehaviour, header 1 time stricly less than header 2 time", func() { + trustedHeight := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + trustedVals, found := suite.chainB.GetValsAtHeight(int64(trustedHeight.RevisionHeight) + 1) + suite.Require().True(found) + + misbehaviour = &types.Misbehaviour{ + Header1: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, suite.chainB.CurrentHeader.Height+3, trustedHeight, suite.chainB.CurrentHeader.Time, suite.chainB.Vals, suite.chainB.NextVals, trustedVals, suite.chainB.Signers), + Header2: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, suite.chainB.CurrentHeader.Height, trustedHeight, suite.chainB.CurrentHeader.Time.Add(time.Hour), suite.chainB.Vals, suite.chainB.NextVals, trustedVals, suite.chainB.Signers), + } + }, + true, + }, + { + "valid misbehavior at height greater than last consensusState", func() { + trustedHeight := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + trustedVals, found := suite.chainB.GetValsAtHeight(int64(trustedHeight.RevisionHeight) + 1) + suite.Require().True(found) + + misbehaviour = &types.Misbehaviour{ + Header1: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, suite.chainB.CurrentHeader.Height+1, trustedHeight, suite.chainB.CurrentHeader.Time, suite.chainB.Vals, suite.chainB.NextVals, trustedVals, suite.chainB.Signers), + Header2: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, suite.chainB.CurrentHeader.Height+1, trustedHeight, suite.chainB.CurrentHeader.Time.Add(time.Minute), suite.chainB.Vals, suite.chainB.NextVals, trustedVals, suite.chainB.Signers), + } + }, true, + }, + { + "valid misbehaviour with different trusted heights", func() { + trustedHeight1 := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + trustedVals1, found := suite.chainB.GetValsAtHeight(int64(trustedHeight1.RevisionHeight) + 1) + suite.Require().True(found) + + err := path.EndpointA.UpdateClient() + suite.Require().NoError(err) + + trustedHeight2 := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + trustedVals2, found := suite.chainB.GetValsAtHeight(int64(trustedHeight2.RevisionHeight) + 1) + suite.Require().True(found) + + misbehaviour = &types.Misbehaviour{ + Header1: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, suite.chainB.CurrentHeader.Height, trustedHeight1, suite.chainB.CurrentHeader.Time.Add(time.Minute), suite.chainB.Vals, suite.chainB.NextVals, trustedVals1, suite.chainB.Signers), + Header2: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, suite.chainB.CurrentHeader.Height, trustedHeight2, suite.chainB.CurrentHeader.Time, suite.chainB.Vals, suite.chainB.NextVals, trustedVals2, suite.chainB.Signers), + } + + }, + true, + }, + /* + { + + "valid misbehaviour at a previous revision", + types.NewClientState(chainIDRevision1, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, clienttypes.NewHeight(1, 1), commitmenttypes.GetSDKSpecs(), upgradePath, false, false), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + heightMinus1, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), suite.valsHash), + heightMinus3, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainIDRevision0, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainIDRevision0, int64(height.RevisionHeight+1), heightMinus3, suite.now.Add(time.Minute), bothValSet, bothValSet, suite.valSet, bothSigners), + ClientId: chainID, + }, + suite.now, + true, + }, + { + "valid misbehaviour at a future revision", + types.NewClientState(chainIDRevision0, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + heightMinus1, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), suite.valsHash), + heightMinus3, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainIDRevision0, 3, heightMinus1, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainIDRevision0, 3, heightMinus3, suite.now.Add(time.Minute), bothValSet, bothValSet, suite.valSet, bothSigners), + ClientId: chainID, + }, + suite.now, + true, + }, + { + "valid misbehaviour with trusted heights at a previous revision", + types.NewClientState(chainIDRevision1, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, clienttypes.NewHeight(1, 1), commitmenttypes.GetSDKSpecs(), upgradePath, false, false), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + heightMinus1, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), suite.valsHash), + heightMinus3, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainIDRevision1, 1, heightMinus1, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainIDRevision1, 1, heightMinus3, suite.now.Add(time.Minute), bothValSet, bothValSet, suite.valSet, bothSigners), + ClientId: chainID, + }, + suite.now, + true, + }, + */ + { + "consensus state's valset hash different from misbehaviour should still pass", func() { + trustedHeight := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + trustedVals, found := suite.chainB.GetValsAtHeight(int64(trustedHeight.RevisionHeight) + 1) + suite.Require().True(found) + + err := path.EndpointA.UpdateClient() + suite.Require().NoError(err) + + height := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + // Create bothValSet with both suite validator and altVal + bothValSet := tmtypes.NewValidatorSet(append(suite.chainB.Vals.Validators, altValSet.Proposer)) + bothSigners := suite.chainB.Signers + bothSigners[altValSet.Proposer.Address.String()] = altPrivVal + + misbehaviour = &types.Misbehaviour{ + Header1: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, int64(height.RevisionHeight), trustedHeight, suite.chainB.CurrentHeader.Time.Add(time.Minute), bothValSet, suite.chainB.NextVals, trustedVals, bothSigners), + Header2: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, int64(height.RevisionHeight), trustedHeight, suite.chainB.CurrentHeader.Time, bothValSet, suite.chainB.NextVals, trustedVals, bothSigners), + } + }, true, + }, + { + "invalid fork misbehaviour: identical headers", func() { + trustedHeight := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + trustedVals, found := suite.chainB.GetValsAtHeight(int64(trustedHeight.RevisionHeight) + 1) + suite.Require().True(found) + + err := path.EndpointA.UpdateClient() + suite.Require().NoError(err) + + height := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + misbehaviourHeader := suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, int64(height.RevisionHeight), trustedHeight, suite.chainB.CurrentHeader.Time.Add(time.Minute), suite.chainB.Vals, suite.chainB.NextVals, trustedVals, suite.chainB.Signers) + misbehaviour = &types.Misbehaviour{ + Header1: misbehaviourHeader, + Header2: misbehaviourHeader, + } + }, false, + }, + { + "invalid time misbehaviour: monotonically increasing time", func() { + trustedHeight := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + trustedVals, found := suite.chainB.GetValsAtHeight(int64(trustedHeight.RevisionHeight) + 1) + suite.Require().True(found) + + misbehaviour = &types.Misbehaviour{ + Header1: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, suite.chainB.CurrentHeader.Height+3, trustedHeight, suite.chainB.CurrentHeader.Time.Add(time.Minute), suite.chainB.Vals, suite.chainB.NextVals, trustedVals, suite.chainB.Signers), + Header2: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, suite.chainB.CurrentHeader.Height, trustedHeight, suite.chainB.CurrentHeader.Time, suite.chainB.Vals, suite.chainB.NextVals, trustedVals, suite.chainB.Signers), + } + }, false, + }, + { + "invalid misbehaviour: misbehaviour from different chain", func() { + trustedHeight := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + trustedVals, found := suite.chainB.GetValsAtHeight(int64(trustedHeight.RevisionHeight) + 1) + suite.Require().True(found) + + err := path.EndpointA.UpdateClient() + suite.Require().NoError(err) + + height := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + misbehaviour = &types.Misbehaviour{ + Header1: suite.chainB.CreateTMClientHeader("evmos", int64(height.RevisionHeight), trustedHeight, suite.chainB.CurrentHeader.Time.Add(time.Minute), suite.chainB.Vals, suite.chainB.NextVals, trustedVals, suite.chainB.Signers), + Header2: suite.chainB.CreateTMClientHeader("evmos", int64(height.RevisionHeight), trustedHeight, suite.chainB.CurrentHeader.Time, suite.chainB.Vals, suite.chainB.NextVals, trustedVals, suite.chainB.Signers), + } + + }, false, + }, + { + "misbehaviour trusted validators does not match validator hash in trusted consensus state", func() { + trustedHeight := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + err := path.EndpointA.UpdateClient() + suite.Require().NoError(err) + + height := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + misbehaviour = &types.Misbehaviour{ + Header1: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, int64(height.RevisionHeight), trustedHeight, suite.chainB.CurrentHeader.Time.Add(time.Minute), suite.chainB.Vals, suite.chainB.NextVals, altValSet, suite.chainB.Signers), + Header2: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, int64(height.RevisionHeight), trustedHeight, suite.chainB.CurrentHeader.Time, suite.chainB.Vals, suite.chainB.NextVals, altValSet, suite.chainB.Signers), + } + }, false, + }, + { + "trusted consensus state does not exist", func() { + trustedHeight := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + trustedVals, found := suite.chainB.GetValsAtHeight(int64(trustedHeight.RevisionHeight) + 1) + suite.Require().True(found) + + misbehaviour = &types.Misbehaviour{ + Header1: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, suite.chainB.CurrentHeader.Height, trustedHeight.Increment().(clienttypes.Height), suite.chainB.CurrentHeader.Time.Add(time.Minute), suite.chainB.Vals, suite.chainB.NextVals, trustedVals, suite.chainB.Signers), + Header2: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, suite.chainB.CurrentHeader.Height, trustedHeight, suite.chainB.CurrentHeader.Time, suite.chainB.Vals, suite.chainB.NextVals, trustedVals, suite.chainB.Signers), + } + }, false, + }, + { + "invalid tendermint misbehaviour", func() { + misbehaviour = &smtypes.Misbehaviour{} + }, false, + }, + { + "trusting period expired", func() { + trustedHeight := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + trustedVals, found := suite.chainB.GetValsAtHeight(int64(trustedHeight.RevisionHeight) + 1) + suite.Require().True(found) + + err := path.EndpointA.UpdateClient() + suite.Require().NoError(err) + + height := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + suite.chainA.ExpireClient(path.EndpointA.ClientConfig.(*ibctesting.TendermintConfig).TrustingPeriod) + + misbehaviour = &types.Misbehaviour{ + Header1: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, int64(height.RevisionHeight), trustedHeight, suite.chainB.CurrentHeader.Time.Add(time.Minute), suite.chainB.Vals, suite.chainB.NextVals, trustedVals, suite.chainB.Signers), + Header2: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, int64(height.RevisionHeight), trustedHeight, suite.chainB.CurrentHeader.Time, suite.chainB.Vals, suite.chainB.NextVals, trustedVals, suite.chainB.Signers), + } + }, false, + }, + { + "header 1 valset has too much change", func() { + trustedHeight := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + trustedVals, found := suite.chainB.GetValsAtHeight(int64(trustedHeight.RevisionHeight) + 1) + suite.Require().True(found) + + err := path.EndpointA.UpdateClient() + suite.Require().NoError(err) + + height := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + misbehaviour = &types.Misbehaviour{ + Header1: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, int64(height.RevisionHeight), trustedHeight, suite.chainB.CurrentHeader.Time.Add(time.Minute), altValSet, suite.chainB.NextVals, trustedVals, altSigners), + Header2: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, int64(height.RevisionHeight), trustedHeight, suite.chainB.CurrentHeader.Time, suite.chainB.Vals, suite.chainB.NextVals, trustedVals, suite.chainB.Signers), + } + }, false, + }, + { + "header 2 valset has too much change", func() { + trustedHeight := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + trustedVals, found := suite.chainB.GetValsAtHeight(int64(trustedHeight.RevisionHeight) + 1) + suite.Require().True(found) + + err := path.EndpointA.UpdateClient() + suite.Require().NoError(err) + + height := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + misbehaviour = &types.Misbehaviour{ + Header1: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, int64(height.RevisionHeight), trustedHeight, suite.chainB.CurrentHeader.Time.Add(time.Minute), suite.chainB.Vals, suite.chainB.NextVals, trustedVals, suite.chainB.Signers), + Header2: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, int64(height.RevisionHeight), trustedHeight, suite.chainB.CurrentHeader.Time, altValSet, suite.chainB.NextVals, trustedVals, altSigners), + } + }, false, + }, + { + "both header 1 and header 2 valsets have too much change", func() { + trustedHeight := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + trustedVals, found := suite.chainB.GetValsAtHeight(int64(trustedHeight.RevisionHeight) + 1) + suite.Require().True(found) + + err := path.EndpointA.UpdateClient() + suite.Require().NoError(err) + + height := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + misbehaviour = &types.Misbehaviour{ + Header1: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, int64(height.RevisionHeight), trustedHeight, suite.chainB.CurrentHeader.Time.Add(time.Minute), altValSet, suite.chainB.NextVals, trustedVals, altSigners), + Header2: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, int64(height.RevisionHeight), trustedHeight, suite.chainB.CurrentHeader.Time, altValSet, suite.chainB.NextVals, trustedVals, altSigners), + } + }, false, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + suite.SetupTest() + path = ibctesting.NewPath(suite.chainA, suite.chainB) + + err := path.EndpointA.CreateClient() + suite.Require().NoError(err) + + clientState := path.EndpointA.GetClientState() + + // TODO: remove casting when `VerifyClientMessage` is apart of ClientState interface + tmClientState, ok := clientState.(*types.ClientState) + suite.Require().True(ok) + + tc.malleate() + + clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID) + + err = tmClientState.VerifyClientMessage(suite.chainA.GetContext(), clientStore, suite.chainA.App.AppCodec(), misbehaviour) + + if tc.expPass { + suite.Require().NoError(err) + } else { + suite.Require().Error(err) + } + }) + } +} diff --git a/modules/light-clients/07-tendermint/types/update.go b/modules/light-clients/07-tendermint/types/update.go index a065f1cf7ba..0862447a4b3 100644 --- a/modules/light-clients/07-tendermint/types/update.go +++ b/modules/light-clients/07-tendermint/types/update.go @@ -126,40 +126,10 @@ func (cs *ClientState) VerifyClientMessage( case *Header: return cs.verifyHeader(ctx, clientStore, cdc, msg) case *Misbehaviour: - // Regardless of the type of misbehaviour, ensure that both headers are valid and would have been accepted by light-client - // Retrieve trusted consensus states for each Header in misbehaviour - // and unmarshal from clientStore - - // Get consensus bytes from clientStore - tmConsensusState1, err := GetConsensusState(clientStore, cdc, msg.Header1.TrustedHeight) - if err != nil { - return sdkerrors.Wrapf(err, "could not get trusted consensus state from clientStore for Header1 at TrustedHeight: %s", msg.Header1) - } - - // Get consensus bytes from clientStore - tmConsensusState2, err := GetConsensusState(clientStore, cdc, msg.Header2.TrustedHeight) - if err != nil { - return sdkerrors.Wrapf(err, "could not get trusted consensus state from clientStore for Header2 at TrustedHeight: %s", msg.Header2) - } - - // Check the validity of the two conflicting headers against their respective - // trusted consensus states - // NOTE: header height and commitment root assertions are checked in - // misbehaviour.ValidateBasic by the client keeper and msg.ValidateBasic - // by the base application. - if err := checkMisbehaviourHeader( - cs, tmConsensusState1, msg.Header1, ctx.BlockTime(), - ); err != nil { - return sdkerrors.Wrap(err, "verifying Header1 in Misbehaviour failed") - } - if err := checkMisbehaviourHeader( - cs, tmConsensusState2, msg.Header2, ctx.BlockTime(), - ); err != nil { - return sdkerrors.Wrap(err, "verifying Header2 in Misbehaviour failed") - } + return cs.verifyMisbehaviour(ctx, clientStore, cdc, msg) + default: + return clienttypes.ErrInvalidClientType } - - return nil } // verifyHeader returns an error if: