refactor: move `update_*` structs from txmempool.h to .cpp file #23211

pull theStack wants to merge 2 commits into bitcoin:master from theStack:202110-refactor-various_mempool_cleanups changing 2 files +53 −53

theStack commented at 1:50 AM on October 7, 2021: contributor

These helpers are exclusively used in txmempool.cpp, hence they should also be moved there. The PR also contains a commit which fixes const-correctness for parents in CTxMemPool::UpdateAncestorsOf and declares them as reference to avoid a copy.
refactor: use const reference for parents in `CTxMemPool::UpdateAncestorsOf` 9947ce6262

refactor: move `update_*` structs from txmempool.h to .cpp file

These helpers are exclusively used in txmempool.cpp, hence they
should also be moved there.

Can be reviewed with "--color-moved=dimmed-zebra".

65aaf9495d

fanquake added the label Refactoring on Oct 7, 2021
practicalswift commented at 7:47 AM on October 7, 2021: contributor

Concept ACK
DrahtBot commented at 5:51 PM on October 7, 2021: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #23418 (Fix signed integer overflow in prioritisetransaction RPC by MarcoFalke)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
DrahtBot cross-referenced this on Oct 8, 2021 from issue refactor: Nuke policy/fees->mempool circular dependencies by hebasto

in src/txmempool.cpp:281 in 9947ce6262 outdated

 275 | @@ -276,7 +276,7 @@ bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry,
 276 |  
 277 |  void CTxMemPool::UpdateAncestorsOf(bool add, txiter it, setEntries &setAncestors)
 278 |  {
 279 | -    CTxMemPoolEntry::Parents parents = it->GetMemPoolParents();
 280 | +    const CTxMemPoolEntry::Parents& parents = it->GetMemPoolParentsConst();
 281 |      // add or remove this tx as a child of each parent
 282 |      for (const CTxMemPoolEntry& parent : parents) {

promag commented at 7:53 PM on October 15, 2021:

9947ce62626c05bd186ae8a4864aa382f673ec1a

Note that parent can be non-const. Maybe GetMemPoolParentsConst should return std::set<const CTxMemPoolEntryRef, CompareIteratorByHash> instead.

promag commented at 8:03 PM on October 15, 2021: member

Code review ACK 65aaf9495d19ea3fb875228a7e14aab6c1f2986d. Verified move-only commit locally.
DrahtBot cross-referenced this on Nov 3, 2021 from issue Fix signed integer overflow in prioritisetransaction RPC by MarcoFalke
MarcoFalke merged this on Nov 3, 2021
MarcoFalke closed this on Nov 3, 2021
theStack deleted the branch on Nov 3, 2021
sidhujag referenced this in commit 83ef64b35f on Nov 3, 2021
glozow cross-referenced this on Nov 15, 2021 from issue cut the validation <-> txmempool circular dependency 2/2 by glozow
Fabcien referenced this in commit 8b97d1b5d5 on Oct 19, 2022
bitcoin locked this on Nov 3, 2022

Contributors

Labels

Linked (view graph)

#22677 cut the validation <-> txmempool circular dependency 2/2

refactor: move `update_*` structs from txmempool.h to .cpp file #23211

Conflicts