diff --git a/src/bench/mempool_eviction.cpp b/src/bench/mempool_eviction.cpp --- a/src/bench/mempool_eviction.cpp +++ b/src/bench/mempool_eviction.cpp @@ -15,8 +15,8 @@ bool spendsCoinbase = false; unsigned int nSigChecks = 1; LockPoints lp; - pool.addUnchecked(CTxMemPoolEntry(tx, nFee, nTime, nHeight, spendsCoinbase, - nSigChecks, lp)); + pool.addUnchecked(std::make_shared( + tx, nFee, nTime, nHeight, spendsCoinbase, nSigChecks, lp)); } // Right now this is only testing eviction performance in an extremely small diff --git a/src/bench/mempool_stress.cpp b/src/bench/mempool_stress.cpp --- a/src/bench/mempool_stress.cpp +++ b/src/bench/mempool_stress.cpp @@ -18,8 +18,8 @@ bool spendsCoinbase = false; unsigned int sigChecks = 1; LockPoints lp; - pool.addUnchecked(CTxMemPoolEntry(tx, 1000 * SATOSHI, nTime, nHeight, - spendsCoinbase, sigChecks, lp)); + pool.addUnchecked(std::make_shared( + tx, 1000 * SATOSHI, nTime, nHeight, spendsCoinbase, sigChecks, lp)); } struct Available { diff --git a/src/bench/rpc_mempool.cpp b/src/bench/rpc_mempool.cpp --- a/src/bench/rpc_mempool.cpp +++ b/src/bench/rpc_mempool.cpp @@ -14,10 +14,11 @@ static void AddTx(const CTransactionRef &tx, const Amount &fee, CTxMemPool &pool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, pool.cs) { LockPoints lp; - pool.addUnchecked(CTxMemPoolEntry(tx, fee, /* time */ 0, - /* height */ 1, - /* spendsCoinbase */ false, - /*_sigChecks=*/1, lp)); + pool.addUnchecked( + std::make_shared(tx, fee, /* time */ 0, + /* height */ 1, + /* spendsCoinbase */ false, + /*_sigChecks=*/1, lp)); } static void RpcMempool(benchmark::Bench &bench) { diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -513,21 +513,21 @@ auto entryNormal = entry.Fee(1000 * SATOSHI).FromTx(tx); BOOST_CHECK_EQUAL(1000 * SATOSHI, - entryNormal.GetModifiedFeeRate().GetFee(1000)); + entryNormal->GetModifiedFeeRate().GetFee(1000)); // Add modified fee - CTxMemPoolEntry entryFeeModified = entry.Fee(1000 * SATOSHI).FromTx(tx); - entryFeeModified.UpdateFeeDelta(1000 * SATOSHI); + CTxMemPoolEntryRef entryFeeModified = entry.Fee(1000 * SATOSHI).FromTx(tx); + entryFeeModified->UpdateFeeDelta(1000 * SATOSHI); BOOST_CHECK_EQUAL(2000 * SATOSHI, - entryFeeModified.GetModifiedFeeRate().GetFee(1000)); + entryFeeModified->GetModifiedFeeRate().GetFee(1000)); // Excessive sigop count "modifies" size - CTxMemPoolEntry entrySizeModified = + CTxMemPoolEntryRef entrySizeModified = entry.Fee(1000 * SATOSHI) .SigChecks(2000 / DEFAULT_BYTES_PER_SIGCHECK) .FromTx(tx); BOOST_CHECK_EQUAL(500 * SATOSHI, - entrySizeModified.GetModifiedFeeRate().GetFee(1000)); + entrySizeModified->GetModifiedFeeRate().GetFee(1000)); } BOOST_AUTO_TEST_CASE(CompareTxMemPoolEntryByModifiedFeeRateTest) { @@ -563,18 +563,18 @@ // Same with fee delta. { - CTxMemPoolEntry entryA = entry.Fee(100 * SATOSHI).FromTx(a); - CTxMemPoolEntry entryB = entry.Fee(200 * SATOSHI).FromTx(b); + CTxMemPoolEntryRef entryA = entry.Fee(100 * SATOSHI).FromTx(a); + CTxMemPoolEntryRef entryB = entry.Fee(200 * SATOSHI).FromTx(b); // .. A and B have same modified fee, ordering is by lowest txid - entryA.UpdateFeeDelta(100 * SATOSHI); + entryA->UpdateFeeDelta(100 * SATOSHI); checkOrdering(entryB, entryA); } // .. A is first entering the mempool - CTxMemPoolEntry entryA = entry.Fee(100 * SATOSHI).EntryId(1).FromTx(a); - CTxMemPoolEntry entryB = entry.Fee(100 * SATOSHI).EntryId(2).FromTx(b); + CTxMemPoolEntryRef entryA = entry.Fee(100 * SATOSHI).EntryId(1).FromTx(a); + CTxMemPoolEntryRef entryB = entry.Fee(100 * SATOSHI).EntryId(2).FromTx(b); checkOrdering(entryA, entryB); // .. B has higher modified fee. - entryB.UpdateFeeDelta(1 * SATOSHI); + entryB->UpdateFeeDelta(1 * SATOSHI); checkOrdering(entryB, entryA); } diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h --- a/src/test/util/setup_common.h +++ b/src/test/util/setup_common.h @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -198,8 +199,6 @@ CKey coinbaseKey; }; -class CTxMemPoolEntry; - struct TestMemPoolEntryHelper { // Default values Amount nFee; @@ -212,8 +211,8 @@ TestMemPoolEntryHelper() : nFee(), nTime(0), nHeight(1), spendsCoinbase(false), nSigChecks(1) {} - CTxMemPoolEntry FromTx(const CMutableTransaction &tx) const; - CTxMemPoolEntry FromTx(const CTransactionRef &tx) const; + CTxMemPoolEntryRef FromTx(const CMutableTransaction &tx) const; + CTxMemPoolEntryRef FromTx(const CTransactionRef &tx) const; // Change the default value TestMemPoolEntryHelper &Fee(Amount _fee) { diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -422,17 +422,17 @@ SetMockTime(0); } -CTxMemPoolEntry +CTxMemPoolEntryRef TestMemPoolEntryHelper::FromTx(const CMutableTransaction &tx) const { return FromTx(MakeTransactionRef(tx)); } -CTxMemPoolEntry +CTxMemPoolEntryRef TestMemPoolEntryHelper::FromTx(const CTransactionRef &tx) const { CTxMemPoolEntry ret(tx, nFee, nTime, nHeight, spendsCoinbase, nSigChecks, LockPoints()); ret.SetEntryId(entryId); - return ret; + return std::make_shared(std::move(ret)); } /** diff --git a/src/txmempool.h b/src/txmempool.h --- a/src/txmempool.h +++ b/src/txmempool.h @@ -126,6 +126,9 @@ unsigned int entry_height, bool spends_coinbase, int64_t sigchecks, LockPoints lp); + CTxMemPoolEntry(const CTxMemPoolEntry &other) = delete; + CTxMemPoolEntry(CTxMemPoolEntry &&other) = default; + uint64_t GetEntryId() const { return entryId; } //! This should only be set by addUnchecked() before entry insertion into //! mempool @@ -453,7 +456,7 @@ // addUnchecked must update state for all parents of a given transaction, // updating child links as necessary. - void addUnchecked(const CTxMemPoolEntry &entry) + void addUnchecked(const CTxMemPoolEntryRef &entry) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); void removeRecursive(const CTransaction &tx, MemPoolRemovalReason reason) diff --git a/src/txmempool.cpp b/src/txmempool.cpp --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -176,22 +176,20 @@ nTransactionsUpdated += n; } -void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entryIn) { - CTxMemPoolEntry entry{entryIn}; +void CTxMemPool::addUnchecked(const CTxMemPoolEntryRef &entry) { // get a guaranteed unique id (in case tests re-use the same object) - entry.SetEntryId(nextEntryId++); + entry->SetEntryId(nextEntryId++); // Update transaction for any feeDelta created by PrioritiseTransaction { Amount feeDelta = Amount::zero(); - ApplyDelta(entry.GetTx().GetId(), feeDelta); - entry.UpdateFeeDelta(feeDelta); + ApplyDelta(entry->GetTx().GetId(), feeDelta); + entry->UpdateFeeDelta(feeDelta); } // Add to memory pool without checking anything. // Used by AcceptToMemoryPool(), which DOES do all the appropriate checks. - auto [newit, inserted] = - mapTx.insert(std::make_shared(entry)); + auto [newit, inserted] = mapTx.insert(entry); // Sanity check: It is a programming error if insertion fails (uniqueness // invariants in mapTx are violated, etc) assert(inserted); @@ -202,9 +200,9 @@ // Update cachedInnerUsage to include contained transaction's usage. // (When we update the entry for in-mempool parents, memory usage will be // further updated.) - cachedInnerUsage += entry.DynamicMemoryUsage(); + cachedInnerUsage += entry->DynamicMemoryUsage(); - const CTransaction &tx = entry.GetTx(); + const CTransaction &tx = entry->GetTx(); std::set setParentTransactions; for (const CTxIn &in : tx.vin) { mapNextTx.insert(std::make_pair(&in.prevout, &tx)); @@ -222,8 +220,8 @@ UpdateParentsOf(true, newit); nTransactionsUpdated++; - totalTxSize += entry.GetTxSize(); - m_total_fee += entry.GetFee(); + totalTxSize += entry->GetTxSize(); + m_total_fee += entry->GetFee(); } void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason) { diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -494,7 +494,6 @@ // Alias what we need out of ws TxValidationState &state = ws.m_state; - std::unique_ptr &entry = ws.m_entry; // Coinbase is only valid in a block, not as a loose transaction. if (!CheckRegularTransaction(tx, state)) { // state filled in by CheckRegularTransaction. @@ -652,12 +651,12 @@ return false; } - entry.reset(new CTxMemPoolEntry( + ws.m_entry = std::make_unique( ptx, ws.m_base_fees, nAcceptTime, heightOverride ? heightOverride : m_active_chainstate.m_chain.Height(), - fSpendsCoinbase, ws.m_sig_checks_standard, lp)); + fSpendsCoinbase, ws.m_sig_checks_standard, lp); - ws.m_vsize = entry->GetTxVirtualSize(); + ws.m_vsize = ws.m_entry->GetTxVirtualSize(); Amount mempoolRejectFee = m_pool @@ -726,10 +725,9 @@ TxValidationState &state = ws.m_state; const bool bypass_limits = args.m_bypass_limits; - std::unique_ptr &entry = ws.m_entry; - - // Store transaction in memory. - m_pool.addUnchecked(*entry); + // Store transaction in memory + CTxMemPoolEntryRef entry = std::shared_ptr(std::move(ws.m_entry)); + m_pool.addUnchecked(entry); // Trim mempool and check if tx was trimmed. // If we are validating a package, don't trim here because we could evict a