Merge bitcoin/bitcoin#30058: Encapsulate warnings in generalized node::Warnings and remove globals

260f8da71a35232d859d7705861fc1a88bfbbe81 refactor: remove warnings globals (stickies-v)
9c4b0b7ce459765fa1a63b410c3423b90f0d2a5f node: update uiInterface whenever warnings updated (stickies-v)
b071ad9770b7ae7fc718dcbfdc8f62dffbf6cfee introduce and use the generalized `node::Warnings` interface (stickies-v)
20e616f86444d00712ac7eb840666e2b0378af4a move-only: move warnings from common to node (stickies-v)
bed29c481aebeb2b0160450c63c03cc68fb89bc6 refactor: remove unnecessary AppendWarning helper function (stickies-v)

Pull request description:

  This PR:
  - moves warnings from common to the node library and into the node namespace (as suggested in https://github.com/bitcoin/bitcoin/pull/29845#discussion_r1570069541)
  - generalizes the warnings interface to `Warnings::Set()` and `Warnings::Unset()` methods, instead of having a separate function and globals for each warning. As a result, this simplifies the `kernel::Notifications` interface.
  - removes warnings.cpp from the kernel library
  - removes warning globals
  - adds testing for the warning logic

  Behaviour change introduced:
  - the `-alertnotify` command is executed for all `KernelNotifications::warningSet` calls, which now also covers the `LARGE_WORK_INVALID_CHAIN` warning
  - the GUI is updated automatically whenever a warning is (un)set, covering some code paths where it previously wouldn't be, e.g. when `node::AbortNode()` is called, or for the `LARGE_WORK_INVALID_CHAIN` warning

  Some discussion points:
  - ~is `const std::string& id` the best way to refer to warnings? Enums are an obvious alternative, but since we need to define warnings across libraries, strings seem like a straightforward solution.~ _edit: updated approach to use `node::Warning` and `kernel::Warning` enums._

ACKs for top commit:
  achow101:
    ACK 260f8da71a35232d859d7705861fc1a88bfbbe81
  ryanofsky:
    Code review ACK 260f8da71a35232d859d7705861fc1a88bfbbe81. Only change since last review was rebasing
  TheCharlatan:
    Re-ACK 260f8da71a35232d859d7705861fc1a88bfbbe81

Tree-SHA512: a3fcedaee0d3ad64e9c111aeb30665162f98e0e72acd6a70b76ff2ddf4f0a34da4f97ce353c322a1668ca6ee4d8a81cc6e6d170c5bbeb7a43cffdaf66646b588

1 files changed, 12 insertions, 15 deletions

diff --git a/src/validation.cpp b/src/validation.cpp
index 6db723d22c..cf9bfe0e2b 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -27,14 +27,15 @@
 #include <kernel/mempool_entry.h>
 #include <kernel/messagestartchars.h>
 #include <kernel/notifications_interface.h>
+#include <kernel/warning.h>
 #include <logging.h>
 #include <logging/timer.h>
 #include <node/blockstorage.h>
 #include <node/utxo_snapshot.h>
-#include <policy/v3_policy.h>
 #include <policy/policy.h>
 #include <policy/rbf.h>
 #include <policy/settings.h>
+#include <policy/v3_policy.h>
 #include <pow.h>
 #include <primitives/block.h>
 #include <primitives/transaction.h>
@@ -57,11 +58,11 @@
 #include <util/result.h>
 #include <util/signalinterrupt.h>
 #include <util/strencodings.h>
+#include <util/string.h>
 #include <util/time.h>
 #include <util/trace.h>
 #include <util/translation.h>
 #include <validationinterface.h>
-#include <warnings.h>
 
 #include <algorithm>
 #include <cassert>
@@ -1921,9 +1922,11 @@ void Chainstate::CheckForkWarningConditions()
 
     if (m_chainman.m_best_invalid && m_chainman.m_best_invalid->nChainWork > m_chain.Tip()->nChainWork + (GetBlockProof(*m_chain.Tip()) * 6)) {
         LogPrintf("%s: Warning: Found invalid chain at least ~6 blocks longer than our best chain.\nChain state database corruption likely.\n", __func__);
-        SetfLargeWorkInvalidChainFound(true);
+        m_chainman.GetNotifications().warningSet(
+            kernel::Warning::LARGE_WORK_INVALID_CHAIN,
+            _("Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade."));
     } else {
-        SetfLargeWorkInvalidChainFound(false);
+        m_chainman.GetNotifications().warningUnset(kernel::Warning::LARGE_WORK_INVALID_CHAIN);
     }
 }
 
@@ -2847,13 +2850,6 @@ void Chainstate::PruneAndFlush()
     }
 }
 
-/** Private helper function that concatenates warning messages. */
-static void AppendWarning(bilingual_str& res, const bilingual_str& warn)
-{
-    if (!res.empty()) res += Untranslated(", ");
-    res += warn;
-}
-
 static void UpdateTipLog(
     const CCoinsViewCache& coins_tip,
     const CBlockIndex* tip,
@@ -2904,7 +2900,7 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew)
         g_best_block_cv.notify_all();
     }
 
-    bilingual_str warning_messages;
+    std::vector<bilingual_str> warning_messages;
     if (!m_chainman.IsInitialBlockDownload()) {
         const CBlockIndex* pindex = pindexNew;
         for (int bit = 0; bit < VERSIONBITS_NUM_BITS; bit++) {
@@ -2913,14 +2909,15 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew)
             if (state == ThresholdState::ACTIVE || state == ThresholdState::LOCKED_IN) {
                 const bilingual_str warning = strprintf(_("Unknown new rules activated (versionbit %i)"), bit);
                 if (state == ThresholdState::ACTIVE) {
-                    m_chainman.GetNotifications().warning(warning);
+                    m_chainman.GetNotifications().warningSet(kernel::Warning::UNKNOWN_NEW_RULES_ACTIVATED, warning);
                 } else {
-                    AppendWarning(warning_messages, warning);
+                    warning_messages.push_back(warning);
                 }
             }
         }
     }
-    UpdateTipLog(coins_tip, pindexNew, params, __func__, "", warning_messages.original);
+    UpdateTipLog(coins_tip, pindexNew, params, __func__, "",
+                 util::Join(warning_messages, Untranslated(", ")).original);
 }
 
 /** Disconnect m_chain's tip.