fix(net): Extend blocks-relay-only to also ignore some Dash-specific messages/invs

src/llmq/instantsend.cpp

@@ -1526,7 +1526,7 @@ void CInstantSendManager::AskNodesForLockedTx(const uint256& txid, const CConnma
         if (nodesToAskFor.size() >= 4) {
             return;
         }
-        if (pnode->m_tx_relay != nullptr) {
+        if (!pnode->m_block_relay_only_peer) {
             LOCK(pnode->m_tx_relay->cs_tx_inventory);
             if (pnode->m_tx_relay->filterInventoryKnown.contains(txid)) {
                 pnode->AddRef();

src/masternode/sync.cpp

@@ -200,7 +200,7 @@ void CMasternodeSync::ProcessTick(CConnman& connman)
                         // Now that the blockchain is synced request the mempool from the connected outbound nodes if possible
                         for (auto pNodeTmp : vNodesCopy) {
                             bool fRequestedEarlier = netfulfilledman.HasFulfilledRequest(pNodeTmp->addr, "mempool-sync");
-                            if (pNodeTmp->nVersion >= 70216 && !pNodeTmp->fInbound && !fRequestedEarlier) {
+                            if (pNodeTmp->nVersion >= 70216 && !pNodeTmp->fInbound && !fRequestedEarlier && !pNodeTmp->IsBlockRelayOnly()) {
                                 netfulfilledman.AddFulfilledRequest(pNodeTmp->addr, "mempool-sync");
                                 connman.PushMessage(pNodeTmp, msgMaker.Make(NetMsgType::MEMPOOL));
                                 LogPrint(BCLog::MNSYNC, "CMasternodeSync::ProcessTick -- nTick %d nCurrentAsset %d -- syncing mempool from peer=%d\n", nTick, nCurrentAsset, pNodeTmp->GetId());

src/net.cpp

@@ -566,7 +566,7 @@ void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap)
     X(addr);
     X(addrBind);
     stats.m_mapped_as = addr.GetMappedAS(m_asmap);
-    if (m_tx_relay != nullptr) {
+    if (!m_block_relay_only_peer) {
         LOCK(m_tx_relay->cs_filter);
         stats.fRelayTxes = m_tx_relay->fRelayTxes;
     } else {
@@ -947,7 +947,7 @@ bool CConnman::AttemptToEvictConnection()
 
             bool peer_relay_txes = false;
             bool peer_filter_not_null = false;
-            if (node->m_tx_relay != nullptr) {
+            if (!node->m_block_relay_only_peer) {
                 LOCK(node->m_tx_relay->cs_filter);
                 peer_relay_txes = node->m_tx_relay->fRelayTxes;
                 peer_filter_not_null = node->m_tx_relay->pfilter != nullptr;
@@ -2159,7 +2159,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
                     // also have the added issue that they're attacker controlled and could be used
                     // to prevent us from connecting to particular hosts if we used them here.
                     setConnected.insert(pnode->addr.GetGroup(addrman.m_asmap));
-                    if (pnode->m_tx_relay == nullptr) {
+                    if (pnode->m_block_relay_only_peer) {
                         nOutboundBlockRelay++;
                     } else if (!pnode->fFeeler) {
                         nOutboundFullRelay++;
@@ -3480,12 +3480,17 @@ void CConnman::RelayInvFiltered(CInv &inv, const CTransaction& relatedTx, const
 {
     LOCK(cs_vNodes);
     for (const auto& pnode : vNodes) {
-        if (pnode->nVersion < minProtoVersion || !pnode->CanRelay())
+        if (pnode->nVersion < minProtoVersion || !pnode->CanRelay() || pnode->m_block_relay_only_peer) {
             continue;
-        if (pnode->m_tx_relay != nullptr) {
+        }
+        {
             LOCK(pnode->m_tx_relay->cs_filter);
-            if(pnode->m_tx_relay->pfilter && !pnode->m_tx_relay->pfilter->IsRelevantAndUpdate(relatedTx))
+            if (!pnode->m_tx_relay->fRelayTxes) {
                 continue;
+            }
+            if (pnode->m_tx_relay->pfilter && !pnode->m_tx_relay->pfilter->IsRelevantAndUpdate(relatedTx)) {
+                continue;
+            }
         }
         pnode->PushInventory(inv);
     }
@@ -3495,11 +3500,17 @@ void CConnman::RelayInvFiltered(CInv &inv, const uint256& relatedTxHash, const i
 {
     LOCK(cs_vNodes);
     for (const auto& pnode : vNodes) {
-        if (pnode->nVersion < minProtoVersion || !pnode->CanRelay())
+        if (pnode->nVersion < minProtoVersion || !pnode->CanRelay() || pnode->m_block_relay_only_peer) {
             continue;
-        if (pnode->m_tx_relay != nullptr) {
+        }
+        {
             LOCK(pnode->m_tx_relay->cs_filter);
-            if(pnode->m_tx_relay->pfilter && !pnode->m_tx_relay->pfilter->contains(relatedTxHash)) continue;
+            if (!pnode->m_tx_relay->fRelayTxes) {
+                continue;
+            }
+            if (pnode->m_tx_relay->pfilter && !pnode->m_tx_relay->pfilter->contains(relatedTxHash)) {
+                continue;
+            }
         }
         pnode->PushInventory(inv);
     }
@@ -3641,10 +3652,7 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
     fInbound(fInboundIn),
     nKeyedNetGroup(nKeyedNetGroupIn),
     addrKnown(5000, 0.001),
-    // Don't relay addr messages to peers that we connect to as block-relay-only
+    m_block_relay_only_peer(block_relay_only),
-    // peers (to prevent adversaries from inferring these links from addr
-    // traffic).
-    m_addr_relay_peer(!block_relay_only),
     id(idIn),
     nLocalHostNonce(nLocalHostNonceIn),
     nLocalServices(nLocalServicesIn),
@@ -3653,9 +3661,7 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
     hSocket = hSocketIn;
     addrName = addrNameIn == "" ? addr.ToStringIPPort() : addrNameIn;
     hashContinue = uint256();
-    if (!block_relay_only) {
+    m_tx_relay = MakeUnique<TxRelay>();
-        m_tx_relay = MakeUnique<TxRelay>();
-    }
 
     for (const std::string &msg : getAllNetMessageTypes())
         mapRecvBytesPerMsgCmd[msg] = 0;

src/net.h

@@ -1028,15 +1028,25 @@ public:
     int64_t nNextAddrSend GUARDED_BY(cs_sendProcessing){0};
     int64_t nNextLocalAddrSend GUARDED_BY(cs_sendProcessing){0};
 
-    const bool m_addr_relay_peer;
+    const bool m_block_relay_only_peer;
-    bool IsAddrRelayPeer() const { return m_addr_relay_peer; }
+
+    // Don't relay addr messages to peers that we connect to as block-relay-only
+    // peers (to prevent adversaries from inferring these links from addr
+    // traffic).
+    bool IsAddrRelayPeer() const { return !m_block_relay_only_peer; }
+
+    bool IsBlockRelayOnly() const
+    {
+        // Stop processing non-block data early if
+        // 1) We are in blocks only mode and peer has no relay permission
+        // 2) This peer is a block-relay-only peer
+        return (!g_relay_txes && !HasPermission(PF_RELAY)) || m_block_relay_only_peer;
+    }
 
     // List of block ids we still have announce.
     // There is no final sorting before sending, as they are always sent immediately
     // and in the order requested.
     std::vector<uint256> vInventoryBlockToSend GUARDED_BY(cs_inventory);
-    // List of non-tx/non-block inventory items
-    std::vector<CInv> vInventoryOtherToSend;
     CCriticalSection cs_inventory;
     /** UNIX epoch time of the last block received from this peer that we had
      * not yet seen (e.g. not already received from another peer), that passed
@@ -1066,7 +1076,9 @@ public:
         CRollingBloomFilter filterInventoryKnown GUARDED_BY(cs_tx_inventory){50000, 0.000001};
         // Set of transaction ids we still have to announce.
         // They are sorted by the mempool before relay, so the order is not important.
-        std::set<uint256> setInventoryTxToSend;
+        std::set<uint256> setInventoryTxToSend GUARDED_BY(cs_tx_inventory);
+        // List of non-tx/non-block inventory items
+        std::vector<CInv> vInventoryOtherToSend GUARDED_BY(cs_tx_inventory);
         // Used for BIP35 mempool sending, also protected by cs_tx_inventory
         bool fSendMempool GUARDED_BY(cs_tx_inventory){false};
         // Last time a "MEMPOOL" request was serviced.
@@ -1074,7 +1086,8 @@ public:
         std::chrono::microseconds nNextInvSend{0};
     };
 
-    // m_tx_relay == nullptr if we're not relaying transactions with this peer
+    // in bitcoin: m_tx_relay == nullptr if we're not relaying transactions with this peer
+    // in dash: m_tx_relay should never be nullptr, use m_block_relay_only_peer == true instead
     std::unique_ptr<TxRelay> m_tx_relay;
 
     // Used for headers announcements - unfiltered blocks to relay
@@ -1234,49 +1247,29 @@ public:
 
     void AddInventoryKnown(const uint256& hash)
     {
-        if (m_tx_relay != nullptr) {
+        LOCK(m_tx_relay->cs_tx_inventory);
-            LOCK(m_tx_relay->cs_tx_inventory);
+        m_tx_relay->filterInventoryKnown.insert(hash);
-            m_tx_relay->filterInventoryKnown.insert(hash);
-        }
     }
 
     void PushInventory(const CInv& inv)
     {
-        if (inv.type == MSG_TX || inv.type == MSG_DSTX) {
+        if (inv.type == MSG_BLOCK) {
-            if (m_tx_relay != nullptr) {
-                LOCK(m_tx_relay->cs_tx_inventory);
-                if (!m_tx_relay->filterInventoryKnown.contains(inv.hash)) {
-                    LogPrint(BCLog::NET, "%s -- adding new inv: %s peer=%d\n", __func__, inv.ToString(), id);
-                    LOCK(m_tx_relay->cs_filter);
-                    m_tx_relay->setInventoryTxToSend.insert(inv.hash);
-                } else {
-                    LogPrint(BCLog::NET, "%s -- skipping known inv: %s peer=%d\n", __func__, inv.ToString(), id);
-                }
-            } else {
-                LogPrint(BCLog::NET, "%s -- skipping unknown inv: %s peer=%d\n", __func__, inv.ToString(), id);
-            }
-        } else 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);
-        } else {
+            return;
-            LOCK(cs_inventory);
-            if (m_tx_relay != nullptr) {
-                LOCK(m_tx_relay->cs_tx_inventory);
-                if (!m_tx_relay->filterInventoryKnown.contains(inv.hash)) {
-                    LogPrint(BCLog::NET, "%s -- adding new inv: %s peer=%d\n", __func__, inv.ToString(), id);
-                    vInventoryOtherToSend.push_back(inv);
-                } else {
-                    LogPrint(BCLog::NET, "%s -- skipping known inv: %s peer=%d\n", __func__, inv.ToString(), id);
-                }
-            } else {
-                // TODO KNST for this case we don't use inventory
-                // accordingly #2292 it can increase size of transmitted data.
-                // Should be added dedicated bloom filter
-                LogPrint(BCLog::NET, "%s -- adding new inv: %s peer=%d\n", __func__, inv.ToString(), id);
-                vInventoryOtherToSend.push_back(inv);
-            }
         }
+        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;
+        }
+        m_tx_relay->vInventoryOtherToSend.push_back(inv);
     }
 
     void PushBlockHash(const uint256 &hash)

src/net_processing.cpp

@@ -489,7 +489,7 @@ static void PushNodeVersion(CNode *pnode, CConnman* connman, int64_t nTime)
     }
 
     connman->PushMessage(pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERSION, nProtocolVersion, (uint64_t)nLocalNodeServices, nTime, addrYou, addrMe,
-            nonce, strSubVersion, nNodeStartingHeight, ::g_relay_txes && pnode->m_tx_relay != nullptr, mnauthChallenge, pnode->m_masternode_connection.load()));
+            nonce, strSubVersion, nNodeStartingHeight, ::g_relay_txes && !pnode->m_block_relay_only_peer, mnauthChallenge, pnode->m_masternode_connection.load()));
 
     if (fLogIPs) {
         LogPrint(BCLog::NET, "send version message: version %d, blocks=%d, us=%s, them=%s, peer=%d\n", nProtocolVersion, nNodeStartingHeight, addrMe.ToString(), addrYou.ToString(), nodeid);
@@ -1688,7 +1688,7 @@ void static ProcessGetBlockData(CNode* pfrom, const CChainParams& chainparams, c
             else if (inv.type == MSG_FILTERED_BLOCK) {
                 bool sendMerkleBlock = false;
                 CMerkleBlock merkleBlock;
-                if (pfrom->m_tx_relay != nullptr) {
+                if (!pfrom->m_block_relay_only_peer) {
                     LOCK(pfrom->m_tx_relay->cs_filter);
                     if (pfrom->m_tx_relay->pfilter) {
                         sendMerkleBlock = true;
@@ -1757,11 +1757,7 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm
     std::vector<CInv> vNotFound;
     const CNetMsgMaker msgMaker(pfrom->GetSendVersion());
 
-    // Note that if we receive a getdata for a MSG_TX or MSG_WITNESS_TX from a
+    {
-    // block-relay-only outbound peer, we will stop processing further getdata
-    // messages from this peer (likely resulting in our peer eventually
-    // disconnecting us).
-    if (pfrom->m_tx_relay != nullptr) {
         LOCK(cs_main);
 
         while (it != pfrom->vRecvGetData.end() && it->IsKnownType()) {
@@ -1775,6 +1771,13 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm
             if (inv.type == MSG_BLOCK || inv.type == MSG_FILTERED_BLOCK || inv.type == MSG_CMPCT_BLOCK) {
                 break;
             }
+            if (pfrom->m_block_relay_only_peer && NetMessageViolatesBlocksOnly(inv.GetCommand())) {
+                // Note that if we receive a getdata for non-block messages
+                // from a block-relay-only outbound peer that violate the policy,
+                // we will stop processing further getdata messages from this peer
+                // (likely resulting in our peer eventually disconnecting us).
+                break;
+            }
             it++;
 
             // Send stream from relay memory
@@ -2141,7 +2144,7 @@ bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, ChainstateMan
             }
         }
 
-        if (!pfrom->fDisconnect && IsOutboundDisconnectionCandidate(pfrom) && nodestate->pindexBestKnownBlock != nullptr && pfrom->m_tx_relay != nullptr) {
+        if (!pfrom->fDisconnect && IsOutboundDisconnectionCandidate(pfrom) && nodestate->pindexBestKnownBlock != nullptr && !pfrom->m_block_relay_only_peer) {
             // If this is an outbound full-relay peer, check to see if we should protect
             // it from the bad/lagging chain logic.
             // Note that block-relay-only peers are already implicitly protected, so we
@@ -2693,7 +2696,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
         // set nodes not capable of serving the complete blockchain history as "limited nodes"
         pfrom->m_limited_node = (!(nServices & NODE_NETWORK) && (nServices & NODE_NETWORK_LIMITED));
 
-        if (pfrom->m_tx_relay != nullptr) {
+        if (!pfrom->m_block_relay_only_peer) {
             LOCK(pfrom->m_tx_relay->cs_filter);
             pfrom->m_tx_relay->fRelayTxes = fRelay; // set to true after we get the first filter* message
         }
@@ -2766,6 +2769,8 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
     // At this point, the outgoing message serialization version can't change.
     const CNetMsgMaker msgMaker(pfrom->GetSendVersion());
 
+    bool fBlocksOnly = pfrom->IsBlockRelayOnly();
+
     if (msg_type == NetMsgType::VERACK)
     {
         pfrom->SetRecvVersion(std::min(pfrom->nVersion.load(), PROTOCOL_VERSION));
@@ -2777,7 +2782,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
             LogPrintf("New outbound peer connected: version: %d, blocks=%d, peer=%d%s (%s)\n",
                       pfrom->nVersion.load(), pfrom->nStartingHeight,
                       pfrom->GetId(), (fLogIPs ? strprintf(", peeraddr=%s", pfrom->addr.ToString()) : ""),
-                      pfrom->m_tx_relay == nullptr ? "block-relay" : "full-relay");
+                      pfrom->m_block_relay_only_peer ? "block-relay" : "full-relay");
         }
 
         if (!pfrom->m_masternode_probe_connection) {
@@ -2802,10 +2807,11 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
         }
 
         // Tell our peer that he should send us CoinJoin queue messages
-        connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::SENDDSQUEUE, true));
+        if (!fBlocksOnly) {
-
+            connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::SENDDSQUEUE, true));
-        if (llmq::CLLMQUtils::IsWatchQuorumsEnabled() && !pfrom->m_masternode_connection) {
+            if (llmq::CLLMQUtils::IsWatchQuorumsEnabled() && !pfrom->m_masternode_connection) {
-            connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::QWATCH));
+                connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::QWATCH));
+            }
         }
 
         pfrom->fSuccessfullyConnected = true;
@@ -2847,6 +2853,13 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
         }
     }
 
+    // Stop processing non-block data early in blocks only mode and for block-relay-only peers
+    if (fBlocksOnly && NetMessageViolatesBlocksOnly(msg_type)) {
+        LogPrint(BCLog::NET, "%s sent in violation of protocol peer=%d\n", msg_type, pfrom->GetId());
+        pfrom->fDisconnect = true;
+        return false;
+    }
+
     if (msg_type == NetMsgType::ADDR || msg_type == NetMsgType::ADDRV2) {
         int stream_version = vRecv.GetVersion();
         if (msg_type == NetMsgType::ADDRV2) {
@@ -2964,14 +2977,6 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
             return false;
         }
 
-        // We won't accept tx inv's if we're in blocks-only mode, or this is a
-        // block-relay-only peer
-        bool fBlocksOnly = !g_relay_txes || (pfrom->m_tx_relay == nullptr);
-
-        // Allow whitelisted peers to send data other than blocks in blocks only mode if whitelistrelay is true
-        if (pfrom->HasPermission(PF_RELAY))
-            fBlocksOnly = false;
-
         LOCK(cs_main);
 
         const auto current_time = GetTime<std::chrono::microseconds>();
@@ -3027,11 +3032,22 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
                 };
 
                 pfrom->AddInventoryKnown(inv);
-                if (fBlocksOnly) {
+                if (fBlocksOnly && NetMessageViolatesBlocksOnly(inv.GetCommand())) {
-                    LogPrint(BCLog::NET, "transaction (%s) inv sent in violation of protocol, disconnecting peer=%d\n", inv.hash.ToString(), pfrom->GetId());
+                    LogPrint(BCLog::NET, "%s (%s) inv sent in violation of protocol, disconnecting peer=%d\n", inv.GetCommand(), inv.hash.ToString(), pfrom->GetId());
                     pfrom->fDisconnect = true;
                     return true;
                 } else if (!fAlreadyHave) {
+                    if (fBlocksOnly && (inv.type == MSG_ISLOCK || inv.type == MSG_ISDLOCK)) {
+                        if (pfrom->GetRecvVersion() <= ADDRV2_PROTO_VERSION) {
+                            // It's ok to receive these invs, we just ignore them
+                            // and do not request corresponding objects.
+                            continue;
+                        }
+                        // Peers with newer versions should never send us these invs when we are in blocks-relay-only mode
+                        LogPrint(BCLog::NET, "%s (%s) inv sent in violation of protocol, disconnecting peer=%d\n", inv.GetCommand(), inv.hash.ToString(), pfrom->GetId());
+                        pfrom->fDisconnect = true;
+                        return true;
+                    }
                     bool allowWhileInIBD = allowWhileInIBDObjs.count(inv.type);
                     if (allowWhileInIBD || !chainman.ActiveChainstate().IsInitialBlockDownload()) {
                         RequestObject(State(pfrom->GetId()), inv, current_time);
@@ -3266,16 +3282,6 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
     }
 
     if (msg_type == NetMsgType::TX || msg_type == NetMsgType::DSTX || msg_type == NetMsgType::LEGACYTXLOCKREQUEST) {
-        // Stop processing the transaction early if
-        // 1) We are in blocks only mode and peer has no relay permission
-        // 2) This peer is a block-relay-only peer
-        if ((!g_relay_txes && !pfrom->HasPermission(PF_RELAY)) || (pfrom->m_tx_relay == nullptr))
-        {
-            LogPrint(BCLog::NET, "transaction sent in violation of protocol peer=%d\n", pfrom->GetId());
-            pfrom->fDisconnect = true;
-            return true;
-        }
-
         CTransactionRef ptx;
         CCoinJoinBroadcastTx dstx;
         int nInvType = MSG_TX;
@@ -3862,7 +3868,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
             return true;
         }
 
-        if (pfrom->m_tx_relay != nullptr) {
+        if (!pfrom->m_block_relay_only_peer) {
             LOCK(pfrom->m_tx_relay->cs_tx_inventory);
             pfrom->m_tx_relay->fSendMempool = true;
         }
@@ -3953,7 +3959,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
             LOCK(cs_main);
             Misbehaving(pfrom->GetId(), 100);
         }
-        else if (pfrom->m_tx_relay != nullptr)
+        else if (!pfrom->m_block_relay_only_peer)
         {
             LOCK(pfrom->m_tx_relay->cs_filter);
             pfrom->m_tx_relay->pfilter.reset(new CBloomFilter(filter));
@@ -3972,7 +3978,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
         bool bad = false;
         if (vData.size() > MAX_SCRIPT_ELEMENT_SIZE) {
             bad = true;
-        } else if (pfrom->m_tx_relay != nullptr) {
+        } else if (!pfrom->m_block_relay_only_peer) {
             LOCK(pfrom->m_tx_relay->cs_filter);
             if (pfrom->m_tx_relay->pfilter) {
                 pfrom->m_tx_relay->pfilter->insert(vData);
@@ -3988,7 +3994,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
     }
 
     if (msg_type == NetMsgType::FILTERCLEAR) {
-        if (pfrom->m_tx_relay == nullptr) {
+        if (pfrom->m_block_relay_only_peer) {
             return true;
         }
         LOCK(pfrom->m_tx_relay->cs_filter);
@@ -4365,7 +4371,7 @@ void PeerLogicValidation::EvictExtraOutboundPeers(int64_t time_in_seconds)
             // Don't evict our protected peers
             if (state->m_chain_sync.m_protect) return;
             // Don't evict our block-relay-only peers.
-            if (pnode->m_tx_relay == nullptr) return;
+            if (pnode->m_block_relay_only_peer) return;
             if (state->m_last_block_announcement < oldest_block_announcement || (state->m_last_block_announcement == oldest_block_announcement && pnode->GetId() > worst_peer)) {
                 worst_peer = pnode->GetId();
                 oldest_block_announcement = state->m_last_block_announcement;
@@ -4724,8 +4730,8 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
             LOCK2(m_mempool.cs, pto->cs_inventory);
 
             size_t reserve = INVENTORY_BROADCAST_MAX_PER_1MB_BLOCK * MaxBlockSize() / 1000000;
-            if (pto->m_tx_relay != nullptr) {
+            if (!pto->m_block_relay_only_peer) {
-                LOCK(pto->m_tx_relay->cs_filter);
+                LOCK(pto->m_tx_relay->cs_tx_inventory);
                 reserve = std::min<size_t>(pto->m_tx_relay->setInventoryTxToSend.size(), reserve);
             }
             reserve = std::max<size_t>(reserve, pto->vInventoryBlockToSend.size());
@@ -4743,10 +4749,8 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
             pto->vInventoryBlockToSend.clear();
 
             auto queueAndMaybePushInv = [this, pto, &vInv, &msgMaker](const CInv& invIn) {
-                if (pto->m_tx_relay != nullptr) {
+                AssertLockHeld(pto->m_tx_relay->cs_tx_inventory);
-                    AssertLockHeld(pto->m_tx_relay->cs_tx_inventory);
+                pto->m_tx_relay->filterInventoryKnown.insert(invIn.hash);
-                    pto->m_tx_relay->filterInventoryKnown.insert(invIn.hash);
-                }
                 LogPrint(BCLog::NET, "SendMessages -- queued inv: %s  index=%d peer=%d\n", invIn.ToString(), vInv.size(), pto->GetId());
                 vInv.push_back(invIn);
                 if (vInv.size() == MAX_INV_SZ) {
@@ -4756,7 +4760,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
                 }
             };
 
-            if (pto->m_tx_relay != nullptr) {
+            if (!pto->m_block_relay_only_peer) {
                 LOCK(pto->m_tx_relay->cs_tx_inventory);
                 // Check whether periodic sends should happen
                 // Note: If this node is running in a Masternode mode, it makes no sense to delay outgoing txes
@@ -4865,21 +4869,27 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
                         queueAndMaybePushInv(CInv(nInvType, hash));
                     }
                 }
+            }
 
+            {
                 // Send non-tx/non-block inventory items
-                for (const auto& inv : pto->vInventoryOtherToSend) {
+                LOCK2(pto->m_tx_relay->cs_tx_inventory, pto->m_tx_relay->cs_filter);
+
+                bool fSendIS = pto->m_tx_relay->fRelayTxes && !pto->IsBlockRelayOnly();
+
+                for (const auto& inv : pto->m_tx_relay->vInventoryOtherToSend) {
+                    if (!pto->m_tx_relay->fRelayTxes && NetMessageViolatesBlocksOnly(inv.GetCommand())) {
+                        continue;
+                    }
                     if (pto->m_tx_relay->filterInventoryKnown.contains(inv.hash)) {
                         continue;
                     }
+                    if (!fSendIS && (inv.type == MSG_ISLOCK || inv.type == MSG_ISDLOCK)) {
+                        continue;
+                    }
                     queueAndMaybePushInv(inv);
                 }
-                pto->vInventoryOtherToSend.clear();
+                pto->m_tx_relay->vInventoryOtherToSend.clear();
-            } else { // m_tx_relay is nullptr but we still need to send items from `vInventoryOtherToSend`
-                // Send non-tx/non-block inventory items
-                for (const auto& inv : pto->vInventoryOtherToSend) {
-                    queueAndMaybePushInv(inv);
-                }
-                pto->vInventoryOtherToSend.clear();
             }
         }
         if (!vInv.empty())

src/protocol.cpp

@@ -174,6 +174,34 @@ const static std::string allNetMessageTypes[] = {
     NetMsgType::HEADERS2};
 const static std::vector<std::string> allNetMessageTypesVec(allNetMessageTypes, allNetMessageTypes+ARRAYLEN(allNetMessageTypes));
 
+const static std::string netMessageTypesViolateBlocksOnly[] = {
+    NetMsgType::DSACCEPT,
+    NetMsgType::DSCOMPLETE,
+    NetMsgType::DSFINALTX,
+    NetMsgType::DSQUEUE,
+    NetMsgType::DSSIGNFINALTX,
+    NetMsgType::DSSTATUSUPDATE,
+    NetMsgType::DSTX,
+    NetMsgType::DSVIN,
+    NetMsgType::LEGACYTXLOCKREQUEST,
+    NetMsgType::QBSIGSHARES,
+    NetMsgType::QCOMPLAINT,
+    NetMsgType::QCONTRIB,
+    NetMsgType::QDATA,
+    NetMsgType::QGETDATA,
+    NetMsgType::QGETSIGSHARES,
+    NetMsgType::QJUSTIFICATION,
+    NetMsgType::QPCOMMITMENT,
+    NetMsgType::QSENDRECSIGS,
+    NetMsgType::QSIGREC,
+    NetMsgType::QSIGSESANN,
+    NetMsgType::QSIGSHARE,
+    NetMsgType::QSIGSHARESINV,
+    NetMsgType::QWATCH,
+    NetMsgType::TX,
+};
+const static std::set<std::string> netMessageTypesViolateBlocksOnlySet(netMessageTypesViolateBlocksOnly, netMessageTypesViolateBlocksOnly+ARRAYLEN(netMessageTypesViolateBlocksOnly));
+
 CMessageHeader::CMessageHeader(const MessageStartChars& pchMessageStartIn)
 {
     memcpy(pchMessageStart, pchMessageStartIn, MESSAGE_START_SIZE);
@@ -337,6 +365,11 @@ const std::vector<std::string> &getAllNetMessageTypes()
     return allNetMessageTypesVec;
 }
 
+bool NetMessageViolatesBlocksOnly(const std::string& msg_type)
+{
+    return netMessageTypesViolateBlocksOnlySet.find(msg_type) != netMessageTypesViolateBlocksOnlySet.end();
+}
+
 /**
  * Convert a service flag (NODE_*) to a human readable string.
  * It supports unknown service flags which will be returned as "UNKNOWN[...]".

src/protocol.h

@@ -306,6 +306,9 @@ extern const char *QUORUMROTATIONINFO;
 /* Get a vector of all valid message types (see above) */
 const std::vector<std::string> &getAllNetMessageTypes();
 
+/* Whether the message type violates blocks-relay-only policy */
+bool NetMessageViolatesBlocksOnly(const std::string& msg_type);
+
 /** nServices flags */
 enum ServiceFlags : uint64_t {
     // NOTE: When adding here, be sure to update serviceFlagToStr too

test/functional/p2p_blocksonly.py

@@ -40,7 +40,7 @@ class P2PBlocksOnly(BitcoinTestFramework):
             }],
         )['hex']
         assert_equal(self.nodes[0].getnetworkinfo()['localrelay'], False)
-        with self.nodes[0].assert_debug_log(['transaction sent in violation of protocol peer=0']):
+        with self.nodes[0].assert_debug_log(['tx sent in violation of protocol peer=0']):
             self.nodes[0].p2p.send_message(msg_tx(FromHex(CTransaction(), sigtx)))
             self.nodes[0].p2p.wait_for_disconnect()
             assert_equal(self.nodes[0].getmempoolinfo()['size'], 0)