You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
The text was updated successfully, but these errors were encountered:
Time
andTimeGetter
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 (viatokio::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 usingInstant
, 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.The text was updated successfully, but these errors were encountered: