From a7a1fe6ac59dec1c68cf9eb45fd4e14164ad8f03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 23 Mar 2022 13:03:49 +0100 Subject: [PATCH 1/9] refactor: move misbehaviour validation into verifyMisbehaviour function --- .../types/misbehaviour_handle.go | 69 +++++++++++++++---- .../07-tendermint/types/update.go | 40 ++--------- 2 files changed, 59 insertions(+), 50 deletions(-) diff --git a/modules/light-clients/07-tendermint/types/misbehaviour_handle.go b/modules/light-clients/07-tendermint/types/misbehaviour_handle.go index b427b905926..087795a3651 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, nil, misbehaviour, ctx.BlockTime()); 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, nil, misbehaviour, ctx.BlockTime()); 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/update.go b/modules/light-clients/07-tendermint/types/update.go index 79af77f3f8a..c4c89f3d7cc 100644 --- a/modules/light-clients/07-tendermint/types/update.go +++ b/modules/light-clients/07-tendermint/types/update.go @@ -165,45 +165,13 @@ func checkTrustedHeader(header *Header, consState *ConsensusState) error { // VerifyClientMessage checks if the clientMessage is of type Header or Misbehaviour and validates the message func (cs *ClientState) VerifyClientMessage( ctx sdk.Context, clientStore sdk.KVStore, cdc codec.BinaryCodec, consState *ConsensusState, - header exported.ClientMessage, currentTimestamp time.Time, + clientMsg exported.ClientMessage, currentTimestamp time.Time, ) error { - switch header.(type) { + switch msg := clientMsg.(type) { case *Header: - return verifyHeader(ctx, cs, clientStore, cdc, consState, header, currentTimestamp) + return verifyHeader(ctx, cs, clientStore, cdc, consState, msg, currentTimestamp) case *Misbehaviour: - misbehaviour := header.(*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, 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) - } - - // Get consensus bytes from clientStore - 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) - } - - // 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 cs.verifyMisbehaviour(ctx, clientStore, cdc, msg) } return nil From afabaa992aca0974f84808351e555834515cb35c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 23 Mar 2022 14:31:19 +0100 Subject: [PATCH 2/9] begin writing misbehaviour tests --- .../types/misbehaviour_handle.go | 2 +- .../types/misbehaviour_handle_test.go | 388 +++++++++++++++++- .../07-tendermint/types/update.go | 27 +- .../07-tendermint/types/update_test.go | 21 +- 4 files changed, 403 insertions(+), 35 deletions(-) diff --git a/modules/light-clients/07-tendermint/types/misbehaviour_handle.go b/modules/light-clients/07-tendermint/types/misbehaviour_handle.go index 087795a3651..932829aa56b 100644 --- a/modules/light-clients/07-tendermint/types/misbehaviour_handle.go +++ b/modules/light-clients/07-tendermint/types/misbehaviour_handle.go @@ -32,7 +32,7 @@ func (cs ClientState) CheckMisbehaviourAndUpdateState( return nil, sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "expected type %T, got %T", misbehaviour, &Misbehaviour{}) } - if err := cs.VerifyClientMessage(ctx, clientStore, cdc, nil, misbehaviour, ctx.BlockTime()); err != nil { + if err := cs.VerifyClientMessage(ctx, clientStore, cdc, tmMisbehaviour); err != nil { return nil, err } 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 93178f57816..70fd86e29e1 100644 --- a/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go +++ b/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go @@ -11,6 +11,7 @@ import ( commitmenttypes "github.com/cosmos/ibc-go/v3/modules/core/23-commitment/types" "github.com/cosmos/ibc-go/v3/modules/core/exported" "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" ) @@ -40,7 +41,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { height1 clienttypes.Height consensusState2 exported.ConsensusState height2 clienttypes.Height - misbehaviour exported.ClientMessage + misbehaviour exported.ClientMessage timestamp time.Time expPass bool }{ @@ -423,3 +424,388 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { }) } } + +func (suite *TendermintTestSuite) TestVerifyMisbehaviour() { + var ( + path *ibctesting.Path + misbehaviour *types.Misbehaviour + ) + + testCases := []struct { + name string + malleate func() + expPass bool + }{ + { + "valid fork misbehaviour", func() { + trustedHeight := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) + + // TODO: unsure if it should be + 1. The trusted height corresponds to the trusted consensus state + // but the check is consState.NextValsHash == trustedVals.Hash(), so it should be + 1 since + // historical info stores the validator set entering the block? + trustedVals, found := suite.chainA.GetValsAtHeight(int64(trustedHeight.RevisionHeight) + 1) + suite.Require().True(found) + + misbehaviour = &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(suite.chainA.ChainID, suite.chainA.CurrentHeader.Height, trustedHeight, suite.chainA.CurrentHeader.Time.Add(time.Minute), suite.chainA.Vals, suite.chainA.NextVals, trustedVals, suite.chainA.Signers), + Header2: suite.chainA.CreateTMClientHeader(suite.chainA.ChainID, suite.chainA.CurrentHeader.Height, trustedHeight, suite.chainA.CurrentHeader.Time, suite.chainA.Vals, suite.chainA.NextVals, trustedVals, suite.chainA.Signers), + } + }, + true, + }, + /* + { + "valid time misbehaviour", + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+3), height, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), + ClientId: chainID, + }, + suite.now, + true, + }, + { + "valid time misbehaviour header 1 stricly less than header 2", + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+3), height, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Hour), bothValSet, bothValSet, bothValSet, bothSigners), + ClientId: chainID, + }, + suite.now, + true, + }, + { + "valid misbehavior at height greater than last consensusState", + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + heightMinus1, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + heightMinus1, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now.Add(time.Minute), bothValSet, bothValSet, bothValSet, bothSigners), + ClientId: chainID, + }, + suite.now, + true, + }, + { + "valid misbehaviour with different trusted heights", + 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(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus3, suite.now.Add(time.Minute), bothValSet, bothValSet, suite.valSet, bothSigners), + ClientId: chainID, + }, + suite.now, + 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", + types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), suite.valsHash), + height, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), suite.valsHash), + height, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, suite.valSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, suite.valSet, bothSigners), + ClientId: chainID, + }, + suite.now, + true, + }, + { + "invalid fork misbehaviour: identical headers", + types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), + ClientId: chainID, + }, + suite.now, + false, + }, + { + "invalid time misbehaviour: monotonically increasing time", + types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+3), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), + ClientId: chainID, + }, + suite.now, + false, + }, + { + "invalid misbehavior misbehaviour from different chain", + types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader("ethermint", int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader("ethermint", int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothValSet, bothSigners), + ClientId: chainID, + }, + suite.now, + false, + }, + { + "invalid misbehavior misbehaviour with trusted height different from trusted consensus state", + types.NewClientState(chainID, 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(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, suite.valSet, bothSigners), + ClientId: chainID, + }, + suite.now, + false, + }, + { + "invalid misbehavior misbehaviour with trusted validators different from trusted consensus state", + types.NewClientState(chainID, 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(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus3, suite.now.Add(time.Minute), bothValSet, bothValSet, bothValSet, bothSigners), + ClientId: chainID, + }, + suite.now, + false, + }, + { + "already frozen client state", + &types.ClientState{FrozenHeight: clienttypes.NewHeight(0, 1)}, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothValSet, bothSigners), + ClientId: chainID, + }, + suite.now, + false, + }, + { + "trusted consensus state does not exist", + types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), + nil, // consensus state for trusted height - 1 does not exist in store + clienttypes.Height{}, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothValSet, bothSigners), + ClientId: chainID, + }, + suite.now, + false, + }, + { + "invalid tendermint misbehaviour", + types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + nil, + suite.now, + false, + }, + { + "provided height > header height", + types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now.Add(time.Minute), bothValSet, bothValSet, bothValSet, bothSigners), + ClientId: chainID, + }, + suite.now, + false, + }, + { + "trusting period expired", + types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), + types.NewConsensusState(time.Time{}, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + heightMinus1, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothValSet, bothSigners), + ClientId: chainID, + }, + suite.now.Add(trustingPeriod), + false, + }, + { + "trusted validators is incorrect for given consensus state", + types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, suite.valSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, suite.valSet, bothSigners), + ClientId: chainID, + }, + suite.now, + false, + }, + { + "first valset has too much change", + types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, altValSet, altValSet, bothValSet, altSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothValSet, bothSigners), + ClientId: chainID, + }, + suite.now, + false, + }, + { + "second valset has too much change", + types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), altValSet, altValSet, bothValSet, altSigners), + ClientId: chainID, + }, + suite.now, + false, + }, + { + "both valsets have too much change", + types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, altValSet, altValSet, bothValSet, altSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), altValSet, altValSet, bothValSet, altSigners), + ClientId: chainID, + }, + suite.now, + 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 c4c89f3d7cc..9f9e857c56a 100644 --- a/modules/light-clients/07-tendermint/types/update.go +++ b/modules/light-clients/07-tendermint/types/update.go @@ -77,14 +77,7 @@ func (cs ClientState) CheckHeaderAndUpdateState( } // get consensus state from clientStore - trustedConsState, err := GetConsensusState(clientStore, cdc, tmHeader.TrustedHeight) - if err != nil { - return nil, nil, sdkerrors.Wrapf( - err, "could not get consensus state from clientstore at TrustedHeight: %s", tmHeader.TrustedHeight, - ) - } - - if err := cs.VerifyClientMessage(ctx, clientStore, cdc, trustedConsState, tmHeader, ctx.BlockTime()); err != nil { + if err := cs.VerifyClientMessage(ctx, clientStore, cdc, tmHeader); err != nil { return nil, nil, err } @@ -164,12 +157,11 @@ func checkTrustedHeader(header *Header, consState *ConsensusState) error { // VerifyClientMessage checks if the clientMessage is of type Header or Misbehaviour and validates the message func (cs *ClientState) VerifyClientMessage( - ctx sdk.Context, clientStore sdk.KVStore, cdc codec.BinaryCodec, consState *ConsensusState, - clientMsg exported.ClientMessage, currentTimestamp time.Time, + ctx sdk.Context, clientStore sdk.KVStore, cdc codec.BinaryCodec, clientMsg exported.ClientMessage, ) error { switch msg := clientMsg.(type) { case *Header: - return verifyHeader(ctx, cs, clientStore, cdc, consState, msg, currentTimestamp) + return verifyHeader(ctx, cs, clientStore, cdc, msg, ctx.BlockTime()) case *Misbehaviour: return cs.verifyMisbehaviour(ctx, clientStore, cdc, msg) } @@ -180,13 +172,18 @@ func (cs *ClientState) VerifyClientMessage( //TODO: comment // verifyHeader func verifyHeader( - ctx sdk.Context, clientState *ClientState, clientStore sdk.KVStore, cdc codec.BinaryCodec, consState *ConsensusState, + ctx sdk.Context, clientState *ClientState, clientStore sdk.KVStore, cdc codec.BinaryCodec, tmHeader exported.ClientMessage, currentTimestamp time.Time, ) error { // TODO: check ok header := tmHeader.(*Header) - if err := checkTrustedHeader(header, consState); err != nil { + trustedConsState, err := GetConsensusState(clientStore, cdc, header.TrustedHeight) + if err != nil { + return sdkerrors.Wrapf(err, "could not get consensus state from clientstore at TrustedHeight: %s", header.TrustedHeight) + } + + if err := checkTrustedHeader(header, trustedConsState); err != nil { return err } @@ -239,8 +236,8 @@ func verifyHeader( trustedHeader := tmtypes.Header{ ChainID: chainID, Height: int64(header.TrustedHeight.RevisionHeight), - Time: consState.Timestamp, - NextValidatorsHash: consState.NextValidatorsHash, + Time: trustedConsState.Timestamp, + NextValidatorsHash: trustedConsState.NextValidatorsHash, } signedHeader := tmtypes.SignedHeader{ Header: &trustedHeader, diff --git a/modules/light-clients/07-tendermint/types/update_test.go b/modules/light-clients/07-tendermint/types/update_test.go index d40546ce47d..a4cf53057ca 100644 --- a/modules/light-clients/07-tendermint/types/update_test.go +++ b/modules/light-clients/07-tendermint/types/update_test.go @@ -4,7 +4,6 @@ import ( "fmt" "time" - sdk "github.com/cosmos/cosmos-sdk/types" 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" @@ -13,12 +12,6 @@ import ( ) func (suite *TendermintTestSuite) TestVerifyHeader() { - var ( - tmConsState *types.ConsensusState - clientStore sdk.KVStore - blockTime time.Time - ) - testCases := []struct { name string malleate func() @@ -49,19 +42,11 @@ func (suite *TendermintTestSuite) TestVerifyHeader() { tmClientState, ok := clientState.(*types.ClientState) suite.Require().True(ok) - consState, ok := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientConsensusState(suite.chainA.GetContext(), path.EndpointA.ClientID, header.TrustedHeight) - suite.Require().True(ok) - - tmConsState, ok = consState.(*types.ConsensusState) - suite.Require().True(ok) - - clientStore = suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID) - - blockTime = suite.chainA.GetContext().BlockTime() + clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID) tc.malleate() - err = tmClientState.VerifyClientMessage(suite.chainA.GetContext(), clientStore, suite.chainA.App.AppCodec(), tmConsState, header, blockTime) + err = tmClientState.VerifyClientMessage(suite.chainA.GetContext(), clientStore, suite.chainA.App.AppCodec(), header) if tc.expPass { suite.Require().NoError(err) @@ -90,7 +75,7 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() { // revisionHeight := int64(height.RevisionHeight) // create modified heights to use for test-cases - heightPlus1 := clienttypes.NewHeight(height.RevisionNumber, height.RevisionHeight+1) + //heightPlus1 := clienttypes.NewHeight(height.RevisionNumber, height.RevisionHeight+1) //heightPlus5 := clienttypes.NewHeight(height.RevisionNumber, height.RevisionHeight+5) //heightMinus1 := clienttypes.NewHeight(height.RevisionNumber, height.RevisionHeight-1) //heightMinus3 := clienttypes.NewHeight(height.RevisionNumber, height.RevisionHeight-3) From 8d442976f12b519341f5a35e9c605f3920acbb64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 23 Mar 2022 14:46:16 +0100 Subject: [PATCH 3/9] fix misbehaviour test --- .../07-tendermint/types/misbehaviour_handle_test.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) 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 70fd86e29e1..ec5f3f82985 100644 --- a/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go +++ b/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go @@ -440,15 +440,12 @@ func (suite *TendermintTestSuite) TestVerifyMisbehaviour() { "valid fork misbehaviour", func() { trustedHeight := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) - // TODO: unsure if it should be + 1. The trusted height corresponds to the trusted consensus state - // but the check is consState.NextValsHash == trustedVals.Hash(), so it should be + 1 since - // historical info stores the validator set entering the block? - trustedVals, found := suite.chainA.GetValsAtHeight(int64(trustedHeight.RevisionHeight) + 1) + trustedVals, found := suite.chainB.GetValsAtHeight(int64(trustedHeight.RevisionHeight) + 1) suite.Require().True(found) misbehaviour = &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(suite.chainA.ChainID, suite.chainA.CurrentHeader.Height, trustedHeight, suite.chainA.CurrentHeader.Time.Add(time.Minute), suite.chainA.Vals, suite.chainA.NextVals, trustedVals, suite.chainA.Signers), - Header2: suite.chainA.CreateTMClientHeader(suite.chainA.ChainID, suite.chainA.CurrentHeader.Height, trustedHeight, suite.chainA.CurrentHeader.Time, suite.chainA.Vals, suite.chainA.NextVals, trustedVals, suite.chainA.Signers), + Header1: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, suite.chainB.CurrentHeader.Height, 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), } }, true, From 929bb4cfb3c77a3fc8d70e90d5b3924b42a3c975 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 23 Mar 2022 15:24:13 +0100 Subject: [PATCH 4/9] continue adding misbehaviour test cases --- .../types/misbehaviour_handle_test.go | 206 +++++++++--------- .../07-tendermint/types/update.go | 4 +- 2 files changed, 110 insertions(+), 100 deletions(-) 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 ec5f3f82985..2da309c6be4 100644 --- a/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go +++ b/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go @@ -10,6 +10,7 @@ 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" @@ -428,7 +429,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { func (suite *TendermintTestSuite) TestVerifyMisbehaviour() { var ( path *ibctesting.Path - misbehaviour *types.Misbehaviour + misbehaviour exported.ClientMessage ) testCases := []struct { @@ -443,71 +444,85 @@ func (suite *TendermintTestSuite) TestVerifyMisbehaviour() { 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, suite.chainB.CurrentHeader.Height, 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), + 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", - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+3), height, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), - ClientId: chainID, - }, - suite.now, - true, - }, - { - "valid time misbehaviour header 1 stricly less than header 2", - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+3), height, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Hour), bothValSet, bothValSet, bothValSet, bothSigners), - ClientId: chainID, - }, - suite.now, - 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), + } }, - { - "valid misbehavior at height greater than last consensusState", - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - heightMinus1, - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - heightMinus1, - &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now.Add(time.Minute), bothValSet, bothValSet, bothValSet, bothSigners), - ClientId: chainID, - }, - suite.now, - true, + 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), + } }, - { - "valid misbehaviour with different trusted heights", - 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(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus3, suite.now.Add(time.Minute), bothValSet, bothValSet, suite.valSet, bothSigners), - ClientId: chainID, - }, - suite.now, - true, + 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), @@ -582,21 +597,22 @@ func (suite *TendermintTestSuite) TestVerifyMisbehaviour() { suite.now, false, }, - { - "invalid time misbehaviour: monotonically increasing time", - types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+3), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), - ClientId: chainID, - }, - suite.now, - 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 misbehavior misbehaviour from different chain", types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), @@ -657,32 +673,26 @@ func (suite *TendermintTestSuite) TestVerifyMisbehaviour() { suite.now, false, }, - { - "trusted consensus state does not exist", - types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), - nil, // consensus state for trusted height - 1 does not exist in store - clienttypes.Height{}, - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothValSet, bothSigners), - ClientId: chainID, - }, - suite.now, - false, - }, - { - "invalid tendermint misbehaviour", - types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - nil, - suite.now, - 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, + }, + /* { "provided height > header height", types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), diff --git a/modules/light-clients/07-tendermint/types/update.go b/modules/light-clients/07-tendermint/types/update.go index 9f9e857c56a..fa78269c8b3 100644 --- a/modules/light-clients/07-tendermint/types/update.go +++ b/modules/light-clients/07-tendermint/types/update.go @@ -164,9 +164,9 @@ func (cs *ClientState) VerifyClientMessage( return verifyHeader(ctx, cs, clientStore, cdc, msg, ctx.BlockTime()) case *Misbehaviour: return cs.verifyMisbehaviour(ctx, clientStore, cdc, msg) + default: + return clienttypes.ErrInvalidClientType } - - return nil } //TODO: comment From 295dc2496d644331d532a9166ffc78480d22954c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 24 Mar 2022 12:57:31 +0100 Subject: [PATCH 5/9] add more test cases to verifyMisbehaviour test --- .../types/misbehaviour_handle_test.go | 99 +++++++------------ 1 file changed, 38 insertions(+), 61 deletions(-) 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 2da309c6be4..3e11b8708af 100644 --- a/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go +++ b/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go @@ -582,23 +582,26 @@ func (suite *TendermintTestSuite) TestVerifyMisbehaviour() { suite.now, true, }, - { - "invalid fork misbehaviour: identical headers", - types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), - ClientId: chainID, - }, - suite.now, - false, - }, */ + { + "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) @@ -612,39 +615,28 @@ func (suite *TendermintTestSuite) TestVerifyMisbehaviour() { } }, false, }, + { + "invalid misbehavior 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, + }, /* { - "invalid misbehavior misbehaviour from different chain", - types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader("ethermint", int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader("ethermint", int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothValSet, bothSigners), - ClientId: chainID, - }, - suite.now, - false, - }, - { - "invalid misbehavior misbehaviour with trusted height different from trusted consensus state", - types.NewClientState(chainID, 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(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, suite.valSet, bothSigners), - ClientId: chainID, - }, - suite.now, - false, - }, - { - "invalid misbehavior misbehaviour with trusted validators different from trusted consensus state", + "misbehaviour trusted validators does not match validator hash in trusted consensus state", types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), heightMinus1, @@ -658,21 +650,6 @@ func (suite *TendermintTestSuite) TestVerifyMisbehaviour() { suite.now, false, }, - { - "already frozen client state", - &types.ClientState{FrozenHeight: clienttypes.NewHeight(0, 1)}, - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothValSet, bothSigners), - ClientId: chainID, - }, - suite.now, - false, - }, */ { "trusted consensus state does not exist", func() { From be6d5382f01b3183f2352ab166227b13b0afc873 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 28 Mar 2022 13:00:16 +0200 Subject: [PATCH 6/9] add changing validator set tests --- .../types/misbehaviour_handle_test.go | 99 ++++++++++--------- .../07-tendermint/types/tendermint_test.go | 16 +++ 2 files changed, 70 insertions(+), 45 deletions(-) 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 3e11b8708af..fe3033726d3 100644 --- a/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go +++ b/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go @@ -715,52 +715,61 @@ func (suite *TendermintTestSuite) TestVerifyMisbehaviour() { suite.now, false, }, - { - "first valset has too much change", - types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, altValSet, altValSet, bothValSet, altSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothValSet, bothSigners), - ClientId: chainID, - }, - suite.now, - false, - }, - { - "second valset has too much change", - types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), altValSet, altValSet, bothValSet, altSigners), - ClientId: chainID, - }, - suite.now, - false, - }, - { - "both valsets have too much change", - types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, altValSet, altValSet, bothValSet, altSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), altValSet, altValSet, bothValSet, altSigners), - ClientId: chainID, - }, - suite.now, - false, - }, */ + { + "first 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.altValSet, suite.chainB.NextVals, trustedVals, suite.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, + }, + { + "second 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, suite.altValSet, suite.chainB.NextVals, trustedVals, suite.altSigners), + } + }, false, + }, + { + "both 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), suite.altValSet, suite.chainB.NextVals, trustedVals, suite.altSigners), + Header2: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, int64(height.RevisionHeight), trustedHeight, suite.chainB.CurrentHeader.Time, suite.altValSet, suite.chainB.NextVals, trustedVals, suite.altSigners), + } + }, false, + }, } for _, tc := range testCases { diff --git a/modules/light-clients/07-tendermint/types/tendermint_test.go b/modules/light-clients/07-tendermint/types/tendermint_test.go index 898a48efe72..b40870091b0 100644 --- a/modules/light-clients/07-tendermint/types/tendermint_test.go +++ b/modules/light-clients/07-tendermint/types/tendermint_test.go @@ -43,6 +43,9 @@ type TendermintTestSuite struct { chainA *ibctesting.TestChain chainB *ibctesting.TestChain + altValSet *tmtypes.ValidatorSet + altSigners map[string]tmtypes.PrivValidator + // TODO: deprecate usage in favor of testing package ctx sdk.Context cdc codec.Codec @@ -64,6 +67,19 @@ func (suite *TendermintTestSuite) SetupTest() { suite.coordinator.CommitNBlocks(suite.chainA, 2) suite.coordinator.CommitNBlocks(suite.chainB, 2) + // 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) + suite.altValSet = tmtypes.NewValidatorSet([]*tmtypes.Validator{altVal}) + + suite.altSigners = getAltSigners(altVal, altPrivVal) + // TODO: deprecate usage in favor of testing package checkTx := false app := simapp.Setup(checkTx) From 597f8956d8e22887aaaa444d9e58362f7c6c781a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 28 Mar 2022 13:28:08 +0200 Subject: [PATCH 7/9] finish rest of tests except revision height testing --- .../types/misbehaviour_handle_test.go | 163 +++++++++--------- .../07-tendermint/types/tendermint_test.go | 16 -- 2 files changed, 77 insertions(+), 102 deletions(-) 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 fe3033726d3..4a83b9fd13d 100644 --- a/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go +++ b/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go @@ -427,6 +427,18 @@ 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 @@ -567,22 +579,30 @@ func (suite *TendermintTestSuite) TestVerifyMisbehaviour() { suite.now, true, }, - { - "consensus state's valset hash different from misbehaviour should still pass", - types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), suite.valsHash), - height, - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), suite.valsHash), - height, - &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, suite.valSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, 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) @@ -634,23 +654,21 @@ func (suite *TendermintTestSuite) TestVerifyMisbehaviour() { }, false, }, - /* - { - "misbehaviour trusted validators does not match validator hash in trusted consensus state", - types.NewClientState(chainID, 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(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus3, suite.now.Add(time.Minute), bothValSet, bothValSet, bothValSet, bothSigners), - ClientId: chainID, - }, - suite.now, - 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) @@ -669,55 +687,28 @@ func (suite *TendermintTestSuite) TestVerifyMisbehaviour() { misbehaviour = &smtypes.Misbehaviour{} }, false, }, - /* - { - "provided height > header height", - types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now.Add(time.Minute), bothValSet, bothValSet, bothValSet, bothSigners), - ClientId: chainID, - }, - suite.now, - false, - }, - { - "trusting period expired", - types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), - types.NewConsensusState(time.Time{}, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - heightMinus1, - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothValSet, bothSigners), - ClientId: chainID, - }, - suite.now.Add(trustingPeriod), - false, - }, - { - "trusted validators is incorrect for given consensus state", - types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), - height, - &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, suite.valSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, suite.valSet, bothSigners), - ClientId: chainID, - }, - suite.now, - false, - }, - */ { - "first valset has too much change", func() { + "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) @@ -729,13 +720,13 @@ func (suite *TendermintTestSuite) TestVerifyMisbehaviour() { 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.altValSet, suite.chainB.NextVals, trustedVals, suite.altSigners), + 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, }, { - "second valset has too much change", func() { + "header 2 valset has too much change", func() { trustedHeight := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) trustedVals, found := suite.chainB.GetValsAtHeight(int64(trustedHeight.RevisionHeight) + 1) @@ -748,12 +739,12 @@ func (suite *TendermintTestSuite) TestVerifyMisbehaviour() { 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.altValSet, suite.chainB.NextVals, trustedVals, suite.altSigners), + Header2: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, int64(height.RevisionHeight), trustedHeight, suite.chainB.CurrentHeader.Time, altValSet, suite.chainB.NextVals, trustedVals, altSigners), } }, false, }, { - "both valsets have too much change", func() { + "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) @@ -765,8 +756,8 @@ func (suite *TendermintTestSuite) TestVerifyMisbehaviour() { 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.altValSet, suite.chainB.NextVals, trustedVals, suite.altSigners), - Header2: suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, int64(height.RevisionHeight), trustedHeight, suite.chainB.CurrentHeader.Time, suite.altValSet, suite.chainB.NextVals, trustedVals, suite.altSigners), + 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, }, diff --git a/modules/light-clients/07-tendermint/types/tendermint_test.go b/modules/light-clients/07-tendermint/types/tendermint_test.go index b40870091b0..898a48efe72 100644 --- a/modules/light-clients/07-tendermint/types/tendermint_test.go +++ b/modules/light-clients/07-tendermint/types/tendermint_test.go @@ -43,9 +43,6 @@ type TendermintTestSuite struct { chainA *ibctesting.TestChain chainB *ibctesting.TestChain - altValSet *tmtypes.ValidatorSet - altSigners map[string]tmtypes.PrivValidator - // TODO: deprecate usage in favor of testing package ctx sdk.Context cdc codec.Codec @@ -67,19 +64,6 @@ func (suite *TendermintTestSuite) SetupTest() { suite.coordinator.CommitNBlocks(suite.chainA, 2) suite.coordinator.CommitNBlocks(suite.chainB, 2) - // 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) - suite.altValSet = tmtypes.NewValidatorSet([]*tmtypes.Validator{altVal}) - - suite.altSigners = getAltSigners(altVal, altPrivVal) - // TODO: deprecate usage in favor of testing package checkTx := false app := simapp.Setup(checkTx) From c9aeb5d58e030461f6753fa1068a83a45a8fa962 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 28 Mar 2022 17:33:22 +0200 Subject: [PATCH 8/9] Update modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go Co-authored-by: Damian Nolan --- .../07-tendermint/types/misbehaviour_handle_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 4a83b9fd13d..4657bae4d34 100644 --- a/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go +++ b/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go @@ -636,7 +636,7 @@ func (suite *TendermintTestSuite) TestVerifyMisbehaviour() { }, false, }, { - "invalid misbehavior misbehaviour from different chain", func() { + "invalid misbehaviour: misbehaviour from different chain", func() { trustedHeight := path.EndpointA.GetClientState().GetLatestHeight().(clienttypes.Height) trustedVals, found := suite.chainB.GetValsAtHeight(int64(trustedHeight.RevisionHeight) + 1) From cb35a939a331ad086fc759603d93c06d8022c7b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 28 Mar 2022 17:41:56 +0200 Subject: [PATCH 9/9] add back misbehaviour type assertion --- .../07-tendermint/types/misbehaviour_handle.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/modules/light-clients/07-tendermint/types/misbehaviour_handle.go b/modules/light-clients/07-tendermint/types/misbehaviour_handle.go index 51f939c7903..932829aa56b 100644 --- a/modules/light-clients/07-tendermint/types/misbehaviour_handle.go +++ b/modules/light-clients/07-tendermint/types/misbehaviour_handle.go @@ -27,7 +27,12 @@ func (cs ClientState) CheckMisbehaviourAndUpdateState( clientStore sdk.KVStore, misbehaviour exported.ClientMessage, ) (exported.ClientState, error) { - if err := cs.VerifyClientMessage(ctx, clientStore, cdc, misbehaviour); err != nil { + tmMisbehaviour, ok := misbehaviour.(*Misbehaviour) + if !ok { + return nil, sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "expected type %T, got %T", misbehaviour, &Misbehaviour{}) + } + + if err := cs.VerifyClientMessage(ctx, clientStore, cdc, tmMisbehaviour); err != nil { return nil, err }