Add new 'dropnode' RPC command #336
Conversation
* Adds a new btcd-extension RPC command: `dropnode`
* `dropnode` can disconnect both inbound and outbound
peers, unlike `addnode <remove> addr` which only drops
persistent peers.
| case dropNodeMsg: | ||
| // Check inbound peers. We pass an empty callback since we don't | ||
| // require any additional actions on disconnect for inbound peers. | ||
| found := disconnectPeer(state.peers, msg.addr, func(p *peer) {}) |
There was a problem hiding this comment.
I'd suggest passing nil for the function here and adding a nil check in the disconnectPeer function. This will avoid creating a closure which is much more expensive than a nil check.
There was a problem hiding this comment.
Great suggestion, I've changed this fragment to pass a nil function instead.
|
This PR gets a firm NO from me. Not in favor of yet another command specific command. The 'addnode' command is already a bad enough name. I am in favor of making a new 'node' command to control nodes such as: btcctl node remove ID Then in the future, i'd like a |
|
What do you mean by: "command specific command"? Are you opposed to this PR strictly simply because it introduces a new command? Or do you feel that the feature it provides is not useful? What if it was altered a bit to introduce a new subcommand under I like your suggestion of introducing a custom, new command giving full control of peer connectivity (persistent vs non, etc.). EDIT: Will open new PR implementing the command suggested above. |
|
I like the idea. I am solely opposed because of the name. :) I would like to organize new commands into subcommands. |
|
Closing in favor of #341 |
This PR adds new JSON-RPC command:
dropnode.This aims to fix #79 by adding a new command that can disconnect any (inbound or outbound) non-persistent peer.