From 6eb33bd0c21b3e075fbab596351cacafdc947472 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Tue, 9 May 2023 11:15:46 +0200 Subject: [PATCH] kernel: Add fatalError method to notifications FatalError replaces what previously was the AbortNode function in shutdown.cpp. This commit is part of the libbitcoinkernel project and further removes the shutdown's and, more generally, the kernel library's dependency on interface_ui with a kernel notification method. By removing interface_ui from the kernel library, its dependency on boost is reduced to just boost::multi_index. At the same time it also takes a step towards de-globalising the interrupt infrastructure. Co-authored-by: Russell Yanofsky Co-authored-by: TheCharlatan --- src/Makefile.am | 3 +- src/bitcoin-chainstate.cpp | 6 ++- src/index/base.cpp | 6 ++- src/index/base.h | 3 ++ src/init.cpp | 6 +-- src/kernel/notifications_interface.h | 12 +++++- src/node/abort.cpp | 27 ++++++++++++++ src/node/abort.h | 17 +++++++++ src/node/blockstorage.cpp | 11 +++--- src/node/kernel_notifications.cpp | 11 +++++- src/node/kernel_notifications.h | 10 +++++ src/shutdown.cpp | 30 +-------------- src/shutdown.h | 12 ------ src/test/blockmanager_tests.cpp | 2 +- src/test/util/setup_common.cpp | 2 +- .../validation_chainstatemanager_tests.cpp | 15 +++----- src/validation.cpp | 37 +++++++++---------- src/validation.h | 7 +--- test/lint/run-lint-format-strings.py | 1 + 19 files changed, 128 insertions(+), 90 deletions(-) create mode 100644 src/node/abort.cpp create mode 100644 src/node/abort.h diff --git a/src/Makefile.am b/src/Makefile.am index fe0b6213402..4e9c161c57d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -205,6 +205,7 @@ BITCOIN_CORE_H = \ netbase.h \ netgroup.h \ netmessagemaker.h \ + node/abort.h \ node/blockmanager_args.h \ node/blockstorage.h \ node/caches.h \ @@ -400,6 +401,7 @@ libbitcoin_node_a_SOURCES = \ net.cpp \ net_processing.cpp \ netgroup.cpp \ + node/abort.cpp \ node/blockmanager_args.cpp \ node/blockstorage.cpp \ node/caches.cpp \ @@ -935,7 +937,6 @@ libbitcoinkernel_la_SOURCES = \ logging.cpp \ node/blockstorage.cpp \ node/chainstate.cpp \ - node/interface_ui.cpp \ node/utxo_snapshot.cpp \ policy/feerate.cpp \ policy/fees.cpp \ diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index cb391e30de5..c36233207e8 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -101,7 +101,11 @@ int main(int argc, char* argv[]) { std::cerr << "Error flushing block data to disk: " << debug_message << std::endl; } - + void fatalError(const std::string& debug_message, const bilingual_str& user_message) override + { + std::cerr << "Error: " << debug_message << std::endl; + std::cerr << (user_message.empty() ? "A fatal internal error occurred." : user_message.original) << std::endl; + } }; auto notifications = std::make_unique(); diff --git a/src/index/base.cpp b/src/index/base.cpp index 99c33d53ba4..cf07cae2864 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -30,9 +31,10 @@ constexpr auto SYNC_LOG_INTERVAL{30s}; constexpr auto SYNC_LOCATOR_WRITE_INTERVAL{30s}; template -static void FatalErrorf(const char* fmt, const Args&... args) +void BaseIndex::FatalErrorf(const char* fmt, const Args&... args) { - AbortNode(tfm::format(fmt, args...)); + auto message = tfm::format(fmt, args...); + node::AbortNode(m_chain->context()->exit_status, message); } CBlockLocator GetLocator(interfaces::Chain& chain, const uint256& block_hash) diff --git a/src/index/base.h b/src/index/base.h index 231f36b6053..8affee90f86 100644 --- a/src/index/base.h +++ b/src/index/base.h @@ -94,6 +94,9 @@ private: virtual bool AllowPrune() const = 0; + template + void FatalErrorf(const char* fmt, const Args&... args); + protected: std::unique_ptr m_chain; Chainstate* m_chainstate{nullptr}; diff --git a/src/init.cpp b/src/init.cpp index a1b210f2014..988976028dd 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -812,8 +812,6 @@ bool AppInitBasicSetup(const ArgsManager& args, std::atomic& exit_status) // Enable heap terminate-on-corruption HeapSetInformation(nullptr, HeapEnableTerminationOnCorruption, nullptr, 0); #endif - InitShutdownState(exit_status); - if (!SetupNetworking()) { return InitError(Untranslated("Initializing networking failed.")); } @@ -986,7 +984,7 @@ bool AppInitParameterInteraction(const ArgsManager& args) // Also report errors from parsing before daemonization { - KernelNotifications notifications{}; + kernel::Notifications notifications{}; ChainstateManager::Options chainman_opts_dummy{ .chainparams = chainparams, .datadir = args.GetDataDirNet(), @@ -1410,7 +1408,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // ********************************************************* Step 7: load block chain - node.notifications = std::make_unique(); + node.notifications = std::make_unique(node.exit_status); fReindex = args.GetBoolArg("-reindex", false); bool fReindexChainState = args.GetBoolArg("-reindex-chainstate", false); ChainstateManager::Options chainman_opts{ diff --git a/src/kernel/notifications_interface.h b/src/kernel/notifications_interface.h index e0098104735..e596a144a88 100644 --- a/src/kernel/notifications_interface.h +++ b/src/kernel/notifications_interface.h @@ -5,12 +5,13 @@ #ifndef BITCOIN_KERNEL_NOTIFICATIONS_INTERFACE_H #define BITCOIN_KERNEL_NOTIFICATIONS_INTERFACE_H +#include + #include #include class CBlockIndex; enum class SynchronizationState; -struct bilingual_str; namespace kernel { @@ -35,6 +36,15 @@ public: //! by logging the error, or notifying the user, or triggering an early //! shutdown as a precaution against causing more errors. virtual void flushError(const std::string& debug_message) {} + + //! The fatal error notification is sent to notify the user when an error + //! occurs in kernel code that can't be recovered from. After this + //! notification is sent, whatever function triggered the error should also + //! return an error code or raise an exception. Applications can choose to + //! handle the fatal error notification by logging the error, or notifying + //! the user, or triggering an early shutdown as a precaution against + //! causing more errors. + virtual void fatalError(const std::string& debug_message, const bilingual_str& user_message = {}) {} }; } // namespace kernel diff --git a/src/node/abort.cpp b/src/node/abort.cpp new file mode 100644 index 00000000000..a554126b86b --- /dev/null +++ b/src/node/abort.cpp @@ -0,0 +1,27 @@ +// Copyright (c) 2023 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 + +#include +#include +#include +#include +#include + +#include +#include +#include + +namespace node { + +void AbortNode(std::atomic& exit_status, const std::string& debug_message, const bilingual_str& user_message, bool shutdown) +{ + SetMiscWarning(Untranslated(debug_message)); + LogPrintf("*** %s\n", debug_message); + InitError(user_message.empty() ? _("A fatal internal error occurred, see debug.log for details") : user_message); + exit_status.store(EXIT_FAILURE); + if (shutdown) StartShutdown(); +} +} // namespace node diff --git a/src/node/abort.h b/src/node/abort.h new file mode 100644 index 00000000000..d6bb0c14d53 --- /dev/null +++ b/src/node/abort.h @@ -0,0 +1,17 @@ +// Copyright (c) 2023 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_ABORT_H +#define BITCOIN_NODE_ABORT_H + +#include + +#include +#include + +namespace node { +void AbortNode(std::atomic& exit_status, const std::string& debug_message, const bilingual_str& user_message = {}, bool shutdown = true); +} // namespace node + +#endif // BITCOIN_NODE_ABORT_H diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 729ac8850a2..191b6dc2f5f 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -659,7 +659,8 @@ bool BlockManager::FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigne bool out_of_space; size_t bytes_allocated = BlockFileSeq().Allocate(pos, nAddSize, out_of_space); if (out_of_space) { - return AbortNode("Disk space is too low!", _("Disk space is too low!")); + m_opts.notifications.fatalError("Disk space is too low!", _("Disk space is too low!")); + return false; } if (bytes_allocated != 0 && IsPruneMode()) { m_check_for_pruning = true; @@ -683,7 +684,7 @@ bool BlockManager::FindUndoPos(BlockValidationState& state, int nFile, FlatFileP bool out_of_space; size_t bytes_allocated = UndoFileSeq().Allocate(pos, nAddSize, out_of_space); if (out_of_space) { - return AbortNode(state, "Disk space is too low!", _("Disk space is too low!")); + return FatalError(m_opts.notifications, state, "Disk space is too low!", _("Disk space is too low!")); } if (bytes_allocated != 0 && IsPruneMode()) { m_check_for_pruning = true; @@ -725,7 +726,7 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid return error("ConnectBlock(): FindUndoPos failed"); } if (!UndoWriteToDisk(blockundo, _pos, block.pprev->GetBlockHash(), GetParams().MessageStart())) { - return AbortNode(state, "Failed to write undo data"); + return FatalError(m_opts.notifications, state, "Failed to write undo data"); } // rev files are written in block height order, whereas blk files are written as blocks come in (often out of order) // we want to flush the rev (undo) file once we've written the last block, which is indicated by the last height @@ -843,7 +844,7 @@ FlatFilePos BlockManager::SaveBlockToDisk(const CBlock& block, int nHeight, CCha } if (!position_known) { if (!WriteBlockToDisk(block, blockPos, GetParams().MessageStart())) { - AbortNode("Failed to write block"); + m_opts.notifications.fatalError("Failed to write block"); return FlatFilePos(); } } @@ -927,7 +928,7 @@ void ThreadImport(ChainstateManager& chainman, std::vector vImportFile for (Chainstate* chainstate : WITH_LOCK(::cs_main, return chainman.GetAll())) { BlockValidationState state; if (!chainstate->ActivateBestChain(state, nullptr)) { - AbortNode(strprintf("Failed to connect best block (%s)", state.ToString())); + chainman.GetNotifications().fatalError(strprintf("Failed to connect best block (%s)", state.ToString())); return; } } diff --git a/src/node/kernel_notifications.cpp b/src/node/kernel_notifications.cpp index 7c7db26e9b9..0be7d51afbe 100644 --- a/src/node/kernel_notifications.cpp +++ b/src/node/kernel_notifications.cpp @@ -10,8 +10,12 @@ #include #include +#include +#include +#include #include #include +#include #include #include #include @@ -75,7 +79,12 @@ void KernelNotifications::warning(const bilingual_str& warning) void KernelNotifications::flushError(const std::string& debug_message) { - AbortNode(debug_message); + AbortNode(m_exit_status, debug_message); +} + +void KernelNotifications::fatalError(const std::string& debug_message, const bilingual_str& user_message) +{ + node::AbortNode(m_exit_status, debug_message, user_message, m_shutdown_on_fatal_error); } } // namespace node diff --git a/src/node/kernel_notifications.h b/src/node/kernel_notifications.h index fa80af11205..d35b6ecca5c 100644 --- a/src/node/kernel_notifications.h +++ b/src/node/kernel_notifications.h @@ -7,6 +7,7 @@ #include +#include #include #include @@ -18,6 +19,8 @@ namespace node { class KernelNotifications : public kernel::Notifications { public: + KernelNotifications(std::atomic& exit_status) : m_exit_status{exit_status} {} + void blockTip(SynchronizationState state, CBlockIndex& index) override; void headerTip(SynchronizationState state, int64_t height, int64_t timestamp, bool presync) override; @@ -27,6 +30,13 @@ public: void warning(const bilingual_str& warning) override; void flushError(const std::string& debug_message) override; + + void fatalError(const std::string& debug_message, const bilingual_str& user_message = {}) override; + + //! Useful for tests, can be set to false to avoid shutdown on fatal error. + bool m_shutdown_on_fatal_error{true}; +private: + std::atomic& m_exit_status; }; } // namespace node diff --git a/src/shutdown.cpp b/src/shutdown.cpp index 185cad5baf6..fc18ccd2076 100644 --- a/src/shutdown.cpp +++ b/src/shutdown.cpp @@ -5,39 +5,13 @@ #include -#if defined(HAVE_CONFIG_H) -#include -#endif - #include #include -#include #include #include -#include -#include -#include - -static std::atomic* g_exit_status{nullptr}; - -bool AbortNode(const std::string& strMessage, bilingual_str user_message) -{ - SetMiscWarning(Untranslated(strMessage)); - LogPrintf("*** %s\n", strMessage); - if (user_message.empty()) { - user_message = _("A fatal internal error occurred, see debug.log for details"); - } - InitError(user_message); - Assert(g_exit_status)->store(EXIT_FAILURE); - StartShutdown(); - return false; -} - -void InitShutdownState(std::atomic& exit_status) -{ - g_exit_status = &exit_status; -} +#include +#include void StartShutdown() { diff --git a/src/shutdown.h b/src/shutdown.h index 0e51575c5a5..2d6ace8d936 100644 --- a/src/shutdown.h +++ b/src/shutdown.h @@ -6,18 +6,6 @@ #ifndef BITCOIN_SHUTDOWN_H #define BITCOIN_SHUTDOWN_H -#include // For bilingual_str - -#include - -/** Abort with a message */ -bool AbortNode(const std::string& strMessage, bilingual_str user_message = bilingual_str{}); - -/** Initialize shutdown state. This must be called before using either StartShutdown(), - * AbortShutdown() or WaitForShutdown(). Calling ShutdownRequested() is always safe. - */ -void InitShutdownState(std::atomic& exit_status); - /** Request shutdown of the application. */ void StartShutdown(); diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index 3e8cbb218d4..2ab2fa55f0a 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -23,7 +23,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{}; + KernelNotifications notifications{m_node.exit_status}; const BlockManager::Options blockman_opts{ .chainparams = *params, .blocks_dir = m_args.GetBlocksDirPath(), diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 629b0cdcb8d..3e2f0ab88d5 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -184,7 +184,7 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vecto m_cache_sizes = CalculateCacheSizes(m_args); - m_node.notifications = std::make_unique(); + m_node.notifications = std::make_unique(m_node.exit_status); const ChainstateManager::Options chainman_opts{ .chainparams = chainparams, diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 00d33114dd3..8f46d546217 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -379,7 +379,7 @@ struct SnapshotTestSetup : TestChain100Setup { LOCK(::cs_main); chainman.ResetChainstates(); BOOST_CHECK_EQUAL(chainman.GetAll().size(), 0); - m_node.notifications = std::make_unique(); + m_node.notifications = std::make_unique(m_node.exit_status); const ChainstateManager::Options chainman_opts{ .chainparams = ::Params(), .datadir = chainman.m_options.datadir, @@ -564,7 +564,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup auto db_cache_before_complete = active_cs.m_coinsdb_cache_size_bytes; SnapshotCompletionResult res; - auto mock_shutdown = [](bilingual_str msg) {}; + m_node.notifications->m_shutdown_on_fatal_error = false; fs::path snapshot_chainstate_dir = *node::FindSnapshotChainstateDir(chainman.m_options.datadir); BOOST_CHECK(fs::exists(snapshot_chainstate_dir)); @@ -574,8 +574,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup const uint256 snapshot_tip_hash = WITH_LOCK(chainman.GetMutex(), return chainman.ActiveTip()->GetBlockHash()); - res = WITH_LOCK(::cs_main, - return chainman.MaybeCompleteSnapshotValidation(mock_shutdown)); + res = WITH_LOCK(::cs_main, return chainman.MaybeCompleteSnapshotValidation()); BOOST_CHECK_EQUAL(res, SnapshotCompletionResult::SUCCESS); WITH_LOCK(::cs_main, BOOST_CHECK(chainman.IsSnapshotValidated())); @@ -591,8 +590,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup BOOST_CHECK_EQUAL(all_chainstates[0], &active_cs); // Trying completion again should return false. - res = WITH_LOCK(::cs_main, - return chainman.MaybeCompleteSnapshotValidation(mock_shutdown)); + res = WITH_LOCK(::cs_main, return chainman.MaybeCompleteSnapshotValidation()); BOOST_CHECK_EQUAL(res, SnapshotCompletionResult::SKIPPED); // The invalid snapshot path should not have been used. @@ -645,7 +643,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion_hash_mismatch, Sna Chainstate& validation_chainstate = *std::get<0>(chainstates); ChainstateManager& chainman = *Assert(m_node.chainman); SnapshotCompletionResult res; - auto mock_shutdown = [](bilingual_str msg) {}; + m_node.notifications->m_shutdown_on_fatal_error = false; // Test tampering with the IBD UTXO set with an extra coin to ensure it causes // snapshot completion to fail. @@ -661,8 +659,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion_hash_mismatch, Sna fs::path snapshot_chainstate_dir = gArgs.GetDataDirNet() / "chainstate_snapshot"; BOOST_CHECK(fs::exists(snapshot_chainstate_dir)); - res = WITH_LOCK(::cs_main, - return chainman.MaybeCompleteSnapshotValidation(mock_shutdown)); + res = WITH_LOCK(::cs_main, return chainman.MaybeCompleteSnapshotValidation()); BOOST_CHECK_EQUAL(res, SnapshotCompletionResult::HASH_MISMATCH); auto all_chainstates = chainman.GetAll(); diff --git a/src/validation.cpp b/src/validation.cpp index fc8d292d6a1..be6afbd3208 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1842,9 +1842,9 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state, return true; } -bool AbortNode(BlockValidationState& state, const std::string& strMessage, const bilingual_str& userMessage) +bool FatalError(Notifications& notifications, BlockValidationState& state, const std::string& strMessage, const bilingual_str& userMessage) { - AbortNode(strMessage, userMessage); + notifications.fatalError(strMessage, userMessage); return state.Error(strMessage); } @@ -2079,7 +2079,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, // We don't write down blocks to disk if they may have been // corrupted, so this should be impossible unless we're having hardware // problems. - return AbortNode(state, "Corrupt block found indicating potential hardware failure; shutting down"); + return FatalError(m_chainman.GetNotifications(), state, "Corrupt block found indicating potential hardware failure; shutting down"); } return error("%s: Consensus::CheckBlock: %s", __func__, state.ToString()); } @@ -2499,7 +2499,7 @@ bool Chainstate::FlushStateToDisk( if (fDoFullFlush || fPeriodicWrite) { // Ensure we can write block index if (!CheckDiskSpace(m_blockman.m_opts.blocks_dir)) { - return AbortNode(state, "Disk space is too low!", _("Disk space is too low!")); + return FatalError(m_chainman.GetNotifications(), state, "Disk space is too low!", _("Disk space is too low!")); } { LOG_TIME_MILLIS_WITH_CATEGORY("write block and undo data to disk", BCLog::BENCH); @@ -2513,7 +2513,7 @@ bool Chainstate::FlushStateToDisk( LOG_TIME_MILLIS_WITH_CATEGORY("write block index to disk", BCLog::BENCH); if (!m_blockman.WriteBlockIndexDB()) { - return AbortNode(state, "Failed to write to block index database"); + return FatalError(m_chainman.GetNotifications(), state, "Failed to write to block index database"); } } // Finally remove any pruned files @@ -2535,11 +2535,11 @@ bool Chainstate::FlushStateToDisk( // an overestimation, as most will delete an existing entry or // overwrite one. Still, use a conservative safety factor of 2. if (!CheckDiskSpace(m_chainman.m_options.datadir, 48 * 2 * 2 * CoinsTip().GetCacheSize())) { - return AbortNode(state, "Disk space is too low!", _("Disk space is too low!")); + return FatalError(m_chainman.GetNotifications(), state, "Disk space is too low!", _("Disk space is too low!")); } // Flush the chainstate (which may refer to block index entries). if (!CoinsTip().Flush()) - return AbortNode(state, "Failed to write to coin database"); + return FatalError(m_chainman.GetNotifications(), state, "Failed to write to coin database"); m_last_flush = nNow; full_flush_completed = true; TRACE5(utxocache, flush, @@ -2555,7 +2555,7 @@ bool Chainstate::FlushStateToDisk( GetMainSignals().ChainStateFlushed(m_chain.GetLocator()); } } catch (const std::runtime_error& e) { - return AbortNode(state, std::string("System error while flushing: ") + e.what()); + return FatalError(m_chainman.GetNotifications(), state, std::string("System error while flushing: ") + e.what()); } return true; } @@ -2791,7 +2791,7 @@ bool Chainstate::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew, if (!pblock) { std::shared_ptr pblockNew = std::make_shared(); if (!m_blockman.ReadBlockFromDisk(*pblockNew, *pindexNew)) { - return AbortNode(state, "Failed to read block"); + return FatalError(m_chainman.GetNotifications(), state, "Failed to read block"); } pthisBlock = pblockNew; } else { @@ -2975,7 +2975,7 @@ bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex* // If we're unable to disconnect a block during normal operation, // then that is a failure of our local system -- we should abort // rather than stay on a less work chain. - AbortNode(state, "Failed to disconnect block; see debug.log for details"); + FatalError(m_chainman.GetNotifications(), state, "Failed to disconnect block; see debug.log for details"); return false; } fBlocksDisconnected = true; @@ -3970,7 +3970,7 @@ bool Chainstate::AcceptBlock(const std::shared_ptr& pblock, BlockV } ReceivedBlockTransactions(block, pindex, blockPos); } catch (const std::runtime_error& e) { - return AbortNode(state, std::string("System error: ") + e.what()); + return FatalError(m_chainman.GetNotifications(), state, std::string("System error: ") + e.what()); } FlushStateToDisk(state, FlushStateMode::NONE); @@ -4661,7 +4661,7 @@ void Chainstate::LoadExternalBlockFile( } } } catch (const std::runtime_error& e) { - AbortNode(std::string("System error: ") + e.what()); + m_chainman.GetNotifications().fatalError(std::string("System error: ") + e.what()); } LogPrintf("Loaded %i blocks from external file in %dms\n", nLoaded, Ticks(SteadyClock::now() - start)); } @@ -5113,7 +5113,7 @@ bool ChainstateManager::ActivateSnapshot( snapshot_chainstate.reset(); bool removed = DeleteCoinsDBFromDisk(*snapshot_datadir, /*is_snapshot=*/true); if (!removed) { - AbortNode(strprintf("Failed to remove snapshot chainstate dir (%s). " + GetNotifications().fatalError(strprintf("Failed to remove snapshot chainstate dir (%s). " "Manually remove it before restarting.\n", fs::PathToString(*snapshot_datadir))); } } @@ -5378,8 +5378,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot( // through IsUsable() checks, or // // (ii) giving each chainstate its own lock instead of using cs_main for everything. -SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation( - std::function shutdown_fnc) +SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation() { AssertLockHeld(cs_main); if (m_ibd_chainstate.get() == &this->ActiveChainstate() || @@ -5431,7 +5430,7 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation( user_error = strprintf(Untranslated("%s\n%s"), user_error, util::ErrorString(rename_result)); } - shutdown_fnc(user_error); + GetNotifications().fatalError(user_error.original, user_error); }; if (index_new.GetBlockHash() != snapshot_blockhash) { @@ -5728,13 +5727,13 @@ bool ChainstateManager::ValidatedSnapshotCleanup() fs::path tmp_old{ibd_chainstate_path + "_todelete"}; - auto rename_failed_abort = []( + auto rename_failed_abort = [this]( fs::path p_old, fs::path p_new, const fs::filesystem_error& err) { LogPrintf("%s: error renaming file (%s): %s\n", __func__, fs::PathToString(p_old), err.what()); - AbortNode(strprintf( + GetNotifications().fatalError(strprintf( "Rename of '%s' -> '%s' failed. " "Cannot clean up the background chainstate leveldb directory.", fs::PathToString(p_old), fs::PathToString(p_new))); @@ -5759,7 +5758,7 @@ bool ChainstateManager::ValidatedSnapshotCleanup() } if (!DeleteCoinsDBFromDisk(tmp_old, /*is_snapshot=*/false)) { - // No need to AbortNode because once the unneeded bg chainstate data is + // No need to FatalError because once the unneeded bg chainstate data is // moved, it will not interfere with subsequent initialization. LogPrintf("Deletion of %s failed. Please remove it manually, as the " /* Continued */ "directory is now unnecessary.\n", diff --git a/src/validation.h b/src/validation.h index 5412cb1d48a..66bc036e668 100644 --- a/src/validation.h +++ b/src/validation.h @@ -106,7 +106,7 @@ void StopScriptCheckWorkerThreads(); CAmount GetBlockSubsidy(int nHeight, const Consensus::Params& consensusParams); -bool AbortNode(BlockValidationState& state, const std::string& strMessage, const bilingual_str& userMessage = bilingual_str{}); +bool FatalError(kernel::Notifications& notifications, BlockValidationState& state, const std::string& strMessage, const bilingual_str& userMessage = {}); /** Guess verification progress (as a fraction between 0.0=genesis and 1.0=current tip). */ double GuessVerificationProgress(const ChainTxData& data, const CBlockIndex* pindex); @@ -1056,10 +1056,7 @@ public: //! If the coins match (expected), then mark the validation chainstate for //! deletion and continue using the snapshot chainstate as active. //! Otherwise, revert to using the ibd chainstate and shutdown. - SnapshotCompletionResult MaybeCompleteSnapshotValidation( - std::function shutdown_fnc = - [](bilingual_str msg) { AbortNode(msg.original, msg); }) - EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + SnapshotCompletionResult MaybeCompleteSnapshotValidation() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); //! The most-work chain. Chainstate& ActiveChainstate() const; diff --git a/test/lint/run-lint-format-strings.py b/test/lint/run-lint-format-strings.py index 2da72f07026..ed98b1b2f81 100755 --- a/test/lint/run-lint-format-strings.py +++ b/test/lint/run-lint-format-strings.py @@ -15,6 +15,7 @@ import sys FALSE_POSITIVES = [ ("src/dbwrapper.cpp", "vsnprintf(p, limit - p, format, backup_ap)"), ("src/index/base.cpp", "FatalErrorf(const char* fmt, const Args&... args)"), + ("src/index/base.h", "FatalErrorf(const char* fmt, const Args&... args)"), ("src/netbase.cpp", "LogConnectFailure(bool manual_connection, const char* fmt, const Args&... args)"), ("src/clientversion.cpp", "strprintf(_(COPYRIGHT_HOLDERS).translated, COPYRIGHT_HOLDERS_SUBSTITUTION)"), ("src/test/translation_tests.cpp", "strprintf(format, arg)"),