Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revise the usage of Time and TimeGetter in p2p #1443

Open
ImplOfAnImpl opened this issue Jan 11, 2024 · 0 comments
Open

Revise the usage of Time and TimeGetter in p2p #1443

ImplOfAnImpl opened this issue Jan 11, 2024 · 0 comments
Labels
p2p p2p related issues

Comments

@ImplOfAnImpl
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2p p2p related issues
Projects
None yet
Development

No branches or pull requests

1 participant