base limits on reservations issued #132
Conversation
marten-seemann
force-pushed
the
rework-limits
branch
from
a89e5cc
to
251c163
Compare
marten-seemann
force-pushed
the
rework-limits
branch
from
251c163
to
8706bab
Compare
|
I'm not sure I understand how this makes us easier to DoS. Before this PR, we'd send out reservations (up to a certain limit). Once a reservation has been used up, we don't count it towards our limits any more. If it's not used, it's garbage collected after a while. This, as @Stebalien noted, would allow a node to DoS us by repeatedly making a reservation and immediately using it. A DoS is only possible if the attacker controls multiple IP addresses (the limit per peer ID doesn't provide any DoS protection, as peer IDs are trivial to generate), as we're limiting the number of reservations to |
|
Hrm, you know, this might indeed better than what we have (refactoring aside, which certainly is :) My concern is that a bunch of peers connect to you, make a reservation each, go over the liimit, and then just disconnect. |
Hm. I guess that's by design, isn't it? We limit each relay's utilization to values far below what they could handle, if they're running on decent hardware and with a decent connection. We do this so we can justify turning it on by default. @Stebalien, wdyt? |
|
@Stebalien Friendly ping. |
We could oversubscribe by some amount, but that's probably overkill. I'd leave it as-is for now and change it if we notice issues. |
|
It's always possible to give the hydras a high limit... |
marten-seemann
force-pushed
the
rework-limits
branch
from
cb04a66
to
54c4a93
Compare
|
|
||
| func (c *constraints) cleanup(now time.Time) { | ||
| c.total = c.cleanupList(c.total, now) | ||
| for k, peerReservations := range c.peers { |
There was a problem hiding this comment.
Small optimization: we can return early when nothing has expired (when c.total wasn't updated).
There was a problem hiding this comment.
I think you're right, but I'm not sure if it's worth it. This seems like an easy place to introduce non-obvious bugs in the future.
... and don't care about reservations that are removed (and fulfilled).
That means a relay won't issue more than 128 reservations per 30 minute interval, no matter how many of them are used. There are further limits per peer, IP and ASN, that all work the same (limiting the number of reservations per 30 minute interval).