Skip to content

Commit 4e1de1f

Browse files
author
MarcoFalke
committed
Merge bitcoin/bitcoin#22340: p2p: Use legacy relaying to download blocks in blocks-only mode
18c5b23 [test] Test that -blocksonly nodes still serve compact blocks. (Niklas Gögge) a79ad65 [test] Test that getdata(CMPCT) is still sent on regular low bandwidth connections. (Niklas Gögge) 5e231c1 [test] Test that -blocksonly nodes do not send getdata(CMPCT) on a low bandwidth connection. (Niklas Gögge) 5bf6587 [test] Test that -blocksonly nodes do not request high bandwidth mode. (Niklas Gögge) 0dc8bf5 [net processing] Dont request compact blocks in blocks-only mode (Niklas Gögge) Pull request description: A blocks-only node does not participate in transaction relay to reduce its own bandwidth usage and therefore does not have a mempool. The use of compact blocks is not beneficial to such a node since it will always have to download full blocks. In both high- and low-bandwidth relaying the `cmpctblock` message is sent. This represent a bandwidth overhead for blocks-only nodes because the `cmpctblock` message is several times larger in the average case than the equivalent `headers` or `inv` announcement. ![compact blocks](https://raw.githubusercontent.com/bitcoin/bips/master/bip-0152/protocol-flow.png) >**Example:** >A block with 2000 txs results in a `cmpctblock` with 2000*6 bytes in short ids. This is several times larger than the equivalent 82 bytes for a `headers` message or 37 bytes for an `inv`. ## Approach This PR makes blocks-only nodes always use the legacy relaying to download new blocks. It does so by making blocks-only nodes never initiate a high-bandwidth block relay connection by disabling the sending of `sendcmpct(1)`. Additionally a blocks-only node will never request a compact block using `getdata(CMPCT)`. A blocks-only node will continue to serve compact blocks to its peers in both high- and low-bandwidth mode. ACKs for top commit: naumenkogs: ACK 18c5b23 rajarshimaitra: tACK bitcoin/bitcoin@18c5b23 jnewbery: reACK 18c5b23 theStack: re-ACK 18c5b23 🥛 Tree-SHA512: 0c78804aa397513d41f97fe314efb815efcd852d452dd903df9d4749280cd3faaa010fa9b51d7d5168b8a77e08c8ab0a491ecdbdb3202f2e9cd5137cddc74624
2 parents 33b0696 + 18c5b23 commit 4e1de1f

File tree

3 files changed

+142
-1
lines changed

3 files changed

+142
-1
lines changed

src/net_processing.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -884,6 +884,12 @@ bool PeerManagerImpl::BlockRequested(NodeId nodeid, const CBlockIndex& block, st
884884
void PeerManagerImpl::MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid)
885885
{
886886
AssertLockHeld(cs_main);
887+
888+
// Never request high-bandwidth mode from peers if we're blocks-only. Our
889+
// mempool will not contain the transactions necessary to reconstruct the
890+
// compact block.
891+
if (m_ignore_incoming_txs) return;
892+
887893
CNodeState* nodestate = State(nodeid);
888894
if (!nodestate || !nodestate->fSupportsDesiredCmpctVersion) {
889895
// Never ask from peers who can't provide witnesses.
@@ -2165,7 +2171,11 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, const Peer& peer,
21652171
pindexLast->GetBlockHash().ToString(), pindexLast->nHeight);
21662172
}
21672173
if (vGetData.size() > 0) {
2168-
if (nodestate->fSupportsDesiredCmpctVersion && vGetData.size() == 1 && mapBlocksInFlight.size() == 1 && pindexLast->pprev->IsValid(BLOCK_VALID_CHAIN)) {
2174+
if (!m_ignore_incoming_txs &&
2175+
nodestate->fSupportsDesiredCmpctVersion &&
2176+
vGetData.size() == 1 &&
2177+
mapBlocksInFlight.size() == 1 &&
2178+
pindexLast->pprev->IsValid(BLOCK_VALID_CHAIN)) {
21692179
// In any case, we want to download using a compact block, not a regular one
21702180
vGetData[0] = CInv(MSG_CMPCT_BLOCK, vGetData[0].hash);
21712181
}
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
#!/usr/bin/env python3
2+
# Copyright (c) 2021-2021 The Bitcoin Core developers
3+
# Distributed under the MIT software license, see the accompanying
4+
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
5+
"""Test that a node in blocksonly mode does not request compact blocks."""
6+
7+
from test_framework.messages import (
8+
MSG_BLOCK,
9+
MSG_CMPCT_BLOCK,
10+
MSG_WITNESS_FLAG,
11+
CBlock,
12+
CBlockHeader,
13+
CInv,
14+
from_hex,
15+
msg_block,
16+
msg_getdata,
17+
msg_headers,
18+
msg_sendcmpct,
19+
)
20+
from test_framework.p2p import P2PInterface
21+
from test_framework.test_framework import BitcoinTestFramework
22+
from test_framework.util import assert_equal
23+
24+
25+
class P2PCompactBlocksBlocksOnly(BitcoinTestFramework):
26+
def set_test_params(self):
27+
self.extra_args = [["-blocksonly"], [], [], []]
28+
self.num_nodes = 4
29+
30+
def setup_network(self):
31+
self.setup_nodes()
32+
# Start network with everyone disconnected
33+
self.sync_all()
34+
35+
def build_block_on_tip(self):
36+
blockhash = self.nodes[2].generate(1)[0]
37+
block_hex = self.nodes[2].getblock(blockhash=blockhash, verbosity=0)
38+
block = from_hex(CBlock(), block_hex)
39+
block.rehash()
40+
return block
41+
42+
def run_test(self):
43+
# Nodes will only request hb compact blocks mode when they're out of IBD
44+
for node in self.nodes:
45+
assert not node.getblockchaininfo()['initialblockdownload']
46+
47+
p2p_conn_blocksonly = self.nodes[0].add_p2p_connection(P2PInterface())
48+
p2p_conn_high_bw = self.nodes[1].add_p2p_connection(P2PInterface())
49+
p2p_conn_low_bw = self.nodes[3].add_p2p_connection(P2PInterface())
50+
for conn in [p2p_conn_blocksonly, p2p_conn_high_bw, p2p_conn_low_bw]:
51+
assert_equal(conn.message_count['sendcmpct'], 2)
52+
conn.send_and_ping(msg_sendcmpct(announce=False, version=2))
53+
54+
# Nodes:
55+
# 0 -> blocksonly
56+
# 1 -> high bandwidth
57+
# 2 -> miner
58+
# 3 -> low bandwidth
59+
#
60+
# Topology:
61+
# p2p_conn_blocksonly ---> node0
62+
# p2p_conn_high_bw ---> node1
63+
# p2p_conn_low_bw ---> node3
64+
# node2 (no connections)
65+
#
66+
# node2 produces blocks that are passed to the rest of the nodes
67+
# through the respective p2p connections.
68+
69+
self.log.info("Test that -blocksonly nodes do not select peers for BIP152 high bandwidth mode")
70+
71+
block0 = self.build_block_on_tip()
72+
73+
# A -blocksonly node should not request BIP152 high bandwidth mode upon
74+
# receiving a new valid block at the tip.
75+
p2p_conn_blocksonly.send_and_ping(msg_block(block0))
76+
assert_equal(int(self.nodes[0].getbestblockhash(), 16), block0.sha256)
77+
assert_equal(p2p_conn_blocksonly.message_count['sendcmpct'], 2)
78+
assert_equal(p2p_conn_blocksonly.last_message['sendcmpct'].announce, False)
79+
80+
# A normal node participating in transaction relay should request BIP152
81+
# high bandwidth mode upon receiving a new valid block at the tip.
82+
p2p_conn_high_bw.send_and_ping(msg_block(block0))
83+
assert_equal(int(self.nodes[1].getbestblockhash(), 16), block0.sha256)
84+
p2p_conn_high_bw.wait_until(lambda: p2p_conn_high_bw.message_count['sendcmpct'] == 3)
85+
assert_equal(p2p_conn_high_bw.last_message['sendcmpct'].announce, True)
86+
87+
# Don't send a block from the p2p_conn_low_bw so the low bandwidth node
88+
# doesn't select it for BIP152 high bandwidth relay.
89+
self.nodes[3].submitblock(block0.serialize().hex())
90+
91+
self.log.info("Test that -blocksonly nodes send getdata(BLOCK) instead"
92+
" of getdata(CMPCT) in BIP152 low bandwidth mode")
93+
94+
block1 = self.build_block_on_tip()
95+
96+
p2p_conn_blocksonly.send_message(msg_headers(headers=[CBlockHeader(block1)]))
97+
p2p_conn_blocksonly.sync_send_with_ping()
98+
assert_equal(p2p_conn_blocksonly.last_message['getdata'].inv, [CInv(MSG_BLOCK | MSG_WITNESS_FLAG, block1.sha256)])
99+
100+
p2p_conn_high_bw.send_message(msg_headers(headers=[CBlockHeader(block1)]))
101+
p2p_conn_high_bw.sync_send_with_ping()
102+
assert_equal(p2p_conn_high_bw.last_message['getdata'].inv, [CInv(MSG_CMPCT_BLOCK, block1.sha256)])
103+
104+
self.log.info("Test that getdata(CMPCT) is still sent on BIP152 low bandwidth connections"
105+
" when no -blocksonly nodes are involved")
106+
107+
p2p_conn_low_bw.send_and_ping(msg_headers(headers=[CBlockHeader(block1)]))
108+
p2p_conn_low_bw.sync_with_ping()
109+
assert_equal(p2p_conn_low_bw.last_message['getdata'].inv, [CInv(MSG_CMPCT_BLOCK, block1.sha256)])
110+
111+
self.log.info("Test that -blocksonly nodes still serve compact blocks")
112+
113+
def test_for_cmpctblock(block):
114+
if 'cmpctblock' not in p2p_conn_blocksonly.last_message:
115+
return False
116+
return p2p_conn_blocksonly.last_message['cmpctblock'].header_and_shortids.header.rehash() == block.sha256
117+
118+
p2p_conn_blocksonly.send_message(msg_getdata([CInv(MSG_CMPCT_BLOCK, block0.sha256)]))
119+
p2p_conn_blocksonly.wait_until(lambda: test_for_cmpctblock(block0))
120+
121+
# Request BIP152 high bandwidth mode from the -blocksonly node.
122+
p2p_conn_blocksonly.send_and_ping(msg_sendcmpct(announce=True, version=2))
123+
124+
block2 = self.build_block_on_tip()
125+
self.nodes[0].submitblock(block1.serialize().hex())
126+
self.nodes[0].submitblock(block2.serialize().hex())
127+
p2p_conn_blocksonly.wait_until(lambda: test_for_cmpctblock(block2))
128+
129+
if __name__ == '__main__':
130+
P2PCompactBlocksBlocksOnly().main()

test/functional/test_runner.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@
9898
'rpc_fundrawtransaction.py --legacy-wallet',
9999
'rpc_fundrawtransaction.py --descriptors',
100100
'p2p_compactblocks.py',
101+
'p2p_compactblocks_blocksonly.py',
101102
'feature_segwit.py --legacy-wallet',
102103
# vv Tests less than 2m vv
103104
'wallet_basic.py --legacy-wallet',

0 commit comments

Comments
 (0)