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

MSC3998: Add timestamp massaging to /join and /knock #3998

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

Conversation

MadLittleMods
Copy link
Contributor

@MadLittleMods MadLittleMods commented Apr 13, 2023

MSC3998: Add timestamp massaging to /join?ts=123 and /knock?ts=123


Rendered

Server implementations:

  • Implementations needed

Other related references:

@MadLittleMods MadLittleMods changed the title MSCXXXX: Add proposal for /join and /knock timestamp massaging MSC3998: Add proposal for /join and /knock timestamp massaging Apr 13, 2023
@MadLittleMods MadLittleMods changed the title MSC3998: Add proposal for /join and /knock timestamp massaging MSC3998: Add proposal for /join and /knock timestamp massaging Apr 13, 2023
@MadLittleMods MadLittleMods marked this pull request as ready for review April 13, 2023 20:09
@MadLittleMods MadLittleMods changed the title MSC3998: Add proposal for /join and /knock timestamp massaging MSC3998: Add timestamp massaging to /join and /knock Apr 13, 2023
@@ -0,0 +1,98 @@
# MSC3998: Add timestamp massaging to `/join` and `/knock`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc @ara4n as the original ?ts proposer from the old version of MSC2716

cc @tulir as an interested party in timestamp massaging (author of MSC3316 and one of the main consumers of the ?ts API)

@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 13, 2023
@@ -0,0 +1,98 @@
# MSC3998: Add timestamp massaging to `/join` and `/knock`
Copy link
Member

Choose a reason for hiding this comment

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

It's not entirely clear to me why /createRoom is isolated from these endpoints at the moment. To ease discussion, it might make sense to merge the MSCs into a single place to catch what other endpoints might need this behaviour then break it up (if needed) prior to FCP.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They need timestamp massaging for different reasons and alternatives vary. I think it makes more sense to split them up but it does make sense to keep the bigger picture in mind.

Comment on lines +32 to +36
In real-life scenarios, practically, this hasn't mattered much for content because the
DAG is ordered topologically and not by timestamp but is a semantic inconsistency that
is becoming more important with API's like `/timestamp_to_event` which find events by
their `origin_server_ts`. And makes things tricky for the Matrix Public Archive to
navigate history by date seamlessly assuming good intentions.
Copy link
Contributor Author

@MadLittleMods MadLittleMods Apr 27, 2023

Choose a reason for hiding this comment

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

Add this context to the MSC: It also mattered to us in real life with historical Gitter rooms where we imported history and ran into these messy client issues:

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.

2 participants