Merge #20624: net processing: Remove nStartingHeight check from block relay

f636008 [net processing] Clarify UpdatedBlockTip() (John Newbery)
94d2cc3 [net processing] Remove unnecesary nNewHeight variable in UpdatedBlockTip() (John Newbery)
8b57013 [net processing] Remove nStartingHeight check from block relay (John Newbery)

Pull request description:

  nStartingHeight was introduced in commit 7a47324 (Bitcoin version
  0.2.9, P2P version 209) with the comment "better prevention of inventory
  relaying during initial download". At that time, there was no function
  to determine whether the node was still in Initial Block Download, so to
  prevent syncing nodes from relaying old blocks to their peers, a check
  was added to never relay a block to a peer where the height was lower
  than 2000 less than the peer's best block. That check was updated
  several times in later commits to ensure that we weren't relaying blocks
  before the latest checkpoint if the peer didn't provide a
  startingheight. The checkpoint comparison was changed to compare with an
  estimate of the highest block in commit eae82d8.

  In commit 202e019, all block relay was gated on being out of Initial
  Block Download. In commit 0278fb5, the comparison to nBlockEstimate was
  removed since "we already checked IsIBD()".

  We can remove the check against nStartingHeight entirely. If the node is
  out of Initial Block Download, then its tip height must have been within
  24 hours of current time, so should not be more than ~144 blocks behind
  the most work tip.

  This simplifies moving block inventory state into the `Peer` object (#19829).

ACKs for top commit:
  Sjors:
    utACK f636008
  jonatack:
    ACK f636008
  MarcoFalke:
    ACK f636008 💽
  ariard:
    Code Review ACK f636008

Tree-SHA512: 4959cf35f1dcde46f34bffec1375729a157e1b2a1fd8a8ca33da9771c3c89a6c43e7050cdeeab8d90bb507b0795703db8c8bc304a1a5065ef00aae7a6992ca4f

Author: MarcoFalke

src/net_processing.cpp

@@ -1290,34 +1290,33 @@ void PeerManager::NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_
  * in ::ChainActive() to our peers.
  */
 void PeerManager::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) {
-    const int nNewHeight = pindexNew->nHeight;
-    m_connman.SetBestHeight(nNewHeight);
-
+    m_connman.SetBestHeight(pindexNew->nHeight);
     SetServiceFlagsIBDCache(!fInitialDownload);
-    if (!fInitialDownload) {
-        // Find the hashes of all blocks that weren't previously in the best chain.
-        std::vector<uint256> vHashes;
-        const CBlockIndex *pindexToAnnounce = pindexNew;
-        while (pindexToAnnounce != pindexFork) {
-            vHashes.push_back(pindexToAnnounce->GetBlockHash());
-            pindexToAnnounce = pindexToAnnounce->pprev;
-            if (vHashes.size() == MAX_BLOCKS_TO_ANNOUNCE) {
-                // Limit announcements in case of a huge reorganization.
-                // Rely on the peer's synchronization mechanism in that case.
-                break;
-            }
+
+    // Don't relay inventory during initial block download.
+    if (fInitialDownload) return;
+
+    // Find the hashes of all blocks that weren't previously in the best chain.
+    std::vector<uint256> vHashes;
+    const CBlockIndex *pindexToAnnounce = pindexNew;
+    while (pindexToAnnounce != pindexFork) {
+        vHashes.push_back(pindexToAnnounce->GetBlockHash());
+        pindexToAnnounce = pindexToAnnounce->pprev;
+        if (vHashes.size() == MAX_BLOCKS_TO_ANNOUNCE) {
+            // Limit announcements in case of a huge reorganization.
+            // Rely on the peer's synchronization mechanism in that case.
+            break;
         }
-        // Relay inventory, but don't relay old inventory during initial block download.
-        m_connman.ForEachNode([nNewHeight, &vHashes](CNode* pnode) {
-            LOCK(pnode->cs_inventory);
-            if (nNewHeight > (pnode->nStartingHeight != -1 ? pnode->nStartingHeight - 2000 : 0)) {
-                for (const uint256& hash : reverse_iterate(vHashes)) {
-                    pnode->vBlockHashesToAnnounce.push_back(hash);
-                }
-            }
-        });
-        m_connman.WakeMessageHandler();
     }
+
+    // Relay to all peers
+    m_connman.ForEachNode([&vHashes](CNode* pnode) {
+        LOCK(pnode->cs_inventory);
+        for (const uint256& hash : reverse_iterate(vHashes)) {
+            pnode->vBlockHashesToAnnounce.push_back(hash);
+        }
+    });
+    m_connman.WakeMessageHandler();
 }
 
 /**