refactor: Pass InitError as callback to ChainstateManager

sedited · sedited · commit 0b159b9256c2 · 2023-05-12T09:39:53.000+02:00
This commit is part of the libbitcoinkernel project and removes the
shutdown's and, more generally, the kernel library's dependency on
interface_ui with a callback function. By removing interface_ui from the
kernel library, its dependency on boost is reduced to just
boost::multi_index.
diff --git a/src/Makefile.am b/src/Makefile.am
@@ -934,7 +934,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
@@ -91,6 +91,7 @@ int main(int argc, char* argv[])
             .notify_header_tip = [](SynchronizationState, int64_t height, int64_t timestamp, bool presync) { std::cout << "Header tip changed: " << height << ", " << timestamp << ", " << presync << std::endl; },
             .show_progress = [](const std::string& title, int nProgress, bool resume_possible) { std::cout << "Progress: " << title << ", " << nProgress << ", " << resume_possible << std::endl; },
             .do_warning = [](const bilingual_str& warning) { std::cout << "Warning: " << warning.original << std::endl; },
+            .init_error = [](const bilingual_str& user_message) { std::cout << "Init error: " << user_message.original << std::endl; },
         },
     };
     const node::BlockManager::Options blockman_opts{
diff --git a/src/init.cpp b/src/init.cpp
@@ -1455,7 +1455,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     BlockManager::Options blockman_opts{
         .chainparams = chainman_opts.chainparams,
         .blocks_dir = args.GetBlocksDirPath(),
-    };
+        .init_error_callback = [](const bilingual_str& user_message) {
+            InitError(user_message);
+        }};
     Assert(!ApplyArgsManOptions(args, blockman_opts)); // no error can happen, already checked in AppInitParameterInteraction
 
     // cache size calculations
diff --git a/src/kernel/blockmanager_opts.h b/src/kernel/blockmanager_opts.h
@@ -6,6 +6,7 @@
 #define BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H
 
 #include <util/fs.h>
+#include <util/translation.h>
 
 #include <cstdint>
 
@@ -25,6 +26,7 @@ struct BlockManagerOpts {
     bool fast_prune{false};
     bool stop_after_block_import{DEFAULT_STOPAFTERBLOCKIMPORT};
     const fs::path blocks_dir;
+    const std::function<void(const bilingual_str& str)> init_error_callback;
 };
 
 } // namespace kernel
diff --git a/src/kernel/chainstatemanager_opts.h b/src/kernel/chainstatemanager_opts.h
@@ -35,6 +35,7 @@ struct ChainstateManagerNotificationCallbacks {
     const std::function<void(SynchronizationState state, int64_t height, int64_t timestamp, bool presync)> notify_header_tip;
     const std::function<void(const std::string& title, int nProgress, bool resume_possible)> show_progress;
     const std::function<void(const bilingual_str& warning)> do_warning;
+    const std::function<void(const bilingual_str& user_message)> init_error;
 };
 
 /**
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
@@ -527,7 +527,7 @@ void BlockManager::FlushUndoFile(int block_file, bool finalize)
 {
     FlatFilePos undo_pos_old(block_file, m_blockfile_info[block_file].nUndoSize);
     if (!UndoFileSeq().Flush(undo_pos_old, finalize)) {
-        AbortNode("Flushing undo file to disk failed. This is likely the result of an I/O error.");
+        AbortNode("Flushing undo file to disk failed. This is likely the result of an I/O error.", m_opts.init_error_callback);
     }
 }
 
@@ -546,7 +546,7 @@ void BlockManager::FlushBlockFile(bool fFinalize, bool finalize_undo)
 
     FlatFilePos block_pos_old(m_last_blockfile, m_blockfile_info[m_last_blockfile].nSize);
     if (!BlockFileSeq().Flush(block_pos_old, fFinalize)) {
-        AbortNode("Flushing block file to disk failed. This is likely the result of an I/O error.");
+        AbortNode("Flushing block file to disk failed. This is likely the result of an I/O error.", m_opts.init_error_callback);
     }
     // we do not always flush the undo file, as the chain tip may be lagging behind the incoming blocks,
     // e.g. during IBD or a sync after a node going offline
@@ -658,7 +658,7 @@ 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!"));
+            return AbortNode("Disk space is too low!", m_opts.init_error_callback, _("Disk space is too low!"));
         }
         if (bytes_allocated != 0 && IsPruneMode()) {
             m_check_for_pruning = true;
@@ -682,7 +682,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 AbortNode(state, "Disk space is too low!", m_opts.init_error_callback, _("Disk space is too low!"));
     }
     if (bytes_allocated != 0 && IsPruneMode()) {
         m_check_for_pruning = true;
@@ -724,7 +724,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 AbortNode(state, "Failed to write undo data", m_opts.init_error_callback);
         }
         // 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
@@ -842,7 +842,7 @@ FlatFilePos BlockManager::SaveBlockToDisk(const CBlock& block, int nHeight, CCha
     }
     if (!position_known) {
         if (!WriteBlockToDisk(block, blockPos, GetParams().MessageStart())) {
-            AbortNode("Failed to write block");
+            AbortNode("Failed to write block", m_opts.init_error_callback);
             return FlatFilePos();
         }
     }
diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h
@@ -154,7 +154,10 @@ class BlockManager
 
     explicit BlockManager(Options opts)
         : m_prune_mode{opts.prune_target > 0},
-          m_opts{std::move(opts)} {};
+          m_opts{std::move(opts)}
+    {
+        assert(m_opts.init_error_callback);
+    };
 
     std::atomic<bool> m_importing{false};
 
diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp
@@ -200,14 +200,17 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize
     // snapshot is actually validated? Because this entails unusual
     // filesystem operations to move leveldb data directories around, and that seems
     // too risky to do in the middle of normal runtime.
-    auto snapshot_completion = chainman.MaybeCompleteSnapshotValidation();
+    auto snapshot_completion = chainman.MaybeCompleteSnapshotValidation(
+        [&init_error = chainman.GetInitErrorCb()](bilingual_str msg) {
+            AbortNode(msg.original, init_error, msg);
+        });
 
     if (snapshot_completion == SnapshotCompletionResult::SKIPPED) {
         // do nothing; expected case
     } else if (snapshot_completion == SnapshotCompletionResult::SUCCESS) {
         LogPrintf("[snapshot] cleaning up unneeded background chainstate, then reinitializing\n");
         if (!chainman.ValidatedSnapshotCleanup()) {
-            AbortNode("Background chainstate cleanup failed unexpectedly.");
+            AbortNode("Background chainstate cleanup failed unexpectedly.", chainman.GetInitErrorCb());
         }
 
         // Because ValidatedSnapshotCleanup() has torn down chainstates with
diff --git a/src/node/chainstatemanager_notifications.cpp b/src/node/chainstatemanager_notifications.cpp
@@ -65,6 +65,7 @@ ChainstateManagerNotificationCallbacks DefaultChainstateManagerNotifications()
         .notify_header_tip = [](SynchronizationState state, int64_t height, int64_t timestamp, bool presync) { uiInterface.NotifyHeaderTip(state, height, timestamp, presync); },
         .show_progress = [](const std::string& title, int nProgress, bool resume_possible) { uiInterface.ShowProgress(title, nProgress, resume_possible); },
         .do_warning = [](const bilingual_str& warning) { DoWarning(warning); },
+        .init_error = [](const bilingual_str& user_message) { InitError(user_message); },
     };
 }
 } // namespace node
diff --git a/src/shutdown.cpp b/src/shutdown.cpp
@@ -10,7 +10,6 @@
 #endif
 
 #include <logging.h>
-#include <node/interface_ui.h>
 #include <util/tokenpipe.h>
 #include <warnings.h>
 
@@ -20,14 +19,14 @@
 #include <condition_variable>
 #endif
 
-bool AbortNode(const std::string& strMessage, bilingual_str user_message)
+bool AbortNode(const std::string& strMessage, const std::function<void(const bilingual_str& user_message)>& init_error, 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);
+    init_error(user_message);
     StartShutdown();
     return false;
 }
diff --git a/src/shutdown.h b/src/shutdown.h
@@ -9,7 +9,7 @@
 #include <util/translation.h> // For bilingual_str
 
 /** Abort with a message */
-bool AbortNode(const std::string& strMessage, bilingual_str user_message = bilingual_str{});
+bool AbortNode(const std::string& strMessage, const std::function<void(const bilingual_str& user_message)>& init_error, 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.
diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp
@@ -4,15 +4,17 @@
 
 #include <chainparams.h>
 #include <node/blockstorage.h>
+#include <node/chainstatemanager_notifications.h>
 #include <node/context.h>
 #include <util/chaintype.h>
 #include <validation.h>
 
 #include <boost/test/unit_test.hpp>
 #include <test/util/setup_common.h>
 
-using node::BlockManager;
 using node::BLOCK_SERIALIZATION_HEADER_SIZE;
+using node::BlockManager;
+using node::DefaultChainstateManagerNotifications;
 using node::MAX_BLOCKFILE_SIZE;
 
 // use BasicTestingSetup here for the data directory configuration, setup, and cleanup
@@ -24,6 +26,7 @@ BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos)
     const BlockManager::Options blockman_opts{
         .chainparams = *params,
         .blocks_dir = m_args.GetBlocksDirPath(),
+        .init_error_callback = DefaultChainstateManagerNotifications().init_error,
     };
     BlockManager blockman{blockman_opts};
     CChain chain {};
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
@@ -193,6 +193,7 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vecto
     const BlockManager::Options blockman_opts{
         .chainparams = chainman_opts.chainparams,
         .blocks_dir = m_args.GetBlocksDirPath(),
+        .init_error_callback = chainman_opts.notification_callbacks.init_error,
     };
     m_node.chainman = std::make_unique<ChainstateManager>(chainman_opts, blockman_opts);
     m_node.chainman->m_blockman.m_block_tree_db = std::make_unique<CBlockTreeDB>(DBParams{
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
@@ -388,6 +388,7 @@ struct SnapshotTestSetup : TestChain100Setup {
             const BlockManager::Options blockman_opts{
                 .chainparams = chainman_opts.chainparams,
                 .blocks_dir = m_args.GetBlocksDirPath(),
+                .init_error_callback = chainman_opts.notification_callbacks.init_error,
             };
             // For robustness, ensure the old manager is destroyed before creating a
             // new one.
diff --git a/src/validation.cpp b/src/validation.cpp
diff --git a/src/validation.h b/src/validation.h

Original file line number	Diff line number	Diff line change
`@@ -527,7 +527,7 @@ void BlockManager::FlushUndoFile(int block_file, bool finalize)`
`527`	`527`	`{`
`528`	`528`	`FlatFilePos undo_pos_old(block_file, m_blockfile_info[block_file].nUndoSize);`
`529`	`529`	`if (!UndoFileSeq().Flush(undo_pos_old, finalize)) {`
`530`		`- AbortNode("Flushing undo file to disk failed. This is likely the result of an I/O error.");`
	`530`	`+ AbortNode("Flushing undo file to disk failed. This is likely the result of an I/O error.", m_opts.init_error_callback);`
`531`	`531`	`}`
`532`	`532`	`}`
`533`	`533`
`@@ -546,7 +546,7 @@ void BlockManager::FlushBlockFile(bool fFinalize, bool finalize_undo)`
`546`	`546`
`547`	`547`	`FlatFilePos block_pos_old(m_last_blockfile, m_blockfile_info[m_last_blockfile].nSize);`
`548`	`548`	`if (!BlockFileSeq().Flush(block_pos_old, fFinalize)) {`
`549`		`- AbortNode("Flushing block file to disk failed. This is likely the result of an I/O error.");`
	`549`	`+ AbortNode("Flushing block file to disk failed. This is likely the result of an I/O error.", m_opts.init_error_callback);`
`550`	`550`	`}`
`551`	`551`	`// we do not always flush the undo file, as the chain tip may be lagging behind the incoming blocks,`
`552`	`552`	`// e.g. during IBD or a sync after a node going offline`
`@@ -658,7 +658,7 @@ bool BlockManager::FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigne`
`658`	`658`	`bool out_of_space;`
`659`	`659`	`size_t bytes_allocated = BlockFileSeq().Allocate(pos, nAddSize, out_of_space);`
`660`	`660`	`if (out_of_space) {`
`661`		`- return AbortNode("Disk space is too low!", _("Disk space is too low!"));`
	`661`	`+ return AbortNode("Disk space is too low!", m_opts.init_error_callback, _("Disk space is too low!"));`
`662`	`662`	`}`
`663`	`663`	`if (bytes_allocated != 0 && IsPruneMode()) {`
`664`	`664`	`m_check_for_pruning = true;`
`@@ -682,7 +682,7 @@ bool BlockManager::FindUndoPos(BlockValidationState& state, int nFile, FlatFileP`
`682`	`682`	`bool out_of_space;`
`683`	`683`	`size_t bytes_allocated = UndoFileSeq().Allocate(pos, nAddSize, out_of_space);`
`684`	`684`	`if (out_of_space) {`
`685`		`- return AbortNode(state, "Disk space is too low!", _("Disk space is too low!"));`
	`685`	`+ return AbortNode(state, "Disk space is too low!", m_opts.init_error_callback, _("Disk space is too low!"));`
`686`	`686`	`}`
`687`	`687`	`if (bytes_allocated != 0 && IsPruneMode()) {`
`688`	`688`	`m_check_for_pruning = true;`
`@@ -724,7 +724,7 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid`
`724`	`724`	`return error("ConnectBlock(): FindUndoPos failed");`
`725`	`725`	`}`
`726`	`726`	`if (!UndoWriteToDisk(blockundo, _pos, block.pprev->GetBlockHash(), GetParams().MessageStart())) {`
`727`		`- return AbortNode(state, "Failed to write undo data");`
	`727`	`+ return AbortNode(state, "Failed to write undo data", m_opts.init_error_callback);`
`728`	`728`	`}`
`729`	`729`	`// rev files are written in block height order, whereas blk files are written as blocks come in (often out of order)`
`730`	`730`	`// we want to flush the rev (undo) file once we've written the last block, which is indicated by the last height`
`@@ -842,7 +842,7 @@ FlatFilePos BlockManager::SaveBlockToDisk(const CBlock& block, int nHeight, CCha`
`842`	`842`	`}`
`843`	`843`	`if (!position_known) {`
`844`	`844`	`if (!WriteBlockToDisk(block, blockPos, GetParams().MessageStart())) {`
`845`		`- AbortNode("Failed to write block");`
	`845`	`+ AbortNode("Failed to write block", m_opts.init_error_callback);`
`846`	`846`	`return FlatFilePos();`
`847`	`847`	`}`
`848`	`848`	`}`
Original file line number	Diff line number	Diff line change
`@@ -65,6 +65,7 @@ ChainstateManagerNotificationCallbacks DefaultChainstateManagerNotifications()`
`65`	`65`	`.notify_header_tip = [](SynchronizationState state, int64_t height, int64_t timestamp, bool presync) { uiInterface.NotifyHeaderTip(state, height, timestamp, presync); },`
`66`	`66`	`.show_progress = [](const std::string& title, int nProgress, bool resume_possible) { uiInterface.ShowProgress(title, nProgress, resume_possible); },`
`67`	`67`	`.do_warning = [](const bilingual_str& warning) { DoWarning(warning); },`
	`68`	`+ .init_error = [](const bilingual_str& user_message) { InitError(user_message); },`
`68`	`69`	`};`
`69`	`70`	`}`
`70`	`71`	`} // namespace node`