Merge bitcoin-core/gui#602: Unify bitcoin-qt and bitcoind persistent settings

e47c6c76561807d30cff3c2e5372ea83c91a3677 Reset settings.json when GUI options are reset (Ryan Ofsky)
99ccc02b652cf67a3aec66371fcb6bbe737571a7 Add release notes about unified bitcoin-qt and bitcoind persistent settings (Ryan Ofsky)
504b06b1dec9d9329c83b13c7c36ca710ebcd349 Migrate -lang setting from QSettings to settings.json (Ryan Ofsky)
9a016a3c07d4becf0651ef58c7160180c5f25a0c Migrate -prune setting from QSettings to settings.json (Ryan Ofsky)
f067e1943361b7bfa78a423528759e9edffa7482 Migrate -proxy and -onion settings from QSettings to settings.json (Ryan Ofsky)
a09e3b7cf29c3b1fd320badbed32275e0aa83cda Migrate -listen and -server settings from QSettings to settings.json (Ryan Ofsky)
d2ada6e63583cad91e92b49c4dbf8c7ff086a758 Migrate -upnp and -natpmp settings from QSettings to settings.json (Ryan Ofsky)
1dc4fc29c1086420b7dc51b20c0b7a18fecb4462 Migrate -spendzeroconfchange and -signer settings from QSettings to settings.json (Ryan Ofsky)
a7ef6d5975a5f40b90b2709b32a00647bd2bd5a3 Migrate -par setting from QSettings to settings.json (Ryan Ofsky)
284f339de68905131331f7fdb4c0b945c9a1b8cd Migrate -dbcache setting from QSettings to settings.json (Ryan Ofsky)

Pull request description:

  If a setting like pruning, port mapping, or a network proxy is enabled in the GUI, it will now be stored in the bitcoin persistent setting file in the datadir and shared with bitcoind, instead of being stored as Qt settings which end up in the the windows registry or platform specific config files and are ignored by bitcoind.

  This PR has been split off from bitcoin/bitcoin#15936 so some review of these commits previously took place in that PR.

ACKs for top commit:
  furszy:
    Code review ACK e47c6c76
  hebasto:
    ACK e47c6c76561807d30cff3c2e5372ea83c91a3677

Tree-SHA512: 076ea7c7efe67805b4a357113bfe1643dce364d0032774106de59566a0ed5771d57a5923920085e03d686beb34b98114bd278555dfdf8bb7af0b778b0f35b7d2

6 files changed, 75 insertions, 13 deletions

diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp
index 3b7a40438b..581735263d 100644
--- a/src/qt/test/addressbooktests.cpp
+++ b/src/qt/test/addressbooktests.cpp
@@ -128,6 +128,8 @@ void TestAddAddressesToSendBook(interfaces::Node& node)
     // Initialize relevant QT models.
     std::unique_ptr<const PlatformStyle> platformStyle(PlatformStyle::instantiate("other"));
     OptionsModel optionsModel(node);
+    bilingual_str error;
+    QVERIFY(optionsModel.Init(error));
     ClientModel clientModel(node, &optionsModel);
     WalletContext& context = *node.walletLoader().context();
     AddWallet(context, wallet);
diff --git a/src/qt/test/apptests.cpp b/src/qt/test/apptests.cpp
index 9648ef6188..6fc7a52435 100644
--- a/src/qt/test/apptests.cpp
+++ b/src/qt/test/apptests.cpp
@@ -70,14 +70,9 @@ void AppTests::appTests()
     }
 #endif
 
-    fs::create_directories([] {
-        BasicTestingSetup test{CBaseChainParams::REGTEST}; // Create a temp data directory to backup the gui settings to
-        return gArgs.GetDataDirNet() / "blocks";
-    }());
-
     qRegisterMetaType<interfaces::BlockAndHeaderTipInfo>("interfaces::BlockAndHeaderTipInfo");
     m_app.parameterSetup();
-    m_app.createOptionsModel(true /* reset settings */);
+    QVERIFY(m_app.createOptionsModel(true /* reset settings */));
     QScopedPointer<const NetworkStyle> style(NetworkStyle::instantiate(Params().NetworkIDString()));
     m_app.setupPlatformStyle();
     m_app.createWindow(style.data());
diff --git a/src/qt/test/optiontests.cpp b/src/qt/test/optiontests.cpp
index 2ef9230c59..3bd0af19ad 100644
--- a/src/qt/test/optiontests.cpp
+++ b/src/qt/test/optiontests.cpp
@@ -13,6 +13,63 @@
 
 #include <univalue.h>
 
+#include <fstream>
+
+OptionTests::OptionTests(interfaces::Node& node) : m_node(node)
+{
+    gArgs.LockSettings([&](util::Settings& s) { m_previous_settings = s; });
+}
+
+void OptionTests::init()
+{
+    // reset args
+    gArgs.LockSettings([&](util::Settings& s) { s = m_previous_settings; });
+    gArgs.ClearPathCache();
+}
+
+void OptionTests::migrateSettings()
+{
+    // Set legacy QSettings and verify that they get cleared and migrated to
+    // settings.json
+    QSettings settings;
+    settings.setValue("nDatabaseCache", 600);
+    settings.setValue("nThreadsScriptVerif", 12);
+    settings.setValue("fUseUPnP", false);
+    settings.setValue("fListen", false);
+    settings.setValue("bPrune", true);
+    settings.setValue("nPruneSize", 3);
+    settings.setValue("fUseProxy", true);
+    settings.setValue("addrProxy", "proxy:123");
+    settings.setValue("fUseSeparateProxyTor", true);
+    settings.setValue("addrSeparateProxyTor", "onion:234");
+
+    settings.sync();
+
+    OptionsModel options{m_node};
+    bilingual_str error;
+    QVERIFY(options.Init(error));
+    QVERIFY(!settings.contains("nDatabaseCache"));
+    QVERIFY(!settings.contains("nThreadsScriptVerif"));
+    QVERIFY(!settings.contains("fUseUPnP"));
+    QVERIFY(!settings.contains("fListen"));
+    QVERIFY(!settings.contains("bPrune"));
+    QVERIFY(!settings.contains("nPruneSize"));
+    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<char>(file), std::istreambuf_iterator<char>()).c_str(), "{\n"
+        "    \"dbcache\": \"600\",\n"
+        "    \"listen\": false,\n"
+        "    \"onion\": \"onion:234\",\n"
+        "    \"par\": \"12\",\n"
+        "    \"proxy\": \"proxy:123\",\n"
+        "    \"prune\": \"2861\"\n"
+        "}\n");
+}
+
 void OptionTests::integerGetArgBug()
 {
     // Test regression https://github.com/bitcoin/bitcoin/issues/24457. Ensure
@@ -23,7 +80,8 @@ void OptionTests::integerGetArgBug()
         settings.rw_settings["prune"] = 3814;
     });
     gArgs.WriteSettingsFile();
-    OptionsModel{m_node};
+    bilingual_str error;
+    QVERIFY(OptionsModel{m_node}.Init(error));
     gArgs.LockSettings([&](util::Settings& settings) {
         settings.rw_settings.erase("prune");
     });
@@ -36,8 +94,6 @@ void OptionTests::parametersInteraction()
     // It was fixed via https://github.com/bitcoin-core/gui/pull/568.
     // With fListen=false in ~/.config/Bitcoin/Bitcoin-Qt.conf and all else left as default,
     // bitcoin-qt should set both -listen and -listenonion to false and start successfully.
-    gArgs.ClearPathCache();
-
     gArgs.LockSettings([&](util::Settings& s) {
         s.forced_settings.erase("listen");
         s.forced_settings.erase("listenonion");
@@ -48,7 +104,8 @@ void OptionTests::parametersInteraction()
     QSettings settings;
     settings.setValue("fListen", false);
 
-    OptionsModel{m_node};
+    bilingual_str error;
+    QVERIFY(OptionsModel{m_node}.Init(error));
 
     const bool expected{false};
 
diff --git a/src/qt/test/optiontests.h b/src/qt/test/optiontests.h
index 257a0b65be..286d785572 100644
--- a/src/qt/test/optiontests.h
+++ b/src/qt/test/optiontests.h
@@ -6,6 +6,8 @@
 #define BITCOIN_QT_TEST_OPTIONTESTS_H
 
 #include <qt/optionsmodel.h>
+#include <univalue.h>
+#include <util/settings.h>
 
 #include <QObject>
 
@@ -13,14 +15,17 @@ class OptionTests : public QObject
 {
     Q_OBJECT
 public:
-    explicit OptionTests(interfaces::Node& node) : m_node(node) {}
+    explicit OptionTests(interfaces::Node& node);
 
 private Q_SLOTS:
+    void init(); // called before each test function execution.
+    void migrateSettings();
     void integerGetArgBug();
     void parametersInteraction();
 
 private:
     interfaces::Node& m_node;
+    util::Settings m_previous_settings;
 };
 
 #endif // BITCOIN_QT_TEST_OPTIONTESTS_H
diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp
index aeedd92834..de23f51c92 100644
--- a/src/qt/test/test_main.cpp
+++ b/src/qt/test/test_main.cpp
@@ -58,9 +58,10 @@ int main(int argc, char* argv[])
     // regtest params.
     //
     // All tests must use their own testing setup (if needed).
-    {
+    fs::create_directories([] {
         BasicTestingSetup dummy{CBaseChainParams::REGTEST};
-    }
+        return gArgs.GetDataDirNet() / "blocks";
+    }());
 
     std::unique_ptr<interfaces::Init> init = interfaces::MakeGuiInit(argc, argv);
     gArgs.ForceSetArg("-listen", "0");
diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp
index bc06f0f23b..7671bfc739 100644
--- a/src/qt/test/wallettests.cpp
+++ b/src/qt/test/wallettests.cpp
@@ -185,6 +185,8 @@ void TestGUI(interfaces::Node& node)
     SendCoinsDialog sendCoinsDialog(platformStyle.get());
     TransactionView transactionView(platformStyle.get());
     OptionsModel optionsModel(node);
+    bilingual_str error;
+    QVERIFY(optionsModel.Init(error));
     ClientModel clientModel(node, &optionsModel);
     WalletContext& context = *node.walletLoader().context();
     AddWallet(context, wallet);