feat: Implement RegisterServices for CoreAPI in module manager (#15133)

Co-authored-by: Marko <marbar3778@yahoo.com>

CHANGELOG.md

@@ -39,6 +39,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Features
 
+* (core) [#15133](https://github.com/cosmos/cosmos-sdk/pull/15133) Implement RegisterServices in the module manager.
 * (x/gov) [#14373](https://github.com/cosmos/cosmos-sdk/pull/14057) Add new proto field `constitution` of type `string` to gov module genesis state, which allows chain builders to lay a strong foundation by specifying purpose.
 * (x/genutil) [#15301](https://github.com/cosmos/cosmos-sdk/pull/15031) Add application genesis. The genesis is now entirely managed by the application and passed to CometBFT at note instantiation. Functions that were taking a `cmttypes.GenesisDoc{}` now takes a `genutiltypes.AppGenesis{}`.
 * (cli) [#14659](https://github.com/cosmos/cosmos-sdk/pull/14659) Added ability to query blocks by events with queries directly passed to Tendermint, which will allow for full query operator support, e.g. `>`.

runtime/builder.go

@@ -44,7 +44,10 @@ func (a *AppBuilder) Build(
 
 	a.app.BaseApp = bApp
 	a.app.configurator = module.NewConfigurator(a.app.cdc, a.app.MsgServiceRouter(), a.app.GRPCQueryRouter())
-	a.app.ModuleManager.RegisterServices(a.app.configurator)
+	err := a.app.ModuleManager.RegisterServices(a.app.configurator)
+	if err != nil {
+		panic(err)
+	}
 
 	return a.app
 }

runtime/services/autocli.go

@@ -4,8 +4,14 @@ import (
 	"context"
 
 	autocliv1 "cosmossdk.io/api/cosmos/autocli/v1"
+	cosmosmsg "cosmossdk.io/api/cosmos/msg/v1"
+	"cosmossdk.io/core/appmodule"
 	gogogrpc "github.com/cosmos/gogoproto/grpc"
+	"github.com/cosmos/gogoproto/proto"
 	"google.golang.org/grpc"
+	protobuf "google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/reflect/protoreflect"
+	"google.golang.org/protobuf/reflect/protoregistry"
 
 	"github.com/cosmos/cosmos-sdk/types/module"
 )
@@ -35,32 +41,48 @@ func ExtractAutoCLIOptions(appModules map[string]interface{}) map[string]*autocl
 			AutoCLIOptions() *autocliv1.ModuleOptions
 		}); ok {
 			moduleOptions[modName] = autoCliMod.AutoCLIOptions()
-		} else if mod, ok := mod.(module.HasServices); ok {
-			// try to auto-discover options based on the last msg and query
-			// services registered for the module
-			cfg := &autocliConfigurator{}
+			continue
+		}
+
+		cfg := &autocliConfigurator{}
+
+		// try to auto-discover options based on the last msg and query
+		// services registered for the module
+		if mod, ok := mod.(module.HasServices); ok {
 			mod.RegisterServices(cfg)
-			modOptions := &autocliv1.ModuleOptions{}
-			haveServices := false
-
-			if cfg.msgServer.serviceName != "" {
-				haveServices = true
-				modOptions.Tx = &autocliv1.ServiceCommandDescriptor{
-					Service: cfg.msgServer.serviceName,
-				}
+		}
+
+		if mod, ok := mod.(appmodule.HasServices); ok {
+			err := mod.RegisterServices(cfg)
+			if err != nil {
+				panic(err)
 			}
+		}
+
+		// check for errors in the configurator
+		if cfg.Error() != nil {
+			panic(cfg.Error())
+		}
 
-			if cfg.queryServer.serviceName != "" {
-				haveServices = true
-				modOptions.Query = &autocliv1.ServiceCommandDescriptor{
-					Service: cfg.queryServer.serviceName,
-				}
+		haveServices := false
+		modOptions := &autocliv1.ModuleOptions{}
+		if cfg.msgServer.serviceName != "" {
+			haveServices = true
+			modOptions.Tx = &autocliv1.ServiceCommandDescriptor{
+				Service: cfg.msgServer.serviceName,
 			}
+		}
 
-			if haveServices {
-				moduleOptions[modName] = modOptions
+		if cfg.queryServer.serviceName != "" {
+			haveServices = true
+			modOptions.Query = &autocliv1.ServiceCommandDescriptor{
+				Service: cfg.queryServer.serviceName,
 			}
 		}
+
+		if haveServices {
+			moduleOptions[modName] = modOptions
+		}
 	}
 	return moduleOptions
 }
@@ -73,10 +95,14 @@ func (a AutoCLIQueryService) AppOptions(context.Context, *autocliv1.AppOptionsRe
 
 // autocliConfigurator allows us to call RegisterServices and introspect the services
 type autocliConfigurator struct {
-	msgServer   autocliServiceRegistrar
-	queryServer autocliServiceRegistrar
+	msgServer     autocliServiceRegistrar
+	queryServer   autocliServiceRegistrar
+	registryCache *protoregistry.Files
+	err           error
 }
 
+var _ module.Configurator = &autocliConfigurator{}
+
 func (a *autocliConfigurator) MsgServer() gogogrpc.Server { return &a.msgServer }
 
 func (a *autocliConfigurator) QueryServer() gogogrpc.Server { return &a.queryServer }
@@ -85,6 +111,25 @@ func (a *autocliConfigurator) RegisterMigration(string, uint64, module.Migration
 	return nil
 }
 
+func (a *autocliConfigurator) RegisterService(sd *grpc.ServiceDesc, ss interface{}) {
+	if a.registryCache == nil {
+		a.registryCache, a.err = proto.MergedRegistry()
+	}
+
+	desc, err := a.registryCache.FindDescriptorByName(protoreflect.FullName(sd.ServiceName))
+	if err != nil {
+		a.err = err
+		return
+	}
+
+	if protobuf.HasExtension(desc.Options(), cosmosmsg.E_Service) {
+		a.msgServer.RegisterService(sd, ss)
+	} else {
+		a.queryServer.RegisterService(sd, ss)
+	}
+}
+func (a *autocliConfigurator) Error() error { return nil }
+
 // autocliServiceRegistrar is used to capture the service name for registered services
 type autocliServiceRegistrar struct {
 	serviceName string

simapp/app.go

@@ -453,7 +453,10 @@ func NewSimApp(
 
 	app.ModuleManager.RegisterInvariants(app.CrisisKeeper)
 	app.configurator = module.NewConfigurator(app.appCodec, app.MsgServiceRouter(), app.GRPCQueryRouter())
-	app.ModuleManager.RegisterServices(app.configurator)
+	err := app.ModuleManager.RegisterServices(app.configurator)
+	if err != nil {
+		panic(err)
+	}
 
 	// RegisterUpgradeHandlers is used for registering any on-chain upgrades.
 	// Make sure it's called after `app.ModuleManager` and `app.configurator` are set.

simapp/app_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/require"
 
+	"cosmossdk.io/core/appmodule"
 	"cosmossdk.io/log"
 	feegrantmodule "cosmossdk.io/x/feegrant/module"
 	"github.com/cosmos/cosmos-sdk/baseapp"
@@ -97,6 +98,13 @@ func TestRunMigrations(t *testing.T) {
 		if mod, ok := mod.(module.HasServices); ok {
 			mod.RegisterServices(configurator)
 		}
+
+		if mod, ok := mod.(appmodule.HasServices); ok {
+			err := mod.RegisterServices(configurator)
+			require.NoError(t, err)
+		}
+
+		require.NoError(t, configurator.Error())
 	}
 
 	// Initialize the chain

types/module/configurator.go

@@ -3,9 +3,14 @@ package module
 import (
 	"fmt"
 
-	"github.com/cosmos/gogoproto/grpc"
-
+	cosmosmsg "cosmossdk.io/api/cosmos/msg/v1"
 	errorsmod "cosmossdk.io/errors"
+	"github.com/cosmos/gogoproto/grpc"
+	"github.com/cosmos/gogoproto/proto"
+	googlegrpc "google.golang.org/grpc"
+	protobuf "google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/reflect/protoreflect"
+	"google.golang.org/protobuf/reflect/protoregistry"
 
 	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -17,6 +22,11 @@ import (
 // support module object capabilities isolation as described in
 // https://github.com/cosmos/cosmos-sdk/issues/7093
 type Configurator interface {
+	grpc.Server
+
+	// Error returns the last error encountered during RegisterService.
+	Error() error
+
 	// MsgServer returns a grpc.Server instance which allows registering services
 	// that will handle TxBody.messages in transactions. These Msg's WILL NOT
 	// be exposed as gRPC services.
@@ -45,32 +55,59 @@ type configurator struct {
 
 	// migrations is a map of moduleName -> fromVersion -> migration script handler
 	migrations map[string]map[uint64]MigrationHandler
+
+	registryCache *protoregistry.Files
+	err           error
+}
+
+// RegisterService implements the grpc.Server interface.
+func (c *configurator) RegisterService(sd *googlegrpc.ServiceDesc, ss interface{}) {
+	if c.registryCache == nil {
+		c.registryCache, c.err = proto.MergedRegistry()
+	}
+
+	desc, err := c.registryCache.FindDescriptorByName(protoreflect.FullName(sd.ServiceName))
+	if err != nil {
+		c.err = err
+		return
+	}
+
+	if protobuf.HasExtension(desc.Options(), cosmosmsg.E_Service) {
+		c.msgServer.RegisterService(sd, ss)
+	} else {
+		c.queryServer.RegisterService(sd, ss)
+	}
+}
+
+// Error returns the last error encountered during RegisterService.
+func (c *configurator) Error() error {
+	return c.err
 }
 
 // NewConfigurator returns a new Configurator instance
 func NewConfigurator(cdc codec.Codec, msgServer grpc.Server, queryServer grpc.Server) Configurator {
-	return configurator{
+	return &configurator{
 		cdc:         cdc,
 		msgServer:   msgServer,
 		queryServer: queryServer,
 		migrations:  map[string]map[uint64]MigrationHandler{},
 	}
 }
 
-var _ Configurator = configurator{}
+var _ Configurator = &configurator{}
 
 // MsgServer implements the Configurator.MsgServer method
-func (c configurator) MsgServer() grpc.Server {
+func (c *configurator) MsgServer() grpc.Server {
 	return c.msgServer
 }
 
 // QueryServer implements the Configurator.QueryServer method
-func (c configurator) QueryServer() grpc.Server {
+func (c *configurator) QueryServer() grpc.Server {
 	return c.queryServer
 }
 
 // RegisterMigration implements the Configurator.RegisterMigration method
-func (c configurator) RegisterMigration(moduleName string, fromVersion uint64, handler MigrationHandler) error {
+func (c *configurator) RegisterMigration(moduleName string, fromVersion uint64, handler MigrationHandler) error {
 	if fromVersion == 0 {
 		return errorsmod.Wrap(sdkerrors.ErrInvalidVersion, "module migration versions should start at 1")
 	}
@@ -90,7 +127,7 @@ func (c configurator) RegisterMigration(moduleName string, fromVersion uint64, h
 
 // runModuleMigrations runs all in-place store migrations for one given module from a
 // version to another version.
-func (c configurator) runModuleMigrations(ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error {
+func (c *configurator) runModuleMigrations(ctx sdk.Context, moduleName string, fromVersion, toVersion uint64) error {
 	// No-op if toVersion is the initial version or if the version is unchanged.
 	if toVersion <= 1 || fromVersion == toVersion {
 		return nil

types/module/core_module.go

@@ -20,6 +20,7 @@ import (
 var (
 	_ AppModuleBasic = coreAppModuleBasicAdapator{}
 	_ HasGenesis     = coreAppModuleBasicAdapator{}
+	_ HasServices    = coreAppModuleBasicAdapator{}
 )
 
 // CoreAppModuleBasicAdaptor wraps the core API module as an AppModule that this version
@@ -161,3 +162,13 @@ func (c coreAppModuleBasicAdapator) RegisterLegacyAminoCodec(amino *codec.Legacy
 		mod.RegisterLegacyAminoCodec(amino)
 	}
 }
+
+// RegisterServices implements HasServices
+func (c coreAppModuleBasicAdapator) RegisterServices(cfg Configurator) {
+	if module, ok := c.module.(appmodule.HasServices); ok {
+		err := module.RegisterServices(cfg)
+		if err != nil {
+			panic(err)
+		}
+	}
+}

types/module/module.go

@@ -368,12 +368,25 @@ func (m *Manager) RegisterInvariants(ir sdk.InvariantRegistry) {
 }
 
 // RegisterServices registers all module services
-func (m *Manager) RegisterServices(cfg Configurator) {
+func (m *Manager) RegisterServices(cfg Configurator) error {
 	for _, module := range m.Modules {
 		if module, ok := module.(HasServices); ok {
 			module.RegisterServices(cfg)
 		}
+
+		if module, ok := module.(appmodule.HasServices); ok {
+			err := module.RegisterServices(cfg)
+			if err != nil {
+				return err
+			}
+		}
+
+		if cfg.Error() != nil {
+			return cfg.Error()
+		}
 	}
+
+	return nil
 }
 
 // InitGenesis performs init genesis functionality for modules. Exactly one
@@ -586,9 +599,9 @@ type VersionMap map[string]uint64
 //
 // Please also refer to docs/core/upgrade.md for more information.
 func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, fromVM VersionMap) (VersionMap, error) {
-	c, ok := cfg.(configurator)
+	c, ok := cfg.(*configurator)
 	if !ok {
-		return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", configurator{}, cfg)
+		return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", &configurator{}, cfg)
 	}
 	modules := m.OrderMigrations
 	if modules == nil {