change TestLockPointValidity to take a const reference

glozow · glozow · commit c4efc4db5484 · 2021-12-02T14:45:18.000Z
The lockpoints are not changed in this function.
There is no reason to pass a pointer.
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
@@ -73,16 +73,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;
         }
     }
@@ -649,8 +648,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
@@ -53,7 +53,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
@@ -357,7 +357,7 @@ void CChainState::MaybeUpdateMempoolForReorg(
         AssertLockHeld(::cs_main);
         const CTransaction& tx = it->GetTx();
         LockPoints lp = it->GetLockPoints();
-        const 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)) {

Original file line number	Diff line number	Diff line change
`@@ -73,16 +73,15 @@ struct update_lock_points`
`73`	`73`	`const LockPoints& lp;`
`74`	`74`	`};`
`75`	`75`
`76`		`-bool TestLockPointValidity(CChain& active_chain, const LockPoints* lp)`
	`76`	`+bool TestLockPointValidity(CChain& active_chain, const LockPoints& lp)`
`77`	`77`	`{`
`78`	`78`	`AssertLockHeld(cs_main);`
`79`		`- assert(lp);`
`80`	`79`	`// If there are relative lock times then the maxInputBlock will be set`
`81`	`80`	`// If there are no relative lock times, the LockPoints don't depend on the chain`
`82`		`- if (lp->maxInputBlock) {`
	`81`	`+ if (lp.maxInputBlock) {`
`83`	`82`	`// Check whether active_chain is an extension of the block at which the LockPoints`
`84`	`83`	`// calculation was valid. If not LockPoints are no longer valid`
`85`		`- if (!active_chain.Contains(lp->maxInputBlock)) {`
	`84`	`+ if (!active_chain.Contains(lp.maxInputBlock)) {`
`86`	`85`	`return false;`
`87`	`86`	`}`
`88`	`87`	`}`
`@@ -649,8 +648,8 @@ void CTxMemPool::removeForReorg(CChain& chain, std::function<bool(txiter)> check`
`649`	`648`	`}`
`650`	`649`	`RemoveStaged(setAllRemoves, false, MemPoolRemovalReason::REORG);`
`651`	`650`	`for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) {`
`652`		`- LockPoints lp = it->GetLockPoints();`
`653`		`- if (!TestLockPointValidity(chain, &lp)) {`
	`651`	`+ const LockPoints lp{it->GetLockPoints()};`
	`652`	`+ if (!TestLockPointValidity(chain, lp)) {`
`654`	`653`	`mapTx.modify(it, update_lock_points(lp));`
`655`	`654`	`}`
`656`	`655`	`}`