From fb194d7319f25f53de168ed1e1b845b23905515d Mon Sep 17 00:00:00 2001 From: Dakoda Greaves Date: Sun, 15 Aug 2021 13:46:11 -0700 Subject: [PATCH 1/2] p2p-acceptblock: add steps 8, 9 to confirm node disconnects --- qa/rpc-tests/p2p-acceptblock.py | 87 ++++++++++++++++++++++++++++++++- 1 file changed, 86 insertions(+), 1 deletion(-) diff --git a/qa/rpc-tests/p2p-acceptblock.py b/qa/rpc-tests/p2p-acceptblock.py index f7ade0696..c6d238d7b 100755 --- a/qa/rpc-tests/p2p-acceptblock.py +++ b/qa/rpc-tests/p2p-acceptblock.py @@ -7,7 +7,7 @@ from test_framework.mininode import * from test_framework.test_framework import BitcoinTestFramework from test_framework.util import * import time -from test_framework.blocktools import create_block, create_coinbase +from test_framework.blocktools import create_block, create_coinbase, create_transaction ''' AcceptBlockTest -- test processing of unrequested blocks. @@ -54,6 +54,14 @@ The test: 7. Send Node0 the missing block again. Node0 should process and the tip should advance. + +8. Create a fork which is invalid at a height longer than the current chain + (ie to which the node will try to reorg) but which has headers built on top + of the invalid block. Check that we get disconnected if we send more headers + on the chain the node now knows to be invalid. + +9. Test Node1 is able to sync when connected to node0 (which should have sufficient + work on its chain). ''' # TestNode: bare-bones "peer". Used mostly as a conduit for a test to sending @@ -271,8 +279,85 @@ class AcceptBlockTest(BitcoinTestFramework): test_node.sync_with_ping() assert_equal(self.nodes[0].getblockcount(), 1442) + self.nodes[0].getblock(all_blocks[1438].hash) + assert_equal(self.nodes[0].getbestblockhash(), all_blocks[1438].hash) + assert_raises_jsonrpc(-1, "Block not found on disk", self.nodes[0].getblock, all_blocks[1439].hash) print("Successfully reorged to longer chain from non-whitelisted peer") + # 8. Create a chain which is invalid at a height longer than the + # current chain, but which has more blocks on top of that + block_1441f = create_block(all_blocks[1436].sha256, create_coinbase(1441), all_blocks[1436].nTime+1) + block_1441f.solve() + block_1442f = create_block(block_1441f.sha256, create_coinbase(1440), block_1441f.nTime+1) + block_1442f.solve() + block_1443 = create_block(block_1442f.sha256, create_coinbase(1441), block_1442f.nTime+1) + # block_1443 spends a coinbase below maturity! + block_1443.vtx.append(create_transaction(block_1442f.vtx[0], 0, b"42", 1)) + block_1443.hashMerkleRoot = block_1443.calc_merkle_root() + block_1443.solve() + block_1444 = create_block(block_1443.sha256, create_coinbase(1444), block_1443.nTime+1) + block_1444.solve() + + # Now send all the headers on the chain and enough blocks to trigger reorg + headers_message = msg_headers() + headers_message.headers.append(CBlockHeader(block_1441f)) + headers_message.headers.append(CBlockHeader(block_1442f)) + headers_message.headers.append(CBlockHeader(block_1443)) + headers_message.headers.append(CBlockHeader(block_1444)) + test_node.send_message(headers_message) + + test_node.sync_with_ping() + tip_entry_found = False + for x in self.nodes[0].getchaintips(): + if x['hash'] == block_1444.hash: + assert_equal(x['status'], "headers-only") + tip_entry_found = True + assert(tip_entry_found) + assert_raises_jsonrpc(-1, "Block not found on disk", self.nodes[0].getblock, block_1444.hash) + + test_node.send_message(msg_block(block_1441f)) + test_node.send_message(msg_block(block_1442f)) + + test_node.sync_with_ping() + self.nodes[0].getblock(block_1441f.hash) + self.nodes[0].getblock(block_1442f.hash) + + test_node.send_message(msg_block(block_1443)) + + # At this point we've sent an obviously-bogus block, wait for full processing + # without assuming whether we will be disconnected or not + try: + # Only wait a short while so the test doesn't take forever if we do get + # disconnected + test_node.sync_with_ping(timeout=1) + except AssertionError: + test_node.wait_for_disconnect() + + test_node = TestNode() # connects to node (not whitelisted) + connections[0] = NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test_node) + test_node.add_connection(connections[0]) + + NetworkThread().start() # Start up network handling in another thread + test_node.wait_for_verack() + + # We should have failed reorg and switched back to 1442 (but have block 1443) + assert_equal(self.nodes[0].getblockcount(), 1442) + assert_equal(self.nodes[0].getbestblockhash(), all_blocks[1438].hash) + assert_equal(self.nodes[0].getblock(block_1443.hash)["confirmations"], -1) + + # Now send a new header on the invalid chain, indicating we're forked off, and expect to get disconnected + block_1445 = create_block(block_1444.sha256, create_coinbase(1445), block_1444.nTime+1) + block_1445.solve() + headers_message = msg_headers() + headers_message.headers.append(CBlockHeader(block_1445)) + test_node.send_message(headers_message) + test_node.wait_for_disconnect() + + # 9. Connect node1 to node0 and ensure it is able to sync + connect_nodes(self.nodes[0], 1) + sync_blocks([self.nodes[0], self.nodes[1]]) + self.log.info("Successfully synced nodes 1 and 0") + [ c.disconnect_node() for c in connections ] if __name__ == '__main__': From caf26b77ab880ecce2ef1eae50cd863ec9d3a52b Mon Sep 17 00:00:00 2001 From: Patrick Lodder Date: Tue, 17 Aug 2021 00:42:58 +0200 Subject: [PATCH 2/2] fixup p2p-acceptblock and mininode to test disconnects without partially backporting a new testframework. - Adds a condition to NodeConn that when asyncore calls handle_read without any data, this must be a disconnect and closes the socket - Adds a little loop in the p2p-acceptblock client that waits for the socket to be in a closed state - Makes expected disconnects non-optional in p2p-acceptblock - Syncs the test descriptions and outputs with reality --- qa/rpc-tests/p2p-acceptblock.py | 45 +++++++++++++++---------- qa/rpc-tests/test_framework/mininode.py | 4 +++ 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/qa/rpc-tests/p2p-acceptblock.py b/qa/rpc-tests/p2p-acceptblock.py index c6d238d7b..d06bcf5a3 100755 --- a/qa/rpc-tests/p2p-acceptblock.py +++ b/qa/rpc-tests/p2p-acceptblock.py @@ -67,13 +67,14 @@ The test: # TestNode: bare-bones "peer". Used mostly as a conduit for a test to sending # p2p messages to a node, generating the messages in the main testing logic. class TestNode(NodeConnCB): - def __init__(self): + def __init__(self, timeout_factor=1): NodeConnCB.__init__(self) self.connection = None self.ping_counter = 1 self.last_pong = msg_pong() def add_connection(self, conn): + self.has_been_disconnected = False self.connection = conn # Track the last getdata message we receive (used in the test) @@ -112,6 +113,17 @@ class TestNode(NodeConnCB): self.ping_counter += 1 return received_pong + # wait for the socket to be in a closed state + def wait_for_disconnect(self, timeout=60): + if self.connection == None: + return True + sleep_time = 0.05 + is_closed = self.connection.state == "closed" + while not is_closed and timeout > 0: + time.sleep(sleep_time) + timeout -= sleep_time + is_closed = self.connection.state == "closed" + return is_closed class AcceptBlockTest(BitcoinTestFramework): def add_options(self, parser): @@ -324,39 +336,36 @@ class AcceptBlockTest(BitcoinTestFramework): test_node.send_message(msg_block(block_1443)) - # At this point we've sent an obviously-bogus block, wait for full processing - # without assuming whether we will be disconnected or not - try: - # Only wait a short while so the test doesn't take forever if we do get - # disconnected - test_node.sync_with_ping(timeout=1) - except AssertionError: - test_node.wait_for_disconnect() + # At this point we've sent an obviously-bogus block, + # and we must get disconnected + assert_equal(test_node.wait_for_disconnect(), True) + print("Successfully got disconnected after sending an invalid block") - test_node = TestNode() # connects to node (not whitelisted) - connections[0] = NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test_node) - test_node.add_connection(connections[0]) - - NetworkThread().start() # Start up network handling in another thread - test_node.wait_for_verack() + # recreate our malicious node + test_node = TestNode() # connects to node (not whitelisted) + connections[0] = NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test_node) + test_node.add_connection(connections[0]) + test_node.wait_for_verack() # We should have failed reorg and switched back to 1442 (but have block 1443) assert_equal(self.nodes[0].getblockcount(), 1442) assert_equal(self.nodes[0].getbestblockhash(), all_blocks[1438].hash) assert_equal(self.nodes[0].getblock(block_1443.hash)["confirmations"], -1) - # Now send a new header on the invalid chain, indicating we're forked off, and expect to get disconnected + # Now send a new header on the invalid chain, indicating we're forked + # off, and expect to get disconnected block_1445 = create_block(block_1444.sha256, create_coinbase(1445), block_1444.nTime+1) block_1445.solve() headers_message = msg_headers() headers_message.headers.append(CBlockHeader(block_1445)) test_node.send_message(headers_message) - test_node.wait_for_disconnect() + assert_equal(test_node.wait_for_disconnect(), True) + print("Successfully got disconnected after building on an invalid block") # 9. Connect node1 to node0 and ensure it is able to sync connect_nodes(self.nodes[0], 1) sync_blocks([self.nodes[0], self.nodes[1]]) - self.log.info("Successfully synced nodes 1 and 0") + print("Successfully synced nodes 1 and 0") [ c.disconnect_node() for c in connections ] diff --git a/qa/rpc-tests/test_framework/mininode.py b/qa/rpc-tests/test_framework/mininode.py index 1520e6754..964e98a1c 100755 --- a/qa/rpc-tests/test_framework/mininode.py +++ b/qa/rpc-tests/test_framework/mininode.py @@ -1684,6 +1684,10 @@ class NodeConn(asyncore.dispatcher): if len(t) > 0: self.recvbuf += t self.got_data() + else: + self.show_debug_msg("MiniNode: Closing connection to %s:%d after peer disconnect..." + % (self.dstaddr, self.dstport)) + self.handle_close() except: pass