go-libp2p icon indicating copy to clipboard operation
go-libp2p copied to clipboard

Published 20 hours ago • verified publisher icon libp2p

Relay reservation constraints "rate limit" vs "active limit"

Open corverroos opened this issue 2 years ago • 3 comments

When a relay-reserver peer is disconnected from a relay "server" (the middleman providing the relay service), its relay reservation is deleted, but the "relay constraints" are not updated accordingly. Relay constraints are only deleted after 30min TTL.

If reservers connect and disconnect multiple times, it soon results in too many reservations or too many reservations for peer to be returned even though there are not actually that many reservations.

Is this the intention behind the constraints, to be a "new reserver connection rate limiter" instead of a "total active reservation limiter"?

Related quirk is that "active relay reservations" can exceed MaxReservations. When a reserver remains connected for longer than 30min (regularly refreshing its TTL), it is deleted from the constraints, while the reservation remains active, allowing additional reservations to be added.

corverroos avatar Aug 20 '22 10:08 corverroos

Yeah, this sounds like two bugs. Thanks for the find. Any interest in taking a stab at the implementation?

Another idea (on top of the fixes) we had while thinking about this was to add some canonical logging when a peer makes a reservation. This way we can hook into fail2ban if a peer is repeatedly connecting, reserving, and disconnecting

MarcoPolo avatar Sep 02 '22 16:09 MarcoPolo

Ok cool, I'll give it a shot.

Here is my proposed solution:

  • Clean "relay constraints" on disconnect.
  • Update "relay constraints TTL" when reservations are refreshed
  • Deprecate "MaxReservations" since there can only ever be 1 reservation per peer.

I'm going to leave out the canonical logging. I'll try to add that subsequently in another PR.

corverroos avatar Sep 05 '22 06:09 corverroos

Deprecate "MaxReservations" since there can only ever be 1 reservation per peer.

Yeah, I'm not sure why the default is 4. @marten-seemann any insight here?

MarcoPolo avatar Sep 13 '22 17:09 MarcoPolo

Deprecate "MaxReservations" since there can only ever be 1 reservation per peer.

Yeah, I'm not sure why the default is 4. @marten-seemann any insight here?

There may be some transitory state where you may have more than 1 reservation per peer, but not sure yet.

@corverroos have you gotten a chance to dig into this? No worries if not

MarcoPolo avatar Oct 03 '22 16:10 MarcoPolo

Oops, seems like we needed more information for this issue, please comment with more details or this issue will be closed in 7 days.

github-actions[bot] avatar Oct 20 '22 00:10 github-actions[bot]

This issue was closed because it is missing author input.

github-actions[bot] avatar Oct 28 '22 00:10 github-actions[bot]

This issue was closed because it is missing author input.

github-actions[bot] avatar Nov 05 '22 00:11 github-actions[bot]