diff --git a/src/txmempool.h b/src/txmempool.h index 92c4d9f9d..4a83c3f84 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -23,6 +23,7 @@ #include "boost/multi_index_container.hpp" #include "boost/multi_index/ordered_index.hpp" #include "boost/multi_index/hashed_index.hpp" +#include #include @@ -191,7 +192,7 @@ private: const LockPoints& lp; }; -// extracts a TxMemPoolEntry's transaction hash +// extracts a transaction hash from CTxMempoolEntry or CTransactionRef struct mempoolentry_txid { typedef uint256 result_type; @@ -199,6 +200,11 @@ struct mempoolentry_txid { return entry.GetTx().GetHash(); } + + result_type operator() (const CTransactionRef& tx) const + { + return tx->GetHash(); + } }; /** \class CompareTxMemPoolEntryByDescendantScore @@ -676,4 +682,95 @@ public: bool HaveCoins(const uint256 &txid) const; }; +/** + * 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. + */ + +// multi_index tag names +struct txid_index {}; +struct insertion_order {}; + +struct DisconnectedBlockTransactions { + typedef boost::multi_index_container< + CTransactionRef, + boost::multi_index::indexed_by< + // sorted by txid + boost::multi_index::hashed_unique< + boost::multi_index::tag, + mempoolentry_txid, + SaltedTxidHasher + >, + // sorted by order in the blockchain + boost::multi_index::sequenced< + boost::multi_index::tag + > + > + > indexed_disconnected_transactions; + + // 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()); } + + 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 { + return memusage::MallocUsage(sizeof(CTransactionRef) + 6 * sizeof(void*)) * queuedTx.size() + cachedInnerUsage; + } + + void addTransaction(const CTransactionRef& tx) + { + queuedTx.insert(tx); + cachedInnerUsage += RecursiveDynamicUsage(*tx) + memusage::DynamicUsage(tx); + } + + // Remove entries based on txid_index, and update memory usage. + 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 (auto const &tx : vtx) { + auto it = queuedTx.find(tx->GetHash()); + if (it != queuedTx.end()) { + cachedInnerUsage -= RecursiveDynamicUsage(**it) + memusage::DynamicUsage(*it); + queuedTx.erase(it); + } + } + } + + // Remove an entry by insertion_order index, and update memory usage. + void removeEntry(indexed_disconnected_transactions::index::type::iterator entry) + { + cachedInnerUsage -= RecursiveDynamicUsage(**entry) + memusage::DynamicUsage(*entry); + queuedTx.get().erase(entry); + } + + void clear() + { + cachedInnerUsage = 0; + queuedTx.clear(); + } +}; + #endif // BITCOIN_TXMEMPOOL_H diff --git a/src/validation.cpp b/src/validation.cpp index 75a35756d..88b071825 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -539,6 +539,56 @@ static bool IsCurrentForFeeEstimation() return true; } +/* 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(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(mempool, stateDummy, *it, false, NULL, NULL, 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)->GetHash())) { + vHashUpdate.push_back((*it)->GetHash()); + } + ++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(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS); + // Re-limit mempool size, in case we added any transactions + LimitMempoolSize(mempool, GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); +} + bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const CTransactionRef& ptx, bool fLimitFree, bool* pfMissingInputs, int64_t nAcceptTime, std::list* plTxnReplaced, bool fOverrideMempoolLimit, const CAmount& nAbsurdFee, std::vector& vHashTxnToUncache) @@ -2119,8 +2169,17 @@ void static UpdateTip(CBlockIndex *pindexNew, const CChainParams& chainParams) { } -/** Disconnect chainActive's tip. You probably want to call mempool.removeForReorg and manually re-limit mempool size after this, with cs_main held. */ -bool static DisconnectTip(CValidationState& state, const CChainParams& chainparams, bool fBare = false) +/** 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. + * with cs_main held. + * + * If disconnectpool is NULL, then no disconnected transactions are added to + * disconnectpool (note that the caller is responsible for mempool consistency + * in any case). + */ +bool static DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions *disconnectpool) { CBlockIndex *pindexDelete = chainActive.Tip(); assert(pindexDelete); @@ -2143,25 +2202,17 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara if (!FlushStateToDisk(state, FLUSH_STATE_IF_NEEDED)) return false; - if (!fBare) { - // Resurrect mempool transactions from the disconnected block. - std::vector vHashUpdate; - for (const auto& it : block.vtx) { - const CTransaction& tx = *it; - // ignore validation errors in resurrected transactions - CValidationState stateDummy; - if (tx.IsCoinBase() || !AcceptToMemoryPool(mempool, stateDummy, it, false, NULL, NULL, true)) { - mempool.removeRecursive(tx, MemPoolRemovalReason::REORG); - } else if (mempool.exists(tx.GetHash())) { - vHashUpdate.push_back(tx.GetHash()); - } + if (disconnectpool) { + // Save transactions to re-add to mempool at end of reorg + for (auto it = block.vtx.rbegin(); it != block.vtx.rend(); ++it) { + disconnectpool->addTransaction(*it); + } + while (disconnectpool->DynamicMemoryUsage() > MAX_DISCONNECTED_TX_POOL_SIZE * 1000) { + // 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); } - // 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 this - // block that were added back and cleans up the mempool state. - mempool.UpdateTransactionsFromBlock(vHashUpdate); } // Update chainActive and related variables. @@ -2249,7 +2300,7 @@ public: * * The block is added to connectTrace if connection succeeds. */ -bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr& pblock, ConnectTrace& connectTrace) +bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool) { assert(pindexNew->pprev == chainActive.Tip()); // Read block from disk. @@ -2291,6 +2342,7 @@ bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, LogPrint(BCLog::BENCH, " - Writing chainstate: %.2fms [%.2fs]\n", (nTime5 - nTime4) * 0.001, nTimeChainState * 0.000001); // Remove conflicting transactions from the mempool.; mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight); + disconnectpool.removeForBlock(blockConnecting.vtx); // Update chainActive & related variables. UpdateTip(pindexNew, chainparams); @@ -2384,9 +2436,14 @@ static bool ActivateBestChainStep(CValidationState& state, const CChainParams& c // Disconnect active blocks which are no longer in the best chain. bool fBlocksDisconnected = false; + DisconnectedBlockTransactions disconnectpool; while (chainActive.Tip() && chainActive.Tip() != pindexFork) { - if (!DisconnectTip(state, chainparams)) + if (!DisconnectTip(state, chainparams, &disconnectpool)) { + // This is likely a fatal error, but keep the mempool consistent, + // just in case. Only remove from the mempool in this case. + UpdateMempoolForReorg(disconnectpool, false); return false; + } fBlocksDisconnected = true; } @@ -2409,7 +2466,7 @@ static bool ActivateBestChainStep(CValidationState& state, const CChainParams& c // Connect new blocks. BOOST_REVERSE_FOREACH(CBlockIndex *pindexConnect, vpindexToConnect) { - if (!ConnectTip(state, chainparams, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr(), connectTrace)) { + if (!ConnectTip(state, chainparams, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr(), connectTrace, disconnectpool)) { if (state.IsInvalid()) { // The block violates a consensus rule. if (!state.CorruptionPossible()) @@ -2420,6 +2477,9 @@ static bool ActivateBestChainStep(CValidationState& state, const CChainParams& c break; } else { // 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(disconnectpool, false); return false; } } else { @@ -2434,8 +2494,9 @@ static bool ActivateBestChainStep(CValidationState& state, const CChainParams& c } if (fBlocksDisconnected) { - mempool.removeForReorg(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS); - LimitMempoolSize(mempool, GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); + // If any blocks were disconnected, disconnectpool may be non empty. Add + // any disconnected transactions back to the mempool. + UpdateMempoolForReorg(disconnectpool, true); } mempool.check(pcoinsTip); @@ -2584,6 +2645,7 @@ bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, C setDirtyBlockIndex.insert(pindex); setBlockIndexCandidates.erase(pindex); + DisconnectedBlockTransactions disconnectpool; while (chainActive.Contains(pindex)) { CBlockIndex *pindexWalk = chainActive.Tip(); pindexWalk->nStatus |= BLOCK_FAILED_CHILD; @@ -2591,13 +2653,17 @@ bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, C setBlockIndexCandidates.erase(pindexWalk); // ActivateBestChain considers blocks already in chainActive // unconditionally valid already, so force disconnect away from it. - if (!DisconnectTip(state, chainparams)) { - mempool.removeForReorg(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS); + if (!DisconnectTip(state, chainparams, &disconnectpool)) { + // It's probably hopeless to try to make the mempool consistent + // here if DisconnectTip failed, but we can try. + UpdateMempoolForReorg(disconnectpool, false); return false; } } - LimitMempoolSize(mempool, GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); + // DisconnectTip will add transactions to disconnectpool; try to add these + // back to the mempool. + UpdateMempoolForReorg(disconnectpool, true); // The resulting new best tip may not be in setBlockIndexCandidates anymore, so // add it again. @@ -2610,7 +2676,6 @@ bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, C } InvalidChainFound(pindex); - mempool.removeForReorg(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS); uiInterface.NotifyBlockTip(IsInitialBlockDownload(), pindex->pprev); return true; } @@ -3724,7 +3789,7 @@ bool RewindBlockIndex(const CChainParams& params) // of the blockchain). break; } - if (!DisconnectTip(state, params, true)) { + if (!DisconnectTip(state, params, NULL)) { return error("RewindBlockIndex: unable to disconnect block at height %i", pindex->nHeight); } // Occasionally flush state to disk. diff --git a/src/validation.h b/src/validation.h index c046ba923..8c91a3806 100644 --- a/src/validation.h +++ b/src/validation.h @@ -69,6 +69,8 @@ static const unsigned int DEFAULT_DESCENDANT_LIMIT = 25; static const unsigned int DEFAULT_DESCENDANT_SIZE_LIMIT = 101; /** Default for -mempoolexpiry, expiration time for mempool transactions in hours */ static const unsigned int DEFAULT_MEMPOOL_EXPIRY = 336; +/** Maximum kilobytes for transactions to store for processing during reorg */ +static const unsigned int MAX_DISCONNECTED_TX_POOL_SIZE = 20000; /** 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) */