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(server/v2): add config & start helper #20505

Merged
merged 146 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from 139 commits
Commits
Show all changes
146 commits
Select commit Hold shift + click to select a range
d530e80
upstream more changes from v2
tac0turtle May 14, 2024
0a15db8
add proto
tac0turtle May 14, 2024
8455318
chagnes
tac0turtle May 15, 2024
62ded17
fix runtime
tac0turtle May 15, 2024
5c4a43a
remove sdk import
tac0turtle May 15, 2024
decf4a2
lint issues
tac0turtle May 15, 2024
dbf509e
remove config stuff
tac0turtle May 15, 2024
d845064
Merge branch 'main' into marko/upstream_more_v2
tac0turtle May 15, 2024
ed638cd
go mod changes
tac0turtle May 15, 2024
cb2cc01
go mod changes
tac0turtle May 15, 2024
da38aff
Merge branch 'main' of github.com:cosmos/cosmos-sdk into marko/upstre…
kocubinski May 16, 2024
2691838
pause here
kocubinski May 16, 2024
faba9d1
revert for cherry-pick
kocubinski May 16, 2024
35fd591
refactor: sdk.Tx implements transaction.Tx (#19588)
likhita-809 Mar 13, 2024
e394e18
go mod tidy
kocubinski May 16, 2024
cce35fc
progress on txs
kocubinski May 16, 2024
d2241fa
seem to have handled all errors
kocubinski May 16, 2024
15619d9
lots of progress
kocubinski May 16, 2024
ca160d8
init, key, and genesis commands working
kocubinski May 16, 2024
65db2cf
back to unwrap errs, yay
kocubinski May 16, 2024
7308c68
progress to same err as server-v2
kocubinski May 16, 2024
2a12963
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/simapp-v2
kocubinski May 17, 2024
a7503e4
defer router build
kocubinski May 17, 2024
1eac3b2
fix store bug
kocubinski May 17, 2024
2c3ce95
wait goroutine
kocubinski May 17, 2024
aa91391
add start up script
kocubinski May 17, 2024
d3b3a24
special case hack for genesis
kocubinski May 17, 2024
05942a0
recursive implementation of GetStateChanges
kocubinski May 19, 2024
3fd6eb3
chore: go mod tidy
kocubinski May 19, 2024
a9ef4e4
use comet info service with comet info in context
kocubinski May 19, 2024
290263f
sort of working
kocubinski May 19, 2024
00f6999
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/simapp-v2
kocubinski May 19, 2024
a63762e
revert x/tx changes
kocubinski May 20, 2024
4b86a66
fix app_v1 build
kocubinski May 20, 2024
04b8f98
fix simapp/v2 go.mod
kocubinski May 20, 2024
73e00c0
fix distribution integration test
kocubinski May 21, 2024
e9e8ab8
fix distribution integration test
kocubinski May 21, 2024
b16d2bc
linting fixes
kocubinski May 21, 2024
330b8ed
more lint fix
kocubinski May 21, 2024
fc684c5
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/simapp-v2
kocubinski May 21, 2024
108978d
fix docker image
kocubinski May 21, 2024
be3b27b
fix test
kocubinski May 21, 2024
24e019b
rm debug print
kocubinski May 21, 2024
2ca0c70
more lint fix
kocubinski May 21, 2024
b427037
Merge branch 'main' into kocu/simapp-v2
kocubinski May 21, 2024
02947fa
add changelog for x/distribution break
kocubinski May 21, 2024
5bf88be
rm conseneus.MsgUpdateCometInfo
kocubinski May 22, 2024
e55810e
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/simapp-v2
kocubinski May 28, 2024
b50f50c
go mod tidy
kocubinski May 28, 2024
b029625
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/simapp-v2
kocubinski May 28, 2024
cbab735
fixes from merging main
kocubinski May 28, 2024
2738109
more fixes, go mod tidy
kocubinski May 28, 2024
92539ec
fix nil ref
kocubinski May 28, 2024
c95c217
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/simapp-v2
kocubinski May 29, 2024
bb09616
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/simapp-v2
kocubinski May 29, 2024
bb1c1cc
go mod tidy all
kocubinski May 29, 2024
f1b9a29
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/simapp-v2
kocubinski May 30, 2024
c529edd
go mod tidy
kocubinski May 30, 2024
3e8ce5a
change server
hieuvubk May 31, 2024
27a75ba
simapp v2
hieuvubk May 31, 2024
4b125e1
testdata
hieuvubk May 31, 2024
8b2025c
Hieu/v2 config start (#20504)
hieuvubk May 31, 2024
6eda0c7
temp appCreator
hieuvubk May 31, 2024
bf16d79
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/simapp-v2
kocubinski May 31, 2024
f235226
add TODO
kocubinski May 31, 2024
87d1c5b
add TODO
kocubinski May 31, 2024
aa21e93
add TODO
kocubinski May 31, 2024
52b883e
clean up
kocubinski May 31, 2024
acfb730
comments
kocubinski May 31, 2024
aa0cebd
lint fix
kocubinski May 31, 2024
fd34264
fix issue #20492
kocubinski May 31, 2024
9557e39
merge upstream
hieuvubk Jun 1, 2024
cadd5e1
refactor
hieuvubk Jun 1, 2024
2a2b3f1
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/simapp-v2
kocubinski Jun 2, 2024
98ff414
refactor server structure
hieuvubk Jun 3, 2024
d6e93ec
remove write config
hieuvubk Jun 3, 2024
0379c26
go mod tidy
hieuvubk Jun 3, 2024
bf3cc8a
use viper.Sub()
hieuvubk Jun 3, 2024
2bb2b90
app version modifier clean up
kocubinski Jun 3, 2024
ce36a1c
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/simapp-v2
kocubinski Jun 3, 2024
b9be347
rename
hieuvubk Jun 5, 2024
647d92b
codectypes => coreapp
hieuvubk Jun 5, 2024
44c465b
lint
tac0turtle Jun 6, 2024
faf1450
remove comment
Jun 6, 2024
a2312d0
add todo
Jun 6, 2024
591bb8e
use separate identity for STF
Jun 6, 2024
bd7efda
write comet config itself
hieuvubk Jun 6, 2024
f9c110d
remove tempDir()
hieuvubk Jun 6, 2024
83f873a
handle config in cmd prerun
hieuvubk Jun 6, 2024
6d2c424
rename
hieuvubk Jun 6, 2024
88c260a
lint
hieuvubk Jun 6, 2024
4588c33
resolved conflict
hieuvubk Jun 6, 2024
6d6cf45
go mod tidy
hieuvubk Jun 6, 2024
128794b
read config test
hieuvubk Jun 6, 2024
1c074d3
Merge branch 'main' into kocu/simapp-v2
tac0turtle Jun 6, 2024
6f36227
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/simapp-v2
kocubinski Jun 6, 2024
2ad7cbf
Merge branch 'kocu/simapp-v2' of github.com:cosmos/cosmos-sdk into ko…
kocubinski Jun 6, 2024
5a9e899
clean up go.mod
kocubinski Jun 6, 2024
6db27f3
address comments
hieuvubk Jun 7, 2024
417029d
merge upstream
hieuvubk Jun 7, 2024
7d72206
replace log
hieuvubk Jun 10, 2024
0a5b6d0
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/simapp-v2
kocubinski Jun 11, 2024
4f54b95
go mod tidy
kocubinski Jun 11, 2024
2759dab
fix bad merge bits
kocubinski Jun 11, 2024
fb32eaf
hardcode provide stf key in runtime/v2
kocubinski Jun 11, 2024
c6f6902
rever to main x/distribution
kocubinski Jun 11, 2024
64a51f9
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/simapp-v2
kocubinski Jun 12, 2024
f59e682
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/simapp-v2
kocubinski Jun 12, 2024
99bc320
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/simapp-v2
kocubinski Jun 13, 2024
8ebdc82
fix linting
tac0turtle Jun 14, 2024
eb6d1f6
update startup script
kocubinski Jun 14, 2024
921b627
fix logic error from #20296
kocubinski Jun 14, 2024
d10b7dd
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/simapp-v2
kocubinski Jun 17, 2024
83440d8
rm whitespace commits
kocubinski Jun 17, 2024
08cf9af
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/simapp-v2
kocubinski Jun 17, 2024
68c61d5
prototype change for x/staking
kocubinski Jun 17, 2024
959542e
improve test script for CI run
kocubinski Jun 17, 2024
88a2a0b
proto CI job
kocubinski Jun 17, 2024
20ef626
Merge branch 'main' into kocu/simapp-v2
kocubinski Jun 17, 2024
e7178c6
go mod tidy
kocubinski Jun 17, 2024
14419bd
Merge branch 'kocu/simapp-v2' of github.com:cosmos/cosmos-sdk into ko…
kocubinski Jun 17, 2024
6ed4b0f
go mod tidy all
kocubinski Jun 17, 2024
343258c
rm sed usage
kocubinski Jun 17, 2024
8c1e2d2
Merge branch 'main' of github.com:cosmos/cosmos-sdk into kocu/simapp-v2
kocubinski Jun 18, 2024
d914de2
Merge branch 'main' into kocu/simapp-v2
kocubinski Jun 18, 2024
6577d23
fetch x/auth/tx changes from #20698
kocubinski Jun 18, 2024
2dc02c0
Merge branch 'kocu/simapp-v2' of github.com:cosmos/cosmos-sdk into ko…
kocubinski Jun 18, 2024
03e791b
rm dead code
kocubinski Jun 18, 2024
fd39547
rm dead file
kocubinski Jun 18, 2024
b02723f
Merge branch 'main' into kocu/simapp-v2
kocubinski Jun 18, 2024
c73e11e
Merge branch 'main' into kocu/simapp-v2
kocubinski Jun 18, 2024
0243a83
Merge branch 'main' into kocu/simapp-v2
kocubinski Jun 19, 2024
40f9393
return err in depinject provider
kocubinski Jun 19, 2024
77e1603
wrap err in depinject provider
kocubinski Jun 19, 2024
921be8a
Merge branch 'main' into kocu/simapp-v2
kocubinski Jun 19, 2024
b461b1e
merge upstream
hieuvubk Jun 19, 2024
5faae0e
Merge branch 'main' into hieu/v2_config_start
julienrbrt Jun 19, 2024
f1b548e
merge main
hieuvubk Jun 19, 2024
a53bec5
merge main
hieuvubk Jun 19, 2024
b55308c
fixes and improvements
julienrbrt Jun 19, 2024
1984513
updates
julienrbrt Jun 19, 2024
5efd125
updates
julienrbrt Jun 19, 2024
badaa02
updates
julienrbrt Jun 19, 2024
4b516cb
updates
julienrbrt Jun 19, 2024
58c7acd
Merge branch 'main' into hieu/v2_config_start
julienrbrt Jun 19, 2024
f4c1db7
go mod tidy
julienrbrt Jun 19, 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
4 changes: 4 additions & 0 deletions runtime/v2/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,7 @@ func (a *App) SetAppVersion(context.Context, uint64) error {
func (a *App) AppVersion(context.Context) (uint64, error) {
return 0, nil
}

func (a *App) GetAppManager() *appmanager.AppManager[transaction.Tx] {
return a.AppManager
}
14 changes: 10 additions & 4 deletions server/v2/api/grpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ import (

_ "cosmossdk.io/api/amino" // Import amino.proto file for reflection
appmanager "cosmossdk.io/core/app"
"cosmossdk.io/core/transaction"
"cosmossdk.io/log"
serverv2 "cosmossdk.io/server/v2"
"cosmossdk.io/server/v2/api/grpc/gogoreflection"
)

Expand All @@ -33,9 +35,13 @@ type GRPCService interface {
RegisterGRPCServer(gogogrpc.Server)
}

// New returns a correctly configured and initialized gRPC server.
func NewGRPCServer() GRPCServer {
return GRPCServer{}
}
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved

// Init returns a correctly configured and initialized gRPC server.
// Note, the caller is responsible for starting the server.
func New(logger log.Logger, v *viper.Viper, interfaceRegistry appmanager.InterfaceRegistry, app GRPCService) (GRPCServer, error) {
func (g GRPCServer) Init(appI serverv2.App[transaction.Tx], v *viper.Viper, logger log.Logger) (serverv2.ServerComponent[transaction.Tx], error) {
cfg := DefaultConfig()
if v != nil {
if err := v.Sub(serverName).Unmarshal(&cfg); err != nil {
hieuvubk marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -44,12 +50,12 @@ func New(logger log.Logger, v *viper.Viper, interfaceRegistry appmanager.Interfa
}

grpcSrv := grpc.NewServer(
grpc.ForceServerCodec(newProtoCodec(interfaceRegistry).GRPCCodec()),
grpc.ForceServerCodec(newProtoCodec(appI.Application.InterfaceRegistry()).GRPCCodec()),
grpc.MaxSendMsgSize(cfg.MaxSendMsgSize),
grpc.MaxRecvMsgSize(cfg.MaxRecvMsgSize),
)

app.RegisterGRPCServer(grpcSrv)
// app.RegisterGRPCServer(grpcSrv)

// Reflection allows external clients to see what services and methods
// the gRPC server exposes.
Expand Down
3 changes: 2 additions & 1 deletion server/v2/cometbft/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ replace (
cosmossdk.io/core => ../../../core
cosmossdk.io/core/testing => ../../../core/testing
cosmossdk.io/depinject => ../../../depinject
cosmossdk.io/log => ../../../log
cosmossdk.io/server/v2 => ../
cosmossdk.io/server/v2/appmanager => ../appmanager
cosmossdk.io/store => ../../../store
Expand Down Expand Up @@ -38,6 +39,7 @@ 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
google.golang.org/genproto/googleapis/api v0.0.0-20240318140521-94a12d6c2237
google.golang.org/grpc v1.64.0
google.golang.org/protobuf v1.34.2
Expand Down Expand Up @@ -156,7 +158,6 @@ require (
github.com/sourcegraph/conc v0.3.0 // indirect
github.com/spf13/afero v1.11.0 // indirect
github.com/spf13/cast v1.6.0 // indirect
github.com/spf13/viper v1.19.0 // indirect
github.com/stretchr/testify v1.9.0 // indirect
github.com/subosito/gotenv v1.6.0 // indirect
github.com/supranational/blst v0.3.11 // indirect
Expand Down
2 changes: 0 additions & 2 deletions server/v2/cometbft/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ cosmossdk.io/collections v0.4.0 h1:PFmwj2W8szgpD5nOd8GWH6AbYNi1f2J6akWXJ7P5t9s=
cosmossdk.io/collections v0.4.0/go.mod h1:oa5lUING2dP+gdDquow+QjlF45eL1t4TJDypgGd+tv0=
cosmossdk.io/errors v1.0.1 h1:bzu+Kcr0kS/1DuPBtUFdWjzLqyUuCiyHjyJB6srBV/0=
cosmossdk.io/errors v1.0.1/go.mod h1:MeelVSZThMi4bEakzhhhE/CKqVv3nOJDA25bIqRDu/U=
cosmossdk.io/log v1.3.1 h1:UZx8nWIkfbbNEWusZqzAx3ZGvu54TZacWib3EzUYmGI=
cosmossdk.io/log v1.3.1/go.mod h1:2/dIomt8mKdk6vl3OWJcPk2be3pGOS8OQaLUM/3/tCM=
cosmossdk.io/math v1.3.0 h1:RC+jryuKeytIiictDslBP9i1fhkVm6ZDmZEoNP316zE=
cosmossdk.io/math v1.3.0/go.mod h1:vnRTxewy+M7BtXBNFybkuhSH4WfedVAAnERHgVFhp3k=
cosmossdk.io/x/accounts/defaults/lockup v0.0.0-20240417181816-5e7aae0db1f5 h1:eb0kcGyaYHSS0do7+MIWg7UKlskSH01biRNENbm/zDA=
Expand Down
52 changes: 37 additions & 15 deletions server/v2/cometbft/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"crypto/sha256"
"encoding/json"
"fmt"
"path/filepath"

abciserver "github.com/cometbft/cometbft/abci/server"
cmtcmd "github.com/cometbft/cometbft/cmd/cometbft/commands"
Expand All @@ -15,7 +16,9 @@ import (
"github.com/cometbft/cometbft/proxy"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"github.com/spf13/viper"

corectx "cosmossdk.io/core/context"
"cosmossdk.io/core/log"
"cosmossdk.io/core/transaction"
serverv2 "cosmossdk.io/server/v2"
Expand All @@ -42,7 +45,7 @@ const (
FlagTrace = "trace"
)

var _ serverv2.ServerModule = (*CometBFTServer[transaction.Tx])(nil)
var _ serverv2.ServerComponent[transaction.Tx] = (*CometBFTServer[transaction.Tx])(nil)

type CometBFTServer[T transaction.Tx] struct {
Node *node.Node
Expand All @@ -61,20 +64,26 @@ type App[T transaction.Tx] interface {
GetStore() types.Store
}

func NewCometBFTServer[T transaction.Tx](
app *appmanager.AppManager[T],
store types.Store,
logger log.Logger,
cfg Config,
txCodec transaction.Codec[T],
) *CometBFTServer[T] {
func New[T transaction.Tx](txCodec transaction.Codec[T]) *CometBFTServer[T] {
consensus := &Consensus[T]{txCodec: txCodec}
return &CometBFTServer[T]{
App: consensus,
}
}

func (s *CometBFTServer[T]) Init(appI serverv2.App[T], v *viper.Viper, logger log.Logger) (serverv2.ServerComponent[T], error) {
app := appI.Application
store := appI.Store.(types.Store)

cfg := Config{CmtConfig: serverv2.GetConfigFromViper(v), ConsensusAuthority: app.GetConsensusAuthority()}
logger = logger.With("module", "cometbft-server")

// create noop mempool
mempool := mempool.NoOpMempool[T]{}

// create consensus
consensus := NewConsensus[T](app, mempool, store, cfg, txCodec, logger)
// txCodec should be in server from New()
consensus := NewConsensus[T](app.GetAppManager(), mempool, store, cfg, s.App.txCodec, logger)

consensus.SetPrepareProposalHandler(handlers.NoOpPrepareProposal[T]())
consensus.SetProcessProposalHandler(handlers.NoOpProcessProposal[T]())
Expand All @@ -93,18 +102,25 @@ func NewCometBFTServer[T transaction.Tx](
sm := snapshots.NewManager(snapshotStore, snapshots.SnapshotOptions{}, sc, ss, nil, logger) // TODO: set options somehow
consensus.SetSnapshotManager(sm)

s.config = cfg
s.App = consensus
s.logger = logger

return &CometBFTServer[T]{
logger: logger,
App: consensus,
config: cfg,
}
}, nil
}

func (s *CometBFTServer[T]) Name() string {
return "cometbft"
}

func (s *CometBFTServer[T]) Start(ctx context.Context) error {
viper := ctx.Value(corectx.ViperContextKey{}).(*viper.Viper)
cometConfig := serverv2.GetConfigFromViper(viper)

wrappedLogger := cometlog.CometLoggerWrapper{Logger: s.logger}
if s.config.Standalone {
svr, err := abciserver.NewServer(s.config.Addr, s.config.Transport, s.App)
Expand All @@ -117,20 +133,20 @@ func (s *CometBFTServer[T]) Start(ctx context.Context) error {
return svr.Start()
}

nodeKey, err := p2p.LoadOrGenNodeKey(s.config.CmtConfig.NodeKeyFile())
nodeKey, err := p2p.LoadOrGenNodeKey(cometConfig.NodeKeyFile())
if err != nil {
return err
}

s.Node, err = node.NewNode(
ctx,
s.config.CmtConfig,
pvm.LoadOrGenFilePV(s.config.CmtConfig.PrivValidatorKeyFile(), s.config.CmtConfig.PrivValidatorStateFile()),
cometConfig,
pvm.LoadOrGenFilePV(cometConfig.PrivValidatorKeyFile(), cometConfig.PrivValidatorStateFile()),
nodeKey,
proxy.NewLocalClientCreator(s.App),
getGenDocProvider(s.config.CmtConfig),
getGenDocProvider(cometConfig),
cmtcfg.DefaultDBProvider,
node.DefaultMetricsProvider(s.config.CmtConfig.Instrumentation),
node.DefaultMetricsProvider(cometConfig.Instrumentation),
wrappedLogger,
)
if err != nil {
Expand Down Expand Up @@ -223,3 +239,9 @@ func (s *CometBFTServer[T]) CLICommands() serverv2.CLIConfig {
},
}
}

func (s *CometBFTServer[T]) WriteDefaultConfigAt(configPath string) error {
cometConfig := cmtcfg.DefaultConfig()
cmtcfg.WriteConfigFile(filepath.Join(configPath, "config.toml"), cometConfig)
return nil
}
87 changes: 74 additions & 13 deletions server/v2/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,30 +11,48 @@

"github.com/spf13/cobra"

"cosmossdk.io/core/transaction"
"cosmossdk.io/log"
"github.com/spf13/viper"
)

func Commands(logger log.Logger, homePath string, modules ...ServerModule) (CLIConfig, error) {
if len(modules) == 0 {
// TODO figure if we should define default modules
// and if so it should be done here to avoid unnecessary dependencies
return CLIConfig{}, errors.New("no modules provided")
}
const flagHome = "home"

v, err := ReadConfig(filepath.Join(homePath, "config"))
if err != nil {
return CLIConfig{}, fmt.Errorf("failed to read config: %w", err)
type App[T transaction.Tx] struct {
Application Application[T]
Store any
}

type AppCreator[T transaction.Tx] func(*viper.Viper, log.Logger) App[T]

func Commands(rootCmd *cobra.Command, newApp AppCreator[transaction.Tx], logger log.Logger, components ...ServerComponent[transaction.Tx]) (CLIConfig, error) {
if len(components) == 0 {
// TODO figure if we should define default components
// and if so it should be done here to avoid uncessary dependencies
return CLIConfig{}, errors.New("no components provided")
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider providing default components when none are supplied.

The current implementation throws an error when no components are provided. Consider providing sensible defaults or a clear message guiding the user on how to supply components.

}

server := NewServer(logger, modules...)
server := NewServer(logger, components...)
flags := server.StartFlags()

startCmd := &cobra.Command{
Use: "start",
Short: "Run the application",
RunE: func(cmd *cobra.Command, args []string) error {
if err := v.BindPFlags(cmd.Flags()); err != nil { // the server modules are already instantiated here, so binding the flags is useless.
v := GetViperFromCmd(cmd)
l := GetLoggerFromCmd(cmd)

for _, startFlags := range flags {
v.BindPFlags(startFlags)
Fixed Show fixed Hide fixed
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
}
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved

if err := v.BindPFlags(cmd.Flags()); err != nil {
return err
}

app := newApp(v, l)
server.Init(app, v, l)
Fixed Show fixed Hide fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

The call to server.Init(app, v, l) lacks error handling, which could lead to issues if the initialization fails. Proper error management is crucial in this context.

- server.Init(app, v, l)
+ if err := server.Init(app, v, l); err != nil {
+     return err
+ }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
server.Init(app, v, l)
if err := server.Init(app, v, l); err != nil {
return err
}
Tools
GitHub Check: gosec

[warning] 54-54: Errors unhandled.
Errors unhandled.

srvConfig := Config{StartBlock: true}
ctx := cmd.Context()
ctx = context.WithValue(ctx, ServerContextKey, srvConfig)
Expand Down Expand Up @@ -65,12 +83,55 @@
return cmds, nil
}

func AddCommands(rootCmd *cobra.Command, logger log.Logger, homePath string, modules ...ServerModule) error {
cmds, err := Commands(logger, homePath, modules...)
func AddCommands(rootCmd *cobra.Command, newApp AppCreator[transaction.Tx], logger log.Logger, components ...ServerComponent[transaction.Tx]) error {
cmds, err := Commands(rootCmd, newApp, logger, components...)
if err != nil {
return err
}

server := NewServer(logger, components...)
originalPersistentPreRunE := rootCmd.PersistentPreRunE
rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error {
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
home, err := cmd.Flags().GetString(flagHome)
if err != nil {
return err
}

err = configHandle(server, home, cmd)
if err != nil {
return err
}

if rootCmd.PersistentPreRun != nil {
rootCmd.PersistentPreRun(cmd, args)
return nil
}

return originalPersistentPreRunE(cmd, args)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure proper handling of PersistentPreRun.

The logic here overrides PersistentPreRunE but does not account for the possibility that PersistentPreRun might be set. Consider integrating or documenting how both should be handled to avoid confusion or unintended behavior.

+ if rootCmd.PersistentPreRun != nil {
+     rootCmd.PersistentPreRun(cmd, args)
+ }

Committable suggestion was skipped due to low confidence.


rootCmd.AddCommand(cmds.Commands...)
return nil
}

func configHandle(s *Server, home string, cmd *cobra.Command) error {
if _, err := os.Stat(filepath.Join(home, "config")); os.IsNotExist(err) {
err = s.WriteConfig(filepath.Join(home, "config"))
if err != nil {
return err
}
}

viper, err := ReadConfig(filepath.Join(home, "config"))
if err != nil {
return err
}
viper.Set(flagHome, home)

log, err := NewLogger(viper, cmd.OutOrStdout())
if err != nil {
return err
}

return SetCmdServerContext(cmd, viper, log)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Optimize configuration reading and context setting.

The current implementation of configHandle could be optimized by checking for the existence of the configuration directory early and simplifying error handling. Consider refactoring to reduce redundancy and improve clarity.

func configHandle(s *Server, home string, cmd *cobra.Command) error {
    configPath := filepath.Join(home, "config")
    if _, err := os.Stat(configPath); os.IsNotExist(err) {
        if err := s.WriteConfig(configPath); err != nil {
            return err
        }
    }

    viper, err := ReadConfig(configPath)
    if err != nil {
        return err
    }
    viper.Set(flagHome, home)

    log, err := NewLogger(viper, cmd.OutOrStdout())
    if err != nil {
        return err
    }

    return SetCmdServerContext(cmd, viper, log)
}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func configHandle(s *Server, home string, cmd *cobra.Command) error {
if _, err := os.Stat(filepath.Join(home, "config")); os.IsNotExist(err) {
err = s.WriteConfig(filepath.Join(home, "config"))
if err != nil {
return err
}
}
viper, err := ReadConfig(filepath.Join(home, "config"))
if err != nil {
return err
}
viper.Set(flagHome, home)
log, err := NewLogger(viper, cmd.OutOrStdout())
if err != nil {
return err
}
return SetCmdServerContext(cmd, viper, log)
}
func configHandle(s *Server, home string, cmd *cobra.Command) error {
configPath := filepath.Join(home, "config")
if _, err := os.Stat(configPath); os.IsNotExist(err) {
if err := s.WriteConfig(configPath); err != nil {
return err
}
}
viper, err := ReadConfig(configPath)
if err != nil {
return err
}
viper.Set(flagHome, home)
log, err := NewLogger(viper, cmd.OutOrStdout())
if err != nil {
return err
}
return SetCmdServerContext(cmd, viper, log)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

The configHandle function performs essential configuration management tasks. However, the error handling within this function could be improved by adding detailed error messages, especially when configuration files are missing or incorrect.

- return err
+ return fmt.Errorf("error processing configuration at %s: %w", filepath.Join(home, "config"), err)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func configHandle(s *Server, home string, cmd *cobra.Command) error {
if _, err := os.Stat(filepath.Join(home, "config")); os.IsNotExist(err) {
err = s.WriteConfig(filepath.Join(home, "config"))
if err != nil {
return err
}
}
viper, err := ReadConfig(filepath.Join(home, "config"))
if err != nil {
return err
}
viper.Set(flagHome, home)
log, err := NewLogger(viper, cmd.OutOrStdout())
if err != nil {
return err
}
return SetCmdServerContext(cmd, viper, log)
}
func configHandle(s *Server, home string, cmd *cobra.Command) error {
if _, err := os.Stat(filepath.Join(home, "config")); os.IsNotExist(err) {
err = s.WriteConfig(filepath.Join(home, "config"))
if err != nil {
return fmt.Errorf("error processing configuration at %s: %w", filepath.Join(home, "config"), err)
}
}
viper, err := ReadConfig(filepath.Join(home, "config"))
if err != nil {
return fmt.Errorf("error processing configuration at %s: %w", filepath.Join(home, "config"), err)
}
viper.Set(flagHome, home)
log, err := NewLogger(viper, cmd.OutOrStdout())
if err != nil {
return fmt.Errorf("error processing configuration at %s: %w", filepath.Join(home, "config"), err)
}
return SetCmdServerContext(cmd, viper, log)
}

Loading
Loading