From 74d0f902628472cd0cee66121ef0311eec201c40 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 21 Oct 2015 17:41:40 -0700 Subject: [PATCH 1/6] Add method to remove a tx from CCoinsViewCache if it is unchanged --- src/coins.cpp | 9 +++++++++ src/coins.h | 6 ++++++ 2 files changed, 15 insertions(+) diff --git a/src/coins.cpp b/src/coins.cpp index 723e11470..060d6b7c5 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -206,6 +206,15 @@ bool CCoinsViewCache::Flush() { return fOk; } +void CCoinsViewCache::Uncache(const uint256& hash) +{ + CCoinsMap::iterator it = cacheCoins.find(hash); + if (it != cacheCoins.end() && it->second.flags == 0) { + cachedCoinsUsage -= it->second.coins.DynamicMemoryUsage(); + cacheCoins.erase(it); + } +} + unsigned int CCoinsViewCache::GetCacheSize() const { return cacheCoins.size(); } diff --git a/src/coins.h b/src/coins.h index d17442210..5beea711b 100644 --- a/src/coins.h +++ b/src/coins.h @@ -437,6 +437,12 @@ public: */ bool Flush(); + /** + * Removes the transaction with the given hash from the cache, if it is + * not modified. + */ + void Uncache(const uint256 &txid); + //! Calculate the size of the cache (in number of transactions) unsigned int GetCacheSize() const; From b2e74bd292460ca00fefc6356594318307365397 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 21 Oct 2015 17:44:00 -0700 Subject: [PATCH 2/6] Get the set of now-uncacheable-txn from CTxMemPool::TrimToSize --- src/txmempool.cpp | 22 ++++++++++++++++++++-- src/txmempool.h | 7 +++++-- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 35be21628..fea5da802 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -944,7 +944,7 @@ void CTxMemPool::trackPackageRemoved(const CFeeRate& rate) { } } -void CTxMemPool::TrimToSize(size_t sizelimit) { +void CTxMemPool::TrimToSize(size_t sizelimit, std::vector* pvNoSpendsRemaining) { LOCK(cs); unsigned nTxnRemoved = 0; @@ -963,8 +963,26 @@ void CTxMemPool::TrimToSize(size_t sizelimit) { setEntries stage; CalculateDescendants(mapTx.project<0>(it), stage); - RemoveStaged(stage); nTxnRemoved += stage.size(); + + std::vector txn; + if (pvNoSpendsRemaining) { + txn.reserve(stage.size()); + BOOST_FOREACH(txiter it, stage) + txn.push_back(it->GetTx()); + } + RemoveStaged(stage); + if (pvNoSpendsRemaining) { + BOOST_FOREACH(const CTransaction& tx, txn) { + BOOST_FOREACH(const CTxIn& txin, tx.vin) { + if (exists(txin.prevout.hash)) + continue; + std::map::iterator it = mapNextTx.lower_bound(COutPoint(txin.prevout.hash, 0)); + if (it == mapNextTx.end() || it->first.hash != txin.prevout.hash) + pvNoSpendsRemaining->push_back(txin.prevout.hash); + } + } + } } if (maxFeeRateRemoved > CFeeRate(0)) diff --git a/src/txmempool.h b/src/txmempool.h index 5652969f4..920317186 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -483,8 +483,11 @@ public: */ CFeeRate GetMinFee(size_t sizelimit) const; - /** Remove transactions from the mempool until its dynamic size is <= sizelimit. */ - void TrimToSize(size_t sizelimit); + /** Remove transactions from the mempool until its dynamic size is <= sizelimit. + * pvNoSpendsRemaining, if set, will be populated with the list of transactions + * which are not in mempool which no longer have any spends in this mempool. + */ + void TrimToSize(size_t sizelimit, std::vector* pvNoSpendsRemaining=NULL); /** Expire all transaction (and their dependencies) in the mempool older than time. Return the number of removed transactions. */ int Expire(int64_t time); From 677aa3d88c018a235d5b6d929e82f7b16e4f3e41 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 22 Oct 2015 11:52:55 -0700 Subject: [PATCH 3/6] Discard txn cache entries that were loaded for removed mempool txn --- src/main.cpp | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index e9e982043..73ca8bb05 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -789,6 +789,17 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state) return true; } +void LimitMempoolSize(CTxMemPool& pool, size_t limit, unsigned long age) { + int expired = pool.Expire(GetTime() - age); + if (expired != 0) + LogPrint("mempool", "Expired %i transactions from the memory pool\n", expired); + + std::vector vNoSpendsRemaining; + pool.TrimToSize(limit, &vNoSpendsRemaining); + BOOST_FOREACH(const uint256& removed, vNoSpendsRemaining) + pcoinsTip->Uncache(removed); +} + CAmount GetMinRelayFee(const CTransaction& tx, const CTxMemPool& pool, unsigned int nBytes, bool fAllowFree) { uint256 hash = tx.GetHash(); @@ -1210,12 +1221,8 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa // trim mempool and check if tx was trimmed if (!fOverrideMempoolLimit) { - int expired = pool.Expire(GetTime() - GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); - if (expired != 0) - LogPrint("mempool", "Expired %i transactions from the memory pool\n", expired); - - pool.TrimToSize(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000); - if (!pool.exists(tx.GetHash())) + LimitMempoolSize(pool, GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); + if (!pool.exists(hash)) return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "mempool full"); } } @@ -2571,7 +2578,7 @@ static bool ActivateBestChainStep(CValidationState& state, const CChainParams& c if (fBlocksDisconnected) { mempool.removeForReorg(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS); - mempool.TrimToSize(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000); + LimitMempoolSize(mempool, GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); } mempool.check(pcoinsTip); @@ -2686,7 +2693,7 @@ bool InvalidateBlock(CValidationState& state, const Consensus::Params& consensus } } - mempool.TrimToSize(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000); + LimitMempoolSize(mempool, GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); // The resulting new best tip may not be in setBlockIndexCandidates anymore, so // add it again. From 97bf377bd1f27ad841e1414e74361923a9f794f5 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 22 Oct 2015 15:49:53 -0700 Subject: [PATCH 4/6] Add CCoinsViewCache::HaveCoinsInCache to check if a tx is cached --- src/coins.cpp | 5 +++++ src/coins.h | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/src/coins.cpp b/src/coins.cpp index 060d6b7c5..122bf4e48 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -144,6 +144,11 @@ bool CCoinsViewCache::HaveCoins(const uint256 &txid) const { return (it != cacheCoins.end() && !it->second.coins.vout.empty()); } +bool CCoinsViewCache::HaveCoinsInCache(const uint256 &txid) const { + CCoinsMap::const_iterator it = cacheCoins.find(txid); + return it != cacheCoins.end(); +} + uint256 CCoinsViewCache::GetBestBlock() const { if (hashBlock.IsNull()) hashBlock = base->GetBestBlock(); diff --git a/src/coins.h b/src/coins.h index 5beea711b..60c1ba8a7 100644 --- a/src/coins.h +++ b/src/coins.h @@ -405,6 +405,13 @@ public: void SetBestBlock(const uint256 &hashBlock); bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock); + /** + * Check if we have the given tx already loaded in this cache. + * The semantics are the same as HaveCoins(), but no calls to + * the backing CCoinsView are made. + */ + bool HaveCoinsInCache(const uint256 &txid) const; + /** * Return a pointer to CCoins in the cache, or NULL if not found. This is * more efficient than GetCoins. Modifications to other cache entries are From bde953e2818ecf44727da128c2aa2ec7667cf7e7 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 22 Oct 2015 15:50:33 -0700 Subject: [PATCH 5/6] Uncache input txn in utxo cache if a tx is not accepted to mempool --- src/main.cpp | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 73ca8bb05..d1ed5c5ed 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -835,8 +835,9 @@ std::string FormatStateMessage(const CValidationState &state) state.GetRejectCode()); } -bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransaction &tx, bool fLimitFree, - bool* pfMissingInputs, bool fOverrideMempoolLimit, bool fRejectAbsurdFee) +bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const CTransaction &tx, bool fLimitFree, + bool* pfMissingInputs, bool fOverrideMempoolLimit, bool fRejectAbsurdFee, + std::vector& vHashTxnToUncache) { AssertLockHeld(cs_main); if (pfMissingInputs) @@ -917,13 +918,19 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa view.SetBackend(viewMemPool); // do we already have it? - if (view.HaveCoins(hash)) + bool fHadTxInCache = pcoinsTip->HaveCoinsInCache(hash); + if (view.HaveCoins(hash)) { + if (!fHadTxInCache) + vHashTxnToUncache.push_back(hash); return state.Invalid(false, REJECT_ALREADY_KNOWN, "txn-already-known"); + } // do all inputs exist? // Note that this does not check for the presence of actual outputs (see the next check for that), // and only helps with filling in pfMissingInputs (to determine missing vs spent). BOOST_FOREACH(const CTxIn txin, tx.vin) { + if (!pcoinsTip->HaveCoinsInCache(txin.prevout.hash)) + vHashTxnToUncache.push_back(txin.prevout.hash); if (!view.HaveCoins(txin.prevout.hash)) { if (pfMissingInputs) *pfMissingInputs = true; @@ -1232,6 +1239,18 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa return true; } +bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransaction &tx, bool fLimitFree, + bool* pfMissingInputs, bool fOverrideMempoolLimit, bool fRejectAbsurdFee) +{ + std::vector vHashTxToUncache; + bool res = AcceptToMemoryPoolWorker(pool, state, tx, fLimitFree, pfMissingInputs, fOverrideMempoolLimit, fRejectAbsurdFee, vHashTxToUncache); + if (!res) { + BOOST_FOREACH(const uint256& hashTx, vHashTxToUncache) + pcoinsTip->Uncache(hashTx); + } + return res; +} + /** Return transaction in tx, and if it was found inside a block, its hash is placed in hashBlock */ bool GetTransaction(const uint256 &hash, CTransaction &txOut, const Consensus::Params& consensusParams, uint256 &hashBlock, bool fAllowSlow) { From dd5862c4cdc02535948042fe519694166bcd2bb7 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 26 Oct 2015 04:22:07 +0100 Subject: [PATCH 6/6] Flush coins cache also after transaction processing --- src/main.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main.cpp b/src/main.cpp index d1ed5c5ed..c41dd58d1 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -4830,6 +4830,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, if (nDoS > 0) Misbehaving(pfrom->GetId(), nDoS); } + FlushStateToDisk(state, FLUSH_STATE_PERIODIC); }