refactor: simplify conversation abstraction layers [WPB-25785] by coriolinus · Pull Request #2173 · wireapp/core-crypto

coriolinus · 2026-05-26T16:10:41Z

What's new in this PR

Ultimately we only want to conversation types: Conversation (immutable) and ConversationMut (mutable). This stands in contrast to the current approach which uses:

struct ImmutableConversation
struct MlsConversation
struct ConversationGuard
trait Conversation
trait ConversationWithMls
trait HasSessionAndCrypto

So we clean all that up here.

PR Submission Checklist for internal contributors

The PR Title
- conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
- contains a reference JIRA issue number like SQPIT-764
- answers the question: If merged, this PR will: ... ³

https://sparkbox.com/foundry/semantic_commit_messages
https://github.com/wireapp/.github#usage
E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

SimonThormeyer

Great work, definitely the most recent big cleanup now!

We ve too many abstraction layers. Ultimately we just want two kinds of conversation: immutable, and mutable. This is a first step toward achieving that.

This sets us up to remove `MlsConversation`.

…Guard` With the mutable conversation API on `ConversationGuard` and the immutable conversation API on `ImmutableConversation` we are now ready to drop the `MlsConversation` type entirely.

…tions Therefore move the calculation from `ConversationGuard` to `ImmutableConversation`.

This is kind of the same as the old `MlsConversation::from_mls_group`, except it's factored very differently. Looking at the call sites, though, this accomplishes the same intent.

…ome_message` This is kind of the same as the old `MlsConversation::from_welcome_message`, except it's factored very differently. Looking at the call sites, though, this accomplishes the same intent.

All functionality has been moved to `ImmutableConversation` or to `ConversationGuard` as appropriate.

Previously the conversation cache kept track of `Arc<RwLock<ImmutableConversation>>`, and so did a `ConversationGuard`. This worked, but had the undesirable effect that we couldn't impl `ConversationGuard: Deref<Target=ImmutableConversation>`, because that is a synchronous trait and locking that rwlock is async. This commit restructures such that the `RwLock` is now only on the `MlsGroup` inside `ImmutableConversation`, which is the only part which requires internal mutability anyway. This enables the `Deref` impl, but has the unfortunate side effect that all group access now needs to be async on the `ImmutableConversation`. Overall, that seems to be a price worth paying.

This replaces the old `.inner().await` calls which used to be so ubiquitous.

Everything should compile now.

Now that we have many fewer types and traits in play, we can use simpler names for things: - `ImmutableConversation` -> `Conversation` - `ConversationGuard` -> `ConversationMut` We also simplify the `conversation` module tree a bit.

We'd had a confusing situation wherein sometimes a `Mls` prefix meant that a type came from OpenMLS, and other times we'd defined it ourselves with the prefix for style. Within Core Crypto, MLS is the default--especially within the `mls` module tree. We don't need to redundantly prefix it onto all of our types as well.

We no longer need to import quite what we used to.

In short: it's pointless and redundant to append the name of an enum as the suffix of each variant, so clippy warns against it. But that's what's happening here (and this is probably the exact situation which prompted someone to write the lint). So we fix that.

Nobody was using it; it was just noise.

We'd changed the structure a bit in a way which changed how the errors are reported within CC. Instead of matching on the precise call path leading to a precise error stack, we just match on the important bit: the innermost error.

I thought I had done that already but apparently it got lost in the shuffle.

SimonThormeyer reviewed May 27, 2026

View reviewed changes

Comment thread crypto/src/mls/conversation/conversation_guard/group_mutation.rs Outdated

coriolinus force-pushed the prgn/refactor/25785-conversation-layer-simplification branch from 8b3fd97 to a3960b8 Compare May 28, 2026 09:29

coriolinus marked this pull request as ready for review May 28, 2026 09:48

coriolinus requested a review from a team May 28, 2026 09:48