diff --git a/src/validation.cpp b/src/validation.cpp index 94d2680db749b..68c50167c4b3a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -472,6 +472,11 @@ class MemPoolAccept * policies such as mempool min fee and min relay fee. */ const bool m_package_feerates; + /** Used for local submission of transactions to catch "absurd" fees + * due to fee miscalculation by wallets. std:nullopt implies unset, allowing any feerates. + * Any individual transaction failing this check causes immediate failure. + */ + const std::optional m_client_maxfeerate; /** Parameters for single transaction mempool validation. */ static ATMPArgs SingleAccept(const CChainParams& chainparams, int64_t accept_time, @@ -485,6 +490,7 @@ class MemPoolAccept /* m_allow_replacement */ true, /* m_package_submission */ false, /* m_package_feerates */ false, + /* m_client_maxfeerate */ {}, // checked by caller }; } @@ -499,12 +505,13 @@ class MemPoolAccept /* m_allow_replacement */ false, /* m_package_submission */ false, // not submitting to mempool /* m_package_feerates */ false, + /* m_client_maxfeerate */ {}, // checked by caller }; } /** Parameters for child-with-unconfirmed-parents package validation. */ static ATMPArgs PackageChildWithParents(const CChainParams& chainparams, int64_t accept_time, - std::vector& coins_to_uncache) { + std::vector& coins_to_uncache, const std::optional& client_maxfeerate) { return ATMPArgs{/* m_chainparams */ chainparams, /* m_accept_time */ accept_time, /* m_bypass_limits */ false, @@ -513,6 +520,7 @@ class MemPoolAccept /* m_allow_replacement */ false, /* m_package_submission */ true, /* m_package_feerates */ true, + /* m_client_maxfeerate */ client_maxfeerate, }; } @@ -526,6 +534,7 @@ class MemPoolAccept /* m_allow_replacement */ true, /* m_package_submission */ true, // do not LimitMempoolSize in Finalize() /* m_package_feerates */ false, // only 1 transaction + /* m_client_maxfeerate */ package_args.m_client_maxfeerate, }; } @@ -539,7 +548,8 @@ class MemPoolAccept bool test_accept, bool allow_replacement, bool package_submission, - bool package_feerates) + bool package_feerates, + std::optional client_maxfeerate) : m_chainparams{chainparams}, m_accept_time{accept_time}, m_bypass_limits{bypass_limits}, @@ -547,7 +557,8 @@ class MemPoolAccept m_test_accept{test_accept}, m_allow_replacement{allow_replacement}, m_package_submission{package_submission}, - m_package_feerates{package_feerates} + m_package_feerates{package_feerates}, + m_client_maxfeerate{client_maxfeerate} { } }; @@ -1285,6 +1296,12 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef return MempoolAcceptResult::Failure(ws.m_state); } + // Individual modified feerate exceeded caller-defined max; abort + if (args.m_client_maxfeerate && CFeeRate(ws.m_modified_fees, ws.m_vsize) > args.m_client_maxfeerate.value()) { + ws.m_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "max feerate exceeded", ""); + return MempoolAcceptResult::Failure(ws.m_state); + } + if (m_rbf && !ReplacementChecks(ws)) return MempoolAcceptResult::Failure(ws.m_state); // Perform the inexpensive checks first and avoid hashing and signature verification unless @@ -1345,6 +1362,16 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state)); return PackageMempoolAcceptResult(package_state, std::move(results)); } + + // Individual modified feerate exceeded caller-defined max; abort + // N.B. this doesn't take into account CPFPs. Chunk-aware validation may be more robust. + if (args.m_client_maxfeerate && CFeeRate(ws.m_modified_fees, ws.m_vsize) > args.m_client_maxfeerate.value()) { + package_state.Invalid(PackageValidationResult::PCKG_TX, "max feerate exceeded"); + // Exit early to avoid doing pointless work. Update the failed tx result; the rest are unfinished. + results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state)); + return PackageMempoolAcceptResult(package_state, std::move(results)); + } + // Make the coins created by this transaction available for subsequent transactions in the // package to spend. Since we already checked conflicts in the package and we don't allow // replacements, we don't need to track the coins spent. Note that this logic will need to be @@ -1689,7 +1716,7 @@ MempoolAcceptResult AcceptToMemoryPool(Chainstate& active_chainstate, const CTra } PackageMempoolAcceptResult ProcessNewPackage(Chainstate& active_chainstate, CTxMemPool& pool, - const Package& package, bool test_accept) + const Package& package, bool test_accept, const std::optional& client_maxfeerate) { AssertLockHeld(cs_main); assert(!package.empty()); @@ -1703,7 +1730,7 @@ PackageMempoolAcceptResult ProcessNewPackage(Chainstate& active_chainstate, CTxM auto args = MemPoolAccept::ATMPArgs::PackageTestAccept(chainparams, GetTime(), coins_to_uncache); return MemPoolAccept(pool, active_chainstate).AcceptMultipleTransactions(package, args); } else { - auto args = MemPoolAccept::ATMPArgs::PackageChildWithParents(chainparams, GetTime(), coins_to_uncache); + auto args = MemPoolAccept::ATMPArgs::PackageChildWithParents(chainparams, GetTime(), coins_to_uncache, client_maxfeerate); return MemPoolAccept(pool, active_chainstate).AcceptPackage(package, args); } }(); @@ -1956,7 +1983,6 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state, uint256 hashCacheEntry; CSHA256 hasher = g_scriptExecutionCacheHasher; hasher.Write(UCharCast(tx.GetWitnessHash().begin()), 32).Write((unsigned char*)&flags, sizeof(flags)).Finalize(hashCacheEntry.begin()); - AssertLockHeld(cs_main); //TODO: Remove this requirement by making CuckooCache not require external locks if (g_scriptExecutionCache.contains(hashCacheEntry, !cacheFullScriptStore)) { return true; } @@ -2018,16 +2044,17 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state, if (cacheFullScriptStore && !pvChecks) { // We executed all of the provided scripts, and were told to // cache the result. Do so now. + AssertLockHeld(cs_main); //TODO: Remove this requirement by making CuckooCache not require external locks g_scriptExecutionCache.insert(hashCacheEntry); } return true; } -bool FatalError(Notifications& notifications, BlockValidationState& state, const std::string& strMessage, const bilingual_str& userMessage) +bool FatalError(Notifications& notifications, BlockValidationState& state, const bilingual_str& message) { - notifications.fatalError(strMessage, userMessage); - return state.Error(strMessage); + notifications.fatalError(message); + return state.Error(message.original); } /** @@ -2249,7 +2276,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, // We don't write down blocks to disk if they may have been // corrupted, so this should be impossible unless we're having hardware // problems. - return FatalError(m_chainman.GetNotifications(), state, "Corrupt block found indicating potential hardware failure; shutting down"); + return FatalError(m_chainman.GetNotifications(), state, _("Corrupt block found indicating potential hardware failure.")); } LogError("%s: Consensus::CheckBlock: %s\n", __func__, state.ToString()); return false; @@ -2675,7 +2702,7 @@ bool Chainstate::FlushStateToDisk( if (fDoFullFlush || fPeriodicWrite) { // Ensure we can write block index if (!CheckDiskSpace(m_blockman.m_opts.blocks_dir)) { - return FatalError(m_chainman.GetNotifications(), state, "Disk space is too low!", _("Disk space is too low!")); + return FatalError(m_chainman.GetNotifications(), state, _("Disk space is too low!")); } { LOG_TIME_MILLIS_WITH_CATEGORY("write block and undo data to disk", BCLog::BENCH); @@ -2693,7 +2720,7 @@ bool Chainstate::FlushStateToDisk( LOG_TIME_MILLIS_WITH_CATEGORY("write block index to disk", BCLog::BENCH); if (!m_blockman.WriteBlockIndexDB()) { - return FatalError(m_chainman.GetNotifications(), state, "Failed to write to block index database"); + return FatalError(m_chainman.GetNotifications(), state, _("Failed to write to block index database.")); } } // Finally remove any pruned files @@ -2715,11 +2742,11 @@ bool Chainstate::FlushStateToDisk( // an overestimation, as most will delete an existing entry or // overwrite one. Still, use a conservative safety factor of 2. if (!CheckDiskSpace(m_chainman.m_options.datadir, 48 * 2 * 2 * CoinsTip().GetCacheSize())) { - return FatalError(m_chainman.GetNotifications(), state, "Disk space is too low!", _("Disk space is too low!")); + return FatalError(m_chainman.GetNotifications(), state, _("Disk space is too low!")); } // Flush the chainstate (which may refer to block index entries). if (!CoinsTip().Flush()) - return FatalError(m_chainman.GetNotifications(), state, "Failed to write to coin database"); + return FatalError(m_chainman.GetNotifications(), state, _("Failed to write to coin database.")); m_last_flush = nNow; full_flush_completed = true; TRACE5(utxocache, flush, @@ -2735,7 +2762,7 @@ bool Chainstate::FlushStateToDisk( m_chainman.m_options.signals->ChainStateFlushed(this->GetRole(), m_chain.GetLocator()); } } catch (const std::runtime_error& e) { - return FatalError(m_chainman.GetNotifications(), state, std::string("System error while flushing: ") + e.what()); + return FatalError(m_chainman.GetNotifications(), state, strprintf(_("System error while flushing: %s"), e.what())); } return true; } @@ -2971,7 +2998,7 @@ bool Chainstate::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew, if (!pblock) { std::shared_ptr pblockNew = std::make_shared(); if (!m_blockman.ReadBlockFromDisk(*pblockNew, *pindexNew)) { - return FatalError(m_chainman.GetNotifications(), state, "Failed to read block"); + return FatalError(m_chainman.GetNotifications(), state, _("Failed to read block.")); } pthisBlock = pblockNew; } else { @@ -3158,7 +3185,7 @@ bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex* // If we're unable to disconnect a block during normal operation, // then that is a failure of our local system -- we should abort // rather than stay on a less work chain. - FatalError(m_chainman.GetNotifications(), state, "Failed to disconnect block; see debug.log for details"); + FatalError(m_chainman.GetNotifications(), state, _("Failed to disconnect block.")); return false; } fBlocksDisconnected = true; @@ -3660,7 +3687,18 @@ void ChainstateManager::ReceivedBlockTransactions(const CBlock& block, CBlockInd { AssertLockHeld(cs_main); pindexNew->nTx = block.vtx.size(); - pindexNew->nChainTx = 0; + // Typically nChainTx will be 0 at this point, but it can be nonzero if this + // is a pruned block which is being downloaded again, or if this is an + // assumeutxo snapshot block which has a hardcoded nChainTx value from the + // snapshot metadata. If the pindex is not the snapshot block and the + // nChainTx value is not zero, assert that value is actually correct. + auto prev_tx_sum = [](CBlockIndex& block) { return block.nTx + (block.pprev ? block.pprev->nChainTx : 0); }; + if (!Assume(pindexNew->nChainTx == 0 || pindexNew->nChainTx == prev_tx_sum(*pindexNew) || + pindexNew == GetSnapshotBaseBlock())) { + LogWarning("Internal bug detected: block %d has unexpected nChainTx %i that should be %i (%s %s). Please report this issue here: %s\n", + pindexNew->nHeight, pindexNew->nChainTx, prev_tx_sum(*pindexNew), PACKAGE_NAME, FormatFullVersion(), PACKAGE_BUGREPORT); + pindexNew->nChainTx = 0; + } pindexNew->nFile = pos.nFile; pindexNew->nDataPos = pos.nPos; pindexNew->nUndoPos = 0; @@ -3680,7 +3718,15 @@ void ChainstateManager::ReceivedBlockTransactions(const CBlock& block, CBlockInd while (!queue.empty()) { CBlockIndex *pindex = queue.front(); queue.pop_front(); - pindex->nChainTx = (pindex->pprev ? pindex->pprev->nChainTx : 0) + pindex->nTx; + // Before setting nChainTx, assert that it is 0 or already set to + // the correct value. This assert will fail after receiving the + // assumeutxo snapshot block if assumeutxo snapshot metadata has an + // incorrect hardcoded AssumeutxoData::nChainTx value. + if (!Assume(pindex->nChainTx == 0 || pindex->nChainTx == prev_tx_sum(*pindex))) { + LogWarning("Internal bug detected: block %d has unexpected nChainTx %i that should be %i (%s %s). Please report this issue here: %s\n", + pindex->nHeight, pindex->nChainTx, prev_tx_sum(*pindex), PACKAGE_NAME, FormatFullVersion(), PACKAGE_BUGREPORT); + } + pindex->nChainTx = prev_tx_sum(*pindex); pindex->nSequenceId = nBlockSequenceId++; for (Chainstate *c : GetAll()) { c->TryAddBlockIndexCandidate(pindex); @@ -4188,9 +4234,10 @@ bool ChainstateManager::ProcessNewBlockHeaders(const std::vector& if (NotifyHeaderTip(*this)) { if (IsInitialBlockDownload() && ppindex && *ppindex) { const CBlockIndex& last_accepted{**ppindex}; - const int64_t blocks_left{(GetTime() - last_accepted.GetBlockTime()) / GetConsensus().nPowTargetSpacing}; + int64_t blocks_left{(NodeClock::now() - last_accepted.Time()) / GetConsensus().PowTargetSpacing()}; + blocks_left = std::max(0, blocks_left); const double progress{100.0 * last_accepted.nHeight / (last_accepted.nHeight + blocks_left)}; - LogPrintf("Synchronizing blockheaders, height: %d (~%.2f%%)\n", last_accepted.nHeight, progress); + LogInfo("Synchronizing blockheaders, height: %d (~%.2f%%)\n", last_accepted.nHeight, progress); } } return true; @@ -4214,9 +4261,10 @@ void ChainstateManager::ReportHeadersPresync(const arith_uint256& work, int64_t bool initial_download = IsInitialBlockDownload(); GetNotifications().headerTip(GetSynchronizationState(initial_download), height, timestamp, /*presync=*/true); if (initial_download) { - const int64_t blocks_left{(GetTime() - timestamp) / GetConsensus().nPowTargetSpacing}; + int64_t blocks_left{(NodeClock::now() - NodeSeconds{std::chrono::seconds{timestamp}}) / GetConsensus().PowTargetSpacing()}; + blocks_left = std::max(0, blocks_left); const double progress{100.0 * height / (height + blocks_left)}; - LogPrintf("Pre-synchronizing blockheaders, height: %d (~%.2f%%)\n", height, progress); + LogInfo("Pre-synchronizing blockheaders, height: %d (~%.2f%%)\n", height, progress); } } @@ -4299,7 +4347,7 @@ bool ChainstateManager::AcceptBlock(const std::shared_ptr& pblock, } ReceivedBlockTransactions(block, pindex, blockPos); } catch (const std::runtime_error& e) { - return FatalError(GetNotifications(), state, std::string("System error: ") + e.what()); + return FatalError(GetNotifications(), state, strprintf(_("System error while saving block to disk: %s"), e.what())); } // TODO: FlushStateToDisk() handles flushing of both block and chainstate @@ -4983,7 +5031,7 @@ void ChainstateManager::LoadExternalBlockFile( } } } catch (const std::runtime_error& e) { - GetNotifications().fatalError(std::string("System error: ") + e.what()); + GetNotifications().fatalError(strprintf(_("System error while loading external block file: %s"), e.what())); } LogPrintf("Loaded %i blocks from external file in %dms\n", nLoaded, Ticks(SteadyClock::now() - start)); } @@ -5023,16 +5071,31 @@ void ChainstateManager::CheckBlockIndex() size_t nNodes = 0; int nHeight = 0; CBlockIndex* pindexFirstInvalid = nullptr; // Oldest ancestor of pindex which is invalid. - CBlockIndex* pindexFirstMissing = nullptr; // Oldest ancestor of pindex which does not have BLOCK_HAVE_DATA. - CBlockIndex* pindexFirstNeverProcessed = nullptr; // Oldest ancestor of pindex for which nTx == 0. + CBlockIndex* pindexFirstMissing = nullptr; // Oldest ancestor of pindex which does not have BLOCK_HAVE_DATA, since assumeutxo snapshot if used. + CBlockIndex* pindexFirstNeverProcessed = nullptr; // Oldest ancestor of pindex for which nTx == 0, since assumeutxo snapshot if used. CBlockIndex* pindexFirstNotTreeValid = nullptr; // Oldest ancestor of pindex which does not have BLOCK_VALID_TREE (regardless of being valid or not). - CBlockIndex* pindexFirstNotTransactionsValid = nullptr; // Oldest ancestor of pindex which does not have BLOCK_VALID_TRANSACTIONS (regardless of being valid or not). - CBlockIndex* pindexFirstNotChainValid = nullptr; // Oldest ancestor of pindex which does not have BLOCK_VALID_CHAIN (regardless of being valid or not). - CBlockIndex* pindexFirstNotScriptsValid = nullptr; // Oldest ancestor of pindex which does not have BLOCK_VALID_SCRIPTS (regardless of being valid or not). - CBlockIndex* pindexFirstAssumeValid = nullptr; // Oldest ancestor of pindex which has BLOCK_ASSUMED_VALID + CBlockIndex* pindexFirstNotTransactionsValid = nullptr; // Oldest ancestor of pindex which does not have BLOCK_VALID_TRANSACTIONS (regardless of being valid or not), since assumeutxo snapshot if used. + CBlockIndex* pindexFirstNotChainValid = nullptr; // Oldest ancestor of pindex which does not have BLOCK_VALID_CHAIN (regardless of being valid or not), since assumeutxo snapshot if used. + CBlockIndex* pindexFirstNotScriptsValid = nullptr; // Oldest ancestor of pindex which does not have BLOCK_VALID_SCRIPTS (regardless of being valid or not), since assumeutxo snapshot if used. + + // After checking an assumeutxo snapshot block, reset pindexFirst pointers + // to earlier blocks that have not been downloaded or validated yet, so + // checks for later blocks can assume the earlier blocks were validated and + // be stricter, testing for more requirements. + const CBlockIndex* snap_base{GetSnapshotBaseBlock()}; + CBlockIndex *snap_first_missing{}, *snap_first_notx{}, *snap_first_notv{}, *snap_first_nocv{}, *snap_first_nosv{}; + auto snap_update_firsts = [&] { + if (pindex == snap_base) { + std::swap(snap_first_missing, pindexFirstMissing); + std::swap(snap_first_notx, pindexFirstNeverProcessed); + std::swap(snap_first_notv, pindexFirstNotTransactionsValid); + std::swap(snap_first_nocv, pindexFirstNotChainValid); + std::swap(snap_first_nosv, pindexFirstNotScriptsValid); + } + }; + while (pindex != nullptr) { nNodes++; - if (pindexFirstAssumeValid == nullptr && pindex->nStatus & BLOCK_ASSUMED_VALID) pindexFirstAssumeValid = pindex; if (pindexFirstInvalid == nullptr && pindex->nStatus & BLOCK_FAILED_VALID) pindexFirstInvalid = pindex; if (pindexFirstMissing == nullptr && !(pindex->nStatus & BLOCK_HAVE_DATA)) { pindexFirstMissing = pindex; @@ -5040,10 +5103,7 @@ void ChainstateManager::CheckBlockIndex() 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 && !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 (pindex->pprev != nullptr) { if (pindexFirstNotTransactionsValid == nullptr && (pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_TRANSACTIONS) { pindexFirstNotTransactionsValid = pindex; @@ -5073,36 +5133,26 @@ void ChainstateManager::CheckBlockIndex() if (!pindex->HaveNumChainTxs()) 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. - // Unless these indexes are assumed valid and pending block download on a - // background chainstate. - if (!m_blockman.m_have_pruned && !pindex->IsAssumedValid()) { + if (!m_blockman.m_have_pruned) { // If we've never pruned, then HAVE_DATA should be equivalent to nTx > 0 assert(!(pindex->nStatus & BLOCK_HAVE_DATA) == (pindex->nTx == 0)); - if (pindexFirstAssumeValid == nullptr) { - // If we've got some assume valid blocks, then we might have - // missing blocks (not HAVE_DATA) but still treat them as - // having been processed (with a fake nTx value). Otherwise, we - // can assert that these are the same. - assert(pindexFirstMissing == pindexFirstNeverProcessed); - } + assert(pindexFirstMissing == pindexFirstNeverProcessed); } else { // If we have pruned, then we can only say that HAVE_DATA implies nTx > 0 if (pindex->nStatus & BLOCK_HAVE_DATA) assert(pindex->nTx > 0); } if (pindex->nStatus & BLOCK_HAVE_UNDO) assert(pindex->nStatus & BLOCK_HAVE_DATA); - if (pindex->IsAssumedValid()) { - // Assumed-valid blocks should have some nTx value. - assert(pindex->nTx > 0); + if (snap_base && snap_base->GetAncestor(pindex->nHeight) == pindex) { // 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. } + // 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 HaveNumChainTxs(). - assert((pindexFirstNeverProcessed == nullptr) == pindex->HaveNumChainTxs()); - assert((pindexFirstNotTransactionsValid == nullptr) == pindex->HaveNumChainTxs()); + // HaveNumChainTxs will also be set in the assumeutxo snapshot block from snapshot metadata. + assert((pindexFirstNeverProcessed == nullptr || pindex == snap_base) == pindex->HaveNumChainTxs()); + assert((pindexFirstNotTransactionsValid == nullptr || pindex == snap_base) == pindex->HaveNumChainTxs()); assert(pindex->nHeight == nHeight); // nHeight must be consistent. assert(pindex->pprev == nullptr || pindex->nChainWork >= pindex->pprev->nChainWork); // For every block except the genesis block, the chainwork must be larger than the parent's. assert(nHeight < 2 || (pindex->pskip && (pindex->pskip->nHeight < nHeight))); // The pskip pointer must point back for all but the first 2 blocks. @@ -5115,30 +5165,64 @@ void ChainstateManager::CheckBlockIndex() assert((pindex->nStatus & BLOCK_FAILED_MASK) == 0); // The failed mask cannot be set for blocks without invalid parents. } // Make sure nChainTx sum is correctly computed. - unsigned int prev_chain_tx = pindex->pprev ? pindex->pprev->nChainTx : 0; - assert((pindex->nChainTx == pindex->nTx + prev_chain_tx) - // Transaction may be completely unset - happens if only the header was accepted but the block hasn't been processed. - || (pindex->nChainTx == 0 && pindex->nTx == 0) - // nChainTx may be unset, but nTx set (if a block has been accepted, but one of its predecessors hasn't been processed yet) - || (pindex->nChainTx == 0 && prev_chain_tx == 0 && pindex->pprev) - // Transaction counts prior to snapshot are unknown. - || pindex->IsAssumedValid()); + if (!pindex->pprev) { + // If no previous block, nTx and nChainTx must be the same. + assert(pindex->nChainTx == pindex->nTx); + } else if (pindex->pprev->nChainTx > 0 && pindex->nTx > 0) { + // If previous nChainTx is set and number of transactions in block is known, sum must be set. + assert(pindex->nChainTx == pindex->nTx + pindex->pprev->nChainTx); + } else { + // Otherwise nChainTx should only be set if this is a snapshot + // block, and must be set if it is. + assert((pindex->nChainTx != 0) == (pindex == snap_base)); + } + // Chainstate-specific checks on setBlockIndexCandidates for (auto c : GetAll()) { if (c->m_chain.Tip() == nullptr) continue; - if (!CBlockIndexWorkComparator()(pindex, c->m_chain.Tip()) && pindexFirstNeverProcessed == nullptr) { + // Two main factors determine whether pindex is a candidate in + // setBlockIndexCandidates: + // + // - If pindex has less work than the chain tip, it should not be a + // candidate, and this will be asserted below. Otherwise it is a + // potential candidate. + // + // - If pindex or one of its parent blocks back to the genesis block + // or an assumeutxo snapshot never downloaded transactions + // (pindexFirstNeverProcessed is non-null), it should not be a + // candidate, and this will be asserted below. The only exception + // is if pindex itself is an assumeutxo snapshot block. Then it is + // also a potential candidate. + if (!CBlockIndexWorkComparator()(pindex, c->m_chain.Tip()) && (pindexFirstNeverProcessed == nullptr || pindex == snap_base)) { + // If pindex was detected as invalid (pindexFirstInvalid is + // non-null), it is not required to be in + // setBlockIndexCandidates. if (pindexFirstInvalid == nullptr) { - const bool is_active = c == &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 pindex and all its parents back to the genesis block + // or an assumeutxo snapshot block downloaded transactions, + // and the transactions were not pruned (pindexFirstMissing + // is null), it is a potential candidate. The check + // excludes pruned blocks, because if any blocks were + // pruned between pindex the current chain tip, pindex will + // only temporarily be added to setBlockIndexCandidates, + // before being moved to m_blocks_unlinked. This check + // could be improved to verify that if all blocks between + // the chain tip and pindex have data, pindex must be a + // candidate. + // + // If pindex is the chain tip, it also is a potential + // candidate. // - if ((pindexFirstMissing == nullptr || pindex == c->m_chain.Tip())) { - // The active chainstate should always have this block - // as a candidate, but a background chainstate should - // only have it if it is an ancestor of the snapshot base. - if (is_active || GetSnapshotBaseBlock()->GetAncestor(pindex->nHeight) == pindex) { + // If the chainstate was loaded from a snapshot and pindex + // is the base of the snapshot, pindex is also a potential + // candidate. + if (pindexFirstMissing == nullptr || pindex == c->m_chain.Tip() || pindex == c->SnapshotBase()) { + // If this chainstate is the active chainstate, pindex + // must be in setBlockIndexCandidates. Otherwise, this + // chainstate is a background validation chainstate, and + // pindex only needs to be added if it is an ancestor of + // the snapshot that is being validated. + if (c == &ActiveChainstate() || snap_base->GetAncestor(pindex->nHeight) == pindex) { assert(c->setBlockIndexCandidates.count(pindex)); } } @@ -5169,7 +5253,7 @@ void ChainstateManager::CheckBlockIndex() if (pindexFirstMissing == nullptr) assert(!foundInUnlinked); // We aren't missing data for any parent -- cannot be in m_blocks_unlinked. if (pindex->pprev && (pindex->nStatus & BLOCK_HAVE_DATA) && pindexFirstNeverProcessed == nullptr && pindexFirstMissing != nullptr) { // We HAVE_DATA for this block, have received data for all parents at some point, but we're currently missing data for some parent. - assert(m_blockman.m_have_pruned || pindexFirstAssumeValid != nullptr); // We must have pruned, or else we're using a snapshot (causing us to have faked the received data for some parent(s)). + assert(m_blockman.m_have_pruned); // This block may have entered m_blocks_unlinked if: // - it has a descendant that at some point had more work than the // tip, and @@ -5182,7 +5266,7 @@ void ChainstateManager::CheckBlockIndex() const bool is_active = c == &ActiveChainstate(); if (!CBlockIndexWorkComparator()(pindex, c->m_chain.Tip()) && c->setBlockIndexCandidates.count(pindex) == 0) { if (pindexFirstInvalid == nullptr) { - if (is_active || GetSnapshotBaseBlock()->GetAncestor(pindex->nHeight) == pindex) { + if (is_active || snap_base->GetAncestor(pindex->nHeight) == pindex) { assert(foundInUnlinked); } } @@ -5193,6 +5277,7 @@ void ChainstateManager::CheckBlockIndex() // End: actual consistency checks. // Try descending into the first subnode. + snap_update_firsts(); std::pair::iterator,std::multimap::iterator> range = forward.equal_range(pindex); if (range.first != range.second) { // A subnode was found. @@ -5204,6 +5289,7 @@ void ChainstateManager::CheckBlockIndex() // Move upwards until we reach a node of which we have not yet visited the last child. while (pindex) { // We are going to either move to a parent or a sibling of pindex. + snap_update_firsts(); // If pindex was the first with a certain property, unset the corresponding variable. if (pindex == pindexFirstInvalid) pindexFirstInvalid = nullptr; if (pindex == pindexFirstMissing) pindexFirstMissing = nullptr; @@ -5212,7 +5298,6 @@ void ChainstateManager::CheckBlockIndex() if (pindex == pindexFirstNotTransactionsValid) pindexFirstNotTransactionsValid = nullptr; if (pindex == pindexFirstNotChainValid) pindexFirstNotChainValid = nullptr; if (pindex == pindexFirstNotScriptsValid) pindexFirstNotScriptsValid = nullptr; - if (pindex == pindexFirstAssumeValid) pindexFirstAssumeValid = nullptr; // Find our parent. CBlockIndex* pindexPar = pindex->pprev; // Find which child we just visited. @@ -5286,6 +5371,12 @@ double GuessVerificationProgress(const ChainTxData& data, const CBlockIndex *pin if (pindex == nullptr) return 0.0; + if (!Assume(pindex->nChainTx > 0)) { + LogWarning("Internal bug detected: block %d has unset nChainTx (%s %s). Please report this issue here: %s\n", + pindex->nHeight, PACKAGE_NAME, FormatFullVersion(), PACKAGE_BUGREPORT); + return 0.0; + } + int64_t nNow = time(nullptr); double fTxTotal; @@ -5450,8 +5541,8 @@ bool ChainstateManager::ActivateSnapshot( snapshot_chainstate.reset(); bool removed = DeleteCoinsDBFromDisk(*snapshot_datadir, /*is_snapshot=*/true); if (!removed) { - GetNotifications().fatalError(strprintf("Failed to remove snapshot chainstate dir (%s). " - "Manually remove it before restarting.\n", fs::PathToString(*snapshot_datadir))); + GetNotifications().fatalError(strprintf(_("Failed to remove snapshot chainstate dir (%s). " + "Manually remove it before restarting.\n"), fs::PathToString(*snapshot_datadir))); } } return false; @@ -5692,30 +5783,14 @@ bool ChainstateManager::PopulateAndValidateSnapshot( // Fake various pieces of CBlockIndex state: CBlockIndex* index = nullptr; - // Don't make any modifications to the genesis block. - // This is especially important because we don't want to erroneously - // apply BLOCK_ASSUMED_VALID to genesis, which would happen if we didn't skip - // it here (since it apparently isn't BLOCK_VALID_SCRIPTS). + // Don't make any modifications to the genesis block since it shouldn't be + // neccessary, and since the genesis block doesn't have normal flags like + // BLOCK_VALID_SCRIPTS set. constexpr int AFTER_GENESIS_START{1}; for (int i = AFTER_GENESIS_START; i <= snapshot_chainstate.m_chain.Height(); ++i) { index = snapshot_chainstate.m_chain[i]; - // Fake nTx so that LoadBlockIndex() loads assumed-valid CBlockIndex - // entries (among other things) - if (!index->nTx) { - index->nTx = 1; - } - // Fake nChainTx so that GuessVerificationProgress reports accurately - index->nChainTx = index->pprev->nChainTx + index->nTx; - - // 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 Chainstate::NeedsRedownload() // won't ask to rewind the entire assumed-valid chain on startup. if (DeploymentActiveAt(*index, *this, Consensus::DEPLOYMENT_SEGWIT)) { @@ -5731,6 +5806,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot( } assert(index); + assert(index == snapshot_start_block); index->nChainTx = au_data.nChainTx; snapshot_chainstate.setBlockIndexCandidates.insert(snapshot_start_block); @@ -5805,7 +5881,7 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation() user_error = strprintf(Untranslated("%s\n%s"), user_error, util::ErrorString(rename_result)); } - GetNotifications().fatalError(user_error.original, user_error); + GetNotifications().fatalError(user_error); }; if (index_new.GetBlockHash() != snapshot_blockhash) { @@ -6146,9 +6222,9 @@ bool ChainstateManager::ValidatedSnapshotCleanup() const fs::filesystem_error& err) { LogPrintf("Error renaming path (%s) -> (%s): %s\n", fs::PathToString(p_old), fs::PathToString(p_new), err.what()); - GetNotifications().fatalError(strprintf( + GetNotifications().fatalError(strprintf(_( "Rename of '%s' -> '%s' failed. " - "Cannot clean up the background chainstate leveldb directory.", + "Cannot clean up the background chainstate leveldb directory."), fs::PathToString(p_old), fs::PathToString(p_new))); };