MSC4013: Poll history cache #4013

alfogrillo · 2023-05-10T09:22:44Z

uhoreg · 2023-05-10T16:57:53Z

proposals/4013-poll-history-cache.md

+```
+
+## Alternatives considered
+


It seems like using /search with "types": ["m.room.poll_history"] would be an alternative.

Do you mean an alternative to the /state?
If yes, does it make any difference?

I mean that, rather than creating a new state event and searching for relations on that state event, why not just use the /search endpoint to search for polls?

I guess because search doesn't work on encrypted room. Poll events are actually sent as m.room.encrypted. 😕
Fwiw in the main proposal just one state event is required.

That's a good reason. It might be worth mentioning that in the MSC.

For unencrypted room /messages with a filter on event type would be better than /search, FWIW. But again doesn't work with encrypted events.

proposals/4013-poll-history-cache.md

Co-authored-by: Travis Ralston <[email protected]>

FSG-Cat · 2023-05-12T17:39:17Z

proposals/4013-poll-history-cache.md

@@ -0,0 +1,183 @@
+# MSC4013: Poll history cache


I have to ask plain and simple where in this MSC do we dismiss the problem that users do not want metadata leaks like this. Or atleast a large group of quite loud users do not want metadata leaks like this.

Now This concern could be dismissed by the fact that polls already use relations, but atleast these relations cant tell you for sure its a poll you can only tell that the events are related. I personally would argue that its somewhat dubious why we even need relations to not respect encryption considering their current use.

Still do we need to add more metadata leaks? Matrix already gets bad articles/blog posts written about it for metadata leaking and MSCs like this one doesnt help that.

Encrypting the state event (#3414) would help mitigate this in future.

Encrypted state would mittigate a bit of the concerns but with plain text relations it would still leave a massive tell. as no other encrypted state event would be the target of this many relations, essentially undoing the whole point in encrypting the cache state event.

I have to ask plain and simple where in this MSC do we dismiss the problem that users do not want metadata leaks like this. Or atleast a large group of quite loud users do not want metadata leaks like this.

This feature should considered optional.
In theory room's creators aren't obliged to send the m.room.poll_history state event for a room.
At the same time clients sending a poll may choose to opt out from this behaviour.
At the same time structures like this are very useful on clients that want navigate content by type (polls in this case) and willing to pay the price of a small metadata leak.

I personally would argue that its somewhat dubious why we even need relations to not respect encryption considering their current use.

I guess the may reason is the server being able to group related events together efficiently. Eg. the same problem is solved in the same way for the "threads" feature.

Encrypted state would mittigate a bit of the concerns but with plain text relations it would still leave a massive tell. as no other encrypted state event would be the target of this many relations, essentially undoing the whole point in encrypting the cache state event.

Yes, probably people could still guess what would be the m.poll_history state event could be in the end, but at least they couldn't be 100% sure I guess.

If the state event is optional, the MSC should note that (I don't believe it does currently).

Aside from encrypting the whole state event though, the other option is encrypting the event relationship (like in MSC4019 or similar). I personally don't believe that encrypting the relationship on either end is a blocker for this MSC, however. Ideally we figure out how to encrypt both sides and then use both to protect metadata.

In the meantime, considering polls is quite far away from entering FCP itself, starting implementation unencrypted should be fine (as long as the client is comfortable with the risks). It may need to change in the future as other, adjacent, MSCs make forward progress though.

turt2live

I actually quite like how this is structured. I was concerned going in that this might end up being a m.room.pinned_events clone of sorts, and was pleasantly surprised to see relations being used like this.

Hopefully it works well in practice :)

proposals/4013-poll-history-cache.md

turt2live · 2023-05-24T00:35:49Z

proposals/4013-poll-history-cache.md

@@ -0,0 +1,183 @@
+# MSC4013: Poll history cache


If the state event is optional, the MSC should note that (I don't believe it does currently).

Aside from encrypting the whole state event though, the other option is encrypting the event relationship (like in MSC4019 or similar). I personally don't believe that encrypting the relationship on either end is a blocker for this MSC, however. Ideally we figure out how to encrypt both sides and then use both to protect metadata.

In the meantime, considering polls is quite far away from entering FCP itself, starting implementation unencrypted should be fine (as long as the client is comfortable with the risks). It may need to change in the future as other, adjacent, MSCs make forward progress though.

jonnyandrew · 2023-11-28T17:23:46Z

proposals/4013-poll-history-cache.md

+        // reference to the state event of type "m.room.poll_history"
+        "m.relates_to": {
+            "event_id": "poll_history_id",
+            "rel_type": "m.reference"
+        }    


Could it be an issue that the poll start event would no longer be a root event and therefore could not be the root of a thread?

From https://spec.matrix.org/v1.8/client-server-api/#threading

Threads [...] reference the thread root (the first event in a thread). It is not possible to create a thread from an event which itself is the child of an event relationship (i.e., one with an m.relates_to property).

Yes, it looks like this would preclude a poll from being a reply in a thread.

Yes, it looks like this would preclude a poll from being a reply in a thread.

This is also a potential problem, but I was actually referring to the fact that a thread root should not be related to any other event. What it means is that there is no way to discuss a poll in a thread, which I imagine is a natural use case for threads.

Yes, it looks like this would preclude a poll from being a reply in a thread.

Fwiw polls aren't a supported inside threads atm on Element clients.
It doesn't seem to me interesting as a use case.

but I was actually referring to the fact that a thread root should not be related to any other event

Why is that?

That was my understanding from the spec which states

It is not possible to create a thread from an event which itself is the child of an event relationship (i.e., one with an m.relates_to property).

I don't see why it shouldn't be possible though. Perhaps that part of the spec could be updated to accommodate this type of use case.

Perhaps the documentation meant that you cannot create a thread from an event having already a rel_type of type m.thread?
@clokep do you know is the home server has checks that may reject this events (events starting a thread from an event with a rel_type equal to m.reference)?

Perhaps the documentation meant that you cannot create a thread from an event having already a rel_type of type m.thread?

The documentation is correct AFAIK.

@clokep do you know is the home server has checks that may reject this events (events starting a thread from an event with a rel_type equal to m.reference)?

They will get rejected: https://github.com/matrix-org/synapse/blob/928e9648578d24dc9f5ed3476d629fca9a64b22a/synapse/handlers/message.py#L1382-L1386

They will get rejected: https://github.com/matrix-org/synapse/blob/928e9648578d24dc9f5ed3476d629fca9a64b22a/synapse/handlers/message.py#L1382-L1386

Looking into the code the rejection seems to happens just if the relation type is m.thread?
If this is the case, polls having a m.reference rel_type can be still be used as root for a thread?

Looking into the code the rejection seems to happens just if the relation type is m.thread?

If the new event's relation is a thread then it checks that the parent event does not have any relations. I think this situation would be rejected, but I did not check it manually.

I see, thanks for clarifying

alfogrillo changed the title ~~MSCXYZW: Poll history cache~~ MSC4013: Poll history cache May 10, 2023

alfogrillo force-pushed the alfogrillo/poll_history_cache branch 2 times, most recently from 62637f9 to e4037c6 Compare May 10, 2023 09:28

Add poll history cache md

f79503a

alfogrillo force-pushed the alfogrillo/poll_history_cache branch from e4037c6 to f79503a Compare May 10, 2023 09:29

Fix code block annotations

3735515

alfogrillo added proposal A matrix spec change proposal client-server Client-Server API labels May 10, 2023

alfogrillo added 2 commits May 10, 2023 11:54

Add unstable prefix section

135220e

Update MSC title

3474452

alfogrillo force-pushed the alfogrillo/poll_history_cache branch from f127517 to 3474452 Compare May 10, 2023 09:58

alfogrillo marked this pull request as ready for review May 10, 2023 13:17

turt2live added kind:feature MSC for not-core and not-maintenance stuff needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. labels May 10, 2023

uhoreg reviewed May 10, 2023

View reviewed changes

turt2live reviewed May 10, 2023

View reviewed changes

proposals/4013-poll-history-cache.md Outdated Show resolved Hide resolved

Alfonso Grillo and others added 2 commits May 11, 2023 10:16

Update proposals/4013-poll-history-cache.md

1585d97

Co-authored-by: Travis Ralston <[email protected]>

Add MSC dependencies

a5f7d0e

FSG-Cat reviewed May 12, 2023

View reviewed changes

turt2live approved these changes May 24, 2023

View reviewed changes

alfogrillo force-pushed the alfogrillo/poll_history_cache branch from ebd68ff to 4623275 Compare May 29, 2023 15:17

Wrap lines to 120 chars

2d2fa1c

alfogrillo force-pushed the alfogrillo/poll_history_cache branch from 4623275 to 2d2fa1c Compare May 29, 2023 15:20

turt2live mentioned this pull request Jun 1, 2023

MSC3381: Polls (mk II) #3381

Merged

jonnyandrew reviewed Nov 28, 2023

View reviewed changes

jonnyandrew mentioned this pull request Nov 29, 2023

[Tech] Implement poll history cache (MSC4013) in the rust SDK element-hq/element-meta#2183

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MSC4013: Poll history cache #4013

MSC4013: Poll history cache #4013

alfogrillo commented May 10, 2023 •

edited

Loading

uhoreg May 10, 2023 •

edited

Loading

alfogrillo May 11, 2023

uhoreg May 11, 2023

alfogrillo May 11, 2023

uhoreg May 11, 2023

clokep May 12, 2023

FSG-Cat May 12, 2023

Johennes May 15, 2023

FSG-Cat May 15, 2023

alfogrillo May 15, 2023

turt2live May 24, 2023

turt2live left a comment

turt2live May 24, 2023

jonnyandrew Nov 28, 2023 •

edited

Loading

clokep Nov 28, 2023

jonnyandrew Nov 29, 2023

alfogrillo Nov 29, 2023

jonnyandrew Nov 29, 2023

alfogrillo Nov 29, 2023

clokep Nov 29, 2023

alfogrillo Nov 29, 2023

clokep Nov 29, 2023

alfogrillo Nov 29, 2023

MSC4013: Poll history cache #4013

Are you sure you want to change the base?

MSC4013: Poll history cache #4013

Conversation

alfogrillo commented May 10, 2023 • edited Loading

uhoreg May 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turt2live left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonnyandrew Nov 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alfogrillo commented May 10, 2023 •

edited

Loading

uhoreg May 10, 2023 •

edited

Loading

jonnyandrew Nov 28, 2023 •

edited

Loading