Skip to content

refactor!: DecryptedMessage is an enum - #2376

Merged
SimonThormeyer merged 11 commits into
mainfrom
simon/refactor/decrypted-message-enums
Jul 29, 2026
Merged

refactor!: DecryptedMessage is an enum#2376
SimonThormeyer merged 11 commits into
mainfrom
simon/refactor/decrypted-message-enums

Conversation

@SimonThormeyer

Copy link
Copy Markdown
Member

This changes the DecryptedMessage struct to an enum in both ffi crate and crypto crate.

@SimonThormeyer
SimonThormeyer force-pushed the simon/refactor/decrypted-message-enums branch 5 times, most recently from 752c5b9 to aece599 Compare July 27, 2026 15:37
@SimonThormeyer
SimonThormeyer marked this pull request as ready for review July 28, 2026 06:52
@SimonThormeyer
SimonThormeyer requested a review from a team July 28, 2026 06:53

@fewerner fewerner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice improvement, I like it!

Comment thread interop/src/clients/corecrypto/web/mls.ts Outdated
@SimonThormeyer
SimonThormeyer force-pushed the simon/refactor/decrypted-message-enums branch 2 times, most recently from dbb907d to 973a169 Compare July 28, 2026 11:56

@coriolinus coriolinus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's quite good!

Comment thread crypto/src/mls/conversation/mutable/decrypt/mod.rs Outdated
Comment thread crypto/src/mls/conversation/mutable/decrypt/mod.rs Outdated
let decrypted = match message.into_content() {
ProcessedMessageContent::ApplicationMessage(app_msg) => {
let conversation = self;
let sender_client_id = sender_client_id_result?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Only propagate sender_client_id_result's error in the event it was an application message, right? Any reason not to move its parsing here?

@SimonThormeyer SimonThormeyer Jul 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not sure if parsing the message above is strictly necessary, however, message.into_content() (3 lines above) moves message, and producing sender_client_id_result requires borrowing it (via message.credential()), so we have to do it before if we're not working around that in another way.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added a comment in the place we produce sender_client_id_result.

@SimonThormeyer
SimonThormeyer force-pushed the simon/refactor/decrypted-message-enums branch 4 times, most recently from eee3d67 to 42025d3 Compare July 29, 2026 12:01
…mit processing

We're processing a commit, so a commit delay is superfluous. This
returns `None` whenever it is called here, because there are no pending
proposals.
This reduces boilerplate when dealing with enums, like the upcoming
`DecryptedMessage`.
The relevant data returned from `decrypt_message()` has three different
variants, depending on whether the message is
- Text
- a Commit
- a Proposal
@SimonThormeyer
SimonThormeyer force-pushed the simon/refactor/decrypted-message-enums branch from 42025d3 to 73a86e1 Compare July 29, 2026 12:09
@SimonThormeyer
SimonThormeyer force-pushed the simon/refactor/decrypted-message-enums branch from 73a86e1 to dce7c71 Compare July 29, 2026 12:15
@SimonThormeyer
SimonThormeyer force-pushed the simon/refactor/decrypted-message-enums branch from dce7c71 to 2499b55 Compare July 29, 2026 12:19
@SimonThormeyer
SimonThormeyer merged commit 2499b55 into main Jul 29, 2026
67 checks passed
@SimonThormeyer
SimonThormeyer deleted the simon/refactor/decrypted-message-enums branch July 29, 2026 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants