test(crypto): CRP-587 Add various missing NI-DKG verification tests (#8406)

Author: randombit

rs/crypto/src/sign/threshold_sig.rs

@@ -149,14 +149,12 @@ impl ThresholdSigVerifierInternal {
             signer,
         )?;
 
-        threshold_sig_csp_client
-            .threshold_verify_individual_signature(
-                AlgorithmId::from(public_key),
-                message.as_signed_bytes().as_slice(),
-                csp_signature,
-                public_key,
-            )
-            .map_err(panic_on_illegal_individual_sig_verification_state)
+        threshold_sig_csp_client.threshold_verify_individual_signature(
+            AlgorithmId::from(public_key),
+            message.as_signed_bytes().as_slice(),
+            csp_signature,
+            public_key,
+        )
     }
 }
 
@@ -254,24 +252,6 @@ fn coeffs_and_index(
     Ok((public_coeffs, node_index))
 }
 
-fn panic_on_illegal_individual_sig_verification_state(error: CryptoError) -> CryptoError {
-    match error {
-        CryptoError::InvalidArgument { .. } | CryptoError::MalformedPublicKey { .. } => panic!(
-            "Illegal state: the algorithm of the public key from the threshold signature data \
-            store (which is based on the algorithm of the public coefficients in the store) is \
-            not supported: {error}"
-        ),
-        CryptoError::MalformedSignature { .. } => unreachable!(
-            "This case cannot occur because `CryptoError::MalformedSignature` is returned only \
-            if the given signature was not a \
-            `CspSignature::ThresBls12_381(ThresBls12_381_Signature::Individual)`, but we know \
-            for sure that it has this type because this is the type returned by \
-            `CspSignature::try_from(ThresholdSigShareOf)`."
-        ),
-        _ => error,
-    }
-}
-
 impl ThresholdSigVerifierInternal {
     pub fn combine_threshold_sig_shares<C: ThresholdSignatureCspClient, H: Signable>(
         lockable_threshold_sig_data_store: &LockableThresholdSigDataStore,
@@ -290,7 +270,7 @@ impl ThresholdSigVerifierInternal {
                 public_coefficients,
             )
             .map_err(map_csp_combine_sigs_error)?;
-        combined_threshold_sig_or_panic(csp_signature)
+        CombinedThresholdSigOf::try_from(csp_signature)
     }
 }
 
@@ -345,15 +325,6 @@ fn index_for_node_id(
         .ok_or_else(|| node_id_missing_error(node_id, dkg_id))
 }
 
-fn combined_threshold_sig_or_panic<H: Signable>(
-    csp_signature: CspSignature,
-) -> CryptoResult<CombinedThresholdSigOf<H>> {
-    Ok(CombinedThresholdSigOf::try_from(csp_signature).expect(
-        "The CSP must return a signature of type \
-        `CspSignature::ThresBls12_381(ThresBls12_381_Signature::Combined)`.",
-    ))
-}
-
 fn map_csp_combine_sigs_error(error: CryptoError) -> CryptoError {
     match error {
         CryptoError::MalformedSignature { .. } | CryptoError::InvalidArgument { .. } => error,

rs/crypto/src/sign/threshold_sig/tests.rs

@@ -2,6 +2,7 @@ use super::*;
 use crate::sign::tests::KEY_ID;
 use crate::sign::tests::KEY_ID_STRING;
 use crate::sign::threshold_sig::ThresholdSigDataStore;
+use assert_matches::assert_matches;
 use ic_crypto_internal_csp::types::{CspPublicCoefficients, ThresBls12_381_Signature};
 use ic_crypto_internal_csp_test_utils::types::{
     csp_sig_thres_bls12381_combined_from_array_of, csp_sig_thres_bls12381_indiv_from_array_of,
@@ -35,7 +36,6 @@ pub const NI_DKG_ID_1: NiDkgId = NiDkgId {
 
 mod sign_threshold {
     use super::*;
-    use assert_matches::assert_matches;
     use ic_crypto_internal_csp::key_id::KeyId;
 
     #[test]
@@ -570,65 +570,60 @@ mod verify_threshold_sig_share {
     }
 
     #[test]
-    #[should_panic(
-        expected = "Illegal state: the algorithm of the public key from the threshold \
-            signature data store (which is based on the algorithm of the public coefficients in \
-            the store) is not supported"
-    )]
-    fn should_panic_if_csp_returns_invalid_argument_error() {
+    fn should_forward_error_if_csp_returns_invalid_argument_error() {
         let (sig_share, message, csp_public_key) = (sig_share(), signable_mock(), csp_public_key());
         let threshold_sig_data_store =
             threshold_sig_data_store_with_coeffs_and_pubkey(&NI_DKG_ID_1, NODE_ID, csp_public_key);
         let csp = csp_with_verify_indiv_sig_returning_once(Err(invalid_argument()));
 
-        let _panic = ThresholdSigVerifierInternal::verify_threshold_sig_share(
+        let result = ThresholdSigVerifierInternal::verify_threshold_sig_share(
             &threshold_sig_data_store,
             &csp,
             &sig_share,
             &message,
             &NI_DKG_ID_1,
             NODE_ID,
         );
+
+        assert_matches!(result, Err(CryptoError::InvalidArgument { .. }));
     }
 
     #[test]
-    #[should_panic(
-        expected = "Illegal state: the algorithm of the public key from the threshold signature data \
-            store (which is based on the algorithm of the public coefficients in the store) is \
-            not supported"
-    )]
-    fn should_panic_if_csp_returns_malformed_public_key_error() {
+    fn should_forward_error_if_csp_returns_malformed_public_key_error() {
         let (sig_share, message, csp_public_key) = (sig_share(), signable_mock(), csp_public_key());
         let threshold_sig_data_store =
             threshold_sig_data_store_with_coeffs_and_pubkey(&NI_DKG_ID_1, NODE_ID, csp_public_key);
         let csp = csp_with_verify_indiv_sig_returning_once(Err(malformed_public_key()));
 
-        let _panic = ThresholdSigVerifierInternal::verify_threshold_sig_share(
+        let result = ThresholdSigVerifierInternal::verify_threshold_sig_share(
             &threshold_sig_data_store,
             &csp,
             &sig_share,
             &message,
             &NI_DKG_ID_1,
             NODE_ID,
         );
+
+        assert_eq!(result, Err(malformed_public_key()));
     }
 
     #[test]
-    #[should_panic(expected = "This case cannot occur")]
-    fn should_panic_if_csp_returns_malformed_signature_error() {
+    fn should_forward_error_if_csp_returns_malformed_signature_error() {
         let (sig_share, message, csp_public_key) = (sig_share(), signable_mock(), csp_public_key());
         let threshold_sig_data_store =
             threshold_sig_data_store_with_coeffs_and_pubkey(&NI_DKG_ID_1, NODE_ID, csp_public_key);
         let csp = csp_with_verify_indiv_sig_returning_once(Err(malformed_signature()));
 
-        let _panic = ThresholdSigVerifierInternal::verify_threshold_sig_share(
+        let result = ThresholdSigVerifierInternal::verify_threshold_sig_share(
             &threshold_sig_data_store,
             &csp,
             &sig_share,
             &message,
             &NI_DKG_ID_1,
             NODE_ID,
         );
+
+        assert_eq!(result, Err(malformed_signature()));
     }
 
     fn csp_with_indiv_pk_returning_once(
@@ -853,9 +848,7 @@ mod combine_threshold_sig_shares {
     }
 
     #[test]
-    #[should_panic(expected = "The CSP must return a signature of type \
-        `CspSignature::ThresBls12_381(ThresBls12_381_Signature::Combined)`.")]
-    fn should_panic_if_csp_returns_wrong_signature_type() {
+    fn should_return_malformed_signature_error_if_csp_returns_wrong_signature_type() {
         let shares = shares(vec![(
             NODE_1,
             threshold_sig_share(vec![1; IndividualSignatureBytes::SIZE]),
@@ -867,12 +860,14 @@ mod combine_threshold_sig_shares {
         let threshold_sig_data_store =
             threshold_sig_data_store_with(&NI_DKG_ID_1, pub_coeffs(), indices);
 
-        let _panic = ThresholdSigVerifierInternal::combine_threshold_sig_shares(
+        let result = ThresholdSigVerifierInternal::combine_threshold_sig_shares(
             &threshold_sig_data_store,
             &csp,
             shares,
             &NI_DKG_ID_1,
         );
+
+        assert_matches!(result, Err(CryptoError::MalformedSignature { .. }));
     }
 
     #[test]