Merge 5caaefd8d4 into b510893d00

src/net_processing.cpp

@@ -2481,7 +2481,7 @@ void PeerManagerImpl::SendBlockTransactions(CNode& pfrom, Peer& peer, const CBlo
         tx_requested_size += resp.txn[i]->GetTotalSize();
     }
 
-    LogDebug(BCLog::CMPCTBLOCK, "Peer %d sent us a GETBLOCKTXN for block %s, sending a BLOCKTXN with %u txns. (%u bytes)\n", pfrom.GetId(), block.GetHash().ToString(), resp.txn.size(), tx_requested_size);
+    LogDebug(BCLog::CMPCTBLOCK, "Peer %d%s sent us a GETBLOCKTXN for block %s, sending a BLOCKTXN with %u txns. (%u bytes)\n", pfrom.GetId(), pfrom.LogIP(fLogIPs), block.GetHash().ToString(), resp.txn.size(), tx_requested_size);
     MakeAndPushMessage(pfrom, NetMsgType::BLOCKTXN, resp);
 }
 
@@ -4412,6 +4412,11 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
             range_flight.first++;
         }
 
+        if (!requested_block_from_this_peer && !pfrom.m_bip152_highbandwidth_to) {
+            LogDebug(BCLog::NET, "Peer %d, not marked as high-bandwidth, sent us an unsolicited compact block!\n", pfrom.GetId());
+            return;
+        }
+
         if (pindex->nChainWork <= m_chainman.ActiveChain().Tip()->nChainWork || // We know something better
                 pindex->nTx != 0) { // We had this block at some point, but pruned it
             if (requested_block_from_this_peer) {

test/functional/p2p_compactblocks.py

@@ -265,9 +265,13 @@ class CompactBlocksTest(BitcoinTestFramework):
 
     # This test actually causes bitcoind to (reasonably!) disconnect us, so do this last.
     def test_invalid_cmpctblock_message(self):
+        self.make_peer_hb_to_candidate(self.nodes[0], self.segwit_node)
         self.generate(self.nodes[0], COINBASE_MATURITY + 1)
         block = self.build_block_on_tip(self.nodes[0])
 
+        self.segwit_node.send_header_for_blocks([block])
+        self.segwit_node.wait_for_getdata([block.hash_int], timeout=30)
+
         cmpct_block = P2PHeaderAndShortIDs()
         cmpct_block.header = CBlockHeader(block)
         cmpct_block.prefilled_txn_length = 1
@@ -565,6 +569,10 @@ class CompactBlocksTest(BitcoinTestFramework):
 
         block = self.build_block_with_transactions(node, utxo, 2)
 
+        # The attacker sends the block header so that we request it.
+        test_node.send_without_ping(msg_headers([block]))
+        test_node.wait_for_getdata([block.hash_int], timeout=30)
+
         # Send compact block
         comp_block = HeaderAndShortIDs()
         comp_block.initialize_from_block(block, prefill_list=[0], use_witness=True)
@@ -794,6 +802,12 @@ class CompactBlocksTest(BitcoinTestFramework):
         msg = msg_cmpctblock(comp_block.to_p2p())
         test_node.send_await_disconnect(msg)
 
+    # peer generates a block and sends it to node, which makes the peer a
+    # candidate for high-bandwidth 'to' (up to 3 peers according to BIP 152)
+    def make_peer_hb_to_candidate(self, node, peer):
+        block = self.build_block_on_tip(node)
+        peer.send_and_ping(msg_block(block))
+
     # Helper for enabling cb announcements
     # Send the sendcmpct request and sync headers
     def request_cb_announcements(self, peer):
@@ -806,6 +820,9 @@ class CompactBlocksTest(BitcoinTestFramework):
         node = self.nodes[0]
         assert len(self.utxos)
 
+        self.make_peer_hb_to_candidate(node, stalling_peer)
+        self.make_peer_hb_to_candidate(node, delivery_peer)
+
         def announce_cmpct_block(node, peer):
             utxo = self.utxos.pop(0)
             block = self.build_block_with_transactions(node, utxo, 5)
@@ -901,6 +918,7 @@ class CompactBlocksTest(BitcoinTestFramework):
 
         for name, peer in [("delivery", delivery_peer), ("inbound", inbound_peer), ("outbound", outbound_peer)]:
             self.log.info(f"Setting {name} as high bandwidth peer")
+            self.make_peer_hb_to_candidate(node, peer)
             block, cmpct_block = announce_cmpct_block(node, peer, 1)
             msg = msg_blocktxn()
             msg.block_transactions.blockhash = block.hash_int
@@ -950,6 +968,63 @@ class CompactBlocksTest(BitcoinTestFramework):
             inbound_peer.clear_getblocktxn()
             outbound_peer.clear_getblocktxn()
 
+    def test_unsolicited_compact_blocks_ignored(self):
+        node = self.nodes[0]
+        # create new p2p connection for a fresh state w/o any prior sendcmpct messages sent
+        unsolicited_peer = self.nodes[0].add_p2p_connection(TestP2PConn())
+
+        # assert the RPC getpeerinfo boolean fields `bip152_hb_{to, from}`
+        # match the given parameters for the last peer of a given node
+        def assert_highbandwidth_states(node, idx, hb_to, hb_from=False):
+            peerinfo = node.getpeerinfo()[idx]
+            assert_equal(peerinfo['bip152_hb_to'], hb_to)
+            assert_equal(peerinfo['bip152_hb_from'], hb_from)
+
+        def build_compact_block():
+            # generate a compact block to send
+            assert len(self.utxos)
+            utxo = self.utxos.pop(0)
+            block = self.build_block_with_transactions(node, utxo, 10)
+            cmpct_block = HeaderAndShortIDs()
+            cmpct_block.initialize_from_block(block)
+            msg = msg_cmpctblock(cmpct_block.to_p2p())
+            return block, msg
+
+        assert_highbandwidth_states(node, idx=-1, hb_to=False)
+
+        # The node won't request transactions from a block which it has not
+        # requested from a non-high-bandwidth peer
+        _, msg = build_compact_block()
+        unsolicited_peer.send_and_ping(msg)
+        with p2p_lock:
+            assert "getblocktxn" not in unsolicited_peer.last_message
+
+        # The node will ask for transactions from a compact block which it has
+        # requested from a non-hb peer
+
+        # peer is still not selected as high-bandwidth
+        assert_highbandwidth_states(node, idx=-1, hb_to=False)
+
+        block, msg = build_compact_block()
+        unsolicited_peer.send_without_ping(msg_headers([block]))
+        unsolicited_peer.wait_for_getdata([block.hash_int], timeout=30)
+        unsolicited_peer.send_and_ping(msg)
+        with p2p_lock:
+            assert "getblocktxn" in unsolicited_peer.last_message
+        unsolicited_peer.clear_getblocktxn()
+
+        # The node will ask for transactions from an unsolicited compact block
+        # it receives from a high bandwidth peer, we need to use one set up earlier,
+        # since all the slots are full.
+        hb_peer_idx = -2
+        assert_highbandwidth_states(node, idx=hb_peer_idx, hb_to=True)
+        hb_peer = self.nodes[0].p2ps[hb_peer_idx]
+        hb_peer.clear_getblocktxn()
+
+        _, msg = build_compact_block()
+        hb_peer.send_and_ping(msg)
+        with p2p_lock:
+            assert "getblocktxn" in hb_peer.last_message
 
     def run_test(self):
         self.wallet = MiniWallet(self.nodes[0])
@@ -1025,6 +1100,9 @@ class CompactBlocksTest(BitcoinTestFramework):
         self.log.info("Testing high-bandwidth mode states via getpeerinfo...")
         self.test_highbandwidth_mode_states_via_getpeerinfo()
 
+        self.log.info("Testing that unsolicited cmpctblock messages are ignored...")
+        self.test_unsolicited_compact_blocks_ignored()
+
 
 if __name__ == '__main__':
     CompactBlocksTest(__file__).main()

test/functional/p2p_mutated_blocks.py

@@ -14,6 +14,7 @@ from test_framework.messages import (
     msg_cmpctblock,
     msg_block,
     msg_blocktxn,
+    msg_headers,
     HeaderAndShortIDs,
 )
 from test_framework.test_framework import BitcoinTestFramework
@@ -57,6 +58,10 @@ class MutatedBlocksTest(BitcoinTestFramework):
         mutated_block = copy.deepcopy(block)
         mutated_block.vtx[1].version = 4
 
+        # Send block header through the honest relayer
+        honest_relayer.send_without_ping(msg_headers([block]))
+        honest_relayer.wait_for_getdata([block.hash_int], timeout=30)
+
         # Announce the new block via a compact block through the honest relayer
         cmpctblock = HeaderAndShortIDs()
         cmpctblock.initialize_from_block(block, use_witness=True)