Migrate -listen and -server settings from QSettings to settings.json

2 files changed, 19 insertions, 41 deletions

diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp
index 52327dd9f9..82da341516 100644
--- a/src/qt/optionsmodel.cpp
+++ b/src/qt/optionsmodel.cpp
@@ -43,6 +43,8 @@ static const char* SettingName(OptionsModel::OptionID option)
     case OptionsModel::ExternalSignerPath: return "signer";
     case OptionsModel::MapPortUPnP: return "upnp";
     case OptionsModel::MapPortNatpmp: return "natpmp";
+    case OptionsModel::Listen: return "listen";
+    case OptionsModel::Server: return "server";
     default: throw std::logic_error(strprintf("GUI option %i has no corresponding node setting.", option));
     }
 }
@@ -131,7 +133,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}) {
+                            MapPortNatpmp, Listen, Server}) {
         std::string setting = SettingName(option);
         if (node().isSettingIgnored(setting)) addOverriddenOption("-" + setting);
         try {
@@ -168,37 +170,6 @@ bool OptionsModel::Init(bilingual_str& error)
 #endif
 
     // Network
-    if (!settings.contains("fListen"))
-        settings.setValue("fListen", DEFAULT_LISTEN);
-    const bool listen{settings.value("fListen").toBool()};
-    if (!gArgs.SoftSetBoolArg("-listen", listen)) {
-        addOverriddenOption("-listen");
-    } else if (!listen) {
-        // We successfully set -listen=0, thus mimic the logic from InitParameterInteraction():
-        // "parameter interaction: -listen=0 -> setting -listenonion=0".
-        //
-        // Both -listen and -listenonion default to true.
-        //
-        // The call order is:
-        //
-        // InitParameterInteraction()
-        //     would set -listenonion=0 if it sees -listen=0, but for bitcoin-qt with
-        //     fListen=false -listen is 1 at this point
-        //
-        // OptionsModel::Init()
-        //     (this method) can flip -listen from 1 to 0 if fListen=false
-        //
-        // AppInitParameterInteraction()
-        //     raises an error if -listen=0 and -listenonion=1
-        gArgs.SoftSetBoolArg("-listenonion", false);
-    }
-
-    if (!settings.contains("server")) {
-        settings.setValue("server", false);
-    }
-    if (!gArgs.SoftSetBoolArg("-server", settings.value("server").toBool())) {
-        addOverriddenOption("-server");
-    }
 
     if (!settings.contains("fUseProxy"))
         settings.setValue("fUseProxy", false);
@@ -439,9 +410,9 @@ QVariant OptionsModel::getOption(OptionID option) const
     case ThreadsScriptVerif:
         return qlonglong(SettingToInt(setting(), DEFAULT_SCRIPTCHECK_THREADS));
     case Listen:
-        return settings.value("fListen");
+        return SettingToBool(setting(), DEFAULT_LISTEN);
     case Server:
-        return settings.value("server");
+        return SettingToBool(setting(), false);
     default:
         return QVariant();
     }
@@ -610,14 +581,9 @@ bool OptionsModel::setOption(OptionID option, const QVariant& value)
         }
         break;
     case Listen:
-        if (settings.value("fListen") != value) {
-            settings.setValue("fListen", value);
-            setRestartRequired(true);
-        }
-        break;
     case Server:
-        if (settings.value("server") != value) {
-            settings.setValue("server", value);
+        if (changed()) {
+            update(value.toBool());
             setRestartRequired(true);
         }
         break;
@@ -697,4 +663,13 @@ void OptionsModel::checkAndMigrate()
 #endif
     migrate_setting(MapPortUPnP, "fUseUPnP");
     migrate_setting(MapPortNatpmp, "fUseNatpmp");
+    migrate_setting(Listen, "fListen");
+    migrate_setting(Server, "server");
+
+    // In case migrating QSettings caused any settings value to change, rerun
+    // parameter interaction code to update other settings. This is particularly
+    // important for the -listen setting, which should cause -listenonion, -upnp,
+    // and other settings to default to false if it was set to false.
+    // (https://github.com/bitcoin-core/gui/issues/567).
+    node().initParameterInteraction();
 }
diff --git a/src/qt/test/optiontests.cpp b/src/qt/test/optiontests.cpp
index d84dc524cd..57b462c50b 100644
--- a/src/qt/test/optiontests.cpp
+++ b/src/qt/test/optiontests.cpp
@@ -35,6 +35,7 @@ void OptionTests::migrateSettings()
     settings.setValue("nDatabaseCache", 600);
     settings.setValue("nThreadsScriptVerif", 12);
     settings.setValue("fUseUPnP", false);
+    settings.setValue("fListen", false);
 
     settings.sync();
 
@@ -44,10 +45,12 @@ void OptionTests::migrateSettings()
     QVERIFY(!settings.contains("nDatabaseCache"));
     QVERIFY(!settings.contains("nThreadsScriptVerif"));
     QVERIFY(!settings.contains("fUseUPnP"));
+    QVERIFY(!settings.contains("fListen"));
 
     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"
         "}\n");
 }