Adding VerifyClientMessage to ClientState interface (#1196)

* feat: adding VerifyClientMessage to ClientState interface

* fix: legacy

* remove todo + fix test

* Update modules/core/exported/client.go

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>

* Update modules/core/02-client/legacy/v100/solomachine.go

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* chore: changelog

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>

Author: 3 people

CHANGELOG.md

@@ -44,6 +44,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 * [\#1186](https://github.com/cosmos/ibc-go/pull/1186/files) Removing `GetRoot` function from ConsensusState interface in `02-client`. `GetRoot` is unused by core IBC. 
+* (modules/core/02-client) [\#1196](https://github.com/cosmos/ibc-go/pull/1196) Adding VerifyClientMessage to ClientState interface. 
 
 ### Features
 

modules/core/02-client/legacy/v100/solomachine.go

@@ -88,6 +88,13 @@ func (cs ClientState) ExportMetadata(_ sdk.KVStore) []exported.GenesisMetadata {
 	panic("legacy solo machine is deprecated!")
 }
 
+// VerifyClientMessage panics!
+func (cs *ClientState) VerifyClientMessage(
+	_ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore, _ exported.ClientMessage,
+) error {
+	panic("legacy solo machine is deprecated!")
+}
+
 // CheckHeaderAndUpdateState panics!
 func (cs *ClientState) CheckHeaderAndUpdateState(
 	_ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore, _ exported.ClientMessage,

modules/core/exported/client.go

@@ -56,8 +56,10 @@ type ClientState interface {
 	// Genesis function
 	ExportMetadata(sdk.KVStore) []GenesisMetadata
 
-	// Update and Misbehaviour functions
+	// VerifyClientMessage verifies a ClientMessage. A ClientMessage could be a Header, Misbehaviour, or batch update.
+	VerifyClientMessage(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, clientMsg ClientMessage) error
 
+	// Update and Misbehaviour functions
 	CheckHeaderAndUpdateState(sdk.Context, codec.BinaryCodec, sdk.KVStore, ClientMessage) (ClientState, ConsensusState, error)
 	CheckMisbehaviourAndUpdateState(sdk.Context, codec.BinaryCodec, sdk.KVStore, ClientMessage) (ClientState, error)
 	CheckSubstituteAndUpdateState(ctx sdk.Context, cdc codec.BinaryCodec, subjectClientStore, substituteClientStore sdk.KVStore, substituteClient ClientState) (ClientState, error)

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, tmMisbehaviour); err != nil {
+	if err := cs.VerifyClientMessage(ctx, cdc, clientStore, tmMisbehaviour); err != nil {
 		return nil, err
 	}
 

modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go

@@ -775,15 +775,11 @@ func (suite *TendermintTestSuite) TestVerifyMisbehaviour() {
 
 			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)
+			err = clientState.VerifyClientMessage(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), clientStore, misbehaviour)
 
 			if tc.expPass {
 				suite.Require().NoError(err)

modules/light-clients/07-tendermint/types/update.go

@@ -64,7 +64,7 @@ func (cs ClientState) CheckHeaderAndUpdateState(
 		conflictingHeader = true
 	}
 
-	if err := cs.VerifyClientMessage(ctx, clientStore, cdc, tmHeader); err != nil {
+	if err := cs.VerifyClientMessage(ctx, cdc, clientStore, tmHeader); err != nil {
 		return nil, nil, err
 	}
 
@@ -119,7 +119,7 @@ func checkTrustedHeader(header *Header, consState *ConsensusState) error {
 
 // VerifyClientMessage checks if the clientMessage is of type Header or Misbehaviour and verifies the message
 func (cs *ClientState) VerifyClientMessage(
-	ctx sdk.Context, clientStore sdk.KVStore, cdc codec.BinaryCodec,
+	ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore,
 	clientMsg exported.ClientMessage,
 ) error {
 	switch msg := clientMsg.(type) {

modules/light-clients/07-tendermint/types/update_test.go

@@ -382,10 +382,7 @@ func (suite *TendermintTestSuite) TestVerifyHeader() {
 
 		tc.malleate()
 
-		tmClientState, ok := clientState.(*types.ClientState)
-		suite.Require().True(ok)
-
-		err = tmClientState.VerifyClientMessage(suite.chainA.GetContext(), clientStore, suite.chainA.App.AppCodec(), header)
+		err = clientState.VerifyClientMessage(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), clientStore, header)
 
 		if tc.expPass {
 			suite.Require().NoError(err)

modules/light-clients/09-localhost/types/client_state.go

@@ -142,6 +142,15 @@ func (cs ClientState) VerifyUpgradeAndUpdateState(
 	return sdkerrors.Wrap(clienttypes.ErrInvalidUpgradeClient, "cannot upgrade localhost client")
 }
 
+// VerifyClientMessage
+// TODO: localhost client will be removed
+func (cs ClientState) VerifyClientMessage(
+	_ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore,
+	_ exported.ClientMessage,
+) error {
+	return nil
+}
+
 // VerifyClientState verifies that the localhost client state is stored locally
 func (cs ClientState) VerifyClientState(
 	store sdk.KVStore, cdc codec.BinaryCodec,