feat: expire messages from the cache based on last seen time

[smrz2001]

This PR attempts to fix https://github.com/libp2p/go-libp2p-pubsub/issues/502.

It also includes a thread-safe copy of the original time cache implementation.

Both the "first seen" (original) and "last seen" (new) implementations have been abstracted behind the TimeCache interface in a new timecache package.

cc @vyzo

[vyzo]

thanks, i was on vacation. I will take a look soon.

[BigLep]

@vyzo : after this is reviewed/merged, can you cut a new release? (I see this repo isn't part of unified ci: https://github.com/protocol/.github/blob/master/configs/go.json). I'm asking so it can be bubbled up to Kubo for https://github.com/ipfs/kubo/pull/9372/files#diff-a75543978c37023721256291000855b02598e5d72668e203f7372cc4b8c159cfR222

[vyzo]

sure

[BigLep]

@vyzo : I know you're juggling a lot. Would it be helpful if someone from the EngRes libp2p Stewards team takes a look?

[vyzo]

It has been a little hectic in fvm land recently, but i do have some time to look at this during the weekend.

[vyzo]

also, feel free to add steward reviewers, it would help disseminate knowledge.

[vyzo]

yes, yes, my weekend plan didnt materialize, but i have it on the list.

[smrz2001]

@vyzo I eyeballed it, but any chance you take a look at this week?

@smrz2001 quick ask: unify default strategy (right now it is different https://github.com/libp2p/go-libp2p-pubsub/pull/513/files#r1071344615)

Fixed! Thanks, @lidel.

[BigLep]

@MarcoPolo @marten-seemann @Wondertan : any chance you can please help take a look at this? I'd like to get this over the line to get @smrz2001 and team unblocked.

[Wondertan]

Happy to review this on the weekends if it will be relevant until then.

[vyzo]

Please retain the default behaviour of first seen.

[oed]

Please retain the default behaviour of first seen.

@vyzo you said previously (https://github.com/libp2p/go-libp2p-pubsub/issues/502#issuecomment-1292586489) that it should be changed. This was the point of the bug that was reported.

[vyzo]

Yes, i know, but i had achange of heart regarding the default behaviour.

It is true that it is desirable to use the last seen behaviour for many applications.

However, some of the biggest users of this library are blockchains which can quite easily post reject and deduplicate messages once the timecache period has expired, because of consensus. That is, after two minutes almost all blockchains can reject older messages reappearing and replays with their consensus based validators (nonce, block height, and so on).

For these applications, changing the default under their feet will not bring any tangible benefit for the increased memory usage. At the very least i think we should be conservative and let the user choose.

Note that we can also consider making last seen the default a bit further down the line, say in a release or two. Also, maturity of implementation concerns.

Does this make sense?

[oed]

It does make sense. Thanks for explaining your reasoning @vyzo

[vyzo]

@lidel you are blocking merge.

[smrz2001]

ok, lets merge it and follow up for background gc.

Great, thanks so much @vyzo and @lidel!

I opened #515 for the GC follow-up discussion.