From afddef39064271f79e28382859c6312b2a5bae0c Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Thu, 27 Jun 2024 21:27:37 +0200 Subject: [PATCH] refactor(server/v2): simplify servers (#20796) --- client/cmd.go | 6 +- core/context/server_context.go | 9 +- server/util.go | 4 +- server/v2/api/grpc/server.go | 8 +- server/v2/cometbft/abci.go | 20 ----- server/v2/cometbft/commands.go | 27 +++--- server/v2/cometbft/config.go | 2 + server/v2/cometbft/flags.go | 52 ++++++++++++ server/v2/cometbft/flags/flags.go | 76 ----------------- server/v2/cometbft/options.go | 32 +++++++ server/v2/cometbft/server.go | 90 +++++++------------- server/v2/commands.go | 106 +++++++++++++++++++----- server/v2/config.go | 9 -- server/v2/logger.go | 3 +- server/v2/server.go | 14 ++-- server/v2/server_test.go | 4 +- server/v2/util.go | 21 +++-- simapp/simd/cmd/testnet_test.go | 4 +- simapp/v2/simdv2/cmd/commands.go | 2 +- simapp/v2/simdv2/cmd/root_di.go | 4 - tests/e2e/genutil/export_test.go | 4 +- x/genutil/client/cli/export_test.go | 4 +- x/genutil/client/cli/genaccount_test.go | 4 +- x/genutil/client/cli/init_test.go | 28 +++---- x/genutil/client/testutil/helpers.go | 4 +- 25 files changed, 275 insertions(+), 262 deletions(-) create mode 100644 server/v2/cometbft/flags.go delete mode 100644 server/v2/cometbft/flags/flags.go create mode 100644 server/v2/cometbft/options.go diff --git a/client/cmd.go b/client/cmd.go index 1ea5d9d6bf..c5b40774d9 100644 --- a/client/cmd.go +++ b/client/cmd.go @@ -379,7 +379,7 @@ func SetCmdClientContext(cmd *cobra.Command, clientCtx Context) error { } func GetViperFromCmd(cmd *cobra.Command) *viper.Viper { - value := cmd.Context().Value(corectx.ViperContextKey{}) + value := cmd.Context().Value(corectx.ViperContextKey) v, ok := value.(*viper.Viper) if !ok { return viper.New() @@ -388,7 +388,7 @@ func GetViperFromCmd(cmd *cobra.Command) *viper.Viper { } func GetConfigFromCmd(cmd *cobra.Command) *cmtcfg.Config { - v := cmd.Context().Value(corectx.ViperContextKey{}) + v := cmd.Context().Value(corectx.ViperContextKey) viper, ok := v.(*viper.Viper) if !ok { return cmtcfg.DefaultConfig() @@ -397,7 +397,7 @@ func GetConfigFromCmd(cmd *cobra.Command) *cmtcfg.Config { } func GetLoggerFromCmd(cmd *cobra.Command) log.Logger { - v := cmd.Context().Value(corectx.LoggerContextKey{}) + v := cmd.Context().Value(corectx.LoggerContextKey) logger, ok := v.(log.Logger) if !ok { return log.NewLogger(cmd.OutOrStdout()) diff --git a/core/context/server_context.go b/core/context/server_context.go index 98c0821550..93975078cc 100644 --- a/core/context/server_context.go +++ b/core/context/server_context.go @@ -1,6 +1,11 @@ package context type ( - LoggerContextKey struct{} - ViperContextKey struct{} + loggerContextKey struct{} + viperContextKey struct{} +) + +var ( + LoggerContextKey loggerContextKey + ViperContextKey viperContextKey ) diff --git a/server/util.go b/server/util.go index d185c98e38..74d3f3d47f 100644 --- a/server/util.go +++ b/server/util.go @@ -227,8 +227,8 @@ func SetCmdServerContext(cmd *cobra.Command, serverCtx *Context) error { } cmdCtx = context.WithValue(cmdCtx, ServerContextKey, serverCtx) - cmdCtx = context.WithValue(cmdCtx, corectx.ViperContextKey{}, serverCtx.Viper) - cmdCtx = context.WithValue(cmdCtx, corectx.LoggerContextKey{}, serverCtx.Logger) + cmdCtx = context.WithValue(cmdCtx, corectx.ViperContextKey, serverCtx.Viper) + cmdCtx = context.WithValue(cmdCtx, corectx.LoggerContextKey, serverCtx.Logger) cmd.SetContext(cmdCtx) diff --git a/server/v2/api/grpc/server.go b/server/v2/api/grpc/server.go index 8521fedfa8..7346ad6ef1 100644 --- a/server/v2/api/grpc/server.go +++ b/server/v2/api/grpc/server.go @@ -21,8 +21,6 @@ import ( "cosmossdk.io/server/v2/api/grpc/gogoreflection" ) -const serverName = "grpc-server" - type GRPCServer[AppT serverv2.AppI[T], T transaction.Tx] struct { logger log.Logger config *Config @@ -44,7 +42,7 @@ func New[AppT serverv2.AppI[T], T transaction.Tx]() *GRPCServer[AppT, T] { func (g *GRPCServer[AppT, T]) Init(appI AppT, v *viper.Viper, logger log.Logger) error { cfg := DefaultConfig() if v != nil { - if err := v.Sub(serverName).Unmarshal(&cfg); err != nil { + if err := v.Sub(g.Name()).Unmarshal(&cfg); err != nil { return fmt.Errorf("failed to unmarshal config: %w", err) } } @@ -63,13 +61,13 @@ func (g *GRPCServer[AppT, T]) Init(appI AppT, v *viper.Viper, logger log.Logger) g.grpcSrv = grpcSrv g.config = cfg - g.logger = logger.With(log.ModuleKey, serverName) + g.logger = logger.With(log.ModuleKey, g.Name()) return nil } func (g *GRPCServer[AppT, T]) Name() string { - return serverName + return "grpc-server" } func (g *GRPCServer[AppT, T]) Start(ctx context.Context) error { diff --git a/server/v2/cometbft/abci.go b/server/v2/cometbft/abci.go index c0860b3c96..3675614427 100644 --- a/server/v2/cometbft/abci.go +++ b/server/v2/cometbft/abci.go @@ -70,10 +70,6 @@ func NewConsensus[T transaction.Tx]( } } -func (c *Consensus[T]) SetMempool(mp mempool.Mempool[T]) { - c.mempool = mp -} - func (c *Consensus[T]) SetStreamingManager(sm streaming.Manager) { c.streaming = sm } @@ -94,22 +90,6 @@ func (c *Consensus[T]) RegisterExtensions(extensions ...snapshots.ExtensionSnaps } } -func (c *Consensus[T]) SetPrepareProposalHandler(handler handlers.PrepareHandler[T]) { - c.prepareProposalHandler = handler -} - -func (c *Consensus[T]) SetProcessProposalHandler(handler handlers.ProcessHandler[T]) { - c.processProposalHandler = handler -} - -func (c *Consensus[T]) SetExtendVoteExtension(handler handlers.ExtendVoteHandler) { - c.extendVote = handler -} - -func (c *Consensus[T]) SetVerifyVoteExtension(handler handlers.VerifyVoteExtensionhandler) { - c.verifyVoteExt = handler -} - // BlockData is used to keep some data about the last committed block. Currently // we only use the height, the rest is not needed right now and might get removed // in the future. diff --git a/server/v2/cometbft/commands.go b/server/v2/cometbft/commands.go index 58d728fcbf..a27a846c58 100644 --- a/server/v2/cometbft/commands.go +++ b/server/v2/cometbft/commands.go @@ -19,7 +19,6 @@ import ( "sigs.k8s.io/yaml" "cosmossdk.io/server/v2/cometbft/client/rpc" - "cosmossdk.io/server/v2/cometbft/flags" auth "cosmossdk.io/x/auth/client/cli" "github.com/cosmos/cosmos-sdk/client" @@ -38,8 +37,8 @@ func (s *CometBFTServer[AppT, T]) rpcClient(cmd *cobra.Command) (rpc.CometRPC, e return client, nil } - if s.Node == nil || cmd.Flags().Changed(flags.FlagNode) { - rpcURI, err := cmd.Flags().GetString(flags.FlagNode) + if s.Node == nil || cmd.Flags().Changed(FlagNode) { + rpcURI, err := cmd.Flags().GetString(FlagNode) if err != nil { return nil, err } @@ -76,8 +75,8 @@ func (s *CometBFTServer[AppT, T]) StatusCommand() *cobra.Command { }, } - cmd.Flags().StringP(flags.FlagNode, "n", "tcp://localhost:26657", "Node to connect to") - cmd.Flags().StringP(flags.FlagOutput, "o", "json", "Output format (text|json)") + cmd.Flags().StringP(FlagNode, "n", "tcp://localhost:26657", "Node to connect to") + cmd.Flags().StringP(FlagOutput, "o", "json", "Output format (text|json)") return cmd } @@ -203,8 +202,8 @@ for. Each module documents its respective events under 'xx_events.md'. } query, _ := cmd.Flags().GetString(auth.FlagQuery) - page, _ := cmd.Flags().GetInt(flags.FlagPage) - limit, _ := cmd.Flags().GetInt(flags.FlagLimit) + page, _ := cmd.Flags().GetInt(FlagPage) + limit, _ := cmd.Flags().GetInt(FlagLimit) orderBy, _ := cmd.Flags().GetString(auth.FlagOrderBy) blocks, err := rpc.QueryBlocks(cmd.Context(), rpcclient, page, limit, query, orderBy) @@ -221,9 +220,9 @@ for. Each module documents its respective events under 'xx_events.md'. }, } - flags.AddQueryFlagsToCmd(cmd) - cmd.Flags().Int(flags.FlagPage, query.DefaultPage, "Query a specific page of paginated results") - cmd.Flags().Int(flags.FlagLimit, query.DefaultLimit, "Query number of transactions results per page returned") + AddQueryFlagsToCmd(cmd) + cmd.Flags().Int(FlagPage, query.DefaultPage, "Query a specific page of paginated results") + cmd.Flags().Int(FlagLimit, query.DefaultLimit, "Query number of transactions results per page returned") cmd.Flags().String(auth.FlagQuery, "", "The blocks events query per CometBFT's query semantics") cmd.Flags().String(auth.FlagOrderBy, "", "The ordering semantics (asc|dsc)") _ = cmd.MarkFlagRequired(auth.FlagQuery) @@ -312,7 +311,7 @@ $ %s query block --%s=%s }, } - flags.AddQueryFlagsToCmd(cmd) + AddQueryFlagsToCmd(cmd) cmd.Flags().String(auth.FlagType, auth.TypeHash, fmt.Sprintf("The type to be used when querying tx, can be one of \"%s\", \"%s\"", auth.TypeHeight, auth.TypeHash)) return cmd @@ -364,7 +363,7 @@ func (s *CometBFTServer[AppT, T]) QueryBlockResultsCmd() *cobra.Command { }, } - flags.AddQueryFlagsToCmd(cmd) + AddQueryFlagsToCmd(cmd) return cmd } @@ -396,7 +395,7 @@ func (s *CometBFTServer[AppT, T]) BootstrapStateCmd() *cobra.Command { return err } if height == 0 { - height, err = s.App.store.GetLatestVersion() + height, err = s.Consensus.store.GetLatestVersion() if err != nil { return err } @@ -414,7 +413,7 @@ func (s *CometBFTServer[AppT, T]) BootstrapStateCmd() *cobra.Command { func printOutput(cmd *cobra.Command, out []byte) error { // Get flags output - outFlag, err := cmd.Flags().GetString(flags.FlagOutput) + outFlag, err := cmd.Flags().GetString(FlagOutput) if err != nil { return err } diff --git a/server/v2/cometbft/config.go b/server/v2/cometbft/config.go index d6a546004f..f5b2359e8b 100644 --- a/server/v2/cometbft/config.go +++ b/server/v2/cometbft/config.go @@ -9,6 +9,8 @@ import ( "cosmossdk.io/server/v2/cometbft/types" ) +// TODO REDO/VERIFY THIS + func GetConfigFromViper(v *viper.Viper) *cmtcfg.Config { conf := cmtcfg.DefaultConfig() err := v.Unmarshal(conf) diff --git a/server/v2/cometbft/flags.go b/server/v2/cometbft/flags.go new file mode 100644 index 0000000000..fe1442edf6 --- /dev/null +++ b/server/v2/cometbft/flags.go @@ -0,0 +1,52 @@ +package cometbft + +import "github.com/spf13/cobra" + +const ( + FlagQuery = "query" + FlagType = "type" + FlagOrderBy = "order_by" +) + +const ( + FlagWithComet = "with-comet" + FlagAddress = "address" + FlagTransport = "transport" + FlagTraceStore = "trace-store" + FlagCPUProfile = "cpu-profile" + FlagMinGasPrices = "minimum-gas-prices" + FlagQueryGasLimit = "query-gas-limit" + FlagHaltHeight = "halt-height" + FlagHaltTime = "halt-time" + FlagTrace = "trace" +) + +const ( + FlagChainID = "chain-id" + FlagNode = "node" + FlagGRPC = "grpc-addr" + FlagGRPCInsecure = "grpc-insecure" + FlagHeight = "height" + FlagPage = "page" + FlagLimit = "limit" + FlagOutput = "output" +) + +// List of supported output formats +const ( + OutputFormatJSON = "json" + OutputFormatText = "text" +) + +// AddQueryFlagsToCmd adds common flags to a module query command. +func AddQueryFlagsToCmd(cmd *cobra.Command) { + cmd.Flags().String(FlagNode, "tcp://localhost:26657", ": to CometBFT RPC interface for this chain") + cmd.Flags().String(FlagGRPC, "", "the gRPC endpoint to use for this chain") + cmd.Flags().Bool(FlagGRPCInsecure, false, "allow gRPC over insecure channels, if not the server must use TLS") + cmd.Flags().Int64(FlagHeight, 0, "Use a specific height to query state at (this can error if the node is pruning state)") + cmd.Flags().StringP(FlagOutput, "o", OutputFormatText, "Output format (text|json)") + + // some base commands does not require chainID e.g `simd testnet` while subcommands do + // hence the flag should not be required for those commands + _ = cmd.MarkFlagRequired(FlagChainID) +} diff --git a/server/v2/cometbft/flags/flags.go b/server/v2/cometbft/flags/flags.go deleted file mode 100644 index 878cc9c282..0000000000 --- a/server/v2/cometbft/flags/flags.go +++ /dev/null @@ -1,76 +0,0 @@ -package flags - -import "github.com/spf13/cobra" - -const ( - FlagQuery = "query" - FlagType = "type" - FlagOrderBy = "order_by" -) - -const ( - FlagChainID = "chain-id" - FlagNode = "node" - FlagGRPC = "grpc-addr" - FlagGRPCInsecure = "grpc-insecure" - FlagHeight = "height" - FlagGasAdjustment = "gas-adjustment" - FlagFrom = "from" - FlagName = "name" - FlagAccountNumber = "account-number" - FlagSequence = "sequence" - FlagNote = "note" - FlagFees = "fees" - FlagGas = "gas" - FlagGasPrices = "gas-prices" - FlagBroadcastMode = "broadcast-mode" - FlagDryRun = "dry-run" - FlagGenerateOnly = "generate-only" - FlagOffline = "offline" - FlagOutputDocument = "output-document" // inspired by wget -O - FlagSkipConfirmation = "yes" - FlagProve = "prove" - FlagKeyringBackend = "keyring-backend" - FlagPage = "page" - FlagLimit = "limit" - FlagSignMode = "sign-mode" - FlagPageKey = "page-key" - FlagOffset = "offset" - FlagCountTotal = "count-total" - FlagTimeoutHeight = "timeout-height" - FlagUnordered = "unordered" - FlagKeyAlgorithm = "algo" - FlagKeyType = "key-type" - FlagFeePayer = "fee-payer" - FlagFeeGranter = "fee-granter" - FlagReverse = "reverse" - FlagTip = "tip" - FlagAux = "aux" - FlagInitHeight = "initial-height" - // FlagOutput is the flag to set the output format. - // This differs from FlagOutputDocument that is used to set the output file. - FlagOutput = "output" - // Logging flags - FlagLogLevel = "log_level" - FlagLogFormat = "log_format" - FlagLogNoColor = "log_no_color" -) - -// List of supported output formats -const ( - OutputFormatJSON = "json" - OutputFormatText = "text" -) - -// AddQueryFlagsToCmd adds common flags to a module query command. -func AddQueryFlagsToCmd(cmd *cobra.Command) { - cmd.Flags().String(FlagNode, "tcp://localhost:26657", ": to CometBFT RPC interface for this chain") - cmd.Flags().String(FlagGRPC, "", "the gRPC endpoint to use for this chain") - cmd.Flags().Bool(FlagGRPCInsecure, false, "allow gRPC over insecure channels, if not the server must use TLS") - cmd.Flags().Int64(FlagHeight, 0, "Use a specific height to query state at (this can error if the node is pruning state)") - cmd.Flags().StringP(FlagOutput, "o", "text", "Output format (text|json)") - - // some base commands does not require chainID e.g `simd testnet` while subcommands do - // hence the flag should not be required for those commands - _ = cmd.MarkFlagRequired(FlagChainID) -} diff --git a/server/v2/cometbft/options.go b/server/v2/cometbft/options.go new file mode 100644 index 0000000000..0950609f7d --- /dev/null +++ b/server/v2/cometbft/options.go @@ -0,0 +1,32 @@ +package cometbft + +import ( + "cosmossdk.io/core/transaction" + "cosmossdk.io/server/v2/cometbft/handlers" + "cosmossdk.io/server/v2/cometbft/mempool" + "cosmossdk.io/store/v2/snapshots" +) + +// ServerOptions defines the options for the CometBFT server. +type ServerOptions[T transaction.Tx] struct { + Mempool mempool.Mempool[T] + PrepareProposalHandler handlers.PrepareHandler[T] + ProcessProposalHandler handlers.ProcessHandler[T] + VerifyVoteExtensionHandler handlers.VerifyVoteExtensionhandler + ExtendVoteHandler handlers.ExtendVoteHandler + + SnapshotOptions snapshots.SnapshotOptions +} + +// DefaultServerOptions returns the default server options. +// It defaults to a NoOpMempool and NoOp handlers. +func DefaultServerOptions[T transaction.Tx]() ServerOptions[T] { + return ServerOptions[T]{ + Mempool: mempool.NoOpMempool[T]{}, + PrepareProposalHandler: handlers.NoOpPrepareProposal[T](), + ProcessProposalHandler: handlers.NoOpProcessProposal[T](), + VerifyVoteExtensionHandler: handlers.NoOpVerifyVoteExtensionHandler(), + ExtendVoteHandler: handlers.NoOpExtendVote(), + SnapshotOptions: snapshots.NewSnapshotOptions(0, 0), + } +} diff --git a/server/v2/cometbft/server.go b/server/v2/cometbft/server.go index b0a9e8eaf2..c13aa66a4b 100644 --- a/server/v2/cometbft/server.go +++ b/server/v2/cometbft/server.go @@ -22,29 +22,13 @@ import ( "cosmossdk.io/core/log" "cosmossdk.io/core/transaction" serverv2 "cosmossdk.io/server/v2" - "cosmossdk.io/server/v2/appmanager" - "cosmossdk.io/server/v2/cometbft/handlers" cometlog "cosmossdk.io/server/v2/cometbft/log" - "cosmossdk.io/server/v2/cometbft/mempool" "cosmossdk.io/server/v2/cometbft/types" "cosmossdk.io/store/v2/snapshots" genutiltypes "github.com/cosmos/cosmos-sdk/x/genutil/types" ) -const ( - flagWithComet = "with-comet" - flagAddress = "address" - flagTransport = "transport" - flagTraceStore = "trace-store" - flagCPUProfile = "cpu-profile" - FlagMinGasPrices = "minimum-gas-prices" - FlagQueryGasLimit = "query-gas-limit" - FlagHaltHeight = "halt-height" - FlagHaltTime = "halt-time" - FlagTrace = "trace" -) - var ( _ serverv2.ServerComponent[ serverv2.AppI[transaction.Tx], transaction.Tx, @@ -54,76 +38,62 @@ var ( ) type CometBFTServer[AppT serverv2.AppI[T], T transaction.Tx] struct { - Node *node.Node - App *Consensus[T] - logger log.Logger + Node *node.Node + Consensus *Consensus[T] - config Config + initTxCodec transaction.Codec[T] + logger log.Logger + config Config + options ServerOptions[T] } -// App is an interface that represents an application in the CometBFT server. -// It provides methods to access the app manager, logger, and store. -type App[T transaction.Tx] interface { - GetApp() *appmanager.AppManager[T] - GetLogger() log.Logger - GetStore() types.Store -} - -func New[AppT serverv2.AppI[T], T transaction.Tx](txCodec transaction.Codec[T]) *CometBFTServer[AppT, T] { - consensus := &Consensus[T]{txCodec: txCodec} +func New[AppT serverv2.AppI[T], T transaction.Tx](txCodec transaction.Codec[T], options ServerOptions[T]) *CometBFTServer[AppT, T] { return &CometBFTServer[AppT, T]{ - App: consensus, + initTxCodec: txCodec, + options: options, } } func (s *CometBFTServer[AppT, T]) Init(appI AppT, v *viper.Viper, logger log.Logger) error { - store := appI.GetStore().(types.Store) - - cfg := Config{CmtConfig: GetConfigFromViper(v), ConsensusAuthority: appI.GetConsensusAuthority()} - logger = logger.With("module", "cometbft-server") - - // create noop mempool - mempool := mempool.NoOpMempool[T]{} + s.config = Config{CmtConfig: GetConfigFromViper(v), ConsensusAuthority: appI.GetConsensusAuthority()} + s.logger = logger.With(log.ModuleKey, s.Name()) // create consensus - // txCodec should be in server from New() - consensus := NewConsensus[T](appI.GetAppManager(), mempool, store, cfg, s.App.txCodec, logger) + store := appI.GetStore().(types.Store) + consensus := NewConsensus[T](appI.GetAppManager(), s.options.Mempool, store, s.config, s.initTxCodec, s.logger) - consensus.SetPrepareProposalHandler(handlers.NoOpPrepareProposal[T]()) - consensus.SetProcessProposalHandler(handlers.NoOpProcessProposal[T]()) - consensus.SetVerifyVoteExtension(handlers.NoOpVerifyVoteExtensionHandler()) - consensus.SetExtendVoteExtension(handlers.NoOpExtendVote()) + consensus.prepareProposalHandler = s.options.PrepareProposalHandler + consensus.processProposalHandler = s.options.ProcessProposalHandler + consensus.verifyVoteExt = s.options.VerifyVoteExtensionHandler + consensus.extendVote = s.options.ExtendVoteHandler // TODO: set these; what is the appropriate presence of the Store interface here? var ss snapshots.StorageSnapshotter var sc snapshots.CommitSnapshotter - snapshotStore, err := GetSnapshotStore(cfg.CmtConfig.RootDir) + snapshotStore, err := GetSnapshotStore(s.config.CmtConfig.RootDir) if err != nil { - panic(err) + return err } - sm := snapshots.NewManager(snapshotStore, snapshots.SnapshotOptions{}, sc, ss, nil, logger) // TODO: set options somehow + sm := snapshots.NewManager(snapshotStore, s.options.SnapshotOptions, sc, ss, nil, s.logger) consensus.SetSnapshotManager(sm) - s.config = cfg - s.App = consensus - s.logger = logger - + s.Consensus = consensus return nil } func (s *CometBFTServer[AppT, T]) Name() string { - return "cometbft" + return "cometbft-server" } func (s *CometBFTServer[AppT, T]) Start(ctx context.Context) error { - viper := ctx.Value(corectx.ViperContextKey{}).(*viper.Viper) + viper := ctx.Value(corectx.ViperContextKey).(*viper.Viper) cometConfig := GetConfigFromViper(viper) wrappedLogger := cometlog.CometLoggerWrapper{Logger: s.logger} if s.config.Standalone { - svr, err := abciserver.NewServer(s.config.Addr, s.config.Transport, s.App) + svr, err := abciserver.NewServer(s.config.Addr, s.config.Transport, s.Consensus) if err != nil { return fmt.Errorf("error creating listener: %w", err) } @@ -143,7 +113,7 @@ func (s *CometBFTServer[AppT, T]) Start(ctx context.Context) error { cometConfig, pvm.LoadOrGenFilePV(cometConfig.PrivValidatorKeyFile(), cometConfig.PrivValidatorStateFile()), nodeKey, - proxy.NewConsensusSyncLocalClientCreator(s.App), + proxy.NewConsensusSyncLocalClientCreator(s.Consensus), getGenDocProvider(cometConfig), cmtcfg.DefaultDBProvider, node.DefaultMetricsProvider(cometConfig.Instrumentation), @@ -204,15 +174,15 @@ func getGenDocProvider(cfg *cmtcfg.Config) func() (node.ChecksummedGenesisDoc, e func (s *CometBFTServer[AppT, T]) StartCmdFlags() *pflag.FlagSet { flags := pflag.NewFlagSet("cometbft", pflag.ExitOnError) - flags.Bool(flagWithComet, true, "Run abci app embedded in-process with CometBFT") - flags.String(flagAddress, "tcp://127.0.0.1:26658", "Listen address") - flags.String(flagTransport, "socket", "Transport protocol: socket, grpc") - flags.String(flagTraceStore, "", "Enable KVStore tracing to an output file") + flags.Bool(FlagWithComet, true, "Run abci app embedded in-process with CometBFT") + flags.String(FlagAddress, "tcp://127.0.0.1:26658", "Listen address") + flags.String(FlagTransport, "socket", "Transport protocol: socket, grpc") + flags.String(FlagTraceStore, "", "Enable KVStore tracing to an output file") flags.String(FlagMinGasPrices, "", "Minimum gas prices to accept for transactions; Any fee in a tx must meet this minimum (e.g. 0.01photino;0.0001stake)") flags.Uint64(FlagQueryGasLimit, 0, "Maximum gas a Rest/Grpc query can consume. Blank and 0 imply unbounded.") flags.Uint64(FlagHaltHeight, 0, "Block height at which to gracefully halt the chain and shutdown the node") flags.Uint64(FlagHaltTime, 0, "Minimum block time (in Unix seconds) at which to gracefully halt the chain and shutdown the node") - flags.String(flagCPUProfile, "", "Enable CPU profiling and write to the provided file") + flags.String(FlagCPUProfile, "", "Enable CPU profiling and write to the provided file") flags.Bool(FlagTrace, false, "Provide full stack traces for errors in ABCI Log") return flags } diff --git a/server/v2/commands.go b/server/v2/commands.go index b5b6e5533e..9f4b3e1816 100644 --- a/server/v2/commands.go +++ b/server/v2/commands.go @@ -33,6 +33,8 @@ func Execute(rootCmd *cobra.Command, envPrefix, defaultHome string) error { return rootCmd.Execute() } +// Commands creates the start command of an application and gives back the CLIConfig containing all the server commands. +// This API is for advanced user only, most users should use AddCommands instead that abstract more. func Commands[AppT AppI[T], T transaction.Tx]( rootCmd *cobra.Command, newApp AppCreator[AppT, T], @@ -69,10 +71,7 @@ func Commands[AppT AppI[T], T transaction.Tx]( return err } - srvConfig := Config{StartBlock: true} - ctx := cmd.Context() - ctx = context.WithValue(ctx, ServerContextKey, srvConfig) - ctx, cancelFn := context.WithCancel(ctx) + ctx, cancelFn := context.WithCancel(cmd.Context()) go func() { sigCh := make(chan os.Signal, 1) signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM) @@ -92,6 +91,7 @@ func Commands[AppT AppI[T], T transaction.Tx]( return nil }, } + startCmd.SetContext(rootCmd.Context()) cmds := server.CLICommands() cmds.Commands = append(cmds.Commands, startCmd) @@ -99,6 +99,8 @@ func Commands[AppT AppI[T], T transaction.Tx]( return cmds, nil } +// AddCommands add the server commands to the root command +// It configure the config handling and the logger handling func AddCommands[AppT AppI[T], T transaction.Tx]( rootCmd *cobra.Command, newApp AppCreator[AppT, T], @@ -110,15 +112,14 @@ func AddCommands[AppT AppI[T], T transaction.Tx]( return err } - server := NewServer(logger, components...) + srv := NewServer(logger, components...) originalPersistentPreRunE := rootCmd.PersistentPreRunE rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { - home, err := cmd.Flags().GetString(FlagHome) - if err != nil { - return err - } + // set the default command outputs + cmd.SetOut(cmd.OutOrStdout()) + cmd.SetErr(cmd.ErrOrStderr()) - if err = configHandle(server, home, cmd); err != nil { + if err = configHandle(srv, cmd); err != nil { return err } @@ -131,11 +132,38 @@ func AddCommands[AppT AppI[T], T transaction.Tx]( } rootCmd.AddCommand(cmds.Commands...) + + if len(cmds.Queries) > 0 { + if queryCmd := findSubCommand(rootCmd, "query"); queryCmd != nil { + queryCmd.AddCommand(cmds.Queries...) + } else { + queryCmd := topLevelCmd(rootCmd.Context(), "query", "Querying subcommands") + queryCmd.Aliases = []string{"q"} + queryCmd.AddCommand(cmds.Queries...) + rootCmd.AddCommand(queryCmd) + } + } + + if len(cmds.Txs) > 0 { + if txCmd := findSubCommand(rootCmd, "tx"); txCmd != nil { + txCmd.AddCommand(cmds.Txs...) + } else { + txCmd := topLevelCmd(rootCmd.Context(), "tx", "Transaction subcommands") + txCmd.AddCommand(cmds.Txs...) + rootCmd.AddCommand(txCmd) + } + } + return nil } // configHandle writes the default config to the home directory if it does not exist and sets the server context -func configHandle[AppT AppI[T], T transaction.Tx](s *Server[AppT, T], home string, cmd *cobra.Command) error { +func configHandle[AppT AppI[T], T transaction.Tx](s *Server[AppT, T], cmd *cobra.Command) error { + home, err := cmd.Flags().GetString(FlagHome) + if err != nil { + return err + } + configDir := filepath.Join(home, "config") // we need to check app.toml as the config folder can already exist for the client.toml @@ -145,19 +173,53 @@ func configHandle[AppT AppI[T], T transaction.Tx](s *Server[AppT, T], home strin } } - viper, err := ReadConfig(configDir) - if err != nil { - return err - } - viper.Set(FlagHome, home) - if err := viper.BindPFlags(cmd.Flags()); err != nil { - return err - } - - log, err := NewLogger(viper, cmd.OutOrStdout()) + v, err := ReadConfig(configDir) if err != nil { return err } - return SetCmdServerContext(cmd, viper, log) + if err := v.BindPFlags(cmd.Flags()); err != nil { + return err + } + + log, err := NewLogger(v, cmd.OutOrStdout()) + if err != nil { + return err + } + + return SetCmdServerContext(cmd, v, log) +} + +// findSubCommand finds a sub-command of the provided command whose Use +// string is or begins with the provided subCmdName. +// It verifies the command's aliases as well. +func findSubCommand(cmd *cobra.Command, subCmdName string) *cobra.Command { + for _, subCmd := range cmd.Commands() { + use := subCmd.Use + if use == subCmdName || strings.HasPrefix(use, subCmdName+" ") { + return subCmd + } + + for _, alias := range subCmd.Aliases { + if alias == subCmdName || strings.HasPrefix(alias, subCmdName+" ") { + return subCmd + } + } + } + return nil +} + +// topLevelCmd creates a new top-level command with the provided name and +// description. The command will have DisableFlagParsing set to false and +// SuggestionsMinimumDistance set to 2. +func topLevelCmd(ctx context.Context, use, short string) *cobra.Command { + cmd := &cobra.Command{ + Use: use, + Short: short, + DisableFlagParsing: false, + SuggestionsMinimumDistance: 2, + } + cmd.SetContext(ctx) + + return cmd } diff --git a/server/v2/config.go b/server/v2/config.go index 1a753df648..cb5fbeae7c 100644 --- a/server/v2/config.go +++ b/server/v2/config.go @@ -2,15 +2,6 @@ package serverv2 import "github.com/spf13/cobra" -var ServerContextKey = struct{}{} - -// Config is the config of the main server. -type Config struct { - // StartBlock indicates if the server should block or not. - // If true, the server will block until the context is canceled. - StartBlock bool -} - // CLIConfig defines the CLI configuration for a module server. type CLIConfig struct { // Commands defines the main command of a module server. diff --git a/server/v2/logger.go b/server/v2/logger.go index 51a64fdccd..5b89486c27 100644 --- a/server/v2/logger.go +++ b/server/v2/logger.go @@ -6,12 +6,13 @@ import ( "github.com/rs/zerolog" "github.com/spf13/viper" + corelog "cosmossdk.io/core/log" "cosmossdk.io/log" ) // NewLogger creates a the default SDK logger. // It reads the log level and format from the server context. -func NewLogger(v *viper.Viper, out io.Writer) (log.Logger, error) { +func NewLogger(v *viper.Viper, out io.Writer) (corelog.Logger, error) { var opts []log.Option if v.GetString(FlagLogFormat) == OutputFormatJSON { opts = append(opts, log.OutputJSONOption()) diff --git a/server/v2/server.go b/server/v2/server.go index 762488e5fa..a8773b7944 100644 --- a/server/v2/server.go +++ b/server/v2/server.go @@ -97,10 +97,7 @@ func (s *Server[AppT, T]) Start(ctx context.Context) error { return fmt.Errorf("failed to start servers: %w", err) } - serverCfg := ctx.Value(ServerContextKey).(Config) - if serverCfg.StartBlock { - <-ctx.Done() - } + <-ctx.Done() return nil } @@ -127,13 +124,13 @@ func (s *Server[AppT, T]) CLICommands() CLIConfig { return cmds[0] } - rootCmd := &cobra.Command{ + subCmd := &cobra.Command{ Use: name, Short: fmt.Sprintf("Commands from the %s server component", name), } - rootCmd.AddCommand(cmds...) + subCmd.AddCommand(cmds...) - return rootCmd + return subCmd } commands := CLIConfig{} @@ -197,6 +194,9 @@ func (s *Server[AppT, T]) WriteConfig(configPath string) error { } for _, component := range s.components { + // undocumented interface to write the component default config in another file than app.toml + // it is used by cometbft for backward compatibility + // it should not be used by other components if mod, ok := component.(interface{ WriteDefaultConfigAt(string) error }); ok { if err := mod.WriteDefaultConfigAt(configPath); err != nil { return err diff --git a/server/v2/server_test.go b/server/v2/server_test.go index 6ad73cffa4..2155e0e1af 100644 --- a/server/v2/server_test.go +++ b/server/v2/server_test.go @@ -99,9 +99,7 @@ func TestServer(t *testing.T) { } // start empty - ctx := context.Background() - ctx = context.WithValue(ctx, serverv2.ServerContextKey, serverv2.Config{StartBlock: true}) - ctx, cancelFn := context.WithCancel(ctx) + ctx, cancelFn := context.WithCancel(context.TODO()) go func() { // wait 5sec and cancel context <-time.After(5 * time.Second) diff --git a/server/v2/util.go b/server/v2/util.go index b2a1f7f7df..ca63ff254c 100644 --- a/server/v2/util.go +++ b/server/v2/util.go @@ -2,12 +2,13 @@ package serverv2 import ( "context" - "os" + "fmt" "github.com/spf13/cobra" "github.com/spf13/viper" corectx "cosmossdk.io/core/context" + corelog "cosmossdk.io/core/log" "cosmossdk.io/log" ) @@ -21,26 +22,28 @@ func SetCmdServerContext(cmd *cobra.Command, viper *viper.Viper, logger log.Logg cmdCtx = cmd.Context() } - cmd.SetContext(context.WithValue(cmdCtx, corectx.LoggerContextKey{}, logger)) - cmd.SetContext(context.WithValue(cmdCtx, corectx.ViperContextKey{}, viper)) + cmdCtx = context.WithValue(cmdCtx, corectx.LoggerContextKey, logger) + cmdCtx = context.WithValue(cmdCtx, corectx.ViperContextKey, viper) + cmd.SetContext(cmdCtx) return nil } func GetViperFromCmd(cmd *cobra.Command) *viper.Viper { - value := cmd.Context().Value(corectx.ViperContextKey{}) + value := cmd.Context().Value(corectx.ViperContextKey) v, ok := value.(*viper.Viper) if !ok { - return viper.New() + panic(fmt.Sprintf("incorrect viper type %T: expected *viper.Viper. Have you forgot to set the viper in the command context?", value)) } return v } -func GetLoggerFromCmd(cmd *cobra.Command) log.Logger { - v := cmd.Context().Value(corectx.LoggerContextKey{}) - logger, ok := v.(log.Logger) +func GetLoggerFromCmd(cmd *cobra.Command) corelog.Logger { + v := cmd.Context().Value(corectx.LoggerContextKey) + logger, ok := v.(corelog.Logger) if !ok { - return log.NewLogger(os.Stdout) + panic(fmt.Sprintf("incorrect logger type %T: expected log.Logger. Have you forgot to set the logger in the command context?", v)) } + return logger } diff --git a/simapp/simd/cmd/testnet_test.go b/simapp/simd/cmd/testnet_test.go index fee803721e..e76818971b 100644 --- a/simapp/simd/cmd/testnet_test.go +++ b/simapp/simd/cmd/testnet_test.go @@ -68,8 +68,8 @@ func Test_TestnetCmd(t *testing.T) { WithValidatorAddressCodec(cdcOpts.GetValidatorCodec()) ctx := context.Background() - ctx = context.WithValue(ctx, corectx.ViperContextKey{}, viper) - ctx = context.WithValue(ctx, corectx.LoggerContextKey{}, logger) + ctx = context.WithValue(ctx, corectx.ViperContextKey, viper) + ctx = context.WithValue(ctx, corectx.LoggerContextKey, logger) ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx) cmd := testnetInitFilesCmd(moduleManager, banktypes.GenesisBalancesIterator{}) cmd.SetArgs([]string{fmt.Sprintf("--%s=test", flags.FlagKeyringBackend), fmt.Sprintf("--output-dir=%s", home)}) diff --git a/simapp/v2/simdv2/cmd/commands.go b/simapp/v2/simdv2/cmd/commands.go index ea7f252b5d..2aeed7aed0 100644 --- a/simapp/v2/simdv2/cmd/commands.go +++ b/simapp/v2/simdv2/cmd/commands.go @@ -104,7 +104,7 @@ func initRootCmd[AppT serverv2.AppI[T], T transaction.Tx]( rootCmd, newApp, logger, - cometbft.New[AppT, T](&temporaryTxDecoder[T]{txConfig}), + cometbft.New[AppT, T](&temporaryTxDecoder[T]{txConfig}, cometbft.DefaultServerOptions[T]()), grpc.New[AppT, T](), ); err != nil { panic(err) diff --git a/simapp/v2/simdv2/cmd/root_di.go b/simapp/v2/simdv2/cmd/root_di.go index aff106a95a..61c3551803 100644 --- a/simapp/v2/simdv2/cmd/root_di.go +++ b/simapp/v2/simdv2/cmd/root_di.go @@ -61,10 +61,6 @@ func NewRootCmd[AppT serverv2.AppI[T], T transaction.Tx]() *cobra.Command { Short: "simulation app", SilenceErrors: true, PersistentPreRunE: func(cmd *cobra.Command, _ []string) error { - // set the default command outputs - cmd.SetOut(cmd.OutOrStdout()) - cmd.SetErr(cmd.ErrOrStderr()) - clientCtx = clientCtx.WithCmdContext(cmd.Context()) clientCtx, err := client.ReadPersistentCommandFlags(clientCtx, cmd.Flags()) if err != nil { diff --git a/tests/e2e/genutil/export_test.go b/tests/e2e/genutil/export_test.go index 7f9ad1b4da..e7ff24f10a 100644 --- a/tests/e2e/genutil/export_test.go +++ b/tests/e2e/genutil/export_test.go @@ -59,7 +59,7 @@ func TestExportCmd_ConsensusParams(t *testing.T) { func TestExportCmd_HomeDir(t *testing.T) { _, ctx, _, cmd := setupApp(t, t.TempDir()) - v := ctx.Value(corectx.ViperContextKey{}) + v := ctx.Value(corectx.ViperContextKey) viper, ok := v.(*viper.Viper) require.True(t, ok) viper.Set(flags.FlagHome, "foobar") @@ -220,7 +220,7 @@ func setupApp(t *testing.T, tempDir string) (*simapp.SimApp, context.Context, ge ctx := context.Background() ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx) - ctx = context.WithValue(ctx, corectx.ViperContextKey{}, viper) + ctx = context.WithValue(ctx, corectx.ViperContextKey, viper) return app, ctx, appGenesis, cmd } diff --git a/x/genutil/client/cli/export_test.go b/x/genutil/client/cli/export_test.go index 3c3c870c7f..e492e100b1 100644 --- a/x/genutil/client/cli/export_test.go +++ b/x/genutil/client/cli/export_test.go @@ -73,8 +73,8 @@ func NewExportSystem(t *testing.T, exporter types.AppExporter) *ExportSystem { cCtx := (client.Context{}).WithHomeDir(homeDir) - ctx := context.WithValue(context.Background(), corectx.ViperContextKey{}, viper) - ctx = context.WithValue(ctx, corectx.LoggerContextKey{}, logger) + ctx := context.WithValue(context.Background(), corectx.ViperContextKey, viper) + ctx = context.WithValue(ctx, corectx.LoggerContextKey, logger) ctx = context.WithValue(ctx, client.ClientContextKey, &cCtx) diff --git a/x/genutil/client/cli/genaccount_test.go b/x/genutil/client/cli/genaccount_test.go index 6351a76ea3..d2c6c08b67 100644 --- a/x/genutil/client/cli/genaccount_test.go +++ b/x/genutil/client/cli/genaccount_test.go @@ -92,8 +92,8 @@ func TestAddGenesisAccountCmd(t *testing.T) { ctx := context.Background() ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx) - ctx = context.WithValue(ctx, corectx.ViperContextKey{}, viper) - ctx = context.WithValue(ctx, corectx.LoggerContextKey{}, logger) + ctx = context.WithValue(ctx, corectx.ViperContextKey, viper) + ctx = context.WithValue(ctx, corectx.LoggerContextKey, logger) cmd := genutilcli.AddGenesisAccountCmd(addresscodec.NewBech32Codec("cosmos")) cmd.SetArgs([]string{ diff --git a/x/genutil/client/cli/init_test.go b/x/genutil/client/cli/init_test.go index ba90403be8..7dfa70b688 100644 --- a/x/genutil/client/cli/init_test.go +++ b/x/genutil/client/cli/init_test.go @@ -78,8 +78,8 @@ func TestInitCmd(t *testing.T) { ctx := context.Background() ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx) - ctx = context.WithValue(ctx, corectx.ViperContextKey{}, viper) - ctx = context.WithValue(ctx, corectx.LoggerContextKey{}, logger) + ctx = context.WithValue(ctx, corectx.ViperContextKey, viper) + ctx = context.WithValue(ctx, corectx.LoggerContextKey, logger) cmd := genutilcli.InitCmd(testMbm) cmd.SetArgs( @@ -112,8 +112,8 @@ func TestInitRecover(t *testing.T) { ctx := context.Background() ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx) - ctx = context.WithValue(ctx, corectx.ViperContextKey{}, viper) - ctx = context.WithValue(ctx, corectx.LoggerContextKey{}, logger) + ctx = context.WithValue(ctx, corectx.ViperContextKey, viper) + ctx = context.WithValue(ctx, corectx.LoggerContextKey, logger) cmd := genutilcli.InitCmd(testMbm) cmd.SetContext(ctx) @@ -145,8 +145,8 @@ func TestInitDefaultBondDenom(t *testing.T) { ctx := context.Background() ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx) - ctx = context.WithValue(ctx, corectx.ViperContextKey{}, viper) - ctx = context.WithValue(ctx, corectx.LoggerContextKey{}, logger) + ctx = context.WithValue(ctx, corectx.ViperContextKey, viper) + ctx = context.WithValue(ctx, corectx.LoggerContextKey, logger) cmd := genutilcli.InitCmd(testMbm) @@ -173,8 +173,8 @@ func TestEmptyState(t *testing.T) { ctx := context.Background() ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx) - ctx = context.WithValue(ctx, corectx.ViperContextKey{}, viper) - ctx = context.WithValue(ctx, corectx.LoggerContextKey{}, logger) + ctx = context.WithValue(ctx, corectx.ViperContextKey, viper) + ctx = context.WithValue(ctx, corectx.LoggerContextKey, logger) cmd := genutilcli.InitCmd(testMbm) cmd.SetArgs([]string{"appnode-test"}) @@ -268,8 +268,8 @@ func TestInitConfig(t *testing.T) { ctx := context.Background() ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx) - ctx = context.WithValue(ctx, corectx.ViperContextKey{}, viper) - ctx = context.WithValue(ctx, corectx.LoggerContextKey{}, logger) + ctx = context.WithValue(ctx, corectx.ViperContextKey, viper) + ctx = context.WithValue(ctx, corectx.LoggerContextKey, logger) cmd := genutilcli.InitCmd(testMbm) cmd.SetArgs([]string{"testnode"}) @@ -318,8 +318,8 @@ func TestInitWithHeight(t *testing.T) { ctx := context.Background() ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx) - ctx = context.WithValue(ctx, corectx.ViperContextKey{}, viper) - ctx = context.WithValue(ctx, corectx.LoggerContextKey{}, logger) + ctx = context.WithValue(ctx, corectx.ViperContextKey, viper) + ctx = context.WithValue(ctx, corectx.LoggerContextKey, logger) testInitialHeight := int64(333) @@ -355,8 +355,8 @@ func TestInitWithNegativeHeight(t *testing.T) { ctx := context.Background() ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx) - ctx = context.WithValue(ctx, corectx.ViperContextKey{}, viper) - ctx = context.WithValue(ctx, corectx.LoggerContextKey{}, logger) + ctx = context.WithValue(ctx, corectx.ViperContextKey, viper) + ctx = context.WithValue(ctx, corectx.LoggerContextKey, logger) testInitialHeight := int64(-333) diff --git a/x/genutil/client/testutil/helpers.go b/x/genutil/client/testutil/helpers.go index 54df530e86..459f325bc7 100644 --- a/x/genutil/client/testutil/helpers.go +++ b/x/genutil/client/testutil/helpers.go @@ -35,8 +35,8 @@ func ExecInitCmd(mm *module.Manager, home string, cdc codec.Codec) error { ctx := context.Background() ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx) - ctx = context.WithValue(ctx, corectx.ViperContextKey{}, viper) - ctx = context.WithValue(ctx, corectx.LoggerContextKey{}, logger) + ctx = context.WithValue(ctx, corectx.ViperContextKey, viper) + ctx = context.WithValue(ctx, corectx.LoggerContextKey, logger) cmd.SetArgs([]string{"appnode-test"})