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

MSC3925: m.replace aggregation with full event #3925

Merged
merged 34 commits into from
Feb 21, 2023
Merged
Changes from 6 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
116eb4e
init m.replace aggregation with full event
benkuly Nov 3, 2022
88c434d
Rename tmp.md to 3925-replace-aggregation-with-full-event.md
benkuly Nov 3, 2022
5a54a03
Update 3925-replace-aggregation-with-full-event.md
benkuly Nov 3, 2022
a065b00
Update 3925-replace-aggregation-with-full-event.md
benkuly Nov 3, 2022
4f5ecde
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 4, 2022
6ea5486
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 4, 2022
09602eb
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 8, 2022
3876a46
redact instead of delete
benkuly Nov 8, 2022
97dfdce
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 8, 2022
c0ec33d
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 8, 2022
f7fa717
Merge branch 'main' of github.com:benkuly/matrix-spec-proposals
benkuly Nov 8, 2022
0bba4e4
word wrap 80
benkuly Nov 8, 2022
db0ca3e
remove immutable argument
benkuly Nov 8, 2022
ff6fc37
add alternative from https://github.com/matrix-org/matrix-spec/issues…
benkuly Nov 8, 2022
291b921
add json example
benkuly Nov 8, 2022
8a4f38f
shorter version of the actual proposal
benkuly Nov 8, 2022
e365e01
describe the actual reason, why encrypted events cannot be replaced
benkuly Nov 8, 2022
05ecbe5
mention discussion
benkuly Nov 8, 2022
919edb5
typo
benkuly Nov 8, 2022
8fb8814
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 11, 2022
eac5551
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 11, 2022
5aced11
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 11, 2022
fe75f9c
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 11, 2022
767acad
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 11, 2022
e3a461e
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 11, 2022
b84a03a
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 11, 2022
5bfffb4
clarify inconsistent behavior with replaced contents
benkuly Nov 11, 2022
1ea94a2
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 11, 2022
fae2c26
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 15, 2022
ee0dffa
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 15, 2022
fd37307
Update proposals/3925-replace-aggregation-with-full-event.md
benkuly Nov 15, 2022
efc6060
clarify inconsistent behavior and put it into introduction
benkuly Nov 15, 2022
64da169
remove superfluous event content field
benkuly Feb 8, 2023
3d9e2ab
add some details to potential issues
benkuly Feb 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions proposals/3925-replace-aggregation-with-full-event.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# MSC3925: m.replace aggregation with full event

The Matrix DAG has immutable PDUs forming an auth chain. The client representation of this PDUs are simply called events.
benkuly marked this conversation as resolved.
Show resolved Hide resolved
While these events also have been immutable until v1.3, since v1.4 they aren't.
richvdh marked this conversation as resolved.
Show resolved Hide resolved
When a client sends a `m.replace` relation, [the server should replace the content of the original event](https://spec.matrix.org/v1.4/client-server-api/#server-side-replacement-of-content).

There are some issues with this requirement:
* Changing the fundamental concept of immutable events is confusing. The server can respond with different event contents for the same `event_id`.
* If an event with `m.replace` relation is deleted, the client would need to detect, if the original content was replaced and possibly needs to fetch the original content.
benkuly marked this conversation as resolved.
Show resolved Hide resolved
* There is an additional server call needed, when the replacing event is encrypted, because the server cannot replace the original event content.
benkuly marked this conversation as resolved.
Show resolved Hide resolved
* There are also some other issues with this spec paragraph, which are discussed [here](https://github.com/matrix-org/matrix-spec/issues/1299)
benkuly marked this conversation as resolved.
Show resolved Hide resolved

## Proposal

Instead of replacing the original content of an event, servers should use the aggregation feature for it.
In fact it is [already used](https://spec.matrix.org/v1.4/client-server-api/#server-side-aggregation-of-mreplace-relationships),
but only `event_id`, `origin_server_ts` and `sender` are included.
Theoretically this is enough to get the replacing content, but when the event with the `event_id` cannot be found locally it needs to be fetched from the server.
To prevent this additional call to the server, the `m.replace` aggregation should just contain the complete replacing event.
benkuly marked this conversation as resolved.
Show resolved Hide resolved

The additional server call is already needed for encrypted events and would be saved by this proposal too.
benkuly marked this conversation as resolved.
Show resolved Hide resolved

## Potential issues

* There could be clients, which rely on the current behavior.
benkuly marked this conversation as resolved.
Show resolved Hide resolved
* It is not as easy for clients like as in the current spec to get the current content of an event by just looking into `content.body`. While this true, it is also a relatively inconsistent behavior. Future replacements of the event would be rendered as "* new content". So the event with the replaced event does look different (without "*") despite the fact, that it is also replaced.
benkuly marked this conversation as resolved.
Show resolved Hide resolved

## Alternatives


richvdh marked this conversation as resolved.
Show resolved Hide resolved

## Security considerations

benkuly marked this conversation as resolved.
Show resolved Hide resolved


## Unstable prefix

I'm not sure, if we need an unstable prefix, because the aggregation would just be extended by additional fields.
richvdh marked this conversation as resolved.
Show resolved Hide resolved