refactor: simplify 02-client router funcs

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

@@ -22,7 +22,7 @@ func (k *Keeper) CreateClient(ctx sdk.Context, clientType string, clientState, c
 
 	clientID := k.GenerateClientIdentifier(ctx, clientType)
 
-	clientModule, err := k.getLightClientModule(ctx, clientID)
+	clientModule, err := k.Route(ctx, clientID)
 	if err != nil {
 		return "", err
 	}
@@ -39,7 +39,6 @@ func (k *Keeper) CreateClient(ctx sdk.Context, clientType string, clientState, c
 	k.Logger(ctx).Info("client created at height", "client-id", clientID, "height", initialHeight.String())
 
 	defer telemetry.ReportCreateClient(clientType)
-
 	emitCreateClientEvent(ctx, clientID, clientType, initialHeight)
 
 	return clientID, nil
@@ -51,12 +50,7 @@ func (k *Keeper) UpdateClient(ctx sdk.Context, clientID string, clientMsg export
 		return errorsmod.Wrapf(types.ErrClientNotActive, "cannot update client (%s) with status %s", clientID, status)
 	}
 
-	clientType, _, err := types.ParseClientIdentifier(clientID)
-	if err != nil {
-		return errorsmod.Wrapf(err, "unable to parse client identifier %s", clientID)
-	}
-
-	clientModule, err := k.getLightClientModule(ctx, clientID)
+	clientModule, err := k.Route(ctx, clientID)
 	if err != nil {
 		return err
 	}
@@ -71,8 +65,8 @@ func (k *Keeper) UpdateClient(ctx sdk.Context, clientID string, clientMsg export
 
 		k.Logger(ctx).Info("client frozen due to misbehaviour", "client-id", clientID)
 
+		clientType := types.MustParseClientIdentifier(clientID)
 		defer telemetry.ReportUpdateClient(foundMisbehaviour, clientType, clientID)
-
 		emitSubmitMisbehaviourEvent(ctx, clientID, clientType)
 
 		return nil
@@ -82,9 +76,8 @@ func (k *Keeper) UpdateClient(ctx sdk.Context, clientID string, clientMsg export
 
 	k.Logger(ctx).Info("client state updated", "client-id", clientID, "heights", consensusHeights)
 
+	clientType := types.MustParseClientIdentifier(clientID)
 	defer telemetry.ReportUpdateClient(foundMisbehaviour, clientType, clientID)
-
-	// emitting events in the keeper emits for both begin block and handler client updates
 	emitUpdateClientEvent(ctx, clientID, clientType, consensusHeights, k.cdc, clientMsg)
 
 	return nil
@@ -101,12 +94,7 @@ func (k *Keeper) UpgradeClient(
 		return errorsmod.Wrapf(types.ErrClientNotActive, "cannot upgrade client (%s) with status %s", clientID, status)
 	}
 
-	clientType, _, err := types.ParseClientIdentifier(clientID)
-	if err != nil {
-		return errorsmod.Wrapf(err, "unable to parse client identifier %s", clientID)
-	}
-
-	clientModule, err := k.getLightClientModule(ctx, clientID)
+	clientModule, err := k.Route(ctx, clientID)
 	if err != nil {
 		return err
 	}
@@ -118,8 +106,8 @@ func (k *Keeper) UpgradeClient(
 	latestHeight := clientModule.LatestHeight(ctx, clientID)
 	k.Logger(ctx).Info("client state upgraded", "client-id", clientID, "height", latestHeight.String())
 
+	clientType := types.MustParseClientIdentifier(clientID)
 	defer telemetry.ReportUpgradeClient(clientType, clientID)
-
 	emitUpgradeClientEvent(ctx, clientID, clientType, latestHeight)
 
 	return nil
@@ -139,13 +127,8 @@ func (k *Keeper) RecoverClient(ctx sdk.Context, subjectClientID, substituteClien
 		return errorsmod.Wrapf(types.ErrClientNotActive, "substitute client is not %s, status is %s", exported.Active, status)
 	}
 
-	clientType, _, err := types.ParseClientIdentifier(subjectClientID)
+	clientModule, err := k.Route(ctx, subjectClientID)
 	if err != nil {
-		return errorsmod.Wrapf(types.ErrClientNotFound, "clientID (%s)", subjectClientID)
-	}
-
-	clientModule, found := k.router.GetRoute(subjectClientID)
-	if !found {
 		return errorsmod.Wrap(types.ErrRouteNotFound, subjectClientID)
 	}
 
@@ -161,9 +144,8 @@ func (k *Keeper) RecoverClient(ctx sdk.Context, subjectClientID, substituteClien
 
 	k.Logger(ctx).Info("client recovered", "client-id", subjectClientID)
 
+	clientType := types.MustParseClientIdentifier(subjectClientID)
 	defer telemetry.ReportRecoverClient(clientType, subjectClientID)
-
-	// emitting events in the keeper for recovering clients
 	emitRecoverClientEvent(ctx, subjectClientID, clientType)
 
 	return nil

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

@@ -571,7 +571,7 @@ func (suite *KeeperTestSuite) TestRecoverClient() {
 			func() {
 				subject = ibctesting.InvalidID
 			},
-			clienttypes.ErrClientNotFound,
+			clienttypes.ErrRouteNotFound,
 		},
 		{
 			"subject is Active",

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

@@ -383,9 +383,9 @@ func (q *queryServer) VerifyMembership(c context.Context, req *types.QueryVerify
 		ctx.GasMeter().ConsumeGas(cachedCtx.GasMeter().GasConsumed(), "verify membership query")
 	}()
 
-	clientModule, found := q.Route(req.ClientId)
-	if !found {
-		return nil, status.Error(codes.NotFound, req.ClientId)
+	clientModule, err := q.Route(ctx, req.ClientId)
+	if err != nil {
+		return nil, status.Error(codes.NotFound, err.Error())
 	}
 
 	if clientStatus := q.GetClientStatus(ctx, req.ClientId); clientStatus != exported.Active {

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

@@ -906,7 +906,7 @@ func (suite *KeeperTestSuite) TestQueryVerifyMembershipProof() {
 			errors.New(wasmClientID),
 		},
 		{
-			"client not active",
+			"client type not allowed",
 			func() {
 				params := types.NewParams("") // disable all clients
 				suite.chainA.GetSimApp().GetIBCKeeper().ClientKeeper.SetParams(suite.chainA.GetContext(), params)
@@ -919,7 +919,7 @@ func (suite *KeeperTestSuite) TestQueryVerifyMembershipProof() {
 					Value:       []byte{0x01},
 				}
 			},
-			types.ErrClientNotActive,
+			types.ErrInvalidClientType,
 		},
 	}
 

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

@@ -72,18 +72,25 @@ func (k *Keeper) GetStoreProvider() exported.ClientStoreProvider {
 }
 
 // Route returns the light client module for the given client identifier.
-func (k *Keeper) Route(clientID string) (exported.LightClientModule, bool) {
-	return k.router.GetRoute(clientID)
-}
+func (k *Keeper) Route(ctx sdk.Context, clientID string) (exported.LightClientModule, error) {
+	clientType, _, err := types.ParseClientIdentifier(clientID)
+	if err != nil {
+		return nil, errorsmod.Wrapf(err, "unable to parse client identifier %s", clientID)
+	}
 
-// UpdateLocalhostClient updates the 09-localhost client to the latest block height and chain ID.
-func (k *Keeper) UpdateLocalhostClient(ctx sdk.Context, clientState exported.ClientState) []exported.Height {
-	clientModule, found := k.router.GetRoute(exported.LocalhostClientID)
+	if !k.GetParams(ctx).IsAllowedClient(clientType) {
+		return nil, errorsmod.Wrapf(
+			types.ErrInvalidClientType,
+			"client (%s) type %s is not in the allowed client list", clientID, clientType,
+		)
+	}
+
+	clientModule, found := k.router.GetRoute(clientType)
 	if !found {
-		panic(errorsmod.Wrap(types.ErrRouteNotFound, exported.LocalhostClientID))
+		return nil, errorsmod.Wrap(types.ErrRouteNotFound, clientID)
 	}
 
-	return clientModule.UpdateState(ctx, exported.LocalhostClientID, nil)
+	return clientModule, nil
 }
 
 // SetConsensusHost sets a custom ConsensusHost for self client state and consensus state validation.
@@ -303,7 +310,7 @@ func (k *Keeper) HasClientConsensusState(ctx sdk.Context, clientID string, heigh
 
 // GetLatestClientConsensusState gets the latest ConsensusState stored for a given client
 func (k *Keeper) GetLatestClientConsensusState(ctx sdk.Context, clientID string) (exported.ConsensusState, bool) {
-	clientModule, err := k.getLightClientModule(ctx, clientID)
+	clientModule, err := k.Route(ctx, clientID)
 	if err != nil {
 		return nil, false
 	}
@@ -328,7 +335,7 @@ func (k *Keeper) ValidateSelfClient(ctx sdk.Context, clientState exported.Client
 
 // VerifyMembershipProof retrieves the light client module for the clientID and verifies the proof of the existence of a key-value pair at a specified height.
 func (k *Keeper) VerifyMembershipProof(ctx sdk.Context, clientID string, height exported.Height, delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, path exported.Path, value []byte) error {
-	clientModule, err := k.getLightClientModule(ctx, clientID)
+	clientModule, err := k.Route(ctx, clientID)
 	if err != nil {
 		return err
 	}
@@ -338,7 +345,7 @@ func (k *Keeper) VerifyMembershipProof(ctx sdk.Context, clientID string, height
 
 // VerifyNonMembership retrieves the light client module for the clientID and verifies the absence of a given key at a specified height.
 func (k *Keeper) VerifyNonMembership(ctx sdk.Context, clientID string, height exported.Height, delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, path exported.Path) error {
-	clientModule, err := k.getLightClientModule(ctx, clientID)
+	clientModule, err := k.Route(ctx, clientID)
 	if err != nil {
 		return err
 	}
@@ -411,7 +418,7 @@ func (k *Keeper) ClientStore(ctx sdk.Context, clientID string) storetypes.KVStor
 // GetClientStatus returns the status for a client state  given a client identifier. If the client type is not in the allowed
 // clients param field, Unauthorized is returned, otherwise the client state status is returned.
 func (k *Keeper) GetClientStatus(ctx sdk.Context, clientID string) exported.Status {
-	clientModule, err := k.getLightClientModule(ctx, clientID)
+	clientModule, err := k.Route(ctx, clientID)
 	if err != nil {
 		return exported.Unauthorized
 	}
@@ -422,7 +429,7 @@ func (k *Keeper) GetClientStatus(ctx sdk.Context, clientID string) exported.Stat
 // GetClientLatestHeight returns the latest height of a client state for a given client identifier. If the client type is not in the allowed
 // clients param field, a zero value height is returned, otherwise the client state latest height is returned.
 func (k *Keeper) GetClientLatestHeight(ctx sdk.Context, clientID string) types.Height {
-	clientModule, err := k.getLightClientModule(ctx, clientID)
+	clientModule, err := k.Route(ctx, clientID)
 	if err != nil {
 		return types.ZeroHeight()
 	}
@@ -437,7 +444,7 @@ func (k *Keeper) GetClientLatestHeight(ctx sdk.Context, clientID string) types.H
 
 // GetClientTimestampAtHeight returns the timestamp in nanoseconds of the consensus state at the given height.
 func (k *Keeper) GetClientTimestampAtHeight(ctx sdk.Context, clientID string, height exported.Height) (uint64, error) {
-	clientModule, err := k.getLightClientModule(ctx, clientID)
+	clientModule, err := k.Route(ctx, clientID)
 	if err != nil {
 		return 0, err
 	}
@@ -494,25 +501,3 @@ func (k *Keeper) ScheduleIBCSoftwareUpgrade(ctx sdk.Context, plan upgradetypes.P
 
 	return nil
 }
-
-// getLightClientModule checks that the clientType of clientID is allowed and returns the corresponding light client module
-func (k *Keeper) getLightClientModule(ctx sdk.Context, clientID string) (exported.LightClientModule, error) {
-	clientType, _, err := types.ParseClientIdentifier(clientID)
-	if err != nil {
-		return nil, errorsmod.Wrapf(err, "unable to parse client identifier %s", clientID)
-	}
-
-	if !k.GetParams(ctx).IsAllowedClient(clientType) {
-		return nil, errorsmod.Wrapf(
-			types.ErrInvalidClientType,
-			"client (%s) type %s is not in the allowed client list", clientID, clientType,
-		)
-	}
-
-	clientModule, found := k.router.GetRoute(clientID)
-	if !found {
-		return nil, errorsmod.Wrap(types.ErrRouteNotFound, clientID)
-	}
-
-	return clientModule, nil
-}

modules/core/02-client/types/keys.go

@@ -80,3 +80,14 @@ func ParseClientIdentifier(clientID string) (string, uint64, error) {
 
 	return clientType, sequence, nil
 }
+
+// MustParseClientIdentifier parses the client type from the provided client identifier.
+// If an invalid client identifier is provided this function will panic.
+func MustParseClientIdentifier(clientID string) string {
+	clientType, _, err := ParseClientIdentifier(clientID)
+	if err != nil {
+		panic(err)
+	}
+
+	return clientType
+}

modules/core/02-client/types/router.go

@@ -42,14 +42,8 @@ func (rtr *Router) HasRoute(clientType string) bool {
 	return ok
 }
 
-// GetRoute returns the LightClientModule registered for the client type
-// associated with the clientID.
-func (rtr *Router) GetRoute(clientID string) (exported.LightClientModule, bool) {
-	clientType, _, err := ParseClientIdentifier(clientID)
-	if err != nil {
-		return nil, false
-	}
-
+// GetRoute returns the LightClientModule registered for the provided client type or false otherwise.
+func (rtr *Router) GetRoute(clientType string) (exported.LightClientModule, bool) {
 	if !rtr.HasRoute(clientType) {
 		return nil, false
 	}

modules/core/02-client/types/router_test.go

@@ -76,11 +76,7 @@ func (suite *TypesTestSuite) TestAddRoute() {
 }
 
 func (suite *TypesTestSuite) TestHasGetRoute() {
-	var (
-		err        error
-		clientType string
-		clientID   string
-	)
+	var clientType string
 
 	testCases := []struct {
 		name     string
@@ -90,25 +86,20 @@ func (suite *TypesTestSuite) TestHasGetRoute() {
 		{
 			"success",
 			func() {
-				clientID = fmt.Sprintf("%s-%d", exported.Tendermint, 0)
-				clientType, _, err = types.ParseClientIdentifier(clientID)
-				suite.Require().NoError(err)
+				clientType = exported.Tendermint
 			},
 			true,
 		},
 		{
 			"failure: route does not exist",
 			func() {
-				clientID = "06-solomachine-0"
-				clientType, _, err = types.ParseClientIdentifier(clientID)
-				suite.Require().NoError(err)
+				clientType = exported.Solomachine
 			},
 			false,
 		},
 		{
 			"failure: invalid client ID",
 			func() {
-				clientID = "invalid-client-id"
 				clientType = "invalid-client-type"
 			},
 			false,
@@ -130,7 +121,7 @@ func (suite *TypesTestSuite) TestHasGetRoute() {
 			tc.malleate()
 
 			hasRoute := router.HasRoute(clientType)
-			route, ok := router.GetRoute(clientID)
+			route, ok := router.GetRoute(clientType)
 
 			if tc.expPass {
 				suite.Require().True(hasRoute)

modules/core/ante/ante.go

@@ -164,9 +164,9 @@ func (rrd RedundantRelayDecorator) updateClientCheckTx(ctx sdk.Context, msg *cli
 		return errorsmod.Wrapf(clienttypes.ErrClientNotActive, "cannot update client (%s) with status %s", msg.ClientId, status)
 	}
 
-	clientModule, found := rrd.k.ClientKeeper.Route(msg.ClientId)
-	if !found {
-		return errorsmod.Wrap(clienttypes.ErrRouteNotFound, msg.ClientId)
+	clientModule, err := rrd.k.ClientKeeper.Route(ctx, msg.ClientId)
+	if err != nil {
+		return err
 	}
 
 	if !ctx.IsReCheckTx() {

modules/core/keeper/msg_server_test.go

@@ -247,7 +247,7 @@ func (suite *KeeperTestSuite) TestRecoverClient() {
 			func() {
 				msg.SubjectClientId = ibctesting.InvalidID
 			},
-			clienttypes.ErrClientNotFound,
+			clienttypes.ErrRouteNotFound,
 		},
 	}