search

vacp2p / nim-libp2p

libp2p implementation in Nim
https://vacp2p.github.io/nim-libp2p/docs/
MIT License
240 stars 52 forks source link

feat: add maxSize to TimedCache #1132

Open diegomrsantos opened 1 week ago

diegomrsantos commented 1 week ago

This PR adds an optional maxSize to TimedCache. In this way, we can avoid the cache growing unbounded.

arnetheduck commented 1 week ago

Where is this complexity needed? The seen cache doesn't need it since all entries go through validation and therefore the peer will be descored if they add junk to it - the idontwant cache is limited in size already, here: https://github.com/vacp2p/nim-libp2p/blob/0f27f896ab69b40e8f566e77646c6980238a3a13/libp2p/protocols/pubsub/gossipsub/behavior.nim#L309

diegomrsantos commented 4 days ago

Where is this complexity needed?

I wouldn't call it complexity, even if we don't use it right now, being able to specify a max size is a common feature in data structures. IMO it doesn't make the code much harder to understand either.

diegomrsantos commented 4 days ago

the idontwant cache is limited in size already, here:

https://github.com/vacp2p/nim-libp2p/blob/0f27f896ab69b40e8f566e77646c6980238a3a13/libp2p/protocols/pubsub/gossipsub/behavior.nim#L309

I'd argue that what is harder to understand and error-prone is adding a limit somewhere else in the code than in the data structure itself.