Skip to content

[Core] Store disconnected blocks txes outside mempool during reorg #2268

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
random-zebra merged 1 commit into from
Apr 6, 2021
Merged

[Core] Store disconnected blocks txes outside mempool during reorg #2268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/memusage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,12 @@ static inline size_t RecursiveDynamicUsage(const X& x)
return DynamicUsage(x);
}

template<typename X>
static inline size_t RecursiveDynamicUsage(const std::shared_ptr<X>& p)
{
return p ? memusage::DynamicUsage(p) + RecursiveDynamicUsage(*p) : 0;
}

}

#endif // BITCOIN_MEMUSAGE_H
98 changes: 97 additions & 1 deletion src/txmempool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "boost/multi_index_container.hpp"
#include "boost/multi_index/ordered_index.hpp"
#include "boost/multi_index/hashed_index.hpp"
#include <boost/multi_index/sequenced_index.hpp>

#include <boost/signals2/signal.hpp>

Expand Down Expand Up @@ -182,14 +183,19 @@ struct update_fee_delta
int64_t feeDelta;
};

// extracts a TxMemPoolEntry's transaction hash
// extracts a transaction hash from CTxMempoolEntry or CTransactionRef
struct mempoolentry_txid
{
typedef uint256 result_type;
result_type operator() (const CTxMemPoolEntry &entry) const
{
return entry.GetTx().GetHash();
}

result_type operator() (const CTransactionRef& tx) const
{
return tx->GetHash();
}
};

/** \class CompareTxMemPoolEntryByDescendantScore
Expand Down Expand Up @@ -753,4 +759,94 @@ struct TxCoinAgePriorityCompare
}
};

/**
* 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<txid_index>,
mempoolentry_txid,
SaltedIdHasher
>,
// sorted by order in the blockchain
boost::multi_index::sequenced<
boost::multi_index::tag<insertion_order>
>
>
> 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 += memusage::RecursiveDynamicUsage(tx);
}

// Remove entries based on txid_index, and update memory usage.
void removeForBlock(const std::vector<CTransactionRef>& 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 -= memusage::RecursiveDynamicUsage(*it);
queuedTx.erase(it);
}
}
}

// Remove an entry by insertion_order index, and update memory usage.
void removeEntry(indexed_disconnected_transactions::index<insertion_order>::type::iterator entry)
{
cachedInnerUsage -= memusage::RecursiveDynamicUsage(*entry);
queuedTx.get<insertion_order>().erase(entry);
}

void clear()
{
cachedInnerUsage = 0;
queuedTx.clear();
}
};

#endif // BITCOIN_TXMEMPOOL_H
130 changes: 103 additions & 27 deletions src/validation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,59 @@ std::string FormatStateMessage(const CValidationState &state)
state.GetRejectCode());
}

/* 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);
AssertLockHeld(mempool.cs);
std::vector<uint256> 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<insertion_order>().rbegin();
while (it != disconnectpool.queuedTx.get<insertion_order>().rend()) {
// ignore validation errors in resurrected transactions
CValidationState stateDummy;
if (!fAddToMempool || (*it)->IsCoinBase() || (*it)->IsCoinStake() ||
!AcceptToMemoryPool(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)->GetHash())) {
vHashUpdate.emplace_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, gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000,
gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60);
}

bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const CTransactionRef& _tx, bool fLimitFree,
bool* pfMissingInputs, int64_t nAcceptTime, bool fOverrideMempoolLimit, bool fRejectAbsurdFee, bool ignoreFees,
std::vector<COutPoint>& coins_to_uncache)
Expand Down Expand Up @@ -1853,10 +1906,20 @@ void static UpdateTip(CBlockIndex* pindexNew)
}
}

/** 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)
/** 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)
{
AssertLockHeld(cs_main);
AssertLockHeld(mempool.cs);
CBlockIndex* pindexDelete = chainActive.Tip();
assert(pindexDelete);
// Read block from disk.
Expand All @@ -1879,23 +1942,18 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara
// Write the chain state to disk, if necessary.
if (!FlushStateToDisk(state, FLUSH_STATE_IF_NEEDED))
return false;
// Resurrect mempool transactions from the disconnected block.
std::vector<uint256> vHashUpdate;
for (const auto& tx : block.vtx) {
// ignore validation errors in resurrected transactions
CValidationState stateDummy;
if (tx->IsCoinBase() || tx->IsCoinStake() || !AcceptToMemoryPool(mempool, stateDummy, tx, false, nullptr, 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<insertion_order>().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 MN manager cache
// replace the cached hash of pindexDelete with the hash of the block
// at depth CACHED_BLOCK_HASHES if it exists, or empty hash otherwise.
Expand Down Expand Up @@ -1997,8 +2055,10 @@ class ConnectTrace {
*
* The block is added to connectTrace if connection succeeds.
*/
bool static ConnectTip(CValidationState& state, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, bool fAlreadyChecked, ConnectTrace& connectTrace)
bool static ConnectTip(CValidationState& state, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, bool fAlreadyChecked, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool)
{
AssertLockHeld(cs_main);
AssertLockHeld(mempool.cs);
assert(pindexNew->pprev == chainActive.Tip());

if (pblock == NULL)
Expand Down Expand Up @@ -2052,6 +2112,7 @@ bool static ConnectTip(CValidationState& state, CBlockIndex* pindexNew, const st

// Remove conflicting transactions from the mempool.
mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight, !IsInitialBlockDownload());
disconnectpool.removeForBlock(blockConnecting.vtx);
// Update chainActive & related variables.
UpdateTip(pindexNew);
// Update MN manager cache
Expand Down Expand Up @@ -2147,16 +2208,22 @@ static void PruneBlockIndexCandidates()
static bool ActivateBestChainStep(CValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool fAlreadyChecked, bool& fInvalidFound, ConnectTrace& connectTrace)
{
AssertLockHeld(cs_main);
AssertLockHeld(mempool.cs);
if (pblock == NULL)
fAlreadyChecked = false;
const CBlockIndex* pindexOldTip = chainActive.Tip();
const CBlockIndex* pindexFork = chainActive.FindFork(pindexMostWork);

// 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, Params()))
if (!DisconnectTip(state, Params(), &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;
}

Expand All @@ -2179,7 +2246,7 @@ static bool ActivateBestChainStep(CValidationState& state, CBlockIndex* pindexMo

// Connect new blocks.
for (CBlockIndex* pindexConnect : reverse_iterate(vpindexToConnect)) {
if (!ConnectTip(state, pindexConnect, (pindexConnect == pindexMostWork) ? pblock : std::shared_ptr<const CBlock>(), fAlreadyChecked, connectTrace)) {
if (!ConnectTip(state, pindexConnect, (pindexConnect == pindexMostWork) ? pblock : std::shared_ptr<const CBlock>(), fAlreadyChecked, connectTrace, disconnectpool)) {
if (state.IsInvalid()) {
// The block violates a consensus rule.
if (!state.CorruptionPossible())
Expand All @@ -2190,6 +2257,9 @@ static bool ActivateBestChainStep(CValidationState& state, CBlockIndex* pindexMo
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 {
Expand All @@ -2204,8 +2274,9 @@ static bool ActivateBestChainStep(CValidationState& state, CBlockIndex* pindexMo
}

if (fBlocksDisconnected) {
mempool.removeForReorg(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS);
LimitMempoolSize(mempool, gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, gArgs.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);

Expand Down Expand Up @@ -2252,6 +2323,7 @@ bool ActivateBestChain(CValidationState& state, std::shared_ptr<const CBlock> pb

{
LOCK(cs_main);
LOCK(mempool.cs); // Lock transaction pool for at least as long as it takes for connectTrace to be consumed
CBlockIndex* starting_tip = chainActive.Tip();
bool blocks_connected = false;
do {
Expand Down Expand Up @@ -2300,7 +2372,6 @@ bool ActivateBestChain(CValidationState& state, std::shared_ptr<const CBlock> pb
uiInterface.NotifyBlockTip(fInitialDownload, pindexNewTip);
}
}
// When we reach this point, we switched to a new tip (stored in pindexNewTip).

// We check shutdown only after giving ActivateBestChainStep a chance to run once so that we
// never shutdown before connecting the genesis block during LoadChainTip(). Previously this
Expand Down Expand Up @@ -2328,20 +2399,26 @@ bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, C
setDirtyBlockIndex.insert(pindex);
setBlockIndexCandidates.erase(pindex);

LOCK(mempool.cs); // Lock for as long as disconnectpool is in scope to make sure UpdateMempoolForReorg is called after DisconnectTip without unlocking in between
DisconnectedBlockTransactions disconnectpool;
while (chainActive.Contains(pindex)) {
CBlockIndex* pindexWalk = chainActive.Tip();
pindexWalk->nStatus |= BLOCK_FAILED_CHILD;
setDirtyBlockIndex.insert(pindexWalk);
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, gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, gArgs.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.
Expand All @@ -2354,7 +2431,6 @@ bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, C
}

InvalidChainFound(pindex);
mempool.removeForReorg(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS);
return true;
}

Expand Down
2 changes: 2 additions & 0 deletions src/validation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ static const unsigned int DEFAULT_LIMITFREERELAY = 30;
/** The maximum size for transactions we're willing to relay/mine */
static const unsigned int MAX_STANDARD_TX_SIZE = 100000;
static const unsigned int MAX_ZEROCOIN_TX_SIZE = 150000;
/** Maximum kilobytes for transactions to store for processing during reorg */
static const unsigned int MAX_DISCONNECTED_TX_POOL_SIZE = 20000;
/** Default for -checkblocks */
static const signed int DEFAULT_CHECKBLOCKS = 10;
/** The maximum size of a blk?????.dat file (since 0.8) */
Expand Down