Skip to content

Commit af9cdfd

Browse files
ryanofskyryanofsky
committed
Migrate -proxy and -onion settings from QSettings to settings.json
1 parent 7ded45d commit af9cdfd

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 {
@@ -168,28 +190,6 @@ bool OptionsModel::Init(bilingual_str& error)
168190
m_sub_fee_from_amount = settings.value("SubFeeFromAmount", false).toBool();
169191
#endif
170192

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

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

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

286285
static const QString GetDefaultProxyAddress()
@@ -366,19 +365,19 @@ QVariant OptionsModel::getOption(OptionID option) const
366365

367366
// default proxy
368367
case ProxyUse:
369-
return settings.value("fUseProxy", false);
368+
return ParseProxyString(SettingToString(setting(), "")).is_set;
370369
case ProxyIP:
371-
return GetProxySetting(settings, "addrProxy").ip;
370+
return m_proxy_ip;
372371
case ProxyPort:
373-
return GetProxySetting(settings, "addrProxy").port;
372+
return m_proxy_port;
374373

375374
// separate Tor proxy
376375
case ProxyUseTor:
377-
return settings.value("fUseSeparateProxyTor", false);
376+
return ParseProxyString(SettingToString(setting(), "")).is_set;
378377
case ProxyIPTor:
379-
return GetProxySetting(settings, "addrSeparateProxyTor").ip;
378+
return m_onion_ip;
380379
case ProxyPortTor:
381-
return GetProxySetting(settings, "addrSeparateProxyTor").port;
380+
return m_onion_port;
382381

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

458457
// default proxy
459458
case ProxyUse:
460-
if (settings.value("fUseProxy") != value) {
461-
settings.setValue("fUseProxy", value.toBool());
459+
if (changed()) {
460+
update(ProxyString(value.toBool(), m_proxy_ip, m_proxy_port));
462461
setRestartRequired(true);
463462
}
464463
break;
465-
case ProxyIP: {
466-
auto ip_port = GetProxySetting(settings, "addrProxy");
467-
if (!ip_port.is_set || ip_port.ip != value.toString()) {
468-
ip_port.ip = value.toString();
469-
SetProxySetting(settings, "addrProxy", ip_port);
470-
setRestartRequired(true);
464+
case ProxyIP:
465+
if (changed()) {
466+
m_proxy_ip = value.toString();
467+
if (getOption(ProxyUse).toBool()) {
468+
update(ProxyString(true, m_proxy_ip, m_proxy_port));
469+
setRestartRequired(true);
470+
}
471471
}
472-
}
473-
break;
474-
case ProxyPort: {
475-
auto ip_port = GetProxySetting(settings, "addrProxy");
476-
if (!ip_port.is_set || ip_port.port != value.toString()) {
477-
ip_port.port = value.toString();
478-
SetProxySetting(settings, "addrProxy", ip_port);
479-
setRestartRequired(true);
472+
break;
473+
case ProxyPort:
474+
if (changed()) {
475+
m_proxy_port = value.toString();
476+
if (getOption(ProxyUse).toBool()) {
477+
update(ProxyString(true, m_proxy_ip, m_proxy_port));
478+
setRestartRequired(true);
479+
}
480480
}
481-
}
482-
break;
481+
break;
483482

484483
// separate Tor proxy
485484
case ProxyUseTor:
486-
if (settings.value("fUseSeparateProxyTor") != value) {
487-
settings.setValue("fUseSeparateProxyTor", value.toBool());
485+
if (changed()) {
486+
update(ProxyString(value.toBool(), m_onion_ip, m_onion_port));
488487
setRestartRequired(true);
489488
}
490489
break;
491-
case ProxyIPTor: {
492-
auto ip_port = GetProxySetting(settings, "addrSeparateProxyTor");
493-
if (!ip_port.is_set || ip_port.ip != value.toString()) {
494-
ip_port.ip = value.toString();
495-
SetProxySetting(settings, "addrSeparateProxyTor", ip_port);
496-
setRestartRequired(true);
490+
case ProxyIPTor:
491+
if (changed()) {
492+
m_onion_ip = value.toString();
493+
if (getOption(ProxyUseTor).toBool()) {
494+
update(ProxyString(true, m_onion_ip, m_onion_port));
495+
setRestartRequired(true);
496+
}
497497
}
498-
}
499-
break;
500-
case ProxyPortTor: {
501-
auto ip_port = GetProxySetting(settings, "addrSeparateProxyTor");
502-
if (!ip_port.is_set || ip_port.port != value.toString()) {
503-
ip_port.port = value.toString();
504-
SetProxySetting(settings, "addrSeparateProxyTor", ip_port);
505-
setRestartRequired(true);
498+
break;
499+
case ProxyPortTor:
500+
if (changed()) {
501+
m_onion_port = value.toString();
502+
if (getOption(ProxyUseTor).toBool()) {
503+
update(ProxyString(true, m_onion_ip, m_onion_port));
504+
setRestartRequired(true);
505+
}
506506
}
507-
}
508-
break;
507+
break;
509508

510509
#ifdef ENABLE_WALLET
511510
case SpendZeroConfChange:
@@ -654,7 +653,17 @@ void OptionsModel::checkAndMigrate()
654653
if (!settings.contains(qt_name)) return;
655654
QVariant value = settings.value(qt_name);
656655
if (node().getPersistentSetting(SettingName(option)).isNull()) {
657-
setOption(option, value);
656+
if (option == ProxyIP) {
657+
ProxySetting parsed = ParseProxyString(value.toString());
658+
setOption(ProxyIP, parsed.ip);
659+
setOption(ProxyPort, parsed.port);
660+
} else if (option == ProxyIPTor) {
661+
ProxySetting parsed = ParseProxyString(value.toString());
662+
setOption(ProxyIPTor, parsed.ip);
663+
setOption(ProxyPortTor, parsed.port);
664+
} else {
665+
setOption(option, value);
666+
}
658667
}
659668
settings.remove(qt_name);
660669
};
@@ -669,6 +678,10 @@ void OptionsModel::checkAndMigrate()
669678
migrate_setting(MapPortNatpmp, "fUseNatpmp");
670679
migrate_setting(Listen, "fListen");
671680
migrate_setting(Server, "server");
681+
migrate_setting(ProxyIP, "addrProxy");
682+
migrate_setting(ProxyUse, "fUseProxy");
683+
migrate_setting(ProxyIPTor, "addrSeparateProxyTor");
684+
migrate_setting(ProxyUseTor, "fUseSeparateProxyTor");
672685

673686
// In case migrating QSettings caused any settings value to change, rerun
674687
// 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)