Skip to content

Commit 1c6a73a

Browse files
seditedsedited
committed
[refactor] Add helper for retrieving mempool entry
In places where the iterator is only needed for accessing the actual entry, it should not be required to first retrieve the iterator.
1 parent 453b481 commit 1c6a73a

File tree

5 files changed

+26
-18
lines changed

5 files changed

+26
-18
lines changed

‎src/node/interfaces.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -648,8 +648,9 @@ class ChainImpl : public Chain
648648
{
649649
if (!m_node.mempool) return false;
650650
LOCK(m_node.mempool->cs);
651-
auto it = m_node.mempool->GetIter(txid);
652-
return it && (*it)->GetCountWithDescendants() > 1;
651+
const auto entry{m_node.mempool->GetEntry(Txid::FromUint256(txid))};
652+
if (entry == nullptr) return false;
653+
return entry->GetCountWithDescendants() > 1;
653654
}
654655
bool broadcastTransaction(const CTransactionRef& tx,
655656
const CAmount& max_tx_fee,

‎src/test/miniminer_tests.cpp‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ BOOST_FIXTURE_TEST_CASE(miniminer_negative, TestChain100Setup)
9494
const CFeeRate feerate_zero(0);
9595
mini_miner_target0.BuildMockTemplate(feerate_zero);
9696
// Check the quit condition:
97-
BOOST_CHECK(negative_modified_fees < feerate_zero.GetFee(pool.GetIter(tx_mod_negative->GetHash()).value()->GetTxSize()));
97+
BOOST_CHECK(negative_modified_fees < feerate_zero.GetFee(Assert(pool.GetEntry(tx_mod_negative->GetHash()))->GetTxSize()));
9898
BOOST_CHECK(mini_miner_target0.GetMockTemplateTxids().empty());
9999

100100
// With no target feerate, the template includes all transactions, even negative feerate ones.
@@ -179,9 +179,9 @@ BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup)
179179
};
180180
std::map<uint256, TxDimensions> tx_dims;
181181
for (const auto& tx : all_transactions) {
182-
const auto it = pool.GetIter(tx->GetHash()).value();
183-
tx_dims.emplace(tx->GetHash(), TxDimensions{it->GetTxSize(), it->GetModifiedFee(),
184-
CFeeRate(it->GetModifiedFee(), it->GetTxSize())});
182+
const auto& entry{*Assert(pool.GetEntry(tx->GetHash()))};
183+
tx_dims.emplace(tx->GetHash(), TxDimensions{entry.GetTxSize(), entry.GetModifiedFee(),
184+
CFeeRate(entry.GetModifiedFee(), entry.GetTxSize())});
185185
}
186186

187187
const std::vector<CFeeRate> various_normal_feerates({CFeeRate(0), CFeeRate(500), CFeeRate(999),
@@ -447,15 +447,15 @@ BOOST_FIXTURE_TEST_CASE(miniminer_overlap, TestChain100Setup)
447447
// tx3's feerate is lower than tx2's. same fee, different weight.
448448
BOOST_CHECK(tx2_feerate > tx3_feerate);
449449
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]);
450-
const auto tx3_iter = pool.GetIter(tx3->GetHash());
451-
BOOST_CHECK(tx3_anc_feerate == CFeeRate(tx3_iter.value()->GetModFeesWithAncestors(), tx3_iter.value()->GetSizeWithAncestors()));
450+
const auto& tx3_entry{*Assert(pool.GetEntry(tx3->GetHash()))};
451+
BOOST_CHECK(tx3_anc_feerate == CFeeRate(tx3_entry.GetModFeesWithAncestors(), tx3_entry.GetSizeWithAncestors()));
452452
const auto tx4_feerate = CFeeRate(high_fee, tx_vsizes[4]);
453453
const auto tx6_anc_feerate = CFeeRate(high_fee + low_fee + med_fee, tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[6]);
454-
const auto tx6_iter = pool.GetIter(tx6->GetHash());
455-
BOOST_CHECK(tx6_anc_feerate == CFeeRate(tx6_iter.value()->GetModFeesWithAncestors(), tx6_iter.value()->GetSizeWithAncestors()));
454+
const auto& tx6_entry{*Assert(pool.GetEntry(tx6->GetHash()))};
455+
BOOST_CHECK(tx6_anc_feerate == CFeeRate(tx6_entry.GetModFeesWithAncestors(), tx6_entry.GetSizeWithAncestors()));
456456
const auto tx7_anc_feerate = CFeeRate(high_fee + low_fee + high_fee, tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[7]);
457-
const auto tx7_iter = pool.GetIter(tx7->GetHash());
458-
BOOST_CHECK(tx7_anc_feerate == CFeeRate(tx7_iter.value()->GetModFeesWithAncestors(), tx7_iter.value()->GetSizeWithAncestors()));
457+
const auto& tx7_entry{*Assert(pool.GetEntry(tx7->GetHash()))};
458+
BOOST_CHECK(tx7_anc_feerate == CFeeRate(tx7_entry.GetModFeesWithAncestors(), tx7_entry.GetSizeWithAncestors()));
459459
BOOST_CHECK(tx4_feerate > tx6_anc_feerate);
460460
BOOST_CHECK(tx4_feerate > tx7_anc_feerate);
461461

‎src/txmempool.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -862,6 +862,13 @@ std::vector<TxMempoolInfo> CTxMemPool::infoAll() const
862862
return ret;
863863
}
864864

865+
const CTxMemPoolEntry* CTxMemPool::GetEntry(const Txid& txid) const
866+
{
867+
AssertLockHeld(cs);
868+
const auto i = mapTx.find(txid);
869+
return i == mapTx.end() ? nullptr : &(*i);
870+
}
871+
865872
CTransactionRef CTxMemPool::get(const uint256& hash) const
866873
{
867874
LOCK(cs);

‎src/txmempool.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,8 @@ class CTxMemPool
684684
return (mapTx.count(gtxid.GetHash()) != 0);
685685
}
686686

687+
const CTxMemPoolEntry* GetEntry(const Txid& txid) const LIFETIMEBOUND EXCLUSIVE_LOCKS_REQUIRED(cs);
688+
687689
CTransactionRef get(const uint256& hash) const;
688690
txiter get_iter_from_wtxid(const uint256& wtxid) const EXCLUSIVE_LOCKS_REQUIRED(cs)
689691
{

‎src/validation.cpp‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1485,9 +1485,8 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
14851485
// transactions that are already in the mempool, and only call AcceptMultipleTransactions() with
14861486
// the new transactions. This ensures we don't double-count transaction counts and sizes when
14871487
// checking ancestor/descendant limits, or double-count transaction fees for fee-related policy.
1488-
auto iter = m_pool.GetIter(txid);
1489-
assert(iter != std::nullopt);
1490-
results_final.emplace(wtxid, MempoolAcceptResult::MempoolTx(iter.value()->GetTxSize(), iter.value()->GetFee()));
1488+
const auto& entry{*Assert(m_pool.GetEntry(txid))};
1489+
results_final.emplace(wtxid, MempoolAcceptResult::MempoolTx(entry.GetTxSize(), entry.GetFee()));
14911490
} else if (m_pool.exists(GenTxid::Txid(txid))) {
14921491
// Transaction with the same non-witness data but different witness (same txid,
14931492
// different wtxid) already exists in the mempool.
@@ -1496,10 +1495,9 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
14961495
// transaction for the mempool one. Note that we are ignoring the validity of the
14971496
// package transaction passed in.
14981497
// TODO: allow witness replacement in packages.
1499-
auto iter = m_pool.GetIter(txid);
1500-
assert(iter != std::nullopt);
1498+
const auto& entry{*Assert(m_pool.GetEntry(txid))};
15011499
// Provide the wtxid of the mempool tx so that the caller can look it up in the mempool.
1502-
results_final.emplace(wtxid, MempoolAcceptResult::MempoolTxDifferentWitness(iter.value()->GetTx().GetWitnessHash()));
1500+
results_final.emplace(wtxid, MempoolAcceptResult::MempoolTxDifferentWitness(entry.GetTx().GetWitnessHash()));
15031501
} else {
15041502
// Transaction does not already exist in the mempool.
15051503
// Try submitting the transaction on its own.

0 commit comments

Comments
 (0)