diff --git a/qa/rpc-tests/nodehandling.py b/qa/rpc-tests/nodehandling.py index 634dba7c8e6..61be27ae2b6 100755 --- a/qa/rpc-tests/nodehandling.py +++ b/qa/rpc-tests/nodehandling.py @@ -7,77 +7,83 @@ # Test node handling # +from test_framework.mininode import wait_until from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import * +from test_framework.util import (assert_equal, + assert_raises_jsonrpc, + connect_nodes_bi, + start_node, + stop_node, + ) -import urllib.parse - -class NodeHandlingTest (BitcoinTestFramework): +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") - 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()), 2) # node1 should have 2 connections to node0 at this point + self.nodes[1].setban("127.0.0.1", "add") + 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() + 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() - 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 + # test persisted banlist + 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']) + assert wait_until(lambda: len(self.nodes[1].listbanned()) == 3, timeout=10) - #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 # ########################### - 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 - for node in self.nodes[0].getpeerinfo(): - assert(node['addr'] != url.hostname+":"+str(p2p_port(1))) + address1 = self.nodes[0].getpeerinfo()[0]['addr'] + self.nodes[0].disconnectnode(address=address1) + 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 - found = False - for node in self.nodes[0].getpeerinfo(): - if node['addr'] == url.hostname+":"+str(p2p_port(1)): - found = True - assert(found) + connect_nodes_bi(self.nodes, 0, 1) # reconnect the node + assert [node for node in self.nodes[0].getpeerinfo() if node['addr'] == address1] if __name__ == '__main__': - NodeHandlingTest ().main () + NodeHandlingTest().main() 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 }