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

Deterministic ordering of competing messages #506

Merged
merged 9 commits into from
Nov 29, 2021
Merged

Conversation

abdulmth
Copy link
Contributor

@abdulmth abdulmth commented Nov 23, 2021

Description of change

Ordering message now is implemented according to the specs, namely:

If two or more root documents, integration documents or diff documents compete in the chain. Eg. 2 integration message have the same previous_message_id they will be sorted according to the milestone that references them. If both messages have the same milestone, they will be alphabetically ordered according based on their MessageId.

Note: Milestone ordering only happens if two or more message compete in the chain!

Links to any relevant issues

fixes issue #480

Type of change

  • Bug fix (a non-breaking change which fixes an issue)
  • Enhancement (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Fix

@abdulmth abdulmth added Bug Something isn't working. Breaking change A change to the API that requires a major release. Part of "Changed" section in changelog labels Nov 23, 2021
@abdulmth abdulmth linked an issue Nov 23, 2021 that may be closed by this pull request
2 tasks
@abdulmth abdulmth marked this pull request as ready for review November 23, 2021 20:29
Copy link
Contributor

@cycraig cycraig left a comment

Choose a reason for hiding this comment

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

Looks fine to me after the changes.

Passing around the Client is not ideal but I don't think we can get around that without fetching all milestones up-front, which is a lot more expensive in general.

Minor comments about errors.

identity-iota/src/chain/milestone.rs Outdated Show resolved Hide resolved
identity-iota/src/chain/integration_chain.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@olivereanderson olivereanderson left a comment

Choose a reason for hiding this comment

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

Looks mostly good to me. I agree with what @cycraig said regarding MilestoneError though.

@cycraig cycraig merged commit 59d1bca into dev Nov 29, 2021
@cycraig cycraig deleted the fix/sort_by_milestone branch November 29, 2021 15:22
@abdulmth abdulmth added the Rust Related to the core Rust code. Becomes part of the Rust changelog. label Dec 15, 2021
@cycraig cycraig added the Wasm Related to Wasm bindings. Becomes part of the Wasm changelog label Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking change A change to the API that requires a major release. Part of "Changed" section in changelog Bug Something isn't working. Rust Related to the core Rust code. Becomes part of the Rust changelog. Wasm Related to Wasm bindings. Becomes part of the Wasm changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task] Use Milestones for ordering DID Messages
3 participants