Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Implement RegisterServices for CoreAPI in module manager #15133

Merged
merged 26 commits into from
Mar 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
33336d2
feat: Implement RegisterServices for CoreAPI modules
facundomedica Feb 23, 2023
66d05dd
suggestions from Aaron
facundomedica Feb 23, 2023
6a45294
return err
facundomedica Feb 23, 2023
5067402
fix autocli
facundomedica Feb 23, 2023
71963fa
Merge branch 'main' into facu/coreapi-service-2
facundomedica Feb 23, 2023
0b66d23
Merge branch 'main' into facu/coreapi-service-2
facundomedica Feb 23, 2023
66c71ef
Merge branch 'main' into facu/coreapi-service-2
tac0turtle Feb 24, 2023
223cc26
Merge branch 'main' into facu/coreapi-service-2
facundomedica Feb 24, 2023
0fd5a01
Merge branch 'main' into facu/coreapi-service-2
facundomedica Feb 27, 2023
51722d1
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica Feb 28, 2023
550aed9
update to core v0.6.0
facundomedica Feb 28, 2023
24064ca
Merge branch 'facu/coreapi-service-2' of https://github.com/cosmos/co…
facundomedica Feb 28, 2023
fe96c15
go mod tidy simapp
facundomedica Feb 28, 2023
109bd53
go mod tidy tests
facundomedica Feb 28, 2023
4edb500
update to use appmodule.HasServices when needed
facundomedica Feb 28, 2023
55df21d
update to use appmodule.HasServices when needed
facundomedica Feb 28, 2023
a5dd4c1
Merge branch 'main' into facu/coreapi-service-2
facundomedica Feb 28, 2023
90a6226
Merge branch 'main' into facu/coreapi-service-2
facundomedica Mar 1, 2023
d7e3d9c
Merge branch 'main' into facu/coreapi-service-2
facundomedica Mar 1, 2023
49c3a05
Merge branch 'main' into facu/coreapi-service-2
facundomedica Mar 2, 2023
2275d6c
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica Mar 2, 2023
4e86967
de-duplicate code in autocli + correct implementation of the configur…
facundomedica Mar 2, 2023
62b2989
Merge branch 'facu/coreapi-service-2' of https://github.com/cosmos/co…
facundomedica Mar 2, 2023
849416b
Merge branch 'main' into facu/coreapi-service-2
facundomedica Mar 2, 2023
eff5ec5
cl++
facundomedica Mar 2, 2023
4a348b2
Merge branch 'facu/coreapi-service-2' of https://github.com/cosmos/co…
facundomedica Mar 2, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. `>`.
Expand Down
5 changes: 4 additions & 1 deletion runtime/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
87 changes: 66 additions & 21 deletions runtime/services/autocli.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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,
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can reduce code duplication a lot here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, I've also did the correct implementation of RegisterServices in the autocli configurator. We should be good to merge now 👌

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, I've also did the correct implementation of RegisterServices in the autocli configurator. We should be good to merge now 👌


if haveServices {
moduleOptions[modName] = modOptions
}
}
return moduleOptions
}
Expand All @@ -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 }
Expand All @@ -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
Expand Down
5 changes: 4 additions & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 8 additions & 0 deletions simapp/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
53 changes: 45 additions & 8 deletions types/module/configurator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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.
Expand Down Expand Up @@ -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")
}
Expand All @@ -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
Expand Down
11 changes: 11 additions & 0 deletions types/module/core_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
var (
_ AppModuleBasic = coreAppModuleBasicAdapator{}
_ HasGenesis = coreAppModuleBasicAdapator{}
_ HasServices = coreAppModuleBasicAdapator{}
)

// CoreAppModuleBasicAdaptor wraps the core API module as an AppModule that this version
Expand Down Expand Up @@ -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)
}
}
}
19 changes: 16 additions & 3 deletions types/module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
Loading