Skip to content

Commit f067e19

Browse files
ryanofskyryanofsky
committed
Migrate -proxy and -onion settings from QSettings to settings.json
1 parent a09e3b7 commit f067e19

File tree

3 files changed

+109
-78
lines changed

3 files changed

+109
-78
lines changed

‎src/qt/optionsmodel.cpp

Lines changed: 90 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,12 @@ static const char* SettingName(OptionsModel::OptionID option)
4545
case OptionsModel::MapPortNatpmp: return "natpmp";
4646
case OptionsModel::Listen: return "listen";
4747
case OptionsModel::Server: return "server";
48+
case OptionsModel::ProxyIP: return "proxy";
49+
case OptionsModel::ProxyPort: return "proxy";
50+
case OptionsModel::ProxyUse: return "proxy";
51+
case OptionsModel::ProxyIPTor: return "onion";
52+
case OptionsModel::ProxyPortTor: return "onion";
53+
case OptionsModel::ProxyUseTor: return "onion";
4854
default: throw std::logic_error(strprintf("GUI option %i has no corresponding node setting.", option));
4955
}
5056
}
@@ -68,6 +74,14 @@ static void UpdateRwSetting(interfaces::Node& node, OptionsModel::OptionID optio
6874
}
6975
}
7076

77+
struct ProxySetting {
78+
bool is_set;
79+
QString ip;
80+
QString port;
81+
};
82+
static ProxySetting ParseProxyString(const std::string& proxy);
83+
static std::string ProxyString(bool is_set, QString ip, QString port);
84+
7185
OptionsModel::OptionsModel(interfaces::Node& node, QObject *parent) :
7286
QAbstractListModel(parent), m_node{node}
7387
{
@@ -81,6 +95,14 @@ void OptionsModel::addOverriddenOption(const std::string &option)
8195
// Writes all missing QSettings with their default values
8296
bool OptionsModel::Init(bilingual_str& error)
8397
{
98+
// Initialize display settings from stored settings.
99+
ProxySetting proxy = ParseProxyString(SettingToString(node().getPersistentSetting("proxy"), GetDefaultProxyAddress().toStdString()));
100+
m_proxy_ip = proxy.ip;
101+
m_proxy_port = proxy.port;
102+
ProxySetting onion = ParseProxyString(SettingToString(node().getPersistentSetting("onion"), GetDefaultProxyAddress().toStdString()));
103+
m_onion_ip = onion.ip;
104+
m_onion_port = onion.port;
105+
84106
checkAndMigrate();
85107

86108
QSettings settings;
@@ -133,7 +155,7 @@ bool OptionsModel::Init(bilingual_str& error)
133155
// These are shared with the core or have a command-line parameter
134156
// and we want command-line parameters to overwrite the GUI settings.
135157
for (OptionID option : {DatabaseCache, ThreadsScriptVerif, SpendZeroConfChange, ExternalSignerPath, MapPortUPnP,
136-
MapPortNatpmp, Listen, Server}) {
158+
MapPortNatpmp, Listen, Server, ProxyUse, ProxyUseTor}) {
137159
std::string setting = SettingName(option);
138160
if (node().isSettingIgnored(setting)) addOverriddenOption("-" + setting);
139161
try {
@@ -169,28 +191,6 @@ bool OptionsModel::Init(bilingual_str& error)
169191
m_sub_fee_from_amount = settings.value("SubFeeFromAmount", false).toBool();
170192
#endif
171193

172-
// Network
173-
174-
if (!settings.contains("fUseProxy"))
175-
settings.setValue("fUseProxy", false);
176-
if (!settings.contains("addrProxy"))
177-
settings.setValue("addrProxy", GetDefaultProxyAddress());
178-
// Only try to set -proxy, if user has enabled fUseProxy
179-
if ((settings.value("fUseProxy").toBool() && !gArgs.SoftSetArg("-proxy", settings.value("addrProxy").toString().toStdString())))
180-
addOverriddenOption("-proxy");
181-
else if(!settings.value("fUseProxy").toBool() && !gArgs.GetArg("-proxy", "").empty())
182-
addOverriddenOption("-proxy");
183-
184-
if (!settings.contains("fUseSeparateProxyTor"))
185-
settings.setValue("fUseSeparateProxyTor", false);
186-
if (!settings.contains("addrSeparateProxyTor"))
187-
settings.setValue("addrSeparateProxyTor", GetDefaultProxyAddress());
188-
// Only try to set -onion, if user has enabled fUseSeparateProxyTor
189-
if ((settings.value("fUseSeparateProxyTor").toBool() && !gArgs.SoftSetArg("-onion", settings.value("addrSeparateProxyTor").toString().toStdString())))
190-
addOverriddenOption("-onion");
191-
else if(!settings.value("fUseSeparateProxyTor").toBool() && !gArgs.GetArg("-onion", "").empty())
192-
addOverriddenOption("-onion");
193-
194194
// Display
195195
if (!settings.contains("language"))
196196
settings.setValue("language", "");
@@ -257,31 +257,30 @@ int OptionsModel::rowCount(const QModelIndex & parent) const
257257
return OptionIDRowCount;
258258
}
259259

260-
struct ProxySetting {
261-
bool is_set;
262-
QString ip;
263-
QString port;
264-
};
265-
266-
static ProxySetting GetProxySetting(QSettings &settings, const QString &name)
260+
static ProxySetting ParseProxyString(const QString& proxy)
267261
{
268262
static const ProxySetting default_val = {false, DEFAULT_GUI_PROXY_HOST, QString("%1").arg(DEFAULT_GUI_PROXY_PORT)};
269263
// Handle the case that the setting is not set at all
270-
if (!settings.contains(name)) {
264+
if (proxy.isEmpty()) {
271265
return default_val;
272266
}
273267
// contains IP at index 0 and port at index 1
274-
QStringList ip_port = GUIUtil::SplitSkipEmptyParts(settings.value(name).toString(), ":");
268+
QStringList ip_port = GUIUtil::SplitSkipEmptyParts(proxy, ":");
275269
if (ip_port.size() == 2) {
276270
return {true, ip_port.at(0), ip_port.at(1)};
277271
} else { // Invalid: return default
278272
return default_val;
279273
}
280274
}
281275

282-
static void SetProxySetting(QSettings &settings, const QString &name, const ProxySetting &ip_port)
276+
static ProxySetting ParseProxyString(const std::string& proxy)
283277
{
284-
settings.setValue(name, QString{ip_port.ip + QLatin1Char(':') + ip_port.port});
278+
return ParseProxyString(QString::fromStdString(proxy));
279+
}
280+
281+
static std::string ProxyString(bool is_set, QString ip, QString port)
282+
{
283+
return is_set ? QString(ip + ":" + port).toStdString() : "";
285284
}
286285

287286
static const QString GetDefaultProxyAddress()
@@ -367,19 +366,19 @@ QVariant OptionsModel::getOption(OptionID option) const
367366

368367
// default proxy
369368
case ProxyUse:
370-
return settings.value("fUseProxy", false);
369+
return ParseProxyString(SettingToString(setting(), "")).is_set;
371370
case ProxyIP:
372-
return GetProxySetting(settings, "addrProxy").ip;
371+
return m_proxy_ip;
373372
case ProxyPort:
374-
return GetProxySetting(settings, "addrProxy").port;
373+
return m_proxy_port;
375374

376375
// separate Tor proxy
377376
case ProxyUseTor:
378-
return settings.value("fUseSeparateProxyTor", false);
377+
return ParseProxyString(SettingToString(setting(), "")).is_set;
379378
case ProxyIPTor:
380-
return GetProxySetting(settings, "addrSeparateProxyTor").ip;
379+
return m_onion_ip;
381380
case ProxyPortTor:
382-
return GetProxySetting(settings, "addrSeparateProxyTor").port;
381+
return m_onion_port;
383382

384383
#ifdef ENABLE_WALLET
385384
case SpendZeroConfChange:
@@ -458,55 +457,55 @@ bool OptionsModel::setOption(OptionID option, const QVariant& value)
458457

459458
// default proxy
460459
case ProxyUse:
461-
if (settings.value("fUseProxy") != value) {
462-
settings.setValue("fUseProxy", value.toBool());
460+
if (changed()) {
461+
update(ProxyString(value.toBool(), m_proxy_ip, m_proxy_port));
463462
setRestartRequired(true);
464463
}
465464
break;
466-
case ProxyIP: {
467-
auto ip_port = GetProxySetting(settings, "addrProxy");
468-
if (!ip_port.is_set || ip_port.ip != value.toString()) {
469-
ip_port.ip = value.toString();
470-
SetProxySetting(settings, "addrProxy", ip_port);
471-
setRestartRequired(true);
465+
case ProxyIP:
466+
if (changed()) {
467+
m_proxy_ip = value.toString();
468+
if (getOption(ProxyUse).toBool()) {
469+
update(ProxyString(true, m_proxy_ip, m_proxy_port));
470+
setRestartRequired(true);
471+
}
472472
}
473-
}
474-
break;
475-
case ProxyPort: {
476-
auto ip_port = GetProxySetting(settings, "addrProxy");
477-
if (!ip_port.is_set || ip_port.port != value.toString()) {
478-
ip_port.port = value.toString();
479-
SetProxySetting(settings, "addrProxy", ip_port);
480-
setRestartRequired(true);
473+
break;
474+
case ProxyPort:
475+
if (changed()) {
476+
m_proxy_port = value.toString();
477+
if (getOption(ProxyUse).toBool()) {
478+
update(ProxyString(true, m_proxy_ip, m_proxy_port));
479+
setRestartRequired(true);
480+
}
481481
}
482-
}
483-
break;
482+
break;
484483

485484
// separate Tor proxy
486485
case ProxyUseTor:
487-
if (settings.value("fUseSeparateProxyTor") != value) {
488-
settings.setValue("fUseSeparateProxyTor", value.toBool());
486+
if (changed()) {
487+
update(ProxyString(value.toBool(), m_onion_ip, m_onion_port));
489488
setRestartRequired(true);
490489
}
491490
break;
492-
case ProxyIPTor: {
493-
auto ip_port = GetProxySetting(settings, "addrSeparateProxyTor");
494-
if (!ip_port.is_set || ip_port.ip != value.toString()) {
495-
ip_port.ip = value.toString();
496-
SetProxySetting(settings, "addrSeparateProxyTor", ip_port);
497-
setRestartRequired(true);
491+
case ProxyIPTor:
492+
if (changed()) {
493+
m_onion_ip = value.toString();
494+
if (getOption(ProxyUseTor).toBool()) {
495+
update(ProxyString(true, m_onion_ip, m_onion_port));
496+
setRestartRequired(true);
497+
}
498498
}
499-
}
500-
break;
501-
case ProxyPortTor: {
502-
auto ip_port = GetProxySetting(settings, "addrSeparateProxyTor");
503-
if (!ip_port.is_set || ip_port.port != value.toString()) {
504-
ip_port.port = value.toString();
505-
SetProxySetting(settings, "addrSeparateProxyTor", ip_port);
506-
setRestartRequired(true);
499+
break;
500+
case ProxyPortTor:
501+
if (changed()) {
502+
m_onion_port = value.toString();
503+
if (getOption(ProxyUseTor).toBool()) {
504+
update(ProxyString(true, m_onion_ip, m_onion_port));
505+
setRestartRequired(true);
506+
}
507507
}
508-
}
509-
break;
508+
break;
510509

511510
#ifdef ENABLE_WALLET
512511
case SpendZeroConfChange:
@@ -650,7 +649,17 @@ void OptionsModel::checkAndMigrate()
650649
if (!settings.contains(qt_name)) return;
651650
QVariant value = settings.value(qt_name);
652651
if (node().getPersistentSetting(SettingName(option)).isNull()) {
653-
setOption(option, value);
652+
if (option == ProxyIP) {
653+
ProxySetting parsed = ParseProxyString(value.toString());
654+
setOption(ProxyIP, parsed.ip);
655+
setOption(ProxyPort, parsed.port);
656+
} else if (option == ProxyIPTor) {
657+
ProxySetting parsed = ParseProxyString(value.toString());
658+
setOption(ProxyIPTor, parsed.ip);
659+
setOption(ProxyPortTor, parsed.port);
660+
} else {
661+
setOption(option, value);
662+
}
654663
}
655664
settings.remove(qt_name);
656665
};
@@ -665,6 +674,10 @@ void OptionsModel::checkAndMigrate()
665674
migrate_setting(MapPortNatpmp, "fUseNatpmp");
666675
migrate_setting(Listen, "fListen");
667676
migrate_setting(Server, "server");
677+
migrate_setting(ProxyIP, "addrProxy");
678+
migrate_setting(ProxyUse, "fUseProxy");
679+
migrate_setting(ProxyIPTor, "addrSeparateProxyTor");
680+
migrate_setting(ProxyUseTor, "fUseSeparateProxyTor");
668681

669682
// In case migrating QSettings caused any settings value to change, rerun
670683
// parameter interaction code to update other settings. This is particularly

‎src/qt/optionsmodel.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,14 @@ class OptionsModel : public QAbstractListModel
122122
bool m_sub_fee_from_amount;
123123
bool m_enable_psbt_controls;
124124

125+
//! In-memory settings for display. These are stored persistently by the
126+
//! bitcoin node but it's also nice to store them in memory to prevent them
127+
//! getting cleared when enable/disable toggles are used in the GUI.
128+
QString m_proxy_ip;
129+
QString m_proxy_port;
130+
QString m_onion_ip;
131+
QString m_onion_port;
132+
125133
/* settings that were overridden by command-line */
126134
QString strOverriddenByCommandLine;
127135

‎src/qt/test/optiontests.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ void OptionTests::migrateSettings()
3636
settings.setValue("nThreadsScriptVerif", 12);
3737
settings.setValue("fUseUPnP", false);
3838
settings.setValue("fListen", false);
39+
settings.setValue("fUseProxy", true);
40+
settings.setValue("addrProxy", "proxy:123");
41+
settings.setValue("fUseSeparateProxyTor", true);
42+
settings.setValue("addrSeparateProxyTor", "onion:234");
3943

4044
settings.sync();
4145

@@ -46,12 +50,18 @@ void OptionTests::migrateSettings()
4650
QVERIFY(!settings.contains("nThreadsScriptVerif"));
4751
QVERIFY(!settings.contains("fUseUPnP"));
4852
QVERIFY(!settings.contains("fListen"));
53+
QVERIFY(!settings.contains("fUseProxy"));
54+
QVERIFY(!settings.contains("addrProxy"));
55+
QVERIFY(!settings.contains("fUseSeparateProxyTor"));
56+
QVERIFY(!settings.contains("addrSeparateProxyTor"));
4957

5058
std::ifstream file(gArgs.GetDataDirNet() / "settings.json");
5159
QCOMPARE(std::string(std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>()).c_str(), "{\n"
5260
" \"dbcache\": \"600\",\n"
5361
" \"listen\": false,\n"
54-
" \"par\": \"12\"\n"
62+
" \"onion\": \"onion:234\",\n"
63+
" \"par\": \"12\",\n"
64+
" \"proxy\": \"proxy:123\"\n"
5565
"}\n");
5666
}
5767

0 commit comments

Comments
 (0)