diff options
author | TheCharlatan <seb.kung@gmail.com> | 2023-05-09 11:15:46 +0200 |
---|---|---|
committer | TheCharlatan <seb.kung@gmail.com> | 2023-06-28 09:52:33 +0200 |
commit | 6eb33bd0c21b3e075fbab596351cacafdc947472 (patch) | |
tree | 70c9082689cd178c864faa39bec9b02ea1ac028b | |
parent | 7320db96f8d2aeff0bc5dc67d8b7b37f5f808990 (diff) | |
download | bitcoin-6eb33bd0c21b3e075fbab596351cacafdc947472.tar.xz |
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 <russ@yanofsky.org>
Co-authored-by: TheCharlatan <seb.kung@gmail.com>
-rw-r--r-- | src/Makefile.am | 3 | ||||
-rw-r--r-- | src/bitcoin-chainstate.cpp | 6 | ||||
-rw-r--r-- | src/index/base.cpp | 6 | ||||
-rw-r--r-- | src/index/base.h | 3 | ||||
-rw-r--r-- | src/init.cpp | 6 | ||||
-rw-r--r-- | src/kernel/notifications_interface.h | 12 | ||||
-rw-r--r-- | src/node/abort.cpp | 27 | ||||
-rw-r--r-- | src/node/abort.h | 17 | ||||
-rw-r--r-- | src/node/blockstorage.cpp | 11 | ||||
-rw-r--r-- | src/node/kernel_notifications.cpp | 11 | ||||
-rw-r--r-- | src/node/kernel_notifications.h | 10 | ||||
-rw-r--r-- | src/shutdown.cpp | 30 | ||||
-rw-r--r-- | src/shutdown.h | 12 | ||||
-rw-r--r-- | src/test/blockmanager_tests.cpp | 2 | ||||
-rw-r--r-- | src/test/util/setup_common.cpp | 2 | ||||
-rw-r--r-- | src/test/validation_chainstatemanager_tests.cpp | 15 | ||||
-rw-r--r-- | src/validation.cpp | 37 | ||||
-rw-r--r-- | src/validation.h | 7 | ||||
-rwxr-xr-x | test/lint/run-lint-format-strings.py | 1 |
19 files changed, 128 insertions, 90 deletions
diff --git a/src/Makefile.am b/src/Makefile.am index fe0b621340..4e9c161c57 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 cb391e30de..c36233207e 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<KernelNotifications>(); diff --git a/src/index/base.cpp b/src/index/base.cpp index 99c33d53ba..cf07cae286 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -8,6 +8,7 @@ #include <interfaces/chain.h> #include <kernel/chain.h> #include <logging.h> +#include <node/abort.h> #include <node/blockstorage.h> #include <node/context.h> #include <node/database_args.h> @@ -30,9 +31,10 @@ constexpr auto SYNC_LOG_INTERVAL{30s}; constexpr auto SYNC_LOCATOR_WRITE_INTERVAL{30s}; template <typename... Args> -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 231f36b605..8affee90f8 100644 --- a/src/index/base.h +++ b/src/index/base.h @@ -94,6 +94,9 @@ private: virtual bool AllowPrune() const = 0; + template <typename... Args> + void FatalErrorf(const char* fmt, const Args&... args); + protected: std::unique_ptr<interfaces::Chain> m_chain; Chainstate* m_chainstate{nullptr}; diff --git a/src/init.cpp b/src/init.cpp index a1b210f201..988976028d 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -812,8 +812,6 @@ bool AppInitBasicSetup(const ArgsManager& args, std::atomic<int>& 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<KernelNotifications>(); + node.notifications = std::make_unique<KernelNotifications>(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 e009810473..e596a144a8 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 <util/translation.h> + #include <cstdint> #include <string> 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 0000000000..a554126b86 --- /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 <node/abort.h> + +#include <logging.h> +#include <node/interface_ui.h> +#include <shutdown.h> +#include <util/translation.h> +#include <warnings.h> + +#include <atomic> +#include <cstdlib> +#include <string> + +namespace node { + +void AbortNode(std::atomic<int>& 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 0000000000..d6bb0c14d5 --- /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 <util/translation.h> + +#include <atomic> +#include <string> + +namespace node { +void AbortNode(std::atomic<int>& 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 729ac8850a..191b6dc2f5 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<fs::path> 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 7c7db26e9b..0be7d51afb 100644 --- a/src/node/kernel_notifications.cpp +++ b/src/node/kernel_notifications.cpp @@ -10,8 +10,12 @@ #include <common/args.h> #include <common/system.h> +#include <kernel/context.h> +#include <logging.h> +#include <node/abort.h> #include <node/interface_ui.h> #include <shutdown.h> +#include <util/check.h> #include <util/strencodings.h> #include <util/string.h> #include <util/translation.h> @@ -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 fa80af1120..d35b6ecca5 100644 --- a/src/node/kernel_notifications.h +++ b/src/node/kernel_notifications.h @@ -7,6 +7,7 @@ #include <kernel/notifications_interface.h> +#include <atomic> #include <cstdint> #include <string> @@ -18,6 +19,8 @@ namespace node { class KernelNotifications : public kernel::Notifications { public: + KernelNotifications(std::atomic<int>& 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<int>& m_exit_status; }; } // namespace node diff --git a/src/shutdown.cpp b/src/shutdown.cpp index 185cad5baf..fc18ccd207 100644 --- a/src/shutdown.cpp +++ b/src/shutdown.cpp @@ -5,39 +5,13 @@ #include <shutdown.h> -#if defined(HAVE_CONFIG_H) -#include <config/bitcoin-config.h> -#endif - #include <kernel/context.h> #include <logging.h> -#include <node/interface_ui.h> #include <util/check.h> #include <util/signalinterrupt.h> -#include <warnings.h> - -#include <atomic> -#include <cassert> - -static std::atomic<int>* 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<int>& exit_status) -{ - g_exit_status = &exit_status; -} +#include <assert.h> +#include <system_error> void StartShutdown() { diff --git a/src/shutdown.h b/src/shutdown.h index 0e51575c5a..2d6ace8d93 100644 --- a/src/shutdown.h +++ b/src/shutdown.h @@ -6,18 +6,6 @@ #ifndef BITCOIN_SHUTDOWN_H #define BITCOIN_SHUTDOWN_H -#include <util/translation.h> // For bilingual_str - -#include <atomic> - -/** 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<int>& exit_status); - /** Request shutdown of the application. */ void StartShutdown(); diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index 3e8cbb218d..2ab2fa55f0 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 629b0cdcb8..3e2f0ab88d 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<KernelNotifications>(); + m_node.notifications = std::make_unique<KernelNotifications>(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 00d33114dd..8f46d54621 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<KernelNotifications>(); + m_node.notifications = std::make_unique<KernelNotifications>(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 fc8d292d6a..be6afbd320 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<CBlock> pblockNew = std::make_shared<CBlock>(); 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<const CBlock>& 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<std::chrono::milliseconds>(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<void(bilingual_str)> 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 5412cb1d48..66bc036e66 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<void(bilingual_str)> 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 2da72f0702..ed98b1b2f8 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)"), |