Skip to content

Commit df20563

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

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
@@ -44,6 +44,12 @@ const char* SettingName(OptionsModel::OptionID option)
4444
case OptionsModel::MapPortNatpmp: return "natpmp";
4545
case OptionsModel::Listen: return "listen";
4646
case OptionsModel::Server: return "server";
47+
case OptionsModel::ProxyIP: return "proxy";
48+
case OptionsModel::ProxyPort: return "proxy";
49+
case OptionsModel::ProxyUse: return "proxy";
50+
case OptionsModel::ProxyIPTor: return "onion";
51+
case OptionsModel::ProxyPortTor: return "onion";
52+
case OptionsModel::ProxyUseTor: return "onion";
4753
default: throw std::logic_error(strprintf("GUI option %i has no corresponding node setting.", option));
4854
};
4955
}
@@ -63,6 +69,14 @@ void UpdateSetting(interfaces::Node& node, OptionsModel::OptionID option, const
6369
}
6470
}
6571

72+
struct ProxySetting {
73+
bool is_set;
74+
QString ip;
75+
QString port;
76+
};
77+
static ProxySetting ParseProxyString(const std::string& proxy);
78+
static std::string ProxyString(bool is_set, QString ip, QString port);
79+
6680
OptionsModel::OptionsModel(interfaces::Node& node, QObject *parent) :
6781
QAbstractListModel(parent), m_node{node}
6882
{
@@ -76,6 +90,14 @@ void OptionsModel::addOverriddenOption(const std::string &option)
7690
// Writes all missing QSettings with their default values
7791
bool OptionsModel::Init(bilingual_str& error)
7892
{
93+
// Initialize display settings from stored settings.
94+
ProxySetting proxy = ParseProxyString(SettingToString(node().getPersistentSetting("proxy"), GetDefaultProxyAddress().toStdString()));
95+
m_proxy_ip = proxy.ip;
96+
m_proxy_port = proxy.port;
97+
ProxySetting onion = ParseProxyString(SettingToString(node().getPersistentSetting("onion"), GetDefaultProxyAddress().toStdString()));
98+
m_onion_ip = onion.ip;
99+
m_onion_port = onion.port;
100+
79101
checkAndMigrate();
80102

81103
QSettings settings;
@@ -128,7 +150,7 @@ bool OptionsModel::Init(bilingual_str& error)
128150
// These are shared with the core or have a command-line parameter
129151
// and we want command-line parameters to overwrite the GUI settings.
130152
for (OptionID option : {DatabaseCache, ThreadsScriptVerif, SpendZeroConfChange, ExternalSignerPath, MapPortUPnP,
131-
MapPortNatpmp, Listen, Server}) {
153+
MapPortNatpmp, Listen, Server, ProxyUse, ProxyUseTor}) {
132154
std::string setting = SettingName(option);
133155
if (node().isSettingIgnored(setting)) addOverriddenOption("-" + setting);
134156
try {
@@ -163,28 +185,6 @@ bool OptionsModel::Init(bilingual_str& error)
163185
m_sub_fee_from_amount = settings.value("SubFeeFromAmount", false).toBool();
164186
#endif
165187

166-
// Network
167-
168-
if (!settings.contains("fUseProxy"))
169-
settings.setValue("fUseProxy", false);
170-
if (!settings.contains("addrProxy"))
171-
settings.setValue("addrProxy", GetDefaultProxyAddress());
172-
// Only try to set -proxy, if user has enabled fUseProxy
173-
if ((settings.value("fUseProxy").toBool() && !gArgs.SoftSetArg("-proxy", settings.value("addrProxy").toString().toStdString())))
174-
addOverriddenOption("-proxy");
175-
else if(!settings.value("fUseProxy").toBool() && !gArgs.GetArg("-proxy", "").empty())
176-
addOverriddenOption("-proxy");
177-
178-
if (!settings.contains("fUseSeparateProxyTor"))
179-
settings.setValue("fUseSeparateProxyTor", false);
180-
if (!settings.contains("addrSeparateProxyTor"))
181-
settings.setValue("addrSeparateProxyTor", GetDefaultProxyAddress());
182-
// Only try to set -onion, if user has enabled fUseSeparateProxyTor
183-
if ((settings.value("fUseSeparateProxyTor").toBool() && !gArgs.SoftSetArg("-onion", settings.value("addrSeparateProxyTor").toString().toStdString())))
184-
addOverriddenOption("-onion");
185-
else if(!settings.value("fUseSeparateProxyTor").toBool() && !gArgs.GetArg("-onion", "").empty())
186-
addOverriddenOption("-onion");
187-
188188
// Display
189189
if (!settings.contains("language"))
190190
settings.setValue("language", "");
@@ -251,31 +251,30 @@ int OptionsModel::rowCount(const QModelIndex & parent) const
251251
return OptionIDRowCount;
252252
}
253253

254-
struct ProxySetting {
255-
bool is_set;
256-
QString ip;
257-
QString port;
258-
};
259-
260-
static ProxySetting GetProxySetting(QSettings &settings, const QString &name)
254+
static ProxySetting ParseProxyString(const QString& proxy)
261255
{
262256
static const ProxySetting default_val = {false, DEFAULT_GUI_PROXY_HOST, QString("%1").arg(DEFAULT_GUI_PROXY_PORT)};
263257
// Handle the case that the setting is not set at all
264-
if (!settings.contains(name)) {
258+
if (proxy.isEmpty()) {
265259
return default_val;
266260
}
267261
// contains IP at index 0 and port at index 1
268-
QStringList ip_port = GUIUtil::SplitSkipEmptyParts(settings.value(name).toString(), ":");
262+
QStringList ip_port = GUIUtil::SplitSkipEmptyParts(proxy, ":");
269263
if (ip_port.size() == 2) {
270264
return {true, ip_port.at(0), ip_port.at(1)};
271265
} else { // Invalid: return default
272266
return default_val;
273267
}
274268
}
275269

276-
static void SetProxySetting(QSettings &settings, const QString &name, const ProxySetting &ip_port)
270+
static ProxySetting ParseProxyString(const std::string& proxy)
277271
{
278-
settings.setValue(name, QString{ip_port.ip + QLatin1Char(':') + ip_port.port});
272+
return ParseProxyString(QString::fromStdString(proxy));
273+
}
274+
275+
static std::string ProxyString(bool is_set, QString ip, QString port)
276+
{
277+
return is_set ? QString(ip + ":" + port).toStdString() : "";
279278
}
280279

281280
static const QString GetDefaultProxyAddress()
@@ -361,19 +360,19 @@ QVariant OptionsModel::getOption(OptionID option) const
361360

362361
// default proxy
363362
case ProxyUse:
364-
return settings.value("fUseProxy", false);
363+
return ParseProxyString(SettingToString(setting(), "")).is_set;
365364
case ProxyIP:
366-
return GetProxySetting(settings, "addrProxy").ip;
365+
return m_proxy_ip;
367366
case ProxyPort:
368-
return GetProxySetting(settings, "addrProxy").port;
367+
return m_proxy_port;
369368

370369
// separate Tor proxy
371370
case ProxyUseTor:
372-
return settings.value("fUseSeparateProxyTor", false);
371+
return ParseProxyString(SettingToString(setting(), "")).is_set;
373372
case ProxyIPTor:
374-
return GetProxySetting(settings, "addrSeparateProxyTor").ip;
373+
return m_onion_ip;
375374
case ProxyPortTor:
376-
return GetProxySetting(settings, "addrSeparateProxyTor").port;
375+
return m_onion_port;
377376

378377
#ifdef ENABLE_WALLET
379378
case SpendZeroConfChange:
@@ -452,55 +451,55 @@ bool OptionsModel::setOption(OptionID option, const QVariant& value)
452451

453452
// default proxy
454453
case ProxyUse:
455-
if (settings.value("fUseProxy") != value) {
456-
settings.setValue("fUseProxy", value.toBool());
454+
if (changed()) {
455+
update(ProxyString(value.toBool(), m_proxy_ip, m_proxy_port));
457456
setRestartRequired(true);
458457
}
459458
break;
460-
case ProxyIP: {
461-
auto ip_port = GetProxySetting(settings, "addrProxy");
462-
if (!ip_port.is_set || ip_port.ip != value.toString()) {
463-
ip_port.ip = value.toString();
464-
SetProxySetting(settings, "addrProxy", ip_port);
465-
setRestartRequired(true);
459+
case ProxyIP:
460+
if (changed()) {
461+
m_proxy_ip = value.toString();
462+
if (getOption(ProxyUse).toBool()) {
463+
update(ProxyString(true, m_proxy_ip, m_proxy_port));
464+
setRestartRequired(true);
465+
}
466466
}
467-
}
468-
break;
469-
case ProxyPort: {
470-
auto ip_port = GetProxySetting(settings, "addrProxy");
471-
if (!ip_port.is_set || ip_port.port != value.toString()) {
472-
ip_port.port = value.toString();
473-
SetProxySetting(settings, "addrProxy", ip_port);
474-
setRestartRequired(true);
467+
break;
468+
case ProxyPort:
469+
if (changed()) {
470+
m_proxy_port = value.toString();
471+
if (getOption(ProxyUse).toBool()) {
472+
update(ProxyString(true, m_proxy_ip, m_proxy_port));
473+
setRestartRequired(true);
474+
}
475475
}
476-
}
477-
break;
476+
break;
478477

479478
// separate Tor proxy
480479
case ProxyUseTor:
481-
if (settings.value("fUseSeparateProxyTor") != value) {
482-
settings.setValue("fUseSeparateProxyTor", value.toBool());
480+
if (changed()) {
481+
update(ProxyString(value.toBool(), m_onion_ip, m_onion_port));
483482
setRestartRequired(true);
484483
}
485484
break;
486-
case ProxyIPTor: {
487-
auto ip_port = GetProxySetting(settings, "addrSeparateProxyTor");
488-
if (!ip_port.is_set || ip_port.ip != value.toString()) {
489-
ip_port.ip = value.toString();
490-
SetProxySetting(settings, "addrSeparateProxyTor", ip_port);
491-
setRestartRequired(true);
485+
case ProxyIPTor:
486+
if (changed()) {
487+
m_onion_ip = value.toString();
488+
if (getOption(ProxyUseTor).toBool()) {
489+
update(ProxyString(true, m_onion_ip, m_onion_port));
490+
setRestartRequired(true);
491+
}
492492
}
493-
}
494-
break;
495-
case ProxyPortTor: {
496-
auto ip_port = GetProxySetting(settings, "addrSeparateProxyTor");
497-
if (!ip_port.is_set || ip_port.port != value.toString()) {
498-
ip_port.port = value.toString();
499-
SetProxySetting(settings, "addrSeparateProxyTor", ip_port);
500-
setRestartRequired(true);
493+
break;
494+
case ProxyPortTor:
495+
if (changed()) {
496+
m_onion_port = value.toString();
497+
if (getOption(ProxyUseTor).toBool()) {
498+
update(ProxyString(true, m_onion_ip, m_onion_port));
499+
setRestartRequired(true);
500+
}
501501
}
502-
}
503-
break;
502+
break;
504503

505504
#ifdef ENABLE_WALLET
506505
case SpendZeroConfChange:
@@ -649,7 +648,17 @@ void OptionsModel::checkAndMigrate()
649648
if (!settings.contains(qt_name)) return;
650649
QVariant value = settings.value(qt_name);
651650
if (node().getPersistentSetting(SettingName(option)).isNull()) {
652-
setOption(option, value);
651+
if (option == ProxyIP) {
652+
ProxySetting parsed = ParseProxyString(value.toString());
653+
setOption(ProxyIP, parsed.ip);
654+
setOption(ProxyPort, parsed.port);
655+
} else if (option == ProxyIPTor) {
656+
ProxySetting parsed = ParseProxyString(value.toString());
657+
setOption(ProxyIPTor, parsed.ip);
658+
setOption(ProxyPortTor, parsed.port);
659+
} else {
660+
setOption(option, value);
661+
}
653662
}
654663
settings.remove(qt_name);
655664
};
@@ -664,6 +673,10 @@ void OptionsModel::checkAndMigrate()
664673
migrate_setting(MapPortNatpmp, "fUseNatpmp");
665674
migrate_setting(Listen, "fListen");
666675
migrate_setting(Server, "server");
676+
migrate_setting(ProxyIP, "addrProxy");
677+
migrate_setting(ProxyUse, "fUseProxy");
678+
migrate_setting(ProxyIPTor, "addrSeparateProxyTor");
679+
migrate_setting(ProxyUseTor, "fUseSeparateProxyTor");
667680

668681
// In case migrating QSettings caused any settings value to change, rerun
669682
// 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
@@ -37,6 +37,10 @@ void OptionTests::migrateSettings()
3737
settings.setValue("nThreadsScriptVerif", 12);
3838
settings.setValue("fUseUPnP", false);
3939
settings.setValue("fListen", false);
40+
settings.setValue("fUseProxy", true);
41+
settings.setValue("addrProxy", "proxy:123");
42+
settings.setValue("fUseSeparateProxyTor", true);
43+
settings.setValue("addrSeparateProxyTor", "onion:234");
4044

4145
settings.sync();
4246

@@ -47,12 +51,18 @@ void OptionTests::migrateSettings()
4751
QVERIFY(!settings.contains("nThreadsScriptVerif"));
4852
QVERIFY(!settings.contains("fUseUPnP"));
4953
QVERIFY(!settings.contains("fListen"));
54+
QVERIFY(!settings.contains("fUseProxy"));
55+
QVERIFY(!settings.contains("addrProxy"));
56+
QVERIFY(!settings.contains("fUseSeparateProxyTor"));
57+
QVERIFY(!settings.contains("addrSeparateProxyTor"));
5058

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

0 commit comments

Comments
 (0)