Add group description by jazzz · Pull Request #161 · logos-messaging/libchat

jazzz · 2026-07-02T17:23:57Z

Problem

Groups lack a human readable identifier. With many groups the hex group_id blur into eachother making it difficult to know which group you are talking in. To avoid this, developers should be able to set a Name and description so that other participants have the require context to communicate. #152

Solution

This PR adds an Custom extension to the GroupContext. Which embeds a Payload into the encryption state. This approach ensures that every welcome contains the data immediately and is cryptographically bound to the group state.

Notes

This PR focuses solely on adding the required feature to the backend core, and uses a default value at the Core API. All callers and upstream APIs will be updated in a followup PR.
This solution is not perfect, and will likely need to be revisited after some feedback has been gathered
The custom Extension is not compatible with other MLS implementations. This is not a critical issue for this protocol, however worth noting for future discussion
Waiting on an upstream update, currently imports the feature branch to allow reviews. Will be removed before merging.

kaichaosun · 2026-07-08T04:03:56Z

 base64 = "0.22"
 chat-proto = { git = "https://github.com/logos-messaging/chat_proto", rev = "37ec98a151f6d50aab2905802ac0a896477e62ea" }
-de-mls = { git = "https://github.com/vacp2p/de-mls", branch = "main"}
+de-mls = { git = "https://github.com/vacp2p/de-mls", branch = "jazzz/expose_extensions"}


[suggestion] We should pin the unpublished/untagged dependency to a commit, it makes the update explicit here. Pin it to a specific branch only updates in cargo.lock.

Thats a fair point. Since it only updates on cargo update .... why not make it explicit when it requires an explicit action.

It makes the update slightly more verbose, but in general I agree

kaichaosun · 2026-07-08T04:24:16Z

+}
+
+impl ConvoMetadata {
+    pub fn empty() -> Self {


[suggestion] if ConvoMetadata can be omitted, it should be optional in usage, make the data empty looks not useful.

ConvoMetadata is not optional, however not supported by all ConvoTypes at this time.

Base on suggestion, I've:

Internally the trait requires Option<ConvoMetadata>

Once all valid types support Metadata the option can be removed; simplifying code

Externally Metadata returns a Result<ConvoMetadata, _> which errors when the convo_id is not found

In the interim it also fails when a type is not supported.

ConvoMetadata::Empty has been removed.

Does this work for you?

kaichaosun · 2026-07-08T04:27:12Z

        participants: &[IdentIdRef],
    ) -> Result<ConversationId, ChatError> {
-        self.create_group_convo_v2(participants)
+        self.create_group_convo_v2(participants, "UnamedGroup", "")


[request change] the hardcoded value doesn't make sense, if the api is not ready, the param should be optional.

Updated PR description to be more clear.

This PR focuses solely on adding the required feature to the backend core, and uses a default value at the Core API. All callers and upstream APIs will be updated in a followup PR.

The proposed API is a non optional value passed in on construction.

Without using a temporary default value this PR gets large and noisy quickly as it forces all the tests, clients, modules to be updated as well.

The intention is to discuss refine, and merge the backend change. Once refined and merged, the callsites can be updated in a follow up.

Does this work?

kaichaosun · 2026-07-08T04:28:39Z

+
+use crate::types::ConvoMetadata;
+
+pub const GROUP_METADATA_EXTENSION_TYPE: u16 = 0xFF01;


[request change] Please document the magic number and necessary background why such numbered extension type is needed.

updated to :

/// MLS extension type carrying our [`ConvoMetadata`]. In the private-use /// range (0xF000–0xFFFF) reserved by RFC 9420 for non-registered extensions. pub const GROUP_METADATA_EXTENSION_TYPE: u16 = 0xFF01;

kaichaosun · 2026-07-08T04:30:08Z

+impl ConvoMetaInfo {
+    pub fn new(name: impl Into<String>, owner: IdentIdRef, desc: impl Into<String>) -> Self {
+        Self {
+            version: 1,


[suggestion] use enum for version is more extensible.

Its an exclusively internal representation, but sure. Added in 0429ed7

kaichaosun · 2026-07-08T04:31:35Z

+    pub fn to_extension_bytes(&self) -> Vec<u8> {
+        // TLS presentation-language encoding — matches the wire format used by
+        // the rest of the MLS stack, so no extra serializer is pulled in.
+        self.tls_serialize_detached().expect("serialization failed")


[request change] avoid panic inside conversation.

Good eye.

In this case its actually an *reasonably infallible function, but I should have Doc'd it better.

Ive updated the expect string as well as the comment to be clearer about the situation. What I don't want is to poison the entire call chain with Results for a function which cannot fail, and cannot be recovered from.

9738a37

Does that sound reasonable?

kaichaosun · 2026-07-08T04:33:21Z

+        None, // default ciphersuites
+        Some(&[ExtensionType::Unknown(GROUP_METADATA_EXTENSION_TYPE)]),
+        None, // default proposal types
+        None, // default credential types


[suggestion] not sure why exactly it looks like this way, but the many Nones make me feel that it's not in a proper abstraction.

This is a by product of OpenMLS using Options for its configuration. None is used to denote using the default value.

My hope is that this Library can avoid such artifacts in our APIs

kaichaosun · 2026-07-08T04:43:26Z

+pub struct ConvoMetaInfo {
+    version: u16,
+    name: String,
+    owner: IdentId,


[suggestion] It's better to move the owner before name to keep it consistent in the codebase, currently some places name comes after owner, and here owner comes after name.

The new method's param order applied too.

Owner removed

osmaczko · 2026-07-09T18:13:56Z

+pub struct ConvoMetaInfo {
+    version: ConvoMetaInfoVersion,
+    name: String,
+    owner: IdentId,


[?] In theory, the owner should be all associated IdentIds (devices) of a given account. Given that, would it make more sense to have an account identifier here? Account is not really a conversation-level concern, though. Any ideas on how to solve this properly?

osmaczko · 2026-07-09T18:16:11Z

 base64 = "0.22"
 chat-proto = { git = "https://github.com/logos-messaging/chat_proto", rev = "37ec98a151f6d50aab2905802ac0a896477e62ea" }
-de-mls = { git = "https://github.com/vacp2p/de-mls", branch = "main"}
+de-mls = { git = "https://github.com/vacp2p/de-mls", rev = "2c7a8669c1492c749c02efd2c5ac45e93e4926a3"} # Expose Mls Extensions (#131)


Should we wait for it to be merged first?

jazzz added 7 commits July 7, 2026 14:43

Add group description

4287f2e

Update config

8997957

Update constructor calls

ccaa080

Enable groupContext for GV2

90c9756

Remove test group type

05a64f1

Add test

6d76ab2

temporary dep; waiting for upstream

1477f6a

jazzz force-pushed the jazzz/group_desc branch from c7de91a to 1477f6a Compare July 7, 2026 23:19

jazzz requested review from kaichaosun and osmaczko July 8, 2026 00:08

jazzz marked this pull request as ready for review July 8, 2026 00:12

kaichaosun reviewed Jul 8, 2026

View reviewed changes

jazzz added 4 commits July 8, 2026 09:03

Remove owner

3008341

Doc MLS extension type

1936f32

Add ConvoMetaInfoVersion

0429ed7

Update infallable function docs

9738a37

jazzz force-pushed the jazzz/group_desc branch from e817fa3 to 9738a37 Compare July 8, 2026 16:45

jazzz added 2 commits July 8, 2026 13:25

strongly typed metadata

750b260

Cleanup tests

31cff06

kaichaosun approved these changes Jul 9, 2026

View reviewed changes

Pin de-mls revision

4270b7f

osmaczko approved these changes Jul 9, 2026

View reviewed changes


		use crate::types::ConvoMetadata;

		pub const GROUP_METADATA_EXTENSION_TYPE: u16 = 0xFF01;

Uh oh!

Conversation

jazzz commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaichaosun Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jazzz Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jazzz Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jazzz commented Jul 2, 2026 •

edited

Loading

kaichaosun Jul 8, 2026 •

edited

Loading

jazzz Jul 8, 2026 •

edited

Loading

jazzz Jul 8, 2026 •

edited

Loading