From b84cabb8f4575c9ef62f41100cce5dba4cb14b67 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 11 Sep 2024 21:12:03 +0200 Subject: [PATCH 1/3] refactor(server/v2): kill viper from server components --- server/v2/api/grpc/server.go | 15 +++++++-------- server/v2/api/grpcgateway/server.go | 11 +++++------ server/v2/cometbft/config.go | 14 -------------- server/v2/cometbft/server.go | 20 +++++++++++++------- server/v2/commands.go | 2 +- server/v2/config.go | 8 +++----- server/v2/config_test.go | 5 +++-- server/v2/server.go | 15 +++++++-------- server/v2/server_mock_test.go | 4 +--- server/v2/server_test.go | 5 +++-- server/v2/store/server.go | 11 +++++------ 11 files changed, 48 insertions(+), 62 deletions(-) diff --git a/server/v2/api/grpc/server.go b/server/v2/api/grpc/server.go index b66be16da602..0003f2343222 100644 --- a/server/v2/api/grpc/server.go +++ b/server/v2/api/grpc/server.go @@ -12,7 +12,6 @@ import ( "github.com/cosmos/gogoproto/proto" "github.com/spf13/pflag" - "github.com/spf13/viper" "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" @@ -47,10 +46,10 @@ func New[T transaction.Tx](cfgOptions ...CfgOption) *Server[T] { // 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], v *viper.Viper, logger log.Logger) error { - cfg := s.Config().(*Config) - if v != nil { - if err := serverv2.UnmarshalSubConfig(v, s.Name(), &cfg); err != nil { +func (s *Server[T]) Init(appI serverv2.AppI[T], cfg map[string]any, logger log.Logger) error { + 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) } } @@ -58,8 +57,8 @@ func (s *Server[T]) Init(appI serverv2.AppI[T], v *viper.Viper, logger log.Logge grpcSrv := grpc.NewServer( grpc.ForceServerCodec(newProtoCodec(appI.InterfaceRegistry()).GRPCCodec()), - grpc.MaxSendMsgSize(cfg.MaxSendMsgSize), - grpc.MaxRecvMsgSize(cfg.MaxRecvMsgSize), + grpc.MaxSendMsgSize(serverCfg.MaxSendMsgSize), + grpc.MaxRecvMsgSize(serverCfg.MaxRecvMsgSize), grpc.UnknownServiceHandler( makeUnknownServiceHandler(methodsMap, appI.GetAppManager()), ), @@ -69,7 +68,7 @@ func (s *Server[T]) Init(appI serverv2.AppI[T], v *viper.Viper, logger log.Logge gogoreflection.Register(grpcSrv, slices.Collect(maps.Keys(methodsMap)), logger.With("sub-module", "grpc-reflection")) s.grpcSrv = grpcSrv - s.config = cfg + s.config = serverCfg s.logger = logger.With(log.ModuleKey, s.Name()) return nil diff --git a/server/v2/api/grpcgateway/server.go b/server/v2/api/grpcgateway/server.go index 028027a83a07..05a6bf1aa829 100644 --- a/server/v2/api/grpcgateway/server.go +++ b/server/v2/api/grpcgateway/server.go @@ -10,7 +10,6 @@ import ( "github.com/cosmos/gogoproto/jsonpb" "github.com/gorilla/mux" "github.com/grpc-ecosystem/grpc-gateway/runtime" - "github.com/spf13/viper" "google.golang.org/grpc" "cosmossdk.io/core/transaction" @@ -83,10 +82,10 @@ func (s *GRPCGatewayServer[T]) Config() any { return s.config } -func (s *GRPCGatewayServer[T]) Init(appI serverv2.AppI[transaction.Tx], v *viper.Viper, logger log.Logger) error { - cfg := s.Config().(*Config) - if v != nil { - if err := serverv2.UnmarshalSubConfig(v, s.Name(), &cfg); err != nil { +func (s *GRPCGatewayServer[T]) Init(appI serverv2.AppI[transaction.Tx], cfg map[string]any, logger log.Logger) error { + 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) } } @@ -95,7 +94,7 @@ func (s *GRPCGatewayServer[T]) Init(appI serverv2.AppI[transaction.Tx], v *viper // appI.RegisterGRPCGatewayRoutes(s.GRPCGatewayRouter, s.GRPCSrv) s.logger = logger - s.config = cfg + s.config = serverCfg return nil } diff --git a/server/v2/cometbft/config.go b/server/v2/cometbft/config.go index 56860a78ddaf..81f3aeb33354 100644 --- a/server/v2/cometbft/config.go +++ b/server/v2/cometbft/config.go @@ -2,9 +2,6 @@ package cometbft import ( cmtcfg "github.com/cometbft/cometbft/config" - "github.com/spf13/viper" - - serverv2 "cosmossdk.io/server/v2" ) // Config is the configuration for the CometBFT application @@ -53,14 +50,3 @@ func OverwriteDefaultAppTomlConfig(newCfg *AppTomlConfig) CfgOption { cfg.AppTomlConfig = newCfg } } - -func getConfigTomlFromViper(v *viper.Viper) *cmtcfg.Config { - rootDir := v.GetString(serverv2.FlagHome) - - conf := cmtcfg.DefaultConfig() - if err := v.Unmarshal(conf); err != nil { - return cmtcfg.DefaultConfig().SetRoot(rootDir) - } - - return conf.SetRoot(rootDir) -} diff --git a/server/v2/cometbft/server.go b/server/v2/cometbft/server.go index 14e7cdb7bcaa..4dfb4e720a18 100644 --- a/server/v2/cometbft/server.go +++ b/server/v2/cometbft/server.go @@ -19,7 +19,6 @@ import ( "github.com/cometbft/cometbft/proxy" "github.com/spf13/cobra" "github.com/spf13/pflag" - "github.com/spf13/viper" "cosmossdk.io/core/transaction" "cosmossdk.io/log" @@ -58,23 +57,30 @@ func New[T transaction.Tx](txCodec transaction.Codec[T], serverOptions ServerOpt } } -func (s *CometBFTServer[T]) Init(appI serverv2.AppI[T], v *viper.Viper, logger log.Logger) error { +func (s *CometBFTServer[T]) Init(appI serverv2.AppI[T], cfg map[string]any, logger log.Logger) error { + home, _ := cfg[serverv2.FlagHome].(string) + // get configs (app.toml + config.toml) from viper appTomlConfig := s.Config().(*AppTomlConfig) - if v != nil { - if err := serverv2.UnmarshalSubConfig(v, s.Name(), &appTomlConfig); err != nil { + configTomlConfig := cmtcfg.DefaultConfig().SetRoot(home) + if len(cfg) > 0 { + if err := serverv2.UnmarshalSubConfig(cfg, s.Name(), &appTomlConfig); err != nil { + return fmt.Errorf("failed to unmarshal config: %w", err) + } + + if err := serverv2.UnmarshalSubConfig(cfg, "", &configTomlConfig); err != nil { return fmt.Errorf("failed to unmarshal config: %w", err) } } s.config = Config{ - ConfigTomlConfig: getConfigTomlFromViper(v), + ConfigTomlConfig: configTomlConfig, AppTomlConfig: appTomlConfig, } - chainID := v.GetString(FlagChainID) + chainID, _ := cfg[FlagChainID].(string) if chainID == "" { // fallback to genesis chain-id - reader, err := os.Open(filepath.Join(v.GetString(serverv2.FlagHome), "config", "genesis.json")) + reader, err := os.Open(filepath.Join(home, "config", "genesis.json")) if err != nil { panic(err) } diff --git a/server/v2/commands.go b/server/v2/commands.go index 101a23d3983e..8651074f406f 100644 --- a/server/v2/commands.go +++ b/server/v2/commands.go @@ -112,7 +112,7 @@ func createStartCommand[T transaction.Tx]( return err } - if err := server.Init(newApp(l, v), v, l); err != nil { + if err := server.Init(newApp(l, v), v.AllSettings(), l); err != nil { return err } diff --git a/server/v2/config.go b/server/v2/config.go index b5c525fdf682..c7df142c3805 100644 --- a/server/v2/config.go +++ b/server/v2/config.go @@ -39,12 +39,10 @@ func ReadConfig(configPath string) (*viper.Viper, error) { return v, nil } -// UnmarshalSubConfig unmarshals the given subconfig from the viper instance. -// It unmarshals the config, env, flags into the target struct. -// Use this instead of viper.Sub because viper does not unmarshal flags. -func UnmarshalSubConfig(v *viper.Viper, subName string, target any) error { +// UnmarshalSubConfig unmarshals the given sub config from the main config (given as a map) into the target. +func UnmarshalSubConfig(cfg map[string]any, subName string, target any) error { var sub any - for k, val := range v.AllSettings() { + for k, val := range cfg { if k == subName { sub = val break diff --git a/server/v2/config_test.go b/server/v2/config_test.go index a24cd4cf3d2e..6577b81192fb 100644 --- a/server/v2/config_test.go +++ b/server/v2/config_test.go @@ -31,16 +31,17 @@ func TestUnmarshalSubConfig(t *testing.T) { v, err := serverv2.ReadConfig(configPath) require.NoError(t, err) + cfg := v.AllSettings() grpcConfig := grpc.DefaultConfig() - err = serverv2.UnmarshalSubConfig(v, "grpc", &grpcConfig) + err = serverv2.UnmarshalSubConfig(cfg, "grpc", &grpcConfig) require.NoError(t, err) require.True(t, grpc.DefaultConfig().Enable) require.False(t, grpcConfig.Enable) storeConfig := store.Config{} - err = serverv2.UnmarshalSubConfig(v, "store", &storeConfig) + err = serverv2.UnmarshalSubConfig(cfg, "store", &storeConfig) require.NoError(t, err) require.Equal(t, *store.DefaultConfig(), storeConfig) } diff --git a/server/v2/server.go b/server/v2/server.go index 08ae9a6a06fa..d1062618ab60 100644 --- a/server/v2/server.go +++ b/server/v2/server.go @@ -10,7 +10,6 @@ import ( "github.com/pelletier/go-toml/v2" "github.com/spf13/cobra" "github.com/spf13/pflag" - "github.com/spf13/viper" "golang.org/x/sync/errgroup" "cosmossdk.io/core/transaction" @@ -23,7 +22,7 @@ type ServerComponent[T transaction.Tx] interface { Start(context.Context) error Stop(context.Context) error - Init(AppI[T], *viper.Viper, log.Logger) error + Init(AppI[T], map[string]any, log.Logger) error } // HasStartFlags is a server module that has start flags. @@ -189,10 +188,10 @@ func (s *Server[T]) StartCmdFlags() *pflag.FlagSet { // Init initializes all server components with the provided application, configuration, and logger. // It returns an error if any component fails to initialize. -func (s *Server[T]) Init(appI AppI[T], v *viper.Viper, logger log.Logger) error { - cfg := s.config - if v != nil { - if err := UnmarshalSubConfig(v, s.Name(), &cfg); err != nil { +func (s *Server[T]) Init(appI AppI[T], cfg map[string]any, logger log.Logger) error { + serverCfg := s.config + if len(cfg) > 0 { + if err := UnmarshalSubConfig(cfg, s.Name(), &serverCfg); err != nil { return fmt.Errorf("failed to unmarshal config: %w", err) } } @@ -200,14 +199,14 @@ func (s *Server[T]) Init(appI AppI[T], v *viper.Viper, logger log.Logger) error var components []ServerComponent[T] for _, mod := range s.components { mod := mod - if err := mod.Init(appI, v, logger); err != nil { + if err := mod.Init(appI, cfg, logger); err != nil { return err } components = append(components, mod) } - s.config = cfg + s.config = serverCfg s.components = components return nil } diff --git a/server/v2/server_mock_test.go b/server/v2/server_mock_test.go index ae238d66a57a..8b590ba2d32f 100644 --- a/server/v2/server_mock_test.go +++ b/server/v2/server_mock_test.go @@ -5,8 +5,6 @@ import ( "fmt" "math/rand" - "github.com/spf13/viper" - "cosmossdk.io/core/transaction" "cosmossdk.io/log" serverv2 "cosmossdk.io/server/v2" @@ -33,7 +31,7 @@ func (s *mockServer) Name() string { return s.name } -func (s *mockServer) Init(appI serverv2.AppI[transaction.Tx], v *viper.Viper, logger log.Logger) error { +func (s *mockServer) Init(appI serverv2.AppI[transaction.Tx], cfg map[string]any, logger log.Logger) error { return nil } diff --git a/server/v2/server_test.go b/server/v2/server_test.go index 98b1def5ab9f..e84bcd598890 100644 --- a/server/v2/server_test.go +++ b/server/v2/server_test.go @@ -56,14 +56,15 @@ func TestServer(t *testing.T) { if err != nil { v = viper.New() } + cfg := v.AllSettings() logger := log.NewLogger(os.Stdout) grpcServer := grpc.New[transaction.Tx]() - err = grpcServer.Init(&mockApp[transaction.Tx]{}, v, logger) + err = grpcServer.Init(&mockApp[transaction.Tx]{}, cfg, logger) require.NoError(t, err) storeServer := store.New[transaction.Tx](nil /* nil appCreator as not using CLI commands */) - err = storeServer.Init(&mockApp[transaction.Tx]{}, v, logger) + err = storeServer.Init(&mockApp[transaction.Tx]{}, cfg, logger) require.NoError(t, err) mockServer := &mockServer{name: "mock-server-1", ch: make(chan string, 100)} diff --git a/server/v2/store/server.go b/server/v2/store/server.go index 1177c6a0c414..e16a09137a4b 100644 --- a/server/v2/store/server.go +++ b/server/v2/store/server.go @@ -5,7 +5,6 @@ import ( "fmt" "github.com/spf13/cobra" - "github.com/spf13/viper" "cosmossdk.io/core/transaction" "cosmossdk.io/log" @@ -24,14 +23,14 @@ func New[T transaction.Tx](appCreator serverv2.AppCreator[T]) *StoreComponent[T] return &StoreComponent[T]{appCreator: appCreator} } -func (s *StoreComponent[T]) Init(appI serverv2.AppI[T], v *viper.Viper, logger log.Logger) error { - cfg := DefaultConfig() - if v != nil { - if err := serverv2.UnmarshalSubConfig(v, s.Name(), &cfg); err != nil { +func (s *StoreComponent[T]) Init(appI serverv2.AppI[T], cfg map[string]any, logger log.Logger) error { + serverCfg := DefaultConfig() + if len(cfg) > 0 { + if err := serverv2.UnmarshalSubConfig(cfg, s.Name(), &serverCfg); err != nil { return fmt.Errorf("failed to unmarshal config: %w", err) } } - s.config = cfg + s.config = serverCfg return nil } From ad8ead558ecf9b278bf8054b5d727723ba7dab48 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 11 Sep 2024 21:13:17 +0200 Subject: [PATCH 2/3] updates --- server/v2/cometbft/go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/v2/cometbft/go.mod b/server/v2/cometbft/go.mod index 3709cd1f4561..57b947da1fe3 100644 --- a/server/v2/cometbft/go.mod +++ b/server/v2/cometbft/go.mod @@ -37,7 +37,6 @@ require ( github.com/grpc-ecosystem/grpc-gateway v1.16.0 github.com/spf13/cobra v1.8.1 github.com/spf13/pflag v1.0.5 - github.com/spf13/viper v1.19.0 github.com/stretchr/testify v1.9.0 google.golang.org/genproto/googleapis/api v0.0.0-20240604185151-ef581f913117 google.golang.org/grpc v1.66.1 @@ -150,6 +149,7 @@ require ( github.com/sourcegraph/conc v0.3.0 // indirect github.com/spf13/afero v1.11.0 // indirect github.com/spf13/cast v1.7.0 // indirect + github.com/spf13/viper v1.19.0 // indirect github.com/subosito/gotenv v1.6.0 // indirect github.com/supranational/blst v0.3.13 // indirect github.com/syndtr/goleveldb v1.0.1-0.20220721030215-126854af5e6d // indirect From 2cca86d5e5e7bee8f884cb6c8792574dc66df354 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 11 Sep 2024 21:19:40 +0200 Subject: [PATCH 3/3] updates --- server/v2/cometbft/server.go | 1 + server/v2/config.go | 15 ++++++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/server/v2/cometbft/server.go b/server/v2/cometbft/server.go index 4dfb4e720a18..26cd99ff97cd 100644 --- a/server/v2/cometbft/server.go +++ b/server/v2/cometbft/server.go @@ -72,6 +72,7 @@ func (s *CometBFTServer[T]) Init(appI serverv2.AppI[T], cfg map[string]any, logg return fmt.Errorf("failed to unmarshal config: %w", err) } } + s.config = Config{ ConfigTomlConfig: configTomlConfig, AppTomlConfig: appTomlConfig, diff --git a/server/v2/config.go b/server/v2/config.go index c7df142c3805..0670bc374a24 100644 --- a/server/v2/config.go +++ b/server/v2/config.go @@ -39,14 +39,19 @@ func ReadConfig(configPath string) (*viper.Viper, error) { return v, nil } -// UnmarshalSubConfig unmarshals the given sub config from the main config (given as a map) into the target. +// UnmarshalSubConfig unmarshals the given (sub) config from the main config (given as a map) into the target. +// If subName is empty, the main config is unmarshaled into the target. func UnmarshalSubConfig(cfg map[string]any, subName string, target any) error { var sub any - for k, val := range cfg { - if k == subName { - sub = val - break + if subName != "" { + for k, val := range cfg { + if k == subName { + sub = val + break + } } + } else { + sub = cfg } // Create a new decoder with custom decoding options