Skip to content
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.

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

#1042 mistakenly changed the description of state res in v2--v10 rooms #1154

Closed
DMRobertson opened this issue Jun 30, 2022 · 1 comment · Fixed by #1158
Closed

#1042 mistakenly changed the description of state res in v2--v10 rooms #1154

DMRobertson opened this issue Jun 30, 2022 · 1 comment · Fixed by #1158
Labels
release-blocker Blocks the next release from happening spec-bug Something which is in the spec, but is wrong

Comments

@DMRobertson
Copy link
Contributor

Link to problem area:

Issue

In v1.2 and prior:

Take all power events and any events in their auth chains, recursively, that appear in the full conflicted set and order them by the reverse topological power ordering.

In v1.3:

Select all power events that appear in the full conflicted set. Compute the union of their auth chains, including the power events themselves. Sort the union using the reverse topological power ordering.

(emphasis mine).

These are not equivalent. In v1.2 one only selects events in the full conflicted set. In the wording of v1.3, one can pull in auth events from outside the full conflicted set.

I still think the v1.2 text is confusing (the word recursively, in particular). I think we should either

  • revert to v1.2 wording
  • find a better way to express v1.2's wording. Perhaps:

    Select the set $X$ of all power events in the full conflicted set. For each such power event $P$, enlarge $X$ by adding the events in the auth chain of $P$ which also belong to the full conflicted set. Order $X$ using the reverse topological power ordering.


I don't know how the spec release process would handle this (I'm pretty sure I read somewhere that spec releases are meant to be immutable?). Maybe a v1.3.1?

cc @erikjohnston.

@DMRobertson DMRobertson added the spec-bug Something which is in the spec, but is wrong label Jun 30, 2022
@turt2live
Copy link
Member

Spec releases can change but it's a royal pain to do so. There's a point where we arbitrarily decide that a change has missed the patch window and can wait until the next release. For something like this, if we get a solution merged before July 6th (arbitrarily) then I'd be happy to do a patch release. Otherwise, it can hold out until v1.4 - it wouldn't be the first time we've shipped a bug in the spec :p

@turt2live turt2live added the release-blocker Blocks the next release from happening label Jun 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-blocker Blocks the next release from happening spec-bug Something which is in the spec, but is wrong
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants