merge bitcoin#23649: circular dependency followups

kwvg · kwvg · commit e85862ba1107 · 2024-10-05T17:10:03.000Z
diff --git 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>
@@ -81,16 +82,15 @@ struct update_lock_points
     const LockPoints& lp;
 };
 
-bool TestLockPointValidity(CChain& active_chain, const LockPoints* lp)
+bool TestLockPointValidity(CChain& active_chain, const LockPoints& lp)
 {
     AssertLockHeld(cs_main);
-    assert(lp);
     // If there are relative lock times then the maxInputBlock will be set
     // If there are no relative lock times, the LockPoints don't depend on the chain
-    if (lp->maxInputBlock) {
+    if (lp.maxInputBlock) {
         // Check whether active_chain is an extension of the block at which the LockPoints
         // calculation was valid.  If not LockPoints are no longer valid
-        if (!active_chain.Contains(lp->maxInputBlock)) {
+        if (!active_chain.Contains(lp.maxInputBlock)) {
             return false;
         }
     }
@@ -892,8 +892,8 @@ void CTxMemPool::removeForReorg(CChain& chain, std::function<bool(txiter)> check
     }
     RemoveStaged(setAllRemoves, false, MemPoolRemovalReason::REORG);
     for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) {
-        LockPoints lp = it->GetLockPoints();
-        if (!TestLockPointValidity(chain, &lp)) {
+        const LockPoints lp{it->GetLockPoints()};
+        if (!TestLockPointValidity(chain, lp)) {
             mapTx.modify(it, update_lock_points(lp));
         }
     }
diff --git a/src/txmempool.h b/src/txmempool.h
@@ -17,7 +17,6 @@
 #include <addressindex.h>
 #include <spentindex.h>
 #include <amount.h>
-#include <chain.h>
 #include <coins.h>
 #include <gsl/pointers.h>
 #include <indirectmap.h>
@@ -31,12 +30,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;
 
@@ -63,7 +63,7 @@ struct LockPoints {
 /**
  * Test whether the LockPoints height and time are still valid on the current chain
  */
-bool TestLockPointValidity(CChain& active_chain, const LockPoints* lp) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+bool TestLockPointValidity(CChain& active_chain, const LockPoints& lp) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
 struct CompareIteratorByHash {
     // SFINAE for T where T is either a pointer type (e.g., a txiter) or a reference_wrapper<T>
diff --git a/src/validation.cpp b/src/validation.cpp
@@ -378,7 +378,7 @@ void CChainState::MaybeUpdateMempoolForReorg(
         AssertLockHeld(::cs_main);
         const CTransaction& tx = it->GetTx();
         LockPoints lp = it->GetLockPoints();
-        bool validLP = TestLockPointValidity(m_chain, &lp);
+        const bool validLP{TestLockPointValidity(m_chain, lp)};
         CCoinsViewMemPool view_mempool(&CoinsTip(), *m_mempool);
         if (!CheckFinalTx(m_chain.Tip(), tx, flags)
             || !CheckSequenceLocks(m_chain.Tip(), view_mempool, tx, flags, &lp, validLP)) {
@@ -392,8 +392,8 @@ void CChainState::MaybeUpdateMempoolForReorg(
                     continue;
                 const Coin &coin = CoinsTip().AccessCoin(txin.prevout);
                 assert(!coin.IsSpent());
-                unsigned int nMemPoolHeight = m_chain.Tip()->nHeight + 1;
-                if (coin.IsSpent() || (coin.IsCoinBase() && ((signed long)nMemPoolHeight) - coin.nHeight < COINBASE_MATURITY)) {
+                const auto mempool_spend_height{m_chain.Tip()->nHeight + 1};
+                if (coin.IsSpent() || (coin.IsCoinBase() && mempool_spend_height - coin.nHeight < COINBASE_MATURITY)) {
                     should_remove = true;
                     break;
                 }

Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,7 @@`
`5`	`5`
`6`	`6`	`#include <txmempool.h>`
`7`	`7`
	`8`	`+#include <chain.h>`
`8`	`9`	`#include <coins.h>`
`9`	`10`	`#include <consensus/consensus.h>`
`10`	`11`	`#include <consensus/tx_verify.h>`
`@@ -81,16 +82,15 @@ struct update_lock_points`
`81`	`82`	`const LockPoints& lp;`
`82`	`83`	`};`
`83`	`84`
`84`		`-bool TestLockPointValidity(CChain& active_chain, const LockPoints* lp)`
	`85`	`+bool TestLockPointValidity(CChain& active_chain, const LockPoints& lp)`
`85`	`86`	`{`
`86`	`87`	`AssertLockHeld(cs_main);`
`87`		`- assert(lp);`
`88`	`88`	`// If there are relative lock times then the maxInputBlock will be set`
`89`	`89`	`// If there are no relative lock times, the LockPoints don't depend on the chain`
`90`		`- if (lp->maxInputBlock) {`
	`90`	`+ if (lp.maxInputBlock) {`
`91`	`91`	`// Check whether active_chain is an extension of the block at which the LockPoints`
`92`	`92`	`// calculation was valid. If not LockPoints are no longer valid`
`93`		`- if (!active_chain.Contains(lp->maxInputBlock)) {`
	`93`	`+ if (!active_chain.Contains(lp.maxInputBlock)) {`
`94`	`94`	`return false;`
`95`	`95`	`}`
`96`	`96`	`}`
`@@ -892,8 +892,8 @@ void CTxMemPool::removeForReorg(CChain& chain, std::function<bool(txiter)> check`
`892`	`892`	`}`
`893`	`893`	`RemoveStaged(setAllRemoves, false, MemPoolRemovalReason::REORG);`
`894`	`894`	`for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) {`
`895`		`- LockPoints lp = it->GetLockPoints();`
`896`		`- if (!TestLockPointValidity(chain, &lp)) {`
	`895`	`+ const LockPoints lp{it->GetLockPoints()};`
	`896`	`+ if (!TestLockPointValidity(chain, lp)) {`
`897`	`897`	`mapTx.modify(it, update_lock_points(lp));`
`898`	`898`	`}`
`899`	`899`	`}`