Skip to content

Commit aeb0f78

Browse files
marcofleonmarcofleon
committed
refactor: Convert mini_miner from uint256 to Txid
1 parent 326f244 commit aeb0f78

File tree

5 files changed

+20
-31
lines changed

5 files changed

+20
-31
lines changed

‎src/node/mini_miner.cpp‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#include <algorithm>
1818
#include <numeric>
19+
#include <ranges>
1920
#include <utility>
2021

2122
namespace node {
@@ -61,12 +62,8 @@ MiniMiner::MiniMiner(const CTxMemPool& mempool, const std::vector<COutPoint>& ou
6162
if (m_requested_outpoints_by_txid.empty()) return;
6263

6364
// Calculate the cluster and construct the entry map.
64-
std::vector<uint256> txids_needed;
65-
txids_needed.reserve(m_requested_outpoints_by_txid.size());
66-
for (const auto& [txid, _]: m_requested_outpoints_by_txid) {
67-
txids_needed.push_back(txid);
68-
}
69-
const auto cluster = mempool.GatherClusters(txids_needed);
65+
auto txids_needed{m_requested_outpoints_by_txid | std::views::keys};
66+
const auto cluster = mempool.GatherClusters({txids_needed.begin(), txids_needed.end()});
7067
if (cluster.empty()) {
7168
// An empty cluster means that at least one of the transactions is missing from the mempool
7269
// (should not be possible given processing above) or DoS limit was hit.
@@ -286,7 +283,7 @@ void MiniMiner::BuildMockTemplate(std::optional<CFeeRate> target_feerate)
286283
}
287284
// Track the order in which transactions were selected.
288285
for (const auto& ancestor : ancestors) {
289-
m_inclusion_order.emplace(Txid::FromUint256(ancestor->first), sequence_num);
286+
m_inclusion_order.emplace(ancestor->first, sequence_num);
290287
}
291288
DeleteAncestorPackage(ancestors);
292289
SanityCheck();
@@ -409,7 +406,7 @@ std::optional<CAmount> MiniMiner::CalculateTotalBumpFees(const CFeeRate& target_
409406
ancestors.insert(iter);
410407
}
411408

412-
std::set<uint256> has_been_processed;
409+
std::set<Txid> has_been_processed;
413410
while (!to_process.empty()) {
414411
auto iter = to_process.begin();
415412
const CTransaction& tx = (*iter)->second.GetTx();

‎src/node/mini_miner.h‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,12 @@ class MiniMiner
8383

8484
// Set once per lifetime, fill in during initialization.
8585
// txids of to-be-replaced transactions
86-
std::set<uint256> m_to_be_replaced;
86+
std::set<Txid> m_to_be_replaced;
8787

8888
// If multiple argument outpoints correspond to the same transaction, cache them together in
8989
// a single entry indexed by txid. Then we can just work with txids since all outpoints from
9090
// the same tx will have the same bumpfee. Excludes non-mempool transactions.
91-
std::map<uint256, std::vector<COutPoint>> m_requested_outpoints_by_txid;
91+
std::map<Txid, std::vector<COutPoint>> m_requested_outpoints_by_txid;
9292

9393
// Txid to a number representing the order in which this transaction was included (smaller
9494
// number = included earlier). Transactions included in an ancestor set together have the same
@@ -98,21 +98,21 @@ class MiniMiner
9898
std::map<COutPoint, CAmount> m_bump_fees;
9999

100100
// The constructed block template
101-
std::set<uint256> m_in_block;
101+
std::set<Txid> m_in_block;
102102

103103
// Information on the current status of the block
104104
CAmount m_total_fees{0};
105105
int32_t m_total_vsize{0};
106106

107107
/** Main data structure holding the entries, can be indexed by txid */
108-
std::map<uint256, MiniMinerMempoolEntry> m_entries_by_txid;
108+
std::map<Txid, MiniMinerMempoolEntry> m_entries_by_txid;
109109
using MockEntryMap = decltype(m_entries_by_txid);
110110

111111
/** Vector of entries, can be sorted by ancestor feerate. */
112112
std::vector<MockEntryMap::iterator> m_entries;
113113

114114
/** Map of txid to its descendants. Should be inclusive. */
115-
std::map<uint256, std::vector<MockEntryMap::iterator>> m_descendant_set_by_txid;
115+
std::map<Txid, std::vector<MockEntryMap::iterator>> m_descendant_set_by_txid;
116116

117117
/** Consider this ancestor package "mined" so remove all these entries from our data structures. */
118118
void DeleteAncestorPackage(const std::set<MockEntryMap::iterator, IteratorComparator>& ancestors);
@@ -129,7 +129,7 @@ class MiniMiner
129129
void BuildMockTemplate(std::optional<CFeeRate> target_feerate);
130130

131131
/** Returns set of txids in the block template if one has been constructed. */
132-
std::set<uint256> GetMockTemplateTxids() const { return m_in_block; }
132+
std::set<Txid> GetMockTemplateTxids() const { return m_in_block; }
133133

134134
/** Constructor that takes a list of outpoints that may or may not belong to transactions in the
135135
* mempool. Copies out information about the relevant transactions in the mempool into

‎src/test/miniminer_tests.cpp‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ BOOST_FIXTURE_TEST_CASE(miniminer_negative, TestChain100Setup)
103103
mini_miner_no_target.BuildMockTemplate(std::nullopt);
104104
const auto template_txids{mini_miner_no_target.GetMockTemplateTxids()};
105105
BOOST_CHECK_EQUAL(template_txids.size(), 1);
106-
BOOST_CHECK(template_txids.count(tx_mod_negative->GetHash().ToUint256()) > 0);
106+
BOOST_CHECK(template_txids.count(tx_mod_negative->GetHash()) > 0);
107107
}
108108

109109
BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup)
@@ -177,7 +177,7 @@ BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup)
177177
struct TxDimensions {
178178
int32_t vsize; CAmount mod_fee; CFeeRate feerate;
179179
};
180-
std::map<uint256, TxDimensions> tx_dims;
180+
std::map<Txid, TxDimensions> tx_dims;
181181
for (const auto& tx : all_transactions) {
182182
const auto& entry{*Assert(pool.GetEntry(tx->GetHash()))};
183183
tx_dims.emplace(tx->GetHash(), TxDimensions{entry.GetTxSize(), entry.GetModifiedFee(),
@@ -590,14 +590,6 @@ BOOST_FIXTURE_TEST_CASE(calculate_cluster, TestChain100Setup)
590590
CTxMemPool& pool = *Assert(m_node.mempool);
591591
LOCK2(cs_main, pool.cs);
592592

593-
// TODO this can be removed once the mempool interface uses Txid, Wtxid
594-
auto convert_to_uint256_vec = [](const std::vector<Txid>& vec) -> std::vector<uint256> {
595-
std::vector<uint256> out;
596-
std::transform(vec.begin(), vec.end(), std::back_inserter(out),
597-
[](const Txid& txid) { return txid.ToUint256(); });
598-
return out;
599-
};
600-
601593
// Add chain of size 500
602594
TestMemPoolEntryHelper entry;
603595
std::vector<Txid> chain_txids;
@@ -611,7 +603,7 @@ BOOST_FIXTURE_TEST_CASE(calculate_cluster, TestChain100Setup)
611603
const auto cluster_500tx = pool.GatherClusters({lasttx->GetHash()});
612604
CTxMemPool::setEntries cluster_500tx_set{cluster_500tx.begin(), cluster_500tx.end()};
613605
BOOST_CHECK_EQUAL(cluster_500tx.size(), cluster_500tx_set.size());
614-
const auto vec_iters_500 = pool.GetIterVec(convert_to_uint256_vec(chain_txids));
606+
const auto vec_iters_500 = pool.GetIterVec(chain_txids);
615607
for (const auto& iter : vec_iters_500) BOOST_CHECK(cluster_500tx_set.count(iter));
616608

617609
// GatherClusters stops at 500 transactions.
@@ -637,7 +629,7 @@ BOOST_FIXTURE_TEST_CASE(calculate_cluster, TestChain100Setup)
637629
AddToMempool(pool, entry.Fee(CENT).FromTx(txc));
638630
zigzag_txids.push_back(txc->GetHash());
639631
}
640-
const auto vec_iters_zigzag = pool.GetIterVec(convert_to_uint256_vec(zigzag_txids));
632+
const auto vec_iters_zigzag = pool.GetIterVec(zigzag_txids);
641633
// It doesn't matter which tx we calculate cluster for, everybody is in it.
642634
const std::vector<size_t> indices{0, 22, 72, zigzag_txids.size() - 1};
643635
for (const auto index : indices) {

‎src/txmempool.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -983,13 +983,13 @@ CTxMemPool::setEntries CTxMemPool::GetIterSet(const std::set<Txid>& hashes) cons
983983
return ret;
984984
}
985985

986-
std::vector<CTxMemPool::txiter> CTxMemPool::GetIterVec(const std::vector<uint256>& txids) const
986+
std::vector<CTxMemPool::txiter> CTxMemPool::GetIterVec(const std::vector<Txid>& txids) const
987987
{
988988
AssertLockHeld(cs);
989989
std::vector<txiter> ret;
990990
ret.reserve(txids.size());
991991
for (const auto& txid : txids) {
992-
const auto it{GetIter(Txid::FromUint256(txid))};
992+
const auto it{GetIter(txid)};
993993
if (!it) return {};
994994
ret.push_back(*it);
995995
}
@@ -1227,7 +1227,7 @@ void CTxMemPool::SetLoadTried(bool load_tried)
12271227
m_load_tried = load_tried;
12281228
}
12291229

1230-
std::vector<CTxMemPool::txiter> CTxMemPool::GatherClusters(const std::vector<uint256>& txids) const
1230+
std::vector<CTxMemPool::txiter> CTxMemPool::GatherClusters(const std::vector<Txid>& txids) const
12311231
{
12321232
AssertLockHeld(cs);
12331233
std::vector<txiter> clustered_txs{GetIterVec(txids)};

‎src/txmempool.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -491,7 +491,7 @@ class CTxMemPool
491491
/** Translate a list of hashes into a list of mempool iterators to avoid repeated lookups.
492492
* The nth element in txids becomes the nth element in the returned vector. If any of the txids
493493
* don't actually exist in the mempool, returns an empty vector. */
494-
std::vector<txiter> GetIterVec(const std::vector<uint256>& txids) const EXCLUSIVE_LOCKS_REQUIRED(cs);
494+
std::vector<txiter> GetIterVec(const std::vector<Txid>& txids) const EXCLUSIVE_LOCKS_REQUIRED(cs);
495495

496496
/** UpdateTransactionsFromBlock is called when adding transactions from a
497497
* disconnected block back to the mempool, new mempool entries may have
@@ -548,7 +548,7 @@ class CTxMemPool
548548
* All txids must correspond to transaction entries in the mempool, otherwise this returns an
549549
* empty vector. This call will also exit early and return an empty vector if it collects 500 or
550550
* more transactions as a DoS protection. */
551-
std::vector<txiter> GatherClusters(const std::vector<uint256>& txids) const EXCLUSIVE_LOCKS_REQUIRED(cs);
551+
std::vector<txiter> GatherClusters(const std::vector<Txid>& txids) const EXCLUSIVE_LOCKS_REQUIRED(cs);
552552

553553
/** Calculate all in-mempool ancestors of a set of transactions not already in the mempool and
554554
* check ancestor and descendant limits. Heuristics are used to estimate the ancestor and

0 commit comments

Comments
 (0)