matrix-org / dendrite

Dendrite is a second-generation Matrix homeserver written in Go!
https://matrix-org.github.io/dendrite/
Apache License 2.0
5.67k stars 664 forks source link

The transaction_id within events is not serialised in many endpoints #3000

Open sandhose opened 1 year ago

sandhose commented 1 year ago

Similar to https://github.com/matrix-org/synapse/issues/15173 in Synapse

There are many endpoints that return events, and in those representations they should include the transaction_id in the unsigned part of the event.

I wrote a Complement test to highlight that Synapse had this issue by testing using the /rooms/{roomId}/event/{eventId} endpoint, and Dendrite also fails on this test. I haven't tested the other endpoints, nor covered them in the Complement test. https://github.com/matrix-org/complement/pull/621

abheekda1 commented 1 year ago

I'd be interested in taking a shot at this, would it be possible for someone to point me in the right direction in terms of the serialization code and behavior?

S7evinK commented 1 year ago

For a starting point: This method gets the events for e.g. /search or the mentioned /rooms/{roomId}/event/{eventId} but doesn't have a device parameter we could pass to StreamEventsToEvents which does the "magic" with transaction_id.

Almost the same is happening for /messages in handleNonEmptyEventsSlice, where we also don't pass a device to StreamEventsToEvents

/context seems to be a little bit trickier, as the database queries don't return StreamEvents.

tmills80 commented 7 months ago

This doesn't seem to have any activity since March so I'm going to have a stab at it. I've got the Complement test passing, but haven't checked any other endpoints yet.