Adding VerifyClientMessage to ClientState interface (cosmos#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>
ComposableFi · Aug 9, 2022 · d9f29cc · d9f29cc
1 parent cfb8e49
commit d9f29cc
Show file tree

Hide file tree

Showing 8 changed files with 20 additions and 68 deletions.
diff --git a/CHANGELOG.md b/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
 

diff --git a/modules/core/02-client/legacy/v100/solomachine.go b/modules/core/02-client/legacy/v100/solomachine.go
@@ -7,7 +7,7 @@ import (
 	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
-	"github.com/cosmos/ibc-go/v5/modules/core/exported"
+	"github.com/cosmos/ibc-go/v3/modules/core/exported"
 )
 
 // NOTE: this is a mock implmentation for exported.ClientState. This implementation
@@ -88,30 +88,13 @@ func (cs ClientState) ExportMetadata(_ sdk.KVStore) []exported.GenesisMetadata {
 	panic("legacy solo machine is deprecated!")
 }
 
-// CheckForMisbehaviour panics!
-func (cs ClientState) CheckForMisbehaviour(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, msg exported.ClientMessage) bool {
-	panic("legacy solo machine is deprecated!")
-}
-
-// UpdateStateOnMisbehaviour panics!
-func (cs *ClientState) UpdateStateOnMisbehaviour(
-	_ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore, _ exported.ClientMessage,
-) {
-	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!")
 }
 
-// UpdateState panis!
-func (cs *ClientState) UpdateState(_ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore, _ exported.ClientMessage) []exported.Height {
-	panic("legacy solo machine is deprecated!")
-}
-
 // CheckHeaderAndUpdateState panics!
 func (cs *ClientState) CheckHeaderAndUpdateState(
 	_ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore, _ exported.ClientMessage,
@@ -130,7 +113,7 @@ func (cs ClientState) CheckMisbehaviourAndUpdateState(
 func (cs ClientState) CheckSubstituteAndUpdateState(
 	ctx sdk.Context, _ codec.BinaryCodec, _, _ sdk.KVStore,
 	_ exported.ClientState,
-) error {
+) (exported.ClientState, error) {
 	panic("legacy solo machine is deprecated!")
 }
 
@@ -211,42 +194,6 @@ func (cs ClientState) VerifyNextSequenceRecv(
 	panic("legacy solo machine is deprecated!")
 }
 
-// GetTimestampAtHeight panics!
-func (cs ClientState) GetTimestampAtHeight(
-	sdk.Context, sdk.KVStore, codec.BinaryCodec, exported.Height,
-) (uint64, error) {
-	panic("legacy solo machine is deprecated!")
-}
-
-// VerifyMembership panics!
-func (cs *ClientState) VerifyMembership(
-	ctx sdk.Context,
-	clientStore sdk.KVStore,
-	cdc codec.BinaryCodec,
-	height exported.Height,
-	delayTimePeriod uint64,
-	delayBlockPeriod uint64,
-	proof []byte,
-	path []byte,
-	value []byte,
-) error {
-	panic("legacy solo machine is deprecated!")
-}
-
-// VerifyNonMembership panics!
-func (cs *ClientState) VerifyNonMembership(
-	ctx sdk.Context,
-	clientStore sdk.KVStore,
-	cdc codec.BinaryCodec,
-	height exported.Height,
-	delayTimePeriod uint64,
-	delayBlockPeriod uint64,
-	proof []byte,
-	path []byte,
-) error {
-	panic("legacy solo machine is deprecated")
-}
-
 // ClientType panics!
 func (ConsensusState) ClientType() string {
 	panic("legacy solo machine is deprecated!")

diff --git a/modules/core/exported/client.go b/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)

diff --git a/modules/light-clients/07-tendermint/misbehaviour_handle.go b/modules/light-clients/07-tendermint/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
 	}
 

diff --git a/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go b/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)

diff --git a/modules/light-clients/07-tendermint/types/update.go b/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) {

diff --git a/modules/light-clients/07-tendermint/types/update_test.go b/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)

diff --git a/modules/light-clients/09-localhost/types/client_state.go b/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,