age logic for events is broken

[matrixbot]

This issue has been migrated from #8429.

---

There is a bunch of code inside synapse which tries to add an unspecced age field to events.

The intention of such a field is to try to mitigate problems with incorrectly-set clocks. Rather than saying "this event was created at 12:00", we can say "this event was created 10 minutes ago"; for certain applications (notably VoIP signalling), that information is more useful. It would be more useful still if it was actually specced, but that's a side-issue.

Considering a flow from client to client over federation, I think the intention of the implementation is this:

• Server A receives event from sending client. It records its best estimate of the timestamp the client created the event (typically the time it received the request from the client) in a field in the event called age_ts.
• Server A creates a transaction including the event to send to server B. Just before it sends that transaction over the wire, it removes age_ts from the event and replaces it with a raw age field giving the number of milliseconds that have elapsed since age_ts.
• Server B receives the transaction, and recreates age_ts based on the time it received the transaction.
• Server B formats the event for a client, and replaces age_ts with age.

Assuming that a given server's clock is consistently inaccurate, it is kinda plausible, though obviously it neglects any delays between formatting transactions and them being received at the destination.

However, I suspect it doesn't actually work. The C-S code in synapse stores age_ts in the unsigned property of the event (which is probably correct no, we have internal_metadata for this sort of thing); however, the logic in the federation sender which tries to replace age_ts actually looks for the property in the top level of the event rather than in unsigned. It therefore leaves age_ts in place in unsigned. The federation receiver logic will accept an age in either unsigned or at the top level, and replaces it with an age_ts at the top level. The C-S code doesn't know to strip it out of there, which means it leaks out to the C-S API (see matrix-org/matrix-spec-proposals#2685).

This has clearly been broken for ages, and nobody has really noticed. Given none of it is specced, I am inclined to say that we should strip it all out. (Update 2023/01/17: it's specced nowadays.)