Merge bitcoin#19322: [net] split PushInventory()

f52d403 [net] split PushInventory() (John Newbery)

Pull request description:

  PushInventory() is currently called with a CInv object, which can be a
  MSG_TX or MSG_BLOCK. PushInventory() only uses the type to determine
  whether to add the hash to setInventoryTxToSend or
  vInventoryBlockToSend.

  Since the caller always knows what type of inventory they're pushing,
  the CInv is wastefully constructed and thrown away, and tx/block relay
  is being split out, we split the function into PushTxInventory() and
  PushBlockInventory().

ACKs for top commit:
  amitiuttarwar:
    utACK f52d403. nice cleanup, this has bothered me :)
  naumenkogs:
    utACK f52d403
  sipa:
    utACK f52d403

Tree-SHA512: 331495199a3b1a2620e6a62beb336e494291b725d8fd64bb44726c02e80807f3974ff4f329bb0f059088e65cd7d41eff276c1065806d2dd6e72c5a9f368e82cd

src/net.h

@@ -1370,25 +1370,19 @@ class CNode
         m_tx_relay->filterInventoryKnown.insert(hash);
     }
 
-    void PushInventory(const CInv& inv)
+    void PushTxInventory(const uint256& hash)
     {
-        if (inv.type == MSG_BLOCK) {
-            LogPrint(BCLog::NET, "%s -- adding new inv: %s peer=%d\n", __func__, inv.ToString(), id);
-            LOCK(cs_inventory);
-            vInventoryBlockToSend.push_back(inv.hash);
-            return;
-        }
+        if (m_tx_relay == nullptr) return;
         LOCK(m_tx_relay->cs_tx_inventory);
-        if (m_tx_relay->filterInventoryKnown.contains(inv.hash)) {
-            LogPrint(BCLog::NET, "%s -- skipping known inv: %s peer=%d\n", __func__, inv.ToString(), id);
-            return;
-        }
-        LogPrint(BCLog::NET, "%s -- adding new inv: %s peer=%d\n", __func__, inv.ToString(), id);
-        if (inv.type == MSG_TX || inv.type == MSG_DSTX) {
-            m_tx_relay->setInventoryTxToSend.insert(inv.hash);
-            return;
+        if (!m_tx_relay->filterInventoryKnown.contains(hash)) {
+            m_tx_relay->setInventoryTxToSend.insert(hash);
         }
-        m_tx_relay->vInventoryOtherToSend.push_back(inv);
+    }
+
+    void PushBlockInventory(const uint256& hash)
+    {
+        LOCK(cs_inventory);
+        vInventoryBlockToSend.push_back(hash);
     }
 
     void PushBlockHash(const uint256 &hash)

src/net_processing.cpp

@@ -1891,7 +1891,7 @@ void PeerManagerImpl::RelayTransaction(const uint256& txid)
     CInv inv(CCoinJoin::GetDSTX(txid) ? MSG_DSTX : MSG_TX, txid);
     m_connman.ForEachNode([&inv](CNode* pnode)
     {
-        pnode->PushInventory(inv);
+        pnode->PushTxInventory(txid);
     });
 }
 
@@ -2068,7 +2068,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, const CChainParams& chai
         // Trigger the peer node to send a getblocks request for the next batch of inventory
         if (inv.hash == pfrom.hashContinue)
         {
-            // Bypass PushInventory, this must send even if redundant,
+            // Bypass PushBlockInventory, 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;
@@ -3387,7 +3387,7 @@ void PeerManagerImpl::ProcessMessage(
                 break;
             }
             if (pfrom.CanRelay()) {
-                pfrom.PushInventory(CInv(MSG_BLOCK, pindex->GetBlockHash()));
+                pfrom.PushBlockInventory(pindex->GetBlockHash());
             }
             if (--nLimit <= 0)
             {
@@ -4902,7 +4902,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
 
                     // If the peer's chain has this block, don't inv it back.
                     if (!PeerHasHeader(&state, pindex)) {
-                        pto->PushInventory(CInv(MSG_BLOCK, hashToAnnounce));
+                        pto->PushBlockInventory(hashToAnnounce);
                         LogPrint(BCLog::NET, "%s: sending inv peer=%d hash=%s\n", __func__,
                             pto->GetId(), hashToAnnounce.ToString());
                     }