Remove lots of unwraps

xmtp_mls/src/api/identity.rs

@@ -140,7 +140,12 @@ where
             .responses
             .into_iter()
             .filter(|inbox_id| inbox_id.inbox_id.is_some())
-            .map(|inbox_id| (inbox_id.address, inbox_id.inbox_id.unwrap()))
+            .map(|inbox_id| {
+                (
+                    inbox_id.address,
+                    inbox_id.inbox_id.expect("Checked for some"),
+                )
+            })
             .collect())
     }
 }

xmtp_mls/src/builder.rs

@@ -480,7 +480,8 @@ mod tests {
             credential: Credential::new(CredentialType::Basic, rand_vec()),
             signature_request: None,
         })
-            .into();
+            .try_into()
+            .unwrap();
 
         stored.store(&store.conn().unwrap()).unwrap();
         let wrapper = ApiClientWrapper::new(mock_api, Retry::default());
@@ -506,7 +507,8 @@ mod tests {
             credential: Credential::new(CredentialType::Basic, rand_vec()),
             signature_request: None,
         })
-            .into();
+            .try_into()
+            .unwrap();
 
         stored.store(&store.conn().unwrap()).unwrap();
 

xmtp_mls/src/client.rs

@@ -95,11 +95,17 @@ pub enum ClientError {
     SignatureRequest(#[from] SignatureRequestError),
     // the box is to prevent infinite cycle between client and group errors
     #[error(transparent)]
-    Group(#[from] Box<GroupError>),
+    Group(Box<GroupError>),
     #[error("generic:{0}")]
     Generic(String),
 }
 
+impl From<GroupError> for ClientError {
+    fn from(err: GroupError) -> ClientError {
+        ClientError::Group(Box::new(err))
+    }
+}
+
 impl crate::retry::RetryableError for ClientError {
     fn is_retryable(&self) -> bool {
         match self {
@@ -355,8 +361,7 @@ where
             GroupMembershipState::Allowed,
             permissions_policy_set.unwrap_or_default(),
             opts,
-        )
-        .map_err(Box::new)?;
+        )?;
 
         // notify any streams of the new group
         let _ = self.local_events.send(LocalEvents::NewGroup(group.clone()));
@@ -366,8 +371,7 @@ where
 
     pub(crate) fn create_sync_group(&self) -> Result<MlsGroup, ClientError> {
         log::info!("creating sync group");
-        let sync_group =
-            MlsGroup::create_and_insert_sync_group(self.context.clone()).map_err(Box::new)?;
+        let sync_group = MlsGroup::create_and_insert_sync_group(self.context.clone())?;
 
         Ok(sync_group)
     }

xmtp_mls/src/groups/message_history.rs

@@ -444,7 +444,9 @@ pub(crate) async fn download_history_bundle(
             response
         );
         Err(MessageHistoryError::Reqwest(
-            response.error_for_status().unwrap_err(),
+            response
+                .error_for_status()
+                .expect_err("Checked for success"),
         ))
     }
 }

xmtp_mls/src/groups/mod.rs

@@ -178,6 +178,8 @@ pub enum GroupError {
     PublishCancelled,
     #[error("the publish failed to complete due to panic")]
     PublishPanicked,
+    #[error("Missing metadata field {name}")]
+    MissingMetadataField { name: String },
 }
 
 impl RetryableError for GroupError {
@@ -1074,7 +1076,11 @@ pub fn build_extensions_for_permissions_update(
         PermissionUpdateType::UpdateMetadata => {
             let mut metadata_policy = existing_policy_set.update_metadata_policy.clone();
             metadata_policy.insert(
-                update_permissions_intent.metadata_field_name.unwrap(),
+                update_permissions_intent.metadata_field_name.ok_or(
+                    GroupError::MissingMetadataField {
+                        name: "metadata_field_name".into(),
+                    },
+                )?,
                 update_permissions_intent.policy_option.into(),
             );
             PolicySet::new(

xmtp_mls/src/groups/subscriptions.rs

@@ -9,7 +9,6 @@ use crate::storage::group_message::StoredGroupMessage;
 use crate::subscriptions::{MessagesStreamInfo, StreamHandle};
 use crate::XmtpApi;
 use crate::{retry::Retry, retry_async, Client};
-use prost::Message;
 use xmtp_proto::xmtp::mls::api::v1::GroupMessage;
 
 impl MlsGroup {
@@ -82,21 +81,6 @@ impl MlsGroup {
         Ok(new_message)
     }
 
-    pub async fn process_streamed_group_message<ApiClient>(
-        &self,
-        envelope_bytes: Vec<u8>,
-        client: Arc<Client<ApiClient>>,
-    ) -> Result<StoredGroupMessage, GroupError>
-    where
-        ApiClient: XmtpApi,
-    {
-        let envelope = GroupMessage::decode(envelope_bytes.as_slice())
-            .map_err(|e| GroupError::Generic(e.to_string()))?;
-
-        let message = self.process_stream_entry(envelope, client).await?;
-        Ok(message.unwrap())
-    }
-
     pub async fn stream<ApiClient>(
         &self,
         client: Arc<Client<ApiClient>>,
@@ -140,6 +124,7 @@ impl MlsGroup {
 
 #[cfg(test)]
 mod tests {
+    use super::*;
     use prost::Message;
     use std::{sync::Arc, time::Duration};
     use tokio_stream::wrappers::UnboundedReceiverStream;
@@ -151,6 +136,23 @@ mod tests {
     };
     use futures::StreamExt;
 
+    impl MlsGroup {
+        pub async fn process_streamed_group_message<ApiClient>(
+            &self,
+            envelope_bytes: Vec<u8>,
+            client: Arc<Client<ApiClient>>,
+        ) -> Result<StoredGroupMessage, GroupError>
+        where
+            ApiClient: XmtpApi,
+        {
+            let envelope = GroupMessage::decode(envelope_bytes.as_slice())
+                .map_err(|e| GroupError::Generic(e.to_string()))?;
+
+            let message = self.process_stream_entry(envelope, client).await?;
+            Ok(message.unwrap())
+        }
+    }
+
     #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
     async fn test_decode_group_message_bytes() {
         let amal = ClientBuilder::new_test_client(&generate_local_wallet()).await;

xmtp_mls/src/groups/sync.rs

@@ -89,7 +89,7 @@ impl MlsGroup {
         log::info!(
             "current epoch for [{}] in sync() is Epoch: [{}]",
             client.inbox_id(),
-            self.load_mls_group(&mls_provider).unwrap().epoch()
+            self.load_mls_group(&mls_provider)?.epoch()
         );
         self.maybe_update_installations(&mls_provider, None, client)
             .await?;
@@ -985,8 +985,7 @@ impl MlsGroup {
         )?;
 
         for intent in intents {
-            if intent.post_commit_data.is_some() {
-                let post_commit_data = intent.post_commit_data.unwrap();
+            if let Some(post_commit_data) = intent.post_commit_data {
                 let post_commit_action = PostCommitAction::from_bytes(post_commit_data.as_slice())?;
                 match post_commit_action {
                     PostCommitAction::SendWelcomes(action) => {

xmtp_mls/src/groups/validated_commit.rs

@@ -782,18 +782,19 @@ fn extract_actor(
         )?;
 
     // If there is both a path update and there are proposals we need to make sure that they are from the same actor
-    if path_update_leaf_node.is_some() && proposal_author_leaf_index.is_some() {
-        let proposal_author = openmls_group
-            .member_at(*proposal_author_leaf_index.unwrap())
+    if let (Some(path_update_leaf_node), Some(proposal_author_leaf_index)) =
+        (path_update_leaf_node, proposal_author_leaf_index)
+    {
+        let proposal_author_leaf_index = openmls_group
+            .member_at(*proposal_author_leaf_index)
             .ok_or(CommitValidationError::ActorCouldNotBeFound)?;
 
         // Verify that the signature keys are the same
         if path_update_leaf_node
-            .unwrap()
             .signature_key()
             .as_slice()
             .to_vec()
-            .ne(&proposal_author.signature_key)
+            .ne(&proposal_author_leaf_index.signature_key)
         {
             return Err(CommitValidationError::MultipleActors);
         }

xmtp_mls/src/identity.rs

@@ -68,7 +68,9 @@ impl IdentityStrategy {
         let stored_identity: Option<Identity> = provider
             .conn_ref()
             .fetch(&())?
-            .map(|i: StoredIdentity| i.into());
+            .map(|i: StoredIdentity| i.try_into())
+            .transpose()?;
+
         debug!("identity in store: {:?}", stored_identity);
         match self {
             IdentityStrategy::CachedOnly => {
@@ -428,7 +430,7 @@ impl Identity {
         let kp_bytes = kp.tls_serialize_detached()?;
         api_client.upload_key_package(kp_bytes, true).await?;
 
-        Ok(StoredIdentity::from(self).store(provider.conn_ref())?)
+        Ok(StoredIdentity::try_from(self)?.store(provider.conn_ref())?)
     }
 }
 
@@ -440,9 +442,7 @@ async fn sign_with_installation_key(
     let verifying_key = signing_key.verifying_key();
     let mut prehashed: Sha512 = Sha512::new();
     prehashed.update(signature_text.clone());
-    let sig = signing_key
-        .sign_prehashed(prehashed, Some(INSTALLATION_KEY_SIGNATURE_CONTEXT))
-        .unwrap();
+    let sig = signing_key.sign_prehashed(prehashed, Some(INSTALLATION_KEY_SIGNATURE_CONTEXT))?;
 
     let installation_key_sig = InstallationKeySignature::new(
         signature_text.clone(),

xmtp_mls/src/identity_updates.rs

@@ -183,7 +183,7 @@ where
             StoredAssociationState::write_to_cache(
                 conn,
                 inbox_id.as_ref().to_string(),
-                last_sequence_id.unwrap(),
+                last_sequence_id.expect("checked for some"),
                 final_state.clone(),
             )?;
         }

xmtp_mls/src/lib.rs

@@ -1,4 +1,8 @@
 #![recursion_limit = "256"]
+#![cfg_attr(
+    not(any(test, feature = "test-utils", feature = "bench")),
+    warn(clippy::unwrap_used)
+)]
 pub mod api;
 pub mod builder;
 pub mod client;

xmtp_mls/src/storage/encrypted_store/association_state.rs

@@ -78,7 +78,7 @@ impl StoredAssociationState {
             })
             .transpose();
 
-        if result.is_ok() && result.as_ref().unwrap().is_some() {
+        if let Ok(Some(_)) = result {
             log::debug!(
                 "Loaded association state from cache: {} {}",
                 inbox_id,

xmtp_mls/src/storage/encrypted_store/group.rs

@@ -220,7 +220,7 @@ impl DbConnection {
                 .optional()?;
 
             if maybe_inserted_group.is_none() {
-                let existing_group: StoredGroup = dsl::groups.find(group.id).first(conn).unwrap();
+                let existing_group: StoredGroup = dsl::groups.find(group.id).first(conn)?;
                 if existing_group.welcome_id == group.welcome_id {
                     // Error so OpenMLS db transaction are rolled back on duplicate welcomes
                     return Err(diesel::result::Error::DatabaseError(

xmtp_mls/src/storage/encrypted_store/identity.rs

@@ -1,4 +1,4 @@
-use crate::storage::encrypted_store::schema::identity;
+use crate::storage::{encrypted_store::schema::identity, StorageError};
 use diesel::prelude::*;
 use xmtp_id::InboxId;
 
@@ -33,25 +33,29 @@ impl StoredIdentity {
     }
 }
 
-impl From<&Identity> for StoredIdentity {
-    fn from(identity: &Identity) -> Self {
-        StoredIdentity {
+impl TryFrom<&Identity> for StoredIdentity {
+    type Error = StorageError;
+
+    fn try_from(identity: &Identity) -> Result<Self, Self::Error> {
+        Ok(StoredIdentity {
             inbox_id: identity.inbox_id.clone(),
-            installation_keys: db_serialize(&identity.installation_keys).unwrap(),
-            credential_bytes: db_serialize(&identity.credential()).unwrap(),
+            installation_keys: db_serialize(&identity.installation_keys)?,
+            credential_bytes: db_serialize(&identity.credential())?,
             rowid: None,
-        }
+        })
     }
 }
 
-impl From<StoredIdentity> for Identity {
-    fn from(identity: StoredIdentity) -> Self {
-        Identity {
+impl TryFrom<StoredIdentity> for Identity {
+    type Error = StorageError;
+
+    fn try_from(identity: StoredIdentity) -> Result<Self, Self::Error> {
+        Ok(Identity {
             inbox_id: identity.inbox_id.clone(),
-            installation_keys: db_deserialize(&identity.installation_keys).unwrap(),
-            credential: db_deserialize(&identity.credential_bytes).unwrap(),
+            installation_keys: db_deserialize(&identity.installation_keys)?,
+            credential: db_deserialize(&identity.credential_bytes)?,
             signature_request: None,
-        }
+        })
     }
 }
 

xmtp_mls/src/subscriptions.rs

@@ -114,7 +114,7 @@ where
             }
         }
 
-        Ok(creation_result.unwrap())
+        Ok(creation_result?)
     }
 
     pub async fn process_streamed_welcome_message(