Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#25308: refactor: Reduce number of LoadChainstat…
Browse files Browse the repository at this point in the history 
…e parameters and return values

1e761a0 ci: Enable IWYU in src/kernel directory (Ryan Ofsky)
6db6552 refactor: Reduce number of SanityChecks return values (Ryan Ofsky)
b3e7de7 refactor: Reduce number of LoadChainstate return values (Russell Yanofsky)
3b91d4b refactor: Reduce number of LoadChainstate parameters (Russell Yanofsky)

Pull request description:

  Replace long LoadChainstate parameters list with options struct. Replace long list of return values with simpler error strings.

  No changes in behavior. Motivation is just to make libbitcoin_kernel API easier to use and more future-proof, and make internal code clearer and more maintainable.

ACKs for top commit:
  MarcoFalke:
    ACK 1e761a0 🕚

Tree-SHA512: 86f251ab820ca6664ade87ccac8330f79b0e48e26b98082f022f592ed1380f8eefc3cce260b85d5eea5d2f5f2531602e03d641e579c15684ecd9093b2aebcc58
  • Loading branch information
MacroFake committed Jul 20, 2022
2 parents 5560682 + 1e761a0 commit 0897b18
Show file tree
Hide file tree
Showing 10 changed files with 168 additions and 263 deletions.
2 changes: 1 addition & 1 deletion ci/test/06_script_b.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ if [ "${RUN_TIDY}" = "true" ]; then
" src/compat"\
" src/dbwrapper.cpp"\
" src/init"\
" src/kernel/mempool_persist.cpp"\
" src/kernel"\
" src/node/chainstate.cpp"\
" src/policy/feerate.cpp"\
" src/policy/packages.cpp"\
Expand Down
29 changes: 11 additions & 18 deletions src/bitcoin-chainstate.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <consensus/validation.h>
#include <core_io.h>
#include <node/blockstorage.h>
#include <node/caches.h>
#include <node/chainstate.h>
#include <scheduler.h>
#include <script/sigcache.h>
Expand Down Expand Up @@ -83,27 +84,19 @@ int main(int argc, char* argv[])
};
ChainstateManager chainman{chainman_opts};

auto rv = node::LoadChainstate(false,
std::ref(chainman),
nullptr,
false,
false,
2 << 20,
2 << 22,
(450 << 20) - (2 << 20) - (2 << 22),
false,
false,
[]() { return false; });
if (rv.has_value()) {
node::CacheSizes cache_sizes;
cache_sizes.block_tree_db = 2 << 20;
cache_sizes.coins_db = 2 << 22;
cache_sizes.coins = (450 << 20) - (2 << 20) - (2 << 22);
node::ChainstateLoadOptions options;
options.check_interrupt = [] { return false; };
auto [status, error] = node::LoadChainstate(chainman, cache_sizes, options);
if (status != node::ChainstateLoadStatus::SUCCESS) {
std::cerr << "Failed to load Chain state from your datadir." << std::endl;
goto epilogue;
} else {
auto maybe_verify_error = node::VerifyLoadedChainstate(std::ref(chainman),
false,
false,
DEFAULT_CHECKBLOCKS,
DEFAULT_CHECKLEVEL);
if (maybe_verify_error.has_value()) {
std::tie(status, error) = node::VerifyLoadedChainstate(chainman, options);
if (status != node::ChainstateLoadStatus::SUCCESS) {
std::cerr << "Failed to verify loaded Chain state from your datadir." << std::endl;
goto epilogue;
}
Expand Down
145 changes: 36 additions & 109 deletions src/init.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,6 @@ using kernel::DumpMempool;

using node::CacheSizes;
using node::CalculateCacheSizes;
using node::ChainstateLoadVerifyError;
using node::ChainstateLoadingError;
using node::DEFAULT_PERSIST_MEMPOOL;
using node::DEFAULT_PRINTPRIORITY;
using node::DEFAULT_STOPAFTERBLOCKIMPORT;
Expand Down Expand Up @@ -1098,21 +1096,8 @@ static bool LockDataDirectory(bool probeOnly)
bool AppInitSanityChecks(const kernel::Context& kernel)
{
// ********************************************************* Step 4: sanity checks
auto maybe_error = kernel::SanityChecks(kernel);

if (maybe_error.has_value()) {
switch (maybe_error.value()) {
case kernel::SanityCheckError::ERROR_ECC:
InitError(Untranslated("Elliptic curve cryptography sanity check failure. Aborting."));
break;
case kernel::SanityCheckError::ERROR_RANDOM:
InitError(Untranslated("OS cryptographic RNG sanity check failure. Aborting."));
break;
case kernel::SanityCheckError::ERROR_CHRONO:
InitError(Untranslated("Clock epoch mismatch. Aborting."));
break;
} // no default case, so the compiler can warn about missing cases

if (auto error = kernel::SanityChecks(kernel)) {
InitError(*error);
return InitError(strprintf(_("Initialization sanity check failed. %s is shutting down."), PACKAGE_NAME));
}

Expand Down Expand Up @@ -1452,112 +1437,54 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
node.chainman = std::make_unique<ChainstateManager>(chainman_opts);
ChainstateManager& chainman = *node.chainman;

const bool fReset = fReindex;
bilingual_str strLoadError;
node::ChainstateLoadOptions options;
options.mempool = Assert(node.mempool.get());
options.reindex = node::fReindex;
options.reindex_chainstate = fReindexChainState;
options.prune = node::fPruneMode;
options.check_blocks = args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS);
options.check_level = args.GetIntArg("-checklevel", DEFAULT_CHECKLEVEL);
options.check_interrupt = ShutdownRequested;
options.coins_error_cb = [] {
uiInterface.ThreadSafeMessageBox(
_("Error reading from database, shutting down."),
"", CClientUIInterface::MSG_ERROR);
};

uiInterface.InitMessage(_("Loading block index…").translated);
const int64_t load_block_index_start_time = GetTimeMillis();
std::optional<ChainstateLoadingError> maybe_load_error;
try {
maybe_load_error = LoadChainstate(fReset,
chainman,
Assert(node.mempool.get()),
fPruneMode,
fReindexChainState,
cache_sizes.block_tree_db,
cache_sizes.coins_db,
cache_sizes.coins,
/*block_tree_db_in_memory=*/false,
/*coins_db_in_memory=*/false,
/*shutdown_requested=*/ShutdownRequested,
/*coins_error_cb=*/[]() {
uiInterface.ThreadSafeMessageBox(
_("Error reading from database, shutting down."),
"", CClientUIInterface::MSG_ERROR);
});
} catch (const std::exception& e) {
LogPrintf("%s\n", e.what());
maybe_load_error = ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED;
}
if (maybe_load_error.has_value()) {
switch (maybe_load_error.value()) {
case ChainstateLoadingError::ERROR_LOADING_BLOCK_DB:
strLoadError = _("Error loading block database");
break;
case ChainstateLoadingError::ERROR_BAD_GENESIS_BLOCK:
// If the loaded chain has a wrong genesis, bail out immediately
// (we're likely using a testnet datadir, or the other way around).
return InitError(_("Incorrect or no genesis block found. Wrong datadir for network?"));
case ChainstateLoadingError::ERROR_PRUNED_NEEDS_REINDEX:
strLoadError = _("You need to rebuild the database using -reindex to go back to unpruned mode. This will redownload the entire blockchain");
break;
case ChainstateLoadingError::ERROR_LOAD_GENESIS_BLOCK_FAILED:
strLoadError = _("Error initializing block database");
break;
case ChainstateLoadingError::ERROR_CHAINSTATE_UPGRADE_FAILED:
return InitError(_("Unsupported chainstate database format found. "
"Please restart with -reindex-chainstate. This will "
"rebuild the chainstate database."));
case ChainstateLoadingError::ERROR_REPLAYBLOCKS_FAILED:
strLoadError = _("Unable to replay blocks. You will need to rebuild the database using -reindex-chainstate.");
break;
case ChainstateLoadingError::ERROR_LOADCHAINTIP_FAILED:
strLoadError = _("Error initializing block database");
break;
case ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED:
strLoadError = _("Error opening block database");
break;
case ChainstateLoadingError::ERROR_BLOCKS_WITNESS_INSUFFICIENTLY_VALIDATED:
strLoadError = strprintf(_("Witness data for blocks after height %d requires validation. Please restart with -reindex."),
chainman.GetConsensus().SegwitHeight);
break;
case ChainstateLoadingError::SHUTDOWN_PROBED:
break;
}
} else {
std::optional<ChainstateLoadVerifyError> maybe_verify_error;
auto catch_exceptions = [](auto&& f) {
try {
uiInterface.InitMessage(_("Verifying blocks…").translated);
auto check_blocks = args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS);
if (chainman.m_blockman.m_have_pruned && check_blocks > MIN_BLOCKS_TO_KEEP) {
LogPrintfCategory(BCLog::PRUNE, "pruned datadir may not have more than %d blocks; only checking available blocks\n",
MIN_BLOCKS_TO_KEEP);
}
maybe_verify_error = VerifyLoadedChainstate(chainman,
fReset,
fReindexChainState,
check_blocks,
args.GetIntArg("-checklevel", DEFAULT_CHECKLEVEL));
return f();
} catch (const std::exception& e) {
LogPrintf("%s\n", e.what());
maybe_verify_error = ChainstateLoadVerifyError::ERROR_GENERIC_FAILURE;
return std::make_tuple(node::ChainstateLoadStatus::FAILURE, _("Error opening block database"));
}
if (maybe_verify_error.has_value()) {
switch (maybe_verify_error.value()) {
case ChainstateLoadVerifyError::ERROR_BLOCK_FROM_FUTURE:
strLoadError = _("The block database contains a block which appears to be from the future. "
"This may be due to your computer's date and time being set incorrectly. "
"Only rebuild the block database if you are sure that your computer's date and time are correct");
break;
case ChainstateLoadVerifyError::ERROR_CORRUPTED_BLOCK_DB:
strLoadError = _("Corrupted block database detected");
break;
case ChainstateLoadVerifyError::ERROR_GENERIC_FAILURE:
strLoadError = _("Error opening block database");
break;
}
} else {
};
auto [status, error] = catch_exceptions([&]{ return LoadChainstate(chainman, cache_sizes, options); });
if (status == node::ChainstateLoadStatus::SUCCESS) {
uiInterface.InitMessage(_("Verifying blocks…").translated);
if (chainman.m_blockman.m_have_pruned && options.check_blocks > MIN_BLOCKS_TO_KEEP) {
LogPrintfCategory(BCLog::PRUNE, "pruned datadir may not have more than %d blocks; only checking available blocks\n",
MIN_BLOCKS_TO_KEEP);
}
std::tie(status, error) = catch_exceptions([&]{ return VerifyLoadedChainstate(chainman, options);});
if (status == node::ChainstateLoadStatus::SUCCESS) {
fLoaded = true;
LogPrintf(" block index %15dms\n", GetTimeMillis() - load_block_index_start_time);
}
}

if (status == node::ChainstateLoadStatus::FAILURE_INCOMPATIBLE_DB) {
return InitError(error);
}

if (!fLoaded && !ShutdownRequested()) {
// first suggest a reindex
if (!fReset) {
if (!options.reindex) {
bool fRet = uiInterface.ThreadSafeQuestion(
strLoadError + Untranslated(".\n\n") + _("Do you want to rebuild the block database now?"),
strLoadError.original + ".\nPlease restart with -reindex or -reindex-chainstate to recover.",
error + Untranslated(".\n\n") + _("Do you want to rebuild the block database now?"),
error.original + ".\nPlease restart with -reindex or -reindex-chainstate to recover.",
"", CClientUIInterface::MSG_ERROR | CClientUIInterface::BTN_ABORT);
if (fRet) {
fReindex = true;
Expand All @@ -1567,7 +1494,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
return false;
}
} else {
return InitError(strLoadError);
return InitError(error);
}
}
}
Expand Down
11 changes: 7 additions & 4 deletions src/kernel/checks.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,24 @@
#include <key.h>
#include <random.h>
#include <util/time.h>
#include <util/translation.h>

#include <memory>

namespace kernel {

std::optional<SanityCheckError> SanityChecks(const Context&)
std::optional<bilingual_str> SanityChecks(const Context&)
{
if (!ECC_InitSanityCheck()) {
return SanityCheckError::ERROR_ECC;
return Untranslated("Elliptic curve cryptography sanity check failure. Aborting.");
}

if (!Random_SanityCheck()) {
return SanityCheckError::ERROR_RANDOM;
return Untranslated("OS cryptographic RNG sanity check failure. Aborting.");
}

if (!ChronoSanityCheck()) {
return SanityCheckError::ERROR_CHRONO;
return Untranslated("Clock epoch mismatch. Aborting.");
}

return std::nullopt;
Expand Down
10 changes: 3 additions & 7 deletions src/kernel/checks.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,16 @@

#include <optional>

struct bilingual_str;

namespace kernel {

struct Context;

enum class SanityCheckError {
ERROR_ECC,
ERROR_RANDOM,
ERROR_CHRONO,
};

/**
* Ensure a usable environment with all necessary library support.
*/
std::optional<SanityCheckError> SanityChecks(const Context&);
std::optional<bilingual_str> SanityChecks(const Context&);

}

Expand Down
16 changes: 16 additions & 0 deletions src/kernel/coinstats.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,32 @@

#include <kernel/coinstats.h>

#include <chain.h>
#include <coins.h>
#include <crypto/muhash.h>
#include <hash.h>
#include <node/blockstorage.h>
#include <primitives/transaction.h>
#include <script/script.h>
#include <serialize.h>
#include <span.h>
#include <streams.h>
#include <sync.h>
#include <tinyformat.h>
#include <uint256.h>
#include <util/check.h>
#include <util/overflow.h>
#include <util/system.h>
#include <validation.h>
#include <version.h>

#include <cassert>
#include <iosfwd>
#include <iterator>
#include <map>
#include <memory>
#include <string>
#include <utility>

namespace kernel {

Expand Down
6 changes: 4 additions & 2 deletions src/kernel/coinstats.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,18 @@
#ifndef BITCOIN_KERNEL_COINSTATS_H
#define BITCOIN_KERNEL_COINSTATS_H

#include <chain.h>
#include <coins.h>
#include <consensus/amount.h>
#include <streams.h>
#include <uint256.h>

#include <cstdint>
#include <functional>
#include <optional>

class CCoinsView;
class Coin;
class COutPoint;
class CScript;
namespace node {
class BlockManager;
} // namespace node
Expand Down
Loading

0 comments on commit 0897b18

Please sign in to comment.