Skip to content

MSC4242: State DAGs #841

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.

Sign up for GitHub

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

Open
kegsay wants to merge 5 commits into
base: main
Choose a base branch
from
Open

MSC4242: State DAGs #841

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e846372
MSC4242: State DAGs
kegsay Feb 2, 2026
0510cd9
Review comments
kegsay Feb 20, 2026
5675156
Apply suggestions from code review
kegsay Feb 20, 2026
c9a7ecf
More review comments
kegsay Feb 20, 2026
843218b
More review comments
kegsay Feb 20, 2026
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
3 changes: 2 additions & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ require (
github.com/docker/go-connections v0.4.0
github.com/gorilla/mux v1.8.0
github.com/matrix-org/gomatrix v0.0.0-20220926102614-ceba4d9f7530
github.com/matrix-org/gomatrixserverlib v0.0.0-20250813150445-9f5070a65744
github.com/matrix-org/gomatrixserverlib v0.0.0-20260202113659-20c9de33969e
github.com/matrix-org/util v0.0.0-20221111132719-399730281e66
github.com/sirupsen/logrus v1.9.3
github.com/tidwall/gjson v1.18.0
Expand All @@ -25,6 +25,7 @@ require (
github.com/Microsoft/go-winio v0.5.2 // indirect
github.com/ajstarks/svgo v0.0.0-20211024235047-1546f124cd8b // indirect
github.com/containerd/log v0.1.0 // indirect
github.com/deckarep/golang-set v1.8.0 // indirect
github.com/distribution/reference v0.6.0 // indirect
github.com/docker/go-units v0.4.0 // indirect
github.com/felixge/httpsnoop v1.0.4 // indirect
Expand Down
4 changes: 4 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/deckarep/golang-set v1.8.0 h1:sk9/l/KqpunDwP7pSjUg0keiOOLEnOBHzykLrsPppp4=
github.com/deckarep/golang-set v1.8.0/go.mod h1:5nI87KwE7wgsBU1F4GKAw2Qod7p5kyS383rP6+o6qqo=
github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5QvfrDyIgxBk=
github.com/distribution/reference v0.6.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E=
github.com/docker/docker v28.0.4+incompatible h1:JNNkBctYKurkw6FrHfKqY0nKIDf5nrbxjVBtS+cdcok=
Expand Down Expand Up @@ -73,6 +75,8 @@ github.com/matrix-org/gomatrix v0.0.0-20220926102614-ceba4d9f7530 h1:kHKxCOLcHH8
github.com/matrix-org/gomatrix v0.0.0-20220926102614-ceba4d9f7530/go.mod h1:/gBX06Kw0exX1HrwmoBibFA98yBk/jxKpGVeyQbff+s=
github.com/matrix-org/gomatrixserverlib v0.0.0-20250813150445-9f5070a65744 h1:5GvC2FD9O/PhuyY95iJQdNYHbDioEhMWdeMP9maDUL8=
github.com/matrix-org/gomatrixserverlib v0.0.0-20250813150445-9f5070a65744/go.mod h1:b6KVfDjXjA5Q7vhpOaMqIhFYvu5BuFVZixlNeTV/CLc=
github.com/matrix-org/gomatrixserverlib v0.0.0-20260202113659-20c9de33969e h1:qzLD8V3Z/qd0QQ0FMgLb1O2JqWIK9VV1dq5Co6d/tqY=
github.com/matrix-org/gomatrixserverlib v0.0.0-20260202113659-20c9de33969e/go.mod h1:b6KVfDjXjA5Q7vhpOaMqIhFYvu5BuFVZixlNeTV/CLc=
github.com/matrix-org/util v0.0.0-20221111132719-399730281e66 h1:6z4KxomXSIGWqhHcfzExgkH3Z3UkIXry4ibJS4Aqz2Y=
github.com/matrix-org/util v0.0.0-20221111132719-399730281e66/go.mod h1:iBI1foelCqA09JJgPV0FYz4qA5dUXYOxMi57FxKBdd4=
github.com/miekg/dns v1.1.66 h1:FeZXOS3VCVsKnEAd+wBkjMC3D2K+ww66Cq3VnCINuJE=
Expand Down
11 changes: 11 additions & 0 deletions tests/msc4242/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package tests

import (
"testing"

"github.com/matrix-org/complement"
)

func TestMain(m *testing.M) {
complement.TestMain(m, "msc4242")
}
Copy link
Collaborator

@MadLittleMods MadLittleMods Feb 20, 2026

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

@kegsay kegsay Mar 11, 2026

Choose a reason for hiding this comment

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

I've shoved this through Claude and it has made some obvious suggestions:

Missing: G: has >0 prev_state_events and it is an m.room.create event

STATE03: XXX receiving a state event via /send which does NOT alter the current state (say it's a concurrent topic change which does not win state res) should | should not go down /sync and be present in the timeline.

Faster remote room joins is commented out.

And some less obvious ones:

No e2e soft-failure convergence test

The spec dedicates a section to the specific failure mode where soft-failing an event prevents a later server from seeing it and converging on correct state. SJ04C covers the fix (including concurrent events in /send_join), but there's no multi-server e2e test (msc4242_e2e_test.go only has the basic TestMSC4242FederationSimple). The spec's scenario: "Bystander soft-fails C; long-dead server rejoins and never learns C's effect on state" is not exercised.

  1. No test for prev_state_events cross-room rejection

The auth rules section adds:

If there are entries which do not belong in the same room, reject.

None of the faultyEventTestCases generates a prev_state_events entry pointing to a state event from a different room. This is distinct from "bogus event ID" (CodeSuffix A/B) as those are unknown IDs, not known-but-wrong-room IDs.

When probed for more missing test cases, it produced:

earliest_events never exercised with non-empty values

Every call to /get_missing_events across all tests passes EarliestEvents: []string{}. The spec explicitly defines the algorithm as mapping earliest_events to an initial "seen" set to bound the walk. There's no test verifying the server actually stops walking when it hits a known event ID. This matters because the recursive pagination in assertGetMissingEventsRecursively works around this by updating latest_events instead, so a broken earliest_events implementation would never be caught.

Outbound prev_state_events on non-state events never checked

The spec states all events — including messages — must carry prev_state_events. The tests verify the HS handles incoming events with various prev_state_events configurations, and TestMSC4242SendJoinSJ01Inbound checks prev_state_events on the join event specifically. But there's no test inspecting events sent by hs1 (messages, profile changes, etc.) to confirm they actually include prev_state_events pointing to the correct state DAG forward extremities. The E2E test only checks event IDs arrive, not event structure.

Duplicate event IDs in prev_state_events not tested

The spec removes the check for duplicate (type, state_key) pairs because prev_state_events can legitimately reference two events with the same tuple on different branches. But there's no faulty event test case for a prev_state_events list containing the same event ID twice. Whether this should be rejected (malformed) or tolerated (treated as deduplicated) isn't tested either way.

No test that /state, /state_ids, /event_auth are not called during normal operation

The spec explicitly obsoletes these three endpoints. While srv.UnexpectedRequestsAreErrors being true (the default) would catch this in some tests, several tests set it to false, and no test specifically asserts the HS does not fall back to these endpoints when processing events or calculating room state. A targeted test with a Complement server that returns 404 for all three endpoints and verifies the HS still converges correctly would strengthen this.

I'll consider these and resolve this issue when done.

Loading
Loading