Skip to content

feat: GroupV2 through the threaded client, group roster, registry retry#167

Merged
osmaczko merged 9 commits into
mainfrom
feat/groupv2
Jul 9, 2026
Merged

feat: GroupV2 through the threaded client, group roster, registry retry#167
osmaczko merged 9 commits into
mainfrom
feat/groupv2

Conversation

@osmaczko

@osmaczko osmaczko commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

feat: expose GroupV2 through the threaded client

GroupV2 (de-mls) conversations were reachable only from Core, and every
conversation ran the hardcoded millisecond timer profile in group_v2.rs
(20-150 ms freeze/consensus windows), which cannot survive real network
latency. Make them reachable through ChatClient (as DirectV1 already is), with
timing that holds up over a real network.

  • ChatClient::create_group_conversation(accounts) and
    add_group_members(convo_id, accounts): resolve each account address to
    its endorsed signer ids through the client-held directory, the same
    resolution create_direct_conversation uses, and drive
    Core::create_group_convo / group_add_member.
  • GroupV2 timing/policy is injectable: ServiceContext carries a
    de_mls::ConversationConfig (re-exported as GroupV2Config) defaulting
    to the de-mls library defaults; Core::set_group_v2_config and the
    builder's group_v2_config setter override it. The creator's phase
    durations reach joiners inside the welcome's ConversationSync, so the
    group runs the creator's phase timing.
  • GroupV2Convo::add_member validates every member's key package before
    proposing any add, skips members de-mls would silently not propose
    (self, already in the group) instead of stranding a pending invite,
    and flushes opened proposals even on a mid-batch failure, so a failed
    batch cannot invite members behind the caller's back.
  • The millisecond test profile moves into the test harnesses
    (integration_tests_core's TestHarness, crates/client/tests/group_v2.rs).
  • New client-level tests: three accounts on the in-process transport
    create a group, a non-creator adds the third member, and messages fan
    out with directory-verified senders; a batch containing a member with
    no key package fails without inviting anyone.

fix: class inbound DirectV1 joins as Private, not Group

The joiner of a DirectV1 (pairwise) conversation received it classed as
Group, because dispatch_to_inbox2 hardcoded ConversationClass::Group for
every InboxV2 join. DirectV1 welcomes (InviteType::GroupV1) and GroupV2
welcomes (InviteType::GroupV2) both arrive over InboxV2, so a plain 1:1
invite surfaced to the display layer as a group. ConversationClass is
documented as stable across protocol versions of the same conversation
shape, and DirectV1 is the pairwise shape, so its joiner must see Private.

  • InboxV2::handle_frame returns the class alongside the convo:
    InviteType::GroupV1 (the DirectV1 welcome carrier) yields Private,
    InviteType::GroupV2 yields Group.
  • dispatch_to_inbox2 propagates that class instead of hardcoding Group.
  • direct_v1_by_account_address asserts the joiner sees Private.

feat: expose a group's roster, deduped to one entry per account

The display layer needs a group's membership, but nothing exposed it:
de-mls holds the authoritative roster (MLS group state) with no public
accessor, and members added by other members stay invisible until they
send a message. Rebuilding the roster from observed messages would fork
state the crypto layer owns and be wrong exactly when a group grows.

  • GroupConvo::members() returns each member's hex-encoded MLS
    leaf-credential content, self included. GroupV2Convo delegates to
    de-mls and guarantees self-inclusion; GroupV1Convo reads its openmls
    leaves.
  • Core::group_members(convo_id) mirrors group_add_member's dispatch: a
    cached group yields its members, a direct conversation is an
    UnsupportedFunction, otherwise the group is loaded.
  • ChatClient::group_members returns Vec, resolving each
    member's account claim through the directory. A member whose account
    claim is unconfirmable is listed by device with account None rather
    than dropped: it is cryptographically in the group, only the account
    claim is unproven. The credential parsing decode_sender did is
    factored into parse_credential and shared by both, leaving
    decode_sender's stricter drop semantics for message senders unchanged.
  • Because resolve_device_ids fans an account out to every endorsed
    device, an account whose devices all join surfaced once per device;
    group_members dedups by account, keeping the first-seen device as the
    account's representative. Members with no confirmed account stay
    individual, keyed by their unique device key.
  • Unit tests cover the tolerant-vs-drop split and the per-account dedup;
    the three-member group integration test asserts the roster converges
    after create and after each add, and a solo group lists only its
    creator.

feat: retry the registry on transient 5xx with backoff and jitter

The keypackage/account registry is reliable request-by-request but sheds concurrent bursts with a 5xx, so several instances registering at once each hard-failed on init. HttpRegistry's four calls now retry network errors and 5xx/429 with exponential backoff and full jitter (the jitter decorrelates concurrent publishers so their retries don't re-collide); 4xx and success return immediately. The total retry window is bounded to a few seconds.

fix: mark InboxV2 key package last-resort so members can join multiple groups

A key package's init key is one-time-use: openmls deletes it after the first
welcome that consumes it. Each installation registers a single key package, so a
second group inviting the same member found no matching key package and rejected
the welcome with "welcome not addressed to this member", the flaky group add.

  • Mark the InboxV2 key package as last-resort (and advertise the extension in
    the leaf capabilities, which key-package validation requires) so openmls
    retains the init key, letting one key package admit an installation to any
    number of groups.
  • This reuses one init key for every join, trading per-join forward secrecy for
    membership that just works. A TODO at the publish site tracks the intended
    one-time key-package pool (the registry pops one per fetch, the client
    replenishes) with last-resort as the exhaustion fallback (Installations publish a single one-time key package, so a member can only join one conversation #169).
  • Regression tests: a member joining two groups (core harness) and two peers
    invited to several groups over the threaded client.

fix: dedup list_conversations across the store and the in-memory cache

A DirectV1 join persists its conversation to the store and also caches it in
memory, so list_conversations saw it twice. It deduped with Vec::dedup, which
only drops consecutive repeats, over cached_convos' nondeterministic HashMap
order, so the duplicate survived whenever another cached conversation fell
between the two copies. A consumer counting conversations then saw a flaky count.

  • Dedup through a set so a conversation held in both stores is listed once
    regardless of iteration order.
  • Add a DirectV1-then-GroupV2 regression test, which also covers key-package
    reuse across conversation types.

@osmaczko osmaczko force-pushed the feat/groupv2 branch 2 times, most recently from c485f5c to 8718747 Compare July 7, 2026 18:16
@osmaczko osmaczko marked this pull request as ready for review July 7, 2026 18:28
@osmaczko osmaczko requested review from Copilot and jazzz July 7, 2026 18:28

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR expands GroupV2 support beyond Core by exposing it through ChatClient, adds a public group roster API (with per-account deduping via the directory), fixes DirectV1 join classification over InboxV2, and hardens registry/keypackage behavior to be more reliable under real network conditions and concurrent startup bursts.

Changes:

  • Expose GroupV2 creation and member-add flows via ChatClient, including injectable GroupV2 timing/policy (GroupV2Config) propagated through Core/ServiceContext.
  • Add a group roster API (Core::group_members + ChatClient::group_members) with directory-backed account verification and per-account deduping, plus related unit/integration tests.
  • Improve operational robustness: retry transient registry failures with backoff+jitter, mark the single InboxV2 key package as last-resort to allow multi-group joins, and dedup list_conversations across store vs cache.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
extensions/components/src/contact_registry/http.rs Add retry-with-backoff/jitter wrapper for registry HTTP operations.
crates/client/src/lib.rs Re-export GroupMember and GroupV2Config for client API consumers.
crates/client/src/client.rs Add GroupV2 APIs (create/add), roster API + credential parsing/dedup utilities, and builder plumbing for GroupV2 config.
crates/client/src/builder.rs Add group_v2_config builder option to inject GroupV2 timing/policy into Core.
crates/client/tests/saro_and_raya.rs Update DirectV1 test to assert joiner receives ConversationClass::Private.
crates/client/tests/group_v2.rs New client-level integration tests for GroupV2 creation, adds, roster convergence, and keypackage-reuse scenarios.
core/integration_tests_core/tests/test_group_v2.rs Add regression tests for joining multiple groups and DirectV1→GroupV2 keypackage reuse.
core/integration_tests_core/src/test_client.rs Configure fast GroupV2 timers in the core harness to keep de-mls progress within test time.
core/conversations/src/service_context.rs Store per-core group_v2_config for GroupV2 conversation construction.
core/conversations/src/lib.rs Re-export de_mls::ConversationConfig as GroupV2Config.
core/conversations/src/inbox_v2.rs Return ConversationClass from invites; mark keypackage as last-resort and advertise extension capabilities.
core/conversations/src/core.rs Add set_group_v2_config, group_members, set-based list_conversations dedup, and propagate InboxV2-derived class.
core/conversations/src/conversation.rs Extend GroupConvo trait with members() roster accessor.
core/conversations/src/conversation/group_v2.rs Use injected GroupV2 config; harden member-add to validate keypackages, skip silent no-ops, and flush proposals on failure; add roster implementation.
core/conversations/src/conversation/group_v1.rs Implement members() roster accessor using OpenMLS leaf credentials.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

) -> Result<reqwest::blocking::Response, HttpRegistryError> {
let mut attempt = 0;
loop {
let outcome = build().send();
Comment on lines +265 to +274
fn signers_from_accounts(
&self,
accounts: &[AccountAddressRef],
) -> Result<Vec<LocalSignerId>, ClientError> {
let mut signers = Vec::new();
for account in accounts {
signers.extend(self.signers_from_account(account)?);
}
Ok(signers)
}
Comment on lines +293 to +299
let mut result = Ok(());
for (member_id, signer_id, kp_bytes) in invites {
if member_id == self.conversation.member_id_bytes()
|| self.conversation.members()?.contains(&member_id)
{
continue;
}
// de-mls would silently not propose (self, or already in the group) —
// recording it would strand a pending invite that later matches a
// re-join and fires a duplicate welcome.
let mut result = Ok(());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Sand] This doc string should be clearer about the problem. I had to read the code in order to understand, rather than the other way around.

Problem: Spurious welcomes can be sent if pending_invites contains members who cannot join the group.
Solution: Check that member is not already a member of the group

This is an interesting case that should be doc'd cleanly

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reworded ✔️

Comment on lines +295 to +299
if member_id == self.conversation.member_id_bytes()
|| self.conversation.members()?.contains(&member_id)
{
continue;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[?] Should this check be occuring when pending_invites is processed in after_op? Seems like its still possible for this issue to occur. If so create an issue for later so bugs can be avoided.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines +306 to +310
) {
self.pending_invites.pop();
result = Err(e.into());
break;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[!] Giving up on a failed add_member call seems reasonable. However curious if there are any error types that we'd want to attempt a retry in the future

// published and the wakeup re-armed, or they sit dormant until an
// unrelated frame drives the conversation.
let flushed = self.after_op(service_ctx).map(drop);
result.and(flushed)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[?] I dont understand why self.after_op(service_ctx)?; doesn't work in this instance.

Doc comment appears to be about removing the early return from self.conversation.add_member

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

All it buys is: on a double failure, surface the add error (the root cause) instead of the flush error.


/// Each current member's MLS leaf-credential content (hex-encoded), self
/// included.
fn members(&self) -> Result<Vec<Vec<u8>>, ChatError>;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Sand] In the future this should be concretely typed so its easy to follow. Vec<Vec> doesn't help with type safety. I'll add this as part of the account work

Comment on lines +178 to +183
/// Timing/policy for GroupV2 conversations created or joined after this
/// call. Existing conversations keep the config they were built with.
pub fn set_group_v2_config(&mut self, config: GroupV2Config) {
self.services.group_v2_config = config;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[?] I hope this will go away once vacp2p/de-mls#130 lands.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines +314 to +317
ConvoTypeOwned::Direct(convo) => Err(ChatError::UnsupportedFunction(
convo.id().into(),
"List Members".into(),
)),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Sand] This will need to be implemented.

Becareful that Member does not mean account, it means delegateSigner. A DirectConvo will have multiple Members as each participant could have multiple installations

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines +319 to +322
} else {
let convo = self.load_group_convo(convo_id)?;
convo.members()
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Future] This code path will be deprecated as PrivateV1 is no longer reachable.

// Remove duplicates
convos.dedup();
// A conversation can live in both the store and the in-memory cache (a
// DirectV1 join persists to the store and is also cached), so drop

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[!] Any caching at this point is going to cause major UX issues, as only partial data is persisted.
i.e the "Convo" is persisted but the backing DeMLS convo and account are not.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Already tracked by: #135 and #113


/// A convo built from an InboxV2 invite, paired with the display class its
/// invite type implies.
type ClassifiedConvo<S> = (Box<dyn GroupConvo<S>>, ConversationClass);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Sand] Managing tuples is less than ideal. Consider adding a "class" concept to the Convo trait.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I do agree, though it is easy to change if one more parameter comes to play.

Comment on lines +207 to +209
.extensions(vec![
ExtensionType::ApplicationId,
ExtensionType::LastResort,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Sand] Id like to avoid using this extension for as long as possible. Adding this should be a careful decision. Discussion in #169

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Let's keep for now, at least until #169 is resolved

causal: CausalHistoryStore::new(),
identity: Identity::new(name),
wakeup_service: NoopWakeups {},
group_v2_config: de_mls::ConversationConfig::default(),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Pebble] I don't think this can stay here long term. An issue is needed to resolve this issue cleanly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Tracked by: #172

@@ -20,6 +20,7 @@ pub struct ChatClientBuilder<I = Unset, T = Unset, R = Unset, S = Unset> {
transport: T,
registration: R,
storage: S,
group_v2: Option<GroupV2Config>,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Boulder Sand] Exposing this at the client layer bleeds out abstraction layers, and will cause interop issues in the future.

Downgrading to a Sand because of the Builder pattern. Impact on developers/api is minimal. But still needs to be resolve soonly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Tracked by: #172

@jazzz jazzz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This PR provides many "Hole punch" solutions, but I don't see any strict blockers here.

No breaking impacts to public API, no issues with state desync as convo do not have persistence.

My ask is to ensure that there are tickets opened for the larger issues(Pebbles+), but happy to merge when you are.

Notes::

I'm not yet onboard with exposing GroupV2Config generation via the service_ctx. This is being fixed in DeMLS. I'd eventually like to see GroupV2 provide a sensible default configuration for network operation - with tests being handled later.

KeyPackage of last resort, is something that requires more consideration, and we'll probably revert later.

a destructive Accounts Update is incoming and will completely change that whole pathway.

For now I've tested it and it works nicely!

Comment on lines +24 to +30
/// A member of a group conversation's roster.
///
/// Shares [`MessageSender`]'s field semantics: `account` is set only when the
/// member's credential claimed an account *and* the directory confirmed this
/// device belongs to it. Unlike a message sender, an unconfirmable claim does
/// not hide the member: it is cryptographically in the group, so it is listed
/// by `local_identity` (its device) with `account: None`.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[!] This is a great temporary solution.

Account update will change this substantially, but this is clean fix for the current state

Comment thread crates/client/src/client.rs Outdated

/// Add accounts' devices to an existing group conversation. Membership
/// changes are agreed and committed by the group asynchronously; the
/// joiners' welcomes go out once the add commits.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

... once the add commits.?

I'm not entirely sure what this means.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not the best wording, changed to:

    /// Add accounts' devices to an existing group conversation. The add is
    /// staged as an MLS proposal and merged by the group's next commit (driven
    /// asynchronously by the wakeup loop); each joiner's welcome is sent when
    /// that commit lands, not when this call returns.

Comment on lines +269 to +272
/// Retry budget for the registry's transient, load-induced 5xx/429 responses.
/// The service is reliable request-by-request but sheds concurrent bursts, so a
/// few backed-off retries let a request land once the burst clears. Sized to
/// stay within chat_module's ~20s init IPC budget even at the worst-case sum.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[!] If the infra is not working, lets create an issue to resolve it closer to the source.

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.

I agree, we should identify the issue asap, retry could tolerate some of the issues.

@osmaczko osmaczko Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Tracked by #173 - retry will be removed once this is completed

causal,
identity,
wakeup_service,
group_v2_config: GroupV2Config::default(),

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.

[suggestion] this field is a config, but others is constructed service, is it possible to consume the ground_v2_config and produce a similar server and injected it here?

Comment on lines +269 to +272
/// Retry budget for the registry's transient, load-induced 5xx/429 responses.
/// The service is reliable request-by-request but sheds concurrent bursts, so a
/// few backed-off retries let a request land once the burst clears. Sized to
/// stay within chat_module's ~20s init IPC budget even at the worst-case sum.

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.

I agree, we should identify the issue asap, retry could tolerate some of the issues.

/// in test time; the library defaults wait 60s before committing an add.
fn fast_group_v2_config() -> GroupV2Config {
GroupV2Config {
commit_inactivity_duration: Duration::from_millis(50),

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.

[request change] we should use const for such values in source code, so that some of the constants can be reused here.

/// adds pax, and a message from each member reaches both others with a
/// directory-verified sender account.
#[test]
fn group_v2_three_members() {

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.

Suggested change
fn group_v2_three_members() {
fn three_members() {

result.and(flushed)
}

fn members(&self) -> Result<Vec<Vec<u8>>, ChatError> {

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.

[suggestion] The function name should be clarified with what's the specific content to a member, for example members_identity.

osmaczko added 8 commits July 9, 2026 18:04
GroupV2 (de-mls) conversations were reachable only from Core, and every
conversation ran the hardcoded millisecond timer profile in group_v2.rs
(20-150 ms freeze/consensus windows), which cannot survive real network
latency. Make them reachable through ChatClient (as DirectV1 already is), with
timing that holds up over a real network.

- ChatClient::create_group_conversation(accounts) and
  add_group_members(convo_id, accounts): resolve each account address to
  its endorsed signer ids through the client-held directory, the same
  resolution create_direct_conversation uses, and drive
  Core::create_group_convo / group_add_member.
- GroupV2 timing/policy is injectable: ServiceContext carries a
  de_mls::ConversationConfig (re-exported as GroupV2Config) defaulting
  to the de-mls library defaults; Core::set_group_v2_config and the
  builder's group_v2_config setter override it. The creator's phase
  durations reach joiners inside the welcome's ConversationSync, so the
  group runs the creator's phase timing.
- GroupV2Convo::add_member validates every member's key package before
  proposing any add, skips members de-mls would silently not propose
  (self, already in the group) instead of stranding a pending invite,
  and flushes opened proposals even on a mid-batch failure, so a failed
  batch cannot invite members behind the caller's back.
- The millisecond test profile moves into the test harnesses
  (integration_tests_core's TestHarness, crates/client/tests/group_v2.rs).
- New client-level tests: three accounts on the in-process transport
  create a group, a non-creator adds the third member, and messages fan
  out with directory-verified senders; a batch containing a member with
  no key package fails without inviting anyone.
The joiner of a DirectV1 (pairwise) conversation received it classed as
Group, because dispatch_to_inbox2 hardcoded ConversationClass::Group for
every InboxV2 join. DirectV1 welcomes (InviteType::GroupV1) and GroupV2
welcomes (InviteType::GroupV2) both arrive over InboxV2, so a plain 1:1
invite surfaced to the display layer as a group. ConversationClass is
documented as stable across protocol versions of the same conversation
shape, and DirectV1 is the pairwise shape, so its joiner must see Private.

- InboxV2::handle_frame returns the class alongside the convo:
  InviteType::GroupV1 (the DirectV1 welcome carrier) yields Private,
  InviteType::GroupV2 yields Group.
- dispatch_to_inbox2 propagates that class instead of hardcoding Group.
- direct_v1_by_account_address asserts the joiner sees Private.
The display layer needs a group's membership, but nothing exposed it:
de-mls holds the authoritative roster (MLS group state) with no public
accessor, and members added by other members stay invisible until they
send a message. Rebuilding the roster from observed messages would fork
state the crypto layer owns and be wrong exactly when a group grows.

- GroupConvo::members() returns each member's hex-encoded MLS
  leaf-credential content, self included. GroupV2Convo delegates to
  de-mls and guarantees self-inclusion; GroupV1Convo reads its openmls
  leaves.
- Core::group_members(convo_id) mirrors group_add_member's dispatch: a
  cached group yields its members, a direct conversation is an
  UnsupportedFunction, otherwise the group is loaded.
- ChatClient::group_members returns Vec<GroupMember>, resolving each
  member's account claim through the directory. A member whose account
  claim is unconfirmable is listed by device with account None rather
  than dropped: it is cryptographically in the group, only the account
  claim is unproven. The credential parsing decode_sender did is
  factored into parse_credential and shared by both, leaving
  decode_sender's stricter drop semantics for message senders unchanged.
- Because resolve_device_ids fans an account out to every endorsed
  device, an account whose devices all join surfaced once per device;
  group_members dedups by account, keeping the first-seen device as the
  account's representative. Members with no confirmed account stay
  individual, keyed by their unique device key.
- Unit tests cover the tolerant-vs-drop split and the per-account dedup;
  the three-member group integration test asserts the roster converges
  after create and after each add, and a solo group lists only its
  creator.
The keypackage/account registry is reliable request-by-request but sheds concurrent bursts with a 5xx, so several instances registering at once each hard-failed on init. HttpRegistry's four calls now retry network errors and 5xx/429 with exponential backoff and full jitter (the jitter decorrelates concurrent publishers so their retries don't re-collide); 4xx and success return immediately. The total retry window is bounded to a few seconds.
…e groups

A key package's init key is one-time-use: openmls deletes it after the first
welcome that consumes it. Each installation registers a single key package, so a
second group inviting the same member found no matching key package and rejected
the welcome with "welcome not addressed to this member", the flaky group add.

Mark the InboxV2 key package as last-resort (and advertise the extension in the
leaf capabilities, which key-package validation requires) so openmls retains the
init key, letting one key package admit an installation to any number of groups.

This reuses one init key for every join, trading per-join forward secrecy for
membership that just works. A TODO at the publish site tracks the intended
one-time key-package pool (the registry pops one per fetch, the client
replenishes) with last-resort as the exhaustion fallback (#169).

Add regression tests: a member joining two groups (core harness) and two peers
invited to several groups over the threaded client.
A DirectV1 join persists its conversation to the store and also caches it in
memory, so list_conversations saw it twice. It deduped with Vec::dedup, which
only drops consecutive repeats, over cached_convos' nondeterministic HashMap
order, so the duplicate survived whenever another cached conversation fell
between the two copies. list_conversations then intermittently returned a
conversation twice, and a consumer counting conversations (e.g. checking that a
peer joined a group while a direct chat already existed) saw a flaky count.

Dedup through a set so a conversation held in both stores is listed once
regardless of iteration order.

Add a DirectV1-then-GroupV2 regression test, which also covers key-package reuse
across conversation types.
…cate invites

Both create_group_convo_v2 and group_add_member funnel through
GroupV2Convo::add_member, so a duplicate signer (an account that resolves
to the same signer twice, or a repeated account) cost a redundant
key-package fetch and a second Add proposal. The existing guard skipped
only self and already-committed members, which a within-batch duplicate
escapes because add_member opens a proposal the committed roster does not
yet reflect, stranding a pending_invite that can later fire a spurious
duplicate welcome.

Dedup the requested signers before fetching, and guard the add loop with a
membership set seeded from the roster and self, hoisting the per-iteration
members() call out of the loop.
The retry-budget comment claimed the ~20s init IPC budget held even at the
worst-case sum, but that only holds on the load-shed path where each retry
returns fast; a fully unreachable registry costs up to MAX_RETRIES times
the reqwest timeout, which no retry budget can rescue. State both.

Reword add_group_members to name the proposal, commit, and welcome flow
rather than the unexplained "once the add commits".
…97 FP)

Stable rolled to 1.97, whose clippy question_mark flags poll()'s match on
`self.shared.borrow().read(next)`. Its suggested `read(next)?` would drop the
RefCell Ref guard and dangle the returned reference, so the lint is a false
positive here. CI tracks floating stable (`rustup update stable`), so this is
pre-existing code newly flagged; suppress it to keep the branch green.
@osmaczko osmaczko merged commit e7e122b into main Jul 9, 2026
5 checks passed
@osmaczko osmaczko deleted the feat/groupv2 branch July 9, 2026 18:02
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.

4 participants