refactor: refactor mls_session #1728

fewerner · 2026-01-06T15:23:24Z

What's new in this PR

This PR simplifies the way an mls_session is created and removes the inner session.
It also removes provideTransport() and transport will now be provided on mlsInit() since every mls session needs transport.
The database will be known by the CC instance and transaction context to avoid database usage through the mls session's crypto provider were it is not needed.

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.

github-actions · 2026-01-06T15:31:36Z

Bencher Report

Branch	felix/refactor/mls-session
Testbed	ubuntu-latest

⚠️ WARNING: No Threshold found!
Without a Threshold, no Alerts will ever be generated.
Latency (nanoseconds (ns))
Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds flag.

Click to view all benchmark results

Benchmark	Latency	microseconds (µs)
Commit add f(group size)/cs1/mem/1002	📈 view plot ⚠️ NO THRESHOLD	18,083.00 µs
Commit add f(group size)/cs1/mem/2	📈 view plot ⚠️ NO THRESHOLD	741.85 µs
Commit add f(group size)/cs1/mem/202	📈 view plot ⚠️ NO THRESHOLD	4,440.50 µs
Commit add f(group size)/cs1/mem/402	📈 view plot ⚠️ NO THRESHOLD	7,758.90 µs
Commit add f(group size)/cs1/mem/602	📈 view plot ⚠️ NO THRESHOLD	11,688.00 µs
Commit add f(group size)/cs1/mem/802	📈 view plot ⚠️ NO THRESHOLD	14,651.00 µs
Commit add f(number clients)/cs1/mem/1002	📈 view plot ⚠️ NO THRESHOLD	984,670.00 µs
Commit add f(number clients)/cs1/mem/2	📈 view plot ⚠️ NO THRESHOLD	715.14 µs
Commit add f(number clients)/cs1/mem/202	📈 view plot ⚠️ NO THRESHOLD	79,310.00 µs
Commit add f(number clients)/cs1/mem/402	📈 view plot ⚠️ NO THRESHOLD	216,690.00 µs
Commit add f(number clients)/cs1/mem/602	📈 view plot ⚠️ NO THRESHOLD	426,550.00 µs
Commit add f(number clients)/cs1/mem/802	📈 view plot ⚠️ NO THRESHOLD	674,420.00 µs
Commit pending proposals f(group size)/cs1/mem/1002	📈 view plot ⚠️ NO THRESHOLD	116,210.00 µs
Commit pending proposals f(group size)/cs1/mem/2	📈 view plot ⚠️ NO THRESHOLD	23,000.00 µs
Commit pending proposals f(group size)/cs1/mem/202	📈 view plot ⚠️ NO THRESHOLD	41,550.00 µs
Commit pending proposals f(group size)/cs1/mem/402	📈 view plot ⚠️ NO THRESHOLD	57,574.00 µs
Commit pending proposals f(group size)/cs1/mem/602	📈 view plot ⚠️ NO THRESHOLD	77,360.00 µs
Commit pending proposals f(group size)/cs1/mem/802	📈 view plot ⚠️ NO THRESHOLD	93,264.00 µs
Commit pending proposals f(pending size)/cs1/mem/1	📈 view plot ⚠️ NO THRESHOLD	17,497.00 µs
Commit pending proposals f(pending size)/cs1/mem/101	📈 view plot ⚠️ NO THRESHOLD	115,660.00 µs
Commit pending proposals f(pending size)/cs1/mem/21	📈 view plot ⚠️ NO THRESHOLD	34,441.00 µs
Commit pending proposals f(pending size)/cs1/mem/41	📈 view plot ⚠️ NO THRESHOLD	55,505.00 µs
Commit pending proposals f(pending size)/cs1/mem/61	📈 view plot ⚠️ NO THRESHOLD	74,937.00 µs
Commit pending proposals f(pending size)/cs1/mem/81	📈 view plot ⚠️ NO THRESHOLD	94,938.00 µs
Commit remove f(group size)/cs1/mem/1002	📈 view plot ⚠️ NO THRESHOLD	11,144.00 µs
Commit remove f(group size)/cs1/mem/2	📈 view plot ⚠️ NO THRESHOLD	573.94 µs
Commit remove f(group size)/cs1/mem/202	📈 view plot ⚠️ NO THRESHOLD	2,573.20 µs
Commit remove f(group size)/cs1/mem/402	📈 view plot ⚠️ NO THRESHOLD	4,109.70 µs
Commit remove f(group size)/cs1/mem/602	📈 view plot ⚠️ NO THRESHOLD	6,093.70 µs
Commit remove f(group size)/cs1/mem/802	📈 view plot ⚠️ NO THRESHOLD	8,153.10 µs
Commit remove f(number clients)/cs1/mem/1002	📈 view plot ⚠️ NO THRESHOLD	13,444.00 µs
Commit remove f(number clients)/cs1/mem/2	📈 view plot ⚠️ NO THRESHOLD	135,440.00 µs
Commit remove f(number clients)/cs1/mem/202	📈 view plot ⚠️ NO THRESHOLD	110,720.00 µs
Commit remove f(number clients)/cs1/mem/402	📈 view plot ⚠️ NO THRESHOLD	87,055.00 µs
Commit remove f(number clients)/cs1/mem/602	📈 view plot ⚠️ NO THRESHOLD	62,619.00 µs
Commit remove f(number clients)/cs1/mem/802	📈 view plot ⚠️ NO THRESHOLD	37,891.00 µs
Commit update f(group size)/cs1/mem/1002	📈 view plot ⚠️ NO THRESHOLD	135,000.00 µs
Commit update f(group size)/cs1/mem/2	📈 view plot ⚠️ NO THRESHOLD	744.99 µs
Commit update f(group size)/cs1/mem/202	📈 view plot ⚠️ NO THRESHOLD	27,973.00 µs
Commit update f(group size)/cs1/mem/402	📈 view plot ⚠️ NO THRESHOLD	55,078.00 µs
Commit update f(group size)/cs1/mem/602	📈 view plot ⚠️ NO THRESHOLD	82,980.00 µs
Commit update f(group size)/cs1/mem/802	📈 view plot ⚠️ NO THRESHOLD	108,920.00 µs

🐰 View full continuous benchmarking report in Bencher

SimonThormeyer · 2026-01-07T08:51:06Z

Could you please mark the commits with breaking changes as such? Also, there is f96d144, looks like it is supposed to be autosquashed?

typfel · 2026-01-07T10:15:21Z

Regarding in c761fea:

I think there is a design flaw in the transaction context that we should
address later.
The transaction context never knows if it is a mls or proteus
transaction. There should probably be separate transaction contexts so
that a transaction context just uses its session instead of checking for
it's existence. It also doesn't need to know anything about the other
session.

This is also currently a feature since it allows clients to decrypt proteus and mls messages within a single transaction.

typfel

Nice work!

Added some comments.

I think there's some clean up necessary on the commit messages. We are following the https://www.conventionalcommits.org/en/v1.0.0/ so you can't stack things like this:

test: ts: set transport on mlsInit()

it should instead be written as:

test(ts): set transport on mlsInit()

crypto/src/ephemeral.rs

crypto/src/transaction_context/mod.rs

crypto/src/transaction_context/e2e_identity/mod.rs

crypto/src/test_utils/mod.rs

crypto/src/proteus.rs

crypto-ffi/bindings/js/src/CoreCryptoContext.ts

crypto-ffi/bindings/js/test/wdio/database.test.ts

fewerner · 2026-01-07T13:43:12Z

Could you please mark the commits with breaking changes as such? Also, there is f96d144, looks like it is supposed to be autosquashed?

Sure. The commit changes one of your commits, so I decided to it as a fixup, I will autosquash it before merge.

The idea here is to create an "empty" CoreCrypto client first that holds no session and no pki environment. We create the mls Session on mls init which allows us to get rid of the inner session and provide transport as part of mls_init. The keystore should live in CoreCrypto since it is shared between proteus and mls. Proteus should not need to know anything about the mls session.

This removes the anti pattern where the session had to be instantiated in order to call `.init()`. Instead, we prepare the constructor arguments beforehand. All we need is the keystore database which we have here which will be the session's crypto_provider's keystore after instantiation.

Getting the client id is no longer async and is a simple `clone()`.

We provide a dummy implementation to the transport argument here. Since the history client is a very special case this should be ok. The general use case of a session always needs transport, so we rather not make it optional.

This should not be needed anymore as we use CoreCrypto::new() and then provide the database on session initialization

We already have a client id and empty identities, so we just create the session directly.

We change the order here to have a mls session before setting up the pki to make the existing tests that call this still pass. Otherwise every call to this method would result in an MlsNotInitialized Error.

proteus should not depend on the mls crypto provider's keystore which is part of a mls session. Instead just use the CC's keystore.

proteus transactions should not rely on the the mls crypto provider to access the keystore. mls members are only available when the mls session exists, so lets access them through the mls session. I think there is a design flaw in the transaction context that we should address later. The transaction context never knows if it is a mls or proteus transaction. There should probably be separate transaction contexts so that a transaction context just uses its session instead of checking for it's existence. It also doesn't need to know anything about the other session. However, for now we keep it like that. Also setting transport on an already initialized session is no longer possible. It only existed for tests which we will have to adjust.

A session always has transport

We no longer have an inner session. A session is always initialized and can't be reset or replaced without droping the session and creating a new one.

Identities used to be locked through the inner session. We still need some kind of lock here to avoid larger refactoring or locking the whole mls session.

Not needed anymore!

We don't need to test behavior of uninitialized MLS sessions, because e2ei can now only be initialized when mls is (this is to change in the future, but not yet).

We can't reinit the inner anymore so the SessionContext has to reinit the entire session.

…n()`

Since the transaction context knows the database, we should get it directly.

Note that this test won't actually work and we will have to ignore it until the pki environment api has been refactored. However, we adjust it to the refactoring already.

These tests initialize the pki without a session which is fine and must be working. However, with the current session refactoring does not support this so we disable the tests for now. The tests have to be enabled and working again with WPB-19578.

…Init()`

These logs should not have been merged previously.

This test assumes that the pki environment can be used before initializing the mls session. This will be the case again with WPB-19578 which should revert this commit.

previously this used the session to access the keystore. Since we didn't initialize it this doesn't work anymore. We can use the transaction which knows the keystore. This is a design flaw overall. We have to discuss the best practice here. It would make sense to move getters to the CC instance which is how it is in the FFI layer.

fewerner force-pushed the felix/refactor/mls-session branch 5 times, most recently from a96ec22 to 805938d Compare January 7, 2026 07:44

fewerner marked this pull request as ready for review January 7, 2026 08:01

fewerner requested a review from a team January 7, 2026 08:01

typfel requested changes Jan 7, 2026

View reviewed changes

fewerner force-pushed the felix/refactor/mls-session branch 2 times, most recently from 51810f1 to 7ad1cb3 Compare January 7, 2026 14:49

typfel approved these changes Jan 8, 2026

View reviewed changes

fewerner added 15 commits January 8, 2026 15:24

refactor: getting client id from session

84bbbb0

Getting the client id is no longer async and is a simple `clone()`.

refactor: adjust generate_history_secret

cf45b88

We provide a dummy implementation to the transport argument here. Since the history client is a very special case this should be ok. The general use case of a session always needs transport, so we rather not make it optional.

refactor: remove in_memory_cc

9d6cdc9

This should not be needed anymore as we use CoreCrypto::new() and then provide the database on session initialization

refactor: adjust history_client

30d24f2

We already have a client id and empty identities, so we just create the session directly.

test: refactor SessionContext.new_with_identifier()

6422456

We change the order here to have a mls session before setting up the pki to make the existing tests that call this still pass. Otherwise every call to this method would result in an MlsNotInitialized Error.

refactor: use cc's keystore directly

319af58

proteus should not depend on the mls crypto provider's keystore which is part of a mls session. Instead just use the CC's keystore.

refactor: rename mls_client to mls_session

fcd9a41

refactor: adjust transport getter

522fcd6

A session always has transport

refactor: remove methods for inner session

fbdc366

We no longer have an inner session. A session is always initialized and can't be reset or replaced without droping the session and creating a new one.

refactor: remove deref from CoreCrypto to mls session

170176d

refactor: lock identities

e477bff

Identities used to be locked through the inner session. We still need some kind of lock here to avoid larger refactoring or locking the whole mls session.

refactor: adjust e2ei_mls_init_only

2df6b64

fewerner and others added 27 commits January 8, 2026 15:24

test: transport is provided with session initialization

53ce1b2

style: access level and format

8afd3bb

test: remove new_uninitialized() helper for SessionContext

dda2548

Not needed anymore!

test: e2ei_enrollment() helper doesn't need a SessionContext

bf782fb

test: update tests to new api of helper function

767c423

test: remove now unnecessary test

1269e96

We don't need to test behavior of uninitialized MLS sessions, because e2ei can now only be initialized when mls is (this is to change in the future, but not yet).

test: implement SessionContext.reinit_session()

190a757

test: move random generate to SessionContext

c1f30af

We can't reinit the inner anymore so the SessionContext has to reinit the entire session.

test: change Session.reset() usage to `SessionContext.reinit_sessio…

487eb6f

…n()`

test: let sessionContext know it's cc instance

3478084

test: remove obsolete test

7abd401

refactor: access database through keystore getter

e52185f

Since the transaction context knows the database, we should get it directly.

test: refactor test

ed36c02

Note that this test won't actually work and we will have to ignore it until the pki environment api has been refactored. However, we adjust it to the refactoring already.

test: disable tests

6d6601e

These tests initialize the pki without a session which is fine and must be working. However, with the current session refactoring does not support this so we disable the tests for now. The tests have to be enabled and working again with WPB-19578.

feat!: ffi: ts: remove .provide_transport(), add transport to `.mls…

f49edc8

…Init()`

test: remove unwanted logs

f2fe442

These logs should not have been merged previously.

test: ts: remove provide transport from test utils

a559df5

test: ts: set transport before ccInit

cf2f6f8

feat!: ffi: kotlin: remove .provideTransport()

bc0e6a1

test: kotlin: set transport on mlsInit()

c83aa24

test: temporarily disable test

de61b2f

This test assumes that the pki environment can be used before initializing the mls session. This will be the case again with WPB-19578 which should revert this commit.

feat!: ffi: swift: remove .provide_transport()

6fa2097

test: swift: set transport on mlsInit()

5821220

test: interop: set transport on 'mlsInit()'.

7070dec

test: ts: set transport on mlsInit()

23c906d

docs: add ffi change to change log

d78584b

fewerner force-pushed the felix/refactor/mls-session branch from 7ad1cb3 to d78584b Compare January 8, 2026 14:24

fewerner merged commit d78584b into main Jan 8, 2026
1 check passed

fewerner deleted the felix/refactor/mls-session branch January 8, 2026 14:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: refactor mls_session #1728

refactor: refactor mls_session #1728

Uh oh!

fewerner commented Jan 6, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Jan 6, 2026 •

edited

Loading

⚠️ WARNING: No Threshold found!

Uh oh!

SimonThormeyer commented Jan 7, 2026

Uh oh!

typfel commented Jan 7, 2026

Uh oh!

typfel left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fewerner commented Jan 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

refactor: refactor mls_session #1728

refactor: refactor mls_session #1728

Uh oh!

Conversation

fewerner commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's new in this PR

PR Submission Checklist for internal contributors

Uh oh!

github-actions bot commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bencher Report

⚠️ WARNING: No Threshold found!

Uh oh!

SimonThormeyer commented Jan 7, 2026

Uh oh!

typfel commented Jan 7, 2026

Uh oh!

typfel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fewerner commented Jan 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fewerner commented Jan 6, 2026 •

edited

Loading

github-actions bot commented Jan 6, 2026 •

edited

Loading