refactor!: decouple client routing from encoding

docs/docs/03-light-clients/01-developer-guide/02-client-state.md

@@ -15,10 +15,6 @@ Learn how to implement the [`ClientState`](https://github.com/cosmos/ibc-go/blob
 `ClientType` should return a unique string identifier of the light client. This will be used when generating a client identifier.
 The format is created as follows: `ClientType-{N}` where `{N}` is the unique global nonce associated with a specific client.
 
-## `GetLatestHeight` method
-
-`GetLatestHeight` should return the latest block height that the client state represents.
-
 ## `Validate` method
 
 `Validate` should validate every client state field and should return an error if any value is invalid. The light client

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

@@ -55,6 +55,29 @@
 
 The `ExportMetadata` interface function has been removed from the `ClientState` interface. Core IBC will export all key/value's within the 02-client store.  
 
+The `ZeroCustomFields` interface function has been removed from the `ClientState` interface.
+
+The following functions have also been removed from the `ClientState` interface: `Initialize`, `Status`, `GetLatestHeight`, `GetTimestampAtHeight`, `VerifyClientMessage`, `VerifyMembership`, `VerifyNonMembership`, `CheckForMisbehaviour`, `UpdateState`, `UpdateStateOnMisbehaviour`,  `CheckSubstituteAndUpdateState` and `VerifyUpgradeAndUpdateState`. ibc-go v9 decouples routing at the `02-client` layer from the light clients' encoding structure (i.e. every light client implementation of the `ClientState` interface is not used anymore to route the requests to the right light client at the `02-client` layer, instead a _light client module_ is registered for every light client type and `02-client` routes the requests to the right light client module based on the client ID). Light client developers must implement the newly introduced `LightClientModule` interface and are encouraged to move the logic implemented in the functions of their light client's implementation of the `ClientState` interface to the equivalent function in the `LightClientModule` interface. The table below shows the equivalence between the `ClientState` interface functions that have been removed and the functions in the `LightClientModule` interface:
+
+|`ClientState` interface|`LightClientModule` interface|
+|-----------------------|-----------------------------|
+|`Initialize`                   |`Initialize`                 |
+|`Status`                       |`Status`                     |
+|`GetLatestHeight`              |`LatestHeight`               |
+|`GetTimestampAtHeight`         |`TimestampAtHeight`          |
+|`VerifyClientMessage`          |`VerifyClientMessage`        |
+|`VerifyMembership`             |`VerifyMembership`           |
+|`VerifyNonMembership`          |`VerifyNonMembership`        |
+|`CheckForMisbehaviour`         |`CheckForMisbehaviour`       |
+|`UpdateState`                  |`UpdateState`                |
+|`UpdateStateOnMisbehaviour`    |`UpdateStateOnMisbehaviour`  |
+|`CheckSubstituteAndUpdateState`|`RecoverClient`              |
+|`VerifyUpgradeAndUpdateState`  |`VerifyUpgradeAndUpdateState`|
+|`ExportMetadata`               |                             |
+|`ZeroCustomFields`             |                             |
+
+Please check also the [light client developer guide](../03-light-clients/01-developer-guide/01-overview.md) for more information. The light client module implementation for `07-tendermint` may also be useful as reference.
+
 ### 07-tendermint
 
 The `IterateConsensusMetadata` function has been removed.

e2e/tests/core/02-client/client_test.go

@@ -361,8 +361,8 @@ func (s *ClientTestSuite) TestClient_Update_Misbehaviour() {
 		tmClientState, ok := clientState.(*ibctm.ClientState)
 		s.Require().True(ok)
 
-		trustedHeight, ok = tmClientState.GetLatestHeight().(clienttypes.Height)
-		s.Require().True(ok)
+		trustedHeight := tmClientState.LatestHeight
+		s.Require().True(trustedHeight.GT(clienttypes.ZeroHeight()))
 	})
 
 	t.Run("update clients", func(t *testing.T) {
@@ -377,8 +377,8 @@ func (s *ClientTestSuite) TestClient_Update_Misbehaviour() {
 		tmClientState, ok := clientState.(*ibctm.ClientState)
 		s.Require().True(ok)
 
-		latestHeight, ok = tmClientState.GetLatestHeight().(clienttypes.Height)
-		s.Require().True(ok)
+		latestHeight := tmClientState.LatestHeight
+		s.Require().True(latestHeight.GT(clienttypes.ZeroHeight()))
 	})
 
 	t.Run("create validator set", func(t *testing.T) {

e2e/tests/transfer/localhost_test.go

@@ -55,13 +55,13 @@ func (s *LocalhostTransferTestSuite) TestMsgTransfer_Localhost() {
 	t.Run("verify begin blocker was executed", func(t *testing.T) {
 		cs, err := s.QueryClientState(ctx, chainA, exported.LocalhostClientID)
 		s.Require().NoError(err)
-		originalHeight := cs.GetLatestHeight()
+		originalHeight := cs.(*localhost.ClientState).LatestHeight
 
 		s.Require().NoError(test.WaitForBlocks(ctx, 1, chainA), "failed to wait for blocks")
 
 		cs, err = s.QueryClientState(ctx, chainA, exported.LocalhostClientID)
 		s.Require().NoError(err)
-		s.Require().True(cs.GetLatestHeight().GT(originalHeight), "client state height was not incremented")
+		s.Require().True(cs.(*localhost.ClientState).LatestHeight.GT(originalHeight), "client state height was not incremented")
 	})
 
 	t.Run("channel open init localhost", func(t *testing.T) {

modules/apps/callbacks/testing/simapp/app.go

@@ -567,6 +567,14 @@ func NewSimApp(
 	// Seal the IBC Router
 	app.IBCKeeper.SetRouter(ibcRouter)
 
+	clientRouter := app.IBCKeeper.ClientKeeper.GetRouter()
+
+	tmLightClientModule := ibctm.NewLightClientModule(appCodec, authtypes.NewModuleAddress(govtypes.ModuleName).String())
+	clientRouter.AddRoute(ibctm.ModuleName, &tmLightClientModule)
+
+	smLightClientModule := solomachine.NewLightClientModule(appCodec)
+	clientRouter.AddRoute(solomachine.ModuleName, &smLightClientModule)
+
 	// create evidence keeper with router
 	evidenceKeeper := evidencekeeper.NewKeeper(
 		appCodec, runtime.NewKVStoreService(keys[evidencetypes.StoreKey]), app.StakingKeeper, app.SlashingKeeper, app.AccountKeeper.AddressCodec(), runtime.ProvideCometInfoService(),
@@ -611,8 +619,8 @@ func NewSimApp(
 		transfer.NewAppModule(app.TransferKeeper),
 		ibcfee.NewAppModule(app.IBCFeeKeeper),
 		ica.NewAppModule(&app.ICAControllerKeeper, &app.ICAHostKeeper),
-		ibctm.NewAppModule(),
-		solomachine.NewAppModule(),
+		ibctm.NewAppModule(tmLightClientModule),
+		solomachine.NewAppModule(smLightClientModule),
 		mockModule,
 	)
 

modules/core/02-client/abci.go

@@ -35,7 +35,7 @@
 
 	// update the localhost client with the latest block height if it is active.
 	if clientState, found := k.GetClientState(ctx, exported.Localhost); found {
-		if k.GetClientStatus(ctx, clientState, exported.Localhost) == exported.Active {
+		if k.GetClientStatus(ctx, exported.LocalhostClientID) == exported.Active {
 			k.UpdateLocalhostClient(ctx, clientState)
 		}
 	}

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

@@ -9,106 +9,113 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	"github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
-	ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
 	"github.com/cosmos/ibc-go/v8/modules/core/exported"
 )
 
 // CreateClient generates a new client identifier and isolated prefix store for the provided client state.
 // The client state is responsible for setting any client-specific data in the store via the Initialize method.
 // This includes the client state, initial consensus state and any associated metadata.
 func (k Keeper) CreateClient(
-	ctx sdk.Context, clientState exported.ClientState, consensusState exported.ConsensusState,
+	ctx sdk.Context, clientType string, clientState []byte, consensusState []byte,
 ) (string, error) {
-	if clientState.ClientType() == exported.Localhost {
-		return "", errorsmod.Wrapf(types.ErrInvalidClientType, "cannot create client of type: %s", clientState.ClientType())
+	if clientType == exported.Localhost {
+		return "", errorsmod.Wrapf(types.ErrInvalidClientType, "cannot create client of type: %s", clientType)
 	}
 
 	params := k.GetParams(ctx)
-	if !params.IsAllowedClient(clientState.ClientType()) {
+	if !params.IsAllowedClient(clientType) {
 		return "", errorsmod.Wrapf(
 			types.ErrInvalidClientType,
-			"client state type %s is not registered in the allowlist", clientState.ClientType(),
+			"client state type %s is not registered in the allowlist", clientType,
 		)
 	}
 
-	clientID := k.GenerateClientIdentifier(ctx, clientState.ClientType())
-	clientStore := k.ClientStore(ctx, clientID)
+	clientID := k.GenerateClientIdentifier(ctx, clientType)
+
+	clientModule, found := k.router.GetRoute(clientID)
+	if !found {
+		return "", errorsmod.Wrap(types.ErrRouteNotFound, clientID)
+	}
 
-	if err := clientState.Initialize(ctx, k.cdc, clientStore, consensusState); err != nil {
+	if err := clientModule.Initialize(ctx, clientID, clientState, consensusState); err != nil {
 		return "", err
 	}
 
-	if status := k.GetClientStatus(ctx, clientState, clientID); status != exported.Active {
+	if status := k.GetClientStatus(ctx, clientID); status != exported.Active {
 		return "", errorsmod.Wrapf(types.ErrClientNotActive, "cannot create client (%s) with status %s", clientID, status)
 	}
 
-	k.Logger(ctx).Info("client created at height", "client-id", clientID, "height", clientState.GetLatestHeight().String())
+	initialHeight := clientModule.LatestHeight(ctx, clientID)
+	k.Logger(ctx).Info("client created at height", "client-id", clientID, "height", initialHeight.String())
 
 	defer telemetry.IncrCounterWithLabels(
 		[]string{"ibc", "client", "create"},
 		1,
-		[]metrics.Label{telemetry.NewLabel(types.LabelClientType, clientState.ClientType())},
+		[]metrics.Label{telemetry.NewLabel(types.LabelClientType, clientType)},
 	)
 
-	emitCreateClientEvent(ctx, clientID, clientState)
+	emitCreateClientEvent(ctx, clientID, clientType, initialHeight)
 
 	return clientID, nil
 }
 
 // UpdateClient updates the consensus state and the state root from a provided header.
 func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, clientMsg exported.ClientMessage) error {
-	clientState, found := k.GetClientState(ctx, clientID)
-	if !found {
-		return errorsmod.Wrapf(types.ErrClientNotFound, "cannot update client with ID %s", clientID)
+	if status := k.GetClientStatus(ctx, clientID); status != exported.Active {
+		return errorsmod.Wrapf(types.ErrClientNotActive, "cannot update client (%s) with status %s", clientID, status)
 	}
 
-	clientStore := k.ClientStore(ctx, clientID)
+	clientType, _, err := types.ParseClientIdentifier(clientID)
+	if err != nil {
+		return errorsmod.Wrapf(types.ErrClientNotFound, "clientID (%s)", clientID)
+	}
 
-	if status := k.GetClientStatus(ctx, clientState, clientID); status != exported.Active {
-		return errorsmod.Wrapf(types.ErrClientNotActive, "cannot update client (%s) with status %s", clientID, status)
+	clientModule, found := k.router.GetRoute(clientID)
+	if !found {
+		return errorsmod.Wrap(types.ErrRouteNotFound, clientID)
 	}
 
-	if err := clientState.VerifyClientMessage(ctx, k.cdc, clientStore, clientMsg); err != nil {
+	if err := clientModule.VerifyClientMessage(ctx, clientID, clientMsg); err != nil {
 		return err
 	}
 
-	foundMisbehaviour := clientState.CheckForMisbehaviour(ctx, k.cdc, clientStore, clientMsg)
+	foundMisbehaviour := clientModule.CheckForMisbehaviour(ctx, clientID, clientMsg)
 	if foundMisbehaviour {
-		clientState.UpdateStateOnMisbehaviour(ctx, k.cdc, clientStore, clientMsg)
+		clientModule.UpdateStateOnMisbehaviour(ctx, clientID, clientMsg)
 
 		k.Logger(ctx).Info("client frozen due to misbehaviour", "client-id", clientID)
 
 		defer telemetry.IncrCounterWithLabels(
 			[]string{"ibc", "client", "misbehaviour"},
 			1,
 			[]metrics.Label{
-				telemetry.NewLabel(types.LabelClientType, clientState.ClientType()),
+				telemetry.NewLabel(types.LabelClientType, clientType),
 				telemetry.NewLabel(types.LabelClientID, clientID),
 				telemetry.NewLabel(types.LabelMsgType, "update"),
 			},
 		)
 
-		emitSubmitMisbehaviourEvent(ctx, clientID, clientState)
+		emitSubmitMisbehaviourEvent(ctx, clientID, clientType)
 
 		return nil
 	}
 
-	consensusHeights := clientState.UpdateState(ctx, k.cdc, clientStore, clientMsg)
+	consensusHeights := clientModule.UpdateState(ctx, clientID, clientMsg)
 
 	k.Logger(ctx).Info("client state updated", "client-id", clientID, "heights", consensusHeights)
 
 	defer telemetry.IncrCounterWithLabels(
 		[]string{"ibc", "client", "update"},
 		1,
 		[]metrics.Label{
-			telemetry.NewLabel(types.LabelClientType, clientState.ClientType()),
+			telemetry.NewLabel(types.LabelClientType, clientType),
 			telemetry.NewLabel(types.LabelClientID, clientID),
 			telemetry.NewLabel(types.LabelUpdateType, "msg"),
 		},
 	)
 
 	// emitting events in the keeper emits for both begin block and handler client updates
-	emitUpdateClientEvent(ctx, clientID, clientState.ClientType(), consensusHeights, k.cdc, clientMsg)
+	emitUpdateClientEvent(ctx, clientID, clientType, consensusHeights, k.cdc, clientMsg)
 
 	return nil
 }
@@ -125,25 +132,27 @@ func (k Keeper) UpgradeClient(ctx sdk.Context, clientID string, upgradedClient e
 
 	clientStore := k.ClientStore(ctx, clientID)
 
-	if status := k.GetClientStatus(ctx, clientState, clientID); status != exported.Active {
+	if status := k.GetClientStatus(ctx, clientID); status != exported.Active {
 		return errorsmod.Wrapf(types.ErrClientNotActive, "cannot upgrade client (%s) with status %s", clientID, status)
 	}
 
+	// TODO: This code is removed in https://github.com/cosmos/ibc-go/pull/5827
 	// last height of current counterparty chain must be client's latest height
-	lastHeight := clientState.GetLatestHeight()
+	// lastHeight := k.GetLatestHeight(ctx, clientID)
 
-	if !upgradedClient.GetLatestHeight().GT(lastHeight) {
-		return errorsmod.Wrapf(ibcerrors.ErrInvalidHeight, "upgraded client height %s must be at greater than current client height %s",
-			upgradedClient.GetLatestHeight(), lastHeight)
-	}
+	// if !upgradedClient.GetLatestHeight().GT(lastHeight) {
+	// 	return errorsmod.Wrapf(ibcerrors.ErrInvalidHeight, "upgraded client height %s must be at greater than current client height %s",
+	// 		upgradedClient.GetLatestHeight(), lastHeight)
+	// }
 
 	if err := clientState.VerifyUpgradeAndUpdateState(ctx, k.cdc, clientStore,
 		upgradedClient, upgradedConsState, upgradeClientProof, upgradeConsensusStateProof,
 	); err != nil {
 		return errorsmod.Wrapf(err, "cannot upgrade client with ID %s", clientID)
 	}
 
-	k.Logger(ctx).Info("client state upgraded", "client-id", clientID, "height", upgradedClient.GetLatestHeight().String())
+	latestHeight := k.GetLatestHeight(ctx, clientID) // TODO: use clientModule when addressing this func in https://github.com/cosmos/ibc-go/pull/5827
+	k.Logger(ctx).Info("client state upgraded", "client-id", clientID, "height", latestHeight.String())
 
 	defer telemetry.IncrCounterWithLabels(
 		[]string{"ibc", "client", "upgrade"},
@@ -154,7 +163,7 @@ func (k Keeper) UpgradeClient(ctx sdk.Context, clientID string, upgradedClient e
 		},
 	)
 
-	emitUpgradeClientEvent(ctx, clientID, upgradedClient)
+	emitUpgradeClientEvent(ctx, clientID, upgradedClient.ClientType(), latestHeight)
 
 	return nil
 }
@@ -167,34 +176,32 @@ func (k Keeper) UpgradeClient(ctx sdk.Context, clientID string, upgradedClient e
 // the necessary consensus states from the substitute to the subject client
 // store. The substitute must be Active and the subject must not be Active.
 func (k Keeper) RecoverClient(ctx sdk.Context, subjectClientID, substituteClientID string) error {
-	subjectClientState, found := k.GetClientState(ctx, subjectClientID)
-	if !found {
-		return errorsmod.Wrapf(types.ErrClientNotFound, "subject client with ID %s", subjectClientID)
-	}
-
-	subjectClientStore := k.ClientStore(ctx, subjectClientID)
-
-	if status := k.GetClientStatus(ctx, subjectClientState, subjectClientID); status == exported.Active {
+	if status := k.GetClientStatus(ctx, subjectClientID); status == exported.Active {
 		return errorsmod.Wrapf(types.ErrInvalidRecoveryClient, "cannot recover %s subject client", exported.Active)
 	}
 
-	substituteClientState, found := k.GetClientState(ctx, substituteClientID)
-	if !found {
-		return errorsmod.Wrapf(types.ErrClientNotFound, "substitute client with ID %s", substituteClientID)
+	if status := k.GetClientStatus(ctx, substituteClientID); status != exported.Active {
+		return errorsmod.Wrapf(types.ErrClientNotActive, "substitute client is not %s, status is %s", exported.Active, status)
 	}
 
-	if subjectClientState.GetLatestHeight().GTE(substituteClientState.GetLatestHeight()) {
-		return errorsmod.Wrapf(types.ErrInvalidHeight, "subject client state latest height is greater or equal to substitute client state latest height (%s >= %s)", subjectClientState.GetLatestHeight(), substituteClientState.GetLatestHeight())
+	clientType, _, err := types.ParseClientIdentifier(subjectClientID)
+	if err != nil {
+		return errorsmod.Wrapf(types.ErrClientNotFound, "clientID (%s)", subjectClientID)
 	}
 
-	substituteClientStore := k.ClientStore(ctx, substituteClientID)
+	clientModule, found := k.router.GetRoute(subjectClientID)
+	if !found {
+		return errorsmod.Wrap(types.ErrRouteNotFound, subjectClientID)
+	}
 
-	if status := k.GetClientStatus(ctx, substituteClientState, substituteClientID); status != exported.Active {
-		return errorsmod.Wrapf(types.ErrClientNotActive, "substitute client is not %s, status is %s", exported.Active, status)
+	subjectLatestHeight := clientModule.LatestHeight(ctx, subjectClientID)
+	substituteLatestHeight := clientModule.LatestHeight(ctx, substituteClientID)
+	if subjectLatestHeight.GTE(substituteLatestHeight) {
+		return errorsmod.Wrapf(types.ErrInvalidHeight, "subject client state latest height is greater or equal to substitute client state latest height (%s >= %s)", subjectLatestHeight, substituteLatestHeight)
 	}
 
-	if err := subjectClientState.CheckSubstituteAndUpdateState(ctx, k.cdc, subjectClientStore, substituteClientStore, substituteClientState); err != nil {
-		return errorsmod.Wrap(err, "failed to validate substitute client")
+	if err := clientModule.RecoverClient(ctx, subjectClientID, substituteClientID); err != nil {
+		return err
 	}
 
 	k.Logger(ctx).Info("client recovered", "client-id", subjectClientID)
@@ -203,14 +210,14 @@ func (k Keeper) RecoverClient(ctx sdk.Context, subjectClientID, substituteClient
 		[]string{"ibc", "client", "update"},
 		1,
 		[]metrics.Label{
-			telemetry.NewLabel(types.LabelClientType, substituteClientState.ClientType()),
+			telemetry.NewLabel(types.LabelClientType, clientType),
 			telemetry.NewLabel(types.LabelClientID, subjectClientID),
 			telemetry.NewLabel(types.LabelUpdateType, "recovery"),
 		},
 	)
 
 	// emitting events in the keeper for recovering clients
-	emitRecoverClientEvent(ctx, subjectClientID, substituteClientState.ClientType())
+	emitRecoverClientEvent(ctx, subjectClientID, clientType)
 
 	return nil
 }