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 the peers before close a connection if MaxConnections is reached. ('Endpoint isn't able to be connected if all seed nodes' peer number up to MAX(40 for now)') #1013

Closed
superboyiii opened this issue Aug 9, 2019 · 10 comments
Labels
design Issue state - Feature accepted but the solution requires a design before being implemented 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

@superboyiii
Copy link
Member

I've met a situation that all seed nodes have the connected number of 40 which is the max. Then I start a clear node to connect to mainnet, however, endpoint isn't able to be connected since there's no free endpoint anymore. Although finally, after two minutes, the node find a free endpont to connect and the block grows successfully, I think this issue may cause besiege-source-attack since it's very easy to achieve. Now we have to make a solution to handle more nodes. The external nodes' number is truely growing up more and more and we mustn't reject them.
@shargon @vncoelho @igormcoelho @erikzhang

@erikzhang
Copy link
Member

https://github.com/neo-project/neo-cli/blob/100a443a1690d77b8923ef39a66c315dd380afda/neo-cli/Settings.cs#L68

We can change the settings of MaxConnections in config.json.

@shargon
Copy link
Member

shargon commented Aug 9, 2019

100 instead of 40?

@shargon
Copy link
Member

shargon commented Aug 9, 2019

But we should implement an ACL for ban or disconnect bad nodes, acording the response time, the expected messages, etc

@superboyiii
Copy link
Member Author

https://github.com/neo-project/neo-cli/blob/100a443a1690d77b8923ef39a66c315dd380afda/neo-cli/Settings.cs#L68

We can change the settings of MaxConnections in config.json.

I know this operation, but its not a final solution since it will push more and more pressure on seed nodes and seed nodes need to upgrade its hardware or bandwidth more often in the future. Maybe we should make a more smart solution, for example, when a seed node is full on connection, make it still be able to recommand its peers to new nodes which would like to join for connection. If its peers are all full too, they could recommend their peers to it and so on, like a tree structure.

@shargon
Copy link
Member

shargon commented Aug 9, 2019

I like your idea @superboyiii

@shargon shargon added the discussion Initial issue state - proposed but not yet accepted label Aug 10, 2019
@shargon
Copy link
Member

shargon commented Aug 11, 2019

For me is ready-to-implement:

  • Send the peers before close a connection because MaxConnections was raised.

@lock9 lock9 changed the title Endpoint isn't able to be connected if all seed nodes' peer number up to MAX(40 for now) Send the peers before close a connection if MaxConnections is reached. ('Endpoint isn't able to be connected if all seed nodes' peer number up to MAX(40 for now)') Aug 11, 2019
@lock9 lock9 added design Issue state - Feature accepted but the solution requires a design before being implemented 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). labels Aug 11, 2019
@lock9
Copy link
Contributor

lock9 commented Aug 11, 2019

@shargon let's give the team at least 2 days to review it. If someone else approves I think we are good to go

@lock9 lock9 removed the discussion Initial issue state - proposed but not yet accepted label Aug 11, 2019
@vncoelho
Copy link
Member

Good insight, @superboyiii!

Sounds an informative and good decision.

When implementing it let's consider the design of a flexible message of information.

In the future nodes could even share statistics between what they think about their peers.

@lock9 lock9 added this to Preview3 - 01/11/2019 in NEO 3 Releases Aug 12, 2019
@superboyiii
Copy link
Member Author

@vncoelho That's great!

@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
design Issue state - Feature accepted but the solution requires a design before being implemented 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
NEO 3 Releases
Preview3 - 01/11/2019
Development

No branches or pull requests

6 participants