fix: noop on UpdateState for invalid misbehaviour (backport #6276) (#…

…6296) * 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 <[email protected]> * chore: add changelog --------- Co-authored-by: colin axnér <[email protected]> Co-authored-by: Carlos Rodriguez <[email protected]> (cherry picked from commit 4f31a3c) # Conflicts: # modules/light-clients/07-tendermint/update.go * chore: fix conflicts --------- Co-authored-by: Damian Nolan <[email protected]>
cosmos · May 13, 2024 · 1c05fa7 · 1c05fa7
1 parent 2e7e3ea
commit 1c05fa7
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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