feat: parse home flag earlier (#20771)

julienrbrt · web-flow · commit 5aaff2109642 · 2024-06-25T18:53:57.000Z
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -102,6 +102,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
 * (proto) [#20098](https://github.com/cosmos/cosmos-sdk/pull/20098) Use cosmos_proto added_in annotation instead of // Since comments.
 * (baseapp) [#20208](https://github.com/cosmos/cosmos-sdk/pull/20208) Skip running validateBasic for rechecking txs.
 * (baseapp) [#20380](https://github.com/cosmos/cosmos-sdk/pull/20380) Enhanced OfferSnapshot documentation.
+* (client) [#20771](https://github.com/cosmos/cosmos-sdk/pull/20771) Remove `ReadDefaultValuesFromDefaultClientConfig` from `client` package. (It was introduced in `v0.50.6` as a quick fix).
 
 ### Bug Fixes
 
diff --git a/client/config/config.go b/client/config/config.go
@@ -53,26 +53,6 @@ func ReadFromClientConfig(ctx client.Context) (client.Context, error) {
 	return CreateClientConfig(ctx, "", nil)
 }
 
-// ReadDefaultValuesFromDefaultClientConfig reads default values from default client.toml file and updates them in client.Context
-// The client.toml is then discarded.
-func ReadDefaultValuesFromDefaultClientConfig(ctx client.Context, customClientTemplate string, customConfig interface{}) (client.Context, error) {
-	prevHomeDir := ctx.HomeDir
-	dir, err := os.MkdirTemp("", "simapp")
-	if err != nil {
-		return ctx, fmt.Errorf("couldn't create temp dir: %w", err)
-	}
-	defer os.RemoveAll(dir)
-
-	ctx.HomeDir = dir
-	ctx, err = CreateClientConfig(ctx, customClientTemplate, customConfig)
-	if err != nil {
-		return ctx, fmt.Errorf("couldn't create client config: %w", err)
-	}
-
-	ctx.HomeDir = prevHomeDir
-	return ctx, nil
-}
-
 // CreateClientConfig reads the client.toml file and returns a new populated client.Context
 // If the client.toml file does not exist, it creates one with default values.
 // It takes a customClientTemplate and customConfig as input that can be used to overwrite the default config and enhance the client.toml file.
diff --git a/client/v2/CHANGELOG.md b/client/v2/CHANGELOG.md
@@ -42,6 +42,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 * [#18626](https://github.com/cosmos/cosmos-sdk/pull/18626) Support for off-chain signing and verification of a file.
 * [#18461](https://github.com/cosmos/cosmos-sdk/pull/18461) Support governance proposals.
+* [#20771](https://github.com/cosmos/cosmos-sdk/pull/20771) Add `GetNodeHomeDirectory` helper.
 
 ### API Breaking Changes
 
diff --git a/client/v2/helpers/home.go b/client/v2/helpers/home.go
@@ -0,0 +1,36 @@
+package helpers
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+)
+
+// GetNodeHomeDirectory gets the home directory of the node (where the config is located).
+// It parses the home flag if set if the `NODE_HOME` environment variable if set (and ignores name).
+// Otherwise, it returns the default home directory given its name.
+func GetNodeHomeDirectory(name string) (string, error) {
+	// get the home directory from the flag
+	args := os.Args
+	for i := 0; i < len(args); i++ {
+		if args[i] == "--home" && i+1 < len(args) {
+			return filepath.Clean(args[i+1]), nil
+		} else if strings.HasPrefix(args[i], "--home=") {
+			return filepath.Clean(args[i][7:]), nil
+		}
+	}
+
+	// get the home directory from the environment variable
+	homeDir := os.Getenv("NODE_HOME")
+	if homeDir != "" {
+		return filepath.Clean(homeDir), nil
+	}
+
+	// return the default home directory
+	userHomeDir, err := os.UserHomeDir()
+	if err != nil {
+		return "", err
+	}
+
+	return filepath.Join(userHomeDir, name), nil
+}
diff --git a/docs/learn/advanced/07-cli.md b/docs/learn/advanced/07-cli.md
@@ -172,7 +172,7 @@ Flags are added to commands directly (generally in the [module's CLI file](../..
 
 ## Environment variables
 
-Each flag is bound to its respective named environment variable. Then name of the environment variable consist of two parts - capital case `basename` followed by flag name of the flag. `-` must be substituted with `_`. For example flag `--home` for application with basename `GAIA` is bound to `GAIA_HOME`. It allows reducing the amount of flags typed for routine operations. For example instead of:
+Each flag is bound to its respective named environment variable. Then name of the environment variable consist of two parts - capital case `basename` followed by flag name of the flag. `-` must be substituted with `_`. For example flag `--node` for application with basename `GAIA` is bound to `GAIA_NODE`. It allows reducing the amount of flags typed for routine operations. For example instead of:
 
 ```shell
 gaia --home=./ --node=<node address> --chain-id="testchain-1" --keyring-backend=test tx ... --from=<key name>
@@ -182,7 +182,7 @@ this will be more convenient:
 
 ```shell
 # define env variables in .env, .envrc etc
-GAIA_HOME=<path to home>
+NODE_HOME=<path to home>
 GAIA_NODE=<node address>
 GAIA_CHAIN_ID="testchain-1"
 GAIA_KEYRING_BACKEND="test"
diff --git a/scripts/init-simapp.sh b/scripts/init-simapp.sh
@@ -4,7 +4,7 @@ SIMD_BIN=${SIMD_BIN:=$(which simd 2>/dev/null)}
 
 if [ -z "$SIMD_BIN" ]; then echo "SIMD_BIN is not set. Make sure to run make install before"; exit 1; fi
 echo "using $SIMD_BIN"
-if [ -d "$($SIMD_BIN config home)" ]; then rm -r $($SIMD_BIN config home); fi
+if [ -d "$($SIMD_BIN config home)" ]; then rm -rv $($SIMD_BIN config home); fi
 $SIMD_BIN config set client chain-id demo
 $SIMD_BIN config set client keyring-backend test
 $SIMD_BIN config set client keyring-default-keyname alice
diff --git a/scripts/simapp-v2-init.sh b/scripts/simapp-v2-init.sh
@@ -11,7 +11,7 @@ CONFIG="${CONFIG:-$HOME/.simappv2/config}"
 
 COSMOS_BUILD_OPTIONS=v2 make build     
 
-if [ -d "$($SIMD config home)" ]; then rm -r $($SIMD config home); fi
+if [ -d "$($SIMD config home)" ]; then rm -rv $($SIMD config home); fi
 
 $SIMD init simapp-v2-node --chain-id simapp-v2-chain
 
@@ -26,6 +26,7 @@ jq '.app_state.mint.minter.inflation = "0.300000000000000000"' genesis.json > te
 # change the initial height to 2 to work around store/v2 and iavl limitations with a genesis block
 jq '.initial_height = 2' genesis.json > temp.json && mv temp.json genesis.json
 
+$SIMD config set client chain-id simapp-v2-chain
 $SIMD keys add test_validator --indiscreet
 VALIDATOR_ADDRESS=$($SIMD keys show test_validator -a --keyring-backend test)
 
diff --git a/server/v2/cometbft/flags/flags.go b/server/v2/cometbft/flags/flags.go
@@ -9,9 +9,6 @@ const (
 )
 
 const (
-	FlagHome             = "home"
-	FlagKeyringDir       = "keyring-dir"
-	FlagUseLedger        = "ledger"
 	FlagChainID          = "chain-id"
 	FlagNode             = "node"
 	FlagGRPC             = "grpc-addr"
diff --git a/server/v2/commands.go b/server/v2/commands.go
@@ -109,7 +109,8 @@ func AddCommands(rootCmd *cobra.Command, newApp AppCreator[transaction.Tx], logg
 
 // configHandle writes the default config to the home directory if it does not exist and sets the server context
 func configHandle(s *Server, home string, cmd *cobra.Command) error {
-	if _, err := os.Stat(filepath.Join(home, "config")); os.IsNotExist(err) {
+	// we need to check app.toml as the config folder can already exist for the client.toml
+	if _, err := os.Stat(filepath.Join(home, "config", "app.toml")); os.IsNotExist(err) {
 		if err = s.WriteConfig(filepath.Join(home, "config")); err != nil {
 			return err
 		}
diff --git a/simapp/CHANGELOG.md b/simapp/CHANGELOG.md
@@ -35,6 +35,7 @@ Always refer to the [UPGRADING.md](https://github.com/cosmos/cosmos-sdk/blob/mai
 * [#20409](https://github.com/cosmos/cosmos-sdk/pull/20409) Add `tx` as `SkipStoreKeys` in `app_config.go`.
 * [#20485](https://github.com/cosmos/cosmos-sdk/pull/20485) The signature of `x/upgrade/types.UpgradeHandler` has changed to accept `appmodule.VersionMap` from `module.VersionMap`.  These types are interchangeable, but usages of `UpradeKeeper.SetUpgradeHandler` may need to adjust their usages to match the new signature.
 * [#20740](https://github.com/cosmos/cosmos-sdk/pull/20740) Update `genutilcli.Commands` to use the genutil modules from the module manager.
+* [#20771](https://github.com/cosmos/cosmos-sdk/pull/20771) Use client/v2 `GetNodeHomeDirectory` helper in `app.go` and use the `DefaultNodeHome` constant everywhere in the app.
 
 <!-- TODO: move changelog.md elements to here -->
 
diff --git a/simapp/app.go b/simapp/app.go
@@ -18,6 +18,7 @@ import (
 	autocliv1 "cosmossdk.io/api/cosmos/autocli/v1"
 	reflectionv1 "cosmossdk.io/api/cosmos/reflection/v1"
 	"cosmossdk.io/client/v2/autocli"
+	clienthelpers "cosmossdk.io/client/v2/helpers"
 	"cosmossdk.io/core/log"
 	storetypes "cosmossdk.io/store/types"
 	"cosmossdk.io/x/accounts"
@@ -183,12 +184,11 @@ type SimApp struct {
 }
 
 func init() {
-	userHomeDir, err := os.UserHomeDir()
+	var err error
+	DefaultNodeHome, err = clienthelpers.GetNodeHomeDirectory(".simapp")
 	if err != nil {
 		panic(err)
 	}
-
-	DefaultNodeHome = filepath.Join(userHomeDir, ".simapp")
 }
 
 // NewSimApp returns a reference to an initialized SimApp.
@@ -536,7 +536,7 @@ func NewSimApp(
 	app.sm = module.NewSimulationManagerFromAppModules(app.ModuleManager.Modules, overrideModules)
 
 	// create, start, and load the unordered tx manager
-	utxDataDir := filepath.Join(cast.ToString(appOpts.Get(flags.FlagHome)), "data")
+	utxDataDir := filepath.Join(homePath, "data")
 	app.UnorderedTxManager = unorderedtx.NewManager(utxDataDir)
 	app.UnorderedTxManager.Start()
 
diff --git a/simapp/app_di.go b/simapp/app_di.go
@@ -6,12 +6,12 @@ import (
 	_ "embed"
 	"fmt"
 	"io"
-	"os"
 	"path/filepath"
 
 	dbm "github.com/cosmos/cosmos-db"
 	"github.com/spf13/cast"
 
+	clienthelpers "cosmossdk.io/client/v2/helpers"
 	"cosmossdk.io/core/legacy"
 	"cosmossdk.io/depinject"
 	"cosmossdk.io/log"
@@ -100,12 +100,11 @@ type SimApp struct {
 }
 
 func init() {
-	userHomeDir, err := os.UserHomeDir()
+	var err error
+	DefaultNodeHome, err = clienthelpers.GetNodeHomeDirectory(".simapp")
 	if err != nil {
 		panic(err)
 	}
-
-	DefaultNodeHome = filepath.Join(userHomeDir, ".simapp")
 }
 
 // AppConfig returns the default app config.
diff --git a/simapp/simd/cmd/commands.go b/simapp/simd/cmd/commands.go
@@ -3,7 +3,6 @@ package cmd
 import (
 	"errors"
 	"io"
-	"os"
 
 	dbm "github.com/cosmos/cosmos-db"
 	"github.com/spf13/cobra"
@@ -18,7 +17,6 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/client/debug"
-	"github.com/cosmos/cosmos-sdk/client/flags"
 	"github.com/cosmos/cosmos-sdk/client/keys"
 	"github.com/cosmos/cosmos-sdk/client/pruning"
 	"github.com/cosmos/cosmos-sdk/client/rpc"
@@ -143,13 +141,6 @@ func appExport(
 	appOpts servertypes.AppOptions,
 	modulesToExport []string,
 ) (servertypes.ExportedApp, error) {
-	// this check is necessary as we use the flag in x/upgrade.
-	// we can exit more gracefully by checking the flag here.
-	homePath, ok := appOpts.Get(flags.FlagHome).(string)
-	if !ok || homePath == "" {
-		return servertypes.ExportedApp{}, errors.New("application home not set")
-	}
-
 	viperAppOpts, ok := appOpts.(*viper.Viper)
 	if !ok {
 		return servertypes.ExportedApp{}, errors.New("appOpts is not viper.Viper")
@@ -172,13 +163,3 @@ func appExport(
 
 	return simApp.ExportAppStateAndValidators(forZeroHeight, jailAllowedAddrs, modulesToExport)
 }
-
-var tempDir = func() string {
-	dir, err := os.MkdirTemp("", "simapp")
-	if err != nil {
-		dir = simapp.DefaultNodeHome
-	}
-	defer os.RemoveAll(dir)
-
-	return dir
-}
diff --git a/simapp/simd/cmd/root.go b/simapp/simd/cmd/root.go
@@ -30,7 +30,7 @@ import (
 func NewRootCmd() *cobra.Command {
 	// we "pre"-instantiate the application for getting the injected/configured encoding configuration
 	// note, this is not necessary when using app wiring, as depinject can be directly used (see root_v2.go)
-	tempApp := simapp.NewSimApp(log.NewNopLogger(), dbm.NewMemDB(), nil, true, simtestutil.NewAppOptionsWithFlagHome(tempDir()))
+	tempApp := simapp.NewSimApp(log.NewNopLogger(), dbm.NewMemDB(), nil, true, simtestutil.NewAppOptionsWithFlagHome(simapp.DefaultNodeHome))
 	encodingConfig := params.EncodingConfig{
 		InterfaceRegistry: tempApp.InterfaceRegistry(),
 		Codec:             tempApp.AppCodec(),
@@ -114,7 +114,7 @@ func NewRootCmd() *cobra.Command {
 	// autocli opts
 	customClientTemplate, customClientConfig := initClientConfig()
 	var err error
-	initClientCtx, err = config.ReadDefaultValuesFromDefaultClientConfig(initClientCtx, customClientTemplate, customClientConfig)
+	initClientCtx, err = config.CreateClientConfig(initClientCtx, customClientTemplate, customClientConfig)
 	if err != nil {
 		panic(err)
 	}
diff --git a/simapp/simd/cmd/root_di.go b/simapp/simd/cmd/root_di.go
@@ -24,7 +24,6 @@ import (
 	"github.com/cosmos/cosmos-sdk/codec"
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	"github.com/cosmos/cosmos-sdk/server"
-	simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
 	"github.com/cosmos/cosmos-sdk/types/module"
 )
 
@@ -38,10 +37,7 @@ func NewRootCmd() *cobra.Command {
 
 	if err := depinject.Inject(
 		depinject.Configs(simapp.AppConfig(),
-			depinject.Supply(
-				log.NewNopLogger(),
-				simtestutil.NewAppOptionsWithFlagHome(tempDir()),
-			),
+			depinject.Supply(log.NewNopLogger()),
 			depinject.Provide(
 				ProvideClientContext,
 			),
@@ -128,7 +124,7 @@ func ProvideClientContext(
 
 	// Read the config to overwrite the default values with the values from the config file
 	customClientTemplate, customClientConfig := initClientConfig()
-	clientCtx, err = config.ReadDefaultValuesFromDefaultClientConfig(clientCtx, customClientTemplate, customClientConfig)
+	clientCtx, err = config.CreateClientConfig(clientCtx, customClientTemplate, customClientConfig)
 	if err != nil {
 		panic(err)
 	}
diff --git a/simapp/v2/app_di.go b/simapp/v2/app_di.go
@@ -2,16 +2,17 @@ package simapp
 
 import (
 	_ "embed"
-	"os"
 	"path/filepath"
 
 	"github.com/spf13/viper"
 
+	clienthelpers "cosmossdk.io/client/v2/helpers"
 	coreapp "cosmossdk.io/core/app"
 	"cosmossdk.io/core/legacy"
 	"cosmossdk.io/core/log"
 	"cosmossdk.io/depinject"
 	"cosmossdk.io/runtime/v2"
+	serverv2 "cosmossdk.io/server/v2"
 	"cosmossdk.io/store/v2"
 	"cosmossdk.io/store/v2/commitment/iavl"
 	"cosmossdk.io/store/v2/db"
@@ -36,10 +37,8 @@ import (
 	upgradekeeper "cosmossdk.io/x/upgrade/keeper"
 
 	"github.com/cosmos/cosmos-sdk/client"
-	"github.com/cosmos/cosmos-sdk/client/flags"
 	"github.com/cosmos/cosmos-sdk/codec"
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
-	servertypes "github.com/cosmos/cosmos-sdk/server/types"
 	"github.com/cosmos/cosmos-sdk/std"
 )
 
@@ -77,12 +76,11 @@ type SimApp struct {
 }
 
 func init() {
-	userHomeDir, err := os.UserHomeDir()
+	var err error
+	DefaultNodeHome, err = clienthelpers.GetNodeHomeDirectory(".simappv2")
 	if err != nil {
 		panic(err)
 	}
-
-	DefaultNodeHome = filepath.Join(userHomeDir, ".simappv2")
 }
 
 // AppConfig returns the default app config.
@@ -97,8 +95,8 @@ func NewSimApp(
 	logger log.Logger,
 	viper *viper.Viper,
 ) *SimApp {
-	homeDir := viper.Get(flags.FlagHome).(string) // TODO
-	scRawDb, err := db.NewGoLevelDB("application", filepath.Join(homeDir, "data"), nil)
+	viper.Set(serverv2.FlagHome, DefaultNodeHome) // TODO possibly set earlier when viper is created
+	scRawDb, err := db.NewGoLevelDB("application", filepath.Join(DefaultNodeHome, "data"), nil)
 	if err != nil {
 		panic(err)
 	}
@@ -113,7 +111,7 @@ func NewSimApp(
 				logger,
 				&root.FactoryOptions{
 					Logger:  logger,
-					RootDir: homeDir,
+					RootDir: DefaultNodeHome,
 					SSType:  0,
 					SCType:  0,
 					SCPruneOptions: &store.PruneOptions{
@@ -126,7 +124,7 @@ func NewSimApp(
 					},
 					SCRawDB: scRawDb,
 				},
-				servertypes.AppOptions(viper),
+				viper,
 
 				// ADVANCED CONFIGURATION
 
diff --git a/simapp/v2/simdv2/cmd/commands.go b/simapp/v2/simdv2/cmd/commands.go
diff --git a/simapp/v2/simdv2/cmd/root_di.go b/simapp/v2/simdv2/cmd/root_di.go
diff --git a/x/upgrade/CHANGELOG.md b/x/upgrade/CHANGELOG.md
diff --git a/x/upgrade/depinject.go b/x/upgrade/depinject.go
diff --git a/x/upgrade/go.mod b/x/upgrade/go.mod
diff --git a/x/upgrade/keeper/keeper.go b/x/upgrade/keeper/keeper.go

Original file line number	Diff line number	Diff line change
`@@ -9,9 +9,6 @@ const (`
`9`	`9`	`)`
`10`	`10`
`11`	`11`	`const (`
`12`		`- FlagHome = "home"`
`13`		`- FlagKeyringDir = "keyring-dir"`
`14`		`- FlagUseLedger = "ledger"`
`15`	`12`	`FlagChainID = "chain-id"`
`16`	`13`	`FlagNode = "node"`
`17`	`14`	`FlagGRPC = "grpc-addr"`
Original file line number	Diff line number	Diff line change
`@@ -109,7 +109,8 @@ func AddCommands(rootCmd *cobra.Command, newApp AppCreator[transaction.Tx], logg`
`109`	`109`
`110`	`110`	`// configHandle writes the default config to the home directory if it does not exist and sets the server context`
`111`	`111`	`func configHandle(s Server, home string, cmd cobra.Command) error {`
`112`		`- if _, err := os.Stat(filepath.Join(home, "config")); os.IsNotExist(err) {`
	`112`	`+ // we need to check app.toml as the config folder can already exist for the client.toml`
	`113`	`+ if _, err := os.Stat(filepath.Join(home, "config", "app.toml")); os.IsNotExist(err) {`
`113`	`114`	`if err = s.WriteConfig(filepath.Join(home, "config")); err != nil {`
`114`	`115`	`return err`
`115`	`116`	`}`