phantom/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-01-25 10:43:19 -03:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Merge bitcoin/bitcoin#28903: refactor: Make CTxMemPoolEntry only explicitly copyable

Browse source 
705e3f1de0 refactor: Make CTxMemPoolEntry only explicitly copyable (TheCharlatan)

Pull request description:

  This has the goal of prohibiting users from accidentally creating runtime failures, e.g. by interacting with iterator_to with a copied entry. This was brought up here:  https://github.com/bitcoin/bitcoin/pull/28886#issuecomment-1814794954.

  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.

ACKs for top commit:
  maflcko:
    ACK 705e3f1de0 🌯
  achow101:
    ACK 705e3f1de0
  ajtowns:
    ACK 705e3f1de0
  ismaelsadeeq:
    ACK 705e3f1de0

Tree-SHA512: 62056905c679c919d00f9ae065ed66ac986e7e7062015aea542843d8deecda57104d7a68d002f7b20afa3164f8e9215d2d2d002c167224129540e3b1bd0712cc
This commit is contained in:
Andrew Chow 2023-11-28 14:38:31 -05:00
commit 535424a10b
No known key found for this signature in database
GPG key ID: 17565732E08E5E41
3 changed files with 15 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
src/kernel/mempool_entry.h
View file

@ -71,6 +71,11 @@ public:
typedef std::set<CTxMemPoolEntryRef, CompareIteratorByHash> Children;
private:
CTxMemPoolEntry(const CTxMemPoolEntry&) = default;
struct ExplicitCopyTag {
explicit ExplicitCopyTag() = default;
};
const CTransactionRef tx;
mutable Parents m_parents;
mutable Children m_children;
@ -122,6 +127,13 @@ public:
nModFeesWithAncestors{nFee},
nSigOpCostWithAncestors{sigOpCost} {}
CTxMemPoolEntry(ExplicitCopyTag, const CTxMemPoolEntry& entry) : CTxMemPoolEntry(entry) {}
CTxMemPoolEntry& operator=(const CTxMemPoolEntry&) = delete;
CTxMemPoolEntry(CTxMemPoolEntry&&) = delete;
CTxMemPoolEntry& operator=(CTxMemPoolEntry&&) = delete;
static constexpr ExplicitCopyTag ExplicitCopy{};
const CTransaction& GetTx() const { return *this->tx; }
CTransactionRef GetSharedTx() const { return this->tx; }
const CAmount& GetFee() const { return nFee; }

4
src/test/fuzz/policy_estimator.cpp
View file

@ -50,7 +50,7 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator)
}
},
[&] {
std::vector<CTxMemPoolEntry> mempool_entries;
std::list<CTxMemPoolEntry> mempool_entries;
LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000)
{
const std::optional<CMutableTransaction> mtx = ConsumeDeserializable<CMutableTransaction>(fuzzed_data_provider, TX_WITH_WITNESS);
@ -59,7 +59,7 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator)
break;
}
const CTransaction tx{*mtx};
mempool_entries.push_back(ConsumeTxMemPoolEntry(fuzzed_data_provider, tx));
mempool_entries.emplace_back(CTxMemPoolEntry::ExplicitCopy, ConsumeTxMemPoolEntry(fuzzed_data_provider, tx));
}
std::vector<const CTxMemPoolEntry*> ptrs;
ptrs.reserve(mempool_entries.size());

2
src/txmempool.cpp
View file

@ -438,7 +438,7 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces
// Add to memory pool without checking anything.
// Used by AcceptToMemoryPool(), which DOES do
// all the appropriate checks.
indexed_transaction_set::iterator newit = mapTx.insert(entry).first;
indexed_transaction_set::iterator newit = mapTx.emplace(CTxMemPoolEntry::ExplicitCopy, entry).first;
// Update transaction for any feeDelta created by PrioritiseTransaction
CAmount delta{0};