Skip to content

Commit

Permalink
Merge #10143: [net] Allow disconnectnode RPC to be called with node id
Browse files Browse the repository at this point in the history
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
  • Loading branch information
laanwj committed Apr 20, 2017
2 parents c91ca0a + d54297f commit a987def
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 88 deletions.
1 change: 1 addition & 0 deletions src/rpc/client.cpp
Expand Up @@ -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" },
Expand Down
34 changes: 27 additions & 7 deletions src/rpc/net.cpp
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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, {} },
Expand Down
109 changes: 109 additions & 0 deletions test/functional/disconnect_ban.py
@@ -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()
80 changes: 0 additions & 80 deletions test/functional/nodehandling.py

This file was deleted.

2 changes: 1 addition & 1 deletion test/functional/test_runner.py
Expand Up @@ -88,7 +88,7 @@
'multi_rpc.py',
'proxy_test.py',
'signrawtransactions.py',
'nodehandling.py',
'disconnect_ban.py',
'decodescript.py',
'blockchain.py',
'disablewallet.py',
Expand Down

0 comments on commit a987def

Please sign in to comment.