[Bug]: --home flag is not respected

[aeryz]

Is there an existing issue for this?

• I have searched the existing issues

What happened?

When running simd, --home flag is not fully respected. The application still creates the config directory under the default home location which is ~/.simapp.

My initial findings show that this line which reads the application flags is called after this line which is used for injecting the sdk's cli options.

Cosmos SDK Version

0.50.x,main

How to reproduce?

• Remove the home directory: rm -rf ~/.simapp
• Start the application by setting the home directory to a non-default location: ./simd start --home ./other-location
• Check if the default home directory is created: ls ~/.simapp

[julienrbrt]

Correct, AutoCLI needs some prefilled default data.
This will be fixed at the same time as this: #18122

[aeryz]

Hey, I wanted to kindly ask what is the ETA on fixing this? We are waiting for this to do an upgrade. We could also help with a PR in case you have other priorities. cc @PoisonPhang

[julienrbrt]

I won't have time before at least end next week for finishing it.
If you want to give it a go, feel free to do it. Happy to review and happy to answer any question.

[PoisonPhang]

Hi @julienrbrt
While looking into this, I had some questions. In v0.50.2, I notice that the x/gov module has CLI implementations for both AutoCLI and Cobra while x/slashing only has an AutoCLI implementation.

Does the x/gov module still use Cobra, or has it moved on to AutoCLI?

[julienrbrt]

Hi, first of all I would advise you to work being based off main, as it will complicate your life when opening a PR otherwise.

To answer your question, in v0.50, all modules are using AutoCLI for queries but mostly not for txs. In main all modules have migrated to AutoCLI (queries + txs) unless the UX of a manually defined CLI was better.

[PoisonPhang]

I think I found the issue, or at least one of them. I'll think about a fix, but I though I'd provide an update in the meantime.

---

When initializing the root command with simd using app/root v2, the function ProvideClientContext is used to bootstrap the clientCtx used throughout the lifespan of the command. To initialize clientCtx, it uses the following string of builder calls:

	clientCtx := client.Context{}.
		WithCodec(appCodec).
		WithInterfaceRegistry(interfaceRegistry).
		WithLegacyAmino(legacyAmino).
		WithInput(os.Stdin).
		WithAccountRetriever(types.AccountRetriever{}).
		WithHomeDir(simapp.DefaultNodeHome).
		WithViper("") // In simapp, we don't use any prefix for env variables.

This sets the default home directory then calls ReadFromClientConfig, using the default home directory rather than the home provided by the --home flag. This then results in a few side effects including:

• Setting the KeyringDir to the default home (which has no logic allowing it be overwritten with the value from the --home flag)
• The app looking for a config file in the default home. Which does not exist resulting in one being created with default values.
• The clientCtx used in ProvideKeyring does not respect flags.
• The viper instance used in the context always checks the default home first as that was the first path added.

[PoisonPhang]

IMO, calling WithKeyringDir(ctx.HomeDir) in ReadFromClientConfig feels like a bug.

[julienrbrt]

Thanks for the sum-up, this is indeed most of the issue.
When would you set the keyring directory then? At flag reading?
A second issue is that in autocli we have no way to re-set the home directory of the keyring because we pass the keyring directly to autocli at command instantiation.

The solution I was trying to implement is to have the commands created by autocli share the context of the root command, removing the instantiation need of the clientCtx. This was still leaving the issue of the keyring, that we require in autocli before building the commands.

[PoisonPhang]

I'm making progress, but still hitting some issues.

In client/config/config.go:ReadFromClientConfig, I've opted to use .WithKeyringDir(ctx.KeyringDir) rather than WithKeyringDir(ctx.HomeDir). This is more correct as if ctx.KeyringDir has a value, it's used and if it does not, it's correctly reevaluated to ctx.KeyringDir or the home passed by the --home flag when ReadPersistentCommandFlags is called.

In client/config/toml.go, I started creating a new viper instance instead of using an existing one. Unless I'm mistaken, we're not extracting any value from storing a viper instance. This results in the correct client.toml file being read.

However, when executing txs, I'm still getting the chain ID required but not specified error. So I'm determining the source of that issue. I'll provide an update once I've figured out where the incorrect client.toml content is persisting.

[PoisonPhang]

Okay, the function client/cmd.go:SetCmdClientContext does not update the command context.

cmd.Context().Value(ClientContextKey)

Returns a value, not a ref

Technically, this returns any but I believe this is initially returning nil

[PoisonPhang]

The solution I was trying to implement is to have the commands created by autocli share the context of the root command, removing the instantiation need of the clientCtx.

@julienrbrt do you have this work on any public branch? In addition to the fixes I've made - I think this issue could be resolved. However, I'm struggling with unifying the AutoCLI context and the root command context.

[PoisonPhang]

Tentative solution #18994
I'm not entirely happy with this, but from what I've grasped - AutoCLI needs some hefty redesign surrounding how it boot straps itself and supplies context.

[PoisonPhang]

@julienrbrt while the changes I proposed in #18994 work fine on main, I notice that the functions in client/v2/autocli/msg.go don't seem to be getting called in v0.50.2.
Are there changes in main that enable this code?

[julienrbrt]

Hey, thanks for the quick PR, I'll review it tomorrow. Are you saying it does not work on v0.50?
Only the function EnhanceRootCommand in root.go suffices to call all those functions in client/v2

do you have this work on any public branch?
...
AutoCLI needs some hefty redesign surrounding how it boot straps itself and supplies context.

Not yet, and right, and we are a bit holding off on it because the short-term plan is to remove completely client.Context of it.
My changes are mainly focusing on getting the keyring to work properly with the flags handler and be able to update it, without breaking the API.

[PoisonPhang]

Are you saying it does not work on v0.50?
Only the function EnhanceRootCommand in root.go suffices to call all those functions in client/v2

Turns out this was an issue with how simapp was configured on our fork. Solution is working on v0.50.2 now.

Not yet, and right, and we are a bit holding off on it because the short-term plan is to remove completely client.Context of it.
My changes are mainly focusing on getting the keyring to work properly with the flags handler and be able to update it, without breaking the API.

I see, thanks. My solution may prove helpful in the meantime while a much larger refactor is conducted. Though, a few of my changes are genuine fixes. See the description of #18994 for more details.

[roy-dydx]

Hi folks, we noticed this issue isn't working on main still for some cases. For example:

./build/simd version --home ~/.simapp2

Now both ~/.simapp2/ and the default ~/.simapp/ exist. Notably only ~/.simapp2/ has the data directory; both have the config directory.