aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMarcoFalke <falke.marco@gmail.com>2018-07-28 11:09:58 -0400
committerMarcoFalke <falke.marco@gmail.com>2018-07-29 08:04:06 -0400
commitfa5ed4f8d2c4cb3507bcc2460725d483f2e5789c (patch)
tree0dabe170ab89702055257cf214ab1de431aa4a9b
parentad51e1372bc645fc8710da2bc9e0a9fc6c0f27bb (diff)
refactor: Avoid locking tx pool cs thrice
-rw-r--r--src/bench/mempool_eviction.cpp3
-rw-r--r--src/test/blockencodings_tests.cpp6
-rw-r--r--src/test/mempool_tests.cpp7
-rw-r--r--src/test/miner_tests.cpp3
-rw-r--r--src/test/policyestimator_tests.cpp1
-rw-r--r--src/txmempool.cpp4
-rw-r--r--src/txmempool.h6
-rw-r--r--src/wallet/wallet.cpp3
8 files changed, 18 insertions, 15 deletions
diff --git a/src/bench/mempool_eviction.cpp b/src/bench/mempool_eviction.cpp
index 4c947a519f..d37291b900 100644
--- a/src/bench/mempool_eviction.cpp
+++ b/src/bench/mempool_eviction.cpp
@@ -9,7 +9,7 @@
#include <list>
#include <vector>
-static void AddTx(const CTransactionRef& tx, const CAmount& nFee, CTxMemPool& pool)
+static void AddTx(const CTransactionRef& tx, const CAmount& nFee, CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(pool.cs)
{
int64_t nTime = 0;
unsigned int nHeight = 1;
@@ -108,6 +108,7 @@ static void MempoolEviction(benchmark::State& state)
tx7.vout[1].nValue = 10 * COIN;
CTxMemPool pool;
+ LOCK(pool.cs);
// Create transaction references outside the "hot loop"
const CTransactionRef tx1_r{MakeTransactionRef(tx1)};
const CTransactionRef tx2_r{MakeTransactionRef(tx2)};
diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp
index 3dd5356164..df839884fe 100644
--- a/src/test/blockencodings_tests.cpp
+++ b/src/test/blockencodings_tests.cpp
@@ -62,8 +62,8 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest)
TestMemPoolEntryHelper entry;
CBlock block(BuildBlockTestCase());
- pool.addUnchecked(block.vtx[2]->GetHash(), entry.FromTx(block.vtx[2]));
LOCK(pool.cs);
+ pool.addUnchecked(block.vtx[2]->GetHash(), entry.FromTx(block.vtx[2]));
BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0);
// Do a simple ShortTxIDs RT
@@ -162,8 +162,8 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest)
TestMemPoolEntryHelper entry;
CBlock block(BuildBlockTestCase());
- pool.addUnchecked(block.vtx[2]->GetHash(), entry.FromTx(block.vtx[2]));
LOCK(pool.cs);
+ pool.addUnchecked(block.vtx[2]->GetHash(), entry.FromTx(block.vtx[2]));
BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0);
uint256 txhash;
@@ -232,8 +232,8 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest)
TestMemPoolEntryHelper entry;
CBlock block(BuildBlockTestCase());
- pool.addUnchecked(block.vtx[1]->GetHash(), entry.FromTx(block.vtx[1]));
LOCK(pool.cs);
+ pool.addUnchecked(block.vtx[1]->GetHash(), entry.FromTx(block.vtx[1]));
BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[1]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0);
uint256 txhash;
diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp
index e2d76dc293..fb80599af7 100644
--- a/src/test/mempool_tests.cpp
+++ b/src/test/mempool_tests.cpp
@@ -55,6 +55,7 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
CTxMemPool testPool;
+ LOCK(testPool.cs);
// Nothing in pool, remove should do nothing:
unsigned int poolSize = testPool.size();
@@ -119,6 +120,7 @@ static void CheckSort(CTxMemPool &pool, std::vector<std::string> &sortedOrder) E
BOOST_AUTO_TEST_CASE(MempoolIndexingTest)
{
CTxMemPool pool;
+ LOCK(pool.cs);
TestMemPoolEntryHelper entry;
/* 3rd highest fee */
@@ -165,7 +167,6 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest)
sortedOrder[2] = tx1.GetHash().ToString(); // 10000
sortedOrder[3] = tx4.GetHash().ToString(); // 15000
sortedOrder[4] = tx2.GetHash().ToString(); // 20000
- LOCK(pool.cs);
CheckSort<descendant_score>(pool, sortedOrder);
/* low fee but with high fee child */
@@ -292,6 +293,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest)
BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)
{
CTxMemPool pool;
+ LOCK(pool.cs);
TestMemPoolEntryHelper entry;
/* 3rd highest fee */
@@ -347,7 +349,6 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)
}
sortedOrder[4] = tx3.GetHash().ToString(); // 0
- LOCK(pool.cs);
CheckSort<ancestor_score>(pool, sortedOrder);
/* low fee parent with high fee child */
@@ -421,6 +422,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)
BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
{
CTxMemPool pool;
+ LOCK(pool.cs);
TestMemPoolEntryHelper entry;
CMutableTransaction tx1 = CMutableTransaction();
@@ -593,6 +595,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests)
size_t ancestors, descendants;
CTxMemPool pool;
+ LOCK(pool.cs);
TestMemPoolEntryHelper entry;
/* Base transaction */
diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp
index 10c7fd00e8..e2424f012d 100644
--- a/src/test/miner_tests.cpp
+++ b/src/test/miner_tests.cpp
@@ -99,7 +99,7 @@ static bool TestSequenceLocks(const CTransaction &tx, int flags)
// Test suite for ancestor feerate transaction selection.
// Implemented as an additional function, rather than a separate test case,
// to allow reusing the blockchain created in CreateNewBlock_validity.
-static void TestPackageSelection(const CChainParams& chainparams, CScript scriptPubKey, std::vector<CTransactionRef>& txFirst)
+static void TestPackageSelection(const CChainParams& chainparams, const CScript& scriptPubKey, const std::vector<CTransactionRef>& txFirst) EXCLUSIVE_LOCKS_REQUIRED(::mempool.cs)
{
// Test the ancestor feerate transaction selection.
TestMemPoolEntryHelper entry;
@@ -253,6 +253,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
}
LOCK(cs_main);
+ LOCK(::mempool.cs);
// Just to make sure we can still make simple blocks
BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey));
diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp
index 8d288ec993..e45fb6d17e 100644
--- a/src/test/policyestimator_tests.cpp
+++ b/src/test/policyestimator_tests.cpp
@@ -18,6 +18,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
{
CBlockPolicyEstimator feeEst;
CTxMemPool mpool(&feeEst);
+ LOCK(mpool.cs);
TestMemPoolEntryHelper entry;
CAmount basefee(2000);
CAmount deltaFee(100);
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 9d705e3d23..d68c38ad4e 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -151,8 +151,6 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashes
bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string &errString, bool fSearchForParents /* = true */) const
{
- LOCK(cs);
-
setEntries parentHashes;
const CTransaction &tx = entry.GetTx();
@@ -363,7 +361,6 @@ void CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry,
// Add to memory pool without checking anything.
// Used by AcceptToMemoryPool(), which DOES do
// all the appropriate checks.
- LOCK(cs);
indexed_transaction_set::iterator newit = mapTx.insert(entry).first;
mapLinks.insert(make_pair(newit, TxLinks()));
@@ -933,7 +930,6 @@ int CTxMemPool::Expire(int64_t time) {
void CTxMemPool::addUnchecked(const uint256&hash, const CTxMemPoolEntry &entry, bool validFeeEstimate)
{
- LOCK(cs);
setEntries setAncestors;
uint64_t nNoLimit = std::numeric_limits<uint64_t>::max();
std::string dummy;
diff --git a/src/txmempool.h b/src/txmempool.h
index bb676cf05d..0feea08f0b 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -539,8 +539,8 @@ public:
// Note that addUnchecked is ONLY called from ATMP outside of tests
// and any other callers may break wallet's in-mempool tracking (due to
// lack of CValidationInterface::TransactionAddedToMempool callbacks).
- void addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, bool validFeeEstimate = true);
- void addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool validFeeEstimate = true);
+ void addUnchecked(const uint256& hash, const CTxMemPoolEntry& entry, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs);
+ void addUnchecked(const uint256& hash, const CTxMemPoolEntry& entry, setEntries& setAncestors, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs);
void removeRecursive(const CTransaction &tx, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN);
void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags);
@@ -596,7 +596,7 @@ public:
* fSearchForParents = whether to search a tx's vin for in-mempool parents, or
* look up parents from mapLinks. Must be true for entries not in the mempool
*/
- bool CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string &errString, bool fSearchForParents = true) const;
+ bool CalculateMemPoolAncestors(const CTxMemPoolEntry& entry, setEntries& setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string& errString, bool fSearchForParents = true) const EXCLUSIVE_LOCKS_REQUIRED(cs);
/** Populate setDescendants with all in-mempool descendants of hash.
* Assumes that setDescendants includes all in-mempool descendants of anything
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 4918100b30..bb048949ca 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -3018,7 +3018,8 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransac
size_t nLimitDescendants = gArgs.GetArg("-limitdescendantcount", DEFAULT_DESCENDANT_LIMIT);
size_t nLimitDescendantSize = gArgs.GetArg("-limitdescendantsize", DEFAULT_DESCENDANT_SIZE_LIMIT)*1000;
std::string errString;
- if (!mempool.CalculateMemPoolAncestors(entry, setAncestors, nLimitAncestors, nLimitAncestorSize, nLimitDescendants, nLimitDescendantSize, errString)) {
+ LOCK(::mempool.cs);
+ if (!::mempool.CalculateMemPoolAncestors(entry, setAncestors, nLimitAncestors, nLimitAncestorSize, nLimitDescendants, nLimitDescendantSize, errString)) {
strFailReason = _("Transaction has too long of a mempool chain");
return false;
}