Merge #10143: [net] Allow disconnectnode RPC to be called with node id

d54297f [tests] disconnect_ban: add tests for disconnect-by-nodeid (John Newbery)
5cc3ee2 [tests] disconnect_ban: remove dependency on urllib (John Newbery)
12de2f2 [tests] disconnect_ban: use wait_until instead of sleep (John Newbery)
2077fda [tests] disconnect_ban: add logging (John Newbery)
395561b [tests] disconnectban test - only use two nodes (John Newbery)
e367ad5 [tests] rename nodehandling to disconnectban (John Newbery)
d6564a2 [tests] fix nodehandling.py flake8 warnings (John Newbery)
23e6e64 Allow disconnectnode() to be called with node id (John Newbery)

Tree-SHA512: a371bb05a24a91cdb16a7ac4fbb091d5d3bf6554b29bd69d74522cb7523d3f1c5b1989d5e7b03f3fc7369fb727098dd2a549de551b731dd480c121d9517d3576
This commit is contained in:
Wladimir J. van der Laan 2017-04-20 11:46:37 +02:00
commit a987def4f6
No known key found for this signature in database
GPG key ID: 74810B012346C9A6
5 changed files with 138 additions and 88 deletions

View file

@ -115,6 +115,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "bumpfee", 1, "options" },
{ "logging", 0, "include" },
{ "logging", 1, "exclude" },
{ "disconnectnode", 1, "nodeid" },
// Echo with conversion (For testing only)
{ "echojson", 0, "arg0" },
{ "echojson", 1, "arg1" },

View file

@ -234,23 +234,43 @@ UniValue addnode(const JSONRPCRequest& request)
UniValue disconnectnode(const JSONRPCRequest& request)
{
if (request.fHelp || request.params.size() != 1)
if (request.fHelp || request.params.size() == 0 || request.params.size() >= 3)
throw std::runtime_error(
"disconnectnode \"address\" \n"
"\nImmediately disconnects from the specified node.\n"
"disconnectnode \"[address]\" [nodeid]\n"
"\nImmediately disconnects from the specified peer node.\n"
"\nStrictly one out of 'address' and 'nodeid' can be provided to identify the node.\n"
"\nTo disconnect by nodeid, either set 'address' to the empty string, or call using the named 'nodeid' argument only.\n"
"\nArguments:\n"
"1. \"address\" (string, required) The IP address/port of the node\n"
"1. \"address\" (string, optional) The IP address/port of the node\n"
"2. \"nodeid\" (number, optional) The node ID (see getpeerinfo for node IDs)\n"
"\nExamples:\n"
+ HelpExampleCli("disconnectnode", "\"192.168.0.6:8333\"")
+ HelpExampleCli("disconnectnode", "\"\" 1")
+ HelpExampleRpc("disconnectnode", "\"192.168.0.6:8333\"")
+ HelpExampleRpc("disconnectnode", "\"\", 1")
);
if(!g_connman)
throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
bool ret = g_connman->DisconnectNode(request.params[0].get_str());
if (!ret)
bool success;
const UniValue &address_arg = request.params[0];
const UniValue &id_arg = request.params.size() < 2 ? NullUniValue : request.params[1];
if (!address_arg.isNull() && id_arg.isNull()) {
/* handle disconnect-by-address */
success = g_connman->DisconnectNode(address_arg.get_str());
} else if (!id_arg.isNull() && (address_arg.isNull() || (address_arg.isStr() && address_arg.get_str().empty()))) {
/* handle disconnect-by-id */
NodeId nodeid = (NodeId) id_arg.get_int64();
success = g_connman->DisconnectNode(nodeid);
} else {
throw JSONRPCError(RPC_INVALID_PARAMS, "Only one of address and nodeid should be provided.");
}
if (!success) {
throw JSONRPCError(RPC_CLIENT_NODE_NOT_CONNECTED, "Node not found in connected nodes");
}
return NullUniValue;
}
@ -607,7 +627,7 @@ static const CRPCCommand commands[] =
{ "network", "ping", &ping, true, {} },
{ "network", "getpeerinfo", &getpeerinfo, true, {} },
{ "network", "addnode", &addnode, true, {"node","command"} },
{ "network", "disconnectnode", &disconnectnode, true, {"address"} },
{ "network", "disconnectnode", &disconnectnode, true, {"address", "nodeid"} },
{ "network", "getaddednodeinfo", &getaddednodeinfo, true, {"node"} },
{ "network", "getnettotals", &getnettotals, true, {} },
{ "network", "getnetworkinfo", &getnetworkinfo, true, {} },

109
test/functional/disconnect_ban.py Executable file
View file

@ -0,0 +1,109 @@
#!/usr/bin/env python3
# Copyright (c) 2014-2016 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test node disconnect and ban behavior"""
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,
start_node,
stop_node,
)
class DisconnectBanTest(BitcoinTestFramework):
def __init__(self):
super().__init__()
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):
self.log.info("Test setban and listbanned RPCs")
self.log.info("setban: successfully ban single IP address")
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_equal(len(self.nodes[1].getpeerinfo()), 0) # all nodes must be disconnected at this point
assert_equal(len(self.nodes[1].listbanned()), 1)
self.log.info("clearbanned: successfully clear ban list")
self.nodes[1].clearbanned()
assert_equal(len(self.nodes[1].listbanned()), 0)
self.nodes[1].setban("127.0.0.0/24", "add")
self.log.info("setban: fail to ban an already banned subnet")
assert_equal(len(self.nodes[1].listbanned()), 1)
assert_raises_jsonrpc(-23, "IP/Subnet already banned", self.nodes[1].setban, "127.0.0.1", "add")
self.log.info("setban: fail to ban an invalid subnet")
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
self.log.info("setban remove: fail to unban a non-banned subnet")
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.log.info("setban remove: successfully unban subnet")
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)
self.log.info("setban: test persistence across node restart")
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'])
wait_until(lambda: len(self.nodes[1].listbanned()) == 3)
stop_node(self.nodes[1], 1)
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)
self.log.info("Test disconnectrnode RPCs")
self.log.info("disconnectnode: fail to disconnect when calling with address and nodeid")
address1 = self.nodes[0].getpeerinfo()[0]['addr']
node1 = self.nodes[0].getpeerinfo()[0]['addr']
assert_raises_jsonrpc(-32602, "Only one of address and nodeid should be provided.", self.nodes[0].disconnectnode, address=address1, nodeid=node1)
self.log.info("disconnectnode: fail to disconnect when calling with junk address")
assert_raises_jsonrpc(-29, "Node not found in connected nodes", self.nodes[0].disconnectnode, address="221B Baker Street")
self.log.info("disconnectnode: successfully disconnect node by address")
address1 = self.nodes[0].getpeerinfo()[0]['addr']
self.nodes[0].disconnectnode(address=address1)
wait_until(lambda: len(self.nodes[0].getpeerinfo()) == 1)
assert not [node for node in self.nodes[0].getpeerinfo() if node['addr'] == address1]
self.log.info("disconnectnode: successfully reconnect node")
connect_nodes_bi(self.nodes, 0, 1) # reconnect the node
assert_equal(len(self.nodes[0].getpeerinfo()), 2)
assert [node for node in self.nodes[0].getpeerinfo() if node['addr'] == address1]
self.log.info("disconnectnode: successfully disconnect node by node id")
id1 = self.nodes[0].getpeerinfo()[0]['id']
self.nodes[0].disconnectnode(nodeid=id1)
wait_until(lambda: len(self.nodes[0].getpeerinfo()) == 1)
assert not [node for node in self.nodes[0].getpeerinfo() if node['id'] == id1]
if __name__ == '__main__':
DisconnectBanTest().main()

View file

@ -1,80 +0,0 @@
#!/usr/bin/env python3
# Copyright (c) 2014-2016 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test node handling."""
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import *
import urllib.parse
class NodeHandlingTest (BitcoinTestFramework):
def __init__(self):
super().__init__()
self.num_nodes = 4
self.setup_clean_chain = False
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)
# 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")
# 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
# 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)
##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
#stop node
stop_node(self.nodes[2], 2)
self.nodes[2] = start_node(2, self.options.tmpdir)
listAfterShutdown = self.nodes[2].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)
###########################
# 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)))
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)
if __name__ == '__main__':
NodeHandlingTest ().main ()

View file

@ -88,7 +88,7 @@ BASE_SCRIPTS= [
'multi_rpc.py',
'proxy_test.py',
'signrawtransactions.py',
'nodehandling.py',
'disconnect_ban.py',
'decodescript.py',
'blockchain.py',
'disablewallet.py',