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 /src/net_processing.cpp | |
parent | d97ddbe797f5b8b3bca0ee71b692e542b8990195 (diff) | |
parent | 260f8da71a35232d859d7705861fc1a88bfbbe81 (diff) | |
download | bitcoin-ddf2ebd4659e926c5c6ee6726511b0b88bf7c2e0.tar.xz |
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
Diffstat (limited to 'src/net_processing.cpp')
-rw-r--r-- | src/net_processing.cpp | 13 |
1 files changed, 8 insertions, 5 deletions
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. |