Skip to content

Commit

Permalink
refactor: Pass InitError as callback to ChainstateManager
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
TheCharlatan committed May 11, 2023
1 parent 2dec94e commit 63be4cc
Show file tree
Hide file tree
Showing 15 changed files with 67 additions and 42 deletions.
1 change: 0 additions & 1 deletion src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
1 change: 1 addition & 0 deletions src/bitcoin-chainstate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
4 changes: 3 additions & 1 deletion src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/kernel/blockmanager_opts.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#define BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H

#include <util/fs.h>
#include <util/translation.h>

#include <cstdint>

Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/kernel/chainstatemanager_opts.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

/**
Expand Down
12 changes: 6 additions & 6 deletions src/node/blockstorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
}
}
Expand Down
7 changes: 5 additions & 2 deletions src/node/chainstate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/node/chainstatemanager_notifications.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 2 additions & 3 deletions src/shutdown.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#endif

#include <logging.h>
#include <node/interface_ui.h>
#include <util/tokenpipe.h>
#include <warnings.h>

Expand All @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/shutdown.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 4 additions & 1 deletion src/test/blockmanager_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {};
Expand Down
1 change: 1 addition & 0 deletions src/test/util/setup_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
1 change: 1 addition & 0 deletions src/test/validation_chainstatemanager_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading

0 comments on commit 63be4cc

Please sign in to comment.