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(cli): remove duplicate --home flag #17215

Merged
merged 7 commits into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/slashing) [#17098](https://github.com/cosmos/cosmos-sdk/pull/17098) `NewMsgUnjail` takes a string instead of `sdk.ValAddress`
* (x/genutil) [#17098](https://github.com/cosmos/cosmos-sdk/pull/17098) `GenAppStateFromConfig`, AddGenesisAccountCmd and `GenTxCmd` takes an addresscodec to decode addresses
* (x/distribution) [#17098](https://github.com/cosmos/cosmos-sdk/pull/17098) `NewMsgDepositValidatorRewardsPool`, `NewMsgFundCommunityPool`, `NewMsgWithdrawValidatorCommission` and `NewMsgWithdrawDelegatorReward` takes a string instead of `sdk.ValAddress` or `sdk.AccAddress`
* (client) [#17215](https://github.com/cosmos/cosmos-sdk/pull/17215) `server.StartCmd`,`server.ExportCmd`,`server.NewRollbackCmd`,`pruning.Cmd`,`genutilcli.InitCmd`,`genutilcli.GenTxCmd`,`genutilcli.CollectGenTxsCmd`,`genutilcli.AddGenesisAccountCmd` does not take a home directory anymore. It is inferred from the root command.
zakir-code marked this conversation as resolved.
Show resolved Hide resolved

### CLI Breaking Changes

Expand Down
13 changes: 6 additions & 7 deletions client/pruning/main.go
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@ import (
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/server"
servertypes "github.com/cosmos/cosmos-sdk/server/types"
"github.com/cosmos/cosmos-sdk/version"
)

const FlagAppDBBackend = "app-db-backend"

// Cmd prunes the sdk root multi store history versions based on the pruning options
// specified by command flags.
func Cmd(appCreator servertypes.AppCreator, defaultNodeHome string) *cobra.Command {
func Cmd(appCreator servertypes.AppCreator) *cobra.Command {
cmd := &cobra.Command{
Use: "prune [pruning-method]",
Short: "Prune app history states by keeping the recent heights and deleting old heights",
Expand All @@ -35,14 +36,17 @@ The pruning option is provided via the 'pruning' argument or alternatively with

Note: When the --app-db-backend flag is not specified, the default backend type is 'goleveldb'.
Supported app-db-backend types include 'goleveldb', 'rocksdb', 'pebbledb'.`,
Example: "prune custom --pruning-keep-recent 100 --app-db-backend 'goleveldb'",
Example: fmt.Sprintf("%s prune custom --pruning-keep-recent 100 --app-db-backend 'goleveldb'", version.AppName),
Args: cobra.RangeArgs(0, 1),
RunE: func(cmd *cobra.Command, args []string) error {
// bind flags to the Context's Viper so we can get pruning options.
vp := viper.New()
if err := vp.BindPFlags(cmd.Flags()); err != nil {
return err
}
if err := vp.BindPFlags(cmd.PersistentFlags()); err != nil {
return err
}

// use the first argument if present to set the pruning method
if len(args) > 0 {
Expand All @@ -61,10 +65,6 @@ Supported app-db-backend types include 'goleveldb', 'rocksdb', 'pebbledb'.`,
)

home := vp.GetString(flags.FlagHome)
if home == "" {
home = defaultNodeHome
}

db, err := openDB(home, server.GetAppDBBackend(vp))
if err != nil {
return err
Expand Down Expand Up @@ -97,7 +97,6 @@ Supported app-db-backend types include 'goleveldb', 'rocksdb', 'pebbledb'.`,
},
}

cmd.Flags().String(flags.FlagHome, defaultNodeHome, "The application home directory")
cmd.Flags().String(FlagAppDBBackend, "", "The type of database for application and snapshots databases")
cmd.Flags().Uint64(server.FlagPruningKeepRecent, 0, "Number of recent heights to keep on disk (ignored if pruning is not 'custom')")
cmd.Flags().Uint64(server.FlagPruningInterval, 10,
Expand Down
6 changes: 1 addition & 5 deletions server/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const (
)

// ExportCmd dumps app state to JSON.
func ExportCmd(appExporter types.AppExporter, defaultNodeHome string) *cobra.Command {
func ExportCmd(appExporter types.AppExporter) *cobra.Command {
cmd := &cobra.Command{
Use: "export",
Short: "Export state to JSON",
Expand All @@ -32,9 +32,6 @@ func ExportCmd(appExporter types.AppExporter, defaultNodeHome string) *cobra.Com
serverCtx := GetServerContextFromCmd(cmd)
config := serverCtx.Config

homeDir, _ := cmd.Flags().GetString(flags.FlagHome)
config.SetRoot(homeDir)

if _, err := os.Stat(config.GenesisFile()); os.IsNotExist(err) {
return err
}
Expand Down Expand Up @@ -115,7 +112,6 @@ func ExportCmd(appExporter types.AppExporter, defaultNodeHome string) *cobra.Com
},
}

cmd.Flags().String(flags.FlagHome, defaultNodeHome, "The application home directory")
cmd.Flags().Int64(FlagHeight, -1, "Export state from a particular height (-1 means latest height)")
cmd.Flags().Bool(FlagForZeroHeight, false, "Export state to start at height zero (perform preproccessing)")
cmd.Flags().StringSlice(FlagJailAllowedAddrs, []string{}, "Comma-separated list of operator addresses of jailed validators to unjail")
Expand Down
4 changes: 2 additions & 2 deletions server/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ func NewExportSystem(t *testing.T, exporter types.AppExporter) *ExportSystem {

sys := cmdtest.NewSystem()
sys.AddCommands(
server.ExportCmd(exporter, homeDir),
genutilcli.InitCmd(module.NewBasicManager(), homeDir),
server.ExportCmd(exporter),
genutilcli.InitCmd(module.NewBasicManager()),
)

tw := zerolog.NewTestWriter(t)
Expand Down
9 changes: 3 additions & 6 deletions server/rollback.go
Copy link
Member

Choose a reason for hiding this comment

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

nit: can we modify fmt.Printf("Rolled back state to height %d and hash %X", height, hash) to fmt.Printf("Rolled back state to height %d and hash %X\n", height, hash) (add a new line)

Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,11 @@ import (
cmtcmd "github.com/cometbft/cometbft/cmd/cometbft/commands"
"github.com/spf13/cobra"

"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/server/types"
)

// NewRollbackCmd creates a command to rollback CometBFT and multistore state by one height.
func NewRollbackCmd(appCreator types.AppCreator, defaultNodeHome string) *cobra.Command {
func NewRollbackCmd(appCreator types.AppCreator) *cobra.Command {
var removeBlock bool

cmd := &cobra.Command{
Expand All @@ -27,9 +26,8 @@ application.
`,
RunE: func(cmd *cobra.Command, args []string) error {
ctx := GetServerContextFromCmd(cmd)
cfg := ctx.Config
home := cfg.RootDir
db, err := openDB(home, GetAppDBBackend(ctx.Viper))

db, err := openDB(ctx.Config.RootDir, GetAppDBBackend(ctx.Viper))
zakir-code marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return err
}
Expand All @@ -50,7 +48,6 @@ application.
},
}

cmd.Flags().String(flags.FlagHome, defaultNodeHome, "The application home directory")
cmd.Flags().BoolVar(&removeBlock, "hard", false, "remove last block as well as state")
return cmd
}
8 changes: 3 additions & 5 deletions server/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
pruningtypes "cosmossdk.io/store/pruning/types"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/server/api"
serverconfig "github.com/cosmos/cosmos-sdk/server/config"
Expand Down Expand Up @@ -103,13 +102,13 @@ type StartCmdOptions struct {

// StartCmd runs the service passed in, either stand-alone or in-process with
// CometBFT.
func StartCmd(appCreator types.AppCreator, defaultNodeHome string) *cobra.Command {
return StartCmdWithOptions(appCreator, defaultNodeHome, StartCmdOptions{})
func StartCmd(appCreator types.AppCreator) *cobra.Command {
return StartCmdWithOptions(appCreator, StartCmdOptions{})
}

// StartCmdWithOptions runs the service passed in, either stand-alone or in-process with
// CometBFT.
func StartCmdWithOptions(appCreator types.AppCreator, defaultNodeHome string, opts StartCmdOptions) *cobra.Command {
func StartCmdWithOptions(appCreator types.AppCreator, opts StartCmdOptions) *cobra.Command {
if opts.DBOpener == nil {
opts.DBOpener = openDB
}
Expand Down Expand Up @@ -174,7 +173,6 @@ is performed. Note, when enabled, gRPC will also be automatically enabled.
},
}

cmd.Flags().String(flags.FlagHome, defaultNodeHome, "The application home directory")
cmd.Flags().Bool(flagWithComet, true, "Run abci app embedded in-process with CometBFT")
cmd.Flags().String(flagAddress, "tcp://0.0.0.0:26658", "Listen address")
cmd.Flags().String(flagTransport, "socket", "Transport protocol: socket, grpc")
Expand Down
8 changes: 4 additions & 4 deletions server/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ func interceptConfigs(rootViper *viper.Viper, customAppTemplate string, customCo
}

// add server commands
func AddCommands(rootCmd *cobra.Command, defaultNodeHome string, appCreator types.AppCreator, appExport types.AppExporter, addStartFlags types.ModuleInitFlags) {
func AddCommands(rootCmd *cobra.Command, appCreator types.AppCreator, appExport types.AppExporter, addStartFlags types.ModuleInitFlags) {
cometCmd := &cobra.Command{
Use: "comet",
Aliases: []string{"cometbft", "tendermint"},
Expand All @@ -328,15 +328,15 @@ func AddCommands(rootCmd *cobra.Command, defaultNodeHome string, appCreator type
BootstrapStateCmd(appCreator),
)

startCmd := StartCmd(appCreator, defaultNodeHome)
startCmd := StartCmd(appCreator)
addStartFlags(startCmd)

rootCmd.AddCommand(
startCmd,
cometCmd,
ExportCmd(appExport, defaultNodeHome),
ExportCmd(appExport),
version.NewVersionCommand(),
NewRollbackCmd(appCreator, defaultNodeHome),
NewRollbackCmd(appCreator),
)
}

Expand Down
56 changes: 32 additions & 24 deletions server/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,17 @@ func preRunETestImpl(cmd *cobra.Command, args []string) error {

func TestInterceptConfigsPreRunHandlerCreatesConfigFilesWhenMissing(t *testing.T) {
tempDir := t.TempDir()
cmd := server.StartCmd(nil, "/foobar")
if err := cmd.Flags().Set(flags.FlagHome, tempDir); err != nil {
cmd := server.StartCmd(nil)
cmd.PersistentFlags().String(flags.FlagHome, "/foobar", "")
if err := cmd.PersistentFlags().Set(flags.FlagHome, tempDir); err != nil {
t.Fatalf("Could not set home flag [%T] %v", err, err)
}

cmd.PreRunE = preRunETestImpl

serverCtx := &server.Context{}
ctx := context.WithValue(context.Background(), server.ServerContextKey, serverCtx)
if err := cmd.ExecuteContext(ctx); err != errCanceledInPreRun {
if err := cmd.ExecuteContext(ctx); !errors.Is(err, errCanceledInPreRun) {
t.Fatalf("function failed with [%T] %v", err, err)
}

Expand Down Expand Up @@ -115,8 +116,9 @@ func TestInterceptConfigsPreRunHandlerReadsConfigToml(t *testing.T) {
t.Fatalf("Failed closing config.toml: %v", err)
}

cmd := server.StartCmd(nil, "/foobar")
if err := cmd.Flags().Set(flags.FlagHome, tempDir); err != nil {
cmd := server.StartCmd(nil)
cmd.PersistentFlags().String(flags.FlagHome, "/foober", "")
zakir-code marked this conversation as resolved.
Show resolved Hide resolved
if err := cmd.PersistentFlags().Set(flags.FlagHome, tempDir); err != nil {
t.Fatalf("Could not set home flag [%T] %v", err, err)
}

Expand All @@ -125,7 +127,7 @@ func TestInterceptConfigsPreRunHandlerReadsConfigToml(t *testing.T) {
serverCtx := &server.Context{}
ctx := context.WithValue(context.Background(), server.ServerContextKey, serverCtx)

if err := cmd.ExecuteContext(ctx); err != errCanceledInPreRun {
if err := cmd.ExecuteContext(ctx); !errors.Is(err, errCanceledInPreRun) {
t.Fatalf("function failed with [%T] %v", err, err)
}

Expand Down Expand Up @@ -155,14 +157,15 @@ func TestInterceptConfigsPreRunHandlerReadsAppToml(t *testing.T) {
if err := writer.Close(); err != nil {
t.Fatalf("Failed closing app.toml: %v", err)
}
cmd := server.StartCmd(nil, tempDir)
cmd := server.StartCmd(nil)
cmd.PersistentFlags().String(flags.FlagHome, tempDir, "")

cmd.PreRunE = preRunETestImpl

serverCtx := &server.Context{}
ctx := context.WithValue(context.Background(), server.ServerContextKey, serverCtx)

if err := cmd.ExecuteContext(ctx); err != errCanceledInPreRun {
if err := cmd.ExecuteContext(ctx); !errors.Is(err, errCanceledInPreRun) {
t.Fatalf("function failed with [%T] %v", err, err)
}

Expand All @@ -174,9 +177,9 @@ func TestInterceptConfigsPreRunHandlerReadsAppToml(t *testing.T) {
func TestInterceptConfigsPreRunHandlerReadsFlags(t *testing.T) {
const testAddr = "tcp://127.1.2.3:12345"
tempDir := t.TempDir()
cmd := server.StartCmd(nil, "/foobar")

if err := cmd.Flags().Set(flags.FlagHome, tempDir); err != nil {
cmd := server.StartCmd(nil)
cmd.PersistentFlags().String(flags.FlagHome, "/foobar", "")
if err := cmd.PersistentFlags().Set(flags.FlagHome, tempDir); err != nil {
t.Fatalf("Could not set home flag [%T] %v", err, err)
}

Expand All @@ -190,7 +193,7 @@ func TestInterceptConfigsPreRunHandlerReadsFlags(t *testing.T) {
serverCtx := &server.Context{}
ctx := context.WithValue(context.Background(), server.ServerContextKey, serverCtx)

if err := cmd.ExecuteContext(ctx); err != errCanceledInPreRun {
if err := cmd.ExecuteContext(ctx); !errors.Is(err, errCanceledInPreRun) {
t.Fatalf("function failed with [%T] %v", err, err)
}

Expand All @@ -202,8 +205,9 @@ func TestInterceptConfigsPreRunHandlerReadsFlags(t *testing.T) {
func TestInterceptConfigsPreRunHandlerReadsEnvVars(t *testing.T) {
const testAddr = "tcp://127.1.2.3:12345"
tempDir := t.TempDir()
cmd := server.StartCmd(nil, "/foobar")
if err := cmd.Flags().Set(flags.FlagHome, tempDir); err != nil {
cmd := server.StartCmd(nil)
cmd.PersistentFlags().String(flags.FlagHome, "/foobar", "")
if err := cmd.PersistentFlags().Set(flags.FlagHome, tempDir); err != nil {
t.Fatalf("Could not set home flag [%T] %v", err, err)
}

Expand All @@ -225,7 +229,7 @@ func TestInterceptConfigsPreRunHandlerReadsEnvVars(t *testing.T) {
serverCtx := &server.Context{}
ctx := context.WithValue(context.Background(), server.ServerContextKey, serverCtx)

if err := cmd.ExecuteContext(ctx); err != errCanceledInPreRun {
if err := cmd.ExecuteContext(ctx); !errors.Is(err, errCanceledInPreRun) {
t.Fatalf("function failed with [%T] %v", err, err)
}

Expand Down Expand Up @@ -289,7 +293,8 @@ func newPrecedenceCommon(t *testing.T) precedenceCommon {
})

// Set up the command object that is used in this test
retval.cmd = server.StartCmd(nil, tempDir)
retval.cmd = server.StartCmd(nil)
retval.cmd.PersistentFlags().String(flags.FlagHome, tempDir, "")
retval.cmd.PreRunE = preRunETestImpl

return retval
Expand Down Expand Up @@ -331,7 +336,7 @@ func TestInterceptConfigsPreRunHandlerPrecedenceFlag(t *testing.T) {
serverCtx := &server.Context{}
ctx := context.WithValue(context.Background(), server.ServerContextKey, serverCtx)

if err := testCommon.cmd.ExecuteContext(ctx); err != errCanceledInPreRun {
if err := testCommon.cmd.ExecuteContext(ctx); !errors.Is(err, errCanceledInPreRun) {
t.Fatalf("function failed with [%T] %v", err, err)
}

Expand All @@ -347,7 +352,7 @@ func TestInterceptConfigsPreRunHandlerPrecedenceEnvVar(t *testing.T) {
serverCtx := &server.Context{}
ctx := context.WithValue(context.Background(), server.ServerContextKey, serverCtx)

if err := testCommon.cmd.ExecuteContext(ctx); err != errCanceledInPreRun {
if err := testCommon.cmd.ExecuteContext(ctx); !errors.Is(err, errCanceledInPreRun) {
t.Fatalf("function failed with [%T] %v", err, err)
}

Expand All @@ -363,7 +368,7 @@ func TestInterceptConfigsPreRunHandlerPrecedenceConfigFile(t *testing.T) {
serverCtx := &server.Context{}
ctx := context.WithValue(context.Background(), server.ServerContextKey, serverCtx)

if err := testCommon.cmd.ExecuteContext(ctx); err != errCanceledInPreRun {
if err := testCommon.cmd.ExecuteContext(ctx); !errors.Is(err, errCanceledInPreRun) {
t.Fatalf("function failed with [%T] %v", err, err)
}

Expand All @@ -379,7 +384,7 @@ func TestInterceptConfigsPreRunHandlerPrecedenceConfigDefault(t *testing.T) {
serverCtx := &server.Context{}
ctx := context.WithValue(context.Background(), server.ServerContextKey, serverCtx)

if err := testCommon.cmd.ExecuteContext(ctx); err != errCanceledInPreRun {
if err := testCommon.cmd.ExecuteContext(ctx); !errors.Is(err, errCanceledInPreRun) {
t.Fatalf("function failed with [%T] %v", err, err)
}

Expand All @@ -397,8 +402,9 @@ func TestInterceptConfigsWithBadPermissions(t *testing.T) {
if err := os.Mkdir(subDir, 0o600); err != nil {
t.Fatalf("Failed to create sub directory: %v", err)
}
cmd := server.StartCmd(nil, "/foobar")
if err := cmd.Flags().Set(flags.FlagHome, subDir); err != nil {
cmd := server.StartCmd(nil)
cmd.PersistentFlags().String(flags.FlagHome, "/foobar", "")
if err := cmd.PersistentFlags().Set(flags.FlagHome, subDir); err != nil {
t.Fatalf("Could not set home flag [%T] %v", err, err)
}

Expand All @@ -420,9 +426,10 @@ func TestEmptyMinGasPrices(t *testing.T) {
// Run InitCmd to create necessary config files.
clientCtx := client.Context{}.WithHomeDir(tempDir).WithCodec(encCfg.Codec)
serverCtx := server.NewDefaultContext()
serverCtx.Config.SetRoot(tempDir)
ctx := context.WithValue(context.Background(), server.ServerContextKey, serverCtx)
ctx = context.WithValue(ctx, client.ClientContextKey, &clientCtx)
cmd := genutilcli.InitCmd(module.NewBasicManager(), tempDir)
cmd := genutilcli.InitCmd(module.NewBasicManager())
cmd.SetArgs([]string{"appnode-test"})
err = cmd.ExecuteContext(ctx)
require.NoError(t, err)
Expand All @@ -434,7 +441,8 @@ func TestEmptyMinGasPrices(t *testing.T) {
config.WriteConfigFile(appCfgTempFilePath, appConf)

// Run StartCmd.
cmd = server.StartCmd(nil, tempDir)
cmd = server.StartCmd(nil)
cmd.PersistentFlags().String(flags.FlagHome, tempDir, "")
cmd.PreRunE = func(cmd *cobra.Command, _ []string) error {
ctx, err := server.InterceptConfigsAndCreateContext(cmd, "", nil, cmtcfg.DefaultConfig())
if err != nil {
Expand Down
Loading