-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Disconnect node on addnode remove #6259
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Disconnect node on addnode remove #6259
Conversation
|
Not against this. But a pure disconnect won't prevent from a reconnect of the just kicked node. I think the setban rpc command (#6158) would suit better for a node kick (kick and ban for 1h). For this PR two things would be cool:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
F.I.Y.: most one-line-ifs in the source codes are without brackets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're absolutely right, missed some clean up there, my bad.
|
I don't think it makes sense to disconnect from a peer merely because you remove it from the "persistent" addnode list... |
|
Thanks for taking a stab at implementing this, concept ACK. @luke-jr Agreed. This is useful functionality (now available in the GUI but not on RPC), however this is the wrong place. Let's just add a |
|
+1 @laanwj RE "disconnectnode" It is also odd that an "addnode" RPC actually performs the opposite of an "add" |
|
@jgarzik Yeh that was my comment on the original issue too, but I thought what the hey, here's an issue I can actually take care of, I'll give it a shot. Will take a look at adding disconnectnode later this week, leave it with me. |
|
disconnectnode imho makes more sense to me. Maybe even just a "node" call that takes specific commands. IE node ban, node add, node disconnect. |
|
@franko-org That would be possible, but e.g. with In retrospect, the RPC mechanism would have benefitted from namespacing (node.X, wallet.X etc) but doing that for one call is inconsistent. |
|
Closing in favor of #6271 |
On calling addnode remove, now disconnects from said node immediately per issue #2729. Requires the port to be specified in the IP parameter for it to be found and disconnected however.