Remove unnecessary panics

benches/sign.rs

@@ -126,7 +126,8 @@ fn criterion_benchmark(c: &mut Criterion) {
     let mut participants_secret_comshares =
         Vec::<SecretCommShareList>::with_capacity(NUMBER_OF_PARTICIPANTS as usize);
     let (p1_public_comshares, p1_secret_comshares) =
-        generate_commitment_share_lists(&mut OsRng, &participants_secret_keys[0].clone(), 1);
+        generate_commitment_share_lists(&mut OsRng, &participants_secret_keys[0].clone(), 1)
+            .unwrap();
     participants_public_comshares.push(p1_public_comshares);
     participants_secret_comshares.push(p1_secret_comshares.clone());
 
@@ -135,7 +136,8 @@ fn criterion_benchmark(c: &mut Criterion) {
             &mut OsRng,
             &participants_secret_keys[(i - 1) as usize].clone(),
             1,
-        );
+        )
+        .unwrap();
         participants_public_comshares.push(pi_public_comshares);
         participants_secret_comshares.push(pi_secret_comshares);
     }

src/dkg/key_generation.rs

@@ -723,6 +723,9 @@ impl<C: CipherSuite> DistributedKeyGeneration<RoundOne, C> {
                                 continue;
                             }
                         };
+
+                        // The two unwrap() below cannot fail, as the participants here are dealers and hence
+                        // always have a proof of secret key and commitments to their private polynomial evaluations.
                         match p
                             .proof_of_secret_key
                             .as_ref()
@@ -789,6 +792,7 @@ impl<C: CipherSuite> DistributedKeyGeneration<RoundOne, C> {
             Vec::with_capacity(parameters.n as usize - 1);
 
         for p in participants.iter() {
+            // This unwrap() cannot fail, as we would have already ended if the participant was a signer.
             let share =
                 SecretShare::<C>::evaluate_polynomial(my_index, &p.index, my_coefficients.unwrap());
 
@@ -885,9 +889,9 @@ impl<C: CipherSuite> DistributedKeyGeneration<RoundOne, C> {
 
                     for commitment in self.state.their_commitments.as_ref().unwrap().iter() {
                         if commitment.index == encrypted_share.sender_index {
-                            // If the decrypted share is incorrect, P_i builds
-                            // a complaint
+                            // If the decrypted share is incorrect, P_i builds a complaint.
 
+                            // This unwrap() cannot fail.
                             if decrypted_share.is_err()
                                 || decrypted_share_ref
                                     .as_ref()
@@ -986,22 +990,22 @@ impl<C: CipherSuite> DistributedKeyGeneration<RoundTwo, C> {
     /// my_commitment is needed for now, but won't be when the distinction
     /// dealers/signers is implemented.
     pub(crate) fn calculate_group_key(&self) -> FrostResult<C, GroupVerifyingKey<C>> {
-        let mut index_vector =
-            Vec::with_capacity(self.state.their_commitments.as_ref().unwrap().len());
+        let commitments = self
+            .state
+            .their_commitments
+            .as_ref()
+            .ok_or(Error::InvalidGroupKey)?;
+        let mut index_vector = Vec::with_capacity(commitments.len());
 
-        for commitment in self.state.their_commitments.as_ref().unwrap().iter() {
+        for commitment in commitments.iter() {
             index_vector.push(commitment.index);
         }
 
         let mut group_key = <C as CipherSuite>::G::zero();
 
         // The group key is the interpolation at 0 of all index 0 of the dealers' commitments.
-        for commitment in self.state.their_commitments.as_ref().unwrap().iter() {
-            let coeff = match calculate_lagrange_coefficients::<C>(commitment.index, &index_vector)
-            {
-                Ok(s) => s,
-                Err(error) => return Err(Error::Custom(error.to_string())),
-            };
+        for commitment in commitments.iter() {
+            let coeff = calculate_lagrange_coefficients::<C>(commitment.index, &index_vector)?;
 
             group_key += commitment.public_key().unwrap().mul(coeff);
         }
@@ -1060,13 +1064,14 @@ impl<C: CipherSuite> DistributedKeyGeneration<RoundTwo, C> {
             return complaint.maker_index;
         };
 
-        let share = decrypt_share(encrypted_share, &dh_key_bytes[..]);
-        if share.is_err() {
-            return complaint.accused_index;
-        }
-        match share.unwrap().verify(&commitment_accused) {
-            Ok(()) => complaint.maker_index,
+        let share_res = decrypt_share(encrypted_share, &dh_key_bytes[..]);
+
+        match share_res {
             Err(_) => complaint.accused_index,
+            Ok(share) => match share.verify(&commitment_accused) {
+                Ok(()) => complaint.maker_index,
+                Err(_) => complaint.accused_index,
+            },
         }
     }
 }
@@ -2569,11 +2574,11 @@ mod test {
 
             // Check that the generated IndividualVerifyingKey from other participants match
             let p1_recovered_public_key =
-                IndividualVerifyingKey::generate_from_commitments(1, &commitments);
+                IndividualVerifyingKey::generate_from_commitments(1, &commitments)?;
             let p2_recovered_public_key =
-                IndividualVerifyingKey::generate_from_commitments(2, &commitments);
+                IndividualVerifyingKey::generate_from_commitments(2, &commitments)?;
             let p3_recovered_public_key =
-                IndividualVerifyingKey::generate_from_commitments(3, &commitments);
+                IndividualVerifyingKey::generate_from_commitments(3, &commitments)?;
 
             assert_eq!(p1_public_key, p1_recovered_public_key);
             assert_eq!(p2_public_key, p2_recovered_public_key);

src/dkg/participant.rs

@@ -85,6 +85,7 @@ impl<C: CipherSuite> Participant<C> {
     ) -> FrostResult<C, (Self, Coefficients<C>, DiffieHellmanPrivateKey<C>)> {
         let (dealer, coeff_option, dh_private_key) =
             Self::new_internal(parameters, false, index, None, &mut rng)?;
+        // This unwrap() cannot fail, as we always have at least an empty vector of coefficients.
         Ok((dealer, coeff_option.unwrap(), dh_private_key))
     }
 
@@ -249,7 +250,7 @@ impl<C: CipherSuite> Participant<C> {
             &mut rng,
         )?;
 
-        // Unwrapping cannot panic here
+        // This unwrap() cannot fail, as we always have at least an empty vector of coefficients.
         let coefficients = coeff_option.unwrap();
 
         let (participant_state, participant_lists) = DistributedKeyGeneration::new_state_internal(
@@ -263,11 +264,7 @@ impl<C: CipherSuite> Participant<C> {
             &mut rng,
         )?;
 
-        // Unwrapping cannot panic here
-        let encrypted_shares = participant_state
-            .their_encrypted_secret_shares()
-            .unwrap()
-            .clone();
+        let encrypted_shares = participant_state.their_encrypted_secret_shares()?.clone();
 
         Ok((dealer, encrypted_shares, participant_lists))
     }
@@ -276,11 +273,7 @@ impl<C: CipherSuite> Participant<C> {
     ///
     /// This is used to pass into the final call to [`DistributedKeyGeneration::<RoundTwo, C>::finish()`] .
     pub fn public_key(&self) -> Option<&C::G> {
-        if self.commitments.is_some() {
-            return self.commitments.as_ref().unwrap().public_key();
-        }
-
-        None
+        self.commitments.as_ref().map(|c| c.public_key())?
     }
 }
 

src/dkg/secret_share.rs

@@ -233,7 +233,7 @@ pub(crate) fn decrypt_share<C: CipherSuite>(
     let hkdf = Hkdf::<Sha256>::new(None, aes_key);
     let mut final_aes_key = [0u8; 16];
     hkdf.expand(&[], &mut final_aes_key)
-        .expect("KDF expansion failed unexpectedly");
+        .map_err(|_| Error::Custom("KDF expansion failed unexpectedly".to_string()))?;
 
     let final_aes_key = GenericArray::from_slice(&final_aes_key);
 

src/keys.rs

@@ -150,7 +150,7 @@ impl<C: CipherSuite> IndividualVerifyingKey<C> {
     pub fn generate_from_commitments(
         participant_index: u32,
         commitments: &[VerifiableSecretSharingCommitment<C>],
-    ) -> Self {
+    ) -> FrostResult<C, Self> {
         let mut share: C::G = <C as CipherSuite>::G::zero();
         let term: <C::G as Group>::ScalarField = participant_index.into();
 
@@ -169,15 +169,14 @@ impl<C: CipherSuite> IndividualVerifyingKey<C> {
                 }
             }
 
-            let coeff =
-                calculate_lagrange_coefficients::<C>(commitment.index, &index_vector).unwrap();
+            let coeff = calculate_lagrange_coefficients::<C>(commitment.index, &index_vector)?;
             share += tmp * coeff;
         }
 
-        IndividualVerifyingKey {
+        Ok(IndividualVerifyingKey {
             index: participant_index,
             share,
-        }
+        })
     }
 }
 
@@ -240,8 +239,7 @@ impl<C: CipherSuite> GroupVerifyingKey<C> {
         signature: &ThresholdSignature<C>,
         message_hash: &[u8],
     ) -> FrostResult<C, ()> {
-        let challenge =
-            compute_challenge::<C>(&signature.group_commitment, self, message_hash).unwrap();
+        let challenge = compute_challenge::<C>(&signature.group_commitment, self, message_hash)?;
 
         let retrieved_commitment: C::G = <C as CipherSuite>::G::msm(
             &[C::G::generator().into(), (-self.key).into()],

src/lib.rs

@@ -908,11 +908,11 @@
 //! # let carol_public_key = carol_secret_key.to_public();
 //!
 //! let (alice_public_comshares, mut alice_secret_comshares) =
-//!     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &alice_secret_key, 1);
+//!     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &alice_secret_key, 1)?;
 //! let (bob_public_comshares, mut bob_secret_comshares) =
-//!     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &bob_secret_key, 1);
+//!     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &bob_secret_key, 1)?;
 //! let (carol_public_comshares, mut carol_secret_comshares) =
-//!     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &carol_secret_key, 1);
+//!     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &carol_secret_key, 1)?;
 //!
 //! let message = b"This is a test of the tsunami alert system. This is only a test.";
 //!
@@ -987,11 +987,11 @@
 //! # let carol_public_key = carol_secret_key.to_public();
 //! #
 //! # let (alice_public_comshares, mut alice_secret_comshares) =
-//! #     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &alice_secret_key, 1);
+//! #     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &alice_secret_key, 1)?;
 //! # let (bob_public_comshares, mut bob_secret_comshares) =
-//! #     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &bob_secret_key, 1);
+//! #     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &bob_secret_key, 1)?;
 //! # let (carol_public_comshares, mut carol_secret_comshares) =
-//! #     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &carol_secret_key, 1);
+//! #     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &carol_secret_key, 1)?;
 //! #
 //! # let message = b"This is a test of the tsunami alert system. This is only a test.";
 //! #
@@ -1061,11 +1061,11 @@
 //! # let carol_public_key = carol_secret_key.to_public();
 //! #
 //! # let (alice_public_comshares, mut alice_secret_comshares) =
-//! #     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &alice_secret_key, 1);
+//! #     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &alice_secret_key, 1)?;
 //! # let (bob_public_comshares, mut bob_secret_comshares) =
-//! #     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &bob_secret_key, 1);
+//! #     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &bob_secret_key, 1)?;
 //! # let (carol_public_comshares, mut carol_secret_comshares) =
-//! #     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &carol_secret_key, 1);
+//! #     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &carol_secret_key, 1)?;
 //! #
 //! # let message = b"This is a test of the tsunami alert system. This is only a test.";
 //! #
@@ -1135,11 +1135,11 @@
 //! # let carol_public_key = carol_secret_key.to_public();
 //! #
 //! # let (alice_public_comshares, mut alice_secret_comshares) =
-//! #     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &alice_secret_key, 1);
+//! #     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &alice_secret_key, 1)?;
 //! # let (bob_public_comshares, mut bob_secret_comshares) =
-//! #     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &bob_secret_key, 1);
+//! #     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &bob_secret_key, 1)?;
 //! # let (carol_public_comshares, mut carol_secret_comshares) =
-//! #     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &carol_secret_key, 1);
+//! #     generate_commitment_share_lists::<Secp256k1Sha256>(&mut rng, &carol_secret_key, 1)?;
 //! #
 //! # let message = b"This is a test of the tsunami alert system. This is only a test.";
 //! #
@@ -1149,7 +1149,7 @@
 //! # aggregator.include_signer(3, carol_public_comshares.commitments[0], (&carol_secret_key).into());
 //! #
 //! # let signers = aggregator.get_signers();
-//! # let message_hash = Secp256k1Sha256::h4(&message[..]).unwrap();
+//! # let message_hash = Secp256k1Sha256::h4(&message[..])?;
 //!
 //! let alice_partial = alice_secret_key.sign(
 //!     &message_hash,

src/sign/precomputation.rs

@@ -43,11 +43,14 @@ impl<C: CipherSuite> Drop for NoncePair<C> {
 }
 
 impl<C: CipherSuite> NoncePair<C> {
-    pub fn new(secret_key: &IndividualSigningKey<C>, mut csprng: impl CryptoRng + Rng) -> Self {
-        NoncePair(
-            nonce_generate(secret_key, &mut csprng).unwrap(),
-            nonce_generate(secret_key, &mut csprng).unwrap(),
-        )
+    pub fn new(
+        secret_key: &IndividualSigningKey<C>,
+        mut csprng: impl CryptoRng + Rng,
+    ) -> FrostResult<C, Self> {
+        Ok(NoncePair(
+            nonce_generate(secret_key, &mut csprng)?,
+            nonce_generate(secret_key, &mut csprng)?,
+        ))
     }
 }
 
@@ -176,14 +179,14 @@ pub fn generate_commitment_share_lists<C: CipherSuite>(
     mut csprng: impl CryptoRng + Rng,
     participant_secret_key: &IndividualSigningKey<C>,
     number_of_shares: usize,
-) -> (PublicCommitmentShareList<C>, SecretCommitmentShareList<C>) {
+) -> FrostResult<C, (PublicCommitmentShareList<C>, SecretCommitmentShareList<C>)> {
     let mut commitments: Vec<CommitmentShare<C>> = Vec::with_capacity(number_of_shares);
 
     for _ in 0..number_of_shares {
         commitments.push(CommitmentShare::from(NoncePair::new(
             participant_secret_key,
             &mut csprng,
-        )));
+        )?));
     }
 
     let mut published: Vec<(C::G, C::G)> = Vec::with_capacity(number_of_shares);
@@ -192,13 +195,13 @@ pub fn generate_commitment_share_lists<C: CipherSuite>(
         published.push(commitment.publish());
     }
 
-    (
+    Ok((
         PublicCommitmentShareList {
             participant_index: participant_secret_key.index,
             commitments: published,
         },
         SecretCommitmentShareList { commitments },
-    )
+    ))
 }
 
 impl<C: CipherSuite> SecretCommitmentShareList<C> {
@@ -251,7 +254,7 @@ mod test {
             key: Fr::zero(),
         };
         let _commitment_share: CommitmentShare<Secp256k1Sha256> =
-            NoncePair::new(&secret_key, &mut OsRng).into();
+            NoncePair::new(&secret_key, &mut OsRng).unwrap().into();
     }
 
     #[test]
@@ -301,7 +304,7 @@ mod test {
             key: Fr::zero(),
         };
         let (public_share_list, secret_share_list) =
-            generate_commitment_share_lists::<Secp256k1Sha256>(&mut OsRng, &secret_key, 1);
+            generate_commitment_share_lists::<Secp256k1Sha256>(&mut OsRng, &secret_key, 1).unwrap();
 
         assert_eq!(
             public_share_list.commitments[0].0.into_affine(),
@@ -317,7 +320,7 @@ mod test {
             key: Fr::zero(),
         };
         let (_public_share_list, mut secret_share_list) =
-            generate_commitment_share_lists(&mut OsRng, &secret_key, 8);
+            generate_commitment_share_lists(&mut OsRng, &secret_key, 8).unwrap();
 
         assert!(secret_share_list.commitments.len() == 8);