Skip to content
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

Send disconnection reason when closing connection with a node #1133

Open
lock9 opened this issue Sep 30, 2019 · 4 comments
Open

Send disconnection reason when closing connection with a node #1133

lock9 opened this issue Sep 30, 2019 · 4 comments
Labels
discussion Initial issue state - proposed but not yet accepted enhancement Type - Changes that may affect performance, usability or add new features to existing modules. p2p Module - peer-to-peer message exchange and network optimisations, at TCP or UDP level (not HTTP).

Comments

@lock9
Copy link
Contributor

lock9 commented Sep 30, 2019

(Originally from @decentralisedkev on #366)

Summary
When a node sends an incompatible message during the handshake or a malformed message, the node silently disconnects. This makes it hard to debug why two nodes are not communicating, or why one node has disconnected from another.

Do you have any solution you want to propose?
If a node disconnects from another node, due to the message that that node had sent, they should send a reject message, stating why the node was disconnected and a byte to signify the reason for the computer.

Where in the software does this update applies to?

  • P2P (TCP)
@lock9 lock9 added enhancement Type - Changes that may affect performance, usability or add new features to existing modules. discussion Initial issue state - proposed but not yet accepted p2p Module - peer-to-peer message exchange and network optimisations, at TCP or UDP level (not HTTP). labels Sep 30, 2019
@ixje
Copy link
Contributor

ixje commented Oct 1, 2019

What reasons should we have?

  • max connection limit reached
  • inactivity
  • shutting down
  • protocol exception (e.g. handshake payload order wrong)
  • format exception (caused by deserialization failures)

The first 2 can help a client in filtering good addresses. For example; if a node force disconnects me, I would now label it as a bad node because it is doing something unexpected. With the above I could differentiate if it's a "soft" disconnect or a hard disconnect because there's something really wrong.

@shargon
Copy link
Member

shargon commented Oct 1, 2019

We can send the reason with the peers for example.

@ixje
Copy link
Contributor

ixje commented Oct 1, 2019

We can send the reason with the peers for example.

I don't understand, what 'peers'?
My understanding of the proposal is that this is an exchange between the local client and exactly 1 remote node.

@Tommo-L
Copy link
Contributor

Tommo-L commented Oct 11, 2019

fixed in #1154, plz review it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Initial issue state - proposed but not yet accepted enhancement Type - Changes that may affect performance, usability or add new features to existing modules. p2p Module - peer-to-peer message exchange and network optimisations, at TCP or UDP level (not HTTP).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants