Merge #569: test: add regression test for #567

4d4dca4 test: add regression test for /issues/567 (Vasil Dimov)
3b82608 options: add a comment for -listenonion and dedup a long expression (Vasil Dimov)

Pull request description:

  Add a test that would fail, should #567 resurface.

  Also, add a comment and dedup a long expression.

ACKs for top commit:
  jarolrod:
    reACK 4d4dca4
  jonatack:
    ACK 4d4dca4
  hebasto:
    ACK 4d4dca4, tested with reverting changes from #568, and getting an expected test failure.
  shaavan:
    ACK 4d4dca4

Tree-SHA512: 59f069bdaa84586bb599e9372f89e4e66a3cafcbf58677fdf913d685c17dfa9c3d5b118829d81021a9a33b4fd8e46d4c7eb68c1dd902cf1c44a41b8e66e2967b

Author: hebasto

src/qt/optionsmodel.cpp

@@ -151,9 +151,26 @@ void OptionsModel::Init(bool resetSettings)
 
     if (!settings.contains("fListen"))
         settings.setValue("fListen", DEFAULT_LISTEN);
-    if (!gArgs.SoftSetBoolArg("-listen", settings.value("fListen").toBool())) {
+    const bool listen{settings.value("fListen").toBool()};
+    if (!gArgs.SoftSetBoolArg("-listen", listen)) {
         addOverriddenOption("-listen");
-    } else if (!settings.value("fListen").toBool()) {
+    } 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);
     }
 

src/qt/test/optiontests.cpp

@@ -2,6 +2,7 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <init.h>
 #include <qt/bitcoin.h>
 #include <qt/test/optiontests.h>
 #include <test/util/setup_common.h>
@@ -29,3 +30,39 @@ void OptionTests::optionTests()
     });
     gArgs.WriteSettingsFile();
 }
+
+void OptionTests::parametersInteraction()
+{
+    // Test that the bug https://github.com/bitcoin-core/gui/issues/567 does not resurface.
+    // 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");
+    });
+    QVERIFY(!gArgs.IsArgSet("-listen"));
+    QVERIFY(!gArgs.IsArgSet("-listenonion"));
+
+    QSettings settings;
+    settings.setValue("fListen", false);
+
+    OptionsModel{};
+
+    const bool expected{false};
+
+    QVERIFY(gArgs.IsArgSet("-listen"));
+    QCOMPARE(gArgs.GetBoolArg("-listen", !expected), expected);
+
+    QVERIFY(gArgs.IsArgSet("-listenonion"));
+    QCOMPARE(gArgs.GetBoolArg("-listenonion", !expected), expected);
+
+    QVERIFY(AppInitParameterInteraction(gArgs));
+
+    // cleanup
+    settings.remove("fListen");
+    QVERIFY(!settings.contains("fListen"));
+    gArgs.ClearPathCache();
+}

src/qt/test/optiontests.h

@@ -17,6 +17,7 @@ class OptionTests : public QObject
 
 private Q_SLOTS:
     void optionTests();
+    void parametersInteraction();
 
 private:
     interfaces::Node& m_node;