use GroupId Everywhere

precursor to #2227 # GroupId Type Migration Design Date: 2026-04-23 Status: Draft ## Goal Adopt `xmtp_proto::types::ids::GroupId` as the canonical group identifier throughout the libxmtp workspace, replacing raw `Vec<u8>` / `&[u8]` usage at application, repo, and subscription layers. Keep the type ergonomic to interoperate with `openmls::group::GroupId`. This migration is phase 1 of a two-phase effort. Phase 2 (separate future PR) swaps the internal representation to `[u8; 16]` and makes `GroupId` `Copy`. Phase 1 design decisions must not block that swap. ## Non-Goals - Phase 2 internal representation change to `[u8; 16]`. - Diesel `ToSql`/`FromSql` impls for `GroupId` (deferred to a later PR). - Tightening conversion surface (e.g. infallible → `TryFrom`). Deferred until phase 2. - Changing the 16-byte group-id generation logic in MLS group creation. - Schema or migration changes in `xmtp_db`. ## Current State `GroupId` already exists at `crates/xmtp_proto/src/types/ids/group_id.rs`. It wraps `bytes::Bytes` and exposes: - `From<Vec<u8>>`, `From<&[u8]>` (infallible). - `FromStr` (hex decode). - `AsRef<[u8]>`, `Borrow<[u8]>`, `Deref<Target = Bytes>`. - `Display` (hex), `Debug` (truncated hex). - `xmtp_common::Generate` under `if_test!`. `xmtp_proto` already depends on `openmls` via workspace. No new dep needed. Usage survey (approximate, from ripgrep): - 130 sites already type `group_id` as `GroupId` / `&GroupId`. - 236 sites still type it as `Vec<u8>` / `&[u8]` / `Bytes`. - 21 sites call `openmls_group.group_id()` directly and feed the resulting `openmls::group::GroupId` into raw-bytes APIs. - openmls-imported `GroupId` appears in 5 files today: `sql_key_store.rs`, `test/mock/openmls_mock.rs`, `groups/oneshot.rs`, `export_stream/group_save.rs`, `groups/mls_ext/commit_log_storer.rs`. Group ids are generated as `xmtp_common::rand_vec::<16>()` today — the 16-byte constraint is established by convention but not enforced at the type level. ## Design Decisions ### 1. Scope — full replacement at application layer `xmtp_proto::GroupId` is used throughout `xmtp_mls`, `xmtp_db` (repo layer), `xmtp_api`, `xmtp_api_d14n`, and `xmtp_archive`. `openmls::group::GroupId` is confined to code that directly calls openmls APIs (MLS sync, MLS extensions, openmls key store, MLS-aware archive code, openmls mocks). Conversion happens at that boundary. Why: xmtp_proto stays MLS-free conceptually even though it holds the openmls dep. Application code speaks one canonical type. The 236 raw-bytes sites collapse to the single `GroupId` type. ### 2. Internal representation — keep `bytes::Bytes` for now `GroupId` continues to wrap `bytes::Bytes` in this phase. Phase 2 swaps the inner representation to `[u8; 16]` and adds `Copy`. Why: deferring the internal change keeps phase 1 mechanical (type swaps, no bytes-handling logic changes). The phase 2 swap should be non-breaking at API level if phase 1 avoids leaking the `Bytes` internal representation to new consumers. **Design constraint driven by phase 2:** all new APIs added in phase 1 must not rely on `Deref<Target = Bytes>`. The existing `Deref<Bytes>` impl stays for back-compat with currently-compiling code, but new call sites use `.as_slice()` / `AsRef<[u8]>` only. Where an existing site relies on `Deref<Bytes>` specifically (not `[u8]`), rewrite it to `AsRef<[u8]>` during migration so phase 2 can remove `Deref`. ### 3. Name collision with `openmls::group::GroupId` — rename openmls on import In files touching both types, import as: ```rust use openmls::group::GroupId as OpenMlsGroupId; ``` `GroupId` unqualified always means `xmtp_proto::types::GroupId`. The handful of files (≤10 after migration) importing the openmls type follow this convention uniformly. ### 4. Conversion ergonomics — `From` impls + inherent method #### Orphan rule analysis Rust's orphan rule (`impl T for X` in crate `C` requires `T` local to `C` OR `X` local to `C`, with generic-parameter constraints): - **`impl From<&openmls::group::GroupId> for GroupId`** in xmtp_proto — target `GroupId` is local. **Allowed.** - **`impl From<openmls::group::GroupId> for GroupId`** in xmtp_proto — target `GroupId` is local. **Allowed.** - **`impl From<&GroupId> for openmls::group::GroupId`** in xmtp_proto — `From` is foreign (core), target `openmls::group::GroupId` is foreign. The *generic parameter* being local is not sufficient — the orphan rule keys off the `Self` (target) type. **Blocked.** So `From`/`Into` works **inbound** (openmls → xmtp) but not **outbound** (xmtp → openmls). The outbound direction must be an inherent method on `GroupId`. #### Impls ```rust impl From<&openmls::group::GroupId> for GroupId { fn from(id: &openmls::group::GroupId) -> Self { GroupId::from(id.as_slice()) } } impl From<openmls::group::GroupId> for GroupId { fn from(id: openmls::group::GroupId) -> Self { GroupId::from(id.as_slice()) } } impl GroupId { pub fn to_openmls(&self) -> openmls::group::GroupId { openmls::group::GroupId::from_slice(self.as_ref()) } } ``` #### Call-site ergonomics ```rust let xmtp_id: GroupId = mls_group.group_id().into(); // From (inbound) let ommls_id: openmls::group::GroupId = xmtp_id.to_openmls(); // inherent (outbound) ``` Raw-bytes inbound already works via existing `From<&[u8]>` / `From<Vec<u8>>`. No extension trait needed. ### 4a. Random generation — `GroupId::random` via `OpenMlsRand` Mirror `openmls::group::GroupId::random` on the xmtp_proto `GroupId`. Reuses the MLS provider's RNG so group-id bytes come from the same CSPRNG openmls uses internally, and matches the 16-byte length. ```rust use openmls_traits::random::OpenMlsRand; impl GroupId { pub fn random<R: OpenMlsRand>(rand: &R) -> Self { let bytes: [u8; 16] = rand .random_array() .expect("OpenMlsRand failed to produce randomness"); GroupId::from(bytes.as_slice()) } } ``` (`rand.random_array::<16>()` is the openmls API shape; implementation follows `openmls/src/group/mod.rs` verbatim.) Group creation then reads: ```rust let group_id = GroupId::random(provider.rand()); let mls_group = MlsGroup::new_with_group_id( provider, &identity.installation_keys, group_config, group_id.to_openmls(), credential_with_key, )?; ``` Replaces `xmtp_common::rand_vec::<16>()` at MLS-group-creation sites. Other `rand_vec::<16>()` usages (non-group-id) stay as-is. Phase 2 alignment: when the internal representation becomes `[u8; 16]`, this `random` impl collapses to `GroupId(rand.random_array().expect(...))` without any call-site change. `xmtp_proto` already depends on `openmls_traits` transitively via `openmls`; if it does not re-export `OpenMlsRand`, add an explicit `openmls_traits` workspace dep to xmtp_proto. Verify during implementation. ### 5. Conversion surface with raw bytes — unchanged Keep existing infallible `From<Vec<u8>>`, `From<&[u8]>`, `FromStr`, `AsRef<[u8]>`, `Borrow<[u8]>`, `Deref<Bytes>`. Phase 2 will replace infallible conversions with checked ones when the `[u8; 16]` invariant becomes enforced at the type level. Migration sites in phase 1 use `From`/`Into` freely, e.g. `let id: GroupId = vec_bytes.into();`. ### 6. DB schema strategy — repo-layer conversion, Diesel models unchanged Diesel model structs continue to hold `Vec<u8>` group_id fields in this phase. Repo functions accept and return `GroupId`, converting to/from `Vec<u8>` at the Diesel-call boundary inside the function body. Example: ```rust pub fn find_group(&self, id: &GroupId) -> Result<Option<GroupRecord>, _> { groups::table .filter(groups::id.eq(id.as_ref())) .first::<RawGroupRecord>(conn) .optional() .map(|opt| opt.map(GroupRecord::from)) } ``` A later PR introduces Diesel `ToSql`/`FromSql` impls for `GroupId` and swaps the struct fields. ## Architecture ### Module layout ``` xmtp_proto::types::ids ├── group_id.rs (GroupId + OpenMlsGroupIdExt + to_openmls) └── installation_id.rs (unchanged) ``` Export path: - `xmtp_proto::types::GroupId` (existing, unchanged). - `xmtp_proto::types::ids::OpenMlsGroupIdExt` (new). - Re-export the trait from `xmtp_proto::prelude` so `use xmtp_proto::prelude::*` brings it in. ### Conversion boundaries ``` Network (protobuf Vec<u8>) │ boundary: extractors / impls in xmtp_api_d14n, xmtp_api ▼ xmtp_proto::GroupId ◄──── canonical type ────► application layer │ │ │ boundary: repo fns in xmtp_db │ boundary: to_openmls() / to_xmtp() ▼ ▼ Diesel Vec<u8> (internal) openmls::group::GroupId ``` Three boundaries, each handled uniformly: 1. **Network boundary.** Protobuf-generated `Vec<u8>` group_id fields convert to `GroupId` in the API-layer extractors / response parsers. Outgoing requests do `.as_ref().to_vec()` when the protobuf encoder demands `Vec<u8>`. 2. **DB boundary.** Diesel queries pass `id.as_ref()` for reads/writes of `Vec<u8>` columns. Repo return types wrap raw rows into domain types holding `GroupId`. 3. **openmls boundary.** `mls_group.group_id().to_xmtp()` inbound, `group_id.to_openmls()` outbound. ### Migration order Crates migrated bottom-up so downstream crates see `GroupId` in their dependencies before they flip themselves: 1. `xmtp_proto` — add `OpenMlsGroupIdExt`, `to_openmls()` inherent method. Prelude re-export. 2. `xmtp_db` — repo functions + domain structs adopt `GroupId`. Diesel struct fields stay `Vec<u8>`. 3. `xmtp_api` / `xmtp_api_d14n` — trait signatures, extractors, query structs. 4. `xmtp_mls` — `MlsGroup::group_id` field, all group-id-receiving methods, subscription streams, mls_sync boundary conversions. Replace `xmtp_common::rand_vec::<16>()` at group-creation sites with `GroupId::random(provider.rand())`; call `.to_openmls()` when invoking `MlsGroup::new_with_group_id`. 5. `xmtp_archive` — archive export/import paths. 6. Bindings crates (`bindings_ffi`, `bindings_node`, `bindings_wasm`) — inspect for FFI-visible group_id fields. Likely these stay `Vec<u8>` at the FFI boundary, with conversion inside the binding implementation. Each step compiles green on its own. Step boundaries are the natural commit boundaries. ## Error Handling No new error paths in phase 1. Conversions are all infallible. Phase 2 introduces `GroupIdError::InvalidLength` when the 16-byte constraint lands. ## Testing - Existing `group_id` unit tests at `crates/xmtp_proto/src/types/ids/group_id.rs` stay green. - Add unit tests for `OpenMlsGroupIdExt::to_xmtp` and `GroupId::to_openmls` round-trips. - Integration coverage: existing `just test v3` and `just test d14n` suites must pass. The migration is a type refactor; behavioral tests act as regression guards. - Per `CLAUDE.md`: `just lint` before commit. For bindings changes, also `just node lint`. ## Risks & Mitigations - **Risk:** migrating 236 sites in one change produces a massive diff. - **Mitigation:** bottom-up per-crate commits. Each commit self-compiles and self-tests. Reviewer reads one crate at a time. - **Risk:** phase 2 `[u8; 16]` swap gets blocked by a `Deref<Bytes>` or `Borrow<[u8]>` consumer added during phase 1. - **Mitigation:** code review checklist item — new call sites must use `AsRef<[u8]>` / `.as_slice()` only, not `Deref<Bytes>`. Existing `Deref` uses flagged for rewrite during their file's migration step. - **Risk:** openmls name collision causes confusion where imports are implicit. - **Mitigation:** the `as OpenMlsGroupId` rename is the single project-wide convention, documented in this spec and applied in every file that crosses the boundary. - **Risk:** FFI boundary inadvertently exposes `GroupId` via uniffi/napi generated code, breaking mobile/node consumers. - **Mitigation:** bindings crates keep `Vec<u8>` at the FFI surface. Conversion wraps into `GroupId` inside the binding implementation only. ## Phase 2 Preview (out of scope, context only) - Swap inner repr from `bytes::Bytes` to `[u8; 16]`. - Add `Copy` impl. - Replace `From<Vec<u8>>` / `From<&[u8]>` with `TryFrom<&[u8]>` returning `GroupIdError::InvalidLength`. - Remove `Deref<Bytes>`. - Add Diesel `ToSql`/`FromSql` impls and swap Diesel model fields to `GroupId`. - Tighten `FromStr` to require 32 hex chars.