Description
Time
and TimeGetter
are used frequently in p2p to represent/obtain some abstract point in time, whose only purpose is to be able to calculate a time difference later. The problem is that the "production" implementation of the time getter uses the wall clock, which is not monotonic, so there is a possibility for the difference to become negative (and even if it's positive, a forward time jump can mess timeouts).
One solution for this is to use Instant::now()
instead of the time getter in such situations; the downside is that this will make the time much harder to mock in tests (via tokio::time::advance
) and some existing tests will have to be refactored to certain extent.
On the other hand, p2p sometimes uses tokio::time::timeout
, which is basically the same as using Instant
, and it's inconsistent with the TimeGetter-based approach.
The proper solution is probably to have two separate time getters or to make the current one be able to provide both kinds of time.
Also, the usages of tokio::time::timeout
should probably be replaced with a custom timeout function that would use the monotonic time getter. But this will require some refactoring of existing tests, because advancing the timer getter will now be required for p2p components to make progress.