diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index 211b4740be9..29306b22290 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -107,12 +107,12 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c std::vector have_txn(txn_available.size()); { LOCK(pool->cs); - for (size_t i = 0; i < pool->vTxHashes.size(); i++) { - uint64_t shortid = cmpctblock.GetShortID(pool->vTxHashes[i].first); + for (const auto& tx : pool->txns_randomized) { + uint64_t shortid = cmpctblock.GetShortID(tx->GetWitnessHash()); std::unordered_map::iterator idit = shorttxids.find(shortid); if (idit != shorttxids.end()) { if (!have_txn[idit->second]) { - txn_available[idit->second] = pool->vTxHashes[i].second->GetSharedTx(); + txn_available[idit->second] = tx; have_txn[idit->second] = true; mempool_count++; } else { diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h index 1f175a5ccf9..7c905ca4f4a 100644 --- a/src/kernel/mempool_entry.h +++ b/src/kernel/mempool_entry.h @@ -83,7 +83,7 @@ private: const bool spendsCoinbase; //!< keep track of transactions that spend a coinbase const int64_t sigOpCost; //!< Total sigop cost CAmount m_modified_fee; //!< Used for determining the priority of the transaction for mining in a block - LockPoints lockPoints; //!< Track the height and time at which tx was final + mutable LockPoints lockPoints; //!< Track the height and time at which tx was final // Information about descendants of this transaction that are in the // mempool; if we remove this transaction we must remove all of these @@ -151,7 +151,7 @@ public: } // Update the LockPoints after a reorg - void UpdateLockPoints(const LockPoints& lp) + void UpdateLockPoints(const LockPoints& lp) const { lockPoints = lp; } @@ -172,8 +172,10 @@ public: Parents& GetMemPoolParents() const { return m_parents; } Children& GetMemPoolChildren() const { return m_children; } - mutable size_t vTxHashesIdx; //!< Index in mempool's vTxHashes + mutable size_t idx_randomized; //!< Index in mempool's txns_randomized mutable Epoch::Marker m_epoch_marker; //!< epoch when last touched, useful for graph algorithms }; +using CTxMemPoolEntryRef = CTxMemPoolEntry::CTxMemPoolEntryRef; + #endif // BITCOIN_KERNEL_MEMPOOL_ENTRY_H diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index f6dbe4f0087..4c83df7eca3 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -648,8 +648,9 @@ public: { if (!m_node.mempool) return false; LOCK(m_node.mempool->cs); - auto it = m_node.mempool->GetIter(txid); - return it && (*it)->GetCountWithDescendants() > 1; + const auto entry{m_node.mempool->GetEntry(Txid::FromUint256(txid))}; + if (entry == nullptr) return false; + return entry->GetCountWithDescendants() > 1; } bool broadcastTransaction(const CTransactionRef& tx, const CAmount& max_tx_fee, @@ -702,9 +703,9 @@ public: if (!m_node.mempool) return true; LockPoints lp; CTxMemPoolEntry entry(tx, 0, 0, 0, 0, false, 0, lp); - const CTxMemPool::Limits& limits{m_node.mempool->m_limits}; LOCK(m_node.mempool->cs); - return m_node.mempool->CalculateMemPoolAncestors(entry, limits).has_value(); + std::string err_string; + return m_node.mempool->CheckPackageLimits({tx}, entry.GetTxSize(), err_string); } CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc) override { @@ -806,7 +807,7 @@ public: { if (!m_node.mempool) return; LOCK2(::cs_main, m_node.mempool->cs); - for (const CTxMemPoolEntry& entry : m_node.mempool->mapTx) { + for (const CTxMemPoolEntry& entry : m_node.mempool->entryAll()) { notifications.transactionAddedToMempool(entry.GetSharedTx()); } } diff --git a/src/node/miner.cpp b/src/node/miner.cpp index caa29918193..ce5452d1f9d 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -188,7 +188,7 @@ void BlockAssembler::onlyUnconfirmed(CTxMemPool::setEntries& testSet) { for (CTxMemPool::setEntries::iterator iit = testSet.begin(); iit != testSet.end(); ) { // Only test txs not already in the block - if (inBlock.count(*iit)) { + if (inBlock.count((*iit)->GetSharedTx()->GetHash())) { testSet.erase(iit++); } else { iit++; @@ -229,7 +229,7 @@ void BlockAssembler::AddToBlock(CTxMemPool::txiter iter) ++nBlockTx; nBlockSigOpsCost += iter->GetSigOpCost(); nFees += iter->GetFee(); - inBlock.insert(iter); + inBlock.insert(iter->GetSharedTx()->GetHash()); bool fPrintPriority = gArgs.GetBoolArg("-printpriority", DEFAULT_PRINTPRIORITY); if (fPrintPriority) { @@ -298,7 +298,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele // because some of their txs are already in the block indexed_modified_transaction_set mapModifiedTx; // Keep track of entries that failed inclusion, to avoid duplicate work - CTxMemPool::setEntries failedTx; + std::set failedTx; CTxMemPool::indexed_transaction_set::index::type::iterator mi = mempool.mapTx.get().begin(); CTxMemPool::txiter iter; @@ -326,7 +326,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele if (mi != mempool.mapTx.get().end()) { auto it = mempool.mapTx.project<0>(mi); assert(it != mempool.mapTx.end()); - if (mapModifiedTx.count(it) || inBlock.count(it) || failedTx.count(it)) { + if (mapModifiedTx.count(it) || inBlock.count(it->GetSharedTx()->GetHash()) || failedTx.count(it->GetSharedTx()->GetHash())) { ++mi; continue; } @@ -360,7 +360,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele // We skip mapTx entries that are inBlock, and mapModifiedTx shouldn't // contain anything that is inBlock. - assert(!inBlock.count(iter)); + assert(!inBlock.count(iter->GetSharedTx()->GetHash())); uint64_t packageSize = iter->GetSizeWithAncestors(); CAmount packageFees = iter->GetModFeesWithAncestors(); @@ -382,7 +382,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele // we must erase failed entries so that we can consider the // next best entry on the next loop iteration mapModifiedTx.get().erase(modit); - failedTx.insert(iter); + failedTx.insert(iter->GetSharedTx()->GetHash()); } ++nConsecutiveFailed; @@ -404,7 +404,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele if (!TestPackageTransactions(ancestors)) { if (fUsingModified) { mapModifiedTx.get().erase(modit); - failedTx.insert(iter); + failedTx.insert(iter->GetSharedTx()->GetHash()); } continue; } diff --git a/src/node/miner.h b/src/node/miner.h index 41735215854..06a917228dc 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -142,7 +142,7 @@ private: uint64_t nBlockTx; uint64_t nBlockSigOpsCost; CAmount nFees; - CTxMemPool::setEntries inBlock; + std::unordered_set inBlock; // Chain context for the block int nHeight; diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp index d032b740088..76ab2b1a969 100644 --- a/src/policy/rbf.cpp +++ b/src/policy/rbf.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -35,7 +36,7 @@ RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool) // If all the inputs have nSequence >= maxint-1, it still might be // signaled for RBF if any unconfirmed parents have signaled. - const CTxMemPoolEntry entry{*pool.mapTx.find(tx.GetHash())}; + const auto& entry{*Assert(pool.GetEntry(tx.GetHash()))}; auto ancestors{pool.AssumeCalculateMemPoolAncestors(__func__, entry, CTxMemPool::Limits::NoLimits(), /*fSearchForParents=*/false)}; diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index b387febc1d2..bf60eae4332 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -290,7 +290,7 @@ static void entryToJSON(const CTxMemPool& pool, UniValue& info, const CTxMemPool info.pushKV("descendantsize", e.GetSizeWithDescendants()); info.pushKV("ancestorcount", e.GetCountWithAncestors()); info.pushKV("ancestorsize", e.GetSizeWithAncestors()); - info.pushKV("wtxid", pool.vTxHashes[e.vTxHashesIdx].first.ToString()); + info.pushKV("wtxid", e.GetTx().GetWitnessHash().ToString()); UniValue fees(UniValue::VOBJ); fees.pushKV("base", ValueFromAmount(e.GetFee())); @@ -316,9 +316,7 @@ static void entryToJSON(const CTxMemPool& pool, UniValue& info, const CTxMemPool info.pushKV("depends", depends); UniValue spent(UniValue::VARR); - const CTxMemPool::txiter& it = pool.mapTx.find(tx.GetHash()); - const CTxMemPoolEntry::Children& children = it->GetMemPoolChildrenConst(); - for (const CTxMemPoolEntry& child : children) { + for (const CTxMemPoolEntry& child : e.GetMemPoolChildrenConst()) { spent.push_back(child.GetTx().GetHash().ToString()); } @@ -345,14 +343,13 @@ UniValue MempoolToJSON(const CTxMemPool& pool, bool verbose, bool include_mempoo } LOCK(pool.cs); UniValue o(UniValue::VOBJ); - for (const CTxMemPoolEntry& e : pool.mapTx) { - const uint256& hash = e.GetTx().GetHash(); + for (const CTxMemPoolEntry& e : pool.entryAll()) { UniValue info(UniValue::VOBJ); entryToJSON(pool, info, e); // Mempool has unique entries so there is no advantage in using // UniValue::pushKV, which checks if the key already exists in O(N). // UniValue::pushKVEnd is used instead which currently is O(1). - o.pushKVEnd(hash.ToString(), info); + o.pushKVEnd(e.GetTx().GetHash().ToString(), info); } return o; } else { @@ -461,12 +458,12 @@ static RPCHelpMan getmempoolancestors() const CTxMemPool& mempool = EnsureAnyMemPool(request.context); LOCK(mempool.cs); - CTxMemPool::txiter it = mempool.mapTx.find(hash); - if (it == mempool.mapTx.end()) { + const auto entry{mempool.GetEntry(Txid::FromUint256(hash))}; + if (entry == nullptr) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not in mempool"); } - auto ancestors{mempool.AssumeCalculateMemPoolAncestors(self.m_name, *it, CTxMemPool::Limits::NoLimits(), /*fSearchForParents=*/false)}; + auto ancestors{mempool.AssumeCalculateMemPoolAncestors(self.m_name, *entry, CTxMemPool::Limits::NoLimits(), /*fSearchForParents=*/false)}; if (!fVerbose) { UniValue o(UniValue::VARR); @@ -522,15 +519,15 @@ static RPCHelpMan getmempooldescendants() const CTxMemPool& mempool = EnsureAnyMemPool(request.context); LOCK(mempool.cs); - CTxMemPool::txiter it = mempool.mapTx.find(hash); - if (it == mempool.mapTx.end()) { + const auto it{mempool.GetIter(hash)}; + if (!it) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not in mempool"); } CTxMemPool::setEntries setDescendants; - mempool.CalculateDescendants(it, setDescendants); + mempool.CalculateDescendants(*it, setDescendants); // CTxMemPool::CalculateDescendants will include the given tx - setDescendants.erase(it); + setDescendants.erase(*it); if (!fVerbose) { UniValue o(UniValue::VARR); @@ -574,14 +571,13 @@ static RPCHelpMan getmempoolentry() const CTxMemPool& mempool = EnsureAnyMemPool(request.context); LOCK(mempool.cs); - CTxMemPool::txiter it = mempool.mapTx.find(hash); - if (it == mempool.mapTx.end()) { + const auto entry{mempool.GetEntry(Txid::FromUint256(hash))}; + if (entry == nullptr) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not in mempool"); } - const CTxMemPoolEntry &e = *it; UniValue info(UniValue::VOBJ); - entryToJSON(mempool, info, e); + entryToJSON(mempool, info, *entry); return info; }, }; diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index 4348a20886f..e4ef019daf6 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -51,8 +51,8 @@ static CBlock BuildBlockTestCase() { } // Number of shared use_counts we expect for a tx we haven't touched -// (block + mempool + our copy from the GetSharedTx call) -constexpr long SHARED_TX_OFFSET{3}; +// (block + mempool entry + mempool txns_randomized + our copy from the GetSharedTx call) +constexpr long SHARED_TX_OFFSET{4}; BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) { @@ -62,7 +62,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) LOCK2(cs_main, pool.cs); pool.addUnchecked(entry.FromTx(block.vtx[2])); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); + BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 0); // Do a simple ShortTxIDs RT { @@ -80,7 +80,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) BOOST_CHECK(!partialBlock.IsTxAvailable(1)); BOOST_CHECK( partialBlock.IsTxAvailable(2)); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); + BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 1); size_t poolSize = pool.size(); pool.removeRecursive(*block.vtx[2], MemPoolRemovalReason::REPLACED); @@ -145,7 +145,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) LOCK2(cs_main, pool.cs); pool.addUnchecked(entry.FromTx(block.vtx[2])); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); + BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 0); uint256 txhash; @@ -170,7 +170,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) BOOST_CHECK( partialBlock.IsTxAvailable(1)); BOOST_CHECK( partialBlock.IsTxAvailable(2)); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); // +1 because of partialBlock + BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 1); // +1 because of partialBlock CBlock block2; { @@ -185,7 +185,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) partialBlock.FillBlock(block2, {block.vtx[1]}); // Current implementation doesn't check txn here, but don't require that partialBlock = tmp; } - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 2); // +2 because of partialBlock and block2 + BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 2); // +2 because of partialBlock and block2 bool mutated; BOOST_CHECK(block.hashMerkleRoot != BlockMerkleRoot(block2, &mutated)); @@ -196,15 +196,15 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) BOOST_CHECK_EQUAL(block.hashMerkleRoot.ToString(), BlockMerkleRoot(block3, &mutated).ToString()); BOOST_CHECK(!mutated); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 3); // +2 because of partialBlock and block2 and block3 + BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 3); // +2 because of partialBlock and block2 and block3 txhash = block.vtx[2]->GetHash(); block.vtx.clear(); block2.vtx.clear(); block3.vtx.clear(); - BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1 - 1); // + 1 because of partialBlock; -1 because of block. + BOOST_CHECK_EQUAL(pool.get(txhash).use_count(), SHARED_TX_OFFSET + 1 - 1); // + 1 because of partialBlock; -1 because of block. } - BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET - 1); // -1 because of block + BOOST_CHECK_EQUAL(pool.get(txhash).use_count(), SHARED_TX_OFFSET - 1); // -1 because of block } BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) @@ -215,7 +215,7 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) LOCK2(cs_main, pool.cs); pool.addUnchecked(entry.FromTx(block.vtx[1])); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[1]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); + BOOST_CHECK_EQUAL(pool.get(block.vtx[1]->GetHash()).use_count(), SHARED_TX_OFFSET + 0); uint256 txhash; @@ -240,7 +240,7 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) BOOST_CHECK( partialBlock.IsTxAvailable(1)); BOOST_CHECK( partialBlock.IsTxAvailable(2)); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[1]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); + BOOST_CHECK_EQUAL(pool.get(block.vtx[1]->GetHash()).use_count(), SHARED_TX_OFFSET + 1); CBlock block2; PartiallyDownloadedBlock partialBlockCopy = partialBlock; @@ -253,9 +253,9 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) txhash = block.vtx[1]->GetHash(); block.vtx.clear(); block2.vtx.clear(); - BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1 - 1); // + 1 because of partialBlock; -1 because of block. + BOOST_CHECK_EQUAL(pool.get(txhash).use_count(), SHARED_TX_OFFSET + 1 - 1); // + 1 because of partialBlock; -1 because of block. } - BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET - 1); // -1 because of block + BOOST_CHECK_EQUAL(pool.get(txhash).use_count(), SHARED_TX_OFFSET - 1); // -1 because of block } BOOST_AUTO_TEST_CASE(EmptyBlockRoundTripTest) diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index db58a0baec1..217e4a6d223 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -191,7 +191,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) CheckSort(pool, sortedOrder); CTxMemPool::setEntries setAncestors; - setAncestors.insert(pool.mapTx.find(tx6.GetHash())); + setAncestors.insert(pool.GetIter(tx6.GetHash()).value()); CMutableTransaction tx7 = CMutableTransaction(); tx7.vin.resize(1); tx7.vin[0].prevout = COutPoint(tx6.GetHash(), 0); @@ -223,7 +223,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) tx8.vout.resize(1); tx8.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx8.vout[0].nValue = 10 * COIN; - setAncestors.insert(pool.mapTx.find(tx7.GetHash())); + setAncestors.insert(pool.GetIter(tx7.GetHash()).value()); pool.addUnchecked(entry.Fee(0LL).Time(NodeSeconds{2s}).FromTx(tx8), setAncestors); // Now tx8 should be sorted low, but tx6/tx both high @@ -247,8 +247,8 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) std::vector snapshotOrder = sortedOrder; - setAncestors.insert(pool.mapTx.find(tx8.GetHash())); - setAncestors.insert(pool.mapTx.find(tx9.GetHash())); + setAncestors.insert(pool.GetIter(tx8.GetHash()).value()); + setAncestors.insert(pool.GetIter(tx9.GetHash()).value()); /* tx10 depends on tx8 and tx9 and has a high fee*/ CMutableTransaction tx10 = CMutableTransaction(); tx10.vin.resize(2); @@ -291,11 +291,11 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) BOOST_CHECK_EQUAL(pool.size(), 10U); // Now try removing tx10 and verify the sort order returns to normal - pool.removeRecursive(pool.mapTx.find(tx10.GetHash())->GetTx(), REMOVAL_REASON_DUMMY); + pool.removeRecursive(*Assert(pool.get(tx10.GetHash())), REMOVAL_REASON_DUMMY); CheckSort(pool, snapshotOrder); - pool.removeRecursive(pool.mapTx.find(tx9.GetHash())->GetTx(), REMOVAL_REASON_DUMMY); - pool.removeRecursive(pool.mapTx.find(tx8.GetHash())->GetTx(), REMOVAL_REASON_DUMMY); + pool.removeRecursive(*Assert(pool.get(tx9.GetHash())), REMOVAL_REASON_DUMMY); + pool.removeRecursive(*Assert(pool.get(tx8.GetHash())), REMOVAL_REASON_DUMMY); } BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) diff --git a/src/test/miniminer_tests.cpp b/src/test/miniminer_tests.cpp index 2531ea7c47e..311e402e3e5 100644 --- a/src/test/miniminer_tests.cpp +++ b/src/test/miniminer_tests.cpp @@ -94,7 +94,7 @@ BOOST_FIXTURE_TEST_CASE(miniminer_negative, TestChain100Setup) const CFeeRate feerate_zero(0); mini_miner_target0.BuildMockTemplate(feerate_zero); // Check the quit condition: - BOOST_CHECK(negative_modified_fees < feerate_zero.GetFee(pool.GetIter(tx_mod_negative->GetHash()).value()->GetTxSize())); + BOOST_CHECK(negative_modified_fees < feerate_zero.GetFee(Assert(pool.GetEntry(tx_mod_negative->GetHash()))->GetTxSize())); BOOST_CHECK(mini_miner_target0.GetMockTemplateTxids().empty()); // With no target feerate, the template includes all transactions, even negative feerate ones. @@ -179,9 +179,9 @@ BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup) }; std::map tx_dims; for (const auto& tx : all_transactions) { - const auto it = pool.GetIter(tx->GetHash()).value(); - tx_dims.emplace(tx->GetHash(), TxDimensions{it->GetTxSize(), it->GetModifiedFee(), - CFeeRate(it->GetModifiedFee(), it->GetTxSize())}); + const auto& entry{*Assert(pool.GetEntry(tx->GetHash()))}; + tx_dims.emplace(tx->GetHash(), TxDimensions{entry.GetTxSize(), entry.GetModifiedFee(), + CFeeRate(entry.GetModifiedFee(), entry.GetTxSize())}); } const std::vector various_normal_feerates({CFeeRate(0), CFeeRate(500), CFeeRate(999), @@ -447,15 +447,15 @@ BOOST_FIXTURE_TEST_CASE(miniminer_overlap, TestChain100Setup) // tx3's feerate is lower than tx2's. same fee, different weight. BOOST_CHECK(tx2_feerate > tx3_feerate); const auto tx3_anc_feerate = CFeeRate(low_fee + med_fee + high_fee + high_fee, tx_vsizes[0] + tx_vsizes[1] + tx_vsizes[2] + tx_vsizes[3]); - const auto tx3_iter = pool.GetIter(tx3->GetHash()); - BOOST_CHECK(tx3_anc_feerate == CFeeRate(tx3_iter.value()->GetModFeesWithAncestors(), tx3_iter.value()->GetSizeWithAncestors())); + const auto& tx3_entry{*Assert(pool.GetEntry(tx3->GetHash()))}; + BOOST_CHECK(tx3_anc_feerate == CFeeRate(tx3_entry.GetModFeesWithAncestors(), tx3_entry.GetSizeWithAncestors())); const auto tx4_feerate = CFeeRate(high_fee, tx_vsizes[4]); const auto tx6_anc_feerate = CFeeRate(high_fee + low_fee + med_fee, tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[6]); - const auto tx6_iter = pool.GetIter(tx6->GetHash()); - BOOST_CHECK(tx6_anc_feerate == CFeeRate(tx6_iter.value()->GetModFeesWithAncestors(), tx6_iter.value()->GetSizeWithAncestors())); + const auto& tx6_entry{*Assert(pool.GetEntry(tx6->GetHash()))}; + BOOST_CHECK(tx6_anc_feerate == CFeeRate(tx6_entry.GetModFeesWithAncestors(), tx6_entry.GetSizeWithAncestors())); const auto tx7_anc_feerate = CFeeRate(high_fee + low_fee + high_fee, tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[7]); - const auto tx7_iter = pool.GetIter(tx7->GetHash()); - BOOST_CHECK(tx7_anc_feerate == CFeeRate(tx7_iter.value()->GetModFeesWithAncestors(), tx7_iter.value()->GetSizeWithAncestors())); + const auto& tx7_entry{*Assert(pool.GetEntry(tx7->GetHash()))}; + BOOST_CHECK(tx7_anc_feerate == CFeeRate(tx7_entry.GetModFeesWithAncestors(), tx7_entry.GetSizeWithAncestors())); BOOST_CHECK(tx4_feerate > tx6_anc_feerate); BOOST_CHECK(tx4_feerate > tx7_anc_feerate); diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 64cb5522ebe..35e5c6a0372 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -283,8 +283,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg) // Check that all txs are in the pool { - LOCK(m_node.mempool->cs); - BOOST_CHECK_EQUAL(m_node.mempool->mapTx.size(), txs.size()); + BOOST_CHECK_EQUAL(m_node.mempool->size(), txs.size()); } // Run a thread that simulates an RPC caller that is polling while @@ -295,7 +294,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg) // not some intermediate amount. while (true) { LOCK(m_node.mempool->cs); - if (m_node.mempool->mapTx.size() == 0) { + if (m_node.mempool->size() == 0) { // We are done with the reorg break; } @@ -304,7 +303,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg) // be atomic. So the caller assumes that the returned mempool // is consistent. That is, it has all txs that were there // before the reorg. - assert(m_node.mempool->mapTx.size() == txs.size()); + assert(m_node.mempool->size() == txs.size()); continue; } LOCK(cs_main); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 461662ad93a..efcb77f47cf 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -480,8 +480,8 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces minerPolicyEstimator->processTransaction(entry, validFeeEstimate); } - vTxHashes.emplace_back(tx.GetWitnessHash(), newit); - newit->vTxHashesIdx = vTxHashes.size() - 1; + txns_randomized.emplace_back(newit->GetSharedTx()); + newit->idx_randomized = txns_randomized.size() - 1; TRACE3(mempool, added, entry.GetTx().GetHash().data(), @@ -517,14 +517,16 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason) RemoveUnbroadcastTx(hash, true /* add logging because unchecked */ ); - if (vTxHashes.size() > 1) { - vTxHashes[it->vTxHashesIdx] = std::move(vTxHashes.back()); - vTxHashes[it->vTxHashesIdx].second->vTxHashesIdx = it->vTxHashesIdx; - vTxHashes.pop_back(); - if (vTxHashes.size() * 2 < vTxHashes.capacity()) - vTxHashes.shrink_to_fit(); + if (txns_randomized.size() > 1) { + // Update idx_randomized of the to-be-moved entry. + Assert(GetEntry(txns_randomized.back()->GetHash()))->idx_randomized = it->idx_randomized; + // Remove entry from txns_randomized by replacing it with the back and deleting the back. + txns_randomized[it->idx_randomized] = std::move(txns_randomized.back()); + txns_randomized.pop_back(); + if (txns_randomized.size() * 2 < txns_randomized.capacity()) + txns_randomized.shrink_to_fit(); } else - vTxHashes.clear(); + txns_randomized.clear(); totalTxSize -= it->GetTxSize(); m_total_fee -= it->GetFee(); @@ -836,6 +838,18 @@ static TxMempoolInfo GetInfo(CTxMemPool::indexed_transaction_set::const_iterator return TxMempoolInfo{it->GetSharedTx(), it->GetTime(), it->GetFee(), it->GetTxSize(), it->GetModifiedFee() - it->GetFee()}; } +std::vector CTxMemPool::entryAll() const +{ + AssertLockHeld(cs); + + std::vector ret; + ret.reserve(mapTx.size()); + for (const auto& it : GetSortedDepthAndScore()) { + ret.emplace_back(*it); + } + return ret; +} + std::vector CTxMemPool::infoAll() const { LOCK(cs); @@ -850,6 +864,13 @@ std::vector CTxMemPool::infoAll() const return ret; } +const CTxMemPoolEntry* CTxMemPool::GetEntry(const Txid& txid) const +{ + AssertLockHeld(cs); + const auto i = mapTx.find(txid); + return i == mapTx.end() ? nullptr : &(*i); +} + CTransactionRef CTxMemPool::get(const uint256& hash) const { LOCK(cs); @@ -1033,7 +1054,7 @@ void CCoinsViewMemPool::Reset() size_t CTxMemPool::DynamicMemoryUsage() const { LOCK(cs); // Estimate the overhead of mapTx to be 15 pointers + an allocation, as no exact formula for boost::multi_index_contained is implemented. - return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 15 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(vTxHashes) + cachedInnerUsage; + return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 15 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(txns_randomized) + cachedInnerUsage; } void CTxMemPool::RemoveUnbroadcastTx(const uint256& txid, const bool unchecked) { diff --git a/src/txmempool.h b/src/txmempool.h index cbeabb31fa6..3b0b8cf519a 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -392,7 +392,7 @@ public: indexed_transaction_set mapTx GUARDED_BY(cs); using txiter = indexed_transaction_set::nth_index<0>::type::const_iterator; - std::vector> vTxHashes GUARDED_BY(cs); //!< All tx witness hashes/entries in mapTx, in random order + std::vector txns_randomized GUARDED_BY(cs); //!< All transactions in mapTx, in random order typedef std::set setEntries; @@ -684,6 +684,8 @@ public: return (mapTx.count(gtxid.GetHash()) != 0); } + const CTxMemPoolEntry* GetEntry(const Txid& txid) const LIFETIMEBOUND EXCLUSIVE_LOCKS_REQUIRED(cs); + CTransactionRef get(const uint256& hash) const; txiter get_iter_from_wtxid(const uint256& wtxid) const EXCLUSIVE_LOCKS_REQUIRED(cs) { @@ -695,6 +697,7 @@ public: /** Returns info for a transaction if its entry_sequence < last_sequence */ TxMempoolInfo info_for_relay(const GenTxid& gtxid, uint64_t last_sequence) const; + std::vector entryAll() const EXCLUSIVE_LOCKS_REQUIRED(cs); std::vector infoAll() const; size_t DynamicMemoryUsage() const; diff --git a/src/validation.cpp b/src/validation.cpp index 018566fa9c3..34103d18bc1 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -353,7 +353,7 @@ void Chainstate::MaybeUpdateMempoolForReorg( const std::optional new_lock_points{CalculateLockPointsAtTip(m_chain.Tip(), view_mempool, tx)}; if (new_lock_points.has_value() && CheckSequenceLocksAtTip(m_chain.Tip(), *new_lock_points)) { // Now update the mempool entry lockpoints as well. - m_mempool->mapTx.modify(it, [&new_lock_points](CTxMemPoolEntry& e) { e.UpdateLockPoints(*new_lock_points); }); + it->UpdateLockPoints(*new_lock_points); } else { return true; } @@ -362,9 +362,7 @@ void Chainstate::MaybeUpdateMempoolForReorg( // If the transaction spends any coinbase outputs, it must be mature. if (it->GetSpendsCoinbase()) { for (const CTxIn& txin : tx.vin) { - auto it2 = m_mempool->mapTx.find(txin.prevout.hash); - if (it2 != m_mempool->mapTx.end()) - continue; + if (m_mempool->exists(GenTxid::Txid(txin.prevout.hash))) continue; const Coin& coin{CoinsTip().AccessCoin(txin.prevout)}; assert(!coin.IsSpent()); const auto mempool_spend_height{m_chain.Tip()->nHeight + 1}; @@ -1506,9 +1504,8 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, // transactions that are already in the mempool, and only call AcceptMultipleTransactions() with // the new transactions. This ensures we don't double-count transaction counts and sizes when // checking ancestor/descendant limits, or double-count transaction fees for fee-related policy. - auto iter = m_pool.GetIter(txid); - assert(iter != std::nullopt); - results_final.emplace(wtxid, MempoolAcceptResult::MempoolTx(iter.value()->GetTxSize(), iter.value()->GetFee())); + const auto& entry{*Assert(m_pool.GetEntry(txid))}; + results_final.emplace(wtxid, MempoolAcceptResult::MempoolTx(entry.GetTxSize(), entry.GetFee())); } else if (m_pool.exists(GenTxid::Txid(txid))) { // Transaction with the same non-witness data but different witness (same txid, // different wtxid) already exists in the mempool. @@ -1517,10 +1514,9 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, // transaction for the mempool one. Note that we are ignoring the validity of the // package transaction passed in. // TODO: allow witness replacement in packages. - auto iter = m_pool.GetIter(txid); - assert(iter != std::nullopt); + const auto& entry{*Assert(m_pool.GetEntry(txid))}; // Provide the wtxid of the mempool tx so that the caller can look it up in the mempool. - results_final.emplace(wtxid, MempoolAcceptResult::MempoolTxDifferentWitness(iter.value()->GetTx().GetWitnessHash())); + results_final.emplace(wtxid, MempoolAcceptResult::MempoolTxDifferentWitness(entry.GetTx().GetWitnessHash())); } else { // Transaction does not already exist in the mempool. // Try submitting the transaction on its own.