Move options from wallet.dat to QT settings files #890

pull gavinandresen wants to merge 2 commits into bitcoin:master from gavinandresen:nooptionsinwallet changing 12 files +172 −143

gavinandresen commented at 8:38 PM on February 23, 2012: contributor

The main motivation for this was a lot of frustration because command-line, bitcoin.conf, and wallet.dat settings were interacting in ill-defined ways.

Now GUI settings are stored in QT settings files; see http://developer.qt.nokia.com/doc/qt-4.8/qsettings.html#platform-specific-notes

... for details on where. Testnet and main network have their own separate files (thanks @laanwj).

The first time you run with this patch settings from your wallet.dat will be read and moved over to the new settings file (and the settings erased from wallet.dat). From then on, settings in the wallet.dat are ignored-- so if you're using --datadir and multiple wallets, don't expect the setting to be read/erased from the other wallets.

The 'proxy' setting is behaving badly (e.g. 127.0.0.1 is turned into an IP6-looking address), but I'm 90% sure that is a pre-existing bug based on other reports of weird strings in the proxy setting with the 0.6 release. I'll file an issue about that.
bitcoind changes to stop storing settings in wallet.dat. a27e813af7
Reworked QT settings 2281e9fef2
sipa commented at 7:16 PM on February 25, 2012: member

Do we want this in 0.6.0 still?
laanwj commented at 7:28 PM on February 25, 2012: member

Hmm, as this has pretty significant changes, I think we should leave this for 0.7.0, and try it out a bit ourselves first.
gavinandresen commented at 9:34 PM on February 25, 2012: contributor

If (a rebased version of) this isn't pulled for 0.6 then another fix for #815 (UpNP settings don't stick) is needed-- trying to fix that bug is what led me to pull my hair out and decide the whole settings system needed to be overhauled.
laanwj commented at 9:46 PM on February 25, 2012: member

ok, in that case it's fine with me to add it for 0.6.0 already
in src/qt/optionsmodel.cpp:None in 2281e9fef2
```
  98 |              return QVariant(fMinimizeOnClose);
  99 |          case ConnectSOCKS4:
 100 | -            return QVariant(fUseProxy);
 101 | +            return settings.value("fUseProxy", false);
 102 |          case ProxyIP:
 103 |              return QVariant(QString::fromStdString(addrProxy.ToStringIP()));
```
sipa commented at 10:55 PM on February 25, 2012:

This should be ToStringIPPort(), I think.

laanwj commented at 7:04 AM on February 26, 2012:

Wouldn't that append the port? It only wants the IP here, port is separate.

sipa commented at 1:33 PM on February 26, 2012:

Hmm, yes it would. Still, the proxy port doesn't seem to get saved to the file?
in src/qt/optionsmodel.cpp:None in 2281e9fef2
```
  30 | +    fMinimizeToTray = settings.value("fMinimizeToTray", false).toBool();
  31 | +    fMinimizeOnClose = settings.value("fMinimizeOnClose", false).toBool();
  32 | +    nTransactionFee = settings.value("nTransactionFee").toLongLong();
  33 | +
  34 | +    // These are shared with core bitcoin; we want
  35 | +    // command-line options to override the GUI settings:
```
sipa commented at 10:57 PM on February 25, 2012:

Does this mean that when settings are present in bitcoin.conf that are also set in the Qt settings, the ones from bitcoin.conf are used, but when trying to modify them, they go to Qt settings, where they are ignored?

laanwj commented at 7:16 AM on February 26, 2012:

That's on purpose, the settings in bitcoin.conf and the command line always override settings in the UI. However, if you change them later on in the UI, they will end up in the QSettings as normal. Which are overridden again on next start, if the user keeps them in the configuration file/command line.

I think that is the right behavior, as only advanced users will change bitcoin.conf and the command line, and they know what they're doing. It allows you to override settings such as proxy and upnp from the UI settings before they are used (when you edit them in the UI settings you're too late, networking code already started).

sipa commented at 1:34 PM on February 26, 2012:

True, in most cases that is the expected behaviour. Still, it could be very confusing at some point.
in src/qt/optionsmodel.cpp:None in 2281e9fef2
```
  23 | +void OptionsModel::Init()
  24 | +{
  25 | +    QSettings settings;
  26 | +
  27 | +    // These are QT-only settings:
  28 | +    nDisplayUnit = settings.value("nDisplayUnit", BitcoinUnits::BTC).toInt();
```
sipa commented at 11:00 PM on February 25, 2012:

I'd prefer the type-prefixes from Bitcoin's coding style not to be used in the setting-names, but that's a detail :)

laanwj commented at 7:27 AM on February 26, 2012:

That was my first though too -- but then I saw that option names would have to be re-mapped in the migration process from wallet.dat. Let's keep it simple :)

sipa commented at 1:35 PM on February 26, 2012:

Good point, never mind.
sipa commented at 5:04 PM on February 27, 2012: member

Included in #904
sipa closed this on Feb 27, 2012
destenson referenced this in commit 078d5d932b on Jun 26, 2016
ptschip referenced this in commit a7278ac15f on Jan 9, 2018
lateminer referenced this in commit fddac44eab on Oct 30, 2019
DrahtBot locked this on Sep 8, 2021

Contributors

gavinandresen

sipa

laanwj

Labels

GUI Refactoring Wallet

Linked (view graph)

#904 No options in wallet + fix proxy settings