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

A first draft of a document describing Matrix state and state resolution (second attempt) #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Magnap
Copy link
Collaborator

@Magnap Magnap commented Nov 21, 2017

Same content as was approved by @maxidor in #5, but that was merged before @non-Jedi had reviewed, leading to #7. So here's the second attempt.

@Magnap Magnap added the state label Nov 21, 2017
@Magnap Magnap requested a review from non-Jedi November 21, 2017 12:16
Copy link
Member

@non-Jedi non-Jedi left a comment

Choose a reason for hiding this comment

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

If the new state resolution doc in matrix-doc is fairly clear, no need to put
more work into this one. Actually, adding a link to it in here probably wouldn't
be a bad idea.

** A total order over states (as by "Erik's draft" and current Synapse)
For two events =A= and =B=,
=A < B= if =A='s depth is less than =B='s,
or, if their depths are equal,
Copy link
Member

Choose a reason for hiding this comment

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

This could be more clear. Do you mean something like:

if depth(A) < depth(B):
  A < B
elseif depth(A) == depth(B) & hash(A) < hash(B):
  A < B
else:
  A > B

The phrasing of the sentence for the second condition is just a bit awkward and
convoluted.

if the hash value of =A= is less than
the hash value of =B=.
Otherwise, =B > A=.

Copy link
Member

Choose a reason for hiding this comment

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

A practical example would be very helpful here. Especially interested in an
example with branches that have more than one event in them. To me what you
describe here sounds like those events would be interleaved, but that doesn't
seem to match what I've seen synapse do in practice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants