Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Allow events to be created with no prev_events (MSC2716) #11243

Merged
merged 14 commits into from
Dec 11, 2021
Merged

Allow events to be created with no prev_events (MSC2716) #11243

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
39efad1
Allow events to be created with no prev_events
MadLittleMods Nov 3, 2021
66f0859
Add changelog
MadLittleMods Nov 3, 2021
9f45d09
Allow member state events without prev_events
MadLittleMods Nov 3, 2021
e093481
More clarification and specificity in errors/comments
MadLittleMods Nov 4, 2021
4bb5fd3
Add tests for creating events with empty prev_events
MadLittleMods Nov 5, 2021
f421a2d
Fix lints
MadLittleMods Nov 5, 2021
d10625e
Update test description to be accurate
MadLittleMods Nov 8, 2021
2370dca
Remove redundant length check
MadLittleMods Nov 9, 2021
e2928b5
No new room version necessary and allow no prev_events via allow_no_p…
MadLittleMods Dec 1, 2021
85364c5
Remove extra verbose check
MadLittleMods Dec 1, 2021
0ed300e
Only worry about having auth_events when allowing no prev_events
MadLittleMods Dec 11, 2021
93ca0f8
Update changelog to describe intended usage
MadLittleMods Dec 11, 2021
9b9deff
Merge branch 'develop' into madlittlemods/empty-prev-events
MadLittleMods Dec 11, 2021
8fada7e
Fix lint
MadLittleMods Dec 11, 2021
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
31 changes: 31 additions & 0 deletions synapse/api/room_versions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ class RoomVersion:
msc2716_historical = attr.ib(type=bool)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the tests! This lgtm, I'd like to just ask another member of the team to double-check the changes make sense (and don't have any security implications).

Thanks for the review @anoadragon453 🦈

# MSC2716: Adds support for redacting "insertion", "chunk", and "marker" events
msc2716_redactions = attr.ib(type=bool)
# MSC2716: Adds support for events with no `prev_events` but with some `auth_events`
msc2716_empty_prev_events = attr.ib(type=bool)


class RoomVersions:
Expand All @@ -99,6 +101,7 @@ class RoomVersions:
msc2403_knocking=False,
msc2716_historical=False,
msc2716_redactions=False,
msc2716_empty_prev_events=False,
)
V2 = RoomVersion(
"2",
Expand All @@ -115,6 +118,7 @@ class RoomVersions:
msc2403_knocking=False,
msc2716_historical=False,
msc2716_redactions=False,
msc2716_empty_prev_events=False,
)
V3 = RoomVersion(
"3",
Expand All @@ -131,6 +135,7 @@ class RoomVersions:
msc2403_knocking=False,
msc2716_historical=False,
msc2716_redactions=False,
msc2716_empty_prev_events=False,
)
V4 = RoomVersion(
"4",
Expand All @@ -147,6 +152,7 @@ class RoomVersions:
msc2403_knocking=False,
msc2716_historical=False,
msc2716_redactions=False,
msc2716_empty_prev_events=False,
)
V5 = RoomVersion(
"5",
Expand All @@ -163,6 +169,7 @@ class RoomVersions:
msc2403_knocking=False,
msc2716_historical=False,
msc2716_redactions=False,
msc2716_empty_prev_events=False,
)
V6 = RoomVersion(
"6",
Expand All @@ -179,6 +186,7 @@ class RoomVersions:
msc2403_knocking=False,
msc2716_historical=False,
msc2716_redactions=False,
msc2716_empty_prev_events=False,
)
MSC2176 = RoomVersion(
"org.matrix.msc2176",
Expand All @@ -195,6 +203,7 @@ class RoomVersions:
msc2403_knocking=False,
msc2716_historical=False,
msc2716_redactions=False,
msc2716_empty_prev_events=False,
)
V7 = RoomVersion(
"7",
Expand All @@ -211,6 +220,7 @@ class RoomVersions:
msc2403_knocking=True,
msc2716_historical=False,
msc2716_redactions=False,
msc2716_empty_prev_events=False,
)
V8 = RoomVersion(
"8",
Expand All @@ -227,6 +237,7 @@ class RoomVersions:
msc2403_knocking=True,
msc2716_historical=False,
msc2716_redactions=False,
msc2716_empty_prev_events=False,
)
V9 = RoomVersion(
"9",
Expand All @@ -243,6 +254,7 @@ class RoomVersions:
msc2403_knocking=True,
msc2716_historical=False,
msc2716_redactions=False,
msc2716_empty_prev_events=False,
)
MSC2716v3 = RoomVersion(
"org.matrix.msc2716v3",
Expand All @@ -259,6 +271,24 @@ class RoomVersions:
msc2403_knocking=True,
msc2716_historical=True,
msc2716_redactions=True,
msc2716_empty_prev_events=False,
)
MSC2716v4 = RoomVersion(
"org.matrix.msc2716v4",
RoomDisposition.UNSTABLE,
EventFormatVersions.V3,
StateResolutionVersions.V2,
enforce_key_validity=True,
special_case_aliases_auth=False,
strict_canonicaljson=True,
limit_notifications_power_levels=True,
msc2176_redaction_rules=False,
msc3083_join_rules=False,
msc3375_redaction_rules=False,
msc2403_knocking=True,
msc2716_historical=True,
msc2716_redactions=True,
msc2716_empty_prev_events=True,
)


Expand All @@ -276,6 +306,7 @@ class RoomVersions:
RoomVersions.V8,
RoomVersions.V9,
RoomVersions.MSC2716v3,
RoomVersions.MSC2716v4,
)
}

Expand Down
22 changes: 16 additions & 6 deletions synapse/handlers/message.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -949,14 +949,24 @@ async def create_new_client_event(
else:
prev_event_ids = await self.store.get_prev_events_for_room(builder.room_id)

# we now ought to have some prev_events (unless it's a create event).
#
# do a quick sanity check here, rather than waiting until we've created the
# Do a quick sanity check here, rather than waiting until we've created the
# event and then try to auth it (which fails with a somewhat confusing "No
# create event in auth events")
assert (
builder.type == EventTypes.Create or len(prev_event_ids) > 0
), "Attempting to create an event with no prev_events"
room_version_obj = await self.store.get_room_version(builder.room_id)
if room_version_obj.msc2716_empty_prev_events:
Copy link
Contributor Author

@MadLittleMods MadLittleMods Nov 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This would be good, but will obviously not be backwards compatible. We probably want to add this to a new room version?

-- @erikjohnston

Does this actually require a new room version? (discussed at #11114 (comment))

My thinking is that this code is only for creating events, not accepting events. So technically any other homeserver nowadays can create events with no prev_events now and it would work in existing room versions.

Am I missing something?

Copy link
Member

Choose a reason for hiding this comment

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

I am not that surprised that we accept outliers with empty prev-events, but we'll need special handling for the insertion event to be accepted, i.e. we'll need some sort of check like "this is an insertion event so we need to go and fetch the state rather than trying to calculate it". Though that can be part of the history import MSC I guess

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we'll need special handling for the insertion event to be accepted, i.e. we'll need some sort of check like "this is an insertion event so we need to go and fetch the state rather than trying to calculate it"

Why is this the case? It seems to work in my Complement tests without any of this 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Spooky! 👻 Is it doing something silly like dropping the event with no extremities and then doing a /state on the event that references it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure yet. If you really want me to dive into it, I can ⛳

# We allow events with no `prev_events` but it better have some `auth_events`
assert (
builder.type == EventTypes.Create
or len(prev_event_ids) > 0
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved
# Allow an event to have empty list of prev_event_ids
# only if it has auth_event_ids.
or (auth_event_ids and len(auth_event_ids) > 0)
erikjohnston marked this conversation as resolved.
Show resolved Hide resolved
), "Attempting to create an event with no prev_events or auth_event_ids"
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved
else:
# we now ought to have some prev_events (unless it's a create event).
assert (
builder.type == EventTypes.Create or len(prev_event_ids) > 0
), "Attempting to create an event with no prev_events"
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved

event = await builder.build(
prev_event_ids=prev_event_ids,
Expand Down