Skip to content

Commit 705e3f1

Browse files
TheCharlatanajtowns
andcommitted
refactor: Make CTxMemPoolEntry only explicitly copyable
This has the goal of prohibiting users from accidentally creating runtime failures, e.g. by interacting with iterator_to with a copied entry. CTxMemPoolEntry is already implicitly not move-constructable. So be explicit about this and use a std::list to collect the values in the policy_estimator fuzz test instead of a std::vector. Co-authored-by: Anthony Towns <aj@erisian.com.au>
1 parent 22025d0 commit 705e3f1

File tree

3 files changed

+15
-3
lines changed

3 files changed

+15
-3
lines changed

src/kernel/mempool_entry.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,11 @@ class CTxMemPoolEntry
7171
typedef std::set<CTxMemPoolEntryRef, CompareIteratorByHash> Children;
7272

7373
private:
74+
CTxMemPoolEntry(const CTxMemPoolEntry&) = default;
75+
struct ExplicitCopyTag {
76+
explicit ExplicitCopyTag() = default;
77+
};
78+
7479
const CTransactionRef tx;
7580
mutable Parents m_parents;
7681
mutable Children m_children;
@@ -122,6 +127,13 @@ class CTxMemPoolEntry
122127
nModFeesWithAncestors{nFee},
123128
nSigOpCostWithAncestors{sigOpCost} {}
124129

130+
CTxMemPoolEntry(ExplicitCopyTag, const CTxMemPoolEntry& entry) : CTxMemPoolEntry(entry) {}
131+
CTxMemPoolEntry& operator=(const CTxMemPoolEntry&) = delete;
132+
CTxMemPoolEntry(CTxMemPoolEntry&&) = delete;
133+
CTxMemPoolEntry& operator=(CTxMemPoolEntry&&) = delete;
134+
135+
static constexpr ExplicitCopyTag ExplicitCopy{};
136+
125137
const CTransaction& GetTx() const { return *this->tx; }
126138
CTransactionRef GetSharedTx() const { return this->tx; }
127139
const CAmount& GetFee() const { return nFee; }

src/test/fuzz/policy_estimator.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator)
4949
}
5050
},
5151
[&] {
52-
std::vector<CTxMemPoolEntry> mempool_entries;
52+
std::list<CTxMemPoolEntry> mempool_entries;
5353
LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000)
5454
{
5555
const std::optional<CMutableTransaction> mtx = ConsumeDeserializable<CMutableTransaction>(fuzzed_data_provider, TX_WITH_WITNESS);
@@ -58,7 +58,7 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator)
5858
break;
5959
}
6060
const CTransaction tx{*mtx};
61-
mempool_entries.push_back(ConsumeTxMemPoolEntry(fuzzed_data_provider, tx));
61+
mempool_entries.emplace_back(CTxMemPoolEntry::ExplicitCopy, ConsumeTxMemPoolEntry(fuzzed_data_provider, tx));
6262
}
6363
std::vector<const CTxMemPoolEntry*> ptrs;
6464
ptrs.reserve(mempool_entries.size());

src/txmempool.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,7 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces
438438
// Add to memory pool without checking anything.
439439
// Used by AcceptToMemoryPool(), which DOES do
440440
// all the appropriate checks.
441-
indexed_transaction_set::iterator newit = mapTx.insert(entry).first;
441+
indexed_transaction_set::iterator newit = mapTx.emplace(CTxMemPoolEntry::ExplicitCopy, entry).first;
442442

443443
// Update transaction for any feeDelta created by PrioritiseTransaction
444444
CAmount delta{0};

0 commit comments

Comments
 (0)