circular dependency followups #23649

pull glozow wants to merge 3 commits into bitcoin:master from glozow:22677-followups changing 3 files +12 −12

glozow commented at 12:05 PM on December 2, 2021: member

Followups from #22677 + clean up TestLockPointValidity
remove unnecessary casts and use braced initialization b01784f027
glozow commented at 12:06 PM on December 2, 2021: member

CC @MarcoFalke, comments from #22677#pullrequestreview-820587394
glozow cross-referenced this on Dec 2, 2021 from issue cut the validation <-> txmempool circular dependency 2/2 by glozow
glozow added the label Refactoring on Dec 2, 2021

in src/txmempool.cpp:651 in c1ba9e8dd8 outdated

 648 | @@ -650,7 +649,7 @@ void CTxMemPool::removeForReorg(CChain& chain, std::function<bool(txiter)> check
 649 |      RemoveStaged(setAllRemoves, false, MemPoolRemovalReason::REORG);
 650 |      for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) {
 651 |          LockPoints lp = it->GetLockPoints();

MarcoFalke commented at 12:40 PM on December 2, 2021:

        const LockPoints lp{it->GetLockPoints()};

MarcoFalke approved
MarcoFalke commented at 12:40 PM on December 2, 2021: member

Nice. ACK

Left one nit in case you retouch

MarcoFalke commented at 2:34 PM on December 2, 2021: member

Thoughts on this suggested diff?

diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 27fbb8acac..2cc0aa1e90 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -5,6 +5,7 @@
 
 #include <txmempool.h>
 
+#include <chain.h>
 #include <coins.h>
 #include <consensus/consensus.h>
 #include <consensus/tx_verify.h>
diff --git a/src/txmempool.h b/src/txmempool.h
index c6e08a3ca5..15a57de922 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -14,7 +14,6 @@
 #include <utility>
 #include <vector>
 
-#include <chain.h>
 #include <coins.h>
 #include <consensus/amount.h>
 #include <indirectmap.h>
@@ -26,12 +25,13 @@
 #include <util/epochguard.h>
 #include <util/hasher.h>
 
-#include <boost/multi_index_container.hpp>
 #include <boost/multi_index/hashed_index.hpp>
 #include <boost/multi_index/ordered_index.hpp>
 #include <boost/multi_index/sequenced_index.hpp>
+#include <boost/multi_index_container.hpp>
 
 class CBlockIndex;
+class CChain;
 class CChainState;
 extern RecursiveMutex cs_main;

change TestLockPointValidity to take a const reference

The lockpoints are not changed in this function.
There is no reason to pass a pointer.

c4efc4db54

glozow force-pushed on Dec 2, 2021
clean up txmempool includes ddd74ff65c
MarcoFalke approved
MarcoFalke commented at 2:52 PM on December 2, 2021: member

re-ack
glozow force-pushed on Dec 2, 2021
glozow commented at 2:55 PM on December 2, 2021: member

Oops forgot the txmempool.cpp include. Added suggestions, thanks @MarcoFalke
theStack approved
theStack commented at 3:16 PM on December 2, 2021: contributor

Code-review ACK ddd74ff65c471c5c25815ac0643d16ea0d51f58f
fanquake merged this on Dec 3, 2021
fanquake closed this on Dec 3, 2021
glozow deleted the branch on Dec 3, 2021
sidhujag referenced this in commit 0898a382b7 on Dec 3, 2021
RandyMcMillan referenced this in commit d28cf54723 on Dec 23, 2021
Fabcien referenced this in commit bd99a74d3e on Nov 10, 2022
Fabcien referenced this in commit 848f2f48c2 on Nov 10, 2022
Fabcien referenced this in commit 3b8d42aff6 on Nov 10, 2022
bitcoin locked this on Dec 3, 2022

Contributors

Labels

Linked (view graph)

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