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, 52 insertions, 0 deletions

diff --git a/src/test/node_warnings_tests.cpp b/src/test/node_warnings_tests.cpp
new file mode 100644
index 0000000000..2bcc2c95ed
--- /dev/null
+++ b/src/test/node_warnings_tests.cpp
@@ -0,0 +1,52 @@
+// Copyright (c) 2024-present The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+//
+
+#include <node/warnings.h>
+#include <util/translation.h>
+
+#include <test/util/setup_common.h>
+
+#include <boost/test/unit_test.hpp>
+
+BOOST_FIXTURE_TEST_SUITE(node_warnings_tests, BasicTestingSetup)
+
+BOOST_AUTO_TEST_CASE(warnings)
+{
+    node::Warnings warnings;
+    // On pre-release builds, a warning is generated automatically
+    warnings.Unset(node::Warning::PRE_RELEASE_TEST_BUILD);
+
+    // For these tests, we don't care what the exact warnings are, so
+    // just refer to them as warning_1 and warning_2
+    const auto warning_1{node::Warning::CLOCK_OUT_OF_SYNC};
+    const auto warning_2{node::Warning::FATAL_INTERNAL_ERROR};
+
+    // Ensure we start without any warnings
+    BOOST_CHECK(warnings.GetMessages().size() == 0);
+    // Add two warnings
+    BOOST_CHECK(warnings.Set(warning_1, _("warning 1")));
+    BOOST_CHECK(warnings.Set(warning_2, _("warning 2")));
+    // Unset the second one
+    BOOST_CHECK(warnings.Unset(warning_2));
+    // Since it's already been unset, this should return false
+    BOOST_CHECK(!warnings.Unset(warning_2));
+    // We should now be able to set w2 again
+    BOOST_CHECK(warnings.Set(warning_2, _("warning 2 - revision 1")));
+    // Setting w2 again should return false since it's already set
+    BOOST_CHECK(!warnings.Set(warning_2, _("warning 2 - revision 2")));
+
+    // Verify messages are correct
+    const auto messages{warnings.GetMessages()};
+    BOOST_CHECK(messages.size() == 2);
+    BOOST_CHECK(messages[0].original == "warning 1");
+    BOOST_CHECK(messages[1].original == "warning 2 - revision 1");
+
+    // Clearing all warnings should also clear all messages
+    BOOST_CHECK(warnings.Unset(warning_1));
+    BOOST_CHECK(warnings.Unset(warning_2));
+    BOOST_CHECK(warnings.GetMessages().size() == 0);
+}
+
+BOOST_AUTO_TEST_SUITE_END()