Skip to content

Commit 260f8da

Browse files
stickies-vstickies-v
committed
refactor: remove warnings globals
1 parent 9c4b0b7 commit 260f8da

27 files changed

+87
-51
lines changed

‎src/bitcoind.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <kernel/context.h>
1818
#include <node/context.h>
1919
#include <node/interface_ui.h>
20+
#include <node/warnings.h>
2021
#include <noui.h>
2122
#include <util/check.h>
2223
#include <util/exception.h>
@@ -181,6 +182,8 @@ static bool AppInit(NodeContext& node)
181182
return false;
182183
}
183184

185+
node.warnings = std::make_unique<node::Warnings>();
186+
184187
node.kernel = std::make_unique<kernel::Context>();
185188
node.ecc_context = std::make_unique<ECC_Context>();
186189
if (!AppInitSanityChecks(*node.kernel))

‎src/index/base.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ template <typename... Args>
3030
void BaseIndex::FatalErrorf(const char* fmt, const Args&... args)
3131
{
3232
auto message = tfm::format(fmt, args...);
33-
node::AbortNode(m_chain->context()->shutdown, m_chain->context()->exit_status, Untranslated(message));
33+
node::AbortNode(m_chain->context()->shutdown, m_chain->context()->exit_status, Untranslated(message), m_chain->context()->warnings.get());
3434
}
3535

3636
CBlockLocator GetLocator(interfaces::Chain& chain, const uint256& block_hash)

‎src/init.cpp

+3-2
Original file line numberDiff line numberDiff line change
@@ -1486,7 +1486,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
14861486

14871487
// ********************************************************* Step 7: load block chain
14881488

1489-
node.notifications = std::make_unique<KernelNotifications>(*Assert(node.shutdown), node.exit_status);
1489+
node.notifications = std::make_unique<KernelNotifications>(*Assert(node.shutdown), node.exit_status, *Assert(node.warnings));
14901490
ReadNotificationArgs(args, *node.notifications);
14911491
ChainstateManager::Options chainman_opts{
14921492
.chainparams = chainparams,
@@ -1639,7 +1639,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
16391639
assert(!node.peerman);
16401640
node.peerman = PeerManager::make(*node.connman, *node.addrman,
16411641
node.banman.get(), chainman,
1642-
*node.mempool, peerman_opts);
1642+
*node.mempool, *node.warnings,
1643+
peerman_opts);
16431644
validation_signals.RegisterValidationInterface(node.peerman.get());
16441645

16451646
// ********************************************************* Step 8: start indexers

‎src/net_processing.cpp

+8-5
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include <node/blockstorage.h>
2626
#include <node/timeoffsets.h>
2727
#include <node/txreconciliation.h>
28+
#include <node/warnings.h>
2829
#include <policy/fees.h>
2930
#include <policy/policy.h>
3031
#include <policy/settings.h>
@@ -489,7 +490,7 @@ class PeerManagerImpl final : public PeerManager
489490
public:
490491
PeerManagerImpl(CConnman& connman, AddrMan& addrman,
491492
BanMan* banman, ChainstateManager& chainman,
492-
CTxMemPool& pool, Options opts);
493+
CTxMemPool& pool, node::Warnings& warnings, Options opts);
493494

494495
/** Overridden from CValidationInterface. */
495496
void BlockConnected(ChainstateRole role, const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexConnected) override
@@ -790,7 +791,8 @@ class PeerManagerImpl final : public PeerManager
790791
/** Next time to check for stale tip */
791792
std::chrono::seconds m_stale_tip_check_time GUARDED_BY(cs_main){0s};
792793

793-
TimeOffsets m_outbound_time_offsets;
794+
node::Warnings& m_warnings;
795+
TimeOffsets m_outbound_time_offsets{m_warnings};
794796

795797
const Options m_opts;
796798

@@ -2042,14 +2044,14 @@ std::optional<std::string> PeerManagerImpl::FetchBlock(NodeId peer_id, const CBl
20422044

20432045
std::unique_ptr<PeerManager> PeerManager::make(CConnman& connman, AddrMan& addrman,
20442046
BanMan* banman, ChainstateManager& chainman,
2045-
CTxMemPool& pool, Options opts)
2047+
CTxMemPool& pool, node::Warnings& warnings, Options opts)
20462048
{
2047-
return std::make_unique<PeerManagerImpl>(connman, addrman, banman, chainman, pool, opts);
2049+
return std::make_unique<PeerManagerImpl>(connman, addrman, banman, chainman, pool, warnings, opts);
20482050
}
20492051

20502052
PeerManagerImpl::PeerManagerImpl(CConnman& connman, AddrMan& addrman,
20512053
BanMan* banman, ChainstateManager& chainman,
2052-
CTxMemPool& pool, Options opts)
2054+
CTxMemPool& pool, node::Warnings& warnings, Options opts)
20532055
: m_rng{opts.deterministic_rng},
20542056
m_fee_filter_rounder{CFeeRate{DEFAULT_MIN_RELAY_TX_FEE}, m_rng},
20552057
m_chainparams(chainman.GetParams()),
@@ -2058,6 +2060,7 @@ PeerManagerImpl::PeerManagerImpl(CConnman& connman, AddrMan& addrman,
20582060
m_banman(banman),
20592061
m_chainman(chainman),
20602062
m_mempool(pool),
2063+
m_warnings{warnings},
20612064
m_opts{opts}
20622065
{
20632066
// While Erlay support is incomplete, it must be enabled explicitly via -txreconciliation.

‎src/net_processing.h

+5-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ class CChainParams;
1616
class CTxMemPool;
1717
class ChainstateManager;
1818

19+
namespace node {
20+
class Warnings;
21+
} // namespace node
22+
1923
/** Whether transaction reconciliation protocol should be enabled by default. */
2024
static constexpr bool DEFAULT_TXRECONCILIATION_ENABLE{false};
2125
/** Default for -maxorphantx, maximum number of orphan transactions kept in memory */
@@ -73,7 +77,7 @@ class PeerManager : public CValidationInterface, public NetEventsInterface
7377

7478
static std::unique_ptr<PeerManager> make(CConnman& connman, AddrMan& addrman,
7579
BanMan* banman, ChainstateManager& chainman,
76-
CTxMemPool& pool, Options opts);
80+
CTxMemPool& pool, node::Warnings& warnings, Options opts);
7781
virtual ~PeerManager() { }
7882

7983
/**

‎src/node/abort.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@
1515

1616
namespace node {
1717

18-
void AbortNode(util::SignalInterrupt* shutdown, std::atomic<int>& exit_status, const bilingual_str& message)
18+
void AbortNode(util::SignalInterrupt* shutdown, std::atomic<int>& exit_status, const bilingual_str& message, node::Warnings* warnings)
1919
{
20-
g_warnings.Set(Warning::FATAL_INTERNAL_ERROR, message);
20+
if (warnings) warnings->Set(Warning::FATAL_INTERNAL_ERROR, message);
2121
InitError(_("A fatal internal error occurred, see debug.log for details: ") + message);
2222
exit_status.store(EXIT_FAILURE);
2323
if (shutdown && !(*shutdown)()) {

‎src/node/abort.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ class SignalInterrupt;
1414
} // namespace util
1515

1616
namespace node {
17-
void AbortNode(util::SignalInterrupt* shutdown, std::atomic<int>& exit_status, const bilingual_str& message);
17+
class Warnings;
18+
void AbortNode(util::SignalInterrupt* shutdown, std::atomic<int>& exit_status, const bilingual_str& message, node::Warnings* warnings);
1819
} // namespace node
1920

2021
#endif // BITCOIN_NODE_ABORT_H

‎src/node/context.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <net_processing.h>
1414
#include <netgroup.h>
1515
#include <node/kernel_notifications.h>
16+
#include <node/warnings.h>
1617
#include <policy/fees.h>
1718
#include <scheduler.h>
1819
#include <txmempool.h>

‎src/node/context.h

+3
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ class SignalInterrupt;
3939

4040
namespace node {
4141
class KernelNotifications;
42+
class Warnings;
4243

4344
//! NodeContext struct containing references to chain state and connection
4445
//! state.
@@ -81,6 +82,8 @@ struct NodeContext {
8182
//! Issues calls about blocks and transactions
8283
std::unique_ptr<ValidationSignals> validation_signals;
8384
std::atomic<int> exit_status{EXIT_SUCCESS};
85+
//! Manages all the node warnings
86+
std::unique_ptr<node::Warnings> warnings;
8487

8588
//! Declare default constructor and destructor that are not inline, so code
8689
//! instantiating the NodeContext struct doesn't need to #include class

‎src/node/interfaces.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -93,14 +93,15 @@ class NodeImpl : public Node
9393
explicit NodeImpl(NodeContext& context) { setContext(&context); }
9494
void initLogging() override { InitLogging(args()); }
9595
void initParameterInteraction() override { InitParameterInteraction(args()); }
96-
bilingual_str getWarnings() override { return Join(node::g_warnings.GetMessages(), Untranslated("<hr />")); }
96+
bilingual_str getWarnings() override { return Join(Assert(m_context->warnings)->GetMessages(), Untranslated("<hr />")); }
9797
int getExitStatus() override { return Assert(m_context)->exit_status.load(); }
9898
uint32_t getLogCategories() override { return LogInstance().GetCategoryMask(); }
9999
bool baseInitialize() override
100100
{
101101
if (!AppInitBasicSetup(args(), Assert(context())->exit_status)) return false;
102102
if (!AppInitParameterInteraction(args())) return false;
103103

104+
m_context->warnings = std::make_unique<node::Warnings>();
104105
m_context->kernel = std::make_unique<kernel::Context>();
105106
m_context->ecc_context = std::make_unique<ECC_Context>();
106107
if (!AppInitSanityChecks(*m_context->kernel)) return false;

‎src/node/kernel_notifications.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -72,25 +72,25 @@ void KernelNotifications::progress(const bilingual_str& title, int progress_perc
7272

7373
void KernelNotifications::warningSet(kernel::Warning id, const bilingual_str& message)
7474
{
75-
if (node::g_warnings.Set(id, message)) {
75+
if (m_warnings.Set(id, message)) {
7676
AlertNotify(message.original);
7777
}
7878
}
7979

8080
void KernelNotifications::warningUnset(kernel::Warning id)
8181
{
82-
g_warnings.Unset(id);
82+
m_warnings.Unset(id);
8383
}
8484

8585
void KernelNotifications::flushError(const bilingual_str& message)
8686
{
87-
AbortNode(&m_shutdown, m_exit_status, message);
87+
AbortNode(&m_shutdown, m_exit_status, message, &m_warnings);
8888
}
8989

9090
void KernelNotifications::fatalError(const bilingual_str& message)
9191
{
9292
node::AbortNode(m_shutdown_on_fatal_error ? &m_shutdown : nullptr,
93-
m_exit_status, message);
93+
m_exit_status, message, &m_warnings);
9494
}
9595

9696
void ReadNotificationArgs(const ArgsManager& args, KernelNotifications& notifications)

‎src/node/kernel_notifications.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,14 @@ class SignalInterrupt;
2525

2626
namespace node {
2727

28+
class Warnings;
2829
static constexpr int DEFAULT_STOPATHEIGHT{0};
2930

3031
class KernelNotifications : public kernel::Notifications
3132
{
3233
public:
33-
KernelNotifications(util::SignalInterrupt& shutdown, std::atomic<int>& exit_status) : m_shutdown(shutdown), m_exit_status{exit_status} {}
34+
KernelNotifications(util::SignalInterrupt& shutdown, std::atomic<int>& exit_status, node::Warnings& warnings)
35+
: m_shutdown(shutdown), m_exit_status{exit_status}, m_warnings{warnings} {}
3436

3537
[[nodiscard]] kernel::InterruptResult blockTip(SynchronizationState state, CBlockIndex& index) override;
3638

@@ -53,6 +55,7 @@ class KernelNotifications : public kernel::Notifications
5355
private:
5456
util::SignalInterrupt& m_shutdown;
5557
std::atomic<int>& m_exit_status;
58+
node::Warnings& m_warnings;
5659
};
5760

5861
void ReadNotificationArgs(const ArgsManager& args, KernelNotifications& notifications);

‎src/node/timeoffsets.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ bool TimeOffsets::WarnIfOutOfSync() const
4848
// when median == std::numeric_limits<int64_t>::min(), calling std::chrono::abs is UB
4949
auto median{std::max(Median(), std::chrono::seconds(std::numeric_limits<int64_t>::min() + 1))};
5050
if (std::chrono::abs(median) <= WARN_THRESHOLD) {
51-
node::g_warnings.Unset(node::Warning::CLOCK_OUT_OF_SYNC);
51+
m_warnings.Unset(node::Warning::CLOCK_OUT_OF_SYNC);
5252
return false;
5353
}
5454

@@ -61,6 +61,6 @@ bool TimeOffsets::WarnIfOutOfSync() const
6161
"RPC methods to get more info."
6262
), Ticks<std::chrono::minutes>(WARN_THRESHOLD))};
6363
LogWarning("%s\n", msg.original);
64-
node::g_warnings.Set(node::Warning::CLOCK_OUT_OF_SYNC, msg);
64+
m_warnings.Set(node::Warning::CLOCK_OUT_OF_SYNC, msg);
6565
return true;
6666
}

‎src/node/timeoffsets.h

+10
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,16 @@
1111
#include <cstddef>
1212
#include <deque>
1313

14+
namespace node {
15+
class Warnings;
16+
} // namespace node
17+
1418
class TimeOffsets
1519
{
20+
public:
21+
TimeOffsets(node::Warnings& warnings) : m_warnings{warnings} {}
22+
23+
private:
1624
//! Maximum number of timeoffsets stored.
1725
static constexpr size_t MAX_SIZE{50};
1826
//! Minimum difference between system and network time for a warning to be raised.
@@ -23,6 +31,8 @@ class TimeOffsets
2331
* positive offset means our peer's clock is ahead of our local clock. */
2432
std::deque<std::chrono::seconds> m_offsets GUARDED_BY(m_mutex){};
2533

34+
node::Warnings& m_warnings;
35+
2636
public:
2737
/** Add a new time offset sample. */
2838
void Add(std::chrono::seconds offset) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);

‎src/node/warnings.cpp

+7-9
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717
#include <vector>
1818

1919
namespace node {
20-
Warnings g_warnings;
21-
2220
Warnings::Warnings()
2321
{
2422
// Pre-release build warning
@@ -54,17 +52,17 @@ std::vector<bilingual_str> Warnings::GetMessages() const
5452
return messages;
5553
}
5654

57-
UniValue GetWarningsForRpc(bool use_deprecated)
55+
UniValue GetWarningsForRpc(const Warnings& warnings, bool use_deprecated)
5856
{
5957
if (use_deprecated) {
60-
const auto all_warnings{g_warnings.GetMessages()};
61-
return all_warnings.empty() ? "" : all_warnings.back().original;
58+
const auto all_messages{warnings.GetMessages()};
59+
return all_messages.empty() ? "" : all_messages.back().original;
6260
}
6361

64-
UniValue warnings{UniValue::VARR};
65-
for (auto&& warning : g_warnings.GetMessages()) {
66-
warnings.push_back(std::move(warning.original));
62+
UniValue messages{UniValue::VARR};
63+
for (auto&& message : warnings.GetMessages()) {
64+
messages.push_back(std::move(message.original));
6765
}
68-
return warnings;
66+
return messages;
6967
}
7068
} // namespace node

‎src/node/warnings.h

+3-5
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,13 @@
77
#define BITCOIN_NODE_WARNINGS_H
88

99
#include <sync.h>
10+
#include <util/translation.h>
1011

1112
#include <map>
1213
#include <variant>
1314
#include <vector>
1415

1516
class UniValue;
16-
struct bilingual_str;
1717

1818
namespace kernel {
1919
enum class Warning;
@@ -79,14 +79,12 @@ class Warnings
7979
};
8080

8181
/**
82-
* RPC helper function that wraps g_warnings.GetMessages().
82+
* RPC helper function that wraps warnings.GetMessages().
8383
*
8484
* Returns a UniValue::VSTR with the latest warning if use_deprecated is
8585
* set to true, or a UniValue::VARR with all warnings otherwise.
8686
*/
87-
UniValue GetWarningsForRpc(bool use_deprecated);
88-
89-
extern Warnings g_warnings;
87+
UniValue GetWarningsForRpc(const Warnings& warnings, bool use_deprecated);
9088
} // namespace node
9189

9290
#endif // BITCOIN_NODE_WARNINGS_H

‎src/rpc/blockchain.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -1309,7 +1309,8 @@ RPCHelpMan getblockchaininfo()
13091309
}
13101310
}
13111311

1312-
obj.pushKV("warnings", node::GetWarningsForRpc(IsDeprecatedRPCEnabled("warnings")));
1312+
NodeContext& node = EnsureAnyNodeContext(request.context);
1313+
obj.pushKV("warnings", node::GetWarningsForRpc(*CHECK_NONFATAL(node.warnings), IsDeprecatedRPCEnabled("warnings")));
13131314
return obj;
13141315
},
13151316
};

‎src/rpc/mining.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,7 @@ static RPCHelpMan getmininginfo()
455455
obj.pushKV("networkhashps", getnetworkhashps().HandleRequest(request));
456456
obj.pushKV("pooledtx", (uint64_t)mempool.size());
457457
obj.pushKV("chain", chainman.GetParams().GetChainTypeString());
458-
obj.pushKV("warnings", node::GetWarningsForRpc(IsDeprecatedRPCEnabled("warnings")));
458+
obj.pushKV("warnings", node::GetWarningsForRpc(*CHECK_NONFATAL(node.warnings), IsDeprecatedRPCEnabled("warnings")));
459459
return obj;
460460
},
461461
};

‎src/rpc/net.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -716,7 +716,7 @@ static RPCHelpMan getnetworkinfo()
716716
}
717717
}
718718
obj.pushKV("localaddresses", std::move(localAddresses));
719-
obj.pushKV("warnings", node::GetWarningsForRpc(IsDeprecatedRPCEnabled("warnings")));
719+
obj.pushKV("warnings", node::GetWarningsForRpc(*CHECK_NONFATAL(node.warnings), IsDeprecatedRPCEnabled("warnings")));
720720
return obj;
721721
},
722722
};

‎src/test/blockmanager_tests.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ BOOST_FIXTURE_TEST_SUITE(blockmanager_tests, BasicTestingSetup)
2727
BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos)
2828
{
2929
const auto params {CreateChainParams(ArgsManager{}, ChainType::MAIN)};
30-
KernelNotifications notifications{*Assert(m_node.shutdown), m_node.exit_status};
30+
KernelNotifications notifications{*Assert(m_node.shutdown), m_node.exit_status, *Assert(m_node.warnings)};
3131
const BlockManager::Options blockman_opts{
3232
.chainparams = *params,
3333
.blocks_dir = m_args.GetBlocksDirPath(),
@@ -134,7 +134,7 @@ BOOST_FIXTURE_TEST_CASE(blockmanager_block_data_availability, TestChain100Setup)
134134

135135
BOOST_AUTO_TEST_CASE(blockmanager_flush_block_file)
136136
{
137-
KernelNotifications notifications{*Assert(m_node.shutdown), m_node.exit_status};
137+
KernelNotifications notifications{*Assert(m_node.shutdown), m_node.exit_status, *Assert(m_node.warnings)};
138138
node::BlockManager::Options blockman_opts{
139139
.chainparams = Params(),
140140
.blocks_dir = m_args.GetBlocksDirPath(),

0 commit comments

Comments
 (0)