Disconnect node on addnode remove #6259

1. 
   Alex-van-der-Peet commented at 7:01 am on June 9, 2015: contributor

   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.
2. Disconnect node on addnode remove b5974eb729
3. 
   jonasschnelli commented at 9:20 am on June 9, 2015: contributor

   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:

   • ban disconnected nodes for 1h (or at least 10mins)
   • allow removing nodes by IP without port (kick all nodes from the given IP)
4. in src/rpcnet.cpp: in b5974eb729 outdated

   205@@ -206,6 +206,13 @@ UniValue addnode(const UniValue& params, bool fHelp)
   206     }
   207     else if(strCommand == "remove")
   208     {
   209+        CNode* pNode = FindNode(strNode.c_str());
   210+
   211+        if (pNode != NULL)
   

   ---

   ---

   

   jonasschnelli commented at 11:57 am on June 9, 2015:

   F.I.Y.: most one-line-ifs in the source codes are without brackets.

   ---

   

   Alex-van-der-Peet commented at 12:50 pm on June 9, 2015:

   You’re absolutely right, missed some clean up there, my bad.
5. laanwj added the label RPC on Jun 9, 2015
6. Bracket cleanup 8e7f85128d
7. 
   luke-jr commented at 6:39 pm on June 9, 2015: member

   I don’t think it makes sense to disconnect from a peer merely because you remove it from the “persistent” addnode list…
8. 
   laanwj commented at 8:57 am on June 10, 2015: member

   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 disconnectnode RPC.
9. 
   jgarzik commented at 1:06 pm on June 10, 2015: contributor

   +1 @laanwj RE “disconnectnode”

   It is also odd that an “addnode” RPC actually performs the opposite of an “add”

10. 
    Alex-van-der-Peet commented at 1:58 pm on June 10, 2015: contributor

    @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.

11. 
    franko-org commented at 4:43 pm on June 10, 2015: none

    disconnectnode imho makes more sense to me. Maybe even just a “node” call that takes specific commands. IE node ban, node add, node disconnect.
12. 
    laanwj commented at 8:42 am on June 11, 2015: member

    @franko-org That would be possible, but e.g. with help it’s handier to see what is available in one glance. I don’t see an advantage to be particlularly thrifty with adding RPC calls. This gave us peculiar constructs like addnode remove in the first place :)

    In retrospect, the RPC mechanism would have benefitted from namespacing (node.X, wallet.X etc) but doing that for one call is inconsistent.

13. 
    laanwj commented at 2:40 pm on June 12, 2015: member

    Closing in favor of #6271
14. laanwj closed this on Jun 12, 2015

    ---

15. MarcoFalke locked this on Sep 8, 2021

Contributors
Alex-van-der-Peet jonasschnelli luke-jr laanwj jgarzik franko-org

Labels
RPC/REST/ZMQ