[net processing] Remove PushBlockInventory and PushBlockHash

jnewbery · jnewbery · commit 344e831de54f · 2020-06-23T08:46:05.000-04:00
PushBlockInventory() and PushBlockHash() are functions that can
be replaced with single-line statements. This also eliminates
the single place that cs_inventory is taken recursively.
diff --git a/src/net.h b/src/net.h
@@ -982,18 +982,6 @@ class CNode
         }
     }
 
-    void PushBlockInventory(const uint256& hash)
-    {
-        LOCK(cs_inventory);
-        vInventoryBlockToSend.push_back(hash);
-    }
-
-    void PushBlockHash(const uint256 &hash)
-    {
-        LOCK(cs_inventory);
-        vBlockHashesToAnnounce.push_back(hash);
-    }
-
     void CloseSocketDisconnect();
 
     void copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap);
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
@@ -1328,9 +1328,10 @@ void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex *pindexNew, const CB
         }
         // Relay inventory, but don't relay old inventory during initial block download.
         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->PushBlockHash(hash);
+                    pnode->vBlockHashesToAnnounce.push_back(hash);
                 }
             }
         });
@@ -1607,7 +1608,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c
         // Trigger the peer node to send a getblocks request for the next batch of inventory
         if (inv.hash == pfrom.hashContinue)
         {
-            // Bypass PushBlockInventory, this must send even if redundant,
+            // Send immediately. This must send even if redundant,
             // and we want it right after the last block so they don't
             // wait for other stuff first.
             std::vector<CInv> vInv;
@@ -2666,7 +2667,7 @@ void ProcessMessage(
                 LogPrint(BCLog::NET, " getblocks stopping, pruned or too old block at %d %s\n", pindex->nHeight, pindex->GetBlockHash().ToString());
                 break;
             }
-            pfrom.PushBlockInventory(pindex->GetBlockHash());
+            WITH_LOCK(pfrom.cs_inventory, pfrom.vInventoryBlockToSend.push_back(pindex->GetBlockHash()));
             if (--nLimit <= 0)
             {
                 // When this block is requested, we'll send an inv that'll
@@ -4083,7 +4084,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
 
                     // If the peer's chain has this block, don't inv it back.
                     if (!PeerHasHeader(&state, pindex)) {
-                        pto->PushBlockInventory(hashToAnnounce);
+                        pto->vInventoryBlockToSend.push_back(hashToAnnounce);
                         LogPrint(BCLog::NET, "%s: sending inv peer=%d hash=%s\n", __func__,
                             pto->GetId(), hashToAnnounce.ToString());
                     }

Original file line number	Diff line number	Diff line change
`@@ -1328,9 +1328,10 @@ void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex *pindexNew, const CB`
`1328`	`1328`	`}`
`1329`	`1329`	`// Relay inventory, but don't relay old inventory during initial block download.`
`1330`	`1330`	`connman->ForEachNode([nNewHeight, &vHashes](CNode* pnode) {`
	`1331`	`+ LOCK(pnode->cs_inventory);`
`1331`	`1332`	`if (nNewHeight > (pnode->nStartingHeight != -1 ? pnode->nStartingHeight - 2000 : 0)) {`
`1332`	`1333`	`for (const uint256& hash : reverse_iterate(vHashes)) {`
`1333`		`- pnode->PushBlockHash(hash);`
	`1334`	`+ pnode->vBlockHashesToAnnounce.push_back(hash);`
`1334`	`1335`	`}`
`1335`	`1336`	`}`
`1336`	`1337`	`});`
`@@ -1607,7 +1608,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c`
`1607`	`1608`	`// Trigger the peer node to send a getblocks request for the next batch of inventory`
`1608`	`1609`	`if (inv.hash == pfrom.hashContinue)`
`1609`	`1610`	`{`
`1610`		`- // Bypass PushBlockInventory, this must send even if redundant,`
	`1611`	`+ // Send immediately. This must send even if redundant,`
`1611`	`1612`	`// and we want it right after the last block so they don't`
`1612`	`1613`	`// wait for other stuff first.`
`1613`	`1614`	`std::vector<CInv> vInv;`
`@@ -2666,7 +2667,7 @@ void ProcessMessage(`
`2666`	`2667`	`LogPrint(BCLog::NET, " getblocks stopping, pruned or too old block at %d %s\n", pindex->nHeight, pindex->GetBlockHash().ToString());`
`2667`	`2668`	`break;`
`2668`	`2669`	`}`
`2669`		`- pfrom.PushBlockInventory(pindex->GetBlockHash());`
	`2670`	`+ WITH_LOCK(pfrom.cs_inventory, pfrom.vInventoryBlockToSend.push_back(pindex->GetBlockHash()));`
`2670`	`2671`	`if (--nLimit <= 0)`
`2671`	`2672`	`{`
`2672`	`2673`	`// When this block is requested, we'll send an inv that'll`
`@@ -4083,7 +4084,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto)`
`4083`	`4084`
`4084`	`4085`	`// If the peer's chain has this block, don't inv it back.`
`4085`	`4086`	`if (!PeerHasHeader(&state, pindex)) {`
`4086`		`- pto->PushBlockInventory(hashToAnnounce);`
	`4087`	`+ pto->vInventoryBlockToSend.push_back(hashToAnnounce);`
`4087`	`4088`	`LogPrint(BCLog::NET, "%s: sending inv peer=%d hash=%s\n", __func__,`
`4088`	`4089`	`pto->GetId(), hashToAnnounce.ToString());`
`4089`	`4090`	`}`