fixup! net_processing: handle ConnectionType::PRIVATE_BROADCAST conne…

…ctions

This change adds INV+GETDATA to the transaction sending sequence.

src/net.cpp

@@ -4002,6 +4002,7 @@ void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg)
     if (pnode->IsPrivateBroadcastConn() &&
         msg.m_type != NetMsgType::VERSION &&
         msg.m_type != NetMsgType::VERACK &&
+        msg.m_type != NetMsgType::INV &&
         msg.m_type != NetMsgType::TX &&
         msg.m_type != NetMsgType::PING) {
         // Ensure private broadcast connections only send the above message types.

src/net_processing.cpp

@@ -1101,7 +1101,7 @@ class PeerManagerImpl final : public PeerManager
         EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex, !m_most_recent_block_mutex);
 
     /**
-     * Schedule a transaction be sent to the given peer (via `PushMessage()`), unsolicited.
+     * Schedule an INV for a transaction be sent to the given peer (via `PushMessage()`).
      * The transaction is picked from the list of transactions for private broadcast.
      * It is assumed that the connection to the peer is `ConnectionType::PRIVATE_BROADCAST`.
      * Calling this for other peers will degrade privacy. Don't do that.
@@ -3875,13 +3875,14 @@ void PeerManagerImpl::PushPrivateBroadcastTx(CNode& node)
 
     LogPrintLevel(BCLog::PRIVATE_BROADCAST,
                   BCLog::Level::Info,
-                  "P2P handshake completed, sending txid=%s%s, peer=%d%s\n",
+                  "P2P handshake completed, sending INV for txid=%s%s, peer=%d%s\n",
                   tx->GetHash().ToString(),
                   tx->HasWitness() ? strprintf(", wtxid=%s", tx->GetWitnessHash().ToString()) : "",
                   node.GetId(),
                   fLogIPs ? strprintf(", peeraddr=%s", node.addr.ToStringAddrPort()) : "");
 
-    MakeAndPushMessage(node, NetMsgType::TX, TX_WITH_WITNESS(*tx));
+    MakeAndPushMessage(node, NetMsgType::INV, std::vector<CInv>{{CInv{MSG_TX, tx->GetHash().ToUint256()}}});
+
     m_tx_for_private_broadcast.PushedToNode(node.GetId(), tx->GetHash());
 }
 
@@ -4196,9 +4197,6 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
             // relay logic is designed to work even in cases when the peer drops
             // the transaction (due to it being too cheap, or for other reasons).
             PushPrivateBroadcastTx(pfrom);
-
-            peer->m_ping_queued = true; // Ensure a ping will be sent: mimick a request via RPC.
-            MaybeSendPing(pfrom, *peer, GetTime<std::chrono::microseconds>());
         }
 
         return;
@@ -4324,7 +4322,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
     }
 
     if (pfrom.IsPrivateBroadcastConn()) {
-        if (msg_type != NetMsgType::PONG) {
+        if (msg_type != NetMsgType::PONG && msg_type != NetMsgType::GETDATA) {
             LogPrintLevel(BCLog::PRIVATE_BROADCAST,
                           BCLog::Level::Debug,
                           "Ignoring incoming message '%s', peer=%d%s\n",
@@ -4539,6 +4537,41 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
             LogDebug(BCLog::NET, "received getdata for: %s peer=%d\n", vInv[0].ToString(), pfrom.GetId());
         }
 
+        if (pfrom.IsPrivateBroadcastConn()) {
+            const auto pushed_tx_opt = m_tx_for_private_broadcast.GetTxPushedToNode(pfrom.GetId());
+            if (!pushed_tx_opt) {
+                LogPrintLevel(BCLog::PRIVATE_BROADCAST,
+                              BCLog::Level::Info,
+                              "Disconnecting: got GETDATA without sending an INV, peer=%d%s\n",
+                              pfrom.GetId(),
+                              fLogIPs ? strprintf(", peeraddr=%s", pfrom.addr.ToStringAddrPort()) : "");
+                pfrom.fDisconnect = true;
+                m_connman.m_private_broadcast.NumToOpenAdd(1);
+                return;
+            }
+
+            const CTransactionRef& pushed_tx{*pushed_tx_opt};
+
+            // The GETDATA request must contain exactly one inv and it must be for the transaction
+            // that we INVed to the peer earlier.
+            if (vInv.size() == 1 && vInv[0].IsMsgTx() && vInv[0].hash == pushed_tx->GetHash()) {
+
+                MakeAndPushMessage(pfrom, NetMsgType::TX, TX_WITH_WITNESS(*pushed_tx));
+
+                peer->m_ping_queued = true; // Ensure a ping will be sent: mimick a request via RPC.
+                MaybeSendPing(pfrom, *peer, GetTime<std::chrono::microseconds>());
+            } else {
+                LogPrintLevel(BCLog::PRIVATE_BROADCAST,
+                              BCLog::Level::Info,
+                              "Disconnecting: got an unexpected GETDATA message, peer=%d%s\n",
+                              pfrom.GetId(),
+                              fLogIPs ? strprintf(", peeraddr=%s", pfrom.addr.ToStringAddrPort()) : "");
+                pfrom.fDisconnect = true;
+                m_connman.m_private_broadcast.NumToOpenAdd(1);
+            }
+            return;
+        }
+
         {
             LOCK(peer->m_getdata_requests_mutex);
             peer->m_getdata_requests.insert(peer->m_getdata_requests.end(), vInv.begin(), vInv.end());

src/private_broadcast.cpp

@@ -54,6 +54,24 @@ void PrivateBroadcast::PushedToNode(const NodeId& nodeid, const Txid& txid) EXCL
     m_by_nodeid.emplace(nodeid, txid);
 }
 
+std::optional<CTransactionRef> PrivateBroadcast::GetTxPushedToNode(const NodeId& nodeid)
+    EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
+{
+    LOCK(m_mutex);
+
+    auto it_by_node = m_by_nodeid.find(nodeid);
+    if (it_by_node == m_by_nodeid.end()) {
+        return std::nullopt;
+    }
+    const Txid txid{it_by_node->second};
+
+    auto it_by_txid = m_by_txid.find(txid);
+    if (it_by_txid == m_by_txid.end()) {
+        return std::nullopt;
+    }
+    return it_by_txid->second.tx;
+}
+
 bool PrivateBroadcast::FinishBroadcast(const NodeId& nodeid, bool confirmed_by_node) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
 {
     LOCK(m_mutex);

src/private_broadcast.h

@@ -56,6 +56,11 @@ class PrivateBroadcast
      */
     void PushedToNode(const NodeId& nodeid, const Txid& txid) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
 
+    /**
+     * Get the transaction that was pushed to a given node by PushedToNode().
+     */
+    std::optional<CTransactionRef> GetTxPushedToNode(const NodeId& nodeid) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
+
     /**
      * Mark the end of a broadcast of a transaction. Either successful by receiving a PONG,
      * or unsuccessful by closing the connection to the node without getting PONG.

test/functional/p2p_private_broadcast.py

@@ -251,6 +251,7 @@ def check_broadcasts(self, label, tx, broadcasts_to_expect, skip_destinations):
             assert_equal(peer.message_count, {
                 "version": 1,
                 "verack": 1,
+                "inv": 1,
                 "tx": 1,
                 "ping": 1
             })