Merge #17750: util: change GetWarnings parameter to bool

7aab8d1 [style] Code style fixups in GetWarnings() (John Newbery)
492c6dc util: change GetWarnings parameter to bool (John Newbery)
869b631 [qt] remove unused parameter from getWarnings() (John Newbery)

Pull request description:

  `GetWarnings()` changes the format of the output warning string based on a passed-in string argument that can be set to "gui" or "statusbar".

  Change the argument to a bool:

  - there are only two types of behaviour, so a bool is a more natural argument type
  - changing the name to `verbose` does not set any expectations for the how the calling code will use the returned string (currently, `statusbar` is used for RPC warnings, not a status bar)
  - removes some error-handling code for when the passed-in string is not one of the two strings expected.

ACKs for top commit:
  laanwj:
    code review ACK 7aab8d1
  practicalswift:
    ACK 7aab8d1 -- diff looks correct :)
  MarcoFalke:
    ACK 7aab8d1 otherwise.
  promag:
    Code review ACK 7aab8d1.

Tree-SHA512: 75882c6e3e44aa9586411b803149b36ba487f4eb9cac3f5c8f07cd9f586870bba4488a51e674cf8147f05718534f482836e6a4e3f66e0d4ef6821900c7dfd04e

Author: MarcoFalke

src/interfaces/node.cpp

@@ -68,7 +68,7 @@ class NodeImpl : public Node
     std::string getNetwork() override { return Params().NetworkIDString(); }
     void initLogging() override { InitLogging(); }
     void initParameterInteraction() override { InitParameterInteraction(); }
-    std::string getWarnings(const std::string& type) override { return GetWarnings(type); }
+    std::string getWarnings() override { return GetWarnings(true); }
     uint32_t getLogCategories() override { return LogInstance().GetCategoryMask(); }
     bool baseInitialize() override
     {

src/interfaces/node.h

@@ -78,7 +78,7 @@ class Node
     virtual void initParameterInteraction() = 0;
 
     //! Get warnings.
-    virtual std::string getWarnings(const std::string& type) = 0;
+    virtual std::string getWarnings() = 0;
 
     // Get log flags.
     virtual uint32_t getLogCategories() = 0;

src/qt/bitcoin.cpp

@@ -135,7 +135,7 @@ BitcoinCore::BitcoinCore(interfaces::Node& node) :
 void BitcoinCore::handleRunawayException(const std::exception *e)
 {
     PrintExceptionContinue(e, "Runaway exception");
-    Q_EMIT runawayException(QString::fromStdString(m_node.getWarnings("gui")));
+    Q_EMIT runawayException(QString::fromStdString(m_node.getWarnings()));
 }
 
 void BitcoinCore::initialize()
@@ -589,10 +589,10 @@ int GuiMain(int argc, char* argv[])
         }
     } catch (const std::exception& e) {
         PrintExceptionContinue(&e, "Runaway exception");
-        app.handleRunawayException(QString::fromStdString(node->getWarnings("gui")));
+        app.handleRunawayException(QString::fromStdString(node->getWarnings()));
     } catch (...) {
         PrintExceptionContinue(nullptr, "Runaway exception");
-        app.handleRunawayException(QString::fromStdString(node->getWarnings("gui")));
+        app.handleRunawayException(QString::fromStdString(node->getWarnings()));
     }
     return rv;
 }

src/qt/clientmodel.cpp

@@ -134,7 +134,7 @@ enum BlockSource ClientModel::getBlockSource() const
 
 QString ClientModel::getStatusBarWarnings() const
 {
-    return QString::fromStdString(m_node.getWarnings("gui"));
+    return QString::fromStdString(m_node.getWarnings());
 }
 
 OptionsModel *ClientModel::getOptionsModel()

src/rpc/blockchain.cpp

@@ -1290,7 +1290,7 @@ UniValue getblockchaininfo(const JSONRPCRequest& request)
     BIP9SoftForkDescPushBack(softforks, "testdummy", consensusParams, Consensus::DEPLOYMENT_TESTDUMMY);
     obj.pushKV("softforks",             softforks);
 
-    obj.pushKV("warnings", GetWarnings("statusbar"));
+    obj.pushKV("warnings", GetWarnings(false));
     return obj;
 }
 

src/rpc/mining.cpp

@@ -254,7 +254,7 @@ static UniValue getmininginfo(const JSONRPCRequest& request)
     obj.pushKV("networkhashps",    getnetworkhashps(request));
     obj.pushKV("pooledtx",         (uint64_t)mempool.size());
     obj.pushKV("chain",            Params().NetworkIDString());
-    obj.pushKV("warnings",         GetWarnings("statusbar"));
+    obj.pushKV("warnings",         GetWarnings(false));
     return obj;
 }
 

src/rpc/net.cpp

@@ -522,7 +522,7 @@ static UniValue getnetworkinfo(const JSONRPCRequest& request)
         }
     }
     obj.pushKV("localaddresses", localAddresses);
-    obj.pushKV("warnings",       GetWarnings("statusbar"));
+    obj.pushKV("warnings",       GetWarnings(false));
     return obj;
 }
 

src/test/timedata_tests.cpp

@@ -65,7 +65,7 @@ BOOST_AUTO_TEST_CASE(addtimedata)
         MultiAddTimeData(1, DEFAULT_MAX_TIME_ADJUSTMENT + 1); //filter size 5
     }
 
-    BOOST_CHECK(GetWarnings("gui").find("clock is wrong") != std::string::npos);
+    BOOST_CHECK(GetWarnings(true).find("clock is wrong") != std::string::npos);
 
     // nTimeOffset is not changed if the median of offsets exceeds DEFAULT_MAX_TIME_ADJUSTMENT
     BOOST_CHECK_EQUAL(GetTimeOffset(), 0);

src/warnings.cpp

@@ -38,41 +38,34 @@ void SetfLargeWorkInvalidChainFound(bool flag)
     fLargeWorkInvalidChainFound = flag;
 }
 
-std::string GetWarnings(const std::string& strFor)
+std::string GetWarnings(bool verbose)
 {
-    std::string strStatusBar;
-    std::string strGUI;
-    const std::string uiAlertSeperator = "<hr />";
+    std::string warnings_concise;
+    std::string warnings_verbose;
+    const std::string warning_separator = "<hr />";
 
     LOCK(cs_warnings);
 
+    // Pre-release build warning
     if (!CLIENT_VERSION_IS_RELEASE) {
-        strStatusBar = "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications";
-        strGUI = _("This is a pre-release test build - use at your own risk - do not use for mining or merchant applications").translated;
+        warnings_concise = "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications";
+        warnings_verbose = _("This is a pre-release test build - use at your own risk - do not use for mining or merchant applications").translated;
     }
 
     // Misc warnings like out of disk space and clock is wrong
-    if (strMiscWarning != "")
-    {
-        strStatusBar = strMiscWarning;
-        strGUI += (strGUI.empty() ? "" : uiAlertSeperator) + strMiscWarning;
+    if (strMiscWarning != "") {
+        warnings_concise = strMiscWarning;
+        warnings_verbose += (warnings_verbose.empty() ? "" : warning_separator) + strMiscWarning;
     }
 
-    if (fLargeWorkForkFound)
-    {
-        strStatusBar = "Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues.";
-        strGUI += (strGUI.empty() ? "" : uiAlertSeperator) + _("Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues.").translated;
-    }
-    else if (fLargeWorkInvalidChainFound)
-    {
-        strStatusBar = "Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade.";
-        strGUI += (strGUI.empty() ? "" : uiAlertSeperator) + _("Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade.").translated;
+    if (fLargeWorkForkFound) {
+        warnings_concise = "Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues.";
+        warnings_verbose += (warnings_verbose.empty() ? "" : warning_separator) + _("Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues.").translated;
+    } else if (fLargeWorkInvalidChainFound) {
+        warnings_concise = "Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade.";
+        warnings_verbose += (warnings_verbose.empty() ? "" : warning_separator) + _("Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade.").translated;
     }
 
-    if (strFor == "gui")
-        return strGUI;
-    else if (strFor == "statusbar")
-        return strStatusBar;
-    assert(!"GetWarnings(): invalid parameter");
-    return "error";
+    if (verbose) return warnings_verbose;
+    else return warnings_concise;
 }

src/warnings.h

@@ -13,11 +13,11 @@ void SetfLargeWorkForkFound(bool flag);
 bool GetfLargeWorkForkFound();
 void SetfLargeWorkInvalidChainFound(bool flag);
 /** Format a string that describes several potential problems detected by the core.
- * @param[in] strFor can have the following values:
- * - "statusbar": get the most important warning
- * - "gui": get all warnings, translated (where possible) for GUI, separated by <hr />
- * @returns the warning string selected by strFor
+ * @param[in] verbose bool
+ * - if true, get all warnings, translated (where possible), separated by <hr />
+ * - if false, get the most important warning
+ * @returns the warning string
  */
-std::string GetWarnings(const std::string& strFor);
+std::string GetWarnings(bool verbose);
 
 #endif //  BITCOIN_WARNINGS_H