From 0dc8bf5b925ca876c0b1a100e426056d741aafde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20G=C3=B6gge?= Date: Fri, 25 Jun 2021 14:26:57 +0200 Subject: [PATCH 1/5] [net processing] Dont request compact blocks in blocks-only mode --- src/net_processing.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index e130272ff1e..008b4d679c6 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -884,6 +884,12 @@ bool PeerManagerImpl::BlockRequested(NodeId nodeid, const CBlockIndex& block, st void PeerManagerImpl::MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid) { AssertLockHeld(cs_main); + + // Never request high-bandwidth mode from peers if we're blocks-only. Our + // mempool will not contain the transactions necessary to reconstruct the + // compact block. + if (m_ignore_incoming_txs) return; + CNodeState* nodestate = State(nodeid); if (!nodestate || !nodestate->fSupportsDesiredCmpctVersion) { // Never ask from peers who can't provide witnesses. @@ -2165,7 +2171,11 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, const Peer& peer, pindexLast->GetBlockHash().ToString(), pindexLast->nHeight); } if (vGetData.size() > 0) { - if (nodestate->fSupportsDesiredCmpctVersion && vGetData.size() == 1 && mapBlocksInFlight.size() == 1 && pindexLast->pprev->IsValid(BLOCK_VALID_CHAIN)) { + if (!m_ignore_incoming_txs && + nodestate->fSupportsDesiredCmpctVersion && + vGetData.size() == 1 && + mapBlocksInFlight.size() == 1 && + pindexLast->pprev->IsValid(BLOCK_VALID_CHAIN)) { // In any case, we want to download using a compact block, not a regular one vGetData[0] = CInv(MSG_CMPCT_BLOCK, vGetData[0].hash); } From 5bf658745782eb571215d6c1e5fe3c655edb55b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20G=C3=B6gge?= Date: Sun, 8 Aug 2021 10:07:58 +0200 Subject: [PATCH 2/5] [test] Test that -blocksonly nodes do not request high bandwidth mode. Co-authored-by: Amiti Uttarwar --- .../p2p_compactblocks_blocksonly.py | 84 +++++++++++++++++++ test/functional/test_runner.py | 1 + 2 files changed, 85 insertions(+) create mode 100755 test/functional/p2p_compactblocks_blocksonly.py diff --git a/test/functional/p2p_compactblocks_blocksonly.py b/test/functional/p2p_compactblocks_blocksonly.py new file mode 100755 index 00000000000..b59a81d3ec4 --- /dev/null +++ b/test/functional/p2p_compactblocks_blocksonly.py @@ -0,0 +1,84 @@ +#!/usr/bin/env python3 +# Copyright (c) 2021-2021 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test that a node in blocksonly mode does not request compact blocks.""" + +from test_framework.messages import ( + MSG_BLOCK, + MSG_CMPCT_BLOCK, + MSG_WITNESS_FLAG, + CBlock, + CBlockHeader, + CInv, + from_hex, + msg_block, + msg_headers, + msg_sendcmpct, +) +from test_framework.p2p import P2PInterface +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal + + +class P2PCompactBlocksBlocksOnly(BitcoinTestFramework): + def set_test_params(self): + self.extra_args = [["-blocksonly"], [], []] + self.num_nodes = 3 + + def setup_network(self): + self.setup_nodes() + # Start network with everyone disconnected + self.sync_all() + + def build_block_on_tip(self): + blockhash = self.nodes[2].generate(1)[0] + block_hex = self.nodes[2].getblock(blockhash=blockhash, verbosity=0) + block = from_hex(CBlock(), block_hex) + block.rehash() + return block + + def run_test(self): + # Nodes will only request hb compact blocks mode when they're out of IBD + for node in self.nodes: + assert not node.getblockchaininfo()['initialblockdownload'] + + p2p_conn_blocksonly = self.nodes[0].add_p2p_connection(P2PInterface()) + p2p_conn_high_bw = self.nodes[1].add_p2p_connection(P2PInterface()) + for conn in [p2p_conn_blocksonly, p2p_conn_high_bw]: + assert_equal(conn.message_count['sendcmpct'], 2) + conn.send_and_ping(msg_sendcmpct(announce=False, version=2)) + + # Nodes: + # 0 -> blocksonly + # 1 -> high bandwidth + # 2 -> miner + # + # Topology: + # p2p_conn_blocksonly ---> node0 + # p2p_conn_high_bw ---> node1 + # node2 (no connections) + # + # node2 produces blocks that are passed to the rest of the nodes + # through the respective p2p connections. + + self.log.info("Test that -blocksonly nodes do not select peers for BIP152 high bandwidth mode") + + block0 = self.build_block_on_tip() + + # A -blocksonly node should not request BIP152 high bandwidth mode upon + # receiving a new valid block at the tip. + p2p_conn_blocksonly.send_and_ping(msg_block(block0)) + assert_equal(int(self.nodes[0].getbestblockhash(), 16), block0.sha256) + assert_equal(p2p_conn_blocksonly.message_count['sendcmpct'], 2) + assert_equal(p2p_conn_blocksonly.last_message['sendcmpct'].announce, False) + + # A normal node participating in transaction relay should request BIP152 + # high bandwidth mode upon receiving a new valid block at the tip. + p2p_conn_high_bw.send_and_ping(msg_block(block0)) + assert_equal(int(self.nodes[1].getbestblockhash(), 16), block0.sha256) + p2p_conn_high_bw.wait_until(lambda: p2p_conn_high_bw.message_count['sendcmpct'] == 3) + assert_equal(p2p_conn_high_bw.last_message['sendcmpct'].announce, True) + +if __name__ == '__main__': + P2PCompactBlocksBlocksOnly().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index b463f058696..6f7f757a3be 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -98,6 +98,7 @@ BASE_SCRIPTS = [ 'rpc_fundrawtransaction.py --legacy-wallet', 'rpc_fundrawtransaction.py --descriptors', 'p2p_compactblocks.py', + 'p2p_compactblocks_blocksonly.py', 'feature_segwit.py --legacy-wallet', # vv Tests less than 2m vv 'wallet_basic.py --legacy-wallet', From 5e231c116ba8165e9d8c795b85ca2833238aed54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20G=C3=B6gge?= Date: Sun, 8 Aug 2021 10:08:52 +0200 Subject: [PATCH 3/5] [test] Test that -blocksonly nodes do not send getdata(CMPCT) on a low bandwidth connection. Co-authored-by: Amiti Uttarwar --- test/functional/p2p_compactblocks_blocksonly.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/functional/p2p_compactblocks_blocksonly.py b/test/functional/p2p_compactblocks_blocksonly.py index b59a81d3ec4..f690fce0d65 100755 --- a/test/functional/p2p_compactblocks_blocksonly.py +++ b/test/functional/p2p_compactblocks_blocksonly.py @@ -80,5 +80,18 @@ class P2PCompactBlocksBlocksOnly(BitcoinTestFramework): p2p_conn_high_bw.wait_until(lambda: p2p_conn_high_bw.message_count['sendcmpct'] == 3) assert_equal(p2p_conn_high_bw.last_message['sendcmpct'].announce, True) + self.log.info("Test that -blocksonly nodes send getdata(BLOCK) instead" + " of getdata(CMPCT) in BIP152 low bandwidth mode") + + block1 = self.build_block_on_tip() + + p2p_conn_blocksonly.send_message(msg_headers(headers=[CBlockHeader(block1)])) + p2p_conn_blocksonly.sync_send_with_ping() + assert_equal(p2p_conn_blocksonly.last_message['getdata'].inv, [CInv(MSG_BLOCK | MSG_WITNESS_FLAG, block1.sha256)]) + + p2p_conn_high_bw.send_message(msg_headers(headers=[CBlockHeader(block1)])) + p2p_conn_high_bw.sync_send_with_ping() + assert_equal(p2p_conn_high_bw.last_message['getdata'].inv, [CInv(MSG_CMPCT_BLOCK, block1.sha256)]) + if __name__ == '__main__': P2PCompactBlocksBlocksOnly().main() From a79ad65fc2c31a2b1132a6aab389d0197a95c395 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20G=C3=B6gge?= Date: Mon, 9 Aug 2021 21:17:57 +0200 Subject: [PATCH 4/5] [test] Test that getdata(CMPCT) is still sent on regular low bandwidth connections. Co-authored-by: Amiti Uttarwar --- .../p2p_compactblocks_blocksonly.py | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/test/functional/p2p_compactblocks_blocksonly.py b/test/functional/p2p_compactblocks_blocksonly.py index f690fce0d65..93164d30344 100755 --- a/test/functional/p2p_compactblocks_blocksonly.py +++ b/test/functional/p2p_compactblocks_blocksonly.py @@ -23,8 +23,8 @@ from test_framework.util import assert_equal class P2PCompactBlocksBlocksOnly(BitcoinTestFramework): def set_test_params(self): - self.extra_args = [["-blocksonly"], [], []] - self.num_nodes = 3 + self.extra_args = [["-blocksonly"], [], [], []] + self.num_nodes = 4 def setup_network(self): self.setup_nodes() @@ -45,7 +45,8 @@ class P2PCompactBlocksBlocksOnly(BitcoinTestFramework): p2p_conn_blocksonly = self.nodes[0].add_p2p_connection(P2PInterface()) p2p_conn_high_bw = self.nodes[1].add_p2p_connection(P2PInterface()) - for conn in [p2p_conn_blocksonly, p2p_conn_high_bw]: + p2p_conn_low_bw = self.nodes[3].add_p2p_connection(P2PInterface()) + for conn in [p2p_conn_blocksonly, p2p_conn_high_bw, p2p_conn_low_bw]: assert_equal(conn.message_count['sendcmpct'], 2) conn.send_and_ping(msg_sendcmpct(announce=False, version=2)) @@ -53,10 +54,12 @@ class P2PCompactBlocksBlocksOnly(BitcoinTestFramework): # 0 -> blocksonly # 1 -> high bandwidth # 2 -> miner + # 3 -> low bandwidth # # Topology: # p2p_conn_blocksonly ---> node0 # p2p_conn_high_bw ---> node1 + # p2p_conn_low_bw ---> node3 # node2 (no connections) # # node2 produces blocks that are passed to the rest of the nodes @@ -80,6 +83,10 @@ class P2PCompactBlocksBlocksOnly(BitcoinTestFramework): p2p_conn_high_bw.wait_until(lambda: p2p_conn_high_bw.message_count['sendcmpct'] == 3) assert_equal(p2p_conn_high_bw.last_message['sendcmpct'].announce, True) + # Don't send a block from the p2p_conn_low_bw so the low bandwidth node + # doesn't select it for BIP152 high bandwidth relay. + self.nodes[3].submitblock(block0.serialize().hex()) + self.log.info("Test that -blocksonly nodes send getdata(BLOCK) instead" " of getdata(CMPCT) in BIP152 low bandwidth mode") @@ -93,5 +100,12 @@ class P2PCompactBlocksBlocksOnly(BitcoinTestFramework): p2p_conn_high_bw.sync_send_with_ping() assert_equal(p2p_conn_high_bw.last_message['getdata'].inv, [CInv(MSG_CMPCT_BLOCK, block1.sha256)]) + self.log.info("Test that getdata(CMPCT) is still sent on BIP152 low bandwidth connections" + " when no -blocksonly nodes are involved") + + p2p_conn_low_bw.send_and_ping(msg_headers(headers=[CBlockHeader(block1)])) + p2p_conn_low_bw.sync_with_ping() + assert_equal(p2p_conn_low_bw.last_message['getdata'].inv, [CInv(MSG_CMPCT_BLOCK, block1.sha256)]) + if __name__ == '__main__': P2PCompactBlocksBlocksOnly().main() From 18c5b23a0f7adf9a08bcaa967ed800badf62d90a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20G=C3=B6gge?= Date: Mon, 9 Aug 2021 21:54:02 +0200 Subject: [PATCH 5/5] [test] Test that -blocksonly nodes still serve compact blocks. --- .../p2p_compactblocks_blocksonly.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/functional/p2p_compactblocks_blocksonly.py b/test/functional/p2p_compactblocks_blocksonly.py index 93164d30344..4073ec03a69 100755 --- a/test/functional/p2p_compactblocks_blocksonly.py +++ b/test/functional/p2p_compactblocks_blocksonly.py @@ -13,6 +13,7 @@ from test_framework.messages import ( CInv, from_hex, msg_block, + msg_getdata, msg_headers, msg_sendcmpct, ) @@ -107,5 +108,23 @@ class P2PCompactBlocksBlocksOnly(BitcoinTestFramework): p2p_conn_low_bw.sync_with_ping() assert_equal(p2p_conn_low_bw.last_message['getdata'].inv, [CInv(MSG_CMPCT_BLOCK, block1.sha256)]) + self.log.info("Test that -blocksonly nodes still serve compact blocks") + + def test_for_cmpctblock(block): + if 'cmpctblock' not in p2p_conn_blocksonly.last_message: + return False + return p2p_conn_blocksonly.last_message['cmpctblock'].header_and_shortids.header.rehash() == block.sha256 + + p2p_conn_blocksonly.send_message(msg_getdata([CInv(MSG_CMPCT_BLOCK, block0.sha256)])) + p2p_conn_blocksonly.wait_until(lambda: test_for_cmpctblock(block0)) + + # Request BIP152 high bandwidth mode from the -blocksonly node. + p2p_conn_blocksonly.send_and_ping(msg_sendcmpct(announce=True, version=2)) + + block2 = self.build_block_on_tip() + self.nodes[0].submitblock(block1.serialize().hex()) + self.nodes[0].submitblock(block2.serialize().hex()) + p2p_conn_blocksonly.wait_until(lambda: test_for_cmpctblock(block2)) + if __name__ == '__main__': P2PCompactBlocksBlocksOnly().main()