refactor: add CacheTipHeight, improve cache API safety

UdjinM6 · UdjinM6 · commit ffd84b6051cc · 2025-11-14T14:44:00.000+03:00
Add CacheTipHeight() helper to eliminate code duplication when updating
tip height cache. Refactor cache methods to accept CBlockIndex* instead
of hash/height pairs for type safety. Handle nullptr cases properly.
diff --git a/src/instantsend/instantsend.cpp b/src/instantsend/instantsend.cpp
@@ -136,7 +136,7 @@ MessageProcessingResult CInstantSendManager::ProcessMessage(NodeId from, std::st
             ret.m_error = MisbehavingError{1};
             return ret;
         }
-        CacheBlockHeight(blockIndex->GetBlockHash(), blockIndex->nHeight);
+        CacheBlockHeight(blockIndex);
         cycleHeightOpt = blockIndex->nHeight;
     }
 
@@ -395,7 +395,7 @@ MessageProcessingResult CInstantSendManager::ProcessInstantSendLock(NodeId from,
         if (!minedHeight.has_value()) {
             const CBlockIndex* pindexMined = WITH_LOCK(::cs_main, return m_chainstate.m_blockman.LookupBlockIndex(hashBlock));
             if (pindexMined != nullptr) {
-                CacheBlockHeight(pindexMined->GetBlockHash(), pindexMined->nHeight);
+                CacheBlockHeight(pindexMined);
                 minedHeight = pindexMined->nHeight;
             }
         }
@@ -508,11 +508,7 @@ void CInstantSendManager::BlockConnected(const std::shared_ptr<const CBlock>& pb
         return;
     }
 
-    {
-        LOCK(cs_height_cache);
-        CacheBlockHeightInternal(pindex->GetBlockHash(), pindex->nHeight);
-        m_cached_tip_height = pindex->nHeight;
-    }
+    CacheTipHeight(pindex);
 
     if (m_mn_sync.IsBlockchainSynced()) {
         const bool has_chainlock = clhandler.HasChainLock(pindex->nHeight, pindex->GetBlockHash());
@@ -544,13 +540,10 @@ void CInstantSendManager::BlockDisconnected(const std::shared_ptr<const CBlock>&
     {
         LOCK(cs_height_cache);
         m_cached_block_heights.erase(pindexDisconnected->GetBlockHash());
-        const CBlockIndex* const new_tip = pindexDisconnected->pprev;
-        m_cached_tip_height = new_tip ? new_tip->nHeight : -1;
-        if (new_tip) {
-            CacheBlockHeightInternal(new_tip->GetBlockHash(), new_tip->nHeight);
-        }
     }
 
+    CacheTipHeight(pindexDisconnected->pprev);
+
     db.RemoveBlockInstantSendLocks(pblock, pindexDisconnected);
 }
 
@@ -672,11 +665,7 @@ void CInstantSendManager::NotifyChainLock(const CBlockIndex* pindexChainLock)
 
 void CInstantSendManager::UpdatedBlockTip(const CBlockIndex* pindexNew)
 {
-    {
-        LOCK(cs_height_cache);
-        CacheBlockHeightInternal(pindexNew->GetBlockHash(), pindexNew->nHeight);
-        m_cached_tip_height = pindexNew->nHeight;
-    }
+    CacheTipHeight(pindexNew);
 
     bool fDIP0008Active = pindexNew->pprev && pindexNew->pprev->nHeight >= Params().GetConsensus().DIP0008Height;
 
@@ -964,16 +953,16 @@ size_t CInstantSendManager::GetInstantSendLockCount() const
     return db.GetInstantSendLockCount();
 }
 
-void CInstantSendManager::CacheBlockHeightInternal(const uint256& hash, int height) const
+void CInstantSendManager::CacheBlockHeightInternal(const CBlockIndex* const block_index) const
 {
     AssertLockHeld(cs_height_cache);
-    m_cached_block_heights.insert(hash, height);
+    m_cached_block_heights.insert(block_index->GetBlockHash(), block_index->nHeight);
 }
 
-void CInstantSendManager::CacheBlockHeight(const uint256& hash, int height) const
+void CInstantSendManager::CacheBlockHeight(const CBlockIndex* const block_index) const
 {
     LOCK(cs_height_cache);
-    CacheBlockHeightInternal(hash, height);
+    CacheBlockHeightInternal(block_index);
 }
 
 std::optional<int> CInstantSendManager::GetBlockHeight(const uint256& hash) const
@@ -992,10 +981,21 @@ std::optional<int> CInstantSendManager::GetBlockHeight(const uint256& hash) cons
         return std::nullopt;
     }
 
-    CacheBlockHeight(pindex->GetBlockHash(), pindex->nHeight);
+    CacheBlockHeight(pindex);
     return pindex->nHeight;
 }
 
+void CInstantSendManager::CacheTipHeight(const CBlockIndex* const tip) const
+{
+    LOCK(cs_height_cache);
+    if (tip) {
+        CacheBlockHeightInternal(tip);
+        m_cached_tip_height = tip->nHeight;
+    } else {
+        m_cached_tip_height = -1;
+    }
+}
+
 int CInstantSendManager::GetTipHeight() const
 {
     {
@@ -1006,14 +1006,9 @@ int CInstantSendManager::GetTipHeight() const
     }
 
     const CBlockIndex* tip = WITH_LOCK(::cs_main, return m_chainstate.m_chain.Tip());
-    assert(tip != nullptr);
 
-    {
-        LOCK(cs_height_cache);
-        CacheBlockHeightInternal(tip->GetBlockHash(), tip->nHeight);
-        m_cached_tip_height = tip->nHeight;
-        return m_cached_tip_height;
-    }
+    CacheTipHeight(tip);
+    return tip ? tip->nHeight : -1;
 }
 
 void CInstantSendManager::WorkThreadMain(PeerManager& peerman)
diff --git a/src/instantsend/instantsend.h b/src/instantsend/instantsend.h
@@ -105,7 +105,7 @@ class CInstantSendManager final : public instantsend::InstantSendSignerParent
         GUARDED_BY(cs_height_cache);
     mutable int m_cached_tip_height GUARDED_BY(cs_height_cache){-1};
 
-    void CacheBlockHeightInternal(const uint256& hash, int height) const EXCLUSIVE_LOCKS_REQUIRED(cs_height_cache);
+    void CacheBlockHeightInternal(const CBlockIndex* const block_index) const EXCLUSIVE_LOCKS_REQUIRED(cs_height_cache);
 
 public:
     CInstantSendManager() = delete;
@@ -192,8 +192,9 @@ class CInstantSendManager final : public instantsend::InstantSendSignerParent
 
     size_t GetInstantSendLockCount() const;
 
-    void CacheBlockHeight(const uint256& hash, int height) const EXCLUSIVE_LOCKS_REQUIRED(!cs_height_cache);
+    void CacheBlockHeight(const CBlockIndex* const block_index) const EXCLUSIVE_LOCKS_REQUIRED(!cs_height_cache);
     std::optional<int> GetBlockHeight(const uint256& hash) const override EXCLUSIVE_LOCKS_REQUIRED(!cs_height_cache);
+    void CacheTipHeight(const CBlockIndex* const tip) const EXCLUSIVE_LOCKS_REQUIRED(!cs_height_cache);
     int GetTipHeight() const override EXCLUSIVE_LOCKS_REQUIRED(!cs_height_cache);
 
     bool IsInstantSendEnabled() const override;

Original file line number	Diff line number	Diff line change
`@@ -136,7 +136,7 @@ MessageProcessingResult CInstantSendManager::ProcessMessage(NodeId from, std::st`
`136`	`136`	`ret.m_error = MisbehavingError{1};`
`137`	`137`	`return ret;`
`138`	`138`	`}`
`139`		`- CacheBlockHeight(blockIndex->GetBlockHash(), blockIndex->nHeight);`
	`139`	`+ CacheBlockHeight(blockIndex);`
`140`	`140`	`cycleHeightOpt = blockIndex->nHeight;`
`141`	`141`	`}`
`142`	`142`
`@@ -395,7 +395,7 @@ MessageProcessingResult CInstantSendManager::ProcessInstantSendLock(NodeId from,`
`395`	`395`	`if (!minedHeight.has_value()) {`
`396`	`396`	`const CBlockIndex* pindexMined = WITH_LOCK(::cs_main, return m_chainstate.m_blockman.LookupBlockIndex(hashBlock));`
`397`	`397`	`if (pindexMined != nullptr) {`
`398`		`- CacheBlockHeight(pindexMined->GetBlockHash(), pindexMined->nHeight);`
	`398`	`+ CacheBlockHeight(pindexMined);`
`399`	`399`	`minedHeight = pindexMined->nHeight;`
`400`	`400`	`}`
`401`	`401`	`}`
`@@ -508,11 +508,7 @@ void CInstantSendManager::BlockConnected(const std::shared_ptr<const CBlock>& pb`
`508`	`508`	`return;`
`509`	`509`	`}`
`510`	`510`
`511`		`- {`
`512`		`- LOCK(cs_height_cache);`
`513`		`- CacheBlockHeightInternal(pindex->GetBlockHash(), pindex->nHeight);`
`514`		`- m_cached_tip_height = pindex->nHeight;`
`515`		`- }`
	`511`	`+ CacheTipHeight(pindex);`
`516`	`512`
`517`	`513`	`if (m_mn_sync.IsBlockchainSynced()) {`
`518`	`514`	`const bool has_chainlock = clhandler.HasChainLock(pindex->nHeight, pindex->GetBlockHash());`
`@@ -544,13 +540,10 @@ void CInstantSendManager::BlockDisconnected(const std::shared_ptr<const CBlock>&`
`544`	`540`	`{`
`545`	`541`	`LOCK(cs_height_cache);`
`546`	`542`	`m_cached_block_heights.erase(pindexDisconnected->GetBlockHash());`
`547`		`- const CBlockIndex* const new_tip = pindexDisconnected->pprev;`
`548`		`- m_cached_tip_height = new_tip ? new_tip->nHeight : -1;`
`549`		`- if (new_tip) {`
`550`		`- CacheBlockHeightInternal(new_tip->GetBlockHash(), new_tip->nHeight);`
`551`		`- }`
`552`	`543`	`}`
`553`	`544`
	`545`	`+ CacheTipHeight(pindexDisconnected->pprev);`
	`546`	`+`
`554`	`547`	`db.RemoveBlockInstantSendLocks(pblock, pindexDisconnected);`
`555`	`548`	`}`
`556`	`549`
`@@ -672,11 +665,7 @@ void CInstantSendManager::NotifyChainLock(const CBlockIndex* pindexChainLock)`
`672`	`665`
`673`	`666`	`void CInstantSendManager::UpdatedBlockTip(const CBlockIndex* pindexNew)`
`674`	`667`	`{`
`675`		`- {`
`676`		`- LOCK(cs_height_cache);`
`677`		`- CacheBlockHeightInternal(pindexNew->GetBlockHash(), pindexNew->nHeight);`
`678`		`- m_cached_tip_height = pindexNew->nHeight;`
`679`		`- }`
	`668`	`+ CacheTipHeight(pindexNew);`
`680`	`669`
`681`	`670`	`bool fDIP0008Active = pindexNew->pprev && pindexNew->pprev->nHeight >= Params().GetConsensus().DIP0008Height;`
`682`	`671`
`@@ -964,16 +953,16 @@ size_t CInstantSendManager::GetInstantSendLockCount() const`
`964`	`953`	`return db.GetInstantSendLockCount();`
`965`	`954`	`}`
`966`	`955`
`967`		`-void CInstantSendManager::CacheBlockHeightInternal(const uint256& hash, int height) const`
	`956`	`+void CInstantSendManager::CacheBlockHeightInternal(const CBlockIndex* const block_index) const`
`968`	`957`	`{`
`969`	`958`	`AssertLockHeld(cs_height_cache);`
`970`		`- m_cached_block_heights.insert(hash, height);`
	`959`	`+ m_cached_block_heights.insert(block_index->GetBlockHash(), block_index->nHeight);`
`971`	`960`	`}`
`972`	`961`
`973`		`-void CInstantSendManager::CacheBlockHeight(const uint256& hash, int height) const`
	`962`	`+void CInstantSendManager::CacheBlockHeight(const CBlockIndex* const block_index) const`
`974`	`963`	`{`
`975`	`964`	`LOCK(cs_height_cache);`
`976`		`- CacheBlockHeightInternal(hash, height);`
	`965`	`+ CacheBlockHeightInternal(block_index);`
`977`	`966`	`}`
`978`	`967`
`979`	`968`	`std::optional<int> CInstantSendManager::GetBlockHeight(const uint256& hash) const`
`@@ -992,10 +981,21 @@ std::optional<int> CInstantSendManager::GetBlockHeight(const uint256& hash) cons`
`992`	`981`	`return std::nullopt;`
`993`	`982`	`}`
`994`	`983`
`995`		`- CacheBlockHeight(pindex->GetBlockHash(), pindex->nHeight);`
	`984`	`+ CacheBlockHeight(pindex);`
`996`	`985`	`return pindex->nHeight;`
`997`	`986`	`}`
`998`	`987`
	`988`	`+void CInstantSendManager::CacheTipHeight(const CBlockIndex* const tip) const`
	`989`	`+{`
	`990`	`+ LOCK(cs_height_cache);`
	`991`	`+ if (tip) {`
	`992`	`+ CacheBlockHeightInternal(tip);`
	`993`	`+ m_cached_tip_height = tip->nHeight;`
	`994`	`+ } else {`
	`995`	`+ m_cached_tip_height = -1;`
	`996`	`+ }`
	`997`	`+}`
	`998`	`+`
`999`	`999`	`int CInstantSendManager::GetTipHeight() const`
`1000`	`1000`	`{`
`1001`	`1001`	`{`
`@@ -1006,14 +1006,9 @@ int CInstantSendManager::GetTipHeight() const`
`1006`	`1006`	`}`
`1007`	`1007`
`1008`	`1008`	`const CBlockIndex* tip = WITH_LOCK(::cs_main, return m_chainstate.m_chain.Tip());`
`1009`		`- assert(tip != nullptr);`
`1010`	`1009`
`1011`		`- {`
`1012`		`- LOCK(cs_height_cache);`
`1013`		`- CacheBlockHeightInternal(tip->GetBlockHash(), tip->nHeight);`
`1014`		`- m_cached_tip_height = tip->nHeight;`
`1015`		`- return m_cached_tip_height;`
`1016`		`- }`
	`1010`	`+ CacheTipHeight(tip);`
	`1011`	`+ return tip ? tip->nHeight : -1;`
`1017`	`1012`	`}`
`1018`	`1013`
`1019`	`1014`	`void CInstantSendManager::WorkThreadMain(PeerManager& peerman)`