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: parse home flag earlier #20771

Merged
merged 14 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -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).
Copy link
Contributor

Choose a reason for hiding this comment

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

Update the changelog entry for clarity and accuracy.

The entry mentions the removal of ReadDefaultValuesFromDefaultClientConfig from the client package. Ensure that the entry clearly states that this removal is part of the broader changes for parsing the --home flag earlier, which is the main focus of the PR.

- * (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).
+ * (client) [#20771](https://github.com/cosmos/cosmos-sdk/pull/20771) Removed `ReadDefaultValuesFromDefaultClientConfig` from the `client` package as part of enhancements to parse the `--home` flag earlier. Introduced in `v0.50.6` as a temporary measure.
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
* (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).
* (client) [#20771](https://github.com/cosmos/cosmos-sdk/pull/20771) Removed `ReadDefaultValuesFromDefaultClientConfig` from the `client` package as part of enhancements to parse the `--home` flag earlier. Introduced in `v0.50.6` as a temporary measure.


### Bug Fixes

Expand Down
20 changes: 0 additions & 20 deletions client/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions client/v2/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
36 changes: 36 additions & 0 deletions client/v2/helpers/home.go
Original file line number Diff line number Diff line change
@@ -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
}
4 changes: 2 additions & 2 deletions docs/learn/advanced/07-cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion scripts/init-simapp.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

Quote the command substitution to prevent word splitting.

The script uses command substitution without quoting, which can lead to word splitting and unexpected behavior, especially when directory names contain spaces.

- if [ -d "$($SIMD_BIN config home)" ]; then rm -rv $($SIMD_BIN config home); fi
+ if [ -d "$("$SIMD_BIN" config home)" ]; then rm -rv "$("$SIMD_BIN" config home)"; fi
Tools
Shellcheck

[warning] 7-7: Quote this to prevent word splitting. (SC2046)

$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
Expand Down
3 changes: 2 additions & 1 deletion scripts/simapp-v2-init.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ CONFIG="${CONFIG:-$HOME/.simappv2/config}"
cd "$SIMAPP_DIR"
go build -o "$ROOT/build/simdv2" simdv2/main.go

if [ -d "$($SIMD config home)" ]; then rm -r $($SIMD config home); fi
if [ -d "$($SIMD config home)" ]; then rm -rv $($SIMD config home); fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Quote the command substitution to prevent word splitting.

As with the previous script, this line suffers from the same issue where command substitution could lead to word splitting. It's a good practice to quote these to ensure the command behaves as expected.

- if [ -d "$($SIMD config home)" ]; then rm -rv $($SIMD config home); fi
+ if [ -d "$("$SIMD" config home)" ]; then rm -rv "$("$SIMD" config home)"; fi
Tools
Shellcheck

[warning] 16-16: Quote this to prevent word splitting. (SC2046)


$SIMD init simapp-v2-node --chain-id simapp-v2-chain

Expand All @@ -28,6 +28,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)

Expand Down
3 changes: 0 additions & 3 deletions server/v2/cometbft/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ const (
)

const (
FlagHome = "home"
FlagKeyringDir = "keyring-dir"
FlagUseLedger = "ledger"
FlagChainID = "chain-id"
FlagNode = "node"
FlagGRPC = "grpc-addr"
Expand Down
3 changes: 2 additions & 1 deletion server/v2/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions simapp/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 -->

Expand Down
8 changes: 4 additions & 4 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -183,12 +184,11 @@ type SimApp struct {
}

func init() {
userHomeDir, err := os.UserHomeDir()
var err error
DefaultNodeHome, err = clienthelpers.GetNodeHomeDirectory(".simapp")
Comment on lines +187 to +188
Copy link
Contributor

Choose a reason for hiding this comment

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

Review the initialization of DefaultNodeHome.

The modification to initialize DefaultNodeHome using clienthelpers.GetNodeHomeDirectory(".simapp") is a critical part of the PR's goal to parse the --home flag earlier. This ensures that the application's default home directory is set early in the process, which is essential for handling custom node locations correctly.

However, the use of panic(err) for error handling is a bit aggressive. Consider using a more graceful error handling strategy that could allow for recovery or at least a cleaner shutdown process.

- panic(err)
+ log.Fatalf("Failed to set DefaultNodeHome: %v", err)

Committable suggestion was skipped due to low confidence.

if err != nil {
panic(err)
}

DefaultNodeHome = filepath.Join(userHomeDir, ".simapp")
}

// NewSimApp returns a reference to an initialized SimApp.
Expand Down Expand Up @@ -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")
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Ensure the directory homePath/data is created if it doesn't exist.

The directory creation or existence check for utxDataDir is not explicitly handled in the current codebase. To avoid potential runtime errors, make sure to add logic to create the directory if it doesn't exist.

  • File: simapp/app.go
  • Lines to review: Around the initialization of utxDataDir and unorderedtx.NewManager(utxDataDir)

Suggested Code:

utxDataDir := filepath.Join(homePath, "data")
if err := os.MkdirAll(utxDataDir, 0755); err != nil {
    log.Fatalf("failed to create directory %s: %v", utxDataDir, err)
}
app.UnorderedTxManager = unorderedtx.NewManager(utxDataDir)

Please review and incorporate this change to ensure the directory is available.

Analysis chain

Review the setup of the Unordered Transaction Manager.

The setup of the UnorderedTxManager in the NewSimApp function is correctly placed to ensure that the transaction manager is initialized with the appropriate directory derived from homePath. This aligns with the PR's objectives to manage the --home flag effectively.

However, ensure that the directory homePath/data is always available or handled gracefully if it's not.


Sub-tasks

Sub-task 1: Verify if the directory creation logic exists in the NewSimApp function.

Sub-task 2: Check if the directory is created or handled gracefully within the codebase.


Sub-tasks

Sub-task 1: Verify if utxDataDir is used or handled within the NewSimApp function.

Sub-task 2: Verify any directory creation or handling logic related to homePath/data in the codebase.

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the directory for UnorderedTxManager exists or is created.

# Test: Check if the directory exists. If not, attempt to create it.
if [ ! -d "$(dirname $(dirname $(readlink -f $0)))/data" ]; then
    mkdir -p "$(dirname $(dirname $(readlink -f $0)))/data"
fi
echo "Directory exists or created successfully."

Length of output: 772



Script:

#!/bin/bash
# Search for the creation of the `homePath/data` directory within the `NewSimApp` function in simapp/app.go.

ast-grep --lang go --pattern $'func NewSimApp($_, $_, $_, $_, $_, $_, $_, $_, $_, $_) $_ {
  $$$
  utxDataDir := filepath.Join(homePath, "data")
  $$$
}' simapp/app.go

Length of output: 168


Script:

#!/bin/bash
# Search for directory creation or existence check related to `utxDataDir` in the codebase.

rg --multiline --multiline-dotall 'if err := os.MkdirAll\(utxDataDir, 0[0-7]{3}\); err != nil|if _, err := os.Stat\(utxDataDir\); os.IsNotExist\(err\)' --glob '!*test.go'

Length of output: 172



Script:

#!/bin/bash
# Search for the usage of `utxDataDir` within the `NewSimApp` function in simapp/app.go.

rg 'utxDataDir' simapp/app.go

Length of output: 137


Script:

#!/bin/bash
# Search for directory creation or existence check related to `homePath/data` in the codebase.

rg --multiline --multiline-dotall 'if err := os.MkdirAll\(filepath.Join\(homePath, "data"\), 0[0-7]{3}\); err != nil|if _, err := os.Stat\(filepath.Join\(homePath, "data"\)\); os.IsNotExist\(err\)' --glob '!*test.go'

Length of output: 218

app.UnorderedTxManager = unorderedtx.NewManager(utxDataDir)
app.UnorderedTxManager.Start()

Expand Down
7 changes: 3 additions & 4 deletions simapp/app_di.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -100,12 +100,11 @@ type SimApp struct {
}

func init() {
userHomeDir, err := os.UserHomeDir()
var err error
DefaultNodeHome, err = clienthelpers.GetNodeHomeDirectory(".simapp")
Comment on lines +103 to +104
Copy link
Contributor

Choose a reason for hiding this comment

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

Use consistent error handling across initializers.

Similar to the previous file, this init function uses panic for error handling. Recommend using a more consistent and graceful error handling approach.

func init() {
	var err error
	DefaultNodeHome, err = clienthelpers.GetNodeHomeDirectory(".simapp")
-	if err != nil {
-		panic(err)
+	if err != nil {
+		log.Fatalf("Failed to get node home directory: %v", 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
var err error
DefaultNodeHome, err = clienthelpers.GetNodeHomeDirectory(".simapp")
func init() {
var err error
DefaultNodeHome, err = clienthelpers.GetNodeHomeDirectory(".simapp")
if err != nil {
log.Fatalf("Failed to get node home directory: %v", err)
}
}

if err != nil {
panic(err)
}

DefaultNodeHome = filepath.Join(userHomeDir, ".simapp")
}

// AppConfig returns the default app config.
Expand Down
19 changes: 0 additions & 19 deletions simapp/simd/cmd/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package cmd
import (
"errors"
"io"
"os"

dbm "github.com/cosmos/cosmos-db"
"github.com/spf13/cobra"
Expand All @@ -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"
Expand Down Expand Up @@ -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")
Expand All @@ -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
}
4 changes: 2 additions & 2 deletions simapp/simd/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

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

Refactor error handling in configuration setup.

Consistent with previous files, consider returning errors instead of using panic in the CreateClientConfig function call.

- panic(err)
+ return nil, err

Committable suggestion was skipped due to low confidence.

if err != nil {
panic(err)
}
Expand Down
8 changes: 2 additions & 6 deletions simapp/simd/cmd/root_di.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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,
),
Expand Down Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

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

Refactor error handling in ProvideClientContext function.

Similar to the previous file, consider improving the error handling by returning errors instead of using panic.

- panic(err)
+ return client.Context{}, 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
clientCtx, err = config.CreateClientConfig(clientCtx, customClientTemplate, customClientConfig)
clientCtx, err = config.CreateClientConfig(clientCtx, customClientTemplate, customClientConfig)
if err != nil {
return client.Context{}, err
}

if err != nil {
panic(err)
}
Expand Down
18 changes: 8 additions & 10 deletions simapp/v2/app_di.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
)

Expand Down Expand Up @@ -77,12 +76,11 @@ type SimApp struct {
}

func init() {
userHomeDir, err := os.UserHomeDir()
var err error
DefaultNodeHome, err = clienthelpers.GetNodeHomeDirectory(".simappv2")
Comment on lines +79 to +80
Copy link
Contributor

Choose a reason for hiding this comment

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

Proper error handling in initialization.

The init function sets the DefaultNodeHome using clienthelpers.GetNodeHomeDirectory. It panics if there's an error. Consider handling this error more gracefully to avoid crashing the application.

func init() {
	var err error
	DefaultNodeHome, err = clienthelpers.GetNodeHomeDirectory(".simappv2")
-	if err != nil {
-		panic(err)
+	if err != nil {
+		log.Fatalf("Failed to get node home directory: %v", 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
var err error
DefaultNodeHome, err = clienthelpers.GetNodeHomeDirectory(".simappv2")
func init() {
var err error
DefaultNodeHome, err = clienthelpers.GetNodeHomeDirectory(".simappv2")
if err != nil {
log.Fatalf("Failed to get node home directory: %v", err)
}
}

if err != nil {
panic(err)
}

DefaultNodeHome = filepath.Join(userHomeDir, ".simappv2")
}

// AppConfig returns the default app config.
Expand All @@ -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)
}
Expand All @@ -113,7 +111,7 @@ func NewSimApp(
logger,
&root.FactoryOptions{
Logger: logger,
RootDir: homeDir,
RootDir: DefaultNodeHome,
SSType: 0,
SCType: 0,
SCPruneOptions: &store.PruneOptions{
Expand All @@ -126,7 +124,7 @@ func NewSimApp(
},
SCRawDB: scRawDb,
},
servertypes.AppOptions(viper),
viper,

// ADVANCED CONFIGURATION

Expand Down
Loading
Loading