Skip to content

Commit

Permalink
fix: noop on UpdateState for invalid misbehaviour (#6276)
Browse files Browse the repository at this point in the history
* 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 <carlos@interchain.io>
  • Loading branch information
3 people authored May 13, 2024
1 parent bca605e commit 4f31a3c
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,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) [\#5533](https://github.com/cosmos/ibc-go/pull/5533) ICA host sets the host connection ID on `OnChanOpenTry`, so that ICA controller implementations are not obliged to set the value on `OnChanOpenInit` if they are not able.
Expand Down
4 changes: 3 additions & 1 deletion modules/light-clients/07-tendermint/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,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 storetypes.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)
Expand Down
9 changes: 6 additions & 3 deletions modules/light-clients/07-tendermint/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -546,9 +546,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
Expand Down

0 comments on commit 4f31a3c

Please sign in to comment.