Merge #9371: Notify on removal
094e4b3
Better document usage of SyncTransaction (Alex Morcos)4afbde6
Introduce MemPoolConflictRemovalTracker (Alex Morcos)ff25c32
mempool: add notification for added/removed entries (Wladimir J. van der Laan)
This commit is contained in:
commit
4a1dc35ca5
5 changed files with 103 additions and 21 deletions
|
@ -393,6 +393,7 @@ void CTxMemPool::AddTransactionsUpdated(unsigned int n)
|
|||
|
||||
bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool validFeeEstimate)
|
||||
{
|
||||
NotifyEntryAdded(entry.GetSharedTx());
|
||||
// Add to memory pool without checking anything.
|
||||
// Used by main.cpp AcceptToMemoryPool(), which DOES do
|
||||
// all the appropriate checks.
|
||||
|
@ -449,8 +450,9 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry,
|
|||
return true;
|
||||
}
|
||||
|
||||
void CTxMemPool::removeUnchecked(txiter it)
|
||||
void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
|
||||
{
|
||||
NotifyEntryRemoved(it->GetSharedTx(), reason);
|
||||
const uint256 hash = it->GetTx().GetHash();
|
||||
BOOST_FOREACH(const CTxIn& txin, it->GetTx().vin)
|
||||
mapNextTx.erase(txin.prevout);
|
||||
|
@ -502,7 +504,7 @@ void CTxMemPool::CalculateDescendants(txiter entryit, setEntries &setDescendants
|
|||
}
|
||||
}
|
||||
|
||||
void CTxMemPool::removeRecursive(const CTransaction &origTx)
|
||||
void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReason reason)
|
||||
{
|
||||
// Remove transaction from memory pool
|
||||
{
|
||||
|
@ -529,7 +531,8 @@ void CTxMemPool::removeRecursive(const CTransaction &origTx)
|
|||
BOOST_FOREACH(txiter it, txToRemove) {
|
||||
CalculateDescendants(it, setAllRemoves);
|
||||
}
|
||||
RemoveStaged(setAllRemoves, false);
|
||||
|
||||
RemoveStaged(setAllRemoves, false, reason);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -567,7 +570,7 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem
|
|||
for (txiter it : txToRemove) {
|
||||
CalculateDescendants(it, setAllRemoves);
|
||||
}
|
||||
RemoveStaged(setAllRemoves, false);
|
||||
RemoveStaged(setAllRemoves, false, MemPoolRemovalReason::REORG);
|
||||
}
|
||||
|
||||
void CTxMemPool::removeConflicts(const CTransaction &tx)
|
||||
|
@ -581,7 +584,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx)
|
|||
if (txConflict != tx)
|
||||
{
|
||||
ClearPrioritisation(txConflict.GetHash());
|
||||
removeRecursive(txConflict);
|
||||
removeRecursive(txConflict, MemPoolRemovalReason::CONFLICT);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -610,7 +613,7 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
|
|||
if (it != mapTx.end()) {
|
||||
setEntries stage;
|
||||
stage.insert(it);
|
||||
RemoveStaged(stage, true);
|
||||
RemoveStaged(stage, true, MemPoolRemovalReason::BLOCK);
|
||||
}
|
||||
removeConflicts(*tx);
|
||||
ClearPrioritisation(tx->GetHash());
|
||||
|
@ -989,11 +992,11 @@ size_t CTxMemPool::DynamicMemoryUsage() const {
|
|||
return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 15 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(mapLinks) + memusage::DynamicUsage(vTxHashes) + cachedInnerUsage;
|
||||
}
|
||||
|
||||
void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants) {
|
||||
void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants, MemPoolRemovalReason reason) {
|
||||
AssertLockHeld(cs);
|
||||
UpdateForRemoveFromMempool(stage, updateDescendants);
|
||||
BOOST_FOREACH(const txiter& it, stage) {
|
||||
removeUnchecked(it);
|
||||
removeUnchecked(it, reason);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -1009,7 +1012,7 @@ int CTxMemPool::Expire(int64_t time) {
|
|||
BOOST_FOREACH(txiter removeit, toremove) {
|
||||
CalculateDescendants(removeit, stage);
|
||||
}
|
||||
RemoveStaged(stage, false);
|
||||
RemoveStaged(stage, false, MemPoolRemovalReason::EXPIRY);
|
||||
return stage.size();
|
||||
}
|
||||
|
||||
|
@ -1118,7 +1121,7 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<uint256>* pvNoSpendsRe
|
|||
BOOST_FOREACH(txiter iter, stage)
|
||||
txn.push_back(iter->GetTx());
|
||||
}
|
||||
RemoveStaged(stage, false);
|
||||
RemoveStaged(stage, false, MemPoolRemovalReason::SIZELIMIT);
|
||||
if (pvNoSpendsRemaining) {
|
||||
BOOST_FOREACH(const CTransaction& tx, txn) {
|
||||
BOOST_FOREACH(const CTxIn& txin, tx.vin) {
|
||||
|
|
|
@ -25,6 +25,8 @@
|
|||
#include "boost/multi_index/ordered_index.hpp"
|
||||
#include "boost/multi_index/hashed_index.hpp"
|
||||
|
||||
#include <boost/signals2/signal.hpp>
|
||||
|
||||
class CAutoFile;
|
||||
class CBlockIndex;
|
||||
|
||||
|
@ -333,6 +335,19 @@ struct TxMempoolInfo
|
|||
int64_t nFeeDelta;
|
||||
};
|
||||
|
||||
/** Reason why a transaction was removed from the mempool,
|
||||
* this is passed to the notification signal.
|
||||
*/
|
||||
enum class MemPoolRemovalReason {
|
||||
UNKNOWN = 0, //! Manually removed or unknown reason
|
||||
EXPIRY, //! Expired from mempool
|
||||
SIZELIMIT, //! Removed in size limiting
|
||||
REORG, //! Removed for reorganization
|
||||
BLOCK, //! Removed for block
|
||||
CONFLICT, //! Removed for conflict with in-block transaction
|
||||
REPLACED //! Removed for replacement
|
||||
};
|
||||
|
||||
/**
|
||||
* CTxMemPool stores valid-according-to-the-current-best-chain transactions
|
||||
* that may be included in the next block.
|
||||
|
@ -521,10 +536,11 @@ public:
|
|||
bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, bool validFeeEstimate = true);
|
||||
bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool validFeeEstimate = true);
|
||||
|
||||
void removeRecursive(const CTransaction &tx);
|
||||
void removeRecursive(const CTransaction &tx, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN);
|
||||
void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags);
|
||||
void removeConflicts(const CTransaction &tx);
|
||||
void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight);
|
||||
|
||||
void clear();
|
||||
void _clear(); //lock free
|
||||
bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb);
|
||||
|
@ -551,7 +567,7 @@ public:
|
|||
* Set updateDescendants to true when removing a tx that was in a block, so
|
||||
* that any in-mempool descendants have their ancestor state updated.
|
||||
*/
|
||||
void RemoveStaged(setEntries &stage, bool updateDescendants);
|
||||
void RemoveStaged(setEntries &stage, bool updateDescendants, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN);
|
||||
|
||||
/** When adding transactions from a disconnected block back to the mempool,
|
||||
* new mempool entries may have children in the mempool (which is generally
|
||||
|
@ -647,6 +663,9 @@ public:
|
|||
|
||||
size_t DynamicMemoryUsage() const;
|
||||
|
||||
boost::signals2::signal<void (CTransactionRef)> NotifyEntryAdded;
|
||||
boost::signals2::signal<void (CTransactionRef, MemPoolRemovalReason)> NotifyEntryRemoved;
|
||||
|
||||
private:
|
||||
/** UpdateForDescendants is used by UpdateTransactionsFromBlock to update
|
||||
* the descendants for a single transaction that has been added to the
|
||||
|
@ -683,7 +702,7 @@ private:
|
|||
* transactions in a chain before we've updated all the state for the
|
||||
* removal.
|
||||
*/
|
||||
void removeUnchecked(txiter entry);
|
||||
void removeUnchecked(txiter entry, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN);
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
|
@ -157,6 +157,39 @@ namespace {
|
|||
set<int> setDirtyFileInfo;
|
||||
} // anon namespace
|
||||
|
||||
/* Use this class to start tracking transactions that are removed from the
|
||||
* mempool and pass all those transactions through SyncTransaction when the
|
||||
* object goes out of scope. This is currently only used to call SyncTransaction
|
||||
* on conflicts removed from the mempool during block connection. Applied in
|
||||
* ActivateBestChain around ActivateBestStep which in turn calls:
|
||||
* ConnectTip->removeForBlock->removeConflicts
|
||||
*/
|
||||
class MemPoolConflictRemovalTracker
|
||||
{
|
||||
private:
|
||||
std::vector<CTransactionRef> conflictedTxs;
|
||||
CTxMemPool &pool;
|
||||
|
||||
public:
|
||||
MemPoolConflictRemovalTracker(CTxMemPool &_pool) : pool(_pool) {
|
||||
pool.NotifyEntryRemoved.connect(boost::bind(&MemPoolConflictRemovalTracker::NotifyEntryRemoved, this, _1, _2));
|
||||
}
|
||||
|
||||
void NotifyEntryRemoved(CTransactionRef txRemoved, MemPoolRemovalReason reason) {
|
||||
if (reason == MemPoolRemovalReason::CONFLICT) {
|
||||
conflictedTxs.push_back(txRemoved);
|
||||
}
|
||||
}
|
||||
|
||||
~MemPoolConflictRemovalTracker() {
|
||||
pool.NotifyEntryRemoved.disconnect(boost::bind(&MemPoolConflictRemovalTracker::NotifyEntryRemoved, this, _1, _2));
|
||||
for (const auto& tx : conflictedTxs) {
|
||||
GetMainSignals().SyncTransaction(*tx, NULL, CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK);
|
||||
}
|
||||
conflictedTxs.clear();
|
||||
}
|
||||
};
|
||||
|
||||
CBlockIndex* FindForkInGlobalIndex(const CChain& chain, const CBlockLocator& locator)
|
||||
{
|
||||
// Find the first block the caller has in the main chain
|
||||
|
@ -956,7 +989,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C
|
|||
if (plTxnReplaced)
|
||||
plTxnReplaced->push_back(it->GetSharedTx());
|
||||
}
|
||||
pool.RemoveStaged(allConflicting, false);
|
||||
pool.RemoveStaged(allConflicting, false, MemPoolRemovalReason::REPLACED);
|
||||
|
||||
// This transaction should only count for fee estimation if
|
||||
// the node is not behind and it is not dependent on any other
|
||||
|
@ -2166,7 +2199,7 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara
|
|||
// ignore validation errors in resurrected transactions
|
||||
CValidationState stateDummy;
|
||||
if (tx.IsCoinBase() || !AcceptToMemoryPool(mempool, stateDummy, it, false, NULL, NULL, true)) {
|
||||
mempool.removeRecursive(tx);
|
||||
mempool.removeRecursive(tx, MemPoolRemovalReason::REORG);
|
||||
} else if (mempool.exists(tx.GetHash())) {
|
||||
vHashUpdate.push_back(tx.GetHash());
|
||||
}
|
||||
|
@ -2453,6 +2486,14 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams,
|
|||
bool fInitialDownload;
|
||||
{
|
||||
LOCK(cs_main);
|
||||
{ // TODO: Tempoarily ensure that mempool removals are notified before
|
||||
// connected transactions. This shouldn't matter, but the abandoned
|
||||
// state of transactions in our wallet is currently cleared when we
|
||||
// receive another notification and there is a race condition where
|
||||
// notification of a connected conflict might cause an outside process
|
||||
// to abandon a transaction and then have it inadvertantly cleared by
|
||||
// the notification that the conflicted transaction was evicted.
|
||||
MemPoolConflictRemovalTracker mrt(mempool);
|
||||
CBlockIndex *pindexOldTip = chainActive.Tip();
|
||||
if (pindexMostWork == NULL) {
|
||||
pindexMostWork = FindMostWorkChain();
|
||||
|
@ -2476,6 +2517,10 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams,
|
|||
fInitialDownload = IsInitialBlockDownload();
|
||||
|
||||
// throw all transactions though the signal-interface
|
||||
|
||||
} // MemPoolConflictRemovalTracker destroyed and conflict evictions are notified
|
||||
|
||||
// Transactions in the connnected block are notified
|
||||
for (const auto& pair : connectTrace.blocksConnected) {
|
||||
assert(pair.second);
|
||||
const CBlock& block = *(pair.second);
|
||||
|
@ -3597,7 +3642,7 @@ bool CVerifyDB::VerifyDB(const CChainParams& chainparams, CCoinsView *coinsview,
|
|||
return error("VerifyDB(): *** ReadBlockFromDisk failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString());
|
||||
// check level 1: verify block validity
|
||||
if (nCheckLevel >= 1 && !CheckBlock(block, state, chainparams.GetConsensus()))
|
||||
return error("%s: *** found bad block at %d, hash=%s (%s)\n", __func__,
|
||||
return error("%s: *** found bad block at %d, hash=%s (%s)\n", __func__,
|
||||
pindex->nHeight, pindex->GetBlockHash().ToString(), FormatStateMessage(state));
|
||||
// check level 2: verify undo validity
|
||||
if (nCheckLevel >= 2 && pindex) {
|
||||
|
@ -3768,7 +3813,7 @@ bool LoadBlockIndex(const CChainParams& chainparams)
|
|||
return true;
|
||||
}
|
||||
|
||||
bool InitBlockIndex(const CChainParams& chainparams)
|
||||
bool InitBlockIndex(const CChainParams& chainparams)
|
||||
{
|
||||
LOCK(cs_main);
|
||||
|
||||
|
|
|
@ -50,9 +50,16 @@ protected:
|
|||
struct CMainSignals {
|
||||
/** Notifies listeners of updated block chain tip */
|
||||
boost::signals2::signal<void (const CBlockIndex *, const CBlockIndex *, bool fInitialDownload)> UpdatedBlockTip;
|
||||
/** A posInBlock value for SyncTransaction which indicates the transaction was conflicted, disconnected, or not in a block */
|
||||
/** A posInBlock value for SyncTransaction calls for tranactions not
|
||||
* included in connected blocks such as transactions removed from mempool,
|
||||
* accepted to mempool or appearing in disconnected blocks.*/
|
||||
static const int SYNC_TRANSACTION_NOT_IN_BLOCK = -1;
|
||||
/** Notifies listeners of updated transaction data (transaction, and optionally the block it is found in. */
|
||||
/** Notifies listeners of updated transaction data (transaction, and
|
||||
* optionally the block it is found in). Called with block data when
|
||||
* transaction is included in a connected block, and without block data when
|
||||
* transaction was accepted to mempool, removed from mempool (only when
|
||||
* removal was due to conflict from connected block), or appeared in a
|
||||
* disconnected block.*/
|
||||
boost::signals2::signal<void (const CTransaction &, const CBlockIndex *pindex, int posInBlock)> SyncTransaction;
|
||||
/** Notifies listeners of an updated transaction without new data (for now: a coinbase potentially becoming visible). */
|
||||
boost::signals2::signal<void (const uint256 &)> UpdatedTransaction;
|
||||
|
|
|
@ -1003,9 +1003,17 @@ bool CWallet::LoadToWallet(const CWalletTx& wtxIn)
|
|||
}
|
||||
|
||||
/**
|
||||
* Add a transaction to the wallet, or update it.
|
||||
* pblock is optional, but should be provided if the transaction is known to be in a block.
|
||||
* Add a transaction to the wallet, or update it. pIndex and posInBlock should
|
||||
* be set when the transaction was known to be included in a block. When
|
||||
* posInBlock = SYNC_TRANSACTION_NOT_IN_BLOCK (-1) , then wallet state is not
|
||||
* updated in AddToWallet, but notifications happen and cached balances are
|
||||
* marked dirty.
|
||||
* If fUpdate is true, existing transactions will be updated.
|
||||
* TODO: One exception to this is that the abandoned state is cleared under the
|
||||
* assumption that any further notification of a transaction that was considered
|
||||
* abandoned is an indication that it is not safe to be considered abandoned.
|
||||
* Abandoned state should probably be more carefuly tracked via different
|
||||
* posInBlock signals or by checking mempool presence when necessary.
|
||||
*/
|
||||
bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlockIndex* pIndex, int posInBlock, bool fUpdate)
|
||||
{
|
||||
|
|
Loading…
Reference in a new issue