Expire channels that have been unused for too long

Expire unused channels based on the discussion in issue #41 (closed) .

@nickm Hi Nick, I have drafted some changes to expire unused channels. Hope I get the idea correctly.

requested review from @Diziet

Assigning myself as reviewer since I've been discussing this with @0x00A5 over the past while.

requested review from @nickm and removed review request for @Diziet

This is an O(n) operation on a structure that used to be O(1) all the time. Maybe we should have the CircMap itself keep track of how many circuits are open-or-opening?

That could be in a different commit, though. We should just make sure not to forget about it.

While working on resolving this, I noticed the CircMap::get_mut method.

pub(super) fn get_mut(&mut self, id: CircId) -> Option<&mut CircEnt>

I figured to keep track of the open circs counter in all other methods but this one is a bit tricky. I guess one can change an entry from open/opening to destroy-sent (or vice versa) with the mutable reference returned, and the counter will be out of sync.

changed this line in version 2 of the diff

Yeah: we'll probably need to change or replace this API if we want to have O(1) behavior here.

A good start! I've left some comments and suggestions.

added 66 commits

• 1e80e8e8...4413645c - 65 commits from branch tpo/core:main
• 8a0e2df6 - Expire channels that have been unused for too long

Compare with previous version

I think using Mutex to wrap an atomic value might be a bit strange? If I replace the whole Mutex<Option<Timestamp>> with just Timestamp, would it actually be a good idea to use never updated timestamps to represent the still-in-use state of channels?

Hm... I think this is possible, but it might require a little work. Here's what I had in mind to get rid of the Mutex.

I'd define a new OptTimestamp type, based on Timestamp, except with the value 0 representing None. It would behave semantically like Option<Timestamp>, but internally it would just be an AtomicU64. Then I'd give it both update() and clear() functions to change its state, along with the other accessors that the functions update_disused_since and duration_unused would need.

That's kind of a lot, though. I'm happy to do try this myself post-merge, unless you want to take it on.

added 40 commits

• 8a0e2df6...cf4edcac - 39 commits from branch tpo/core:main
• f6f732a4 - Expire channels that have been unused for too long

Compare with previous version