From f067e1943361b7bfa78a423528759e9edffa7482 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Mon, 29 Apr 2019 15:29:00 -0400 Subject: [PATCH] Migrate -proxy and -onion settings from QSettings to settings.json --- src/qt/optionsmodel.cpp | 167 +++++++++++++++++++----------------- src/qt/optionsmodel.h | 8 ++ src/qt/test/optiontests.cpp | 12 ++- 3 files changed, 109 insertions(+), 78 deletions(-) diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index 82da3415161..238727e6697 100644 --- a/src/qt/optionsmodel.cpp +++ b/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 { - bool is_set; - QString ip; - QString port; -}; - -static ProxySetting GetProxySetting(QSettings &settings, const QString &name) +static ProxySetting ParseProxyString(const QString& proxy) { 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) { - settings.setValue("fUseProxy", value.toBool()); + if (changed()) { + update(ProxyString(value.toBool(), m_proxy_ip, m_proxy_port)); setRestartRequired(true); } break; - case ProxyIP: { - auto ip_port = GetProxySetting(settings, "addrProxy"); - if (!ip_port.is_set || ip_port.ip != value.toString()) { - ip_port.ip = value.toString(); - SetProxySetting(settings, "addrProxy", ip_port); - setRestartRequired(true); + case ProxyIP: + if (changed()) { + m_proxy_ip = value.toString(); + if (getOption(ProxyUse).toBool()) { + update(ProxyString(true, m_proxy_ip, m_proxy_port)); + setRestartRequired(true); + } } - } - break; - case ProxyPort: { - auto ip_port = GetProxySetting(settings, "addrProxy"); - if (!ip_port.is_set || ip_port.port != value.toString()) { - ip_port.port = value.toString(); - SetProxySetting(settings, "addrProxy", ip_port); - setRestartRequired(true); + break; + case ProxyPort: + if (changed()) { + m_proxy_port = value.toString(); + if (getOption(ProxyUse).toBool()) { + update(ProxyString(true, m_proxy_ip, m_proxy_port)); + setRestartRequired(true); + } } - } - break; + break; // separate Tor proxy case ProxyUseTor: - if (settings.value("fUseSeparateProxyTor") != value) { - settings.setValue("fUseSeparateProxyTor", value.toBool()); + if (changed()) { + update(ProxyString(value.toBool(), m_onion_ip, m_onion_port)); setRestartRequired(true); } break; - case ProxyIPTor: { - auto ip_port = GetProxySetting(settings, "addrSeparateProxyTor"); - if (!ip_port.is_set || ip_port.ip != value.toString()) { - ip_port.ip = value.toString(); - SetProxySetting(settings, "addrSeparateProxyTor", ip_port); - setRestartRequired(true); + case ProxyIPTor: + if (changed()) { + m_onion_ip = value.toString(); + if (getOption(ProxyUseTor).toBool()) { + update(ProxyString(true, m_onion_ip, m_onion_port)); + setRestartRequired(true); + } } - } - break; - case ProxyPortTor: { - auto ip_port = GetProxySetting(settings, "addrSeparateProxyTor"); - if (!ip_port.is_set || ip_port.port != value.toString()) { - ip_port.port = value.toString(); - SetProxySetting(settings, "addrSeparateProxyTor", ip_port); - setRestartRequired(true); + break; + case ProxyPortTor: + if (changed()) { + m_onion_port = value.toString(); + if (getOption(ProxyUseTor).toBool()) { + update(ProxyString(true, m_onion_ip, m_onion_port)); + 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 diff --git a/src/qt/optionsmodel.h b/src/qt/optionsmodel.h index 9f8d17b305c..ca07a268e8e 100644 --- a/src/qt/optionsmodel.h +++ b/src/qt/optionsmodel.h @@ -122,6 +122,14 @@ class OptionsModel : public QAbstractListModel 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; diff --git a/src/qt/test/optiontests.cpp b/src/qt/test/optiontests.cpp index 57b462c50b7..443c9f2a93b 100644 --- a/src/qt/test/optiontests.cpp +++ b/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(file), std::istreambuf_iterator()).c_str(), "{\n" " \"dbcache\": \"600\",\n" " \"listen\": false,\n" - " \"par\": \"12\"\n" + " \"onion\": \"onion:234\",\n" + " \"par\": \"12\",\n" + " \"proxy\": \"proxy:123\"\n" "}\n"); }