refactor(xmtp_mls): parse the client's own pkg_version once into LibXMTPVersion

crates/xmtp_mls/src/groups/app_data/mod.rs

@@ -40,6 +40,7 @@ use xmtp_mls_common::app_data::{component_id::ComponentId, component_registry::C
 use self::component_source::{
     ComponentSourceError, apply_app_data_update_payload, read_from_app_data_dict,
 };
+use crate::groups::validated_commit::LibXMTPVersion;
 
 #[cfg(any(test, feature = "test-utils"))]
 tokio::task_local! {
@@ -220,14 +221,14 @@ where
 /// Callers replace `mls_group.process_message(provider, message)` with
 /// `process_message_with_app_data(mls_group, provider, message)` and get
 /// back the same `ProcessedMessage` they used to.
-/// `own_version` is the client's pkg_version (threaded from the caller's
+/// `own` is the client's parsed pkg_version (threaded from the caller's
 /// context rather than read from a constant so cross-version tests can
 /// override it).
 pub(crate) fn process_message_with_app_data<Provider: OpenMlsProvider>(
     mls_group: &mut OpenMlsGroup,
     provider: &Provider,
     message: impl Into<ProtocolMessage>,
-    own_version: &str,
+    own: &LibXMTPVersion,
 ) -> Result<ProcessedMessage, ProcessMessageWithAppDataError<Provider::StorageError>> {
     let unverified = mls_group.unprotect_message(provider, message)?;
 
@@ -249,10 +250,10 @@ pub(crate) fn process_message_with_app_data<Provider: OpenMlsProvider>(
             // pausing on unvalidated input would let any member freeze
             // the group for everyone. Application messages are
             // unaffected (`committed_proposals()` is `None` for them).
-            if let Some(min_version) = committed_floor_exceeding(mls_group, own_version) {
+            if let Some(min_version) = committed_floor_exceeding(mls_group, own) {
                 return Err(ProcessMessageWithAppDataError::ProtocolVersionTooLow {
                     min_version,
-                    own_version: own_version.to_string(),
+                    own_version: own.to_string(),
                 });
             }
             // Collect owned (id, operation) tuples so the iterator doesn't
@@ -620,9 +621,9 @@ pub(crate) fn load_component_registry(
 /// treatment of malformed priors: garbage must never brick the group.
 pub(crate) fn committed_floor_exceeding(
     mls_group: &OpenMlsGroup,
-    own_version: &str,
+    own: &LibXMTPVersion,
 ) -> Option<String> {
-    committed_floor_exceeding_in_extensions(mls_group.extensions(), own_version)
+    committed_floor_exceeding_in_extensions(mls_group.extensions(), own)
 }
 
 /// Extensions-only variant of [`committed_floor_exceeding`], split out
@@ -631,19 +632,16 @@ pub(crate) fn committed_floor_exceeding(
 /// `OpenMlsGroup`.
 pub(crate) fn committed_floor_exceeding_in_extensions(
     extensions: &openmls::extensions::Extensions<openmls::group::GroupContext>,
-    own_version: &str,
+    own: &LibXMTPVersion,
 ) -> Option<String> {
-    use super::validated_commit::LibXMTPVersion;
-
     let bytes = extensions
         .app_data_dictionary()?
         .dictionary()
         .get(&ComponentId::MIN_SUPPORTED_PROTOCOL_VERSION.as_u16())?
         .to_vec();
     let floor = String::from_utf8(bytes).ok()?;
-    let own = LibXMTPVersion::parse(own_version).ok()?;
     let floor_version = LibXMTPVersion::parse(&floor).ok()?;
-    (floor_version > own).then_some(floor)
+    (floor_version > *own).then_some(floor)
 }
 
 /// Extensions-only variant of [`load_component_registry`]. Mirrors the
@@ -730,6 +728,13 @@ mod tests {
         Extensions::from_vec(vec![]).expect("empty extensions are always valid")
     }
 
+    /// Parse a semver string the way the production caller does (once, from
+    /// the client's own `pkg_version`). Panics on invalid input — matching
+    /// `VersionInfo`, which asserts its own version is valid at construction.
+    fn ver(s: &str) -> LibXMTPVersion {
+        LibXMTPVersion::parse(s).unwrap()
+    }
+
     #[test]
     fn unmigrated_without_override_is_not_migrated() {
         // Invariant (a): no dict, no override → legacy authoritative.
@@ -808,7 +813,7 @@ mod tests {
             b"2.0.0".to_vec(),
         )]);
         assert_eq!(
-            committed_floor_exceeding_in_extensions(&exts, "1.11.0"),
+            committed_floor_exceeding_in_extensions(&exts, &ver("1.11.0")),
             Some("2.0.0".to_string())
         );
         // Prerelease floors order correctly under semver: 1.11.0-dev
@@ -818,11 +823,11 @@ mod tests {
             b"1.11.0-dev".to_vec(),
         )]);
         assert_eq!(
-            committed_floor_exceeding_in_extensions(&exts, "1.10.0"),
+            committed_floor_exceeding_in_extensions(&exts, &ver("1.10.0")),
             Some("1.11.0-dev".to_string())
         );
         assert_eq!(
-            committed_floor_exceeding_in_extensions(&exts, "1.11.0"),
+            committed_floor_exceeding_in_extensions(&exts, &ver("1.11.0")),
             None
         );
     }
@@ -835,30 +840,30 @@ mod tests {
         )]);
         // Equal: not paused — the floor is inclusive.
         assert_eq!(
-            committed_floor_exceeding_in_extensions(&exts, "1.11.0"),
+            committed_floor_exceeding_in_extensions(&exts, &ver("1.11.0")),
             None
         );
         // Above: not paused.
         assert_eq!(
-            committed_floor_exceeding_in_extensions(&exts, "1.12.0"),
+            committed_floor_exceeding_in_extensions(&exts, &ver("1.12.0")),
             None
         );
     }
 
     #[test]
     fn missing_floor_or_dict_does_not_fire() {
         assert_eq!(
-            committed_floor_exceeding_in_extensions(&empty_extensions(), "1.11.0"),
+            committed_floor_exceeding_in_extensions(&empty_extensions(), &ver("1.11.0")),
             None
         );
         assert_eq!(
-            committed_floor_exceeding_in_extensions(&extensions_with_dict(&[]), "1.11.0"),
+            committed_floor_exceeding_in_extensions(&extensions_with_dict(&[]), &ver("1.11.0")),
             None
         );
         // Dict present with other components but no floor entry.
         let exts = extensions_with_dict(&[(ComponentId::GROUP_NAME.as_u16(), b"name".to_vec())]);
         assert_eq!(
-            committed_floor_exceeding_in_extensions(&exts, "1.11.0"),
+            committed_floor_exceeding_in_extensions(&exts, &ver("1.11.0")),
             None
         );
     }
@@ -871,28 +876,21 @@ mod tests {
             vec![0xFF, 0xFE],
         )]);
         assert_eq!(
-            committed_floor_exceeding_in_extensions(&exts, "1.11.0"),
+            committed_floor_exceeding_in_extensions(&exts, &ver("1.11.0")),
             None
         );
-        // Unparseable semver → no floor.
+        // Unparseable floor semver → no floor.
         let exts = extensions_with_dict(&[(
             ComponentId::MIN_SUPPORTED_PROTOCOL_VERSION.as_u16(),
             b"not-a-version".to_vec(),
         )]);
         assert_eq!(
-            committed_floor_exceeding_in_extensions(&exts, "1.11.0"),
-            None
-        );
-        // Own version unparseable (shouldn't happen in a real build) →
-        // skip the guard rather than pausing on garbage.
-        let exts = extensions_with_dict(&[(
-            ComponentId::MIN_SUPPORTED_PROTOCOL_VERSION.as_u16(),
-            b"2.0.0".to_vec(),
-        )]);
-        assert_eq!(
-            committed_floor_exceeding_in_extensions(&exts, "garbage"),
+            committed_floor_exceeding_in_extensions(&exts, &ver("1.11.0")),
             None
         );
+        // The client's own version can no longer be unparseable here: it is
+        // parsed once and asserted valid when `VersionInfo` is built, so this
+        // guard only ever compares against a valid `LibXMTPVersion`.
     }
 
     // ========================================================================

crates/xmtp_mls/src/groups/mls_sync.rs

@@ -482,10 +482,10 @@ where
                 required_min_version_str
             );
             let current_version_str = self.context.version_info().pkg_version();
-            let current_version = LibXMTPVersion::parse(current_version_str)?;
+            let current_version = self.context.version_info().pkg_semver();
             let required_min_version = LibXMTPVersion::parse(&required_min_version_str)?;
 
-            if required_min_version <= current_version {
+            if required_min_version <= *current_version {
                 tracing::info!(
                     "Unpausing group since version requirements are met. \
                      Group ID: {}",
@@ -1164,7 +1164,7 @@ where
                 mls_group,
                 &provider,
                 message.clone(),
-                self.context.version_info().pkg_version(),
+                self.context.version_info().pkg_semver(),
             ));
             // Roll back: sync with the server before committing.
             Ok::<TransactionOutcome<()>, StorageError>(Rollback)
@@ -1254,7 +1254,7 @@ where
                 // by a member to freeze a group.
                 if let Some(min_version) = super::app_data::committed_floor_exceeding(
                     mls_group,
-                    self.context.version_info().pkg_version(),
+                    self.context.version_info().pkg_semver(),
                 ) {
                     return Err(CommitValidationError::ProtocolVersionTooLow(min_version).into());
                 }
@@ -1433,7 +1433,7 @@ where
                 mls_group,
                 &provider,
                 message.clone(),
-                self.context.version_info().pkg_version(),
+                self.context.version_info().pkg_semver(),
             )
             .map_err(GroupMessageProcessingError::from_app_data_processing)?;
             let identifier = self.process_external_message(

crates/xmtp_mls/src/groups/tests/test_commit_log_fork_detection.rs

@@ -921,7 +921,7 @@ async fn test_merge_staged_commit_logged_rejects_non_advancing_authenticator()
             &mut group_copy,
             &provider,
             commit_envelope.message.clone(),
-            bo.context.version_info().pkg_version(),
+            bo.context.version_info().pkg_semver(),
         ));
         Ok::<TransactionOutcome<()>, StorageError>(Rollback)
     });

crates/xmtp_mls/src/groups/validated_commit.rs

@@ -311,6 +311,12 @@ impl LibXMTPVersion {
     }
 }
 
+impl std::fmt::Display for LibXMTPVersion {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        std::fmt::Display::fmt(&self.0, f)
+    }
+}
+
 /**
  * A [`ValidatedCommit`] is a summary of changes coming from a MLS commit, after all of our validation rules have been applied
  *
@@ -387,7 +393,7 @@ impl ValidatedCommit {
         if is_migrated
             && let Some(min_version) = super::app_data::committed_floor_exceeding(
                 openmls_group,
-                context.version_info().pkg_version(),
+                context.version_info().pkg_semver(),
             )
         {
             return Err(CommitValidationError::ProtocolVersionTooLow(min_version));
@@ -741,15 +747,15 @@ impl ValidatedCommit {
             .metadata_validation_info
             .minimum_supported_protocol_version
         {
-            let current_version = LibXMTPVersion::parse(context.version_info().pkg_version())?;
+            let current_version = context.version_info().pkg_semver();
             let min_supported_version = LibXMTPVersion::parse(min_version)?;
             tracing::info!(
                 "Validating commit with min_supported_version: {:?}, current_version: {:?}",
                 min_supported_version,
                 current_version
             );
 
-            if min_supported_version > current_version {
+            if min_supported_version > *current_version {
                 return Err(CommitValidationError::ProtocolVersionTooLow(
                     min_version.clone(),
                 ));

crates/xmtp_mls/src/groups/welcome_sync.rs

@@ -284,9 +284,10 @@ where
         if paused.is_empty() {
             return Ok(0);
         }
-        // Parse current version once; reuse across every paused group.
+        // The client's own version is parsed once at `VersionInfo`
+        // construction; reuse it across every paused group.
         let own_version_str = self.context.version_info().pkg_version().to_string();
-        let own_v = LibXMTPVersion::parse(&own_version_str)?;
+        let own_v = self.context.version_info().pkg_semver();
 
         let mut unstuck = 0usize;
         for (group_id, required_str) in paused {
@@ -301,7 +302,7 @@ where
                 );
                 continue;
             };
-            if required_v <= own_v {
+            if required_v <= *own_v {
                 // Same leniency as the parse-error branch above: a
                 // transient DB failure on one row shouldn't abort the
                 // sweep for the others. The next sync sweep will pick

crates/xmtp_mls/src/groups/welcomes/xmtp_welcome.rs

@@ -428,10 +428,9 @@ where
             let min_version = MlsGroup::<C>::min_protocol_version_from_extensions(metadata);
             if let Some(min_version) = min_version {
                 let current_version_str = context.version_info().pkg_version();
-                let current_version =
-                    LibXMTPVersion::parse(current_version_str).ok()?;
+                let current_version = context.version_info().pkg_semver();
                 let required_min_version = LibXMTPVersion::parse(&min_version.clone()).ok()?;
-                if required_min_version > current_version {
+                if required_min_version > *current_version {
                     tracing::warn!(
                         "Saving group from welcome as paused since version requirements are not met. \
                         Group ID: {}, \

crates/xmtp_mls/src/utils/mod.rs

@@ -1,5 +1,7 @@
 use std::sync::Arc;
 
+use crate::groups::validated_commit::LibXMTPVersion;
+
 #[cfg(feature = "bench")]
 pub mod bench;
 pub mod cleanup_duplicate_updates;
@@ -85,24 +87,48 @@ pub mod id {
 #[derive(Clone, Debug, PartialEq)]
 pub struct VersionInfo {
     pkg_version: Arc<str>,
+    /// `pkg_version` parsed once at construction. This is the client's own
+    /// build identity, not remote data, so it is always valid semver
+    /// (see [`VersionInfo::new`]) — receive-path guards can compare against
+    /// it without re-parsing the string on every message.
+    pkg_semver: LibXMTPVersion,
 }
 
 impl Default for VersionInfo {
     fn default() -> Self {
-        Self {
-            pkg_version: env!("CARGO_PKG_VERSION").into(),
-        }
+        Self::new(env!("CARGO_PKG_VERSION"))
     }
 }
 
 impl VersionInfo {
+    /// Build a `VersionInfo`, parsing and caching the semver form.
+    ///
+    /// `version` is the client's own package version — in production always
+    /// the compile-time `CARGO_PKG_VERSION`. A non-semver value is a build
+    /// or configuration bug, not runtime data, so it panics here rather than
+    /// silently disabling the below-floor pause on every group later.
+    fn new(version: &str) -> Self {
+        let pkg_semver = LibXMTPVersion::parse(version)
+            .unwrap_or_else(|_| panic!("client pkg_version {version:?} is not valid semver"));
+        Self {
+            pkg_version: version.into(),
+            pkg_semver,
+        }
+    }
+
     pub fn pkg_version(&self) -> &str {
         &self.pkg_version
     }
 
+    /// The client's own version, parsed once at construction. Prefer this
+    /// over re-parsing [`pkg_version`](Self::pkg_version) on hot paths.
+    pub fn pkg_semver(&self) -> &LibXMTPVersion {
+        &self.pkg_semver
+    }
+
     // Test only function to update the version of the client
     #[cfg(test)]
     pub fn test_update_version(&mut self, version: &str) {
-        self.pkg_version = version.into();
+        *self = Self::new(version);
     }
 }