Skip to content

Adds conversation_type to MlsGroup struct in favor of reading group metadata state for conversation_type#2249

Merged
cameronvoell merged 8 commits intomainfrom
cv/07-24-use_storedgroup_in_favor_of_mls_group_state_for_conv_type
Jul 25, 2025
Merged

Adds conversation_type to MlsGroup struct in favor of reading group metadata state for conversation_type#2249
cameronvoell merged 8 commits intomainfrom
cv/07-24-use_storedgroup_in_favor_of_mls_group_state_for_conv_type

Conversation

@cameronvoell
Copy link
Contributor

@cameronvoell cameronvoell commented Jul 24, 2025

See iOS repro test PR this was tested against for performance:

xmtp/xmtp-ios#542

got this three times in a row on my repro before this libxmtp update:

syncAllConversations() ▸ 65.385 s for 1201 convos

=== TIMING STATISTICS ===
conversations.list() ▸ avg: 1.465s, max: 50.721s (43 calls)
messages() ▸ avg: 0.002s, max: 0.005s (43 calls)
findConversation() ▸ avg: 0.001s, max: 0.003s (43 calls)
updateConsent() ▸ avg: 0.000s, max: 0.001s (43 calls)
=== END ===

and then after libxmtp update conversations.list() never hit the high max again:

conversations.list() ▸ avg: 0.017s, max: 0.039s (1698 calls)
messages() ▸ avg: 0.001s, max: 0.013s (1698 calls)
findConversation() ▸ avg: 0.000s, max: 0.006s (1698 calls)
updateConsent() ▸ avg: 0.000s, max: 0.006s (1698 calls)

Store conversation type as field in MlsGroup struct instead of retrieving from group state metadata

The MlsGroup struct now stores conversation_type as a direct field rather than retrieving it asynchronously from metadata. The conversation_type() method changes from an async method that queries metadata to a synchronous method that returns the stored field. All MlsGroup creation methods across the codebase are updated to accept and pass the conversation_type parameter. The FfiConversation.conversation_type() method in bindings_ffi/src/mls.rs becomes synchronous and directly returns the stored value. A new get_conversation_type() method is added to the QueryGroup trait in xmtp_db/src/encrypted_store/group.rs to retrieve conversation types directly from the database.

📍Where to Start

Start with the MlsGroup struct definition and its conversation_type() method in xmtp_mls/src/groups/mod.rs to understand how the conversation type field is now stored and accessed.

Changes since #2249 opened

  • Added conversation_type parameter to MlsGroup constructor call [0c65c4a]
  • Changed conversation type parameter in MlsGroup initialization during message recovery [ee2d364]

Macroscope summarized ee2d364.

Copy link
Contributor Author

@cameronvoell cameronvoell marked this pull request as ready for review July 24, 2025 23:00
@cameronvoell cameronvoell requested a review from a team as a code owner July 24, 2025 23:00
@cameronvoell cameronvoell changed the title use StoredGroup in favor of mls group state for conv_type use StoredGroup in favor of mls group state for conversation_type Jul 24, 2025
@cameronvoell cameronvoell force-pushed the cv/07-24-use_storedgroup_in_favor_of_mls_group_state_for_conv_type branch from 7727f5a to 2dd66f6 Compare July 25, 2025 07:01
@cameronvoell cameronvoell changed the title use StoredGroup in favor of mls group state for conversation_type Adds conversation_type to MlsGroup struct in favor of reading group metadata state for conversation_type Jul 25, 2025
self.0.clone(),
msg.group_id.clone(),
None,
ConversationType::Group,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the one place the new conversation type needs to be added where we dont know its value. I think we're also populating dummy data for dm id here and the code path looks like conversation_type is not used, so I think this is safe for now, but seems like a code smell.

@cameronvoell cameronvoell enabled auto-merge (squash) July 25, 2025 22:54
@cameronvoell cameronvoell merged commit 5127554 into main Jul 25, 2025
17 of 18 checks passed
@cameronvoell cameronvoell deleted the cv/07-24-use_storedgroup_in_favor_of_mls_group_state_for_conv_type branch July 25, 2025 22:59
@nplasterer nplasterer mentioned this pull request Jul 25, 2025
3 tasks
nplasterer pushed a commit that referenced this pull request Jul 25, 2025
…etadata state for conversation_type (#2249)

retrieving from group state metadata
The `MlsGroup` struct now stores `conversation_type` as a direct field
rather than retrieving it asynchronously from metadata. The
`conversation_type()` method changes from an async method that queries
metadata to a synchronous method that returns the stored field. All
`MlsGroup` creation methods across the codebase are updated to accept
and pass the `conversation_type` parameter. The
`FfiConversation.conversation_type()` method in
[bindings_ffi/src/mls.rs](https://github.com/xmtp/libxmtp/pull/2249/files#diff-3a24c3e76565487a710ac9863ac05160128f4f90892e07849b555a6de43a6e8f)
becomes synchronous and directly returns the stored value. A new
`get_conversation_type()` method is added to the `QueryGroup` trait in
[xmtp_db/src/encrypted_store/group.rs](https://github.com/xmtp/libxmtp/pull/2249/files#diff-e3d4940853a1045db514d5c1a48581d9ef53da4ba45a7cb8387c72c99c475b5a)
to retrieve conversation types directly from the database.

Start with the `MlsGroup` struct definition and its
`conversation_type()` method in
[xmtp_mls/src/groups/mod.rs](https://github.com/xmtp/libxmtp/pull/2249/files#diff-c29f56a38916c7410eff8091df1a2e43487ffe20646d96827e846e475f4608d3)
to understand how the conversation type field is now stored and
accessed.

- Added conversation_type parameter to MlsGroup constructor call
[0c65c4a]
- Changed conversation type parameter in MlsGroup initialization during
message recovery [ee2d364]
----

_[Macroscope](https://app.macroscope.com) summarized ee2d364._

---------

Co-authored-by: cameronvoell <cameronvoell@users.noreply.github.com>
nplasterer added a commit that referenced this pull request Jul 25, 2025
Includes:

- [x]  #2252
- [x] Reverts #2224
- [x] #2249

---------

Co-authored-by: Mojtaba Chenani <chenani@outlook.com>
Co-authored-by: Cameron Voell <1103838+cameronvoell@users.noreply.github.com>
Co-authored-by: cameronvoell <cameronvoell@users.noreply.github.com>
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.

2 participants