Skip to content

Use the generation time, not last seen time, for update timestamps #96

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

Closed
wants to merge 1 commit into from

Conversation

TheBlueMatt
Copy link
Contributor

@TheBlueMatt TheBlueMatt commented Apr 29, 2025

When we write an update, we have to include a timestamp that represents both the the time that the client will use to fetch the next diff and the timestamp used as a reference point when adding updates to the network graph (the client actually uses the included timestamp minus two weeks).

Because of the second use, updates were being generated with the highest timestmap of all included updates, rounded down to the nearest update-interval multiple (because of the first use).

In practice, because we expect to see many updates in any hour window, this meant we were really calculating the generation reference timestamp in a very indirect way. Here we simply change to using the reference timestamp directly.

This updates regtest results when generating an initial sync against an empty graph to include a real timestamp, rather than 0, though this shouldn't impact clients' correctness.

When we write an update, we have to include a timestamp that
represents both the the time that the client will use to fetch the
next diff and the timestamp used as a reference point when adding
updates to the network graph (the client actually uses the included
timestamp minus two weeks).

Because of the second use, updates were being generated with the
highest timestmap of all included updates, rounded down to the
nearest update-interval multiple (because of the first use).

In practice, because we expect to see many updates in any hour
window, this meant we were really calculating the generation
reference timestamp in a very indirect way. Here we simply change
to using the reference timestamp directly.

This updates regtest results when generating an initial sync
against an empty graph to include a real timestamp, rather than 0,
though this shouldn't impact clients' correctness.
@TheBlueMatt
Copy link
Contributor Author

Closing since the tests rely on this behavior a good bit and while this is a fine cleanup its not really that important.

@TheBlueMatt TheBlueMatt closed this May 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant