From cf5f1faa037e9a40a5029cc7dd4ee61454b62466 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 | 128 ++++++++++++++++++ .../validation_chainstatemanager_tests.cpp | 1 + src/txmempool.h | 114 ---------------- src/validation.cpp | 1 + 6 files changed, 133 insertions(+), 115 deletions(-) create mode 100644 src/kernel/disconnected_transactions.h diff --git a/src/Makefile.am b/src/Makefile.am index feed4a0061..fa063dac85 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 096165bd1a..72ae9d6c5c 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 0000000000..a5d02c33ee --- /dev/null +++ b/src/kernel/disconnected_transactions.h @@ -0,0 +1,128 @@ +// 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. + * + * Order of queuedTx: + * The front of the list should be the most recently-confirmed transactions (transactions at the + * end of vtx of blocks closer to the tip). If memory usage grows too large, we trim from the front + * of the list. After trimming, transactions can be re-added to the mempool from the back of the + * list to the front without running into missing inputs. + */ +class DisconnectedBlockTransactions { +private: + /** Cached dynamic memory usage for the CTransactions (memory for the shared pointers is + * included in the container calculations). */ + uint64_t cachedInnerUsage = 0; + std::list queuedTx; + using TxList = decltype(queuedTx); + std::unordered_map iters_by_txid; + +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, iterating through vtx in reverse order. Callers should call + * this function for blocks in descending order by block height. + * We assume that callers never pass multiple transactions with the same txid, otherwise things + * can go very wrong in removeForBlock due to queuedTx containing an item without a + * corresponding entry in iters_by_txid. + */ + void AddTransactionsFromBlock(const std::vector& vtx) + { + 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 any 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; + } + + size_t size() const { return queuedTx.size(); } + + void clear() + { + cachedInnerUsage = 0; + iters_by_txid.clear(); + queuedTx.clear(); + } + + /** 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 9e359eeee4..a793f56cba 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 44f79a9826..ad4d9e7101 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -836,118 +836,4 @@ public: * 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. - * - * Order of queuedTx: - * The front of the list should be the most recently-confirmed transactions (transactions at the - * end of vtx of blocks closer to the tip). If memory usage grows too large, we trim from the front - * of the list. After trimming, transactions can be re-added to the mempool from the back of the - * list to the front without running into missing inputs. - */ -class DisconnectedBlockTransactions { -private: - /** Cached dynamic memory usage for the CTransactions (memory for the shared pointers is - * included in the container calculations). */ - uint64_t cachedInnerUsage = 0; - std::list queuedTx; - using TxList = decltype(queuedTx); - std::unordered_map iters_by_txid; - -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, iterating through vtx in reverse order. Callers should call - * this function for blocks in descending order by block height. - * We assume that callers never pass multiple transactions with the same txid, otherwise things - * can go very wrong in removeForBlock due to queuedTx containing an item without a - * corresponding entry in iters_by_txid. - */ - void AddTransactionsFromBlock(const std::vector& vtx) - { - 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 any 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; - } - - size_t size() const { return queuedTx.size(); } - - void clear() - { - cachedInnerUsage = 0; - iters_by_txid.clear(); - queuedTx.clear(); - } - - /** 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 b1561f4906..84846add80 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include