From 12853120484e19bbd9cf90574d5eaa9cb46255a5 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Mon, 11 Dec 2017 11:55:54 -0500 Subject: [PATCH 1/5] [tests] fix flake8 warnings in node_network_limited.py --- test/functional/node_network_limited.py | 47 +++++++++++++------------ 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/test/functional/node_network_limited.py b/test/functional/node_network_limited.py index 40b876243..5229dfc99 100755 --- a/test/functional/node_network_limited.py +++ b/test/functional/node_network_limited.py @@ -2,9 +2,10 @@ # Copyright (c) 2017 The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. +from test_framework.messages import CInv, msg_getdata, msg_verack +from test_framework.mininode import NetworkThread, P2PInterface from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import * -from test_framework.mininode import * +from test_framework.util import assert_equal class BaseNode(P2PInterface): nServices = 0 @@ -17,7 +18,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): self.num_nodes = 1 self.extra_args = [['-prune=550']] - def getSignaledServiceFlags(self): + def get_signalled_service_flags(self): node = self.nodes[0].add_p2p_connection(BaseNode()) NetworkThread().start() node.wait_for_verack() @@ -26,7 +27,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): node.wait_for_disconnect() return services - def tryGetBlockViaGetData(self, blockhash, must_disconnect): + def try_get_block_via_getdata(self, blockhash, must_disconnect): node = self.nodes[0].add_p2p_connection(BaseNode()) NetworkThread().start() node.wait_for_verack() @@ -36,7 +37,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): node.send_message(getdata_request) if (must_disconnect): - #ensure we get disconnected + # Ensure we get disconnected node.wait_for_disconnect(5) else: # check if the peer sends us the requested block @@ -45,36 +46,36 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): node.wait_for_disconnect() def run_test(self): - #NODE_BLOOM & NODE_WITNESS & NODE_NETWORK_LIMITED must now be signaled - assert_equal(self.getSignaledServiceFlags(), 1036) #1036 == 0x40C == 0100 0000 1100 -# | || -# | |^--- NODE_BLOOM -# | ^---- NODE_WITNESS -# ^-- NODE_NETWORK_LIMITED + # NODE_BLOOM & NODE_WITNESS & NODE_NETWORK_LIMITED must now be signaled + assert_equal(self.get_signalled_service_flags(), 1036) # 1036 == 0x40C == 0100 0000 1100 +# | || +# | |^--- NODE_BLOOM +# | ^---- NODE_WITNESS +# ^-- NODE_NETWORK_LIMITED - #now mine some blocks over the NODE_NETWORK_LIMITED + 2(racy buffer ext.) target + # Now mine some blocks over the NODE_NETWORK_LIMITED + 2(racy buffer ext.) target firstblock = self.nodes[0].generate(1)[0] blocks = self.nodes[0].generate(292) - blockWithinLimitedRange = blocks[-1] + block_within_limited_range = blocks[-1] - #make sure we can max retrive block at tip-288 - #requesting block at height 2 (tip-289) must fail (ignored) - self.tryGetBlockViaGetData(firstblock, True) #first block must lead to disconnect - self.tryGetBlockViaGetData(blocks[1], False) #last block in valid range - self.tryGetBlockViaGetData(blocks[0], True) #first block outside of the 288+2 limit + # Make sure we can max retrive block at tip-288 + # requesting block at height 2 (tip-289) must fail (ignored) + self.try_get_block_via_getdata(firstblock, True) # first block must lead to disconnect + self.try_get_block_via_getdata(blocks[1], False) # last block in valid range + self.try_get_block_via_getdata(blocks[0], True) # first block outside of the 288+2 limit - #NODE_NETWORK_LIMITED must still be signaled after restart + # NODE_NETWORK_LIMITED must still be signaled after restart self.restart_node(0) - assert_equal(self.getSignaledServiceFlags(), 1036) + assert_equal(self.get_signalled_service_flags(), 1036) - #test the RPC service flags + # Test the RPC service flags assert_equal(self.nodes[0].getnetworkinfo()['localservices'], "000000000000040c") # getdata a block above the NODE_NETWORK_LIMITED threshold must be possible - self.tryGetBlockViaGetData(blockWithinLimitedRange, False) + self.try_get_block_via_getdata(block_within_limited_range, False) # getdata a block below the NODE_NETWORK_LIMITED threshold must be ignored - self.tryGetBlockViaGetData(firstblock, True) + self.try_get_block_via_getdata(firstblock, True) if __name__ == '__main__': NodeNetworkLimitedTest().main() From dbfe294805a094ba4ed6178a56d0a7588fcd8c27 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Mon, 11 Dec 2017 11:56:24 -0500 Subject: [PATCH 2/5] [tests] define NODE_NETWORK_LIMITED in test framework --- test/functional/node_network_limited.py | 12 ++++-------- test/functional/test_framework/messages.py | 3 ++- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/test/functional/node_network_limited.py b/test/functional/node_network_limited.py index 5229dfc99..1e7c61b44 100755 --- a/test/functional/node_network_limited.py +++ b/test/functional/node_network_limited.py @@ -3,7 +3,7 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. from test_framework.messages import CInv, msg_getdata, msg_verack -from test_framework.mininode import NetworkThread, P2PInterface +from test_framework.mininode import NODE_BLOOM, NODE_NETWORK_LIMITED, NODE_WITNESS, NetworkThread, P2PInterface from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal @@ -47,11 +47,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): def run_test(self): # NODE_BLOOM & NODE_WITNESS & NODE_NETWORK_LIMITED must now be signaled - assert_equal(self.get_signalled_service_flags(), 1036) # 1036 == 0x40C == 0100 0000 1100 -# | || -# | |^--- NODE_BLOOM -# | ^---- NODE_WITNESS -# ^-- NODE_NETWORK_LIMITED + assert_equal(self.get_signalled_service_flags(), NODE_BLOOM | NODE_WITNESS | NODE_NETWORK_LIMITED) # Now mine some blocks over the NODE_NETWORK_LIMITED + 2(racy buffer ext.) target firstblock = self.nodes[0].generate(1)[0] @@ -66,10 +62,10 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): # NODE_NETWORK_LIMITED must still be signaled after restart self.restart_node(0) - assert_equal(self.get_signalled_service_flags(), 1036) + assert_equal(self.get_signalled_service_flags(), NODE_BLOOM | NODE_WITNESS | NODE_NETWORK_LIMITED) # Test the RPC service flags - assert_equal(self.nodes[0].getnetworkinfo()['localservices'], "000000000000040c") + assert_equal(int(self.nodes[0].getnetworkinfo()['localservices'], 16), NODE_BLOOM | NODE_WITNESS | NODE_NETWORK_LIMITED) # getdata a block above the NODE_NETWORK_LIMITED threshold must be possible self.try_get_block_via_getdata(block_within_limited_range, False) diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 2ab1bdac0..d8032e443 100644 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -38,10 +38,11 @@ COIN = 100000000 # 1 btc in satoshis NODE_NETWORK = (1 << 0) # NODE_GETUTXO = (1 << 1) -# NODE_BLOOM = (1 << 2) +NODE_BLOOM = (1 << 2) NODE_WITNESS = (1 << 3) NODE_UNSUPPORTED_SERVICE_BIT_5 = (1 << 5) NODE_UNSUPPORTED_SERVICE_BIT_7 = (1 << 7) +NODE_NETWORK_LIMITED = (1 << 10) # Serialization/deserialization tools def sha256(s): From 2e029845919eedcb4c4ff5f951cc85d95da68ad1 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Mon, 11 Dec 2017 12:08:33 -0500 Subject: [PATCH 3/5] [tests] node_network_limited - remove race condition node_network_limited had a race condition, since wait_for_block() doesn't do what you might expect. It only checks the most recent block received over the P2P interface (perhaps we should rename the method wait_for_most_recent_block() to avoid future confusion). The test can fail if the node sends us invs for other blocks, we respond with a getdata, and the node sends us one of those blocks in the 0.05 second wait_until loop window. Fix this by not responding to inv messages with getdata messages. --- test/functional/node_network_limited.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/test/functional/node_network_limited.py b/test/functional/node_network_limited.py index 1e7c61b44..6f1c60eec 100755 --- a/test/functional/node_network_limited.py +++ b/test/functional/node_network_limited.py @@ -2,15 +2,15 @@ # Copyright (c) 2017 The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. -from test_framework.messages import CInv, msg_getdata, msg_verack +from test_framework.messages import CInv, msg_getdata from test_framework.mininode import NODE_BLOOM, NODE_NETWORK_LIMITED, NODE_WITNESS, NetworkThread, P2PInterface from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal -class BaseNode(P2PInterface): - nServices = 0 - def on_version(self, message): - self.nServices = message.nServices +class P2PIgnoreInv(P2PInterface): + def on_inv(self, message): + # The node will send us invs for other blocks. Ignore them. + pass class NodeNetworkLimitedTest(BitcoinTestFramework): def set_test_params(self): @@ -19,7 +19,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): self.extra_args = [['-prune=550']] def get_signalled_service_flags(self): - node = self.nodes[0].add_p2p_connection(BaseNode()) + node = self.nodes[0].add_p2p_connection(P2PIgnoreInv()) NetworkThread().start() node.wait_for_verack() services = node.nServices @@ -28,10 +28,9 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): return services def try_get_block_via_getdata(self, blockhash, must_disconnect): - node = self.nodes[0].add_p2p_connection(BaseNode()) + node = self.nodes[0].add_p2p_connection(P2PIgnoreInv()) NetworkThread().start() node.wait_for_verack() - node.send_message(msg_verack()) getdata_request = msg_getdata() getdata_request.inv.append(CInv(2, int(blockhash, 16))) node.send_message(getdata_request) From b425131f5ace294f0d8be579f5ef596907cf1e16 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Mon, 11 Dec 2017 12:38:50 -0500 Subject: [PATCH 4/5] [tests] remove redundant duplicate tests from node_network_limited --- test/functional/node_network_limited.py | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/test/functional/node_network_limited.py b/test/functional/node_network_limited.py index 6f1c60eec..9313e3d35 100755 --- a/test/functional/node_network_limited.py +++ b/test/functional/node_network_limited.py @@ -48,29 +48,16 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): # NODE_BLOOM & NODE_WITNESS & NODE_NETWORK_LIMITED must now be signaled assert_equal(self.get_signalled_service_flags(), NODE_BLOOM | NODE_WITNESS | NODE_NETWORK_LIMITED) - # Now mine some blocks over the NODE_NETWORK_LIMITED + 2(racy buffer ext.) target - firstblock = self.nodes[0].generate(1)[0] - blocks = self.nodes[0].generate(292) - block_within_limited_range = blocks[-1] - - # Make sure we can max retrive block at tip-288 - # requesting block at height 2 (tip-289) must fail (ignored) - self.try_get_block_via_getdata(firstblock, True) # first block must lead to disconnect - self.try_get_block_via_getdata(blocks[1], False) # last block in valid range - self.try_get_block_via_getdata(blocks[0], True) # first block outside of the 288+2 limit - - # NODE_NETWORK_LIMITED must still be signaled after restart - self.restart_node(0) - assert_equal(self.get_signalled_service_flags(), NODE_BLOOM | NODE_WITNESS | NODE_NETWORK_LIMITED) - # Test the RPC service flags assert_equal(int(self.nodes[0].getnetworkinfo()['localservices'], 16), NODE_BLOOM | NODE_WITNESS | NODE_NETWORK_LIMITED) - # getdata a block above the NODE_NETWORK_LIMITED threshold must be possible - self.try_get_block_via_getdata(block_within_limited_range, False) + # Now mine some blocks over the NODE_NETWORK_LIMITED + 2(racy buffer ext.) target + blocks = self.nodes[0].generate(292) - # getdata a block below the NODE_NETWORK_LIMITED threshold must be ignored - self.try_get_block_via_getdata(firstblock, True) + # Make sure we can max retrive block at tip-288 + # requesting block at height 2 (tip-289) must fail (ignored) + self.try_get_block_via_getdata(blocks[1], False) # last block in valid range + self.try_get_block_via_getdata(blocks[0], True) # first block outside of the 288+2 limit if __name__ == '__main__': NodeNetworkLimitedTest().main() From ee5efad6cfb60d8efe678b1a9285a73d265ea79b Mon Sep 17 00:00:00 2001 From: John Newbery Date: Mon, 11 Dec 2017 12:51:14 -0500 Subject: [PATCH 5/5] [tests] refactor node_network_limited --- test/functional/node_network_limited.py | 64 +++++++++++-------------- 1 file changed, 29 insertions(+), 35 deletions(-) diff --git a/test/functional/node_network_limited.py b/test/functional/node_network_limited.py index 9313e3d35..70415e016 100755 --- a/test/functional/node_network_limited.py +++ b/test/functional/node_network_limited.py @@ -2,6 +2,12 @@ # Copyright (c) 2017 The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Tests NODE_NETWORK_LIMITED. + +Tests that a node configured with -prune=550 signals NODE_NETWORK_LIMITED correctly +and that it responds to getdata requests for blocks correctly: + - send a block within 288 + 2 of the tip + - disconnect peers who request blocks older than that.""" from test_framework.messages import CInv, msg_getdata from test_framework.mininode import NODE_BLOOM, NODE_NETWORK_LIMITED, NODE_WITNESS, NetworkThread, P2PInterface from test_framework.test_framework import BitcoinTestFramework @@ -12,52 +18,40 @@ class P2PIgnoreInv(P2PInterface): # The node will send us invs for other blocks. Ignore them. pass + def send_getdata_for_block(self, blockhash): + getdata_request = msg_getdata() + getdata_request.inv.append(CInv(2, int(blockhash, 16))) + self.send_message(getdata_request) + class NodeNetworkLimitedTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 self.extra_args = [['-prune=550']] - def get_signalled_service_flags(self): - node = self.nodes[0].add_p2p_connection(P2PIgnoreInv()) - NetworkThread().start() - node.wait_for_verack() - services = node.nServices - self.nodes[0].disconnect_p2ps() - node.wait_for_disconnect() - return services - - def try_get_block_via_getdata(self, blockhash, must_disconnect): - node = self.nodes[0].add_p2p_connection(P2PIgnoreInv()) - NetworkThread().start() - node.wait_for_verack() - getdata_request = msg_getdata() - getdata_request.inv.append(CInv(2, int(blockhash, 16))) - node.send_message(getdata_request) - - if (must_disconnect): - # Ensure we get disconnected - node.wait_for_disconnect(5) - else: - # check if the peer sends us the requested block - node.wait_for_block(int(blockhash, 16), 3) - self.nodes[0].disconnect_p2ps() - node.wait_for_disconnect() - def run_test(self): - # NODE_BLOOM & NODE_WITNESS & NODE_NETWORK_LIMITED must now be signaled - assert_equal(self.get_signalled_service_flags(), NODE_BLOOM | NODE_WITNESS | NODE_NETWORK_LIMITED) + node = self.nodes[0].add_p2p_connection(P2PIgnoreInv()) + NetworkThread().start() + node.wait_for_verack() - # Test the RPC service flags - assert_equal(int(self.nodes[0].getnetworkinfo()['localservices'], 16), NODE_BLOOM | NODE_WITNESS | NODE_NETWORK_LIMITED) + expected_services = NODE_BLOOM | NODE_WITNESS | NODE_NETWORK_LIMITED - # Now mine some blocks over the NODE_NETWORK_LIMITED + 2(racy buffer ext.) target + self.log.info("Check that node has signalled expected services.") + assert_equal(node.nServices, expected_services) + + self.log.info("Check that the localservices is as expected.") + assert_equal(int(self.nodes[0].getnetworkinfo()['localservices'], 16), expected_services) + + self.log.info("Mine enough blocks to reach the NODE_NETWORK_LIMITED range.") blocks = self.nodes[0].generate(292) - # Make sure we can max retrive block at tip-288 - # requesting block at height 2 (tip-289) must fail (ignored) - self.try_get_block_via_getdata(blocks[1], False) # last block in valid range - self.try_get_block_via_getdata(blocks[0], True) # first block outside of the 288+2 limit + self.log.info("Make sure we can max retrive block at tip-288.") + node.send_getdata_for_block(blocks[1]) # last block in valid range + node.wait_for_block(int(blocks[1], 16), timeout=3) + + self.log.info("Requesting block at height 2 (tip-289) must fail (ignored).") + node.send_getdata_for_block(blocks[0]) # first block outside of the 288+2 limit + node.wait_for_disconnect(5) if __name__ == '__main__': NodeNetworkLimitedTest().main()