diff options
author | Ava Chow <github@achow101.com> | 2024-06-17 17:09:18 -0400 |
---|---|---|
committer | Ava Chow <github@achow101.com> | 2024-06-17 17:09:18 -0400 |
commit | ddf2ebd4659e926c5c6ee6726511b0b88bf7c2e0 (patch) | |
tree | b58d747b2676dff61342636a0faf742847981214 | |
parent | d97ddbe797f5b8b3bca0ee71b692e542b8990195 (diff) | |
parent | 260f8da71a35232d859d7705861fc1a88bfbbe81 (diff) |
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
39 files changed, 361 insertions, 186 deletions
diff --git a/doc/release-notes-30058.md b/doc/release-notes-30058.md new file mode 100644 index 0000000000..47e7ae704e --- /dev/null +++ b/doc/release-notes-30058.md @@ -0,0 +1,7 @@ +- When running with -alertnotify, an alert can now be raised multiple +times instead of just once. Previously, it was only raised when unknown +new consensus rules were activated, whereas the scope has now been +increased to include all kernel warnings. Specifically, alerts will now +also be raised when an invalid chain with a large amount of work has +been detected. Additional warnings may be added in the future. +(#30058) diff --git a/src/Makefile.am b/src/Makefile.am index 2562c4cc65..4a1973aa87 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -196,6 +196,7 @@ BITCOIN_CORE_H = \ kernel/messagestartchars.h \ kernel/notifications_interface.h \ kernel/validation_cache_sizes.h \ + kernel/warning.h \ key.h \ key_io.h \ logging.h \ @@ -239,6 +240,7 @@ BITCOIN_CORE_H = \ node/types.h \ node/utxo_snapshot.h \ node/validation_cache_args.h \ + node/warnings.h \ noui.h \ outputtype.h \ policy/v3_policy.h \ @@ -367,7 +369,6 @@ BITCOIN_CORE_H = \ wallet/wallettool.h \ wallet/walletutil.h \ walletinitinterface.h \ - warnings.h \ zmq/zmqabstractnotifier.h \ zmq/zmqnotificationinterface.h \ zmq/zmqpublishnotifier.h \ @@ -444,6 +445,7 @@ libbitcoin_node_a_SOURCES = \ node/txreconciliation.cpp \ node/utxo_snapshot.cpp \ node/validation_cache_args.cpp \ + node/warnings.cpp \ noui.cpp \ policy/v3_policy.cpp \ policy/fees.cpp \ @@ -721,7 +723,6 @@ libbitcoin_common_a_SOURCES = \ script/sign.cpp \ script/signingprovider.cpp \ script/solver.cpp \ - warnings.cpp \ $(BITCOIN_CORE_H) # @@ -996,8 +997,7 @@ libbitcoinkernel_la_SOURCES = \ util/tokenpipe.cpp \ validation.cpp \ validationinterface.cpp \ - versionbits.cpp \ - warnings.cpp + versionbits.cpp # Required for obj/build.h to be generated first. # More details: https://www.gnu.org/software/automake/manual/html_node/Built-Sources-Example.html diff --git a/src/Makefile.test.include b/src/Makefile.test.include index fa7cadb726..633d0776f5 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -118,6 +118,7 @@ BITCOIN_TESTS =\ test/net_peer_eviction_tests.cpp \ test/net_tests.cpp \ test/netbase_tests.cpp \ + test/node_warnings_tests.cpp \ test/orphanage_tests.cpp \ test/peerman_tests.cpp \ test/pmt_tests.cpp \ diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index ca4434a882..ecbdcd48bb 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -16,6 +16,7 @@ #include <kernel/checks.h> #include <kernel/context.h> #include <kernel/validation_cache_sizes.h> +#include <kernel/warning.h> #include <consensus/validation.h> #include <core_io.h> @@ -28,6 +29,7 @@ #include <util/fs.h> #include <util/signalinterrupt.h> #include <util/task_runner.h> +#include <util/translation.h> #include <validation.h> #include <validationinterface.h> @@ -86,9 +88,13 @@ int main(int argc, char* argv[]) { std::cout << "Progress: " << title.original << ", " << progress_percent << ", " << resume_possible << std::endl; } - void warning(const bilingual_str& warning) override + void warningSet(kernel::Warning id, const bilingual_str& message) override { - std::cout << "Warning: " << warning.original << std::endl; + std::cout << "Warning " << static_cast<int>(id) << " set: " << message.original << std::endl; + } + void warningUnset(kernel::Warning id) override + { + std::cout << "Warning " << static_cast<int>(id) << " unset" << std::endl; } void flushError(const bilingual_str& message) override { diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 0b89aa42af..a09bb5c9da 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -17,6 +17,7 @@ #include <kernel/context.h> #include <node/context.h> #include <node/interface_ui.h> +#include <node/warnings.h> #include <noui.h> #include <util/check.h> #include <util/exception.h> @@ -181,6 +182,8 @@ static bool AppInit(NodeContext& node) return false; } + node.warnings = std::make_unique<node::Warnings>(); + node.kernel = std::make_unique<kernel::Context>(); node.ecc_context = std::make_unique<ECC_Context>(); if (!AppInitSanityChecks(*node.kernel)) diff --git a/src/index/base.cpp b/src/index/base.cpp index e66c89f9e4..955d7b67c9 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -17,7 +17,6 @@ #include <util/thread.h> #include <util/translation.h> #include <validation.h> // For g_chainman -#include <warnings.h> #include <string> #include <utility> @@ -31,7 +30,7 @@ template <typename... Args> void BaseIndex::FatalErrorf(const char* fmt, const Args&... args) { auto message = tfm::format(fmt, args...); - node::AbortNode(m_chain->context()->shutdown, m_chain->context()->exit_status, Untranslated(message)); + node::AbortNode(m_chain->context()->shutdown, m_chain->context()->exit_status, Untranslated(message), m_chain->context()->warnings.get()); } CBlockLocator GetLocator(interfaces::Chain& chain, const uint256& block_hash) diff --git a/src/init.cpp b/src/init.cpp index b7d4c72b0a..5bb82dc320 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1486,7 +1486,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // ********************************************************* Step 7: load block chain - node.notifications = std::make_unique<KernelNotifications>(*Assert(node.shutdown), node.exit_status); + node.notifications = std::make_unique<KernelNotifications>(*Assert(node.shutdown), node.exit_status, *Assert(node.warnings)); ReadNotificationArgs(args, *node.notifications); ChainstateManager::Options chainman_opts{ .chainparams = chainparams, @@ -1639,7 +1639,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) assert(!node.peerman); node.peerman = PeerManager::make(*node.connman, *node.addrman, node.banman.get(), chainman, - *node.mempool, peerman_opts); + *node.mempool, *node.warnings, + peerman_opts); validation_signals.RegisterValidationInterface(node.peerman.get()); // ********************************************************* Step 8: start indexers diff --git a/src/kernel/notifications_interface.h b/src/kernel/notifications_interface.h index 7283a88e86..8e090dd7db 100644 --- a/src/kernel/notifications_interface.h +++ b/src/kernel/notifications_interface.h @@ -16,6 +16,8 @@ namespace kernel { //! Result type for use with std::variant to indicate that an operation should be interrupted. struct Interrupted{}; +enum class Warning; + //! Simple result type for functions that need to propagate an interrupt status and don't have other return values. using InterruptResult = std::variant<std::monostate, Interrupted>; @@ -38,7 +40,8 @@ public: [[nodiscard]] virtual InterruptResult blockTip(SynchronizationState state, CBlockIndex& index) { return {}; } virtual void headerTip(SynchronizationState state, int64_t height, int64_t timestamp, bool presync) {} virtual void progress(const bilingual_str& title, int progress_percent, bool resume_possible) {} - virtual void warning(const bilingual_str& warning) {} + virtual void warningSet(Warning id, const bilingual_str& message) {} + virtual void warningUnset(Warning id) {} //! The flush error notification is sent to notify the user that an error //! occurred while flushing block data to disk. Kernel code may ignore flush diff --git a/src/kernel/warning.h b/src/kernel/warning.h new file mode 100644 index 0000000000..453f36c552 --- /dev/null +++ b/src/kernel/warning.h @@ -0,0 +1,14 @@ +// 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. + +#ifndef BITCOIN_KERNEL_WARNING_H +#define BITCOIN_KERNEL_WARNING_H + +namespace kernel { +enum class Warning { + UNKNOWN_NEW_RULES_ACTIVATED, + LARGE_WORK_INVALID_CHAIN, +}; +} // namespace kernel +#endif // BITCOIN_KERNEL_WARNING_H diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6374cb52c1..795b798f9c 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -25,6 +25,7 @@ #include <node/blockstorage.h> #include <node/timeoffsets.h> #include <node/txreconciliation.h> +#include <node/warnings.h> #include <policy/fees.h> #include <policy/policy.h> #include <policy/settings.h> @@ -489,7 +490,7 @@ class PeerManagerImpl final : public PeerManager public: PeerManagerImpl(CConnman& connman, AddrMan& addrman, BanMan* banman, ChainstateManager& chainman, - CTxMemPool& pool, Options opts); + CTxMemPool& pool, node::Warnings& warnings, Options opts); /** Overridden from CValidationInterface. */ void BlockConnected(ChainstateRole role, const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexConnected) override @@ -790,7 +791,8 @@ private: /** Next time to check for stale tip */ std::chrono::seconds m_stale_tip_check_time GUARDED_BY(cs_main){0s}; - TimeOffsets m_outbound_time_offsets; + node::Warnings& m_warnings; + TimeOffsets m_outbound_time_offsets{m_warnings}; const Options m_opts; @@ -2042,14 +2044,14 @@ std::optional<std::string> PeerManagerImpl::FetchBlock(NodeId peer_id, const CBl std::unique_ptr<PeerManager> PeerManager::make(CConnman& connman, AddrMan& addrman, BanMan* banman, ChainstateManager& chainman, - CTxMemPool& pool, Options opts) + CTxMemPool& pool, node::Warnings& warnings, Options opts) { - return std::make_unique<PeerManagerImpl>(connman, addrman, banman, chainman, pool, opts); + return std::make_unique<PeerManagerImpl>(connman, addrman, banman, chainman, pool, warnings, opts); } PeerManagerImpl::PeerManagerImpl(CConnman& connman, AddrMan& addrman, BanMan* banman, ChainstateManager& chainman, - CTxMemPool& pool, Options opts) + CTxMemPool& pool, node::Warnings& warnings, Options opts) : m_rng{opts.deterministic_rng}, m_fee_filter_rounder{CFeeRate{DEFAULT_MIN_RELAY_TX_FEE}, m_rng}, m_chainparams(chainman.GetParams()), @@ -2058,6 +2060,7 @@ PeerManagerImpl::PeerManagerImpl(CConnman& connman, AddrMan& addrman, m_banman(banman), m_chainman(chainman), m_mempool(pool), + m_warnings{warnings}, m_opts{opts} { // While Erlay support is incomplete, it must be enabled explicitly via -txreconciliation. diff --git a/src/net_processing.h b/src/net_processing.h index 85e399d948..e6430d390e 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -16,6 +16,10 @@ class CChainParams; class CTxMemPool; class ChainstateManager; +namespace node { +class Warnings; +} // namespace node + /** Whether transaction reconciliation protocol should be enabled by default. */ static constexpr bool DEFAULT_TXRECONCILIATION_ENABLE{false}; /** Default for -maxorphantx, maximum number of orphan transactions kept in memory */ @@ -73,7 +77,7 @@ public: static std::unique_ptr<PeerManager> make(CConnman& connman, AddrMan& addrman, BanMan* banman, ChainstateManager& chainman, - CTxMemPool& pool, Options opts); + CTxMemPool& pool, node::Warnings& warnings, Options opts); virtual ~PeerManager() { } /** diff --git a/src/node/abort.cpp b/src/node/abort.cpp index b727608384..8a17c41fd2 100644 --- a/src/node/abort.cpp +++ b/src/node/abort.cpp @@ -6,19 +6,18 @@ #include <logging.h> #include <node/interface_ui.h> +#include <node/warnings.h> #include <util/signalinterrupt.h> #include <util/translation.h> -#include <warnings.h> #include <atomic> #include <cstdlib> -#include <string> namespace node { -void AbortNode(util::SignalInterrupt* shutdown, std::atomic<int>& exit_status, const bilingual_str& message) +void AbortNode(util::SignalInterrupt* shutdown, std::atomic<int>& exit_status, const bilingual_str& message, node::Warnings* warnings) { - SetMiscWarning(message); + if (warnings) warnings->Set(Warning::FATAL_INTERNAL_ERROR, message); InitError(_("A fatal internal error occurred, see debug.log for details: ") + message); exit_status.store(EXIT_FAILURE); if (shutdown && !(*shutdown)()) { diff --git a/src/node/abort.h b/src/node/abort.h index 1092279142..c881af4634 100644 --- a/src/node/abort.h +++ b/src/node/abort.h @@ -14,7 +14,8 @@ class SignalInterrupt; } // namespace util namespace node { -void AbortNode(util::SignalInterrupt* shutdown, std::atomic<int>& exit_status, const bilingual_str& message); +class Warnings; +void AbortNode(util::SignalInterrupt* shutdown, std::atomic<int>& exit_status, const bilingual_str& message, node::Warnings* warnings); } // namespace node #endif // BITCOIN_NODE_ABORT_H diff --git a/src/node/context.cpp b/src/node/context.cpp index e32d21b383..da05fde6ee 100644 --- a/src/node/context.cpp +++ b/src/node/context.cpp @@ -13,6 +13,7 @@ #include <net_processing.h> #include <netgroup.h> #include <node/kernel_notifications.h> +#include <node/warnings.h> #include <policy/fees.h> #include <scheduler.h> #include <txmempool.h> diff --git a/src/node/context.h b/src/node/context.h index a7d92989dd..77838ea99b 100644 --- a/src/node/context.h +++ b/src/node/context.h @@ -39,6 +39,7 @@ class SignalInterrupt; namespace node { class KernelNotifications; +class Warnings; //! NodeContext struct containing references to chain state and connection //! state. @@ -81,6 +82,8 @@ struct NodeContext { //! Issues calls about blocks and transactions std::unique_ptr<ValidationSignals> validation_signals; std::atomic<int> exit_status{EXIT_SUCCESS}; + //! Manages all the node warnings + std::unique_ptr<node::Warnings> warnings; //! Declare default constructor and destructor that are not inline, so code //! instantiating the NodeContext struct doesn't need to #include class diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 9e10f4ca59..2b36f4ceae 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -32,6 +32,7 @@ #include <node/mini_miner.h> #include <node/transaction.h> #include <node/types.h> +#include <node/warnings.h> #include <policy/feerate.h> #include <policy/fees.h> #include <policy/policy.h> @@ -53,7 +54,6 @@ #include <util/translation.h> #include <validation.h> #include <validationinterface.h> -#include <warnings.h> #include <config/bitcoin-config.h> // IWYU pragma: keep @@ -93,7 +93,7 @@ public: explicit NodeImpl(NodeContext& context) { setContext(&context); } void initLogging() override { InitLogging(args()); } void initParameterInteraction() override { InitParameterInteraction(args()); } - bilingual_str getWarnings() override { return Join(GetWarnings(), Untranslated("<hr />")); } + bilingual_str getWarnings() override { return Join(Assert(m_context->warnings)->GetMessages(), Untranslated("<hr />")); } int getExitStatus() override { return Assert(m_context)->exit_status.load(); } uint32_t getLogCategories() override { return LogInstance().GetCategoryMask(); } bool baseInitialize() override @@ -101,6 +101,7 @@ public: if (!AppInitBasicSetup(args(), Assert(context())->exit_status)) return false; if (!AppInitParameterInteraction(args())) return false; + m_context->warnings = std::make_unique<node::Warnings>(); m_context->kernel = std::make_unique<kernel::Context>(); m_context->ecc_context = std::make_unique<ECC_Context>(); if (!AppInitSanityChecks(*m_context->kernel)) return false; diff --git a/src/node/kernel_notifications.cpp b/src/node/kernel_notifications.cpp index 44828b8adb..9894052a3a 100644 --- a/src/node/kernel_notifications.cpp +++ b/src/node/kernel_notifications.cpp @@ -10,15 +10,16 @@ #include <common/args.h> #include <common/system.h> #include <kernel/context.h> +#include <kernel/warning.h> #include <logging.h> #include <node/abort.h> #include <node/interface_ui.h> +#include <node/warnings.h> #include <util/check.h> #include <util/signalinterrupt.h> #include <util/strencodings.h> #include <util/string.h> #include <util/translation.h> -#include <warnings.h> #include <cstdint> #include <string> @@ -28,7 +29,6 @@ using util::ReplaceAll; static void AlertNotify(const std::string& strMessage) { - uiInterface.NotifyAlertChanged(); #if HAVE_SYSTEM std::string strCmd = gArgs.GetArg("-alertnotify", ""); if (strCmd.empty()) return; @@ -46,16 +46,6 @@ static void AlertNotify(const std::string& strMessage) #endif } -static void DoWarning(const bilingual_str& warning) -{ - static bool fWarned = false; - SetMiscWarning(warning); - if (!fWarned) { - AlertNotify(warning.original); - fWarned = true; - } -} - namespace node { kernel::InterruptResult KernelNotifications::blockTip(SynchronizationState state, CBlockIndex& index) @@ -80,20 +70,27 @@ void KernelNotifications::progress(const bilingual_str& title, int progress_perc uiInterface.ShowProgress(title.translated, progress_percent, resume_possible); } -void KernelNotifications::warning(const bilingual_str& warning) +void KernelNotifications::warningSet(kernel::Warning id, const bilingual_str& message) +{ + if (m_warnings.Set(id, message)) { + AlertNotify(message.original); + } +} + +void KernelNotifications::warningUnset(kernel::Warning id) { - DoWarning(warning); + m_warnings.Unset(id); } void KernelNotifications::flushError(const bilingual_str& message) { - AbortNode(&m_shutdown, m_exit_status, message); + AbortNode(&m_shutdown, m_exit_status, message, &m_warnings); } void KernelNotifications::fatalError(const bilingual_str& message) { node::AbortNode(m_shutdown_on_fatal_error ? &m_shutdown : nullptr, - m_exit_status, message); + m_exit_status, message, &m_warnings); } void ReadNotificationArgs(const ArgsManager& args, KernelNotifications& notifications) diff --git a/src/node/kernel_notifications.h b/src/node/kernel_notifications.h index f4d97a0fff..e37f4d4e1e 100644 --- a/src/node/kernel_notifications.h +++ b/src/node/kernel_notifications.h @@ -15,18 +15,24 @@ class CBlockIndex; enum class SynchronizationState; struct bilingual_str; +namespace kernel { +enum class Warning; +} // namespace kernel + namespace util { class SignalInterrupt; } // namespace util namespace node { +class Warnings; static constexpr int DEFAULT_STOPATHEIGHT{0}; class KernelNotifications : public kernel::Notifications { public: - KernelNotifications(util::SignalInterrupt& shutdown, std::atomic<int>& exit_status) : m_shutdown(shutdown), m_exit_status{exit_status} {} + KernelNotifications(util::SignalInterrupt& shutdown, std::atomic<int>& exit_status, node::Warnings& warnings) + : m_shutdown(shutdown), m_exit_status{exit_status}, m_warnings{warnings} {} [[nodiscard]] kernel::InterruptResult blockTip(SynchronizationState state, CBlockIndex& index) override; @@ -34,7 +40,9 @@ public: void progress(const bilingual_str& title, int progress_percent, bool resume_possible) override; - void warning(const bilingual_str& warning) override; + void warningSet(kernel::Warning id, const bilingual_str& message) override; + + void warningUnset(kernel::Warning id) override; void flushError(const bilingual_str& message) override; @@ -47,6 +55,7 @@ public: private: util::SignalInterrupt& m_shutdown; std::atomic<int>& m_exit_status; + node::Warnings& m_warnings; }; void ReadNotificationArgs(const ArgsManager& args, KernelNotifications& notifications); diff --git a/src/node/timeoffsets.cpp b/src/node/timeoffsets.cpp index 62f527be8a..002c00d245 100644 --- a/src/node/timeoffsets.cpp +++ b/src/node/timeoffsets.cpp @@ -3,13 +3,12 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <logging.h> -#include <node/interface_ui.h> #include <node/timeoffsets.h> +#include <node/warnings.h> #include <sync.h> #include <tinyformat.h> #include <util/time.h> #include <util/translation.h> -#include <warnings.h> #include <algorithm> #include <chrono> @@ -49,8 +48,7 @@ bool TimeOffsets::WarnIfOutOfSync() const // when median == std::numeric_limits<int64_t>::min(), calling std::chrono::abs is UB auto median{std::max(Median(), std::chrono::seconds(std::numeric_limits<int64_t>::min() + 1))}; if (std::chrono::abs(median) <= WARN_THRESHOLD) { - SetMedianTimeOffsetWarning(std::nullopt); - uiInterface.NotifyAlertChanged(); + m_warnings.Unset(node::Warning::CLOCK_OUT_OF_SYNC); return false; } @@ -63,7 +61,6 @@ bool TimeOffsets::WarnIfOutOfSync() const "RPC methods to get more info." ), Ticks<std::chrono::minutes>(WARN_THRESHOLD))}; LogWarning("%s\n", msg.original); - SetMedianTimeOffsetWarning(msg); - uiInterface.NotifyAlertChanged(); + m_warnings.Set(node::Warning::CLOCK_OUT_OF_SYNC, msg); return true; } diff --git a/src/node/timeoffsets.h b/src/node/timeoffsets.h index 2b12584e12..eba706ac1e 100644 --- a/src/node/timeoffsets.h +++ b/src/node/timeoffsets.h @@ -11,8 +11,16 @@ #include <cstddef> #include <deque> +namespace node { +class Warnings; +} // namespace node + class TimeOffsets { +public: + TimeOffsets(node::Warnings& warnings) : m_warnings{warnings} {} + +private: //! Maximum number of timeoffsets stored. static constexpr size_t MAX_SIZE{50}; //! Minimum difference between system and network time for a warning to be raised. @@ -23,6 +31,8 @@ class TimeOffsets * positive offset means our peer's clock is ahead of our local clock. */ std::deque<std::chrono::seconds> m_offsets GUARDED_BY(m_mutex){}; + node::Warnings& m_warnings; + public: /** Add a new time offset sample. */ void Add(std::chrono::seconds offset) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); diff --git a/src/node/warnings.cpp b/src/node/warnings.cpp new file mode 100644 index 0000000000..b99c845900 --- /dev/null +++ b/src/node/warnings.cpp @@ -0,0 +1,68 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2022 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 <config/bitcoin-config.h> // IWYU pragma: keep + +#include <node/warnings.h> + +#include <common/system.h> +#include <node/interface_ui.h> +#include <sync.h> +#include <univalue.h> +#include <util/translation.h> + +#include <utility> +#include <vector> + +namespace node { +Warnings::Warnings() +{ + // Pre-release build warning + if (!CLIENT_VERSION_IS_RELEASE) { + m_warnings.insert( + {Warning::PRE_RELEASE_TEST_BUILD, + _("This is a pre-release test build - use at your own risk - do not use for mining or merchant applications")}); + } +} +bool Warnings::Set(warning_type id, bilingual_str message) +{ + LOCK(m_mutex); + const auto& [_, inserted]{m_warnings.insert({id, std::move(message)})}; + if (inserted) uiInterface.NotifyAlertChanged(); + return inserted; +} + +bool Warnings::Unset(warning_type id) +{ + auto success{WITH_LOCK(m_mutex, return m_warnings.erase(id))}; + if (success) uiInterface.NotifyAlertChanged(); + return success; +} + +std::vector<bilingual_str> Warnings::GetMessages() const +{ + LOCK(m_mutex); + std::vector<bilingual_str> messages; + messages.reserve(m_warnings.size()); + for (const auto& [id, msg] : m_warnings) { + messages.push_back(msg); + } + return messages; +} + +UniValue GetWarningsForRpc(const Warnings& warnings, bool use_deprecated) +{ + if (use_deprecated) { + const auto all_messages{warnings.GetMessages()}; + return all_messages.empty() ? "" : all_messages.back().original; + } + + UniValue messages{UniValue::VARR}; + for (auto&& message : warnings.GetMessages()) { + messages.push_back(std::move(message.original)); + } + return messages; +} +} // namespace node diff --git a/src/node/warnings.h b/src/node/warnings.h new file mode 100644 index 0000000000..24aeb8a922 --- /dev/null +++ b/src/node/warnings.h @@ -0,0 +1,90 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_NODE_WARNINGS_H +#define BITCOIN_NODE_WARNINGS_H + +#include <sync.h> +#include <util/translation.h> + +#include <map> +#include <variant> +#include <vector> + +class UniValue; + +namespace kernel { +enum class Warning; +} // namespace kernel + +namespace node { +enum class Warning { + CLOCK_OUT_OF_SYNC, + PRE_RELEASE_TEST_BUILD, + FATAL_INTERNAL_ERROR, +}; + +/** + * @class Warnings + * @brief Manages warning messages within a node. + * + * The Warnings class provides mechanisms to set, unset, and retrieve + * warning messages. It updates the GUI when warnings are changed. + * + * This class is designed to be non-copyable to ensure warnings + * are managed centrally. + */ +class Warnings +{ + typedef std::variant<kernel::Warning, node::Warning> warning_type; + + mutable Mutex m_mutex; + std::map<warning_type, bilingual_str> m_warnings GUARDED_BY(m_mutex); + +public: + Warnings(); + //! A warnings instance should always be passed by reference, never copied. + Warnings(const Warnings&) = delete; + Warnings& operator=(const Warnings&) = delete; + /** + * @brief Set a warning message. If a warning with the specified + * `id` is already active, false is returned and the new + * warning is ignored. If `id` does not yet exist, the + * warning is set, the UI is updated, and true is returned. + * + * @param[in] id Unique identifier of the warning. + * @param[in] message Warning message to be shown. + * + * @returns true if the warning was indeed set (i.e. there is no + * active warning with this `id`), otherwise false. + */ + bool Set(warning_type id, bilingual_str message) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); + /** + * @brief Unset a warning message. If a warning with the specified + * `id` is active, it is unset, the UI is updated, and true + * is returned. Otherwise, no warning is unset and false is + * returned. + * + * @param[in] id Unique identifier of the warning. + * + * @returns true if the warning was indeed unset (i.e. there is an + * active warning with this `id`), otherwise false. + */ + bool Unset(warning_type id) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); + /** Return potential problems detected by the node, sorted by the + * warning_type id */ + std::vector<bilingual_str> GetMessages() const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex); +}; + +/** + * RPC helper function that wraps warnings.GetMessages(). + * + * Returns a UniValue::VSTR with the latest warning if use_deprecated is + * set to true, or a UniValue::VARR with all warnings otherwise. + */ +UniValue GetWarningsForRpc(const Warnings& warnings, bool use_deprecated); +} // namespace node + +#endif // BITCOIN_NODE_WARNINGS_H diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 46b0ae161f..e785678614 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -29,6 +29,7 @@ #include <node/context.h> #include <node/transaction.h> #include <node/utxo_snapshot.h> +#include <node/warnings.h> #include <primitives/transaction.h> #include <rpc/server.h> #include <rpc/server_util.h> @@ -1308,7 +1309,8 @@ RPCHelpMan getblockchaininfo() } } - obj.pushKV("warnings", GetNodeWarnings(IsDeprecatedRPCEnabled("warnings"))); + NodeContext& node = EnsureAnyNodeContext(request.context); + obj.pushKV("warnings", node::GetWarningsForRpc(*CHECK_NONFATAL(node.warnings), IsDeprecatedRPCEnabled("warnings"))); return obj; }, }; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 6412fb35ec..0f6853ef37 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -20,6 +20,7 @@ #include <net.h> #include <node/context.h> #include <node/miner.h> +#include <node/warnings.h> #include <pow.h> #include <rpc/blockchain.h> #include <rpc/mining.h> @@ -454,7 +455,7 @@ static RPCHelpMan getmininginfo() obj.pushKV("networkhashps", getnetworkhashps().HandleRequest(request)); obj.pushKV("pooledtx", (uint64_t)mempool.size()); obj.pushKV("chain", chainman.GetParams().GetChainTypeString()); - obj.pushKV("warnings", GetNodeWarnings(IsDeprecatedRPCEnabled("warnings"))); + obj.pushKV("warnings", node::GetWarningsForRpc(*CHECK_NONFATAL(node.warnings), IsDeprecatedRPCEnabled("warnings"))); return obj; }, }; diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 1cc55f891a..1119a3e668 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -16,6 +16,7 @@ #include <netbase.h> #include <node/context.h> #include <node/protocol_version.h> +#include <node/warnings.h> #include <policy/settings.h> #include <protocol.h> #include <rpc/blockchain.h> @@ -715,7 +716,7 @@ static RPCHelpMan getnetworkinfo() } } obj.pushKV("localaddresses", std::move(localAddresses)); - obj.pushKV("warnings", GetNodeWarnings(IsDeprecatedRPCEnabled("warnings"))); + obj.pushKV("warnings", node::GetWarningsForRpc(*CHECK_NONFATAL(node.warnings), IsDeprecatedRPCEnabled("warnings"))); return obj; }, }; diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index bb1aef63f4..4df4466c49 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -5,17 +5,17 @@ #include <config/bitcoin-config.h> // IWYU pragma: keep #include <clientversion.h> -#include <core_io.h> #include <common/args.h> #include <common/messages.h> #include <common/types.h> #include <consensus/amount.h> -#include <script/interpreter.h> +#include <core_io.h> #include <key_io.h> #include <node/types.h> #include <outputtype.h> #include <rpc/util.h> #include <script/descriptor.h> +#include <script/interpreter.h> #include <script/signingprovider.h> #include <script/solver.h> #include <tinyformat.h> @@ -25,7 +25,6 @@ #include <util/strencodings.h> #include <util/string.h> #include <util/translation.h> -#include <warnings.h> #include <algorithm> #include <iterator> @@ -1382,17 +1381,3 @@ void PushWarnings(const std::vector<bilingual_str>& warnings, UniValue& obj) if (warnings.empty()) return; obj.pushKV("warnings", BilingualStringsToUniValue(warnings)); } - -UniValue GetNodeWarnings(bool use_deprecated) -{ - if (use_deprecated) { - const auto all_warnings{GetWarnings()}; - return all_warnings.empty() ? "" : all_warnings.back().original; - } - - UniValue warnings{UniValue::VARR}; - for (auto&& warning : GetWarnings()) { - warnings.push_back(std::move(warning.original)); - } - return warnings; -} diff --git a/src/rpc/util.h b/src/rpc/util.h index ca6ca6007b..23024376e0 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -503,6 +503,4 @@ private: void PushWarnings(const UniValue& warnings, UniValue& obj); void PushWarnings(const std::vector<bilingual_str>& warnings, UniValue& obj); -UniValue GetNodeWarnings(bool use_deprecated); - #endif // BITCOIN_RPC_UTIL_H diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index efe0983698..9eb7acc3ca 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -27,7 +27,7 @@ BOOST_FIXTURE_TEST_SUITE(blockmanager_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos) { const auto params {CreateChainParams(ArgsManager{}, ChainType::MAIN)}; - KernelNotifications notifications{*Assert(m_node.shutdown), m_node.exit_status}; + KernelNotifications notifications{*Assert(m_node.shutdown), m_node.exit_status, *Assert(m_node.warnings)}; const BlockManager::Options blockman_opts{ .chainparams = *params, .blocks_dir = m_args.GetBlocksDirPath(), @@ -134,7 +134,7 @@ BOOST_FIXTURE_TEST_CASE(blockmanager_block_data_availability, TestChain100Setup) BOOST_AUTO_TEST_CASE(blockmanager_flush_block_file) { - KernelNotifications notifications{*Assert(m_node.shutdown), m_node.exit_status}; + KernelNotifications notifications{*Assert(m_node.shutdown), m_node.exit_status, *Assert(m_node.warnings)}; node::BlockManager::Options blockman_opts{ .chainparams = Params(), .blocks_dir = m_args.GetBlocksDirPath(), diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 5e9ae78681..e493d532a5 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -141,7 +141,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) { NodeId id{0}; auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()); - auto peerLogic = PeerManager::make(*connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, {}); + auto peerLogic = PeerManager::make(*connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, *m_node.warnings, {}); constexpr int max_outbound_full_relay = MAX_OUTBOUND_FULL_RELAY_CONNECTIONS; CConnman::Options options; @@ -239,7 +239,7 @@ BOOST_AUTO_TEST_CASE(block_relay_only_eviction) { NodeId id{0}; auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()); - auto peerLogic = PeerManager::make(*connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, {}); + auto peerLogic = PeerManager::make(*connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, *m_node.warnings, {}); constexpr int max_outbound_block_relay{MAX_BLOCK_RELAY_ONLY_CONNECTIONS}; constexpr int64_t MINIMUM_CONNECT_TIME{30}; @@ -300,7 +300,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) auto banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()); - auto peerLogic = PeerManager::make(*connman, *m_node.addrman, banman.get(), *m_node.chainman, *m_node.mempool, {}); + auto peerLogic = PeerManager::make(*connman, *m_node.addrman, banman.get(), *m_node.chainman, *m_node.mempool, *m_node.warnings, {}); CNetAddr tor_netaddr; BOOST_REQUIRE( @@ -402,7 +402,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) auto banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()); - auto peerLogic = PeerManager::make(*connman, *m_node.addrman, banman.get(), *m_node.chainman, *m_node.mempool, {}); + auto peerLogic = PeerManager::make(*connman, *m_node.addrman, banman.get(), *m_node.chainman, *m_node.mempool, *m_node.warnings, {}); banman->ClearBanned(); int64_t nStartTime = GetTime(); diff --git a/src/test/fuzz/timeoffsets.cpp b/src/test/fuzz/timeoffsets.cpp index 019337a94a..dfa4dd705d 100644 --- a/src/test/fuzz/timeoffsets.cpp +++ b/src/test/fuzz/timeoffsets.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <node/timeoffsets.h> +#include <node/warnings.h> #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> #include <test/util/setup_common.h> @@ -19,7 +20,8 @@ void initialize_timeoffsets() FUZZ_TARGET(timeoffsets, .init = initialize_timeoffsets) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); - TimeOffsets offsets{}; + node::Warnings warnings{}; + TimeOffsets offsets{warnings}; LIMITED_WHILE(fuzzed_data_provider.remaining_bytes() > 0, 4'000) { (void)offsets.Median(); offsets.Add(std::chrono::seconds{fuzzed_data_provider.ConsumeIntegral<std::chrono::seconds::rep>()}); diff --git a/src/test/net_peer_connection_tests.cpp b/src/test/net_peer_connection_tests.cpp index 00bc1fdb6a..5f38ce112c 100644 --- a/src/test/net_peer_connection_tests.cpp +++ b/src/test/net_peer_connection_tests.cpp @@ -84,7 +84,7 @@ static void AddPeer(NodeId& id, std::vector<CNode*>& nodes, PeerManager& peerman BOOST_AUTO_TEST_CASE(test_addnode_getaddednodeinfo_and_connection_detection) { auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()); - auto peerman = PeerManager::make(*connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, {}); + auto peerman = PeerManager::make(*connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, *m_node.warnings, {}); NodeId id{0}; std::vector<CNode*> nodes; 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() diff --git a/src/test/peerman_tests.cpp b/src/test/peerman_tests.cpp index 28866695bc..397b1d8c2d 100644 --- a/src/test/peerman_tests.cpp +++ b/src/test/peerman_tests.cpp @@ -31,7 +31,7 @@ static void mineBlock(const node::NodeContext& node, std::chrono::seconds block_ // Verifying when network-limited peer connections are desirable based on the node's proximity to the tip BOOST_AUTO_TEST_CASE(connections_desirable_service_flags) { - std::unique_ptr<PeerManager> peerman = PeerManager::make(*m_node.connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, {}); + std::unique_ptr<PeerManager> peerman = PeerManager::make(*m_node.connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, *m_node.warnings, {}); auto consensus = m_node.chainman->GetParams().GetConsensus(); // Check we start connecting to full nodes diff --git a/src/test/timeoffsets_tests.cpp b/src/test/timeoffsets_tests.cpp index 008f1a9db9..5f85a5feeb 100644 --- a/src/test/timeoffsets_tests.cpp +++ b/src/test/timeoffsets_tests.cpp @@ -4,6 +4,7 @@ // #include <node/timeoffsets.h> +#include <node/warnings.h> #include <test/util/setup_common.h> #include <boost/test/unit_test.hpp> @@ -24,7 +25,8 @@ BOOST_FIXTURE_TEST_SUITE(timeoffsets_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(timeoffsets) { - TimeOffsets offsets{}; + node::Warnings warnings{}; + TimeOffsets offsets{warnings}; BOOST_CHECK(offsets.Median() == 0s); AddMulti(offsets, {{0s, -1s, -2s, -3s}}); @@ -50,7 +52,8 @@ BOOST_AUTO_TEST_CASE(timeoffsets) static bool IsWarningRaised(const std::vector<std::chrono::seconds>& check_offsets) { - TimeOffsets offsets{}; + node::Warnings warnings{}; + TimeOffsets offsets{warnings}; AddMulti(offsets, check_offsets); return offsets.WarnIfOutOfSync(); } diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 9e9db32351..79e33eacec 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -31,6 +31,7 @@ #include <node/miner.h> #include <node/peerman_args.h> #include <node/validation_cache_args.h> +#include <node/warnings.h> #include <noui.h> #include <policy/fees.h> #include <policy/fees_args.h> @@ -182,6 +183,7 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vecto InitLogging(*m_node.args); AppInitParameterInteraction(*m_node.args); LogInstance().StartLogging(); + m_node.warnings = std::make_unique<node::Warnings>(); m_node.kernel = std::make_unique<kernel::Context>(); m_node.ecc_context = std::make_unique<ECC_Context>(); SetupEnvironment(); @@ -230,10 +232,11 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vecto bilingual_str error{}; m_node.mempool = std::make_unique<CTxMemPool>(MemPoolOptionsForTest(m_node), error); Assert(error.empty()); + m_node.warnings = std::make_unique<node::Warnings>(); m_cache_sizes = CalculateCacheSizes(m_args); - m_node.notifications = std::make_unique<KernelNotifications>(*Assert(m_node.shutdown), m_node.exit_status); + m_node.notifications = std::make_unique<KernelNotifications>(*Assert(m_node.shutdown), m_node.exit_status, *Assert(m_node.warnings)); const ChainstateManager::Options chainman_opts{ .chainparams = chainparams, @@ -322,7 +325,8 @@ TestingSetup::TestingSetup( peerman_opts.deterministic_rng = true; m_node.peerman = PeerManager::make(*m_node.connman, *m_node.addrman, m_node.banman.get(), *m_node.chainman, - *m_node.mempool, peerman_opts); + *m_node.mempool, *m_node.warnings, + peerman_opts); { CConnman::Options options; diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 1f6b7368a2..1641c4cd22 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -378,7 +378,7 @@ struct SnapshotTestSetup : TestChain100Setup { LOCK(::cs_main); chainman.ResetChainstates(); BOOST_CHECK_EQUAL(chainman.GetAll().size(), 0); - m_node.notifications = std::make_unique<KernelNotifications>(*Assert(m_node.shutdown), m_node.exit_status); + m_node.notifications = std::make_unique<KernelNotifications>(*Assert(m_node.shutdown), m_node.exit_status, *Assert(m_node.warnings)); const ChainstateManager::Options chainman_opts{ .chainparams = ::Params(), .datadir = chainman.m_options.datadir, 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. diff --git a/src/warnings.cpp b/src/warnings.cpp deleted file mode 100644 index 38c0554cf2..0000000000 --- a/src/warnings.cpp +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2022 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 <config/bitcoin-config.h> // IWYU pragma: keep - -#include <warnings.h> - -#include <common/system.h> -#include <sync.h> -#include <util/translation.h> - -#include <optional> -#include <vector> - -static GlobalMutex g_warnings_mutex; -static bilingual_str g_misc_warnings GUARDED_BY(g_warnings_mutex); -static bool fLargeWorkInvalidChainFound GUARDED_BY(g_warnings_mutex) = false; -static std::optional<bilingual_str> g_timeoffset_warning GUARDED_BY(g_warnings_mutex){}; - -void SetMiscWarning(const bilingual_str& warning) -{ - LOCK(g_warnings_mutex); - g_misc_warnings = warning; -} - -void SetfLargeWorkInvalidChainFound(bool flag) -{ - LOCK(g_warnings_mutex); - fLargeWorkInvalidChainFound = flag; -} - -void SetMedianTimeOffsetWarning(std::optional<bilingual_str> warning) -{ - LOCK(g_warnings_mutex); - g_timeoffset_warning = warning; -} - -std::vector<bilingual_str> GetWarnings() -{ - std::vector<bilingual_str> warnings; - - LOCK(g_warnings_mutex); - - // Pre-release build warning - if (!CLIENT_VERSION_IS_RELEASE) { - warnings.emplace_back(_("This is a pre-release test build - use at your own risk - do not use for mining or merchant applications")); - } - - // Misc warnings like out of disk space and clock is wrong - if (!g_misc_warnings.empty()) { - warnings.emplace_back(g_misc_warnings); - } - - if (fLargeWorkInvalidChainFound) { - warnings.emplace_back(_("Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade.")); - } - - if (g_timeoffset_warning) { - warnings.emplace_back(g_timeoffset_warning.value()); - } - - return warnings; -} diff --git a/src/warnings.h b/src/warnings.h deleted file mode 100644 index 79dc2ffabf..0000000000 --- a/src/warnings.h +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2021 The Bitcoin Core developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#ifndef BITCOIN_WARNINGS_H -#define BITCOIN_WARNINGS_H - -#include <optional> -#include <string> -#include <vector> - -struct bilingual_str; - -void SetMiscWarning(const bilingual_str& warning); -void SetfLargeWorkInvalidChainFound(bool flag); -/** Pass std::nullopt to disable the warning */ -void SetMedianTimeOffsetWarning(std::optional<bilingual_str> warning); -/** Return potential problems detected by the node. */ -std::vector<bilingual_str> GetWarnings(); - -#endif // BITCOIN_WARNINGS_H |