Refine async payment cache admission#910
Conversation
Keep existing onion-message mailbox queues when the peer map is full, and recycle rate limiter bucket state when admitting a new user at capacity. Signed-off-by: Elias Rohrer <dev@tnull.de>
|
👋 Thanks for assigning @joostjager as a reviewer! |
joostjager
left a comment
joostjager
left a comment
There was a problem hiding this comment.
I’m fine with these changes, but I’m not sure they make the system strictly better so much as change the failure mode.
The more fundamental issue is of course that onion-message handling is exposed to Sybil-style pressure. Not sure if it was a good idea to rely on it for async payments.
| pub(crate) fn onion_message_intercepted(&self, peer_node_id: PublicKey, message: OnionMessage) { | ||
| let mut map = self.map.lock().expect("lock"); | ||
|
|
||
| if !map.contains_key(&peer_node_id) && map.len() >= Self::MAX_PEERS { |
There was a problem hiding this comment.
I think we talked about this before. Is it really better to allow an attacker to fill up all slots and DoS legit users?
| self.garbage_collect(self.max_idle); | ||
| if self.users.len() >= MAX_USERS { | ||
| return false; | ||
| self.evict_least_recently_seen(); |
There was a problem hiding this comment.
This can now be a rate-limiter bypass for attackers?
| fn garbage_collect(&mut self, max_idle: Duration) { | ||
| let now = Instant::now(); | ||
| self.users.retain(|_, bucket| now.duration_since(bucket.last_refill) < max_idle); | ||
| self.users.retain(|_, bucket| now.duration_since(bucket.last_seen) < max_idle); |
There was a problem hiding this comment.
The doc comment correctly now says "remained unused for longer than the max idle duration", but no test exercises the new last_seen-based GC distinct from the old last_refill behavior — e.g. a bucket whose last_refill is stale (full of tokens, never refilled) but recently touched via allow() should not be GC'd. Worth a unit test?
4 participants
Keep existing onion-message mailbox queues when the peer map is full, and recycle rate limiter bucket state when admitting a new user at capacity.