mirrors/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-22 06:52:36 +01:00
Code Issues Projects Releases Wiki Activity

Merge bitcoin/bitcoin#23855: refactor: Post-"Chainstate loading sequence coalescence" fixups

Browse source 
e3544c864e init: Use clang-tidy named args syntax (Carl Dong)
3401630417 style-only: Rename *Chainstate return values (Carl Dong)
1dd582782d docs: Make LoadChainstate comment more accurate (Carl Dong)
6b83576388 node/chainstate: Use MAX_FUTURE_BLOCK_TIME (Carl Dong)

Pull request description:

  There are 2 proposed fixups in discussions in #23280 which I have not implemented:

  1. An overhaul to return types and an option type for the two `*Chainstate` functions: https://github.com/bitcoin/bitcoin/pull/23280#issuecomment-984149564
      - The change reintroduces stringy return types and is quite involved. It could be discussed in a separate PR.
  2. Passing in the unix time to `VerifyChainstate` instead of a callback to get the time: https://github.com/bitcoin/bitcoin/pull/23280#discussion_r765051533
      - I'm not sure it matters much whether it's a callback or just the actual unix time. Also, I think `VerifyDB` can take quite a while, and I don't want to impose that the function have to "run quickly" in order to have it be correct.

  If reviewers feel strongly about either of the two fixups listed above, please feel free to open a PR based on mine and I'll close this one!

ACKs for top commit:
  ryanofsky:
    Code review ACK e3544c864e
  MarcoFalke:
    ACK e3544c864e 🐸

Tree-SHA512: dd1de0265b6785eef306e724b678ce03d7c54ea9f4b5ea0ccd7af59cce2ea3aba73fd4af0c15e2dca9265807dc4075f9afa2ec103672677b6638b1a4fc090904
This commit is contained in:
MarcoFalke 2022-01-06 13:55:47 +01:00
commit 3917dff732
No known key found for this signature in database
GPG key ID: CE2B75697E69A548
4 changed files with 49 additions and 49 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

64
src/init.cpp
View file

@ -1404,31 +1404,31 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
uiInterface.InitMessage(_("Loading block index…").translated);
const int64_t load_block_index_start_time = GetTimeMillis();
std::optional<ChainstateLoadingError> rv;
std::optional<ChainstateLoadingError> maybe_load_error;
try {
rv = LoadChainstate(fReset,
chainman,
Assert(node.mempool.get()),
fPruneMode,
chainparams.GetConsensus(),
fReindexChainState,
cache_sizes.block_tree_db,
cache_sizes.coins_db,
cache_sizes.coins,
false,
false,
ShutdownRequested,
[]() {
uiInterface.ThreadSafeMessageBox(
_("Error reading from database, shutting down."),
"", CClientUIInterface::MSG_ERROR);
});
maybe_load_error = LoadChainstate(fReset,
chainman,
Assert(node.mempool.get()),
fPruneMode,
chainparams.GetConsensus(),
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());
rv = ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED;
maybe_load_error = ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED;
}
if (rv.has_value()) {
switch (rv.value()) {
if (maybe_load_error.has_value()) {
switch (maybe_load_error.value()) {
case ChainstateLoadingError::ERROR_LOADING_BLOCK_DB:
strLoadError = _("Error loading block database");
break;
@ -1462,7 +1462,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
break;
}
} else {
std::optional<ChainstateLoadVerifyError> rv2;
std::optional<ChainstateLoadVerifyError> maybe_verify_error;
try {
uiInterface.InitMessage(_("Verifying blocks…").translated);
auto check_blocks = args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS);
@ -1470,19 +1470,19 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
LogPrintf("Prune: pruned datadir may not have more than %d blocks; only checking available blocks\n",
MIN_BLOCKS_TO_KEEP);
}
rv2 = VerifyLoadedChainstate(chainman,
fReset,
fReindexChainState,
chainparams.GetConsensus(),
check_blocks,
args.GetIntArg("-checklevel", DEFAULT_CHECKLEVEL),
static_cast<int64_t(*)()>(GetTime));
maybe_verify_error = VerifyLoadedChainstate(chainman,
fReset,
fReindexChainState,
chainparams.GetConsensus(),
check_blocks,
args.GetIntArg("-checklevel", DEFAULT_CHECKLEVEL),
/*get_unix_time_seconds=*/static_cast<int64_t(*)()>(GetTime));
} catch (const std::exception& e) {
LogPrintf("%s\n", e.what());
rv2 = ChainstateLoadVerifyError::ERROR_GENERIC_FAILURE;
maybe_verify_error = ChainstateLoadVerifyError::ERROR_GENERIC_FAILURE;
}
if (rv2.has_value()) {
switch (rv2.value()) {
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. "

2
src/node/chainstate.cpp
View file

@ -141,7 +141,7 @@ std::optional<ChainstateLoadVerifyError> VerifyLoadedChainstate(ChainstateManage
for (CChainState* chainstate : chainman.GetAll()) {
if (!is_coinsview_empty(chainstate)) {
const CBlockIndex* tip = chainstate->m_chain.Tip();
if (tip && tip->nTime > get_unix_time_seconds() + 2 * 60 * 60) {
if (tip && tip->nTime > get_unix_time_seconds() + MAX_FUTURE_BLOCK_TIME) {
return ChainstateLoadVerifyError::ERROR_BLOCK_FROM_FUTURE;
}

2
src/node/chainstate.h
View file

@ -50,7 +50,7 @@ enum class ChainstateLoadingError {
* this sequence, when we explicitly checked shutdown_requested() at
* arbitrary points, one of those calls returned true". Therefore, a
* return value other than SHUTDOWN_PROBED does not guarantee that
* shutdown_requested() hasn't been called indirectly.
* shutdown hasn't been called indirectly.
* - else
* - Success!
*/

30
src/test/util/setup_common.cpp
View file

@ -182,28 +182,28 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const
// instead of unit tests, but for now we need these here.
RegisterAllCoreRPCCommands(tableRPC);
auto rv = LoadChainstate(fReindex.load(),
*Assert(m_node.chainman.get()),
Assert(m_node.mempool.get()),
fPruneMode,
chainparams.GetConsensus(),
m_args.GetBoolArg("-reindex-chainstate", false),
m_cache_sizes.block_tree_db,
m_cache_sizes.coins_db,
m_cache_sizes.coins,
true,
true);
assert(!rv.has_value());
auto maybe_load_error = LoadChainstate(fReindex.load(),
*Assert(m_node.chainman.get()),
Assert(m_node.mempool.get()),
fPruneMode,
chainparams.GetConsensus(),
m_args.GetBoolArg("-reindex-chainstate", false),
m_cache_sizes.block_tree_db,
m_cache_sizes.coins_db,
m_cache_sizes.coins,
/*block_tree_db_in_memory=*/true,
/*coins_db_in_memory=*/true);
assert(!maybe_load_error.has_value());
auto maybe_verify_failure = VerifyLoadedChainstate(
auto maybe_verify_error = VerifyLoadedChainstate(
*Assert(m_node.chainman),
fReindex.load(),
m_args.GetBoolArg("-reindex-chainstate", false),
chainparams.GetConsensus(),
m_args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS),
m_args.GetIntArg("-checklevel", DEFAULT_CHECKLEVEL),
static_cast<int64_t(*)()>(GetTime));
assert(!maybe_verify_failure.has_value());
/*get_unix_time_seconds=*/static_cast<int64_t(*)()>(GetTime));
assert(!maybe_verify_error.has_value());
BlockValidationState state;
if (!m_node.chainman->ActiveChainstate().ActivateBestChain(state)) {