From 1c05fa7dbd86a0c711bdd6575f0abbbc331d0600 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 13 May 2024 10:38:18 +0200 Subject: [PATCH] fix: noop on UpdateState for invalid misbehaviour (backport #6276) (#6296) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: noop on UpdateState for invalid misbehaviour (#6276) * fix: noop on UpdateState for invalid misbehaviour * godoc: update godoc for UpdateState * Update modules/light-clients/07-tendermint/update.go Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> * chore: add changelog --------- Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> Co-authored-by: Carlos Rodriguez (cherry picked from commit 4f31a3c6201bfd53df0c5533448c3197e3b49f1f) # Conflicts: # modules/light-clients/07-tendermint/update.go * chore: fix conflicts --------- Co-authored-by: Damian Nolan --- CHANGELOG.md | 2 ++ modules/light-clients/07-tendermint/update.go | 5 +++-- modules/light-clients/07-tendermint/update_test.go | 9 ++++++--- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ca36a43c73..ece47d19b21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### State Machine Breaking +* (light-clients/07-tendermint) Fix: No-op to avoid panicking on `UpdateState` for invalid misbehaviour submissions. + ### Improvements * (apps/27-interchain-accounts) [\#6147](https://github.com/cosmos/ibc-go/pull/6147) Emit an event signalling that the host submodule is disabled. diff --git a/modules/light-clients/07-tendermint/update.go b/modules/light-clients/07-tendermint/update.go index bceb28d8d83..87fb64ba168 100644 --- a/modules/light-clients/07-tendermint/update.go +++ b/modules/light-clients/07-tendermint/update.go @@ -2,7 +2,6 @@ package tendermint import ( "bytes" - "fmt" "github.com/cometbft/cometbft/light" tmtypes "github.com/cometbft/cometbft/types" @@ -128,10 +127,12 @@ func (cs *ClientState) verifyHeader( // UpdateState must only be used to update within a single revision, thus header revision number and trusted height's revision // number must be the same. To update to a new revision, use a separate upgrade path // UpdateState will prune the oldest consensus state if it is expired. +// If the provided clientMsg is not of type of Header then the handler will noop and empty slice is returned. func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, clientMsg exported.ClientMessage) []exported.Height { header, ok := clientMsg.(*Header) if !ok { - panic(fmt.Errorf("expected type %T, got %T", &Header{}, clientMsg)) + // clientMsg is invalid Misbehaviour, no update necessary + return []exported.Height{} } cs.pruneOldestConsensusState(ctx, cdc, clientStore) diff --git a/modules/light-clients/07-tendermint/update_test.go b/modules/light-clients/07-tendermint/update_test.go index b4e072ecbef..ba591c22b37 100644 --- a/modules/light-clients/07-tendermint/update_test.go +++ b/modules/light-clients/07-tendermint/update_test.go @@ -506,9 +506,12 @@ func (suite *TendermintTestSuite) TestUpdateState() { suite.Require().Equal(expConsensusState, updatedConsensusState) } else { - suite.Require().Panics(func() { - clientState.UpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), clientStore, clientMessage) - }) + consensusHeights = clientState.UpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), clientStore, clientMessage) + suite.Require().Empty(consensusHeights) + + consensusState, found := suite.chainA.GetSimApp().GetIBCKeeper().ClientKeeper.GetClientConsensusState(suite.chainA.GetContext(), path.EndpointA.ClientID, clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()))) + suite.Require().False(found) + suite.Require().Nil(consensusState) } // perform custom checks