Skip to content

Commit e976c36

Browse files
TheBlueMattMarcoFalke
TheBlueMatt
authored and
MarcoFalke
committed
Rewrite p2p-acceptblock in preparation for slight behavior changes
Removes checking whitelisted behavior (which will be removed, the difference in behavior here makes little sense) and no longer requires that blocks at the same work as our tip be dropped if not requested (in part because we *do* request those blocks). Github-Pull: #11531 Rebased-From: 3b4ac43
1 parent ec8dedf commit e976c36

File tree

1 file changed

+122
-114
lines changed

1 file changed

+122
-114
lines changed

‎test/functional/p2p-acceptblock.py‎

Lines changed: 122 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -4,42 +4,32 @@
44
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
55
"""Test processing of unrequested blocks.
66
7-
Since behavior differs when receiving unrequested blocks from whitelisted peers
8-
versus non-whitelisted peers, this tests the behavior of both (effectively two
9-
separate tests running in parallel).
7+
Setup: two nodes, node0+node1, not connected to each other. Node1 will have
8+
nMinimumChainWork set to 0x10, so it won't process low-work unrequested blocks.
109
11-
Setup: three nodes, node0+node1+node2, not connected to each other. Node0 does not
12-
whitelist localhost, but node1 does. They will each be on their own chain for
13-
this test. Node2 will have nMinimumChainWork set to 0x10, so it won't process
14-
low-work unrequested blocks.
15-
16-
We have one NodeConn connection to each, test_node, white_node, and min_work_node,
17-
respectively.
10+
We have one NodeConn connection to node0 called test_node, and one to node1
11+
called min_work_node.
1812
1913
The test:
2014
1. Generate one block on each node, to leave IBD.
2115
2216
2. Mine a new block on each tip, and deliver to each node from node's peer.
23-
The tip should advance for node0 and node1, but node2 should skip processing
24-
due to nMinimumChainWork.
17+
The tip should advance for node0, but node1 should skip processing due to
18+
nMinimumChainWork.
2519
26-
Node2 is unused in tests 3-7:
20+
Node1 is unused in tests 3-7:
2721
28-
3. Mine a block that forks the previous block, and deliver to each node from
29-
corresponding peer.
30-
Node0 should not process this block (just accept the header), because it is
31-
unrequested and doesn't have more work than the tip.
32-
Node1 should process because this is coming from a whitelisted peer.
22+
3. Mine a block that forks from the genesis block, and deliver to test_node.
23+
Node0 should not process this block (just accept the header), because it
24+
is unrequested and doesn't have more or equal work to the tip.
3325
34-
4. Send another block that builds on the forking block.
35-
Node0 should process this block but be stuck on the shorter chain, because
36-
it's missing an intermediate block.
37-
Node1 should reorg to this longer chain.
26+
4a,b. Send another two blocks that build on the forking block.
27+
Node0 should process the second block but be stuck on the shorter chain,
28+
because it's missing an intermediate block.
3829
39-
4b.Send 288 more blocks on the longer chain.
30+
4c.Send 288 more blocks on the longer chain (the number of blocks ahead
31+
we currently store).
4032
Node0 should process all but the last block (too far ahead in height).
41-
Send all headers to Node1, and then send the last block in that chain.
42-
Node1 should accept the block because it's coming from a whitelisted peer.
4333
4434
5. Send a duplicate of the block in #3 to Node0.
4535
Node0 should not process the block because it is unrequested, and stay on
@@ -52,7 +42,7 @@
5242
7. Send Node0 the missing block again.
5343
Node0 should process and the tip should advance.
5444
55-
8. Test Node2 is able to sync when connected to node0 (which should have sufficient
45+
8. Test Node1 is able to sync when connected to node0 (which should have sufficient
5646
work on its chain).
5747
5848
"""
@@ -71,8 +61,8 @@ def add_options(self, parser):
7161

7262
def set_test_params(self):
7363
self.setup_clean_chain = True
74-
self.num_nodes = 3
75-
self.extra_args = [[], ["-whitelist=127.0.0.1"], ["-minimumchainwork=0x10"]]
64+
self.num_nodes = 2
65+
self.extra_args = [[], ["-minimumchainwork=0x10"]]
7666

7767
def setup_network(self):
7868
# Node0 will be used to test behavior of processing unrequested blocks
@@ -84,132 +74,149 @@ def setup_network(self):
8474

8575
def run_test(self):
8676
# Setup the p2p connections and start up the network thread.
87-
test_node = NodeConnCB() # connects to node0 (not whitelisted)
88-
white_node = NodeConnCB() # connects to node1 (whitelisted)
89-
min_work_node = NodeConnCB() # connects to node2 (not whitelisted)
77+
test_node = NodeConnCB() # connects to node0
78+
min_work_node = NodeConnCB() # connects to node1
9079

9180
connections = []
9281
connections.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test_node))
93-
connections.append(NodeConn('127.0.0.1', p2p_port(1), self.nodes[1], white_node))
94-
connections.append(NodeConn('127.0.0.1', p2p_port(2), self.nodes[2], min_work_node))
82+
connections.append(NodeConn('127.0.0.1', p2p_port(1), self.nodes[1], min_work_node))
9583
test_node.add_connection(connections[0])
96-
white_node.add_connection(connections[1])
97-
min_work_node.add_connection(connections[2])
84+
min_work_node.add_connection(connections[1])
9885

9986
NetworkThread().start() # Start up network handling in another thread
10087

10188
# Test logic begins here
10289
test_node.wait_for_verack()
103-
white_node.wait_for_verack()
10490
min_work_node.wait_for_verack()
10591

106-
# 1. Have nodes mine a block (nodes1/2 leave IBD)
92+
# 1. Have nodes mine a block (leave IBD)
10793
[ n.generate(1) for n in self.nodes ]
10894
tips = [ int("0x" + n.getbestblockhash(), 0) for n in self.nodes ]
10995

11096
# 2. Send one block that builds on each tip.
111-
# This should be accepted by nodes 1/2
97+
# This should be accepted by node0
11298
blocks_h2 = [] # the height 2 blocks on each node's chain
11399
block_time = int(time.time()) + 1
114-
for i in range(3):
100+
for i in range(2):
115101
blocks_h2.append(create_block(tips[i], create_coinbase(2), block_time))
116102
blocks_h2[i].solve()
117103
block_time += 1
118104
test_node.send_message(msg_block(blocks_h2[0]))
119-
white_node.send_message(msg_block(blocks_h2[1]))
120-
min_work_node.send_message(msg_block(blocks_h2[2]))
105+
min_work_node.send_message(msg_block(blocks_h2[1]))
121106

122-
for x in [test_node, white_node, min_work_node]:
107+
for x in [test_node, min_work_node]:
123108
x.sync_with_ping()
124109
assert_equal(self.nodes[0].getblockcount(), 2)
125-
assert_equal(self.nodes[1].getblockcount(), 2)
126-
assert_equal(self.nodes[2].getblockcount(), 1)
127-
self.log.info("First height 2 block accepted by node0/node1; correctly rejected by node2")
110+
assert_equal(self.nodes[1].getblockcount(), 1)
111+
self.log.info("First height 2 block accepted by node0; correctly rejected by node1")
128112

129-
# 3. Send another block that builds on the original tip.
130-
blocks_h2f = [] # Blocks at height 2 that fork off the main chain
131-
for i in range(2):
132-
blocks_h2f.append(create_block(tips[i], create_coinbase(2), blocks_h2[i].nTime+1))
133-
blocks_h2f[i].solve()
134-
test_node.send_message(msg_block(blocks_h2f[0]))
135-
white_node.send_message(msg_block(blocks_h2f[1]))
113+
# 3. Send another block that builds on genesis.
114+
block_h1f = create_block(int("0x" + self.nodes[0].getblockhash(0), 0), create_coinbase(1), block_time)
115+
block_time += 1
116+
block_h1f.solve()
117+
test_node.send_message(msg_block(block_h1f))
136118

137-
for x in [test_node, white_node]:
138-
x.sync_with_ping()
119+
test_node.sync_with_ping()
120+
tip_entry_found = False
139121
for x in self.nodes[0].getchaintips():
140-
if x['hash'] == blocks_h2f[0].hash:
122+
if x['hash'] == block_h1f.hash:
141123
assert_equal(x['status'], "headers-only")
124+
tip_entry_found = True
125+
assert(tip_entry_found)
126+
assert_raises_rpc_error(-1, "Block not found on disk", self.nodes[0].getblock, block_h1f.hash)
127+
128+
# 4. Send another two block that build on the fork.
129+
block_h2f = create_block(block_h1f.sha256, create_coinbase(2), block_time)
130+
block_time += 1
131+
block_h2f.solve()
132+
test_node.send_message(msg_block(block_h2f))
142133

143-
for x in self.nodes[1].getchaintips():
144-
if x['hash'] == blocks_h2f[1].hash:
145-
assert_equal(x['status'], "valid-headers")
134+
test_node.sync_with_ping()
135+
# Since the earlier block was not processed by node, the new block
136+
# can't be fully validated.
137+
tip_entry_found = False
138+
for x in self.nodes[0].getchaintips():
139+
if x['hash'] == block_h2f.hash:
140+
assert_equal(x['status'], "headers-only")
141+
tip_entry_found = True
142+
assert(tip_entry_found)
146143

147-
self.log.info("Second height 2 block accepted only from whitelisted peer")
144+
# But this block should be accepted by node since it has equal work.
145+
# TODO: We currently drop this block but likely shouldn't
146+
#self.nodes[0].getblock(block_h2f.hash)
147+
self.log.info("Second height 2 block accepted, but not reorg'ed to")
148148

149-
# 4. Now send another block that builds on the forking chain.
150-
blocks_h3 = []
151-
for i in range(2):
152-
blocks_h3.append(create_block(blocks_h2f[i].sha256, create_coinbase(3), blocks_h2f[i].nTime+1))
153-
blocks_h3[i].solve()
154-
test_node.send_message(msg_block(blocks_h3[0]))
155-
white_node.send_message(msg_block(blocks_h3[1]))
149+
# 4b. Now send another block that builds on the forking chain.
150+
block_h3 = create_block(block_h2f.sha256, create_coinbase(3), block_h2f.nTime+1)
151+
block_h3.solve()
152+
test_node.send_message(msg_block(block_h3))
156153

157-
for x in [test_node, white_node]:
158-
x.sync_with_ping()
159-
# Since the earlier block was not processed by node0, the new block
154+
test_node.sync_with_ping()
155+
# Since the earlier block was not processed by node, the new block
160156
# can't be fully validated.
157+
tip_entry_found = False
161158
for x in self.nodes[0].getchaintips():
162-
if x['hash'] == blocks_h3[0].hash:
159+
if x['hash'] == block_h3.hash:
163160
assert_equal(x['status'], "headers-only")
161+
tip_entry_found = True
162+
assert(tip_entry_found)
163+
self.nodes[0].getblock(block_h3.hash)
164+
165+
# But this block should be accepted by node since it has more work.
166+
self.nodes[0].getblock(block_h3.hash)
167+
self.log.info("Unrequested more-work block accepted")
168+
169+
# 4c. Now mine 288 more blocks and deliver; all should be processed but
170+
# the last (height-too-high) on node (as long as its not missing any headers)
171+
tip = block_h3
172+
all_blocks = []
173+
for i in range(288):
174+
next_block = create_block(tip.sha256, create_coinbase(i + 4), tip.nTime+1)
175+
next_block.solve()
176+
all_blocks.append(next_block)
177+
tip = next_block
178+
179+
# Now send the block at height 5 and check that it wasn't accepted (missing header)
180+
test_node.send_message(msg_block(all_blocks[1]))
181+
test_node.sync_with_ping()
182+
assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getblock, all_blocks[1].hash)
183+
assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getblockheader, all_blocks[1].hash)
164184

165-
# But this block should be accepted by node0 since it has more work.
166-
self.nodes[0].getblock(blocks_h3[0].hash)
167-
self.log.info("Unrequested more-work block accepted from non-whitelisted peer")
185+
# The block at height 5 should be accepted if we provide the missing header, though
186+
headers_message = msg_headers()
187+
headers_message.headers.append(CBlockHeader(all_blocks[0]))
188+
test_node.send_message(headers_message)
189+
test_node.send_message(msg_block(all_blocks[1]))
190+
test_node.sync_with_ping()
191+
self.nodes[0].getblock(all_blocks[1].hash)
168192

169-
# Node1 should have accepted and reorged.
170-
assert_equal(self.nodes[1].getblockcount(), 3)
171-
self.log.info("Successfully reorged to length 3 chain from whitelisted peer")
193+
# Now send the blocks in all_blocks
194+
for i in range(288):
195+
test_node.send_message(msg_block(all_blocks[i]))
196+
test_node.sync_with_ping()
172197

173-
# 4b. Now mine 288 more blocks and deliver; all should be processed but
174-
# the last (height-too-high) on node0. Node1 should process the tip if
175-
# we give it the headers chain leading to the tip.
176-
tips = blocks_h3
177-
headers_message = msg_headers()
178-
all_blocks = [] # node0's blocks
179-
for j in range(2):
180-
for i in range(288):
181-
next_block = create_block(tips[j].sha256, create_coinbase(i + 4), tips[j].nTime+1)
182-
next_block.solve()
183-
if j==0:
184-
test_node.send_message(msg_block(next_block))
185-
all_blocks.append(next_block)
186-
else:
187-
headers_message.headers.append(CBlockHeader(next_block))
188-
tips[j] = next_block
189-
190-
time.sleep(2)
191198
# Blocks 1-287 should be accepted, block 288 should be ignored because it's too far ahead
192199
for x in all_blocks[:-1]:
193200
self.nodes[0].getblock(x.hash)
194201
assert_raises_rpc_error(-1, "Block not found on disk", self.nodes[0].getblock, all_blocks[-1].hash)
195202

196-
headers_message.headers.pop() # Ensure the last block is unrequested
197-
white_node.send_message(headers_message) # Send headers leading to tip
198-
white_node.send_message(msg_block(tips[1])) # Now deliver the tip
199-
white_node.sync_with_ping()
200-
self.nodes[1].getblock(tips[1].hash)
201-
self.log.info("Unrequested block far ahead of tip accepted from whitelisted peer")
202-
203203
# 5. Test handling of unrequested block on the node that didn't process
204204
# Should still not be processed (even though it has a child that has more
205205
# work).
206-
test_node.send_message(msg_block(blocks_h2f[0]))
207206

208-
# Here, if the sleep is too short, the test could falsely succeed (if the
209-
# node hasn't processed the block by the time the sleep returns, and then
210-
# the node processes it and incorrectly advances the tip).
211-
# But this would be caught later on, when we verify that an inv triggers
212-
# a getdata request for this block.
207+
# The node should have requested the blocks at some point, so
208+
# disconnect/reconnect first
209+
connections[0].disconnect_node()
210+
test_node.wait_for_disconnect()
211+
212+
test_node = NodeConnCB() # connects to node (not whitelisted)
213+
connections[0] = NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test_node)
214+
test_node.add_connection(connections[0])
215+
216+
test_node.wait_for_verack()
217+
test_node.send_message(msg_block(block_h1f))
218+
test_node.send_message(msg_block(block_h2f)) # This should not be required
219+
213220
test_node.sync_with_ping()
214221
assert_equal(self.nodes[0].getblockcount(), 2)
215222
self.log.info("Unrequested block that would complete more-work chain was ignored")
@@ -220,27 +227,28 @@ def run_test(self):
220227
with mininode_lock:
221228
# Clear state so we can check the getdata request
222229
test_node.last_message.pop("getdata", None)
223-
test_node.send_message(msg_inv([CInv(2, blocks_h3[0].sha256)]))
230+
test_node.send_message(msg_inv([CInv(2, block_h3.sha256)]))
224231

225232
test_node.sync_with_ping()
226233
with mininode_lock:
227234
getdata = test_node.last_message["getdata"]
228235

229236
# Check that the getdata includes the right block
230-
assert_equal(getdata.inv[0].hash, blocks_h2f[0].sha256)
237+
assert_equal(getdata.inv[0].hash, block_h1f.sha256)
231238
self.log.info("Inv at tip triggered getdata for unprocessed block")
232239

233240
# 7. Send the missing block for the third time (now it is requested)
234-
test_node.send_message(msg_block(blocks_h2f[0]))
241+
test_node.send_message(msg_block(block_h1f))
242+
test_node.send_message(msg_block(block_h2f)) # This should not be required
235243

236244
test_node.sync_with_ping()
237245
assert_equal(self.nodes[0].getblockcount(), 290)
238246
self.log.info("Successfully reorged to longer chain from non-whitelisted peer")
239247

240-
# 8. Connect node2 to node0 and ensure it is able to sync
241-
connect_nodes(self.nodes[0], 2)
242-
sync_blocks([self.nodes[0], self.nodes[2]])
243-
self.log.info("Successfully synced nodes 2 and 0")
248+
# 8. Connect node1 to node0 and ensure it is able to sync
249+
connect_nodes(self.nodes[0], 1)
250+
sync_blocks([self.nodes[0], self.nodes[1]])
251+
self.log.info("Successfully synced nodes 1 and 0")
244252

245253
[ c.disconnect_node() for c in connections ]
246254

0 commit comments

Comments
 (0)