awslabs / mls-rs

An implementation of Messaging Layer Security (RFC 9420)
Apache License 2.0
105 stars 19 forks source link

Remove StateUpdate in favor of CommitEffect #175

Closed tomleavy closed 4 months ago

tomleavy commented 4 months ago

Issues:

Resolves #30

Description of changes:

As mentioned in #30 there are a bunch of downsides to the way that StateUpdate works currently. Upon further evaluation, the API of StateUpdate is not desirable to keep by simply merging it with ProvisionalState. It makes more sense to expose the raw ProposalInfo objects that were committed to (or not committed to) so that developers can use that metadata downstream. For example, if a commit has proposals from multiple authors, it is very helpful to know specifically where each proposal that was committed to originated.

We also had a pattern of having a StateUpdate with "active" set to true or false that was somewhat overloaded and did not make sense from an idiomatic Rust perspective. The boolean is now replaced by a CommitEffect enum at the top level to let the caller know that a commit resulted in the local client being removed from the group, the group being reinitialized, or a new epoch.

Call-outs:

This is a breaking change so versions were bumped up accordingly. I tried to adapt the uniffi wrappers the best that I could, but they are already lacking in details enough that it proved to be difficult. If we are really going to support uniffi long term in this main repo we should address missing APIs.

Testing:

All existing tests were adjusted, except for one that was removed due to it not really being necessary after the changes.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT license.

codecov-commenter commented 4 months ago

Codecov Report

Attention: Patch coverage is 76.10619% with 27 lines in your changes missing coverage. Please review.

Project coverage is 89.96%. Comparing base (834f4b4) to head (34094fc).

Files Patch % Lines
mls-rs/src/group/message_processor.rs 59.25% 11 Missing :warning:
mls-rs/src/group/mod.rs 85.71% 7 Missing :warning:
mls-rs/src/group/state.rs 57.14% 6 Missing :warning:
mls-rs-uniffi/src/lib.rs 88.88% 2 Missing :warning:
mls-rs/src/client.rs 50.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #175 +/- ## ========================================== + Coverage 89.62% 89.96% +0.33% ========================================== Files 175 175 Lines 31599 31084 -515 ========================================== - Hits 28320 27964 -356 + Misses 3279 3120 -159 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.