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..ef92046b9a8 100644 --- a/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go +++ b/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go @@ -40,7 +40,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 }{ diff --git a/modules/light-clients/07-tendermint/types/update.go b/modules/light-clients/07-tendermint/types/update.go index 69e6936ac4f..18b4a2daa50 100644 --- a/modules/light-clients/07-tendermint/types/update.go +++ b/modules/light-clients/07-tendermint/types/update.go @@ -285,6 +285,51 @@ func (cs ClientState) pruneOldestConsensusState(ctx sdk.Context, cdc codec.Binar } } +// CheckForMisbehaviour detects duplicate height misbehaviour and BFT time violation misbehaviour +func (cs ClientState) CheckForMisbehaviour(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, msg exported.ClientMessage) bool { + switch msg := msg.(type) { + case *Header: + tmHeader := msg + consState := tmHeader.ConsensusState() + + // Check if the Client store already has a consensus state for the header's height + // If the consensus state exists, and it matches the header then we return early + // since header has already been submitted in a previous UpdateClient. + prevConsState, _ := GetConsensusState(clientStore, cdc, tmHeader.GetHeight()) + if prevConsState != nil { + // This header has already been submitted and the necessary state is already stored + // in client store, thus we can return early without further validation. + if reflect.DeepEqual(prevConsState, tmHeader.ConsensusState()) { + return false + } + + // A consensus state already exists for this height, but it does not match the provided header. + // The assumption is that Header has already been validated. Thus we can return true as misbehaviour is present + return true + } + + // Check that consensus state timestamps are monotonic + prevCons, prevOk := GetPreviousConsensusState(clientStore, cdc, tmHeader.GetHeight()) + nextCons, nextOk := GetNextConsensusState(clientStore, cdc, tmHeader.GetHeight()) + // if previous consensus state exists, check consensus state time is greater than previous consensus state time + // if previous consensus state is not before current consensus state return true + if prevOk && !prevCons.Timestamp.Before(consState.Timestamp) { + return true + } + // if next consensus state exists, check consensus state time is less than next consensus state time + // if next consensus state is not after current consensus state return true + if nextOk && !nextCons.Timestamp.After(consState.Timestamp) { + return true + } + case *Misbehaviour: + // The correctness of Misbehaviour ClientMessage types is ensured by calling VerifyClientMessage prior to this function + // Thus, here we can return true, as ClientMessage is of type Misbehaviour + return true + } + + return false +} + // UpdateStateOnMisbehaviour updates state upon misbehaviour, freezing the ClientState. This method should only be called when misbehaviour is detected // as it does not perform any misbehaviour checks. func (cs ClientState) UpdateStateOnMisbehaviour(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore) { diff --git a/modules/light-clients/07-tendermint/types/update_test.go b/modules/light-clients/07-tendermint/types/update_test.go index 1fd5a5253d1..7216f48d33f 100644 --- a/modules/light-clients/07-tendermint/types/update_test.go +++ b/modules/light-clients/07-tendermint/types/update_test.go @@ -8,6 +8,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" + host "github.com/cosmos/ibc-go/v3/modules/core/24-host" "github.com/cosmos/ibc-go/v3/modules/core/exported" types "github.com/cosmos/ibc-go/v3/modules/light-clients/07-tendermint/types" ibctesting "github.com/cosmos/ibc-go/v3/testing" @@ -591,3 +592,196 @@ func (suite *TendermintTestSuite) TestPruneConsensusState() { consKey = types.GetIterationKey(clientStore, expiredHeight) suite.Require().Equal(expectedConsKey, consKey, "iteration key incorrectly pruned") } + +func (suite *TendermintTestSuite) TestCheckForMisbehaviour() { + var ( + path *ibctesting.Path + clientMessage exported.ClientMessage + ) + + testCases := []struct { + name string + malleate func() + expPass bool + }{ + { + "valid update no misbehaviour", + func() {}, + false, + }, + { + "consensus state already exists, already updated", + func() { + header, ok := clientMessage.(*types.Header) + suite.Require().True(ok) + + consensusState := &types.ConsensusState{ + Timestamp: header.GetTime(), + Root: commitmenttypes.NewMerkleRoot(header.Header.GetAppHash()), + NextValidatorsHash: header.Header.NextValidatorsHash, + } + + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), path.EndpointA.ClientID, clientMessage.GetHeight(), consensusState) + }, + false, + }, + { + "consensus state already exists, app hash mismatch", + func() { + header, ok := clientMessage.(*types.Header) + suite.Require().True(ok) + + consensusState := &types.ConsensusState{ + Timestamp: header.GetTime(), + Root: commitmenttypes.NewMerkleRoot([]byte{}), // empty bytes + NextValidatorsHash: header.Header.NextValidatorsHash, + } + + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), path.EndpointA.ClientID, clientMessage.GetHeight(), consensusState) + }, + true, + }, + { + "previous consensus state exists and header time is before previous consensus state time", + func() { + header, ok := clientMessage.(*types.Header) + suite.Require().True(ok) + + // offset header timestamp before previous consensus state timestamp + header.Header.Time = header.GetTime().Add(-time.Hour) + }, + true, + }, + { + "next consensus state exists and header time is after next consensus state time", + func() { + header, ok := clientMessage.(*types.Header) + suite.Require().True(ok) + + // commit block and update client, adding a new consensus state + suite.coordinator.CommitBlock(suite.chainB) + err := path.EndpointA.UpdateClient() + suite.Require().NoError(err) + + // increase timestamp of current header + header.Header.Time = header.Header.Time.Add(time.Hour) + }, + true, + }, + { + "valid fork misbehaviour returns true", + func() { + header1, err := path.EndpointA.Chain.ConstructUpdateTMClientHeader(path.EndpointA.Counterparty.Chain, path.EndpointA.ClientID) + suite.Require().NoError(err) + + // commit block and update client + suite.coordinator.CommitBlock(suite.chainB) + err = path.EndpointA.UpdateClient() + suite.Require().NoError(err) + + header2, err := path.EndpointA.Chain.ConstructUpdateTMClientHeader(path.EndpointA.Counterparty.Chain, path.EndpointA.ClientID) + suite.Require().NoError(err) + + // assign the same height, each header will have a different commit hash + header1.Header.Height = header2.Header.Height + + clientMessage = &types.Misbehaviour{ + Header1: header1, + Header2: header2, + ClientId: path.EndpointA.ClientID, + } + }, + true, + }, + } + + for _, tc := range testCases { + tc := tc + suite.Run(tc.name, func() { + // reset suite to create fresh application state + suite.SetupTest() + path = ibctesting.NewPath(suite.chainA, suite.chainB) + + err := path.EndpointA.CreateClient() + suite.Require().NoError(err) + + // ensure counterparty state is committed + suite.coordinator.CommitBlock(suite.chainB) + clientMessage, err = path.EndpointA.Chain.ConstructUpdateTMClientHeader(path.EndpointA.Counterparty.Chain, path.EndpointA.ClientID) + suite.Require().NoError(err) + + tc.malleate() + + clientState := path.EndpointA.GetClientState() + + // TODO: remove casting when 'UpdateState' is an interface function. + tmClientState, ok := clientState.(*types.ClientState) + suite.Require().True(ok) + + clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID) + + foundMisbehaviour := tmClientState.CheckForMisbehaviour( + suite.chainA.GetContext(), + suite.chainA.App.AppCodec(), + clientStore, // pass in clientID prefixed clientStore + clientMessage, + ) + + if tc.expPass { + suite.Require().True(foundMisbehaviour) + } else { + suite.Require().False(foundMisbehaviour) + } + }) + } +} + +func (suite *TendermintTestSuite) TestUpdateStateOnMisbehaviour() { + var ( + path *ibctesting.Path + ) + + testCases := []struct { + name string + malleate func() + expPass bool + }{ + { + "success", + func() {}, + true, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + // reset suite to create fresh application state + suite.SetupTest() + path = ibctesting.NewPath(suite.chainA, suite.chainB) + + err := path.EndpointA.CreateClient() + suite.Require().NoError(err) + + tc.malleate() + + clientState := path.EndpointA.GetClientState() + clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID) + + // TODO: remove casting when 'UpdateState' is an interface function. + tmClientState, ok := clientState.(*types.ClientState) + suite.Require().True(ok) + + tmClientState.UpdateStateOnMisbehaviour(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), clientStore) + + if tc.expPass { + clientStateBz := clientStore.Get(host.ClientStateKey()) + suite.Require().NotEmpty(clientStateBz) + + newClientState := clienttypes.MustUnmarshalClientState(suite.chainA.Codec, clientStateBz) + suite.Require().Equal(frozenHeight, newClientState.(*types.ClientState).FrozenHeight) + } + }) + } +} diff --git a/modules/light-clients/07-tendermint/upgrade_test.go b/modules/light-clients/07-tendermint/upgrade_test.go index aaa8289f864..112d3366cda 100644 --- a/modules/light-clients/07-tendermint/upgrade_test.go +++ b/modules/light-clients/07-tendermint/upgrade_test.go @@ -5,7 +5,6 @@ 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" - host "github.com/cosmos/ibc-go/v3/modules/core/24-host" "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" @@ -482,53 +481,3 @@ func (suite *TendermintTestSuite) TestVerifyUpgrade() { } } } - -func (suite *TendermintTestSuite) TestUpdateStateOnMisbehaviour() { - var ( - path *ibctesting.Path - ) - - testCases := []struct { - name string - malleate func() - expPass bool - }{ - { - "success", - func() {}, - true, - }, - } - - for _, tc := range testCases { - tc := tc - - suite.Run(tc.name, func() { - // reset suite to create fresh application state - suite.SetupTest() - path = ibctesting.NewPath(suite.chainA, suite.chainB) - - err := path.EndpointA.CreateClient() - suite.Require().NoError(err) - - tc.malleate() - - clientState := path.EndpointA.GetClientState() - clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID) - - // TODO: remove casting when 'UpdateState' is an interface function. - tmClientState, ok := clientState.(*types.ClientState) - suite.Require().True(ok) - - tmClientState.UpdateStateOnMisbehaviour(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), clientStore) - - if tc.expPass { - clientStateBz := clientStore.Get(host.ClientStateKey()) - suite.Require().NotEmpty(clientStateBz) - - newClientState := clienttypes.MustUnmarshalClientState(suite.chainA.Codec, clientStateBz) - suite.Require().Equal(frozenHeight, newClientState.(*types.ClientState).FrozenHeight) - } - }) - } -}