aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/bench/mempool_stress.cpp43
-rw-r--r--src/net_processing.cpp6
-rw-r--r--src/test/fuzz/tx_pool.cpp4
-rw-r--r--src/txmempool.cpp55
-rw-r--r--src/txmempool.h2
-rw-r--r--src/validation.cpp8
-rw-r--r--src/validation.h3
7 files changed, 59 insertions, 62 deletions
diff --git a/src/bench/mempool_stress.cpp b/src/bench/mempool_stress.cpp
index f28768efc8..a0a82ea359 100644
--- a/src/bench/mempool_stress.cpp
+++ b/src/bench/mempool_stress.cpp
@@ -6,6 +6,7 @@
#include <policy/policy.h>
#include <test/util/setup_common.h>
#include <txmempool.h>
+#include <validation.h>
#include <vector>
@@ -26,14 +27,8 @@ struct Available {
Available(CTransactionRef& ref, size_t tx_count) : ref(ref), tx_count(tx_count){}
};
-static void ComplexMemPool(benchmark::Bench& bench)
+static std::vector<CTransactionRef> CreateOrderedCoins(FastRandomContext& det_rand, int childTxs, int min_ancestors)
{
- int childTxs = 800;
- if (bench.complexityN() > 1) {
- childTxs = static_cast<int>(bench.complexityN());
- }
-
- FastRandomContext det_rand{true};
std::vector<Available> available_coins;
std::vector<CTransactionRef> ordered_coins;
// Create some base transactions
@@ -58,8 +53,10 @@ static void ComplexMemPool(benchmark::Bench& bench)
size_t idx = det_rand.randrange(available_coins.size());
Available coin = available_coins[idx];
uint256 hash = coin.ref->GetHash();
- // biased towards taking just one ancestor, but maybe more
- size_t n_to_take = det_rand.randrange(2) == 0 ? 1 : 1+det_rand.randrange(coin.ref->vout.size() - coin.vin_left);
+ // biased towards taking min_ancestors parents, but maybe more
+ size_t n_to_take = det_rand.randrange(2) == 0 ?
+ min_ancestors :
+ min_ancestors + det_rand.randrange(coin.ref->vout.size() - coin.vin_left);
for (size_t i = 0; i < n_to_take; ++i) {
tx.vin.emplace_back();
tx.vin.back().prevout = COutPoint(hash, coin.vin_left++);
@@ -79,6 +76,17 @@ static void ComplexMemPool(benchmark::Bench& bench)
ordered_coins.emplace_back(MakeTransactionRef(tx));
available_coins.emplace_back(ordered_coins.back(), tx_counter++);
}
+ return ordered_coins;
+}
+
+static void ComplexMemPool(benchmark::Bench& bench)
+{
+ FastRandomContext det_rand{true};
+ int childTxs = 800;
+ if (bench.complexityN() > 1) {
+ childTxs = static_cast<int>(bench.complexityN());
+ }
+ std::vector<CTransactionRef> ordered_coins = CreateOrderedCoins(det_rand, childTxs, /* min_ancestors */ 1);
const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(CBaseChainParams::MAIN);
CTxMemPool pool;
LOCK2(cs_main, pool.cs);
@@ -91,4 +99,21 @@ static void ComplexMemPool(benchmark::Bench& bench)
});
}
+static void MempoolCheck(benchmark::Bench& bench)
+{
+ FastRandomContext det_rand{true};
+ const int childTxs = bench.complexityN() > 1 ? static_cast<int>(bench.complexityN()) : 2000;
+ const std::vector<CTransactionRef> ordered_coins = CreateOrderedCoins(det_rand, childTxs, /* min_ancestors */ 5);
+ const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(CBaseChainParams::MAIN, {"-checkmempool=1"});
+ CTxMemPool pool;
+ LOCK2(cs_main, pool.cs);
+ const CCoinsViewCache& coins_tip = testing_setup.get()->m_node.chainman->ActiveChainstate().CoinsTip();
+ for (auto& tx : ordered_coins) AddTx(tx, pool);
+
+ bench.run([&]() NO_THREAD_SAFETY_ANALYSIS {
+ pool.check(coins_tip, /* spendheight */ 2);
+ });
+}
+
BENCHMARK(ComplexMemPool);
+BENCHMARK(MempoolCheck);
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 13f13d5285..9f3aa5b4a3 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -2299,7 +2299,8 @@ void PeerManagerImpl::ProcessOrphanTx(std::set<uint256>& orphan_work_set)
break;
}
}
- m_mempool.check(m_chainman.ActiveChainstate());
+ CChainState& active_chainstate = m_chainman.ActiveChainstate();
+ m_mempool.check(active_chainstate.CoinsTip(), active_chainstate.m_chain.Height() + 1);
}
bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& peer,
@@ -3262,7 +3263,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
const TxValidationState& state = result.m_state;
if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
- m_mempool.check(m_chainman.ActiveChainstate());
+ CChainState& active_chainstate = m_chainman.ActiveChainstate();
+ m_mempool.check(active_chainstate.CoinsTip(), active_chainstate.m_chain.Height() + 1);
// As this version of the transaction was acceptable, we can forget about any
// requests for it.
m_txrequest.ForgetTxHash(tx.GetHash());
diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp
index 6201cc813c..17b5ef88b9 100644
--- a/src/test/fuzz/tx_pool.cpp
+++ b/src/test/fuzz/tx_pool.cpp
@@ -81,7 +81,7 @@ void SetMempoolConstraints(ArgsManager& args, FuzzedDataProvider& fuzzed_data_pr
void Finish(FuzzedDataProvider& fuzzed_data_provider, MockedTxPool& tx_pool, CChainState& chainstate)
{
- WITH_LOCK(::cs_main, tx_pool.check(chainstate));
+ WITH_LOCK(::cs_main, tx_pool.check(chainstate.CoinsTip(), chainstate.m_chain.Height() + 1));
{
BlockAssembler::Options options;
options.nBlockMaxWeight = fuzzed_data_provider.ConsumeIntegralInRange(0U, MAX_BLOCK_WEIGHT);
@@ -97,7 +97,7 @@ void Finish(FuzzedDataProvider& fuzzed_data_provider, MockedTxPool& tx_pool, CCh
std::vector<uint256> all_txids;
tx_pool.queryHashes(all_txids);
assert(all_txids.size() < info_all.size());
- WITH_LOCK(::cs_main, tx_pool.check(chainstate));
+ WITH_LOCK(::cs_main, tx_pool.check(chainstate.CoinsTip(), chainstate.m_chain.Height() + 1));
}
SyncWithValidationInterfaceQueue();
}
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 70084ea1d1..9bc2377c63 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -5,6 +5,7 @@
#include <txmempool.h>
+#include <coins.h>
#include <consensus/consensus.h>
#include <consensus/tx_verify.h>
#include <consensus/validation.h>
@@ -671,16 +672,7 @@ void CTxMemPool::clear()
_clear();
}
-static void CheckInputsAndUpdateCoins(const CTransaction& tx, CCoinsViewCache& mempoolDuplicate, const int64_t spendheight)
-{
- TxValidationState dummy_state; // Not used. CheckTxInputs() should always pass
- CAmount txfee = 0;
- bool fCheckResult = tx.IsCoinBase() || Consensus::CheckTxInputs(tx, dummy_state, mempoolDuplicate, spendheight, txfee);
- assert(fCheckResult);
- UpdateCoins(tx, mempoolDuplicate, std::numeric_limits<int>::max());
-}
-
-void CTxMemPool::check(CChainState& active_chainstate) const
+void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendheight) const
{
if (m_check_ratio == 0) return;
@@ -693,20 +685,16 @@ void CTxMemPool::check(CChainState& active_chainstate) const
uint64_t checkTotal = 0;
CAmount check_total_fee{0};
uint64_t innerUsage = 0;
+ uint64_t prev_ancestor_count{0};
- CCoinsViewCache& active_coins_tip = active_chainstate.CoinsTip();
CCoinsViewCache mempoolDuplicate(const_cast<CCoinsViewCache*>(&active_coins_tip));
- const int64_t spendheight = active_chainstate.m_chain.Height() + 1;
- std::list<const CTxMemPoolEntry*> waitingOnDependants;
- for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) {
- unsigned int i = 0;
+ for (const auto& it : GetSortedDepthAndScore()) {
checkTotal += it->GetTxSize();
check_total_fee += it->GetFee();
innerUsage += it->DynamicMemoryUsage();
const CTransaction& tx = it->GetTx();
innerUsage += memusage::DynamicUsage(it->GetMemPoolParentsConst()) + memusage::DynamicUsage(it->GetMemPoolChildrenConst());
- bool fDependsWait = false;
CTxMemPoolEntry::Parents setParentCheck;
for (const CTxIn &txin : tx.vin) {
// Check that every mempool transaction's inputs refer to available coins, or other mempool tx's.
@@ -714,17 +702,17 @@ void CTxMemPool::check(CChainState& active_chainstate) const
if (it2 != mapTx.end()) {
const CTransaction& tx2 = it2->GetTx();
assert(tx2.vout.size() > txin.prevout.n && !tx2.vout[txin.prevout.n].IsNull());
- fDependsWait = true;
setParentCheck.insert(*it2);
- } else {
- assert(active_coins_tip.HaveCoin(txin.prevout));
}
+ // We are iterating through the mempool entries sorted in order by ancestor count.
+ // All parents must have been checked before their children and their coins added to
+ // the mempoolDuplicate coins cache.
+ assert(mempoolDuplicate.HaveCoin(txin.prevout));
// Check whether its inputs are marked in mapNextTx.
auto it3 = mapNextTx.find(txin.prevout);
assert(it3 != mapNextTx.end());
assert(it3->first == &txin.prevout);
assert(it3->second == &tx);
- i++;
}
auto comp = [](const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) -> bool {
return a.GetTx().GetHash() == b.GetTx().GetHash();
@@ -751,6 +739,9 @@ void CTxMemPool::check(CChainState& active_chainstate) const
assert(it->GetSizeWithAncestors() == nSizeCheck);
assert(it->GetSigOpCostWithAncestors() == nSigOpCheck);
assert(it->GetModFeesWithAncestors() == nFeesCheck);
+ // Sanity check: we are walking in ascending ancestor count order.
+ assert(prev_ancestor_count <= it->GetCountWithAncestors());
+ prev_ancestor_count = it->GetCountWithAncestors();
// Check children against mapNextTx
CTxMemPoolEntry::Children setChildrenCheck;
@@ -769,24 +760,12 @@ void CTxMemPool::check(CChainState& active_chainstate) const
// just a sanity check, not definitive that this calc is correct...
assert(it->GetSizeWithDescendants() >= child_sizes + it->GetTxSize());
- if (fDependsWait)
- waitingOnDependants.push_back(&(*it));
- else {
- CheckInputsAndUpdateCoins(tx, mempoolDuplicate, spendheight);
- }
- }
- unsigned int stepsSinceLastRemove = 0;
- while (!waitingOnDependants.empty()) {
- const CTxMemPoolEntry* entry = waitingOnDependants.front();
- waitingOnDependants.pop_front();
- if (!mempoolDuplicate.HaveInputs(entry->GetTx())) {
- waitingOnDependants.push_back(entry);
- stepsSinceLastRemove++;
- assert(stepsSinceLastRemove < waitingOnDependants.size());
- } else {
- CheckInputsAndUpdateCoins(entry->GetTx(), mempoolDuplicate, spendheight);
- stepsSinceLastRemove = 0;
- }
+ TxValidationState dummy_state; // Not used. CheckTxInputs() should always pass
+ CAmount txfee = 0;
+ assert(!tx.IsCoinBase());
+ assert(Consensus::CheckTxInputs(tx, dummy_state, mempoolDuplicate, spendheight, txfee));
+ for (const auto& input: tx.vin) mempoolDuplicate.SpendCoin(input.prevout);
+ AddCoins(mempoolDuplicate, tx, std::numeric_limits<int>::max());
}
for (auto it = mapNextTx.cbegin(); it != mapNextTx.cend(); it++) {
uint256 hash = it->second->GetHash();
diff --git a/src/txmempool.h b/src/txmempool.h
index c63522225a..90b2aee371 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -622,7 +622,7 @@ public:
* all inputs are in the mapNextTx array). If sanity-checking is turned off,
* check does nothing.
*/
- void check(CChainState& active_chainstate) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+ void check(const CCoinsViewCache& active_coins_tip, int64_t spendheight) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
// addUnchecked must updated state for all ancestors of a given transaction,
// to track size/count of descendant transactions. First version of
diff --git a/src/validation.cpp b/src/validation.cpp
index 8d64a9a460..207cdc8233 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1236,12 +1236,6 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo &txund
AddCoins(inputs, tx, nHeight);
}
-void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight)
-{
- CTxUndo txundo;
- UpdateCoins(tx, inputs, txundo, nHeight);
-}
-
bool CScriptCheck::operator()() {
const CScript &scriptSig = ptxTo->vin[nIn].scriptSig;
const CScriptWitness *witness = &ptxTo->vin[nIn].scriptWitness;
@@ -2487,7 +2481,7 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, CBlockIndex
// any disconnected transactions back to the mempool.
MaybeUpdateMempoolForReorg(disconnectpool, true);
}
- if (m_mempool) m_mempool->check(*this);
+ if (m_mempool) m_mempool->check(this->CoinsTip(), this->m_chain.Height() + 1);
CheckForkWarningConditions();
diff --git a/src/validation.h b/src/validation.h
index 1c23b3ad5c..4da8ec8d24 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -229,9 +229,6 @@ PackageMempoolAcceptResult ProcessNewPackage(CChainState& active_chainstate, CTx
const Package& txns, bool test_accept)
EXCLUSIVE_LOCKS_REQUIRED(cs_main);
-/** Apply the effects of this transaction on the UTXO set represented by view */
-void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight);
-
/** Transaction validation functions */
/**
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-07 14:05:11 +0000