electrum icon indicating copy to clipboard operation
electrum copied to clipboard

Impossible to set custom default wallet path after #8609

Open koniiiik opened this issue 1 year ago • 5 comments

With #8609 merged, it doesn't appear to be possible to change the default wallet path with

electrum setconfig wallet_path /custom/wallet/path

This works just fine prior to that change, with the obvious effect, even though I don't see the setting being defined in the explicit list of configuration options.

Would it be possible to consider adding support for this configuration options? Our use case is running electrum in a docker container with the wallet stored in an obviously noticeable volume that's not in a dot-directory in a homedir, without the need to explicitly specify the path in every single RPC call.

koniiiik avatar Feb 19 '24 13:02 koniiiik

wallet_path conflicts with the -w parameter to indicate a specific wallet.

However I do see the benefit of specifying a wallet folder outside the data dir tree.

accumulator avatar Feb 21 '24 09:02 accumulator

I might be misunderstanding something, but I don't see the problem with this conflict – if -w is provided on the command line, it takes precedence, and if it's not provided, it falls back to the value set in the config, ultimately falling back to {electrum_path}/wallets/default_wallet. Or is there something else that breaks when this option is reintroduced?

koniiiik avatar Feb 21 '24 10:02 koniiiik

Aha. I think I misunderstood you.

If this used to work, I guess it is ok to re-introduce it like this.

This config key is referenced in many places though, not sure if e.g. cmdline option always trumps a config file setting.

@SomberNight ?

accumulator avatar Feb 21 '24 11:02 accumulator

I have done some more testing with the change I proposed in #8903, and when running just the daemon, and interacting with it over the RPC, everything seems fine. I went through the places where wallet path is referenced, and I found some inconsistencies in how it's used in various places, mostly in the GUI. Some places use dirname(config.get_wallet_path()) to determine the location of the wallet directory, others use config.get_datadir_wallet_path().

The dirname pattern is used in these:

  • electrum.gui.qml.qedaemon
  • Daemon.update_password_for_directory (which is only used by qedaemon)
  • electrum.gui.qt.wizard.wallet.QENewWalletWizard.create_storage
  • a couple of scripts ln_features, quick_start
  • electrum.plugins.trustedcoin.qt

get_datadir_wallet_path is used in some others:

  • electrum.gui.qml.qewizard
  • electrum.gui.qt.wizard.wallet.WCWalletName
  • electrum.gui.qt.main_window

Now, without the ability to set a wallet_path config option, both return the same path, but with that ability, one returns the path inside ~/.electrum (on Linux), while the other returns the directory containing the configured wallet_path. This could lead to trouble especially in cases where the same component mixes both interchangeably, and I suspect that the correct choice in those cases would be get_datadir_wallet_path.

Does this make sense? Is there something else that I'm missing that should also be investigated?

koniiiik avatar Feb 26 '24 15:02 koniiiik

related: https://github.com/spesmilo/electrum/pull/7296

$ electrum setconfig wallet_path /custom/wallet/path

This works just fine prior to that change, with the obvious effect, even though I don't see the setting being defined in the explicit list of configuration options.

There are a few "config keys" that not really intended to be set in the config file, they are just there for parsing CLI parameters. wallet_path is one of those, but there is also e.g. cmd or password.

This config key is referenced in many places though, not sure if e.g. cmdline option always trumps a config file setting.

It does, the config is designed with that guarantee. https://github.com/spesmilo/electrum/blob/9214291cdbd3dd1321c884c75821d3e09a031e36/electrum/simple_config.py#L179-L182

I found some inconsistencies in how it's used in various places, mostly in the GUI. Some places use dirname(config.get_wallet_path()) to determine the location of the wallet directory, others use config.get_datadir_wallet_path().

Indeed it would be nice to fix the inconsistencies regardless of this change.

SomberNight avatar May 29 '24 13:05 SomberNight