Skip to content

Commit

Permalink
Merge bitcoin#19696: rpc: Fix addnode remove command error
Browse files Browse the repository at this point in the history
a51d0ad rpc: Improve addnode remove command error message (Fabian Jahr)

Pull request description:

  The `addnode` RPC with the `remove` command parameter is used to remove a node from the "added nodes". It did not have test coverage and in case of failure to remove the node it responded with the confusing message "Error: Node has not been added.".

  This PR adds test coverage and introduces a new error code as well as changes the error message to something that makes sense.

ACKs for top commit:
  laanwj:
    Code review ACK a51d0ad
  theStack:
    Tested ACK bitcoin@a51d0ad2de

Tree-SHA512: 033ef5de0d4d49d58ef4df3759b838c9d19ee9dfb0aff9f814a3a63d124ca231a442c930efa7d343fe1f65727c4b59fc23dd5e26fe6ea69f9e84fda48b5c5cc2
  • Loading branch information
laanwj committed Aug 12, 2020
2 parents bd00d3b + a51d0ad commit 13c4635
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/rpc/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ static UniValue addnode(const JSONRPCRequest& request)
else if(strCommand == "remove")
{
if(!node.connman->RemoveAddedNode(strNode))
throw JSONRPCError(RPC_CLIENT_NODE_NOT_ADDED, "Error: Node has not been added.");
throw JSONRPCError(RPC_CLIENT_NODE_NOT_ADDED, "Error: Node could not be removed. It has not been added previously.");
}

return NullUniValue;
Expand Down
7 changes: 7 additions & 0 deletions test/functional/rpc_net.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,13 @@ def _test_getaddednodeinfo(self):
added_nodes = self.nodes[0].getaddednodeinfo(ip_port)
assert_equal(len(added_nodes), 1)
assert_equal(added_nodes[0]['addednode'], ip_port)
# check that node cannot be added again
assert_raises_rpc_error(-23, "Node already added", self.nodes[0].addnode, node=ip_port, command='add')
# check that node can be removed
self.nodes[0].addnode(node=ip_port, command='remove')
assert_equal(self.nodes[0].getaddednodeinfo(), [])
# check that trying to remove the node again returns an error
assert_raises_rpc_error(-24, "Node could not be removed", self.nodes[0].addnode, node=ip_port, command='remove')
# check that a non-existent node returns an error
assert_raises_rpc_error(-24, "Node has not been added", self.nodes[0].getaddednodeinfo, '1.1.1.1')

Expand Down

0 comments on commit 13c4635

Please sign in to comment.