paritytech / substrate

Substrate: The platform for blockchain innovators
Apache License 2.0
8.39k stars 2.65k forks source link

system_removeReservedPeer RPC does not disconnect the node from the removed peer #7704

Open cwsatpolymath opened 3 years ago

cwsatpolymath commented 3 years ago

Problem description

Consider a scenario where a node is launched with the --reserved-only flag and no --reserved-peers option. This node will not connect to any peers. Then use the system_addReservedPeer to add a reserved node, e.g. /dns4/some.reserved.node/tcp/30333/p2p/123456789abcdefgh. The local node will then connect to said other node and start syncing. So far so good.

If we then use the system_removeReservedPeer RPC method to remove peer 123456789abcdefgh the response is

{
  "jsonrpc": "2.0",
  "result": null,
  "id": 1
}

indicating success. However the node will stay connected to that peer. One would expect that removing a reserved node with the RPC call should disconnect the node from said peer (at least when using --reserved-only).

Resolution:

I'd like to see one of the following two implemented:

tomaka commented 3 years ago

By "disconnect", are you talking about the TCP connection? If yes, then it's working as intended. The reserved nodes feature only covers which connections are used for syncing. In other words, the number of peers shown in the informant should decrease by one. Then, after 10 seconds of inactivity, the TCP connection will actually close.

cwsatpolymath commented 3 years ago

By "disconnect", are you talking about the TCP connection? If yes, then it's working as intended.

By "disconnect" I mean stop syncing with this peer.

In other words, the number of peers shown in the informant should decrease by one.

This is not happening in my test case. The number of peers stays the same.

Then, after 10 seconds of inactivity, the TCP connection will actually close.

Perhaps I'm hitting an edge case - I'm starting the node with zero peers, then adding one reserved peer, then removing it. Is the connection to this peer not closing because there are no other peers to fall back to?