review: replace `expect` by `if let Some`

pallets/smith-members/src/impls.rs

@@ -4,18 +4,18 @@ use sp_runtime::traits::Convert;
 
 impl<T: Config> pallet_authority_members::OnOutgoingMember<T::MemberId> for Pallet<T> {
     fn on_outgoing_member(member_id: T::MemberId) {
-        Pallet::<T>::on_smith_goes_offline(
-            T::IdtyIdOfAuthorityId::convert(member_id).expect("convertion should be ok"),
-        );
+        if let Some(member_id) = T::IdtyIdOfAuthorityId::convert(member_id) {
+            Pallet::<T>::on_smith_goes_offline(member_id);
+        }
     }
 }
 
 /// As long as a Smith is in the authority set, he will not expire.
 impl<T: Config> pallet_authority_members::OnIncomingMember<T::MemberId> for Pallet<T> {
     fn on_incoming_member(member_id: T::MemberId) {
-        Pallet::<T>::on_smith_goes_online(
-            T::IdtyIdOfAuthorityId::convert(member_id).expect("convertion should be ok"),
-        );
+        if let Some(member_id) = T::IdtyIdOfAuthorityId::convert(member_id) {
+            Pallet::<T>::on_smith_goes_online(member_id);
+        }
     }
 }
 

pallets/smith-members/src/lib.rs

@@ -212,10 +212,9 @@ pub mod pallet {
             for (issuer, issued_certs) in cert_meta_by_issuer {
                 // Write CertsByIssuer
                 Smiths::<T>::mutate(issuer, |maybe_smith_meta| {
-                    let maybe_smith_meta = maybe_smith_meta
-                        .as_mut()
-                        .expect("issuers must have received certs as well");
-                    maybe_smith_meta.issued_certs = issued_certs;
+                    if let Some(smith_meta) = maybe_smith_meta {
+                        smith_meta.issued_certs = issued_certs;
+                    }
                 });
             }
         }
@@ -372,8 +371,9 @@ impl<T: Config> Pallet<T> {
 
     fn do_accept_invitation(receiver: T::IdtyIndex) -> DispatchResultWithPostInfo {
         Smiths::<T>::mutate(receiver, |maybe_smith_meta| {
-            let maybe_smith_meta = maybe_smith_meta.as_mut().expect("status checked earlier");
-            maybe_smith_meta.status = SmithStatus::Pending;
+            if let Some(smith_meta) = maybe_smith_meta {
+                smith_meta.status = SmithStatus::Pending;
+            }
         });
         Self::deposit_event(Event::<T>::InvitationAccepted {
             idty_index: receiver,
@@ -426,34 +426,36 @@ impl<T: Config> Pallet<T> {
 
     fn do_certify_smith(receiver: T::IdtyIndex, issuer: T::IdtyIndex) {
         Smiths::<T>::mutate(issuer, |maybe_smith_meta| {
-            let maybe_smith_meta = maybe_smith_meta.as_mut().expect("issuer checked earlier");
-            maybe_smith_meta.issued_certs.push(receiver);
-            maybe_smith_meta.issued_certs.sort();
+            if let Some(smith_meta) = maybe_smith_meta {
+                smith_meta.issued_certs.push(receiver);
+                smith_meta.issued_certs.sort();
+            }
         });
         Smiths::<T>::mutate(receiver, |maybe_smith_meta| {
-            let maybe_smith_meta = maybe_smith_meta.as_mut().expect("receiver checked earlier");
-            maybe_smith_meta.received_certs.push(issuer);
-            maybe_smith_meta.received_certs.sort();
-            maybe_smith_meta.status = if maybe_smith_meta.received_certs.len()
-                >= T::MinCertForMembership::get() as usize
-            {
+            if let Some(smith_meta) = maybe_smith_meta {
+                smith_meta.received_certs.push(issuer);
+                smith_meta.received_certs.sort();
+                smith_meta.status =
+                    if smith_meta.received_certs.len() >= T::MinCertForMembership::get() as usize {
                         SmithStatus::Smith
                     } else {
                         SmithStatus::Pending
                     };
                 // expiry postponed
-            let new_expires_on = CurrentSession::<T>::get() + T::SmithInactivityMaxDuration::get();
-            maybe_smith_meta.expires_on = Some(new_expires_on);
+                let new_expires_on =
+                    CurrentSession::<T>::get() + T::SmithInactivityMaxDuration::get();
+                smith_meta.expires_on = Some(new_expires_on);
                 Self::deposit_event(Event::<T>::CertificationReceived {
                     idty_index: receiver,
                     issued_by: issuer,
                 });
-            if maybe_smith_meta.status == SmithStatus::Smith {
+                if smith_meta.status == SmithStatus::Smith {
                     Self::deposit_event(Event::<T>::PromotedToSmith {
                         idty_index: receiver,
                     });
                 }
                 // TODO: unschedule old expiry
+            }
         });
     }
 
@@ -480,21 +482,23 @@ impl<T: Config> Pallet<T> {
     fn _do_exclude_smith(idty_index: T::IdtyIndex, reason: SmithRemovalReason) {
         let mut lost_certs = vec![];
         Smiths::<T>::mutate(idty_index, |maybe_smith_meta| {
-            let maybe_smith_meta = maybe_smith_meta.as_mut().expect("checked earlier");
-            maybe_smith_meta.expires_on = None;
-            maybe_smith_meta.status = SmithStatus::Excluded;
-            for cert in &maybe_smith_meta.received_certs {
+            if let Some(smith_meta) = maybe_smith_meta {
+                smith_meta.expires_on = None;
+                smith_meta.status = SmithStatus::Excluded;
+                for cert in &smith_meta.received_certs {
                     lost_certs.push(*cert);
                 }
-            maybe_smith_meta.received_certs = vec![];
+                smith_meta.received_certs = vec![];
                 // N.B.: the issued certs are kept in case the smith joins back
+            }
         });
         // We remove the lost certs from their issuer's stock
         for lost_cert in lost_certs {
             Smiths::<T>::mutate(lost_cert, |maybe_smith_meta| {
-                let maybe_smith_meta = maybe_smith_meta.as_mut().expect("checked earlier");
-                if let Ok(index) = maybe_smith_meta.issued_certs.binary_search(&idty_index) {
-                    maybe_smith_meta.issued_certs.remove(index);
+                if let Some(smith_meta) = maybe_smith_meta {
+                    if let Ok(index) = smith_meta.issued_certs.binary_search(&idty_index) {
+                        smith_meta.issued_certs.remove(index);
+                    }
                 }
             });
         }
@@ -507,10 +511,11 @@ impl<T: Config> Pallet<T> {
         if let Some(smith_meta) = Smiths::<T>::get(idty_index) {
             if smith_meta.expires_on.is_some() {
                 Smiths::<T>::mutate(idty_index, |maybe_smith_meta| {
-                    let maybe_smith_meta = maybe_smith_meta.as_mut().expect("checked earlier");
+                    if let Some(smith_meta) = maybe_smith_meta {
                         // As long as the smith is online, it cannot expire
-                    maybe_smith_meta.expires_on = None;
+                        smith_meta.expires_on = None;
                         // FIXME: unschedule old expiry
+                    }
                 });
             }
         }
@@ -520,12 +525,13 @@ impl<T: Config> Pallet<T> {
         if let Some(smith_meta) = Smiths::<T>::get(idty_index) {
             if smith_meta.expires_on.is_none() {
                 Smiths::<T>::mutate(idty_index, |maybe_smith_meta| {
-                    let maybe_smith_meta = maybe_smith_meta.as_mut().expect("checked earlier");
+                    if let Some(smith_meta) = maybe_smith_meta {
                         // As long as the smith is online, it cannot expire
                         let new_expires_on =
                             CurrentSession::<T>::get() + T::SmithInactivityMaxDuration::get();
-                    maybe_smith_meta.expires_on = Some(new_expires_on);
+                        smith_meta.expires_on = Some(new_expires_on);
                         ExpiresOn::<T>::append(new_expires_on, idty_index);
+                    }
                 });
             }
         }