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 9 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
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
}
5 changes: 5 additions & 0 deletions runtime/services/autocli.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ type autocliConfigurator struct {
queryServer autocliServiceRegistrar
}

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 +87,9 @@ func (a *autocliConfigurator) RegisterMigration(string, uint64, module.Migration
return nil
}

func (*autocliConfigurator) RegisterService(sd *grpc.ServiceDesc, ss interface{}) {}
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
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
8 changes: 8 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,10 @@ 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 {
module.RegisterServices(cfg)
}
}
16 changes: 13 additions & 3 deletions types/module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,12 +368,22 @@ 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 {
module.RegisterServices(cfg)
}

if cfg.Error() != nil {
return cfg.Error()
}
}

return nil
}

// InitGenesis performs init genesis functionality for modules. Exactly one
Expand Down Expand Up @@ -586,9 +596,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
21 changes: 17 additions & 4 deletions types/module/module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@ import (
"github.com/golang/mock/gomock"
"github.com/spf13/cobra"
"github.com/stretchr/testify/require"
"google.golang.org/grpc"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/testutil/mock"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
)

var errFoo = errors.New("dummy")
Expand Down Expand Up @@ -155,7 +157,7 @@ func TestManager_RegisterQueryServices(t *testing.T) {

mockAppModule1 := mock.NewMockAppModuleWithAllExtensions(mockCtrl)
mockAppModule2 := mock.NewMockAppModuleWithAllExtensions(mockCtrl)
mockAppModule3 := mock.NewMockCoreAppModule(mockCtrl)
mockAppModule3 := MockCoreAppModule{}
mockAppModule1.EXPECT().Name().Times(2).Return("module1")
mockAppModule2.EXPECT().Name().Times(2).Return("module2")
// TODO: This is not working for Core API modules yet
Expand All @@ -164,14 +166,17 @@ func TestManager_RegisterQueryServices(t *testing.T) {
require.Equal(t, 3, len(mm.Modules))

msgRouter := mock.NewMockServer(mockCtrl)
msgRouter.EXPECT().RegisterService(gomock.Any(), gomock.Any()).Times(1)
queryRouter := mock.NewMockServer(mockCtrl)
queryRouter.EXPECT().RegisterService(gomock.Any(), gomock.Any()).Times(1)

interfaceRegistry := types.NewInterfaceRegistry()
cdc := codec.NewProtoCodec(interfaceRegistry)
cfg := module.NewConfigurator(cdc, msgRouter, queryRouter)
mockAppModule1.EXPECT().RegisterServices(cfg).Times(1)
mockAppModule2.EXPECT().RegisterServices(cfg).Times(1)

mm.RegisterServices(cfg)
require.NotPanics(t, func() { mm.RegisterServices(cfg) })
}

func TestManager_InitGenesis(t *testing.T) {
Expand Down Expand Up @@ -484,6 +489,13 @@ func TestCoreAPIManager_EndBlock(t *testing.T) {
// MockCoreAppModule allows us to test functions like DefaultGenesis
type MockCoreAppModule struct{}

// RegisterServices implements appmodule.HasServices
func (MockCoreAppModule) RegisterServices(reg grpc.ServiceRegistrar) {
// Use Auth's service definitions as a placeholder
authtypes.RegisterQueryServer(reg, &authtypes.UnimplementedQueryServer{})
authtypes.RegisterMsgServer(reg, &authtypes.UnimplementedMsgServer{})
}

func (MockCoreAppModule) IsOnePerModuleType() {}
func (MockCoreAppModule) IsAppModule() {}
func (MockCoreAppModule) DefaultGenesis(target appmodule.GenesisTarget) error {
Expand Down Expand Up @@ -523,6 +535,7 @@ func (MockCoreAppModule) ExportGenesis(ctx context.Context, target appmodule.Gen
}

var (
_ appmodule.AppModule = MockCoreAppModule{}
_ appmodule.HasGenesis = MockCoreAppModule{}
_ appmodule.AppModule = MockCoreAppModule{}
_ appmodule.HasGenesis = MockCoreAppModule{}
_ appmodule.HasServices = MockCoreAppModule{}
)