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
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?
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.