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

fix: rejecting excess relay connections #3065

Merged
merged 8 commits into from
Sep 27, 2024

Conversation

gabrielmer
Copy link
Contributor

@gabrielmer gabrielmer commented Sep 27, 2024

Description

Rejecting incoming Relay connections whenever we reached our in connections target.

Currently, we accept connections and prune the excess connections periodically. However, this approach is not working out properly, as nodes can reconnect to us right after we disconnect to them, making no difference.

This makes the node vulnerable to attacks, as other nodes can fill all our connections without leaving libp2p connections to serve non-Relay nodes.

This PR addresses the issue, disconnecting from a Relay node right after receiving their request, in case we already reached our target.

The content of this PR was actually proposed by @Ivansete-status some weeks ago :) Thanks so much!

Changes

  • disconnecting from incoming connections after reaching inRelayPeersTarget
  • don't set an arbitrary minimum of outRelayPeersTarget
  • remove unused variables

Issue

closes #3063

Copy link

github-actions bot commented Sep 27, 2024

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:3065

Built from dc78de1

@gabrielmer gabrielmer force-pushed the fix-relay-connection-targets branch from 9289a73 to 139bf9a Compare September 27, 2024 14:47
@gabrielmer gabrielmer changed the title chore: [DEBUG] investigating excess in connections chore: rejecting excess relay connections Sep 27, 2024
@gabrielmer gabrielmer changed the title chore: rejecting excess relay connections fix: rejecting excess relay connections Sep 27, 2024
Copy link
Collaborator

@Ivansete-status Ivansete-status left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for it! 💯

@gabrielmer gabrielmer merged commit 8b0884c into master Sep 27, 2024
12 of 13 checks passed
@gabrielmer gabrielmer deleted the fix-relay-connection-targets branch September 27, 2024 16:35
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.

bug: excess relay connections are not being properly pruned
3 participants