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

refactor(server/v2): eager config loading #22267

Merged
merged 55 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
b2072de
begin config rationalization
kocubinski Oct 11, 2024
ecf73cf
the concept is proven
kocubinski Oct 13, 2024
8cb7307
interesting, start command works
kocubinski Oct 14, 2024
3803716
even more clean up
kocubinski Oct 15, 2024
30d1789
genesis export working
kocubinski Oct 15, 2024
23780e2
only build store in app builder
kocubinski Oct 15, 2024
da16c69
wipg
kocubinski Oct 23, 2024
ddfac2b
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/config-rf
kocubinski Oct 23, 2024
9c38263
big progress!
kocubinski Oct 23, 2024
cf96c1f
fix testnets command
kocubinski Oct 23, 2024
d3012f1
fix simapp/v2 tests
kocubinski Oct 23, 2024
9ca9e6b
adapt for runtime/v1
kocubinski Oct 23, 2024
43c10ad
rm dead method
kocubinski Oct 23, 2024
4c93ad7
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/config-rf
kocubinski Oct 23, 2024
3e0ded0
rm Init() method
kocubinski Oct 23, 2024
72b278a
clean up
kocubinski Oct 23, 2024
a3c111f
go mod tidy all
kocubinski Oct 23, 2024
b689605
lint fix
kocubinski Oct 23, 2024
10e2d4b
moar dep fix
kocubinski Oct 23, 2024
b894a32
default comet config
kocubinski Oct 23, 2024
be3a0b3
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/config-rf
kocubinski Oct 24, 2024
fc68979
minor clean up
kocubinski Oct 24, 2024
348028f
refactor
kocubinski Oct 24, 2024
40bfd6b
demo consensus override
kocubinski Oct 24, 2024
68f543f
changelog
kocubinski Oct 24, 2024
bb95533
fix
kocubinski Oct 24, 2024
e4047cc
Merge branch 'main' into kocu/config-rf
kocubinski Oct 25, 2024
0f79666
clean up
kocubinski Oct 25, 2024
df178ab
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/config-rf
kocubinski Oct 25, 2024
9055ee2
Merge branch 'kocu/config-rf' of github.com:cosmos/cosmos-sdk into ko…
kocubinski Oct 25, 2024
d39386d
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/config-rf
kocubinski Oct 25, 2024
dcc3ddc
rm core replaces
kocubinski Oct 25, 2024
17eadc8
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/config-rf
kocubinski Oct 25, 2024
ca06258
use factory pattern instead
kocubinski Oct 25, 2024
20e7ec1
fix factory pattern
kocubinski Oct 25, 2024
94d30f5
rm redundant
kocubinski Oct 25, 2024
ff0fa66
revert .vscode change
kocubinski Oct 25, 2024
9e60b5e
add call
kocubinski Oct 25, 2024
3a83516
rm comment
kocubinski Oct 25, 2024
b21b825
rm verbose
kocubinski Oct 25, 2024
ce56ae9
rm annotation, add helper command to server/v2
kocubinski Oct 28, 2024
4380b02
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/config-rf
kocubinski Oct 28, 2024
d3bb0c3
rf remove redudnancy
kocubinski Oct 28, 2024
c48172d
typos
kocubinski Oct 28, 2024
9addef4
minor cleanup
kocubinski Oct 28, 2024
e9c4005
export two functions
kocubinski Oct 28, 2024
6d18467
add CODEOWNER entry for server/v2
kocubinski Oct 28, 2024
d200520
add comments
kocubinski Oct 28, 2024
13d9771
mv helper fn
kocubinski Oct 28, 2024
1cadeeb
rm helper fn
kocubinski Oct 28, 2024
6472136
fix
kocubinski Oct 28, 2024
9c38df2
grpcgateway fix
kocubinski Oct 28, 2024
7014d0d
rename file
kocubinski Oct 28, 2024
6272619
typos and exports
kocubinski Oct 28, 2024
e7603cf
configurable homedir in ClientCtx, remove global var
kocubinski Oct 28, 2024
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
2 changes: 1 addition & 1 deletion .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
/runtime/v2/ @julienrbrt @hieuvubk @cosmos/sdk-core-dev
/schema/ @aaronc @testinginprod @cosmos/sdk-core-dev
/server/ @cosmos/sdk-core-dev
/server/v2/ @julienrbrt @hieuvubk @cosmos/sdk-core-dev
/server/v2/ @julienrbrt @hieuvubk @kocubinski @cosmos/sdk-core-dev
/server/v2/stf/ @testinginprod @kocubinski @cosmos/sdk-core-dev
/server/v2/appmanager/ @testinginprod @facundomedica @cosmos/sdk-core-dev
/server/v2/cometbft/ @facundomedica @sontrinh16 @cosmos/sdk-core-dev
Expand Down
46 changes: 46 additions & 0 deletions runtime/config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package runtime

import (
"cosmossdk.io/core/server"
"cosmossdk.io/depinject"
)

// ModuleConfigMaps is a map module scoped ConfigMaps
type ModuleConfigMaps map[string]server.ConfigMap

type ModuleConfigMapsInput struct {
depinject.In

ModuleConfigs []server.ModuleConfigMap
DynamicConfig server.DynamicConfig `optional:"true"`
}

// ProvideModuleConfigMaps returns a map of module name to module config map.
// The module config map is a map of flag to value.
func ProvideModuleConfigMaps(in ModuleConfigMapsInput) ModuleConfigMaps {
moduleConfigMaps := make(ModuleConfigMaps)
if in.DynamicConfig == nil {
return moduleConfigMaps
}
for _, moduleConfig := range in.ModuleConfigs {
cfg := moduleConfig.Config
name := moduleConfig.Module
moduleConfigMaps[name] = make(server.ConfigMap)
for flag, df := range cfg {
val := in.DynamicConfig.Get(flag)
if val != nil {
moduleConfigMaps[name][flag] = val
} else {
moduleConfigMaps[name][flag] = df
}
}
}
return moduleConfigMaps
}

func ProvideModuleScopedConfigMap(
key depinject.ModuleKey,
moduleConfigs ModuleConfigMaps,
) server.ConfigMap {
return moduleConfigs[key.Name()]
}
kocubinski marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 2 additions & 0 deletions runtime/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ func init() {
ProvideTransientStoreService,
ProvideModuleManager,
ProvideCometService,
ProvideModuleConfigMaps,
ProvideModuleScopedConfigMap,
),
appconfig.Invoke(SetupAppBuilder),
)
Expand Down
2 changes: 1 addition & 1 deletion runtime/v2/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (a *App[T]) LoadLatestHeight() (uint64, error) {
return a.db.GetLatestVersion()
}

// GetQueryHandlers returns the query handlers.
// QueryHandlers returns the query handlers.
func (a *App[T]) QueryHandlers() map[string]appmodulev2.Handler {
return a.queryHandlers
}
Expand Down
10 changes: 6 additions & 4 deletions runtime/v2/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
type AppBuilder[T transaction.Tx] struct {
app *App[T]
storeBuilder root.Builder
storeConfig *root.Config

// the following fields are used to overwrite the default
branch func(state store.ReaderMap) store.WriterMap
Expand Down Expand Up @@ -82,12 +83,13 @@ func (a *AppBuilder[T]) Build(opts ...AppBuilderOption[T]) (*App[T], error) {
}
}

a.app.db = a.storeBuilder.Get()
if a.app.db == nil {
return nil, fmt.Errorf("storeBuilder did not return a db")
var err error
a.app.db, err = a.storeBuilder.Build(a.app.logger, a.storeConfig)
if err != nil {
return nil, err
}

if err := a.app.moduleManager.RegisterServices(a.app); err != nil {
if err = a.app.moduleManager.RegisterServices(a.app); err != nil {
return nil, err
}

Expand Down
67 changes: 67 additions & 0 deletions runtime/v2/config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package runtime

import (
"strings"

"cosmossdk.io/core/server"
"cosmossdk.io/depinject"
)

// GlobalConfig is a recursive configuration map containing configuration
// key-value pairs parsed from the configuration file, flags, or other
// input sources.
//
// It is aliased to server.ConfigMap so that DI can distinguish between
// module-scoped and global configuration maps. In the DI container `server.ConfigMap`
// objects are module-scoped and `GlobalConfig` is global-scoped.
type GlobalConfig server.ConfigMap
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved

// ModuleConfigMaps is a map module scoped ConfigMaps
type ModuleConfigMaps map[string]server.ConfigMap

// ProvideModuleConfigMaps returns a map of module name to module config map.
// The module config map is a map of flag to value.
func ProvideModuleConfigMaps(
moduleConfigs []server.ModuleConfigMap,
globalConfig GlobalConfig,
) ModuleConfigMaps {
moduleConfigMaps := make(ModuleConfigMaps)
for _, moduleConfig := range moduleConfigs {
cfg := moduleConfig.Config
name := moduleConfig.Module
moduleConfigMaps[name] = make(server.ConfigMap)
for flag, df := range cfg {
m := globalConfig
fetchFlag := flag
// splitting on "." is required to handle nested flags which are defined
// in other modules that are not the current module
// for example: "server.minimum-gas-prices" is defined in the server module
// but required by x/validate
for _, part := range strings.Split(flag, ".") {
kocubinski marked this conversation as resolved.
Show resolved Hide resolved
if maybeMap, ok := m[part]; ok {
innerMap, ok := maybeMap.(map[string]any)
if !ok {
fetchFlag = part
break
}
m = innerMap
} else {
break
}
}
if val, ok := m[fetchFlag]; ok {
moduleConfigMaps[name][flag] = val
} else {
moduleConfigMaps[name][flag] = df
}
}
}
return moduleConfigMaps
}

func ProvideModuleScopedConfigMap(
key depinject.ModuleKey,
moduleConfigs ModuleConfigMaps,
) server.ConfigMap {
return moduleConfigs[key.Name()]
}
kocubinski marked this conversation as resolved.
Show resolved Hide resolved
6 changes: 5 additions & 1 deletion runtime/v2/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ func init() {
ProvideModuleManager[transaction.Tx],
ProvideEnvironment,
ProvideKVService,
ProvideModuleConfigMaps,
ProvideModuleScopedConfigMap,
),
appconfig.Invoke(SetupAppBuilder),
)
Expand All @@ -108,6 +110,7 @@ func ProvideAppBuilder[T transaction.Tx](
interfaceRegistrar registry.InterfaceRegistrar,
amino registry.AminoRegistrar,
storeBuilder root.Builder,
storeConfig *root.Config,
) (
*AppBuilder[T],
*stf.MsgRouterBuilder,
Expand All @@ -134,14 +137,15 @@ func ProvideAppBuilder[T transaction.Tx](
queryHandlers: map[string]appmodulev2.Handler{},
storeLoader: DefaultStoreLoader,
}
appBuilder := &AppBuilder[T]{app: app, storeBuilder: storeBuilder}
appBuilder := &AppBuilder[T]{app: app, storeBuilder: storeBuilder, storeConfig: storeConfig}

return appBuilder, msgRouterBuilder, appModule[T]{app}, protoFiles, protoTypes
}

type AppInputs struct {
depinject.In

StoreConfig *root.Config
Config *runtimev2.Module
AppBuilder *AppBuilder[transaction.Tx]
ModuleManager *MM[transaction.Tx]
Expand Down
53 changes: 32 additions & 21 deletions server/v2/api/grpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"google.golang.org/protobuf/reflect/protoreflect"

appmodulev2 "cosmossdk.io/core/appmodule/v2"
"cosmossdk.io/core/server"
"cosmossdk.io/core/transaction"
"cosmossdk.io/log"
serverv2 "cosmossdk.io/server/v2"
Expand All @@ -42,40 +43,50 @@ type Server[T transaction.Tx] struct {
}

// New creates a new grpc server.
func New[T transaction.Tx](cfgOptions ...CfgOption) *Server[T] {
return &Server[T]{
func New[T transaction.Tx](
logger log.Logger,
interfaceRegistry server.InterfaceRegistry,
queryHandlers map[string]appmodulev2.Handler,
queryable interface {
Query(ctx context.Context, version uint64, msg transaction.Msg) (transaction.Msg, error)
},
cfg server.ConfigMap,
cfgOptions ...CfgOption,
) (*Server[T], error) {
srv := &Server[T]{
cfgOptions: cfgOptions,
}
}

// Init returns a correctly configured and initialized gRPC server.
// Note, the caller is responsible for starting the server.
func (s *Server[T]) Init(appI serverv2.AppI[T], cfg map[string]any, logger log.Logger) error {
serverCfg := s.Config().(*Config)
serverCfg := srv.Config().(*Config)
if len(cfg) > 0 {
if err := serverv2.UnmarshalSubConfig(cfg, s.Name(), &serverCfg); err != nil {
return fmt.Errorf("failed to unmarshal config: %w", err)
if err := serverv2.UnmarshalSubConfig(cfg, srv.Name(), &serverCfg); err != nil {
return nil, fmt.Errorf("failed to unmarshal config: %w", err)
kocubinski marked this conversation as resolved.
Show resolved Hide resolved
}
}
methodsMap := appI.QueryHandlers()

grpcSrv := grpc.NewServer(
grpc.ForceServerCodec(newProtoCodec(appI.InterfaceRegistry()).GRPCCodec()),
grpc.ForceServerCodec(newProtoCodec(interfaceRegistry).GRPCCodec()),
grpc.MaxSendMsgSize(serverCfg.MaxSendMsgSize),
grpc.MaxRecvMsgSize(serverCfg.MaxRecvMsgSize),
grpc.UnknownServiceHandler(
makeUnknownServiceHandler(methodsMap, appI),
),
grpc.UnknownServiceHandler(makeUnknownServiceHandler(queryHandlers, queryable)),
)

// Reflection allows external clients to see what services and methods the gRPC server exposes.
gogoreflection.Register(grpcSrv, slices.Collect(maps.Keys(methodsMap)), logger.With("sub-module", "grpc-reflection"))
gogoreflection.Register(grpcSrv, slices.Collect(maps.Keys(queryHandlers)), logger.With("sub-module", "grpc-reflection"))

s.grpcSrv = grpcSrv
s.config = serverCfg
s.logger = logger.With(log.ModuleKey, s.Name())
srv.grpcSrv = grpcSrv
srv.config = serverCfg
srv.logger = logger.With(log.ModuleKey, srv.Name())

return nil
return srv, nil
}

// NewWithConfigOptions creates a new GRPC server with the provided config options.
// It is *not* a fully functional server (since it has been created without dependencies)
// The returned server should only be used to get and set configuration.
func NewWithConfigOptions[T transaction.Tx](opts ...CfgOption) *Server[T] {
return &Server[T]{
cfgOptions: opts,
}
}

func (s *Server[T]) StartCmdFlags() *pflag.FlagSet {
Expand Down Expand Up @@ -186,7 +197,7 @@ func (s *Server[T]) Config() any {
return s.config
}

func (s *Server[T]) Start(ctx context.Context) error {
func (s *Server[T]) Start(context.Context) error {
kocubinski marked this conversation as resolved.
Show resolved Hide resolved
if !s.config.Enable {
s.logger.Info(fmt.Sprintf("%s server is disabled via config", s.Name()))
return nil
Expand Down
41 changes: 23 additions & 18 deletions server/v2/api/grpcgateway/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/grpc-ecosystem/grpc-gateway/runtime"
"google.golang.org/grpc"

"cosmossdk.io/core/server"
"cosmossdk.io/core/transaction"
"cosmossdk.io/log"
serverv2 "cosmossdk.io/server/v2"
Expand All @@ -34,7 +35,13 @@ type Server[T transaction.Tx] struct {
}

// New creates a new gRPC-gateway server.
func New[T transaction.Tx](grpcSrv *grpc.Server, ir jsonpb.AnyResolver, cfgOptions ...CfgOption) *Server[T] {
func New[T transaction.Tx](
logger log.Logger,
config server.ConfigMap,
grpcSrv *grpc.Server,
ir jsonpb.AnyResolver,
cfgOptions ...CfgOption,
) (*Server[T], error) {
// The default JSON marshaller used by the gRPC-Gateway is unable to marshal non-nullable non-scalar fields.
// Using the gogo/gateway package with the gRPC-Gateway WithMarshaler option fixes the scalar field marshaling issue.
marshalerOption := &gateway.JSONPb{
Expand All @@ -44,7 +51,7 @@ func New[T transaction.Tx](grpcSrv *grpc.Server, ir jsonpb.AnyResolver, cfgOptio
AnyResolver: ir,
}

return &Server[T]{
s := &Server[T]{
gRPCSrv: grpcSrv,
gRPCGatewayRouter: runtime.NewServeMux(
// Custom marshaler option is required for gogo proto
Expand All @@ -60,6 +67,20 @@ func New[T transaction.Tx](grpcSrv *grpc.Server, ir jsonpb.AnyResolver, cfgOptio
),
cfgOptions: cfgOptions,
}

serverCfg := s.Config().(*Config)
if len(config) > 0 {
if err := serverv2.UnmarshalSubConfig(config, s.Name(), &serverCfg); err != nil {
return s, fmt.Errorf("failed to unmarshal config: %w", err)
}
Comment on lines +71 to +75
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid type assertion by changing Config() method's return type

Change the Config() method to return *Config instead of any to eliminate the need for type assertion and improve type safety.

Apply this diff:

-func (s *Server[T]) Config() any {
+func (s *Server[T]) Config() *Config {

And update the usage in the New function:

-serverCfg := s.Config().(*Config)
+serverCfg := s.Config()

Committable suggestion was skipped due to low confidence.

}

// TODO: register the gRPC-Gateway routes

s.logger = logger.With(log.ModuleKey, s.Name())
s.config = serverCfg

return s, nil
}

func (s *Server[T]) Name() string {
Expand All @@ -80,22 +101,6 @@ func (s *Server[T]) Config() any {
return s.config
}

func (s *Server[T]) Init(appI serverv2.AppI[transaction.Tx], cfg map[string]any, logger log.Logger) error {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be moved up?

Copy link
Member

Choose a reason for hiding this comment

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

the config reading and the logger key should be in New

Copy link
Member

Choose a reason for hiding this comment

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

serverCfg := s.Config().(*Config)
if len(cfg) > 0 {
if err := serverv2.UnmarshalSubConfig(cfg, s.Name(), &serverCfg); err != nil {
return fmt.Errorf("failed to unmarshal config: %w", err)
}
}

// TODO: register the gRPC-Gateway routes

s.logger = logger.With(log.ModuleKey, s.Name())
s.config = serverCfg

return nil
}

func (s *Server[T]) Start(ctx context.Context) error {
if !s.config.Enable {
s.logger.Info(fmt.Sprintf("%s server is disabled via config", s.Name()))
Expand Down
Loading
Loading