highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-01-20 23:29:12 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#28391: refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access

Browse Source 
4dd94ca18f [refactor] remove access to mapTx in validation_block_tests (TheCharlatan)
d0cd2e804e [refactor] rewrite BlockAssembler inBlock and failedTx as sets of txids (glozow)
55b0939cab scripted-diff: rename vTxHashes to txns_randomized (TheCharlatan)
a03aef9cec [refactor] rewrite vTxHashes as a vector of CTransactionRef (glozow)
938643c3b2 [refactor] remove access to mapTx in validation.cpp (glozow)
333367a940 [txmempool] make CTxMemPoolEntry::lockPoints mutable (glozow)
1bf4855016 [refactor] use CheckPackageLimits for checkChainLimits (glozow)
dbc5bdbf59 [refactor] remove access to mapTx.find in mempool_tests.cpp (glozow)
f80909e7a3 [refactor] remove access to mapTx in blockencodings_tests.cpp (glozow)
8892d6b744 [refactor] remove access to mapTx from rpc/mempool.cpp (glozow)
fad61aa561 [refactor] get wtxid from entry instead of vTxHashes (glozow)
9cd8cafb77 [refactor] use exists() instead of mapTx.find() (glozow)
14804699e5 [refactor] remove access to mapTx from policy/rbf.cpp (glozow)
1c6a73abbd [refactor] Add helper for retrieving mempool entry (TheCharlatan)
453b4813eb [refactor] Add helper for iterating through mempool entries (stickies-v)

Pull request description:

  Motivation
  * It seems preferable to use stdlib data structures instead of boost if they can achieve close to the same thing.
  * Code external to mempool should ideally use its public helper methods instead of accessing `mapTx` or its iterators directly.
  * Reduce the number of complex boost multi index type interactions
  * Also see #28335 for further context/motivation. This PR together with #28385 simplifies that one.

  Overview of things done in this PR:
  * Make `vTxHashes` a vector of transaction references instead of a pair of transaction hash and iterator. The trade off here is that the data is retrieved on the fly with `GetEntry` instead of being cached in `vTxHashes`.
  * Introduce `GetEntry` helper method to replace the more involved `GetIter` where applicable
  * Replace `mapTx` access with `CTxMemPool` helper methods
  * Simplify `checkChainLimits` call in `node/interfaces.cpp`
  * Make `CTxMemPoolEntry`s `lockPoints`mutable such that they can be changed with a const iterator directly instead of going through `mapTx`
  * Make `BlockAssembler`'s `inBlock` and `failedTx` sets of transaction hashes.

ACKs for top commit:
  glozow:
    reACK 4dd94ca
  maflcko:
    re-ACK 4dd94ca18f 👝
  stickies-v:
    re-ACK 4dd94ca18f

Tree-SHA512: c4d043f2186e4fde337591883fac66cade3058173987b49502bd65cecf69207a3df1077f6626809652ab63230013167b7f39a2b39f1c5166959e5495df57065f
This commit is contained in:
fanquake
2023-11-13 10:36:34 +00:00
parent e11b7587a1 4dd94ca18f
commit dd5f5713bc
14 changed files with 113 additions and 94 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
src/validation.cpp
View File

@@ -353,7 +353,7 @@ void Chainstate::MaybeUpdateMempoolForReorg(
const std::optional<LockPoints> 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.