diff --git a/src/txmempool.h b/src/txmempool.h --- a/src/txmempool.h +++ b/src/txmempool.h @@ -682,12 +682,21 @@ void TrimToSize(size_t sizelimit, std::vector *pvNoSpendsRemaining = nullptr); - /** Expire all transaction (and their dependencies) in the mempool older - * than time. Return the number of removed transactions. */ + /** + * Expire all transaction (and their dependencies) in the mempool older than + * time. Return the number of removed transactions. + */ int Expire(int64_t time); - /** Returns false if the transaction is in the mempool and not within the - * chain limit specified. */ + /** + * Reduce the size of the mempool by expiring and then trimming the mempool. + */ + void LimitSize(size_t limit, unsigned long age); + + /** + * Returns false if the transaction is in the mempool and not within the + * chain limit specified. + */ bool TransactionWithinChainLimit(const uint256 &txid, size_t chainLimit) const; @@ -753,7 +762,8 @@ */ void UpdateForDescendants(txiter updateIt, cacheMap &cachedDescendants, const std::set &setExclude); - /** Update ancestors of hash to add/remove it as a descendant transaction. + /** + * Update ancestors of hash to add/remove it as a descendant transaction. */ void UpdateAncestorsOf(bool add, txiter hash, setEntries &setAncestors); /** Set ancestor state for an entry */ @@ -810,7 +820,7 @@ /** * 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 @@ -822,12 +832,12 @@ * that are included in blocks in the new chain, and then process the remaining * still-unconfirmed transactions at the end. */ - // multi_index tag names struct txid_index {}; struct insertion_order {}; -struct DisconnectedBlockTransactions { +class DisconnectedBlockTransactions { +private: typedef boost::multi_index_container< CTransactionRef, boost::multi_index::indexed_by< // sorted by txid @@ -839,6 +849,10 @@ boost::multi_index::tag>>> indexed_disconnected_transactions; + indexed_disconnected_transactions queuedTx; + uint64_t cachedInnerUsage = 0; + +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. @@ -849,9 +863,6 @@ // reorg, besides draining this object). ~DisconnectedBlockTransactions() { assert(queuedTx.empty()); } - indexed_disconnected_transactions queuedTx; - uint64_t cachedInnerUsage = 0; - // Estimate the overhead of queuedTx to be 6 pointers + an allocation, as // no exact formula for boost::multi_index_contained is implemented. size_t DynamicMemoryUsage() const { @@ -861,6 +872,10 @@ cachedInnerUsage; } + // Add entries for a block while reconstructing the topological ordering so + // they can be added back to the mempool simply. + void addForBlock(const std::vector &vtx); + void addTransaction(const CTransactionRef &tx) { queuedTx.insert(tx); cachedInnerUsage += RecursiveDynamicUsage(tx); @@ -873,7 +888,7 @@ return; } for (auto const &tx : vtx) { - auto it = queuedTx.find(tx->GetHash()); + auto it = queuedTx.find(tx->GetId()); if (it != queuedTx.end()) { cachedInnerUsage -= RecursiveDynamicUsage(*it); queuedTx.erase(it); @@ -892,6 +907,21 @@ cachedInnerUsage = 0; queuedTx.clear(); } + + /** + * Make mempool consistent after a reorg, by re-adding or recursively + * erasing disconnected block transactions from the mempool, and also + * removing any other transactions from the mempool that are no longer valid + * given the new tip/height. + * + * Note: we assume that disconnectpool only contains transactions that are + * NOT confirmed in the current chain nor already in the mempool (otherwise, + * in-mempool descendants of such transactions would be removed). + * + * Passing fAddToMempool=false will skip trying to add the transactions + * back, and instead just erase from the mempool as needed. + */ + void updateMempoolForReorg(const Config &config, bool fAddToMempool); }; #endif // BITCOIN_TXMEMPOOL_H diff --git a/src/txmempool.cpp b/src/txmempool.cpp --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1108,6 +1108,20 @@ return stage.size(); } +void CTxMemPool::LimitSize(size_t limit, unsigned long age) { + int expired = Expire(GetTime() - age); + if (expired != 0) { + LogPrint(BCLog::MEMPOOL, + "Expired %i transactions from the memory pool\n", expired); + } + + std::vector vNoSpendsRemaining; + TrimToSize(limit, &vNoSpendsRemaining); + for (const COutPoint &removed : vNoSpendsRemaining) { + pcoinsTip->Uncache(removed); + } +} + bool CTxMemPool::addUnchecked(const uint256 &hash, const CTxMemPoolEntry &entry, bool validFeeEstimate) { LOCK(cs); @@ -1250,3 +1264,65 @@ SaltedTxidHasher::SaltedTxidHasher() : k0(GetRand(std::numeric_limits::max())), k1(GetRand(std::numeric_limits::max())) {} + +/** Maximum bytes for transactions to store for processing during reorg */ +static const size_t MAX_DISCONNECTED_TX_POOL_SIZE = 20 * DEFAULT_MAX_BLOCK_SIZE; + +void DisconnectedBlockTransactions::addForBlock( + const std::vector &vtx) { + // Save transactions to re-add to mempool at end of reorg + for (const auto &tx : boost::adaptors::reverse(vtx)) { + addTransaction(tx); + } + while (DynamicMemoryUsage() > MAX_DISCONNECTED_TX_POOL_SIZE) { + // Drop the earliest entry, and remove its children from the + // mempool. + auto it = queuedTx.get().begin(); + mempool.removeRecursive(**it, MemPoolRemovalReason::REORG); + removeEntry(it); + } +} + +void DisconnectedBlockTransactions::updateMempoolForReorg(const Config &config, + bool fAddToMempool) { + AssertLockHeld(cs_main); + std::vector vHashUpdate; + // disconnectpool's insertion_order index sorts the entries from oldest to + // newest, but the oldest entry will be the last tx from the latest mined + // block that was disconnected. + // Iterate disconnectpool in reverse, so that we add transactions back to + // the mempool starting with the earliest transaction that had been + // previously seen in a block. + auto it = queuedTx.get().rbegin(); + while (it != queuedTx.get().rend()) { + // ignore validation errors in resurrected transactions + CValidationState stateDummy; + if (!fAddToMempool || (*it)->IsCoinBase() || + !AcceptToMemoryPool(config, mempool, stateDummy, *it, false, + nullptr, true)) { + // If the transaction doesn't make it in to the mempool, remove any + // transactions that depend on it (which would now be orphans). + mempool.removeRecursive(**it, MemPoolRemovalReason::REORG); + } else if (mempool.exists((*it)->GetId())) { + vHashUpdate.push_back((*it)->GetId()); + } + ++it; + } + + queuedTx.clear(); + + // AcceptToMemoryPool/addUnchecked all assume that new mempool entries have + // no in-mempool children, which is generally not true when adding + // previously-confirmed transactions back to the mempool. + // UpdateTransactionsFromBlock finds descendants of any transactions in the + // disconnectpool that were added back and cleans up the mempool state. + mempool.UpdateTransactionsFromBlock(vHashUpdate); + + // We also need to remove any now-immature transactions + mempool.removeForReorg(config, pcoinsTip, chainActive.Tip()->nHeight + 1, + STANDARD_LOCKTIME_VERIFY_FLAGS); + // Re-limit mempool size, in case we added any transactions + mempool.LimitSize( + gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, + gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); +} diff --git a/src/validation.h b/src/validation.h --- a/src/validation.h +++ b/src/validation.h @@ -80,9 +80,6 @@ /** Default for -mempoolexpiry, expiration time for mempool transactions in * hours */ static const unsigned int DEFAULT_MEMPOOL_EXPIRY = 336; -/** Maximum bytes for transactions to store for processing during reorg */ -static const unsigned int MAX_DISCONNECTED_TX_POOL_SIZE = - 20 * DEFAULT_MAX_BLOCK_SIZE; /** The maximum size of a blk?????.dat file (since 0.8) */ static const unsigned int MAX_BLOCKFILE_SIZE = 0x8000000; // 128 MiB /** The pre-allocation chunk size for blk?????.dat files (since 0.8) */ diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -513,21 +513,6 @@ return true; } -static void LimitMempoolSize(CTxMemPool &pool, size_t limit, - unsigned long age) { - int expired = pool.Expire(GetTime() - age); - if (expired != 0) { - LogPrint(BCLog::MEMPOOL, - "Expired %i transactions from the memory pool\n", expired); - } - - std::vector vNoSpendsRemaining; - pool.TrimToSize(limit, &vNoSpendsRemaining); - for (const COutPoint &removed : vNoSpendsRemaining) { - pcoinsTip->Uncache(removed); - } -} - /** Convert CValidationState to a human-readable message for logging */ std::string FormatStateMessage(const CValidationState &state) { return strprintf( @@ -606,10 +591,10 @@ // . Defaults to the pre-defined timestamp when not set. static bool IsReplayProtectionEnabled(const Config &config, int64_t nMedianTimePast) { - return nMedianTimePast >= gArgs.GetArg("-replayprotectionactivationtime", - config.GetChainParams() - .GetConsensus() - .greatWallActivationTime); + return nMedianTimePast >= + gArgs.GetArg( + "-replayprotectionactivationtime", + config.GetChainParams().GetConsensus().greatWallActivationTime); } static bool IsReplayProtectionEnabled(const Config &config, @@ -626,64 +611,6 @@ return IsReplayProtectionEnabled(config, chainActive.Tip()); } -/** - * Make mempool consistent after a reorg, by re-adding or recursively erasing - * disconnected block transactions from the mempool, and also removing any other - * transactions from the mempool that are no longer valid given the new - * tip/height. - * - * Note: we assume that disconnectpool only contains transactions that are NOT - * confirmed in the current chain nor already in the mempool (otherwise, - * in-mempool descendants of such transactions would be removed). - * - * Passing fAddToMempool=false will skip trying to add the transactions back, - * and instead just erase from the mempool as needed. - */ -void UpdateMempoolForReorg(const Config &config, - DisconnectedBlockTransactions &disconnectpool, - bool fAddToMempool) { - AssertLockHeld(cs_main); - std::vector vHashUpdate; - // disconnectpool's insertion_order index sorts the entries from oldest to - // newest, but the oldest entry will be the last tx from the latest mined - // block that was disconnected. - // Iterate disconnectpool in reverse, so that we add transactions back to - // the mempool starting with the earliest transaction that had been - // previously seen in a block. - auto it = disconnectpool.queuedTx.get().rbegin(); - while (it != disconnectpool.queuedTx.get().rend()) { - // ignore validation errors in resurrected transactions - CValidationState stateDummy; - if (!fAddToMempool || (*it)->IsCoinBase() || - !AcceptToMemoryPool(config, mempool, stateDummy, *it, false, - nullptr, true)) { - // If the transaction doesn't make it in to the mempool, remove any - // transactions that depend on it (which would now be orphans). - mempool.removeRecursive(**it, MemPoolRemovalReason::REORG); - } else if (mempool.exists((*it)->GetId())) { - vHashUpdate.push_back((*it)->GetId()); - } - ++it; - } - - disconnectpool.queuedTx.clear(); - // AcceptToMemoryPool/addUnchecked all assume that new mempool entries have - // no in-mempool children, which is generally not true when adding - // previously-confirmed transactions back to the mempool. - // UpdateTransactionsFromBlock finds descendants of any transactions in the - // disconnectpool that were added back and cleans up the mempool state. - mempool.UpdateTransactionsFromBlock(vHashUpdate); - - // We also need to remove any now-immature transactions - mempool.removeForReorg(config, pcoinsTip, chainActive.Tip()->nHeight + 1, - STANDARD_LOCKTIME_VERIFY_FLAGS); - // Re-limit mempool size, in case we added any transactions - LimitMempoolSize( - mempool, - gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, - gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); -} - // Used to avoid mempool polluting consensus critical paths if CCoinsViewMempool // were somehow broken and returning the wrong scriptPubKeys static bool @@ -1061,8 +988,7 @@ // Trim mempool and check if tx was trimmed. if (!fOverrideMempoolLimit) { - LimitMempoolSize( - pool, + pool.LimitSize( gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); @@ -2539,7 +2465,7 @@ * Disconnect chainActive's tip. * After calling, the mempool will be in an inconsistent state, with * transactions from disconnected blocks being added to disconnectpool. You - * should make the mempool consistent again by calling UpdateMempoolForReorg. + * should make the mempool consistent again by calling updateMempoolForReorg. * with cs_main held. * * If disconnectpool is nullptr, then no disconnected transactions are added to @@ -2605,18 +2531,7 @@ } if (disconnectpool) { - // Save transactions to re-add to mempool at end of reorg - for (const auto &tx : boost::adaptors::reverse(block.vtx)) { - disconnectpool->addTransaction(tx); - } - while (disconnectpool->DynamicMemoryUsage() > - MAX_DISCONNECTED_TX_POOL_SIZE) { - // Drop the earliest entry, and remove its children from the - // mempool. - auto it = disconnectpool->queuedTx.get().begin(); - mempool.removeRecursive(**it, MemPoolRemovalReason::REORG); - disconnectpool->removeEntry(it); - } + disconnectpool->addForBlock(block.vtx); } // Update chainActive and related variables. @@ -2908,7 +2823,7 @@ if (!DisconnectTip(config, state, &disconnectpool)) { // This is likely a fatal error, but keep the mempool consistent, // just in case. Only remove from the mempool in this case. - UpdateMempoolForReorg(config, disconnectpool, false); + disconnectpool.updateMempoolForReorg(config, false); return false; } @@ -2956,7 +2871,7 @@ // A system error occurred (disk space, database error, ...). // Make the mempool consistent with the current tip, just in // case any observers try to use it before shutdown. - UpdateMempoolForReorg(config, disconnectpool, false); + disconnectpool.updateMempoolForReorg(config, false); return false; } else { PruneBlockIndexCandidates(); @@ -2974,7 +2889,7 @@ if (fBlocksDisconnected) { // If any blocks were disconnected, disconnectpool may be non empty. Add // any disconnected transactions back to the mempool. - UpdateMempoolForReorg(config, disconnectpool, true); + disconnectpool.updateMempoolForReorg(config, true); } mempool.check(pcoinsTip); @@ -3158,14 +3073,14 @@ if (!DisconnectTip(config, state, &disconnectpool)) { // It's probably hopeless to try to make the mempool consistent // here if DisconnectTip failed, but we can try. - UpdateMempoolForReorg(config, disconnectpool, false); + disconnectpool.updateMempoolForReorg(config, false); return false; } } // DisconnectTip will add transactions to disconnectpool; try to add these // back to the mempool. - UpdateMempoolForReorg(config, disconnectpool, true); + disconnectpool.updateMempoolForReorg(config, true); // The resulting new best tip may not be in setBlockIndexCandidates anymore, // so add it again.