Add Peer Limit to Swarm

[nisdas]

This PR adds a peer limit to the Swarm. The purpose of this is to limit new connections/streams being opened with peers once the limit is reached. For my usecase I needed to enforce a peer limit so that my libp2p host would not be connected to more peers than specified. However I could not find a way to configure and enforce this with the current swarm object. If there is a better way to achieve this I would be all ears.

For your reference, this is another branch that adds in a Peer Limit Option, in go-libp2p
libp2p/go-libp2p@master...nisdas:peerLimiter

[aarshkshah1992]

@nisdas

I think connection manager is what you are looking for. Here's an example:

connManager := connmgr.NewConnManager(nMinConnections, nMaxConnections)
opts := []libp2p.Option{
  libp2p.ConnectionManager(connManager),
}
host, err := libp2p.New(ctx, opts...)

Let me know if you have any questions.

[nisdas]

Unfortunately, connection manager does not satisfy my usecase, its a reactive solution to what I need. It still opens connections with peers and only trims them at every interval. This leads to a lot of peer churn which destabilizes the network, I am trying to avoid connecting to any new peers completely and adding them to my peerstore.

[prestonvanloon]

If it’s helpful, we also filed a question on the forum a few days ago.

https://discuss.libp2p.io/t/what-is-the-proper-way-to-limit-connections/357

[vyzo]

I am not convinced this is the right thing to do.

[nisdas]

@vyzo addressed your comments and also added in a test case, let me know what you think

[vyzo]

looking good, but let's also return an error from the options.

[vyzo]

that seems complete unnecessary.

[vyzo]

LGTM, but let's wait for @Stebalien as well because I am not 100% convinced we want this.

[vyzo]

travis test failure because of a data race.

[nisdas]

@vyzo fixed the data race, sounds good I will be waiting for @Stebalien review

[raulk]

From a design perspective, it's the connection manager's responsibility to control/limit the amount of connections that are open at any given point in time. The connection manager gets notified of all connections that are opened, when they're opened, so it can decide what to do.

The default ConnManager that ships with go-libp2p strives to keep the connection count between lower and upper bounds. However, if you seek different behaviour, you should implement an alternative connection manager that adheres to the ConnManager interface.

In fact, the way that you've implemented this logic is entirely reactive -- just like the connection manager. Unless there's something I'm ignoring, adding this pathway here yields no practical benefit as opposed to adding it where it really belongs (in the connection manager).

If what you were doing was preventing the dial altogether before it occurred, then that would be a proactive limiting and it could potentially belong in this component. But as it stands, it can go in the right place (conn manager) with no perceivable functional difference.

Here's an idea: instead of implementing an alternative connection manager, how about enhancing the existing connection manager with a strict/hard-limit mode, that does what you are submitting here?

We also need a DISCONNECT protocol so we can signal to the other party that they should not try to reconnect to us in N minutes.

Note that this approach may lead to weaker security. An attacker could eclipse you by probing you every N minutes/seconds with sybils that take over the slots that are left by peers that disconnect from you, eventually hoarding your entire connection table. It also leaks a binary indicator of whether you're under or over capacity. Those are some of the reasons our default implementation is elastic within boundaries, and uses scoring to decide which peers to prune.

[nisdas]

@raulk
Hey thanks for the detailed reply, looking through the code some more you are right that this is still a reactive solution. It seems like addCon is called after the connections are initiated so it would essentially be a more extreme version of the connection manager.

For outgoing connections, it is simple enough to put this check right before we dial a peer https://github.com/libp2p/go-libp2p-swarm/blob/master/swarm_dial.go#L293 . This would prevent any new outgoing connection from being created entirely. However for inbound connections this is a bit more complicated as there is no way to prevent an external peer from dialing us, from what I understand of the code the best place we can probably put a limit check is here https://github.com/libp2p/go-libp2p-swarm/blob/master/swarm_listen.go#L79.

My main purpose for this PR was to prevent any connection notification handlers from being called if we exceed the peer limit. https://github.com/libp2p/go-libp2p-swarm/blob/master/swarm.go#L235 . Since this lead to a lot of the peer churn issues we were seeing when using the connection manager previously.

I will look into implementing a much more strict connection manager for this, since it looks like adding this PR in might bring about some security risks to the protocol.

[nisdas]

Closing this PR as we have chosen to take an alternative approach of using a strict connection manager over here prysmaticlabs/prysm#4110 .