From 5bc75bb8eea566258ddd9c858e2cf6361318d777 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Wed, 19 Apr 2017 11:07:44 -0400 Subject: [PATCH 1/6] [tests] fix nodehandling.py flake8 warnings Github-Pull: #10143 Rebased-From: d6564a26f4afc28d7d1a24b94946916387c9bf24 --- qa/rpc-tests/nodehandling.py | 46 +++++++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/qa/rpc-tests/nodehandling.py b/qa/rpc-tests/nodehandling.py index 634dba7c8e6..95dec42fe09 100755 --- a/qa/rpc-tests/nodehandling.py +++ b/qa/rpc-tests/nodehandling.py @@ -6,13 +6,19 @@ # # Test node handling # +import time +import urllib.parse from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import * - -import urllib.parse +from test_framework.util import (assert_equal, + assert_raises_jsonrpc, + connect_nodes_bi, + p2p_port, + start_node, + stop_node, + ) -class NodeHandlingTest (BitcoinTestFramework): +class NodeHandlingTest(BitcoinTestFramework): def __init__(self): super().__init__() @@ -23,10 +29,10 @@ def run_test(self): ########################### # setban/listbanned tests # ########################### - assert_equal(len(self.nodes[2].getpeerinfo()), 4) #we should have 4 nodes at this point + assert_equal(len(self.nodes[2].getpeerinfo()), 4) # we should have 4 nodes at this point self.nodes[2].setban("127.0.0.1", "add") - time.sleep(3) #wait till the nodes are disconected - assert_equal(len(self.nodes[2].getpeerinfo()), 0) #all nodes must be disconnected at this point + time.sleep(3) # wait till the nodes are disconected + assert_equal(len(self.nodes[2].getpeerinfo()), 0) # all nodes must be disconnected at this point assert_equal(len(self.nodes[2].listbanned()), 1) self.nodes[2].clearbanned() assert_equal(len(self.nodes[2].listbanned()), 0) @@ -36,7 +42,7 @@ def run_test(self): assert_raises_jsonrpc(-23, "IP/Subnet already banned", self.nodes[2].setban, "127.0.0.1", "add") # This will throw an exception because 127.0.0.1/42 is not a real subnet assert_raises_jsonrpc(-30, "Error: Invalid IP/Subnet", self.nodes[2].setban, "127.0.0.1/42", "add") - assert_equal(len(self.nodes[2].listbanned()), 1) #still only one banned ip because 127.0.0.1 is within the range of 127.0.0.0/24 + assert_equal(len(self.nodes[2].listbanned()), 1) # still only one banned ip because 127.0.0.1 is within the range of 127.0.0.0/24 # This will throw an exception because 127.0.0.1 was not added above assert_raises_jsonrpc(-30, "Error: Unban failed", self.nodes[2].setban, "127.0.0.1", "remove") assert_equal(len(self.nodes[2].listbanned()), 1) @@ -45,16 +51,16 @@ def run_test(self): self.nodes[2].clearbanned() assert_equal(len(self.nodes[2].listbanned()), 0) - ##test persisted banlist + # test persisted banlist self.nodes[2].setban("127.0.0.0/32", "add") self.nodes[2].setban("127.0.0.0/24", "add") - self.nodes[2].setban("192.168.0.1", "add", 1) #ban for 1 seconds - self.nodes[2].setban("2001:4d48:ac57:400:cacf:e9ff:fe1d:9c63/19", "add", 1000) #ban for 1000 seconds + self.nodes[2].setban("192.168.0.1", "add", 1) # ban for 1 seconds + self.nodes[2].setban("2001:4d48:ac57:400:cacf:e9ff:fe1d:9c63/19", "add", 1000) # ban for 1000 seconds listBeforeShutdown = self.nodes[2].listbanned() - assert_equal("192.168.0.1/32", listBeforeShutdown[2]['address']) #must be here - time.sleep(2) #make 100% sure we expired 192.168.0.1 node time + assert_equal("192.168.0.1/32", listBeforeShutdown[2]['address']) # must be here + time.sleep(2) # make 100% sure we expired 192.168.0.1 node time - #stop node + # stop node stop_node(self.nodes[2], 2) self.nodes[2] = start_node(2, self.options.tmpdir) @@ -67,17 +73,17 @@ def run_test(self): # RPC disconnectnode test # ########################### url = urllib.parse.urlparse(self.nodes[1].url) - self.nodes[0].disconnectnode(url.hostname+":"+str(p2p_port(1))) - time.sleep(2) #disconnecting a node needs a little bit of time + self.nodes[0].disconnectnode(url.hostname + ":" + str(p2p_port(1))) + time.sleep(2) # disconnecting a node needs a little bit of time for node in self.nodes[0].getpeerinfo(): - assert(node['addr'] != url.hostname+":"+str(p2p_port(1))) + assert(node['addr'] != url.hostname + ":" + str(p2p_port(1))) - connect_nodes_bi(self.nodes,0,1) #reconnect the node + connect_nodes_bi(self.nodes, 0, 1) # reconnect the node found = False for node in self.nodes[0].getpeerinfo(): - if node['addr'] == url.hostname+":"+str(p2p_port(1)): + if node['addr'] == url.hostname + ":" + str(p2p_port(1)): found = True assert(found) if __name__ == '__main__': - NodeHandlingTest ().main () + NodeHandlingTest().main() From bfd1cf6713f1f44b88b135c995b9ddb9e99ff7aa Mon Sep 17 00:00:00 2001 From: John Newbery Date: Wed, 19 Apr 2017 11:25:31 -0400 Subject: [PATCH 2/6] [tests] disconnectban test - only use two nodes Github-Pull: #10143 Rebased-From: 395561becfa612fedec74fd841cb4f28afdc23d7 --- qa/rpc-tests/nodehandling.py | 60 +++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 26 deletions(-) diff --git a/qa/rpc-tests/nodehandling.py b/qa/rpc-tests/nodehandling.py index 95dec42fe09..ec32cb069fb 100755 --- a/qa/rpc-tests/nodehandling.py +++ b/qa/rpc-tests/nodehandling.py @@ -22,53 +22,61 @@ class NodeHandlingTest(BitcoinTestFramework): def __init__(self): super().__init__() - self.num_nodes = 4 + self.num_nodes = 2 self.setup_clean_chain = False + def setup_network(self): + self.nodes = self.setup_nodes() + connect_nodes_bi(self.nodes, 0, 1) + def run_test(self): ########################### # setban/listbanned tests # ########################### - assert_equal(len(self.nodes[2].getpeerinfo()), 4) # we should have 4 nodes at this point - self.nodes[2].setban("127.0.0.1", "add") + assert_equal(len(self.nodes[1].getpeerinfo()), 2) # node1 should have 2 connections to node0 at this point + self.nodes[1].setban("127.0.0.1", "add") time.sleep(3) # wait till the nodes are disconected - assert_equal(len(self.nodes[2].getpeerinfo()), 0) # all nodes must be disconnected at this point - assert_equal(len(self.nodes[2].listbanned()), 1) - self.nodes[2].clearbanned() - assert_equal(len(self.nodes[2].listbanned()), 0) - self.nodes[2].setban("127.0.0.0/24", "add") - assert_equal(len(self.nodes[2].listbanned()), 1) + assert_equal(len(self.nodes[1].getpeerinfo()), 0) # all nodes must be disconnected at this point + assert_equal(len(self.nodes[1].listbanned()), 1) + self.nodes[1].clearbanned() + assert_equal(len(self.nodes[1].listbanned()), 0) + self.nodes[1].setban("127.0.0.0/24", "add") + assert_equal(len(self.nodes[1].listbanned()), 1) # This will throw an exception because 127.0.0.1 is within range 127.0.0.0/24 - assert_raises_jsonrpc(-23, "IP/Subnet already banned", self.nodes[2].setban, "127.0.0.1", "add") + assert_raises_jsonrpc(-23, "IP/Subnet already banned", self.nodes[1].setban, "127.0.0.1", "add") # This will throw an exception because 127.0.0.1/42 is not a real subnet - assert_raises_jsonrpc(-30, "Error: Invalid IP/Subnet", self.nodes[2].setban, "127.0.0.1/42", "add") - assert_equal(len(self.nodes[2].listbanned()), 1) # still only one banned ip because 127.0.0.1 is within the range of 127.0.0.0/24 + assert_raises_jsonrpc(-30, "Error: Invalid IP/Subnet", self.nodes[1].setban, "127.0.0.1/42", "add") + assert_equal(len(self.nodes[1].listbanned()), 1) # still only one banned ip because 127.0.0.1 is within the range of 127.0.0.0/24 # This will throw an exception because 127.0.0.1 was not added above - assert_raises_jsonrpc(-30, "Error: Unban failed", self.nodes[2].setban, "127.0.0.1", "remove") - assert_equal(len(self.nodes[2].listbanned()), 1) - self.nodes[2].setban("127.0.0.0/24", "remove") - assert_equal(len(self.nodes[2].listbanned()), 0) - self.nodes[2].clearbanned() - assert_equal(len(self.nodes[2].listbanned()), 0) + assert_raises_jsonrpc(-30, "Error: Unban failed", self.nodes[1].setban, "127.0.0.1", "remove") + assert_equal(len(self.nodes[1].listbanned()), 1) + self.nodes[1].setban("127.0.0.0/24", "remove") + assert_equal(len(self.nodes[1].listbanned()), 0) + self.nodes[1].clearbanned() + assert_equal(len(self.nodes[1].listbanned()), 0) # test persisted banlist - self.nodes[2].setban("127.0.0.0/32", "add") - self.nodes[2].setban("127.0.0.0/24", "add") - self.nodes[2].setban("192.168.0.1", "add", 1) # ban for 1 seconds - self.nodes[2].setban("2001:4d48:ac57:400:cacf:e9ff:fe1d:9c63/19", "add", 1000) # ban for 1000 seconds - listBeforeShutdown = self.nodes[2].listbanned() + self.nodes[1].setban("127.0.0.0/32", "add") + self.nodes[1].setban("127.0.0.0/24", "add") + self.nodes[1].setban("192.168.0.1", "add", 1) # ban for 1 seconds + self.nodes[1].setban("2001:4d48:ac57:400:cacf:e9ff:fe1d:9c63/19", "add", 1000) # ban for 1000 seconds + listBeforeShutdown = self.nodes[1].listbanned() assert_equal("192.168.0.1/32", listBeforeShutdown[2]['address']) # must be here time.sleep(2) # make 100% sure we expired 192.168.0.1 node time # stop node - stop_node(self.nodes[2], 2) + stop_node(self.nodes[1], 1) - self.nodes[2] = start_node(2, self.options.tmpdir) - listAfterShutdown = self.nodes[2].listbanned() + self.nodes[1] = start_node(1, self.options.tmpdir) + listAfterShutdown = self.nodes[1].listbanned() assert_equal("127.0.0.0/24", listAfterShutdown[0]['address']) assert_equal("127.0.0.0/32", listAfterShutdown[1]['address']) assert_equal("/19" in listAfterShutdown[2]['address'], True) + # Clear ban lists + self.nodes[1].clearbanned() + connect_nodes_bi(self.nodes, 0, 1) + ########################### # RPC disconnectnode test # ########################### From 98bd0c338b1886d12ea3d90597ab0438acbc07ff Mon Sep 17 00:00:00 2001 From: John Newbery Date: Wed, 19 Apr 2017 12:42:39 -0400 Subject: [PATCH 3/6] [tests] disconnect_ban: use wait_until instead of sleep Github-Pull: #10143 Rebased-From: 12de2f252c8f48e05c579cb679866a68af8c660e --- qa/rpc-tests/nodehandling.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/qa/rpc-tests/nodehandling.py b/qa/rpc-tests/nodehandling.py index ec32cb069fb..f540d3aa958 100755 --- a/qa/rpc-tests/nodehandling.py +++ b/qa/rpc-tests/nodehandling.py @@ -6,9 +6,9 @@ # # Test node handling # -import time import urllib.parse +from test_framework.mininode import wait_until from test_framework.test_framework import BitcoinTestFramework from test_framework.util import (assert_equal, assert_raises_jsonrpc, @@ -35,7 +35,7 @@ def run_test(self): ########################### assert_equal(len(self.nodes[1].getpeerinfo()), 2) # node1 should have 2 connections to node0 at this point self.nodes[1].setban("127.0.0.1", "add") - time.sleep(3) # wait till the nodes are disconected + wait_until(lambda: len(self.nodes[1].getpeerinfo()) == 0) assert_equal(len(self.nodes[1].getpeerinfo()), 0) # all nodes must be disconnected at this point assert_equal(len(self.nodes[1].listbanned()), 1) self.nodes[1].clearbanned() @@ -61,10 +61,9 @@ def run_test(self): self.nodes[1].setban("192.168.0.1", "add", 1) # ban for 1 seconds self.nodes[1].setban("2001:4d48:ac57:400:cacf:e9ff:fe1d:9c63/19", "add", 1000) # ban for 1000 seconds listBeforeShutdown = self.nodes[1].listbanned() - assert_equal("192.168.0.1/32", listBeforeShutdown[2]['address']) # must be here - time.sleep(2) # make 100% sure we expired 192.168.0.1 node time + assert_equal("192.168.0.1/32", listBeforeShutdown[2]['address']) + wait_until(lambda: len(self.nodes[1].listbanned()) == 3) - # stop node stop_node(self.nodes[1], 1) self.nodes[1] = start_node(1, self.options.tmpdir) @@ -82,7 +81,7 @@ def run_test(self): ########################### url = urllib.parse.urlparse(self.nodes[1].url) self.nodes[0].disconnectnode(url.hostname + ":" + str(p2p_port(1))) - time.sleep(2) # disconnecting a node needs a little bit of time + wait_until(lambda: len(self.nodes[0].getpeerinfo()) == 1) for node in self.nodes[0].getpeerinfo(): assert(node['addr'] != url.hostname + ":" + str(p2p_port(1))) From 04226938a3c675faaca81906ae9ff7a936bcb6a7 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Wed, 19 Apr 2017 13:35:51 -0400 Subject: [PATCH 4/6] [tests] disconnect_ban: remove dependency on urllib Github-Pull: #10143 Rebased-From: 5cc3ee24d29397737f2746d78b19a2509c0dedbb --- qa/rpc-tests/nodehandling.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/qa/rpc-tests/nodehandling.py b/qa/rpc-tests/nodehandling.py index f540d3aa958..df6e2bc6af3 100755 --- a/qa/rpc-tests/nodehandling.py +++ b/qa/rpc-tests/nodehandling.py @@ -6,14 +6,12 @@ # # Test node handling # -import urllib.parse from test_framework.mininode import wait_until from test_framework.test_framework import BitcoinTestFramework from test_framework.util import (assert_equal, assert_raises_jsonrpc, connect_nodes_bi, - p2p_port, start_node, stop_node, ) @@ -79,18 +77,13 @@ def run_test(self): ########################### # RPC disconnectnode test # ########################### - url = urllib.parse.urlparse(self.nodes[1].url) - self.nodes[0].disconnectnode(url.hostname + ":" + str(p2p_port(1))) + address1 = self.nodes[0].getpeerinfo()[0]['addr'] + self.nodes[0].disconnectnode(address=address1) wait_until(lambda: len(self.nodes[0].getpeerinfo()) == 1) - for node in self.nodes[0].getpeerinfo(): - assert(node['addr'] != url.hostname + ":" + str(p2p_port(1))) + assert not [node for node in self.nodes[0].getpeerinfo() if node['addr'] == address1] connect_nodes_bi(self.nodes, 0, 1) # reconnect the node - found = False - for node in self.nodes[0].getpeerinfo(): - if node['addr'] == url.hostname + ":" + str(p2p_port(1)): - found = True - assert(found) + assert [node for node in self.nodes[0].getpeerinfo() if node['addr'] == address1] if __name__ == '__main__': NodeHandlingTest().main() From d289b564e3ae125cb54c3d9157a13e7bad48c5f5 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Wed, 19 Apr 2017 12:49:11 -0400 Subject: [PATCH 5/6] [net] listbanned RPC and QT should show correct banned subnets Github-Pull: #10234 Rebased-From: 77c54b270dd3b469d662c8f69e06f7b00fc4136d --- src/net.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 1752c7707a7..5bc886c3408 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -413,10 +413,10 @@ void CConnman::DumpBanlist() CBanDB bandb; banmap_t banmap; - SetBannedSetDirty(false); GetBanned(banmap); - if (!bandb.Write(banmap)) - SetBannedSetDirty(true); + if (bandb.Write(banmap)) { + SetBannedSetDirty(false); + } LogPrint("net", "Flushed %d banned node ips/subnets to banlist.dat %dms\n", banmap.size(), GetTimeMillis() - nStart); @@ -536,6 +536,8 @@ bool CConnman::Unban(const CSubNet &subNet) { void CConnman::GetBanned(banmap_t &banMap) { LOCK(cs_setBanned); + // Sweep the banlist so expired bans are not returned + SweepBanned(); banMap = setBanned; //create a thread safe copy } From ee1a60d15609930954b1b85da7f33774711123cd Mon Sep 17 00:00:00 2001 From: John Newbery Date: Tue, 25 Apr 2017 10:11:31 -0400 Subject: [PATCH 6/6] [tests] update disconnect_ban.py test case to work with listbanned Github-Pull: #10234 Rebased-From: d6732d832aa901e733e63799260d409821a2c37a --- qa/rpc-tests/nodehandling.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/qa/rpc-tests/nodehandling.py b/qa/rpc-tests/nodehandling.py index df6e2bc6af3..61be27ae2b6 100755 --- a/qa/rpc-tests/nodehandling.py +++ b/qa/rpc-tests/nodehandling.py @@ -33,7 +33,7 @@ def run_test(self): ########################### assert_equal(len(self.nodes[1].getpeerinfo()), 2) # node1 should have 2 connections to node0 at this point self.nodes[1].setban("127.0.0.1", "add") - wait_until(lambda: len(self.nodes[1].getpeerinfo()) == 0) + assert wait_until(lambda: len(self.nodes[1].getpeerinfo()) == 0, timeout=10) assert_equal(len(self.nodes[1].getpeerinfo()), 0) # all nodes must be disconnected at this point assert_equal(len(self.nodes[1].listbanned()), 1) self.nodes[1].clearbanned() @@ -60,7 +60,7 @@ def run_test(self): self.nodes[1].setban("2001:4d48:ac57:400:cacf:e9ff:fe1d:9c63/19", "add", 1000) # ban for 1000 seconds listBeforeShutdown = self.nodes[1].listbanned() assert_equal("192.168.0.1/32", listBeforeShutdown[2]['address']) - wait_until(lambda: len(self.nodes[1].listbanned()) == 3) + assert wait_until(lambda: len(self.nodes[1].listbanned()) == 3, timeout=10) stop_node(self.nodes[1], 1) @@ -79,7 +79,7 @@ def run_test(self): ########################### address1 = self.nodes[0].getpeerinfo()[0]['addr'] self.nodes[0].disconnectnode(address=address1) - wait_until(lambda: len(self.nodes[0].getpeerinfo()) == 1) + assert wait_until(lambda: len(self.nodes[0].getpeerinfo()) == 1, timeout=10) assert not [node for node in self.nodes[0].getpeerinfo() if node['addr'] == address1] connect_nodes_bi(self.nodes, 0, 1) # reconnect the node