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

MSC3999: Add causal parameter to /timestamp_to_event #3999

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

MadLittleMods
Copy link
Contributor

@MadLittleMods MadLittleMods commented Apr 14, 2023

MSC3999: Add causal ?event_id=$abc query parameter to /timestamp_to_event to signal that we want to keep progressing from this event regardless of what timestamp shenanigans are going on.


Rendered

Server implementations:

  • Implementations needed

Other related references:

@MadLittleMods MadLittleMods changed the title MSCXXXX: Add causal parameter to /timestamp_to_event MSC3999: Add causal parameter to /timestamp_to_event Apr 14, 2023
@MadLittleMods MadLittleMods marked this pull request as ready for review April 14, 2023 11:13
@turt2live turt2live added proposal A matrix spec change proposal client-server Client-Server API kind:maintenance MSC which clarifies/updates existing spec needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. labels Apr 14, 2023
Comment on lines +28 to +30
Instead of only defining a timestamp, we add an optional `event_id` query parameter
which represents a topological spot in the DAG that we can easily determine what comes
before or after from (a casual relationship).
Copy link
Contributor Author

@MadLittleMods MadLittleMods Apr 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should clarify that this should be added to the client and federation (server) /timestamp_to_event API's

Comment on lines +28 to +29
Instead of only defining a timestamp, we add an optional `event_id` query parameter
which represents a topological spot in the DAG that we can easily determine what comes
Copy link
Contributor Author

@MadLittleMods MadLittleMods Apr 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Event ID's are globally unique but we're after a topological position in the DAG which is per-room so the event lookup should be constrained to the room.

Comment on lines +28 to +29
Instead of only defining a timestamp, we add an optional `event_id` query parameter
which represents a topological spot in the DAG that we can easily determine what comes
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when event_id can't be found?

Should the homeserver try to backfill the event from other homeservers?

What error code should we throw if we can't find it in any case?


In less scrupulous scenarios or with bad intentioned actors, these timestamp loops can
occur throughout the room timeline.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another case why the causal parameter is useful: Events can occur at the same exact time and it's not possible to get the next one with /timestamp_to_event atm

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the purpose is to allow Matrix Public Archive to go back through events in the room, why not use /messages and the from/to/start/end fields for pagination?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client-server Client-Server API kind:maintenance MSC which clarifies/updates existing spec needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. proposal A matrix spec change proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants