Migrate -proxy and -onion settings from QSettings to settings.json

src/qt/optionsmodel.cpp

@@ -45,6 +45,12 @@ static const char* SettingName(OptionsModel::OptionID option)
     case OptionsModel::MapPortNatpmp: return "natpmp";
     case OptionsModel::Listen: return "listen";
     case OptionsModel::Server: return "server";
+    case OptionsModel::ProxyIP: return "proxy";
+    case OptionsModel::ProxyPort: return "proxy";
+    case OptionsModel::ProxyUse: return "proxy";
+    case OptionsModel::ProxyIPTor: return "onion";
+    case OptionsModel::ProxyPortTor: return "onion";
+    case OptionsModel::ProxyUseTor: return "onion";
     default: throw std::logic_error(strprintf("GUI option %i has no corresponding node setting.", option));
     }
 }
@@ -68,6 +74,14 @@ static void UpdateRwSetting(interfaces::Node& node, OptionsModel::OptionID optio
     }
 }
 
+struct ProxySetting {
+    bool is_set;
+    QString ip;
+    QString port;
+};
+static ProxySetting ParseProxyString(const std::string& proxy);
+static std::string ProxyString(bool is_set, QString ip, QString port);
+
 OptionsModel::OptionsModel(interfaces::Node& node, QObject *parent) :
     QAbstractListModel(parent), m_node{node}
 {
@@ -81,6 +95,14 @@ void OptionsModel::addOverriddenOption(const std::string &option)
 // Writes all missing QSettings with their default values
 bool OptionsModel::Init(bilingual_str& error)
 {
+    // Initialize display settings from stored settings.
+    ProxySetting proxy = ParseProxyString(SettingToString(node().getPersistentSetting("proxy"), GetDefaultProxyAddress().toStdString()));
+    m_proxy_ip = proxy.ip;
+    m_proxy_port = proxy.port;
+    ProxySetting onion = ParseProxyString(SettingToString(node().getPersistentSetting("onion"), GetDefaultProxyAddress().toStdString()));
+    m_onion_ip = onion.ip;
+    m_onion_port = onion.port;
+
     checkAndMigrate();
 
     QSettings settings;
@@ -133,7 +155,7 @@ bool OptionsModel::Init(bilingual_str& error)
     // These are shared with the core or have a command-line parameter
     // and we want command-line parameters to overwrite the GUI settings.
     for (OptionID option : {DatabaseCache, ThreadsScriptVerif, SpendZeroConfChange, ExternalSignerPath, MapPortUPnP,
-                            MapPortNatpmp, Listen, Server}) {
+                            MapPortNatpmp, Listen, Server, ProxyUse, ProxyUseTor}) {
         std::string setting = SettingName(option);
         if (node().isSettingIgnored(setting)) addOverriddenOption("-" + setting);
         try {
@@ -169,28 +191,6 @@ bool OptionsModel::Init(bilingual_str& error)
     m_sub_fee_from_amount = settings.value("SubFeeFromAmount", false).toBool();
 #endif
 
-    // Network
-
-    if (!settings.contains("fUseProxy"))
-        settings.setValue("fUseProxy", false);
-    if (!settings.contains("addrProxy"))
-        settings.setValue("addrProxy", GetDefaultProxyAddress());
-    // Only try to set -proxy, if user has enabled fUseProxy
-    if ((settings.value("fUseProxy").toBool() && !gArgs.SoftSetArg("-proxy", settings.value("addrProxy").toString().toStdString())))
-        addOverriddenOption("-proxy");
-    else if(!settings.value("fUseProxy").toBool() && !gArgs.GetArg("-proxy", "").empty())
-        addOverriddenOption("-proxy");
-
-    if (!settings.contains("fUseSeparateProxyTor"))
-        settings.setValue("fUseSeparateProxyTor", false);
-    if (!settings.contains("addrSeparateProxyTor"))
-        settings.setValue("addrSeparateProxyTor", GetDefaultProxyAddress());
-    // Only try to set -onion, if user has enabled fUseSeparateProxyTor
-    if ((settings.value("fUseSeparateProxyTor").toBool() && !gArgs.SoftSetArg("-onion", settings.value("addrSeparateProxyTor").toString().toStdString())))
-        addOverriddenOption("-onion");
-    else if(!settings.value("fUseSeparateProxyTor").toBool() && !gArgs.GetArg("-onion", "").empty())
-        addOverriddenOption("-onion");
-
     // Display
     if (!settings.contains("language"))
         settings.setValue("language", "");
@@ -257,21 +257,15 @@ int OptionsModel::rowCount(const QModelIndex & parent) const
     return OptionIDRowCount;
 }
 
-struct ProxySetting {
+static ProxySetting ParseProxyString(const QString& proxy)
-    bool is_set;
-    QString ip;
-    QString port;
-};
-
-static ProxySetting GetProxySetting(QSettings &settings, const QString &name)
 {
     static const ProxySetting default_val = {false, DEFAULT_GUI_PROXY_HOST, QString("%1").arg(DEFAULT_GUI_PROXY_PORT)};
     // Handle the case that the setting is not set at all
-    if (!settings.contains(name)) {
+    if (proxy.isEmpty()) {
         return default_val;
     }
     // contains IP at index 0 and port at index 1
-    QStringList ip_port = GUIUtil::SplitSkipEmptyParts(settings.value(name).toString(), ":");
+    QStringList ip_port = GUIUtil::SplitSkipEmptyParts(proxy, ":");
     if (ip_port.size() == 2) {
         return {true, ip_port.at(0), ip_port.at(1)};
     } else { // Invalid: return default
@@ -279,9 +273,14 @@ static ProxySetting GetProxySetting(QSettings &settings, const QString &name)
     }
 }
 
-static void SetProxySetting(QSettings &settings, const QString &name, const ProxySetting &ip_port)
+static ProxySetting ParseProxyString(const std::string& proxy)
 {
-    settings.setValue(name, QString{ip_port.ip + QLatin1Char(':') + ip_port.port});
+    return ParseProxyString(QString::fromStdString(proxy));
+}
+
+static std::string ProxyString(bool is_set, QString ip, QString port)
+{
+    return is_set ? QString(ip + ":" + port).toStdString() : "";
 }
 
 static const QString GetDefaultProxyAddress()
@@ -367,19 +366,19 @@ QVariant OptionsModel::getOption(OptionID option) const
 
     // default proxy
     case ProxyUse:
-        return settings.value("fUseProxy", false);
+        return ParseProxyString(SettingToString(setting(), "")).is_set;
     case ProxyIP:
-        return GetProxySetting(settings, "addrProxy").ip;
+        return m_proxy_ip;
     case ProxyPort:
-        return GetProxySetting(settings, "addrProxy").port;
+        return m_proxy_port;
 
     // separate Tor proxy
     case ProxyUseTor:
-        return settings.value("fUseSeparateProxyTor", false);
+        return ParseProxyString(SettingToString(setting(), "")).is_set;
     case ProxyIPTor:
-        return GetProxySetting(settings, "addrSeparateProxyTor").ip;
+        return m_onion_ip;
     case ProxyPortTor:
-        return GetProxySetting(settings, "addrSeparateProxyTor").port;
+        return m_onion_port;
 
 #ifdef ENABLE_WALLET
     case SpendZeroConfChange:
@@ -458,55 +457,55 @@ bool OptionsModel::setOption(OptionID option, const QVariant& value)
 
     // default proxy
     case ProxyUse:
-        if (settings.value("fUseProxy") != value) {
+        if (changed()) {
-            settings.setValue("fUseProxy", value.toBool());
+            update(ProxyString(value.toBool(), m_proxy_ip, m_proxy_port));
             setRestartRequired(true);
         }
         break;
-    case ProxyIP: {
+    case ProxyIP:
-        auto ip_port = GetProxySetting(settings, "addrProxy");
+        if (changed()) {
-        if (!ip_port.is_set || ip_port.ip != value.toString()) {
+            m_proxy_ip = value.toString();
-            ip_port.ip = value.toString();
+            if (getOption(ProxyUse).toBool()) {
-            SetProxySetting(settings, "addrProxy", ip_port);
+                update(ProxyString(true, m_proxy_ip, m_proxy_port));
-            setRestartRequired(true);
+                setRestartRequired(true);
+            }
         }
-    }
+        break;
-    break;
+    case ProxyPort:
-    case ProxyPort: {
+        if (changed()) {
-        auto ip_port = GetProxySetting(settings, "addrProxy");
+            m_proxy_port = value.toString();
-        if (!ip_port.is_set || ip_port.port != value.toString()) {
+            if (getOption(ProxyUse).toBool()) {
-            ip_port.port = value.toString();
+                update(ProxyString(true, m_proxy_ip, m_proxy_port));
-            SetProxySetting(settings, "addrProxy", ip_port);
+                setRestartRequired(true);
-            setRestartRequired(true);
+            }
         }
-    }
+        break;
-    break;
 
     // separate Tor proxy
     case ProxyUseTor:
-        if (settings.value("fUseSeparateProxyTor") != value) {
+        if (changed()) {
-            settings.setValue("fUseSeparateProxyTor", value.toBool());
+            update(ProxyString(value.toBool(), m_onion_ip, m_onion_port));
             setRestartRequired(true);
         }
         break;
-    case ProxyIPTor: {
+    case ProxyIPTor:
-        auto ip_port = GetProxySetting(settings, "addrSeparateProxyTor");
+        if (changed()) {
-        if (!ip_port.is_set || ip_port.ip != value.toString()) {
+            m_onion_ip = value.toString();
-            ip_port.ip = value.toString();
+            if (getOption(ProxyUseTor).toBool()) {
-            SetProxySetting(settings, "addrSeparateProxyTor", ip_port);
+                update(ProxyString(true, m_onion_ip, m_onion_port));
-            setRestartRequired(true);
+                setRestartRequired(true);
+            }
         }
-    }
+        break;
-    break;
+    case ProxyPortTor:
-    case ProxyPortTor: {
+        if (changed()) {
-        auto ip_port = GetProxySetting(settings, "addrSeparateProxyTor");
+            m_onion_port = value.toString();
-        if (!ip_port.is_set || ip_port.port != value.toString()) {
+            if (getOption(ProxyUseTor).toBool()) {
-            ip_port.port = value.toString();
+                update(ProxyString(true, m_onion_ip, m_onion_port));
-            SetProxySetting(settings, "addrSeparateProxyTor", ip_port);
+                setRestartRequired(true);
-            setRestartRequired(true);
+            }
         }
-    }
+        break;
-    break;
 
 #ifdef ENABLE_WALLET
     case SpendZeroConfChange:
@@ -650,7 +649,17 @@ void OptionsModel::checkAndMigrate()
         if (!settings.contains(qt_name)) return;
         QVariant value = settings.value(qt_name);
         if (node().getPersistentSetting(SettingName(option)).isNull()) {
-            setOption(option, value);
+            if (option == ProxyIP) {
+                ProxySetting parsed = ParseProxyString(value.toString());
+                setOption(ProxyIP, parsed.ip);
+                setOption(ProxyPort, parsed.port);
+            } else if (option == ProxyIPTor) {
+                ProxySetting parsed = ParseProxyString(value.toString());
+                setOption(ProxyIPTor, parsed.ip);
+                setOption(ProxyPortTor, parsed.port);
+            } else {
+                setOption(option, value);
+            }
         }
         settings.remove(qt_name);
     };
@@ -665,6 +674,10 @@ void OptionsModel::checkAndMigrate()
     migrate_setting(MapPortNatpmp, "fUseNatpmp");
     migrate_setting(Listen, "fListen");
     migrate_setting(Server, "server");
+    migrate_setting(ProxyIP, "addrProxy");
+    migrate_setting(ProxyUse, "fUseProxy");
+    migrate_setting(ProxyIPTor, "addrSeparateProxyTor");
+    migrate_setting(ProxyUseTor, "fUseSeparateProxyTor");
 
     // In case migrating QSettings caused any settings value to change, rerun
     // parameter interaction code to update other settings. This is particularly

src/qt/optionsmodel.h

@@ -122,6 +122,14 @@ private:
     bool m_sub_fee_from_amount;
     bool m_enable_psbt_controls;
 
+    //! In-memory settings for display. These are stored persistently by the
+    //! bitcoin node but it's also nice to store them in memory to prevent them
+    //! getting cleared when enable/disable toggles are used in the GUI.
+    QString m_proxy_ip;
+    QString m_proxy_port;
+    QString m_onion_ip;
+    QString m_onion_port;
+
     /* settings that were overridden by command-line */
     QString strOverriddenByCommandLine;
 

src/qt/test/optiontests.cpp

@@ -36,6 +36,10 @@ void OptionTests::migrateSettings()
     settings.setValue("nThreadsScriptVerif", 12);
     settings.setValue("fUseUPnP", false);
     settings.setValue("fListen", false);
+    settings.setValue("fUseProxy", true);
+    settings.setValue("addrProxy", "proxy:123");
+    settings.setValue("fUseSeparateProxyTor", true);
+    settings.setValue("addrSeparateProxyTor", "onion:234");
 
     settings.sync();
 
@@ -46,12 +50,18 @@ void OptionTests::migrateSettings()
     QVERIFY(!settings.contains("nThreadsScriptVerif"));
     QVERIFY(!settings.contains("fUseUPnP"));
     QVERIFY(!settings.contains("fListen"));
+    QVERIFY(!settings.contains("fUseProxy"));
+    QVERIFY(!settings.contains("addrProxy"));
+    QVERIFY(!settings.contains("fUseSeparateProxyTor"));
+    QVERIFY(!settings.contains("addrSeparateProxyTor"));
 
     std::ifstream file(gArgs.GetDataDirNet() / "settings.json");
     QCOMPARE(std::string(std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>()).c_str(), "{\n"
         "    \"dbcache\": \"600\",\n"
         "    \"listen\": false,\n"
-        "    \"par\": \"12\"\n"
+        "    \"onion\": \"onion:234\",\n"
+        "    \"par\": \"12\",\n"
+        "    \"proxy\": \"proxy:123\"\n"
         "}\n");
 }