chore(06-solomachine, 07-tendermint): make client state methods moved…

… to lcm private (#6891) (#6912)

* chore(06-solomachine, 07-tendermint)make client state methods moved to lcm private.

* docs: add changelog, add to migration doc.

* Update CHANGELOG.md

* Update docs/docs/05-migrations/13-v8-to-v9.md

(cherry picked from commit a6fd4d7)

Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>

CHANGELOG.md

@@ -66,6 +66,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (core/02-client) [\#6777](https://github.com/cosmos/ibc-go/pull/6777) The `NewClientProposalHandler` of `02-client` has been removed.
 * (core/types) [\#6794](https://github.com/cosmos/ibc-go/pull/6794) The composite interface `QueryServer` has been removed from package `core/types`. Please use the granular `QueryServer` interfaces provided by each core submodule.
 * (light-clients/06-solomachine) [\#6888](https://github.com/cosmos/ibc-go/pull/6888) Remove `TypeClientMisbehaviour` constant and the `Type` method on `Misbehaviour`.
+* (light-clients/06-solomachine, light-clients/07-tendermint) [\#6891](https://github.com/cosmos/ibc-go/pull/6891) The `VerifyMembership` and `VerifyNonMembership` functions of solomachine's `ClientState` have been made private. The `VerifyMembership`, `VerifyNonMembership`, `GetTimestampAtHeight`, `Status` and `Initialize` functions of tendermint's `ClientState` have been made private.
 
 ### State Machine Breaking
 

docs/docs/05-migrations/13-v8-to-v9.md

@@ -203,12 +203,12 @@ Please check also the [Light client developer guide](../03-light-clients/01-deve
 
 ### 06-solomachine
 
-- The `Initialize`, `Status`, `GetTimestampAtHeight` and `UpdateStateOnMisbehaviour` functions in `ClientState` have been removed and all their logic has been moved to functions of the `LightClientModule`.
+- The `Initialize`, `Status`, `GetTimestampAtHeight` and `UpdateStateOnMisbehaviour` functions in `ClientState` have been removed and all their logic has been moved to functions of the `LightClientModule`. The `VerifyMembership` and `VerifyNonMembership` functions have been made private.
 - The `Type` method on `Misbehaviour` has been removed.
 
 ### 07-tendermint
 
-The `IterateConsensusMetadata` function has been removed.
+The `IterateConsensusMetadata` function has been removed. The `VerifyMembership`, `VerifyNonMembership`, `GetTimestampAtHeight`, `Status` and `Initialize` functions have been made private.
 
 ### 08-wasm
 

modules/core/02-client/keeper/client_test.go

@@ -682,10 +682,9 @@ func (suite *KeeperTestSuite) TestRecoverClient() {
 				ibctesting.AssertEvents(&suite.Suite, expectedEvents, ctx.EventManager().Events().ToABCIEvents())
 
 				// Assert that client status is now Active
-				clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID)
-				tmClientState, ok := subjectPath.EndpointA.GetClientState().(*ibctm.ClientState)
-				suite.Require().True(ok)
-				suite.Require().Equal(tmClientState.Status(suite.chainA.GetContext(), clientStore, suite.chainA.App.AppCodec()), exported.Active)
+				lightClientModule, err := suite.chainA.App.GetIBCKeeper().ClientKeeper.Route(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID)
+				suite.Require().NoError(err)
+				suite.Require().Equal(lightClientModule.Status(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID), exported.Active)
 
 			} else {
 				suite.Require().Error(err)

modules/core/keeper/msg_server_test.go

@@ -287,10 +287,10 @@ func (suite *KeeperTestSuite) TestRecoverClient() {
 				suite.Require().NoError(err)
 
 				// Assert that client status is now Active
-				clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID)
-				tmClientState, ok := subjectPath.EndpointA.GetClientState().(*ibctm.ClientState)
-				suite.Require().True(ok)
-				suite.Require().Equal(tmClientState.Status(suite.chainA.GetContext(), clientStore, suite.chainA.App.AppCodec()), exported.Active)
+
+				lightClientModule, err := suite.chainA.App.GetIBCKeeper().ClientKeeper.Route(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID)
+				suite.Require().NoError(err)
+				suite.Require().Equal(lightClientModule.Status(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID), exported.Active)
 			} else {
 				suite.Require().Error(err)
 				suite.Require().ErrorIs(err, tc.expErr)

modules/light-clients/06-solomachine/client_state.go

@@ -6,7 +6,6 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/codec"
 	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
-	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/tx/signing"
 
 	clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types"
@@ -43,15 +42,11 @@ func (cs ClientState) Validate() error {
 	return cs.ConsensusState.ValidateBasic()
 }
 
-// VerifyMembership is a generic proof verification method which verifies a proof of the existence of a value at a given CommitmentPath at the latest sequence.
+// verifyMembership is a generic proof verification method which verifies a proof of the existence of a value at a given CommitmentPath at the latest sequence.
 // The caller is expected to construct the full CommitmentPath from a CommitmentPrefix and a standardized path (as defined in ICS 24).
-func (cs *ClientState) VerifyMembership(
-	ctx sdk.Context,
+func (cs *ClientState) verifyMembership(
 	clientStore storetypes.KVStore,
 	cdc codec.BinaryCodec,
-	_ exported.Height,
-	delayTimePeriod uint64,
-	delayBlockPeriod uint64,
 	proof []byte,
 	path exported.Path,
 	value []byte,
@@ -100,15 +95,11 @@ func (cs *ClientState) VerifyMembership(
 	return nil
 }
 
-// VerifyNonMembership is a generic proof verification method which verifies the absence of a given CommitmentPath at the latest sequence.
+// verifyNonMembership is a generic proof verification method which verifies the absence of a given CommitmentPath at the latest sequence.
 // The caller is expected to construct the full CommitmentPath from a CommitmentPrefix and a standardized path (as defined in ICS 24).
-func (cs *ClientState) VerifyNonMembership(
-	ctx sdk.Context,
+func (cs *ClientState) verifyNonMembership(
 	clientStore storetypes.KVStore,
 	cdc codec.BinaryCodec,
-	_ exported.Height,
-	delayTimePeriod uint64,
-	delayBlockPeriod uint64,
 	proof []byte,
 	path exported.Path,
 ) error {

modules/light-clients/06-solomachine/light_client_module.go

@@ -125,7 +125,7 @@ func (l LightClientModule) VerifyMembership(
 		return errorsmod.Wrap(clienttypes.ErrClientNotFound, clientID)
 	}
 
-	return clientState.VerifyMembership(ctx, clientStore, l.cdc, height, delayTimePeriod, delayBlockPeriod, proof, path, value)
+	return clientState.verifyMembership(clientStore, l.cdc, proof, path, value)
 }
 
 // VerifyNonMembership obtains the client state associated with the client identifier and calls into the clientState.verifyNonMembership method.
@@ -144,7 +144,7 @@ func (l LightClientModule) VerifyNonMembership(
 		return errorsmod.Wrap(clienttypes.ErrClientNotFound, clientID)
 	}
 
-	return clientState.VerifyNonMembership(ctx, clientStore, l.cdc, height, delayTimePeriod, delayBlockPeriod, proof, path)
+	return clientState.verifyNonMembership(clientStore, l.cdc, proof, path)
 }
 
 // Status returns the status of the solo machine client.

modules/light-clients/07-tendermint/client_state.go

@@ -54,9 +54,8 @@ func (ClientState) ClientType() string {
 	return exported.Tendermint
 }
 
-// GetTimestampAtHeight returns the timestamp in nanoseconds of the consensus state at the given height.
-func (ClientState) GetTimestampAtHeight(
-	ctx sdk.Context,
+// getTimestampAtHeight returns the timestamp in nanoseconds of the consensus state at the given height.
+func (ClientState) getTimestampAtHeight(
 	clientStore storetypes.KVStore,
 	cdc codec.BinaryCodec,
 	height exported.Height,
@@ -69,15 +68,15 @@ func (ClientState) GetTimestampAtHeight(
 	return consState.GetTimestamp(), nil
 }
 
-// Status returns the status of the tendermint client.
+// status returns the status of the tendermint client.
 // The client may be:
 // - Active: FrozenHeight is zero and client is not expired
 // - Frozen: Frozen Height is not zero
 // - Expired: the latest consensus state timestamp + trusting period <= current time
 //
 // A frozen client will become expired, so the Frozen status
 // has higher precedence.
-func (cs ClientState) Status(
+func (cs ClientState) status(
 	ctx sdk.Context,
 	clientStore storetypes.KVStore,
 	cdc codec.BinaryCodec,
@@ -185,9 +184,9 @@ func (cs ClientState) ZeroCustomFields() *ClientState {
 	}
 }
 
-// Initialize checks that the initial consensus state is an 07-tendermint consensus state and
+// initialize checks that the initial consensus state is an 07-tendermint consensus state and
 // sets the client state, consensus state and associated metadata in the provided client store.
-func (cs ClientState) Initialize(ctx sdk.Context, cdc codec.BinaryCodec, clientStore storetypes.KVStore, consState exported.ConsensusState) error {
+func (cs ClientState) initialize(ctx sdk.Context, cdc codec.BinaryCodec, clientStore storetypes.KVStore, consState exported.ConsensusState) error {
 	consensusState, ok := consState.(*ConsensusState)
 	if !ok {
 		return errorsmod.Wrapf(clienttypes.ErrInvalidConsensus, "invalid initial consensus state. expected type: %T, got: %T",
@@ -201,10 +200,10 @@ func (cs ClientState) Initialize(ctx sdk.Context, cdc codec.BinaryCodec, clientS
 	return nil
 }
 
-// VerifyMembership is a generic proof verification method which verifies a proof of the existence of a value at a given CommitmentPath at the specified height.
+// verifyMembership is a generic proof verification method which verifies a proof of the existence of a value at a given CommitmentPath at the specified height.
 // The caller is expected to construct the full CommitmentPath from a CommitmentPrefix and a standardized path (as defined in ICS 24).
 // If a zero proof height is passed in, it will fail to retrieve the associated consensus state.
-func (cs ClientState) VerifyMembership(
+func (cs ClientState) verifyMembership(
 	ctx sdk.Context,
 	clientStore storetypes.KVStore,
 	cdc codec.BinaryCodec,
@@ -244,10 +243,10 @@ func (cs ClientState) VerifyMembership(
 	return merkleProof.VerifyMembership(cs.ProofSpecs, consensusState.GetRoot(), merklePath, value)
 }
 
-// VerifyNonMembership is a generic proof verification method which verifies the absence of a given CommitmentPath at a specified height.
+// verifyNonMembership is a generic proof verification method which verifies the absence of a given CommitmentPath at a specified height.
 // The caller is expected to construct the full CommitmentPath from a CommitmentPrefix and a standardized path (as defined in ICS 24).
 // If a zero proof height is passed in, it will fail to retrieve the associated consensus state.
-func (cs ClientState) VerifyNonMembership(
+func (cs ClientState) verifyNonMembership(
 	ctx sdk.Context,
 	clientStore storetypes.KVStore,
 	cdc codec.BinaryCodec,

modules/light-clients/07-tendermint/light_client_module.go

@@ -52,7 +52,7 @@ func (l LightClientModule) Initialize(ctx sdk.Context, clientID string, clientSt
 
 	clientStore := l.storeProvider.ClientStore(ctx, clientID)
 
-	return clientState.Initialize(ctx, l.cdc, clientStore, &consensusState)
+	return clientState.initialize(ctx, l.cdc, clientStore, &consensusState)
 }
 
 // VerifyClientMessage obtains the client state associated with the client identifier and calls into the clientState.VerifyClientMessage method.
@@ -116,7 +116,7 @@ func (l LightClientModule) VerifyMembership(
 		return errorsmod.Wrap(clienttypes.ErrClientNotFound, clientID)
 	}
 
-	return clientState.VerifyMembership(ctx, clientStore, l.cdc, height, delayTimePeriod, delayBlockPeriod, proof, path, value)
+	return clientState.verifyMembership(ctx, clientStore, l.cdc, height, delayTimePeriod, delayBlockPeriod, proof, path, value)
 }
 
 // VerifyNonMembership obtains the client state associated with the client identifier and calls into the clientState.verifyNonMembership method.
@@ -135,7 +135,7 @@ func (l LightClientModule) VerifyNonMembership(
 		return errorsmod.Wrap(clienttypes.ErrClientNotFound, clientID)
 	}
 
-	return clientState.VerifyNonMembership(ctx, clientStore, l.cdc, height, delayTimePeriod, delayBlockPeriod, proof, path)
+	return clientState.verifyNonMembership(ctx, clientStore, l.cdc, height, delayTimePeriod, delayBlockPeriod, proof, path)
 }
 
 // Status obtains the client state associated with the client identifier and calls into the clientState.status method.
@@ -146,7 +146,7 @@ func (l LightClientModule) Status(ctx sdk.Context, clientID string) exported.Sta
 		return exported.Unknown
 	}
 
-	return clientState.Status(ctx, clientStore, l.cdc)
+	return clientState.status(ctx, clientStore, l.cdc)
 }
 
 // LatestHeight returns the latest height for the client state for the given client identifier.
@@ -173,7 +173,7 @@ func (l LightClientModule) TimestampAtHeight(
 		return 0, errorsmod.Wrap(clienttypes.ErrClientNotFound, clientID)
 	}
 
-	return clientState.GetTimestampAtHeight(ctx, clientStore, l.cdc, height)
+	return clientState.getTimestampAtHeight(clientStore, l.cdc, height)
 }
 
 // RecoverClient asserts that the substitute client is a tendermint client. It obtains the client state associated with the

modules/light-clients/07-tendermint/light_client_module_test.go

@@ -1026,10 +1026,9 @@ func (suite *TendermintTestSuite) TestRecoverClient() {
 				suite.Require().NoError(err)
 
 				// assert that status of subject client is now Active
-				clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(ctx, subjectClientID)
-				tmClientState, ok := subjectPath.EndpointA.GetClientState().(*ibctm.ClientState)
-				suite.Require().True(ok)
-				suite.Require().Equal(exported.Active, tmClientState.Status(ctx, clientStore, suite.chainA.App.AppCodec()))
+				lightClientModule, err := suite.chainA.App.GetIBCKeeper().ClientKeeper.Route(suite.chainA.GetContext(), subjectClientID)
+				suite.Require().NoError(err)
+				suite.Require().Equal(lightClientModule.Status(suite.chainA.GetContext(), subjectClientID), exported.Active)
 			} else {
 				suite.Require().Error(err)
 				suite.Require().ErrorIs(err, tc.expErr)

modules/light-clients/07-tendermint/proposal_handle.go

@@ -39,7 +39,7 @@ func (cs ClientState) CheckSubstituteAndUpdateState(
 		return errorsmod.Wrap(clienttypes.ErrInvalidSubstitute, "subject client state does not match substitute client state")
 	}
 
-	if cs.Status(ctx, subjectClientStore, cdc) == exported.Frozen {
+	if cs.status(ctx, subjectClientStore, cdc) == exported.Frozen {
 		// unfreeze the client
 		cs.FrozenHeight = clienttypes.ZeroHeight()
 	}