From 9a560a1abebc6e2295c7db5662abb2907328c2d0 Mon Sep 17 00:00:00 2001 From: glozow Date: Tue, 5 Sep 2023 13:45:09 +0100 Subject: [PATCH] MOVEONLY: DisconnectedBlockTransactions to its own file This struct is only used in validation + tests and has very little to do with txmempool. --- src/Makefile.am | 1 + src/bench/disconnected_transactions.cpp | 3 +- src/kernel/disconnected_transactions.h | 116 ++++++++++++++++++ .../validation_chainstatemanager_tests.cpp | 1 + src/txmempool.h | 102 --------------- src/validation.cpp | 1 + 6 files changed, 121 insertions(+), 103 deletions(-) create mode 100644 src/kernel/disconnected_transactions.h diff --git a/src/Makefile.am b/src/Makefile.am index feed4a0061cd5e..fa063dac8513c2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -186,6 +186,7 @@ BITCOIN_CORE_H = \ kernel/coinstats.h \ kernel/context.h \ kernel/cs_main.h \ + kernel/disconnected_transactions.h \ kernel/mempool_entry.h \ kernel/mempool_limits.h \ kernel/mempool_options.h \ diff --git a/src/bench/disconnected_transactions.cpp b/src/bench/disconnected_transactions.cpp index 4db082b3183306..37fbae428f0895 100644 --- a/src/bench/disconnected_transactions.cpp +++ b/src/bench/disconnected_transactions.cpp @@ -3,9 +3,10 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include +#include #include #include -#include constexpr size_t BLOCK_VTX_COUNT{4000}; constexpr size_t BLOCK_VTX_COUNT_10PERCENT{400}; diff --git a/src/kernel/disconnected_transactions.h b/src/kernel/disconnected_transactions.h new file mode 100644 index 00000000000000..77fd33a57f6130 --- /dev/null +++ b/src/kernel/disconnected_transactions.h @@ -0,0 +1,116 @@ +// Copyright (c) 2023 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_KERNEL_DISCONNECTED_TRANSACTIONS_H +#define BITCOIN_KERNEL_DISCONNECTED_TRANSACTIONS_H + +#include +#include +#include +#include + +#include +#include + +/** + * DisconnectedBlockTransactions + + * During the reorg, it's desirable to re-add previously confirmed transactions + * to the mempool, so that anything not re-confirmed in the new chain is + * available to be mined. However, it's more efficient to wait until the reorg + * is complete and process all still-unconfirmed transactions at that time, + * since we expect most confirmed transactions to (typically) still be + * confirmed in the new chain, and re-accepting to the memory pool is expensive + * (and therefore better to not do in the middle of reorg-processing). + * Instead, store the disconnected transactions (in order!) as we go, remove any + * that are included in blocks in the new chain, and then process the remaining + * still-unconfirmed transactions at the end. + */ +struct DisconnectedBlockTransactions { +private: + uint64_t cachedInnerUsage = 0; + std::list queuedTx; + using List = decltype(queuedTx); + std::unordered_map iters_by_txid; + + void clear() + { + cachedInnerUsage = 0; + iters_by_txid.clear(); + queuedTx.clear(); + } +public: + // It's almost certainly a logic bug if we don't clear out queuedTx before + // destruction, as we add to it while disconnecting blocks, and then we + // need to re-process remaining transactions to ensure mempool consistency. + // For now, assert() that we've emptied out this object on destruction. + // This assert() can always be removed if the reorg-processing code were + // to be refactored such that this assumption is no longer true (for + // instance if there was some other way we cleaned up the mempool after a + // reorg, besides draining this object). + ~DisconnectedBlockTransactions() { + assert(queuedTx.empty()); + assert(iters_by_txid.empty()); + assert(cachedInnerUsage == 0); + } + + size_t DynamicMemoryUsage() const { + return cachedInnerUsage + memusage::DynamicUsage(iters_by_txid) + memusage::DynamicUsage(queuedTx); + } + + // Add transactions from the block, in reverse order. + void AddTransactionsFromBlock(const std::vector& vtx) + { + // Blocks are disconnected in descending order by block height. Within each block, add + // transactions in reverse order so that transactions with dependencies on other + // transactions (if any) are at the beginning. If this data structure grows too large, we + // will trim transactions from the front. Transactions with fewer dependencies should be + // removed first. + iters_by_txid.reserve(iters_by_txid.size() + vtx.size()); + for (auto block_it = vtx.rbegin(); block_it != vtx.rend(); ++block_it) { + auto it = queuedTx.insert(queuedTx.end(), *block_it); + iters_by_txid.emplace((*block_it)->GetHash(), it); + cachedInnerUsage += RecursiveDynamicUsage(*block_it); + } + } + + // Remove entries that are in this block. + void removeForBlock(const std::vector& vtx) + { + // Short-circuit in the common case of a block being added to the tip + if (queuedTx.empty()) { + return; + } + for (const auto& tx : vtx) { + auto iter = iters_by_txid.find(tx->GetHash()); + if (iter != iters_by_txid.end()) { + auto list_iter = iter->second; + iters_by_txid.erase(iter); + cachedInnerUsage -= RecursiveDynamicUsage(*list_iter); + queuedTx.erase(list_iter); + } + } + } + + // Remove the first entry and update memory usage. + CTransactionRef take_first() + { + CTransactionRef first_tx; + if (!queuedTx.empty()) { + first_tx = queuedTx.front(); + cachedInnerUsage -= RecursiveDynamicUsage(queuedTx.front()); + iters_by_txid.erase(queuedTx.front()->GetHash()); + queuedTx.pop_front(); + } + return first_tx; + } + + // Clear all data structures and return the list of transactions. + std::list take() { + std::list ret = std::move(queuedTx); + clear(); + return ret; + } +}; +#endif // BITCOIN_KERNEL_DISCONNECTED_TRANSACTIONS_H diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index caa4866f6c55f7..2ca70696e53d89 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -4,6 +4,7 @@ // #include #include +#include #include #include #include diff --git a/src/txmempool.h b/src/txmempool.h index 803ac4aff73a40..ad4d9e7101169f 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -836,106 +836,4 @@ class CCoinsViewMemPool : public CCoinsViewBacked * m_temp_added and cannot be flushed to the back end. Only used for package validation. */ void PackageAddTransaction(const CTransactionRef& tx); }; - -/** - * DisconnectedBlockTransactions - - * During the reorg, it's desirable to re-add previously confirmed transactions - * to the mempool, so that anything not re-confirmed in the new chain is - * available to be mined. However, it's more efficient to wait until the reorg - * is complete and process all still-unconfirmed transactions at that time, - * since we expect most confirmed transactions to (typically) still be - * confirmed in the new chain, and re-accepting to the memory pool is expensive - * (and therefore better to not do in the middle of reorg-processing). - * Instead, store the disconnected transactions (in order!) as we go, remove any - * that are included in blocks in the new chain, and then process the remaining - * still-unconfirmed transactions at the end. - */ -struct DisconnectedBlockTransactions { -private: - uint64_t cachedInnerUsage = 0; - std::list queuedTx; - using List = decltype(queuedTx); - std::unordered_map iters_by_txid; - - void clear() - { - cachedInnerUsage = 0; - iters_by_txid.clear(); - queuedTx.clear(); - } -public: - // It's almost certainly a logic bug if we don't clear out queuedTx before - // destruction, as we add to it while disconnecting blocks, and then we - // need to re-process remaining transactions to ensure mempool consistency. - // For now, assert() that we've emptied out this object on destruction. - // This assert() can always be removed if the reorg-processing code were - // to be refactored such that this assumption is no longer true (for - // instance if there was some other way we cleaned up the mempool after a - // reorg, besides draining this object). - ~DisconnectedBlockTransactions() { - assert(queuedTx.empty()); - assert(iters_by_txid.empty()); - assert(cachedInnerUsage == 0); - } - - size_t DynamicMemoryUsage() const { - return cachedInnerUsage + memusage::DynamicUsage(iters_by_txid) + memusage::DynamicUsage(queuedTx); - } - - // Add transactions from the block, in reverse order. - void AddTransactionsFromBlock(const std::vector& vtx) - { - // Blocks are disconnected in descending order by block height. Within each block, add - // transactions in reverse order so that transactions with dependencies on other - // transactions (if any) are at the beginning. If this data structure grows too large, we - // will trim transactions from the front. Transactions with fewer dependencies should be - // removed first. - iters_by_txid.reserve(iters_by_txid.size() + vtx.size()); - for (auto block_it = vtx.rbegin(); block_it != vtx.rend(); ++block_it) { - auto it = queuedTx.insert(queuedTx.end(), *block_it); - iters_by_txid.emplace((*block_it)->GetHash(), it); - cachedInnerUsage += RecursiveDynamicUsage(*block_it); - } - } - - // Remove entries that are in this block. - void removeForBlock(const std::vector& vtx) - { - // Short-circuit in the common case of a block being added to the tip - if (queuedTx.empty()) { - return; - } - for (const auto& tx : vtx) { - auto iter = iters_by_txid.find(tx->GetHash()); - if (iter != iters_by_txid.end()) { - auto list_iter = iter->second; - iters_by_txid.erase(iter); - cachedInnerUsage -= RecursiveDynamicUsage(*list_iter); - queuedTx.erase(list_iter); - } - } - } - - // Remove the first entry and update memory usage. - CTransactionRef take_first() - { - CTransactionRef first_tx; - if (!queuedTx.empty()) { - first_tx = queuedTx.front(); - cachedInnerUsage -= RecursiveDynamicUsage(queuedTx.front()); - iters_by_txid.erase(queuedTx.front()->GetHash()); - queuedTx.pop_front(); - } - return first_tx; - } - - // Clear all data structures and return the list of transactions. - std::list take() { - std::list ret = std::move(queuedTx); - clear(); - return ret; - } -}; - #endif // BITCOIN_TXMEMPOOL_H diff --git a/src/validation.cpp b/src/validation.cpp index c7e31ef6916609..5591e0c9c0b4ff 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include