MSC4242: State DAGs by kegsay · Pull Request #841 · matrix-org/complement

kegsay · 2026-02-02T15:22:58Z

MSC4242: State DAGs

Pull Request Checklist

Pull request includes a sign off

MadLittleMods

Partial review

Reviewed up to TestMSC4242OnSendJoinSJ04

Haven't looked at the utils in tests/msc4242/msc4242_api_test.go yet

tests/msc4242/msc4242_e2e_test.go

tests/msc4242/msc4242_api_test.go

MadLittleMods · 2026-02-19T21:55:55Z

tests/msc4242/msc4242_faulty_events_test.go

+						PrevEvents: []string{room.CurrentState(spec.MRoomMember, sender).EventID()},
+					},
+					// no event in the state DAG has this event ID.
+					PrevStateEvents: []string{"$4PRgaFIMcD9z4vzgkUUm0YI5CZHYORUPzWGJac6guAo"},


We can name this more obviously

Suggested change

PrevStateEvents: []string{"$4PRgaFIMcD9z4vzgkUUm0YI5CZHYORUPzWGJac6guAo"},

PrevStateEvents: []string{"$eventIdDoesNotExist"},

(also applies below)

That's not the same because it isn't a valid event ID.

What's not valid about it?

$opaque_id

However, the precise format depends upon the room version specification: early room versions included a domain component, whereas more recent versions omit the domain and use a base64-encoded hash instead.

-- https://spec.matrix.org/v1.16/appendices/#event-ids

It seems like the only requirement maybe is that it should be base64 decodable but there isn't any hash comparison to do so I don't think that's a requirement either.

3.3 Event IDs

The event ID is the reference hash of the event encoded using a variation of Unpadded Base64 which replaces the 62nd and 63rd characters with - and _ instead of using + and /. This matches RFC4648’s definition of URL-safe base64.

Event IDs are still prefixed with $ and might result in looking like $Rqnc-F-dvnEYJTyHq_iKxU2bZ1CI92-kuZq3a5lr5Zg.

-- https://spec.matrix.org/v1.16/rooms/v12/#event-ids

No, I want this to look like a normal event ID, else HSes may reject early because it isn't a valid SHA256 (length).

-- @kegsay, #841 (comment)

We can pad it out however long it needs to be. But seems like servers should treat these opaquely.

Suggested change

PrevStateEvents: []string{"$4PRgaFIMcD9z4vzgkUUm0YI5CZHYORUPzWGJac6guAo"},

PrevStateEvents: []string{"$eventIdDoesNotExist000000000000000000000000"},

MadLittleMods · 2026-02-19T23:54:12Z

tests/msc4242/msc4242_send_join_test.go

+}
+
+/*
+func TestMSC4242SendJoinFasterSJ03Inbound(t *testing.T) {


Why commented out?

Because the Synapse implementation does not support faster room joins yet.

MadLittleMods · 2026-02-19T23:54:58Z

tests/msc4242/msc4242_send_join_test.go

+	defer cancel()
+	bob := srv.UserID("bob")
+	testCases := faultyEventTestCases
+	var mu sync.Mutex


What's the point of this lock? (comment)

This was added:

var mu sync.Mutex // protects faultyEvents

We should name it more obviously

tests/msc4242/msc4242_send_join_test.go

Co-authored-by: Eric Eastwood <erice@element.io>

MadLittleMods · 2026-02-20T20:20:42Z

tests/msc4242/msc4242_get_missing_events_test.go

+			if endIndex > len(eventIDsPerRequest) {
+				endIndex = len(eventIDsPerRequest)
+			}
+			wantEventIDs := eventIDsPerRequest[startIndex:endIndex]


We might as well move this lower down to the actual comparison

MadLittleMods · 2026-02-20T20:22:17Z

tests/msc4242/msc4242_get_missing_events_test.go

+	}
+	// Repeatedly calls /get_missing_events on hs1 starting from fromEvent, in batches of batchSize.
+	// If stateDAG is set, asks the server to walk the state DAG.
+	// Asserts that events are returned (over multiple requests) in the order provided by eventIDsPerRequest.


We should document which order eventIDsPerRequest should be in

MadLittleMods · 2026-02-20T20:23:55Z

tests/msc4242/msc4242_get_missing_events_test.go

+			if !slices.Equal(got, wantEventIDs) {
+				ct.Errorf(t, "failed to see correct event IDs in test '%s'. \nGot  %v \nWant %v", name, got, wantEventIDs)
+				return
+			}


To get a nicer diff, it would be nice to re-use the assertEventsInOrder utility from tests/csapi/room_messages_test.go

MadLittleMods · 2026-02-20T20:26:29Z

tests/msc4242/msc4242_get_missing_events_test.go

+				ct.Errorf(t, "failed to see correct event IDs in test '%s'. \nGot  %v \nWant %v", name, got, wantEventIDs)
+				return
+			}
+			if endIndex == len(eventIDsPerRequest) {


We have the explicit endIndex = len(eventIDsPerRequest) above to prevent over-runs but seems sketchy as code evolves over time.

Perhaps we should adjust the condition to be more forgiving or have an additional assert(endIndex <= eventIDsPerRequest, "programming error")

Suggested change

if endIndex == len(eventIDsPerRequest) {

if endIndex >= len(eventIDsPerRequest) {

MadLittleMods · 2026-02-20T20:28:40Z

tests/msc4242/msc4242_send_join_test.go

+	must.Equal(t, lastEvent.Exists(), true, "last timeline entry for room does not exist")
+	expectedJoinPrevStateEvents := prevStateEvents(t, lastEvent)
+	bob := srv.UserID("bob")
+	_, sendJoinResp := MustJoinRoom(t, srv, deployment, spec.ServerName("hs1"), roomID, bob)


Suggested change

_, sendJoinResp := MustJoinRoom(t, srv, deployment, spec.ServerName("hs1"), roomID, bob)

_, sendJoinResp := MustJoinRoom(t, srv, deployment, deployment.GetFullyQualifiedHomeserverName(t, "hs1"), roomID, bob)

MadLittleMods · 2026-02-20T22:14:41Z

tests/msc4242/msc4242_api_test.go

+				return room.Timeline[i].EventID()
+			}
+		}
+		t.Logf("%s: failed to find any state event in %d timeline events, no prev_state_events will be set!", room.RoomID, len(room.Timeline))


Is this ever desired? (or programming error)

MadLittleMods · 2026-02-20T22:16:07Z

tests/msc4242/msc4242_api_test.go

+			serversInRoom := room.ServersInRoom()
+			for _, srvName := range serversInRoom {
+				res.ServersInRoom = append(res.ServersInRoom, string(srvName))
+			}


Looks like this should be gated by omitServersInRoom

MadLittleMods · 2026-02-20T22:19:31Z

tests/msc4242/msc4242_api_test.go

+// LoadStateDAG loads all the events under state_dag and puts their event_id: prev_state_events into a map.
+// Performs no verification that the DAG is connected.
+func LoadStateDAG(t ct.TestLike, sendJoinResp fclient.RespSendJoin) (joinEventID string, stateDAG map[string][]string) {
+	roomVer := gomatrixserverlib.MustGetRoomVersion(roomVersion)
+	joinEvent, err := roomVer.NewEventFromTrustedJSON(sendJoinResp.Event, false)
+	must.NotError(t, "failed to load join event", err)
+	stateDAG = make(map[string][]string) // event_id => prev_state_events
+	for _, stateEventJSON := range sendJoinResp.StateDAG {
+		stateEvent, err := roomVer.NewEventFromTrustedJSON(stateEventJSON, false)
+		must.NotError(t, "failed to load event json", err)
+		stateEventGJSON := gjson.ParseBytes(stateEventJSON)
+		prevStateEvents := prevStateEvents(t, stateEventGJSON)
+		stateDAG[stateEvent.EventID()] = prevStateEvents
+	}
+
+	// Include the prev state events of the join event itself as that is what we care about
+	joinEventPrevStateEvents := prevStateEvents(t, gjson.ParseBytes(sendJoinResp.Event))
+	stateDAG[joinEvent.EventID()] = joinEventPrevStateEvents
+	return joinEvent.EventID(), stateDAG
+}


LoadStateDAG is unused

MadLittleMods · 2026-02-20T22:19:57Z

tests/msc4242/msc4242_api_test.go

+	return joinEvent.EventID(), stateDAG
+}
+
+// We have to implement our own /make|send_join dance here as gmsl is unaware of state DAGs.


MadLittleMods · 2026-02-20T22:20:52Z

tests/msc4242/main_test.go

+
+func TestMain(m *testing.M) {
+	complement.TestMain(m, "msc4242")
+}


Note: While I have read the entire MSC and this PR, I don't think I've onboarded enough to recognize if there are missing test cases. It's my first time with my own hand in the fire when it comes to State DAG's.

This PR is so large, it's probably worth having an LLM also sanity check this against the MSC for completeness.

MSC4242: State DAGs

e846372

kegsay requested review from a team as code owners February 2, 2026 15:22

kegsay mentioned this pull request Feb 3, 2026

MSC4242: State DAGs matrix-org/matrix-spec-proposals#4242

Open

This was referenced Feb 19, 2026

MSC4242: State DAGs (CSAPI) element-hq/synapse#19424

Open

MSC4242: State DAGs (Federation) element-hq/synapse#19425

Open

MadLittleMods reviewed Feb 20, 2026

View reviewed changes

kegsay and others added 4 commits February 20, 2026 15:35

Review comments

0510cd9

Apply suggestions from code review

5675156

Co-authored-by: Eric Eastwood <erice@element.io>

More review comments

c9a7ecf

More review comments

843218b

MadLittleMods reviewed Feb 20, 2026

View reviewed changes

	PrevStateEvents: []string{"$4PRgaFIMcD9z4vzgkUUm0YI5CZHYORUPzWGJac6guAo"},
	PrevStateEvents: []string{"$eventIdDoesNotExist"},

	if endIndex == len(eventIDsPerRequest) {
	if endIndex >= len(eventIDsPerRequest) {

	_, sendJoinResp := MustJoinRoom(t, srv, deployment, spec.ServerName("hs1"), roomID, bob)
	_, sendJoinResp := MustJoinRoom(t, srv, deployment, deployment.GetFullyQualifiedHomeserverName(t, "hs1"), roomID, bob)

Comments

Conversation

kegsay commented Feb 2, 2026 • edited by MadLittleMods Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Checklist

Uh oh!

MadLittleMods left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

3.3 Event IDs

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kegsay commented Feb 2, 2026 •

edited by MadLittleMods

Loading