Merge #15894: Remove duplicated "Error: " prefix in logs

f724f31401b963c75bd64f5e2c5b9d9561a9a9dd Make AbortNode() aware of MSG_NOPREFIX flag (Hennadii Stepanov)
96fd4ee02f6c3be21cade729b95a85c60634b0f8 Add MSG_NOPREFIX flag for user messages (Hennadii Stepanov)
f0641f274ffe94fbe7cae43c07a9373013739df2 Prepend the error/warning prefix for GUI messages (Hennadii Stepanov)

Pull request description:

  The "Error" prefix/title is set already in the next functions:
  - `noui_ThreadSafeMessageBox()`https://github.com/bitcoin/bitcoin/blob/2068f089c8b7b90eb4557d3f67ea0f0ed2059a23/src/noui.cpp#L17
  - `ThreadSafeMessageBox()`https://github.com/bitcoin/bitcoin/blob/a720a983015c9ef8cc814c16a5b9ef6379695817/src/qt/bitcoingui.cpp#L1351

  Currently on master:
  ![Screenshot from 2019-04-25 22-08-54](https://user-images.githubusercontent.com/32963518/56763092-25ee8280-67aa-11e9-86c8-6a029dd9ab08.png)

  With this PR:
  ![Screenshot from 2019-04-25 22-26-36](https://user-images.githubusercontent.com/32963518/56763107-30108100-67aa-11e9-9021-683cbd7e2aaa.png)

ACKs for top commit:
  laanwj:
    concept and code-review ACK f724f31401b963c75bd64f5e2c5b9d9561a9a9dd

Tree-SHA512: 218a179b81cc2ac64239d833c02b4c4d4da9b976728a2dcd645966726c4c660b6f1fe43aa28f33d1cb566785a4329e7f93bf5a502bf202316db79d2ff5fce0f8

5 files changed, 52 insertions, 41 deletions

diff --git a/src/noui.cpp b/src/noui.cpp
index 0c18b0e231..caab9f326e 100644
--- a/src/noui.cpp
+++ b/src/noui.cpp
@@ -18,26 +18,30 @@ bool noui_ThreadSafeMessageBox(const std::string& message, const std::string& ca
 {
     bool fSecure = style & CClientUIInterface::SECURE;
     style &= ~CClientUIInterface::SECURE;
+    bool prefix = !(style & CClientUIInterface::MSG_NOPREFIX);
+    style &= ~CClientUIInterface::MSG_NOPREFIX;
 
     std::string strCaption;
-    // Check for usage of predefined caption
-    switch (style) {
-    case CClientUIInterface::MSG_ERROR:
-        strCaption += _("Error");
-        break;
-    case CClientUIInterface::MSG_WARNING:
-        strCaption += _("Warning");
-        break;
-    case CClientUIInterface::MSG_INFORMATION:
-        strCaption += _("Information");
-        break;
-    default:
-        strCaption += caption; // Use supplied caption (can be empty)
+    if (prefix) {
+        switch (style) {
+        case CClientUIInterface::MSG_ERROR:
+            strCaption = "Error: ";
+            break;
+        case CClientUIInterface::MSG_WARNING:
+            strCaption = "Warning: ";
+            break;
+        case CClientUIInterface::MSG_INFORMATION:
+            strCaption = "Information: ";
+            break;
+        default:
+            strCaption = caption + ": "; // Use supplied caption (can be empty)
+        }
     }
 
-    if (!fSecure)
-        LogPrintf("%s: %s\n", strCaption, message);
-    tfm::format(std::cerr, "%s: %s\n", strCaption.c_str(), message.c_str());
+    if (!fSecure) {
+        LogPrintf("%s%s\n", strCaption, message);
+    }
+    tfm::format(std::cerr, "%s%s\n", strCaption.c_str(), message.c_str());
     return false;
 }
 
diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp
index 8388e8d36e..babb2ce518 100644
--- a/src/qt/bitcoingui.cpp
+++ b/src/qt/bitcoingui.cpp
@@ -1039,49 +1039,51 @@ void BitcoinGUI::setNumBlocks(int count, const QDateTime& blockDate, double nVer
     progressBar->setToolTip(tooltip);
 }
 
-void BitcoinGUI::message(const QString &title, const QString &message, unsigned int style, bool *ret)
+void BitcoinGUI::message(const QString& title, QString message, unsigned int style, bool* ret)
 {
-    QString strTitle = tr("Bitcoin"); // default title
+    // Default title. On macOS, the window title is ignored (as required by the macOS Guidelines).
+    QString strTitle{PACKAGE_NAME};
     // Default to information icon
     int nMBoxIcon = QMessageBox::Information;
     int nNotifyIcon = Notificator::Information;
 
-    QString msgType;
+    bool prefix = !(style & CClientUIInterface::MSG_NOPREFIX);
+    style &= ~CClientUIInterface::MSG_NOPREFIX;
 
-    // Prefer supplied title over style based title
+    QString msgType;
     if (!title.isEmpty()) {
         msgType = title;
-    }
-    else {
+    } else {
         switch (style) {
         case CClientUIInterface::MSG_ERROR:
             msgType = tr("Error");
+            if (prefix) message = tr("Error: %1").arg(message);
             break;
         case CClientUIInterface::MSG_WARNING:
             msgType = tr("Warning");
+            if (prefix) message = tr("Warning: %1").arg(message);
             break;
         case CClientUIInterface::MSG_INFORMATION:
             msgType = tr("Information");
+            // No need to prepend the prefix here.
             break;
         default:
             break;
         }
     }
-    // Append title to "Bitcoin - "
-    if (!msgType.isEmpty())
+
+    if (!msgType.isEmpty()) {
         strTitle += " - " + msgType;
+    }
 
-    // Check for error/warning icon
     if (style & CClientUIInterface::ICON_ERROR) {
         nMBoxIcon = QMessageBox::Critical;
         nNotifyIcon = Notificator::Critical;
-    }
-    else if (style & CClientUIInterface::ICON_WARNING) {
+    } else if (style & CClientUIInterface::ICON_WARNING) {
         nMBoxIcon = QMessageBox::Warning;
         nNotifyIcon = Notificator::Warning;
     }
 
-    // Display message
     if (style & CClientUIInterface::MODAL) {
         // Check for buttons, use OK as default, if none was supplied
         QMessageBox::StandardButton buttons;
@@ -1094,9 +1096,9 @@ void BitcoinGUI::message(const QString &title, const QString &message, unsigned
         int r = mBox.exec();
         if (ret != nullptr)
             *ret = r == QMessageBox::Ok;
-    }
-    else
+    } else {
         notificator->notify(static_cast<Notificator::Class>(nNotifyIcon), strTitle, message);
+    }
 }
 
 void BitcoinGUI::changeEvent(QEvent *e)
diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h
index 608f10750a..46ced79007 100644
--- a/src/qt/bitcoingui.h
+++ b/src/qt/bitcoingui.h
@@ -220,7 +220,7 @@ public Q_SLOTS:
                             @see CClientUIInterface::MessageBoxFlags
        @param[in] ret       pointer to a bool that will be modified to whether Ok was clicked (modal only)
     */
-    void message(const QString &title, const QString &message, unsigned int style, bool *ret = nullptr);
+    void message(const QString& title, QString message, unsigned int style, bool* ret = nullptr);
 
 #ifdef ENABLE_WALLET
     void setCurrentWallet(WalletModel* wallet_model);
diff --git a/src/ui_interface.h b/src/ui_interface.h
index d408f6f889..5e0380dc45 100644
--- a/src/ui_interface.h
+++ b/src/ui_interface.h
@@ -69,6 +69,9 @@ public:
         /** Force blocking, modal message box dialog (not just OS notification) */
         MODAL               = 0x10000000U,
 
+        /** Do not prepend error/warning prefix */
+        MSG_NOPREFIX        = 0x20000000U,
+
         /** Do not print contents of message to debug log */
         SECURE              = 0x40000000U,
 
diff --git a/src/validation.cpp b/src/validation.cpp
index 2d2252c251..d39b78614c 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1374,20 +1374,22 @@ bool UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex* pindex)
 }
 
 /** Abort with a message */
-static bool AbortNode(const std::string& strMessage, const std::string& userMessage="")
+static bool AbortNode(const std::string& strMessage, const std::string& userMessage = "", unsigned int prefix = 0)
 {
     SetMiscWarning(strMessage);
     LogPrintf("*** %s\n", strMessage);
-    uiInterface.ThreadSafeMessageBox(
-        userMessage.empty() ? _("Error: A fatal internal error occurred, see debug.log for details") : userMessage,
-        "", CClientUIInterface::MSG_ERROR);
+    if (!userMessage.empty()) {
+        uiInterface.ThreadSafeMessageBox(userMessage, "", CClientUIInterface::MSG_ERROR | prefix);
+    } else {
+        uiInterface.ThreadSafeMessageBox(_("Error: A fatal internal error occurred, see debug.log for details"), "", CClientUIInterface::MSG_ERROR | CClientUIInterface::MSG_NOPREFIX);
+    }
     StartShutdown();
     return false;
 }
 
-static bool AbortNode(CValidationState& state, const std::string& strMessage, const std::string& userMessage="")
+static bool AbortNode(CValidationState& state, const std::string& strMessage, const std::string& userMessage = "", unsigned int prefix = 0)
 {
-    AbortNode(strMessage, userMessage);
+    AbortNode(strMessage, userMessage, prefix);
     return state.Error(strMessage);
 }
 
@@ -1998,7 +2000,7 @@ bool CChainState::FlushStateToDisk(
         if (fDoFullFlush || fPeriodicWrite) {
             // Depend on nMinDiskSpace to ensure we can write block index
             if (!CheckDiskSpace(GetBlocksDir())) {
-                return AbortNode(state, "Disk space is low!", _("Error: Disk space is low!"));
+                return AbortNode(state, "Disk space is too low!", _("Error: Disk space is too low!"), CClientUIInterface::MSG_NOPREFIX);
             }
             // First make sure all block and undo data is flushed to disk.
             FlushBlockFile();
@@ -2033,7 +2035,7 @@ bool CChainState::FlushStateToDisk(
             // an overestimation, as most will delete an existing entry or
             // overwrite one. Still, use a conservative safety factor of 2.
             if (!CheckDiskSpace(GetDataDir(), 48 * 2 * 2 * pcoinsTip->GetCacheSize())) {
-                return AbortNode(state, "Disk space is low!", _("Error: Disk space is low!"));
+                return AbortNode(state, "Disk space is too low!", _("Error: Disk space is too low!"), CClientUIInterface::MSG_NOPREFIX);
             }
             // Flush the chainstate (which may refer to block index entries).
             if (!pcoinsTip->Flush())
@@ -2899,7 +2901,7 @@ static bool FindBlockPos(FlatFilePos &pos, unsigned int nAddSize, unsigned int n
         bool out_of_space;
         size_t bytes_allocated = BlockFileSeq().Allocate(pos, nAddSize, out_of_space);
         if (out_of_space) {
-            return AbortNode("Disk space is low!", _("Error: Disk space is low!"));
+            return AbortNode("Disk space is too low!", _("Error: Disk space is too low!"), CClientUIInterface::MSG_NOPREFIX);
         }
         if (bytes_allocated != 0 && fPruneMode) {
             fCheckForPruning = true;
@@ -2923,7 +2925,7 @@ static bool FindUndoPos(CValidationState &state, int nFile, FlatFilePos &pos, un
     bool out_of_space;
     size_t bytes_allocated = UndoFileSeq().Allocate(pos, nAddSize, out_of_space);
     if (out_of_space) {
-        return AbortNode(state, "Disk space is low!", _("Error: Disk space is low!"));
+        return AbortNode(state, "Disk space is too low!", _("Error: Disk space is too low!"), CClientUIInterface::MSG_NOPREFIX);
     }
     if (bytes_allocated != 0 && fPruneMode) {
         fCheckForPruning = true;