unsigned.redacted_because field is required but optional

[kegsay]

Link to problem area: https://spec.matrix.org/latest/client-server-api/#redactions

Servers should include a copy of the m.room.redaction event under unsigned as redacted_because when serving the redacted event to clients.

(emph mine). In reality, this field is how clients like Rust SDK detect if an event is redacted or not. This detection then changes how the event is deserialised, to make previously required fields optional. It's a super sane and reasonable approach to take. However, this field is optional. When the sliding sync proxy implemented redactions, it did not set this field, causing breakages on Element X, due to ruma (the underlying rust library) failing to deserialise the redacted form of the event.

There's a few options here, going from most lax to most strict:

• say what ruma does is wrong and make it not do that, forcing it to loosen validation checks.
• say what ruma does isn't advised, but not wrong, and make it optimistically do that.
• provide some other required field to use as a way to detect redacted-ness and tell ruma to use that.
• change the spec to enforce that unsigned.redacted_because is always set for redacted events.

I think the extremes are bad. Less validation on clients is terrible and increases the scope for data errors. Enforcing that the field is always set is potentially impossible to do in the following case:

• Send m.room.name with content.name set.
• Redact the event, causing content: {}.
• Send 100 events.
• Join the room from a different server.
• This new server sees the redacted room name event in room state, but not the redaction event itself.
• At this point, to detect "redactedness", the joining server must trust (!) unsigned.redacted_because A) being set and B) truthful (which it may not be!).
• If we have to trust other servers to set fields to make guarantees to our own clients, then we cannot guarantee this at all.

I suspect the 2nd option is the only possible option here: "say what ruma does isn't advised, but not wrong, and make it optimistically do that." as it isn't possible for servers to always detect redactions, so they can neither guarantee unsigned.redacted_because is set nor any other key to signal redactedness.

Either way, the spec should go into more detail about this in the redactions section of the CSAPI.

[richvdh]

Yes. The js-sdk also uses redacted_because to decide if an event is redacted.

My interpretation of this text is actually option 4: servers MUST set redacted_because on redacted events. The spec tends to use the word "should" rather sloppily when it means MUST (#701).

However, as you point out, this is impractical: there are reasons for an event to be redacted in which the server doesn't have the redaction event.

[richvdh]

All that said:

causing breakages on Element X, due to ruma (the underlying rust library) failing to deserialise the redacted form of the event.

This seems like the real problem here. If clients are relying on events having a particular shape, they are doing it wrong, as explained at https://spec.matrix.org/latest/client-server-api/#room-event-format. If the event has the wrong shape, you should ignore it, not explode.

[jplatte]

causing breakages on Element X, due to ruma (the underlying rust library) failing to deserialise the redacted form of the event.

This seems like the real problem here. If clients are relying on events having a particular shape, they are doing it wrong, as explained at https://spec.matrix.org/latest/client-server-api/#room-event-format. If the event has the wrong shape, you should ignore it, not explode.

I don't think the breakages were ever that the client or the SDK "exploded".

[richvdh]

In which case, I'm maybe misunderstanding, What exactly was the failure mode?