From d20642f4e053ee30bc6e82f5f631af8019d3ff15 Mon Sep 17 00:00:00 2001 From: ismaelsadeeq Date: Thu, 21 May 2026 20:15:34 +0100 Subject: [PATCH 1/6] validation: extract block import into node/blockimport ImportBlocks, which loads block files from disk during startup (-reindex and -loadblock), does not belong in blockstorage.cpp alongside the BlockManager class it merely uses. Move it to node/blockimport.{cpp,h} so that each file has a single clear responsibility. This is a pure move: no logic is changed. blockimport.cpp is given only the includes it directly requires; util/result.h is removed from blockstorage.cpp where it became unused once ImportBlocks left. --- src/CMakeLists.txt | 1 + src/init.cpp | 1 + src/kernel/CMakeLists.txt | 1 + src/kernel/bitcoinkernel.cpp | 1 + src/node/blockimport.cpp | 101 +++++++++++++++++++++++++++++++++++ src/node/blockimport.h | 21 ++++++++ src/node/blockstorage.cpp | 74 ------------------------- src/node/blockstorage.h | 3 -- 8 files changed, 126 insertions(+), 77 deletions(-) create mode 100644 src/node/blockimport.cpp create mode 100644 src/node/blockimport.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 7bfa5a89bcfa..f9bc43494796 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -171,6 +171,7 @@ add_library(bitcoin_node STATIC EXCLUDE_FROM_ALL net_processing.cpp netgroup.cpp node/abort.cpp + node/blockimport.cpp node/blockmanager_args.cpp node/blockstorage.cpp node/caches.cpp diff --git a/src/init.cpp b/src/init.cpp index 2f1367dd4807..01a06e919147 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -40,6 +40,7 @@ #include #include #include +#include #include #include #include diff --git a/src/kernel/CMakeLists.txt b/src/kernel/CMakeLists.txt index 5d7db3e6cf8f..11c39b383ef0 100644 --- a/src/kernel/CMakeLists.txt +++ b/src/kernel/CMakeLists.txt @@ -31,6 +31,7 @@ add_library(bitcoinkernel ../flatfile.cpp ../hash.cpp ../logging.cpp + ../node/blockimport.cpp ../node/blockstorage.cpp ../node/chainstate.cpp ../policy/ephemeral_policy.cpp diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp index 9ec51351c439..c6b1f6259ac8 100644 --- a/src/kernel/bitcoinkernel.cpp +++ b/src/kernel/bitcoinkernel.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include diff --git a/src/node/blockimport.cpp b/src/node/blockimport.cpp new file mode 100644 index 000000000000..3e784f76d030 --- /dev/null +++ b/src/node/blockimport.cpp @@ -0,0 +1,101 @@ +// Copyright (c) 2011-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +namespace node { + +class ImportingNow +{ + std::atomic& m_importing; + +public: + ImportingNow(std::atomic& importing) : m_importing{importing} + { + assert(m_importing == false); + m_importing = true; + } + ~ImportingNow() + { + assert(m_importing == true); + m_importing = false; + } +}; + +void ImportBlocks(ChainstateManager& chainman, std::span import_paths) +{ + ImportingNow imp{chainman.m_blockman.m_importing}; + + // -reindex + if (!chainman.m_blockman.m_blockfiles_indexed) { + int total_files{0}; + while (fs::exists(chainman.m_blockman.GetBlockPosFilename(FlatFilePos(total_files, 0)))) { + total_files++; + } + + // Map of disk positions for blocks with unknown parent (only used for reindex); + // parent hash -> child disk position, multiple children can have the same parent. + std::multimap blocks_with_unknown_parent; + + for (int nFile{0}; nFile < total_files; ++nFile) { + FlatFilePos pos(nFile, 0); + AutoFile file{chainman.m_blockman.OpenBlockFile(pos, /*fReadOnly=*/true)}; + if (file.IsNull()) { + break; // This error is logged in OpenBlockFile + } + LogInfo("Reindexing block file blk%05u.dat (%d%% complete)...", (unsigned int)nFile, nFile * 100 / total_files); + chainman.LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent); + if (chainman.m_interrupt) { + LogInfo("Interrupt requested. Exit reindexing."); + return; + } + } + WITH_LOCK(::cs_main, chainman.m_blockman.m_block_tree_db->WriteReindexing(false)); + chainman.m_blockman.m_blockfiles_indexed = true; + LogInfo("Reindexing finished"); + // To avoid ending up in a situation without genesis block, re-try initializing (no-op if reindexing worked): + chainman.ActiveChainstate().LoadGenesisBlock(); + } + + // -loadblock= + for (const fs::path& path : import_paths) { + AutoFile file{fsbridge::fopen(path, "rb")}; + if (!file.IsNull()) { + LogInfo("Importing blocks file %s...", fs::PathToString(path)); + chainman.LoadExternalBlockFile(file); + if (chainman.m_interrupt) { + LogInfo("Interrupt requested. Exit block importing."); + return; + } + } else { + LogWarning("Could not open blocks file %s", fs::PathToString(path)); + } + } + + // scan for better chains in the block chain database, that are not yet connected in the active best chain + if (auto result = chainman.ActivateBestChains(); !result) { + chainman.GetNotifications().fatalError(util::ErrorString(result)); + } + // End scope of ImportingNow +} + +} // namespace node diff --git a/src/node/blockimport.h b/src/node/blockimport.h new file mode 100644 index 000000000000..759cc15888eb --- /dev/null +++ b/src/node/blockimport.h @@ -0,0 +1,21 @@ +// Copyright (c) 2011-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_NODE_BLOCKIMPORT_H +#define BITCOIN_NODE_BLOCKIMPORT_H + +#include + +#include + +class ChainstateManager; + +namespace node { + +// Calls ActivateBestChain() even if no blocks are imported. +void ImportBlocks(ChainstateManager& chainman, std::span import_paths); + +} // namespace node + +#endif // BITCOIN_NODE_BLOCKIMPORT_H diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 42bf15154c91..0743d80f22f8 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -32,7 +32,6 @@ #include #include #include -#include #include #include #include @@ -1180,79 +1179,6 @@ BlockManager::BlockManager(const util::SignalInterrupt& interrupt, Options opts) } } -class ImportingNow -{ - std::atomic& m_importing; - -public: - ImportingNow(std::atomic& importing) : m_importing{importing} - { - assert(m_importing == false); - m_importing = true; - } - ~ImportingNow() - { - assert(m_importing == true); - m_importing = false; - } -}; - -void ImportBlocks(ChainstateManager& chainman, std::span import_paths) -{ - ImportingNow imp{chainman.m_blockman.m_importing}; - - // -reindex - if (!chainman.m_blockman.m_blockfiles_indexed) { - int total_files{0}; - while (fs::exists(chainman.m_blockman.GetBlockPosFilename(FlatFilePos(total_files, 0)))) { - total_files++; - } - - // Map of disk positions for blocks with unknown parent (only used for reindex); - // parent hash -> child disk position, multiple children can have the same parent. - std::multimap blocks_with_unknown_parent; - - for (int nFile{0}; nFile < total_files; ++nFile) { - FlatFilePos pos(nFile, 0); - AutoFile file{chainman.m_blockman.OpenBlockFile(pos, /*fReadOnly=*/true)}; - if (file.IsNull()) { - break; // This error is logged in OpenBlockFile - } - LogInfo("Reindexing block file blk%05u.dat (%d%% complete)...", (unsigned int)nFile, nFile * 100 / total_files); - chainman.LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent); - if (chainman.m_interrupt) { - LogInfo("Interrupt requested. Exit reindexing."); - return; - } - } - WITH_LOCK(::cs_main, chainman.m_blockman.m_block_tree_db->WriteReindexing(false)); - chainman.m_blockman.m_blockfiles_indexed = true; - LogInfo("Reindexing finished"); - // To avoid ending up in a situation without genesis block, re-try initializing (no-op if reindexing worked): - chainman.ActiveChainstate().LoadGenesisBlock(); - } - - // -loadblock= - for (const fs::path& path : import_paths) { - AutoFile file{fsbridge::fopen(path, "rb")}; - if (!file.IsNull()) { - LogInfo("Importing blocks file %s...", fs::PathToString(path)); - chainman.LoadExternalBlockFile(file); - if (chainman.m_interrupt) { - LogInfo("Interrupt requested. Exit block importing."); - return; - } - } else { - LogWarning("Could not open blocks file %s", fs::PathToString(path)); - } - } - - // scan for better chains in the block chain database, that are not yet connected in the active best chain - if (auto result = chainman.ActivateBestChains(); !result) { - chainman.GetNotifications().fatalError(util::ErrorString(result)); - } - // End scope of ImportingNow -} std::ostream& operator<<(std::ostream& os, const BlockfileCursor& cursor) { os << strprintf("BlockfileCursor(file_num=%d, undo_height=%d)", cursor.file_num, cursor.undo_height); diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 1acc60d011d6..b08a45a366e6 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -37,7 +37,6 @@ #include #include #include -#include #include #include #include @@ -443,8 +442,6 @@ class BlockManager void CleanupBlockRevFiles() const; }; -// Calls ActivateBestChain() even if no blocks are imported. -void ImportBlocks(ChainstateManager& chainman, std::span import_paths); } // namespace node #endif // BITCOIN_NODE_BLOCKSTORAGE_H From ebba7bbb0f9d5d0bfa80dd37dca9d2814e43329e Mon Sep 17 00:00:00 2001 From: ismaelsadeeq Date: Thu, 21 May 2026 20:19:39 +0100 Subject: [PATCH 2/6] validation: split consensus helpers out of validation.cpp CheckFinalTxAtTip is a pure transaction finality check with no dependency on chainstate. Move it to consensus/tx_verify.{cpp,h} where it sits alongside the other transaction verification helpers. DisconnectResult, ApplyTxInUndo, and UpdateCoins are UTXO mutation primitives called during block (dis)connection. They depend only on CCoinsViewCache and CTransaction, with no chainstate coupling. Move them to coins.{cpp,h} alongside the existing AddCoins/SpendCoin helpers they naturally extend. These moves eliminate the need for validation.cpp callers to reach into validation.h solely for these low-level helpers. Moving ApplyTxInUndo and UpdateCoins into coins.h introduces a new expected circular dependency: coins -> undo -> coins. undo.h includes coins.h for Coin, and coins.h now includes undo.h for BlockUndo. --- src/coins.cpp | 44 +++++++++++++++ src/coins.h | 21 +++++++ src/consensus/tx_verify.cpp | 20 +++++++ src/consensus/tx_verify.h | 3 + src/test/coins_tests.cpp | 4 +- src/validation.cpp | 74 +------------------------ src/validation.h | 13 +---- test/lint/lint-circular-dependencies.py | 1 + 8 files changed, 92 insertions(+), 88 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index c403e006c85c..c30e69fdf82d 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -407,3 +408,46 @@ std::optional CCoinsViewErrorCatcher::PeekCoin(const COutPoint& outpoint) { return ExecuteBackedWrapper>([&]() { return CCoinsViewBacked::PeekCoin(outpoint); }, m_err_callbacks); } + +DisconnectResult ApplyTxInUndo(Coin&& undo, CCoinsViewCache& view, const COutPoint& out) +{ + bool fClean = true; + + if (view.HaveCoin(out)) fClean = false; // overwriting transaction output + + if (undo.nHeight == 0) { + // Missing undo metadata (height and coinbase). Older versions included this + // information only in undo records for the last spend of a transactions' + // outputs. This implies that it must be present for some other output of the same tx. + const Coin& alternate = AccessByTxid(view, out.hash); + if (!alternate.IsSpent()) { + undo.nHeight = alternate.nHeight; + undo.fCoinBase = alternate.fCoinBase; + } else { + return DISCONNECT_FAILED; // adding output for transaction without known metadata + } + } + // If the coin already exists as an unspent coin in the cache, then the + // possible_overwrite parameter to AddCoin must be set to true. We have + // already checked whether an unspent coin exists above using HaveCoin, so + // we don't need to guess. When fClean is false, an unspent coin already + // existed and it is an overwrite. + view.AddCoin(out, std::move(undo), !fClean); + + return fClean ? DISCONNECT_OK : DISCONNECT_UNCLEAN; +} + +void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo& txundo, int nHeight) +{ + // mark inputs spent + if (!tx.IsCoinBase()) { + txundo.vprevout.reserve(tx.vin.size()); + for (const CTxIn& txin : tx.vin) { + txundo.vprevout.emplace_back(); + bool is_spent = inputs.SpendCoin(txin.prevout, &txundo.vprevout.back()); + assert(is_spent); + } + } + // add outputs + AddCoins(inputs, tx, nHeight); +} diff --git a/src/coins.h b/src/coins.h index fd781fb74369..d19581b4b420 100644 --- a/src/coins.h +++ b/src/coins.h @@ -24,6 +24,8 @@ #include #include +class CTxUndo; + /** * A UTXO entry. * @@ -613,4 +615,23 @@ class CCoinsViewErrorCatcher final : public CCoinsViewBacked }; +enum DisconnectResult +{ + DISCONNECT_OK, // All good. + DISCONNECT_UNCLEAN, // Rolled back, but UTXO set was inconsistent with block. + DISCONNECT_FAILED // Something else went wrong. +}; + +/** + * Restore the UTXO in a Coin at a given COutPoint + * @param undo The Coin to be restored. + * @param view The coins view to which to apply the changes. + * @param out The out point that corresponds to the tx input. + * @return A DisconnectResult. + */ +DisconnectResult ApplyTxInUndo(Coin&& undo, CCoinsViewCache& view, const COutPoint& out); + +/** Apply the effects of this transaction on the UTXO set represented by view */ +void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo& txundo, int nHeight); + #endif // BITCOIN_COINS_H diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index 4efed70fd411..c91a676b6df0 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -212,3 +212,23 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state, txfee = txfee_aux; return true; } + +bool CheckFinalTxAtTip(const CBlockIndex& active_chain_tip, const CTransaction& tx) +{ + // CheckFinalTxAtTip() uses active_chain_tip.Height()+1 to evaluate + // nLockTime because when IsFinalTx() is called within + // AcceptBlock(), the height of the block *being* + // evaluated is what is used. Thus if we want to know if a + // transaction can be part of the *next* block, we need to call + // IsFinalTx() with one more than active_chain_tip.Height(). + const int nBlockHeight = active_chain_tip.nHeight + 1; + + // BIP113 requires that time-locked transactions have nLockTime set to + // less than the median time of the previous block they're contained in. + // When the next block is created its previous block will be the current + // chain tip, so we use that to calculate the median time passed to + // IsFinalTx(). + const int64_t nBlockTime{active_chain_tip.GetMedianTimePast()}; + + return IsFinalTx(tx, nBlockHeight, nBlockTime); +} diff --git a/src/consensus/tx_verify.h b/src/consensus/tx_verify.h index ed44d435c1b9..2ccb9cdc5eab 100644 --- a/src/consensus/tx_verify.h +++ b/src/consensus/tx_verify.h @@ -76,4 +76,7 @@ bool EvaluateSequenceLocks(const CBlockIndex& block, std::pair loc */ bool SequenceLocks(const CTransaction &tx, int flags, std::vector& prevHeights, const CBlockIndex& block); +/** Check if transaction will be final in the next block to be created. */ +bool CheckFinalTxAtTip(const CBlockIndex& active_chain_tip, const CTransaction& tx); + #endif // BITCOIN_CONSENSUS_TX_VERIFY_H diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 14ccb1c443c4..5ae46b3badf4 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -25,9 +26,6 @@ using namespace util::hex_literals; -int ApplyTxInUndo(Coin&& undo, CCoinsViewCache& view, const COutPoint& out); -void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo &txundo, int nHeight); - namespace { //! equality test diff --git a/src/validation.cpp b/src/validation.cpp index 056b3baa5713..d9350ba9624c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -138,28 +138,6 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state, std::vector* pvChecks = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_main); -bool CheckFinalTxAtTip(const CBlockIndex& active_chain_tip, const CTransaction& tx) -{ - AssertLockHeld(cs_main); - - // CheckFinalTxAtTip() uses active_chain_tip.Height()+1 to evaluate - // nLockTime because when IsFinalTx() is called within - // AcceptBlock(), the height of the block *being* - // evaluated is what is used. Thus if we want to know if a - // transaction can be part of the *next* block, we need to call - // IsFinalTx() with one more than active_chain_tip.Height(). - const int nBlockHeight = active_chain_tip.nHeight + 1; - - // BIP113 requires that time-locked transactions have nLockTime set to - // less than the median time of the previous block they're contained in. - // When the next block is created its previous block will be the current - // chain tip, so we use that to calculate the median time passed to - // IsFinalTx(). - const int64_t nBlockTime{active_chain_tip.GetMedianTimePast()}; - - return IsFinalTx(tx, nBlockHeight, nBlockTime); -} - namespace { /** * A helper which calculates heights of inputs of a given transaction. @@ -1945,21 +1923,6 @@ void Chainstate::InvalidBlockFound(CBlockIndex* pindex, const BlockValidationSta } } -void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo &txundo, int nHeight) -{ - // mark inputs spent - if (!tx.IsCoinBase()) { - txundo.vprevout.reserve(tx.vin.size()); - for (const CTxIn &txin : tx.vin) { - txundo.vprevout.emplace_back(); - bool is_spent = inputs.SpendCoin(txin.prevout, &txundo.vprevout.back()); - assert(is_spent); - } - } - // add outputs - AddCoins(inputs, tx, nHeight); -} - std::optional> CScriptCheck::operator()() { const CScript &scriptSig = ptxTo->vin[nIn].scriptSig; const CScriptWitness *witness = &ptxTo->vin[nIn].scriptWitness; @@ -2088,41 +2051,6 @@ bool FatalError(Notifications& notifications, BlockValidationState& state, const return state.Error(message.original); } -/** - * Restore the UTXO in a Coin at a given COutPoint - * @param undo The Coin to be restored. - * @param view The coins view to which to apply the changes. - * @param out The out point that corresponds to the tx input. - * @return A DisconnectResult as an int - */ -int ApplyTxInUndo(Coin&& undo, CCoinsViewCache& view, const COutPoint& out) -{ - bool fClean = true; - - if (view.HaveCoin(out)) fClean = false; // overwriting transaction output - - if (undo.nHeight == 0) { - // Missing undo metadata (height and coinbase). Older versions included this - // information only in undo records for the last spend of a transactions' - // outputs. This implies that it must be present for some other output of the same tx. - const Coin& alternate = AccessByTxid(view, out.hash); - if (!alternate.IsSpent()) { - undo.nHeight = alternate.nHeight; - undo.fCoinBase = alternate.fCoinBase; - } else { - return DISCONNECT_FAILED; // adding output for transaction without known metadata - } - } - // If the coin already exists as an unspent coin in the cache, then the - // possible_overwrite parameter to AddCoin must be set to true. We have - // already checked whether an unspent coin exists above using HaveCoin, so - // we don't need to guess. When fClean is false, an unspent coin already - // existed and it is an overwrite. - view.AddCoin(out, std::move(undo), !fClean); - - return fClean ? DISCONNECT_OK : DISCONNECT_UNCLEAN; -} - /** Undo the effects of this block (with given index) on the UTXO set represented by coins. * When FAILED is returned, view is left in an indeterminate state. */ DisconnectResult Chainstate::DisconnectBlock(const CBlock& block, const CBlockIndex* pindex, CCoinsViewCache& view) @@ -2182,7 +2110,7 @@ DisconnectResult Chainstate::DisconnectBlock(const CBlock& block, const CBlockIn for (unsigned int j = tx.vin.size(); j > 0;) { --j; const COutPoint& out = tx.vin[j].prevout; - int res = ApplyTxInUndo(std::move(txundo.vprevout[j]), view, out); + DisconnectResult res = ApplyTxInUndo(std::move(txundo.vprevout[j]), view, out); if (res == DISCONNECT_FAILED) return DISCONNECT_FAILED; fClean = fClean && res != DISCONNECT_UNCLEAN; } diff --git a/src/validation.h b/src/validation.h index 840240ce355f..dce35f39468a 100644 --- a/src/validation.h +++ b/src/validation.h @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -284,11 +285,6 @@ PackageMempoolAcceptResult ProcessNewPackage(Chainstate& active_chainstate, CTxM /* Mempool validation helper functions */ -/** - * Check if transaction will be final in the next block to be created. - */ -bool CheckFinalTxAtTip(const CBlockIndex& active_chain_tip, const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - /** * Calculate LockPoints required to check if transaction will be BIP68 final in the next block * to be created on top of tip. @@ -441,13 +437,6 @@ class CVerifyDB int nCheckDepth) EXCLUSIVE_LOCKS_REQUIRED(cs_main); }; -enum DisconnectResult -{ - DISCONNECT_OK, // All good. - DISCONNECT_UNCLEAN, // Rolled back, but UTXO set was inconsistent with block. - DISCONNECT_FAILED // Something else went wrong. -}; - struct ConnectedBlock; /** @see Chainstate::FlushStateToDisk */ diff --git a/test/lint/lint-circular-dependencies.py b/test/lint/lint-circular-dependencies.py index b7b23776002e..eac609d16822 100755 --- a/test/lint/lint-circular-dependencies.py +++ b/test/lint/lint-circular-dependencies.py @@ -15,6 +15,7 @@ "chainparamsbase -> common/args -> chainparamsbase", "node/blockstorage -> validation -> node/blockstorage", "kernel/coinstats -> validation -> kernel/coinstats", + "coins -> undo -> coins", "versionbits -> versionbits_impl -> versionbits", ) From 38b01a921cc59fb6c164d16491ba0592688ca748 Mon Sep 17 00:00:00 2001 From: ismaelsadeeq Date: Thu, 21 May 2026 20:31:38 +0100 Subject: [PATCH 3/6] validation: extract mempool validation into mempool_validation.{h,cpp} Extract mempool-related validation functions from validation.cpp into new files mempool_validation.{h,cpp}: - MempoolAcceptResult and PackageMempoolAcceptResult result types - ProcessTransaction, AcceptToMemoryPool, ProcessNewPackage - LimitMempoolSize, CalculateLockPointsAtTip, CheckSequenceLocksAtTip - MaybeUpdateMempoolForReorg Behavioral change: MaybeUpdateMempoolForReorg now takes an explicit CTxMemPool& mempool parameter instead of obtaining it via chainstate.GetMempool(). This is required because the Clang Thread Safety Analyzer cannot alias *chainstate.MempoolMutex() to mempool.cs when they are obtained via different expressions. The four call sites in validation.cpp are guarded with `if (m_mempool)` null checks. Introduces circular dependency: mempool_validation -> validation -> mempool_validation (validation.cpp still includes mempool_validation.h for the transition period; to be resolved in a follow-up) --- src/CMakeLists.txt | 1 + src/bench/block_assemble.cpp | 3 +- src/kernel/CMakeLists.txt | 1 + src/mempool_validation.cpp | 1731 +++++++++++++++++++++++ src/mempool_validation.h | 278 ++++ src/net_processing.cpp | 7 +- src/node/mempool_persist.cpp | 1 + src/node/transaction.cpp | 8 +- src/test/fuzz/package_eval.cpp | 1 + src/test/fuzz/tx_pool.cpp | 1 + src/test/miner_tests.cpp | 1 + src/test/txdownload_tests.cpp | 3 +- src/test/txpackage_tests.cpp | 3 +- src/test/txvalidation_tests.cpp | 3 +- src/test/txvalidationcache_tests.cpp | 9 +- src/test/util/setup_common.cpp | 3 +- src/test/util/txmempool.cpp | 1 + src/test/validation_block_tests.cpp | 3 +- src/validation.cpp | 1701 +--------------------- src/validation.h | 232 +-- test/lint/lint-circular-dependencies.py | 1 + 21 files changed, 2053 insertions(+), 1939 deletions(-) create mode 100644 src/mempool_validation.cpp create mode 100644 src/mempool_validation.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index f9bc43494796..3df11154f2eb 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -215,6 +215,7 @@ add_library(bitcoin_node STATIC EXCLUDE_FROM_ALL txgraph.cpp txmempool.cpp txrequest.cpp + mempool_validation.cpp validation.cpp validationinterface.cpp versionbits.cpp diff --git a/src/bench/block_assemble.cpp b/src/bench/block_assemble.cpp index 297465be80f5..2815cdc0f4f3 100644 --- a/src/bench/block_assemble.cpp +++ b/src/bench/block_assemble.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -46,7 +47,7 @@ static void AssembleBlock(benchmark::Bench& bench) LOCK(::cs_main); for (const auto& txr : txs) { - const MempoolAcceptResult res = test_setup->m_node.chainman->ProcessTransaction(txr); + const MempoolAcceptResult res = ProcessTransaction(*test_setup->m_node.chainman, txr); assert(res.m_result_type == MempoolAcceptResult::ResultType::VALID); } } diff --git a/src/kernel/CMakeLists.txt b/src/kernel/CMakeLists.txt index 11c39b383ef0..b2cfd9131a82 100644 --- a/src/kernel/CMakeLists.txt +++ b/src/kernel/CMakeLists.txt @@ -73,6 +73,7 @@ add_library(bitcoinkernel ../util/threadnames.cpp ../util/time.cpp ../util/tokenpipe.cpp + ../mempool_validation.cpp ../validation.cpp ../validationinterface.cpp ../versionbits.cpp diff --git a/src/mempool_validation.cpp b/src/mempool_validation.cpp new file mode 100644 index 000000000000..5a49d66d22d3 --- /dev/null +++ b/src/mempool_validation.cpp @@ -0,0 +1,1731 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include