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

Implement rlpx peer connections limit #3149

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mjfh
Copy link
Contributor

@mjfh mjfh commented Mar 17, 2025

why
The number of incoming connections were not restricted in the p2p
handler. Even worse, there was a nimbus option --max-peers which
was used to set up the minimum(sic) number of peers to keep
connected with.

Now, the re-organised nimbus options mean:

  • --min-peers -- minimum number of connections to hold
  • --max-peers -- maximum number of all peer connections

caveat
While the p2p module now rejects new incoming connections when
the limit is reached it does allow more connection list entries
for outgoing connection when explicitly using the connectToNode()
function (as in rpc/common.nim, sync/peers.nim and probably
others.) In practice, this lead to at most one more connections on
the list.

why
  The number of incoming connections were not restricted in the `p2p`
  handler. Even worse, there was a nimbus option `--max-peers` which
  was used to set up the minimum(sic) number of peers to keep
  connected with.

  Now, the re-organised nimbus options mean:
  * `--min-peers` -- minimum number of connections to hold
  * `--max-peers` -- maximum number of all peer connections

caveat
  While the p2p module now rejects new incoming connections when
  the limit is reached it does allow more connection list entries
  for outgoing connection when explicitly using the `connectToNode()`
  function (as in `rpc/common.nim`, `sync/peers.nim` and probably
  others.) In practice, this lead to at most one more connections on
  the list.
@mjfh mjfh marked this pull request as draft March 17, 2025 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants