Merge bitcoin/bitcoin#21526: validation: UpdateTip/CheckBlockIndex assumeutxo support

673a5bd337 test: validation: add unittest for UpdateTip behavior (James O'Beirne)
2705570109 test: refactor: separate CreateBlock in TestChain100Setup (James O'Beirne)
298bf5d563 test: refactor: declare NoMalleation const auto (James O'Beirne)
071200993f move-only: unittest: add test/util/chainstate.h (James O'Beirne)
8f5710fd0a validation: fix CheckBlockIndex for multiple chainstates (James O'Beirne)
5a807736da validation: insert assumed-valid block index entries into candidates (James O'Beirne)
01a9b8fe71 validation: set BLOCK_ASSUMED_VALID during snapshot load (James O'Beirne)
42b2520db9 chain: add BLOCK_ASSUMED_VALID for use with assumeutxo (James O'Beirne)
b217020df7 validation: change UpdateTip for multiple chainstates (James O'Beirne)
665072a36d doc: add comment for g_best_block (James O'Beirne)
ac4051d891 refactor: remove unused assumeutxo methods (James O'Beirne)
9f6bb53935 validation: add chainman ref to CChainState (James O'Beirne)

Pull request description:

  This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: #15606)

  ---

  Modify UpdateTip and CheckBlockIndex for use with multiple chainstates. Includes a new unittest verifying `g_best_block` behavior (previously untested at the unit level) and various changes necessary for running and testing `ProcessNewBlock()`-like behavior on the background validation chainstate.

  This changeset introduces a new block index `nStatus` flag called `BLOCK_ASSUMED_VALID`, and it is applied to block index entries that are beneath the UTXO snapshot base block upon snapshot load. Once each block is validated (during async background validation), the flag is removed. This allows us to avoid (ab)using `BLOCK_VALID_*` flags for snapshot chain block entries, and preserves the original meaning of those flags.

  Note: this PR previously incorporated changes to `LoadBlockIndex()` and `RewindBlockIndex()` as noted in Russ' comments below, but once I generated the changes necessary to test the UpdateTip change, I decided to split this changes out into another PR due to the size of this one.

ACKs for top commit:
  achow101:
    ACK 673a5bd337
  jonatack:
    Code-review re-ACK 673a5bd337 reviewed diff, rebased to master/debug build/ran unit+functional tests
  naumenkogs:
    ACK 673a5bd337
  fjahr:
    Code review ACK 673a5bd337
  ariard:
    utACK 673a5bd3
  ryanofsky:
    Code review ACK 673a5bd337. Just linker fix and split commit changes mentioned https://github.com/bitcoin/bitcoin/pull/21526#issuecomment-921064563 since last review
  benthecarman:
    ACK 673a5bd337

Tree-SHA512: 0a6dc23d041b27ed9fd0ee1f3e5971b92fb1d2df2fc9b655d5dc48594235321ab1798d06de2ec55482ac3966a9ed56de8d56e9e29cae75bbe8690bafc2dda383
This commit is contained in:
W. J. van der Laan
2021-09-23 21:07:32 +02:00
10 changed files with 314 additions and 119 deletions

View File

@@ -1095,10 +1095,15 @@ void CoinsViews::InitCache()
m_cacheview = std::make_unique<CCoinsViewCache>(&m_catcherview);
}
CChainState::CChainState(CTxMemPool* mempool, BlockManager& blockman, std::optional<uint256> from_snapshot_blockhash)
CChainState::CChainState(
CTxMemPool* mempool,
BlockManager& blockman,
ChainstateManager& chainman,
std::optional<uint256> from_snapshot_blockhash)
: m_mempool(mempool),
m_params(::Params()),
m_blockman(blockman),
m_chainman(chainman),
m_from_snapshot_blockhash(from_snapshot_blockhash) {}
void CChainState::InitCoinsDB(
@@ -2090,8 +2095,42 @@ static void AppendWarning(bilingual_str& res, const bilingual_str& warn)
res += warn;
}
static void UpdateTipLog(
const CCoinsViewCache& coins_tip,
const CBlockIndex* tip,
const CChainParams& params,
const std::string& func_name,
const std::string& prefix,
const std::string& warning_messages) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
{
AssertLockHeld(::cs_main);
LogPrintf("%s%s: new best=%s height=%d version=0x%08x log2_work=%f tx=%lu date='%s' progress=%f cache=%.1fMiB(%utxo)%s\n",
prefix, func_name,
tip->GetBlockHash().ToString(), tip->nHeight, tip->nVersion,
log(tip->nChainWork.getdouble()) / log(2.0), (unsigned long)tip->nChainTx,
FormatISO8601DateTime(tip->GetBlockTime()),
GuessVerificationProgress(params.TxData(), tip),
coins_tip.DynamicMemoryUsage() * (1.0 / (1 << 20)),
coins_tip.GetCacheSize(),
!warning_messages.empty() ? strprintf(" warning='%s'", warning_messages) : "");
}
void CChainState::UpdateTip(const CBlockIndex* pindexNew)
{
const auto& coins_tip = this->CoinsTip();
// The remainder of the function isn't relevant if we are not acting on
// the active chainstate, so return if need be.
if (this != &m_chainman.ActiveChainstate()) {
// Only log every so often so that we don't bury log messages at the tip.
constexpr int BACKGROUND_LOG_INTERVAL = 2000;
if (pindexNew->nHeight % BACKGROUND_LOG_INTERVAL == 0) {
UpdateTipLog(coins_tip, pindexNew, m_params, __func__, "[background validation] ", "");
}
return;
}
// New best block
if (m_mempool) {
m_mempool->AddTransactionsUpdated(1);
@@ -2119,12 +2158,7 @@ void CChainState::UpdateTip(const CBlockIndex* pindexNew)
}
}
}
LogPrintf("%s: new best=%s height=%d version=0x%08x log2_work=%f tx=%lu date='%s' progress=%f cache=%.1fMiB(%utxo)%s\n", __func__,
pindexNew->GetBlockHash().ToString(), pindexNew->nHeight, pindexNew->nVersion,
log(pindexNew->nChainWork.getdouble())/log(2.0), (unsigned long)pindexNew->nChainTx,
FormatISO8601DateTime(pindexNew->GetBlockTime()),
GuessVerificationProgress(m_params.TxData(), pindexNew), this->CoinsTip().DynamicMemoryUsage() * (1.0 / (1<<20)), this->CoinsTip().GetCacheSize(),
!warning_messages.empty() ? strprintf(" warning='%s'", warning_messages.original) : "");
UpdateTipLog(coins_tip, pindexNew, m_params, __func__, "", warning_messages.original);
}
/** Disconnect m_chain's tip.
@@ -3621,7 +3655,9 @@ bool BlockManager::LoadBlockIndex(
pindex->nStatus |= BLOCK_FAILED_CHILD;
setDirtyBlockIndex.insert(pindex);
}
if (pindex->IsValid(BLOCK_VALID_TRANSACTIONS) && (pindex->HaveTxsDownloaded() || pindex->pprev == nullptr)) {
if (pindex->IsAssumedValid() ||
(pindex->IsValid(BLOCK_VALID_TRANSACTIONS) &&
(pindex->HaveTxsDownloaded() || pindex->pprev == nullptr))) {
block_index_candidates.insert(pindex);
}
if (pindex->nStatus & BLOCK_FAILED_MASK && (!pindexBestInvalid || pindex->nChainWork > pindexBestInvalid->nChainWork))
@@ -4200,12 +4236,33 @@ void CChainState::CheckBlockIndex()
while (pindex != nullptr) {
nNodes++;
if (pindexFirstInvalid == nullptr && pindex->nStatus & BLOCK_FAILED_VALID) pindexFirstInvalid = pindex;
if (pindexFirstMissing == nullptr && !(pindex->nStatus & BLOCK_HAVE_DATA)) pindexFirstMissing = pindex;
// Assumed-valid index entries will not have data since we haven't downloaded the
// full block yet.
if (pindexFirstMissing == nullptr && !(pindex->nStatus & BLOCK_HAVE_DATA) && !pindex->IsAssumedValid()) {
pindexFirstMissing = pindex;
}
if (pindexFirstNeverProcessed == nullptr && pindex->nTx == 0) pindexFirstNeverProcessed = pindex;
if (pindex->pprev != nullptr && pindexFirstNotTreeValid == nullptr && (pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_TREE) pindexFirstNotTreeValid = pindex;
if (pindex->pprev != nullptr && pindexFirstNotTransactionsValid == nullptr && (pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_TRANSACTIONS) pindexFirstNotTransactionsValid = pindex;
if (pindex->pprev != nullptr && pindexFirstNotChainValid == nullptr && (pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_CHAIN) pindexFirstNotChainValid = pindex;
if (pindex->pprev != nullptr && pindexFirstNotScriptsValid == nullptr && (pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_SCRIPTS) pindexFirstNotScriptsValid = pindex;
if (pindex->pprev != nullptr && !pindex->IsAssumedValid()) {
// Skip validity flag checks for BLOCK_ASSUMED_VALID index entries, since these
// *_VALID_MASK flags will not be present for index entries we are temporarily assuming
// valid.
if (pindexFirstNotTransactionsValid == nullptr &&
(pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_TRANSACTIONS) {
pindexFirstNotTransactionsValid = pindex;
}
if (pindexFirstNotChainValid == nullptr &&
(pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_CHAIN) {
pindexFirstNotChainValid = pindex;
}
if (pindexFirstNotScriptsValid == nullptr &&
(pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_SCRIPTS) {
pindexFirstNotScriptsValid = pindex;
}
}
// Begin: actual consistency checks.
if (pindex->pprev == nullptr) {
@@ -4216,7 +4273,9 @@ void CChainState::CheckBlockIndex()
if (!pindex->HaveTxsDownloaded()) assert(pindex->nSequenceId <= 0); // nSequenceId can't be set positive for blocks that aren't linked (negative is used for preciousblock)
// VALID_TRANSACTIONS is equivalent to nTx > 0 for all nodes (whether or not pruning has occurred).
// HAVE_DATA is only equivalent to nTx > 0 (or VALID_TRANSACTIONS) if no pruning has occurred.
if (!fHavePruned) {
// Unless these indexes are assumed valid and pending block download on a
// background chainstate.
if (!fHavePruned && !pindex->IsAssumedValid()) {
// If we've never pruned, then HAVE_DATA should be equivalent to nTx > 0
assert(!(pindex->nStatus & BLOCK_HAVE_DATA) == (pindex->nTx == 0));
assert(pindexFirstMissing == pindexFirstNeverProcessed);
@@ -4225,7 +4284,16 @@ void CChainState::CheckBlockIndex()
if (pindex->nStatus & BLOCK_HAVE_DATA) assert(pindex->nTx > 0);
}
if (pindex->nStatus & BLOCK_HAVE_UNDO) assert(pindex->nStatus & BLOCK_HAVE_DATA);
assert(((pindex->nStatus & BLOCK_VALID_MASK) >= BLOCK_VALID_TRANSACTIONS) == (pindex->nTx > 0)); // This is pruning-independent.
if (pindex->IsAssumedValid()) {
// Assumed-valid blocks should have some nTx value.
assert(pindex->nTx > 0);
// Assumed-valid blocks should connect to the main chain.
assert((pindex->nStatus & BLOCK_VALID_MASK) >= BLOCK_VALID_TREE);
} else {
// Otherwise there should only be an nTx value if we have
// actually seen a block's transactions.
assert(((pindex->nStatus & BLOCK_VALID_MASK) >= BLOCK_VALID_TRANSACTIONS) == (pindex->nTx > 0)); // This is pruning-independent.
}
// All parents having had data (at some point) is equivalent to all parents being VALID_TRANSACTIONS, which is equivalent to HaveTxsDownloaded().
assert((pindexFirstNeverProcessed == nullptr) == pindex->HaveTxsDownloaded());
assert((pindexFirstNotTransactionsValid == nullptr) == pindex->HaveTxsDownloaded());
@@ -4242,11 +4310,17 @@ void CChainState::CheckBlockIndex()
}
if (!CBlockIndexWorkComparator()(pindex, m_chain.Tip()) && pindexFirstNeverProcessed == nullptr) {
if (pindexFirstInvalid == nullptr) {
const bool is_active = this == &m_chainman.ActiveChainstate();
// If this block sorts at least as good as the current tip and
// is valid and we have all data for its parents, it must be in
// setBlockIndexCandidates. m_chain.Tip() must also be there
// even if some data has been pruned.
if (pindexFirstMissing == nullptr || pindex == m_chain.Tip()) {
//
// Don't perform this check for the background chainstate since
// its setBlockIndexCandidates shouldn't have some entries (i.e. those past the
// snapshot block) which do exist in the block index for the active chainstate.
if (is_active && (pindexFirstMissing == nullptr || pindex == m_chain.Tip())) {
assert(setBlockIndexCandidates.count(pindex));
}
// If some parent is missing, then it could be that this block was in
@@ -4581,7 +4655,7 @@ CChainState& ChainstateManager::InitializeChainstate(
if (to_modify) {
throw std::logic_error("should not be overwriting a chainstate");
}
to_modify.reset(new CChainState(mempool, m_blockman, snapshot_blockhash));
to_modify.reset(new CChainState(mempool, m_blockman, *this, snapshot_blockhash));
// Snapshot chainstates and initial IBD chaintates always become active.
if (is_snapshot || (!is_snapshot && !m_active_chainstate)) {
@@ -4650,8 +4724,9 @@ bool ChainstateManager::ActivateSnapshot(
static_cast<size_t>(current_coinsdb_cache_size * IBD_CACHE_PERC));
}
auto snapshot_chainstate = WITH_LOCK(::cs_main, return std::make_unique<CChainState>(
/* mempool */ nullptr, m_blockman, base_blockhash));
auto snapshot_chainstate = WITH_LOCK(::cs_main,
return std::make_unique<CChainState>(
/* mempool */ nullptr, m_blockman, *this, base_blockhash));
{
LOCK(::cs_main);
@@ -4854,11 +4929,25 @@ bool ChainstateManager::PopulateAndValidateSnapshot(
// Fake nChainTx so that GuessVerificationProgress reports accurately
index->nChainTx = index->pprev ? index->pprev->nChainTx + index->nTx : 1;
// Mark unvalidated block index entries beneath the snapshot base block as assumed-valid.
if (!index->IsValid(BLOCK_VALID_SCRIPTS)) {
// This flag will be removed once the block is fully validated by a
// background chainstate.
index->nStatus |= BLOCK_ASSUMED_VALID;
}
// Fake BLOCK_OPT_WITNESS so that CChainState::NeedsRedownload()
// won't ask to rewind the entire assumed-valid chain on startup.
if (index->pprev && DeploymentActiveAt(*index, ::Params().GetConsensus(), Consensus::DEPLOYMENT_SEGWIT)) {
index->nStatus |= BLOCK_OPT_WITNESS;
}
setDirtyBlockIndex.insert(index);
// Changes to the block index will be flushed to disk after this call
// returns in `ActivateSnapshot()`, when `MaybeRebalanceCaches()` is
// called, since we've added a snapshot chainstate and therefore will
// have to downsize the IBD chainstate, which will result in a call to
// `FlushStateToDisk(ALWAYS)`.
}
assert(index);
@@ -4883,22 +4972,6 @@ bool ChainstateManager::IsSnapshotActive() const
return m_snapshot_chainstate && m_active_chainstate == m_snapshot_chainstate.get();
}
CChainState& ChainstateManager::ValidatedChainstate() const
{
LOCK(::cs_main);
if (m_snapshot_chainstate && IsSnapshotValidated()) {
return *m_snapshot_chainstate.get();
}
assert(m_ibd_chainstate);
return *m_ibd_chainstate.get();
}
bool ChainstateManager::IsBackgroundIBD(CChainState* chainstate) const
{
LOCK(::cs_main);
return (m_snapshot_chainstate && chainstate == m_ibd_chainstate.get());
}
void ChainstateManager::Unload()
{
for (CChainState* chainstate : this->GetAll()) {