chore(crypto): CRP-1317 Removed unnecessary clone from multisig library

Author: andreacerulli

rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/api.rs

@@ -1,30 +1,33 @@
 //! External API for the multisignature library
 use super::crypto;
 use super::types::{
-    CombinedSignatureBytes, IndividualSignature, IndividualSignatureBytes, Pop, PopBytes,
-    PublicKey, PublicKeyBytes, SecretKeyBytes,
+    CombinedSignature, CombinedSignatureBytes, IndividualSignature, IndividualSignatureBytes, Pop,
+    PopBytes, PublicKey, PublicKeyBytes, SecretKey, SecretKeyBytes,
 };
 use ic_types::crypto::{AlgorithmId, CryptoError};
 use rand::{CryptoRng, Rng};
 use std::convert::TryFrom;
-use std::convert::TryInto;
 
 #[cfg(test)]
 mod tests;
 
 /// Generates a keypair using the given `rng`.
 pub fn keypair_from_rng<R: Rng + CryptoRng>(rng: &mut R) -> (SecretKeyBytes, PublicKeyBytes) {
     let (secret_key, public_key) = crypto::keypair_from_rng(rng);
-    (secret_key.into(), public_key.into())
+    (
+        SecretKeyBytes::from(&secret_key),
+        PublicKeyBytes::from(&public_key),
+    )
 }
 
 /// Generates a multisignature on the given `message` using the given
 /// `secret_key`.
 ///
 /// Note: This hashes the message to be signed.  If we pre-hash, the hashing
 /// can be skipped. https://docs.rs/threshold_crypto/0.3.2/threshold_crypto/struct.SecretKey.html#method.sign
-pub fn sign(message: &[u8], secret_key: SecretKeyBytes) -> IndividualSignatureBytes {
-    crypto::sign_message(message, &secret_key.into()).into()
+pub fn sign(message: &[u8], secret_key: &SecretKeyBytes) -> IndividualSignatureBytes {
+    let signature = crypto::sign_message(message, &SecretKey::from(secret_key));
+    IndividualSignatureBytes::from(&signature)
 }
 
 /// Creates a proof of possession (PoP) of `secret_key_bytes`.
@@ -35,11 +38,12 @@ pub fn sign(message: &[u8], secret_key: SecretKeyBytes) -> IndividualSignatureBy
 /// * `CryptoError::MalformedPublicKey` if `public_key_bytes` cannot be parsed
 ///   as a valid G2 point.
 pub fn create_pop(
-    public_key_bytes: PublicKeyBytes,
-    secret_key_bytes: SecretKeyBytes,
+    public_key_bytes: &PublicKeyBytes,
+    secret_key_bytes: &SecretKeyBytes,
 ) -> Result<PopBytes, CryptoError> {
-    let public_key = public_key_bytes.try_into()?;
-    Ok(crypto::create_pop(&public_key, &secret_key_bytes.into()).into())
+    let public_key = PublicKey::try_from(public_key_bytes)?;
+    let pop = crypto::create_pop(&public_key, &SecretKey::from(secret_key_bytes));
+    Ok(PopBytes::from(&pop))
 }
 
 /// Verifies a public key's proof of possession (PoP).
@@ -54,8 +58,8 @@ pub fn create_pop(
 ///   as a valid G2 point.
 /// * `CryptoError::PopVerification` if the given PoP fails to verify.
 pub fn verify_pop(
-    pop_bytes: PopBytes,
-    public_key_bytes: PublicKeyBytes,
+    pop_bytes: &PopBytes,
+    public_key_bytes: &PublicKeyBytes,
 ) -> Result<(), CryptoError> {
     let pop = Pop::try_from(pop_bytes)?;
     let public_key = PublicKey::try_from(public_key_bytes)?;
@@ -81,11 +85,10 @@ pub fn combine(
 ) -> Result<CombinedSignatureBytes, CryptoError> {
     let signatures: Result<Vec<IndividualSignature>, CryptoError> = signatures
         .iter()
-        .cloned()
-        .map(|signature_bytes| signature_bytes.try_into())
+        .map(|signature_bytes| IndividualSignature::try_from(signature_bytes))
         .collect();
     let signature = crypto::combine_signatures(&signatures?);
-    Ok(signature.into())
+    Ok(CombinedSignatureBytes::from(&signature))
 }
 
 /// Verifies an individual signature over the given `message` using the given
@@ -100,11 +103,11 @@ pub fn combine(
 ///   fails.
 pub fn verify_individual(
     message: &[u8],
-    signature_bytes: IndividualSignatureBytes,
-    public_key_bytes: PublicKeyBytes,
+    signature_bytes: &IndividualSignatureBytes,
+    public_key_bytes: &PublicKeyBytes,
 ) -> Result<(), CryptoError> {
-    let signature = signature_bytes.try_into()?;
-    let public_key = public_key_bytes.try_into()?;
+    let signature = IndividualSignature::try_from(signature_bytes)?;
+    let public_key = PublicKey::try_from(public_key_bytes)?;
     if crypto::verify_individual_message_signature(message, &signature, &public_key) {
         Ok(())
     } else {
@@ -130,13 +133,16 @@ pub fn verify_individual(
 ///   fails.
 pub fn verify_combined(
     message: &[u8],
-    signature: CombinedSignatureBytes,
+    signature: &CombinedSignatureBytes,
     public_keys: &[PublicKeyBytes],
 ) -> Result<(), CryptoError> {
     let public_keys: Result<Vec<PublicKey>, CryptoError> =
-        public_keys.iter().cloned().map(|x| x.try_into()).collect();
-    if crypto::verify_combined_message_signature(message, &signature.try_into()?, &public_keys?[..])
-    {
+        public_keys.iter().map(|x| PublicKey::try_from(x)).collect();
+    if crypto::verify_combined_message_signature(
+        message,
+        &CombinedSignature::try_from(signature)?,
+        &public_keys?[..],
+    ) {
         Ok(())
     } else {
         Err(CryptoError::SignatureVerification {

rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/api/tests.rs

@@ -36,11 +36,11 @@ fn test_happy_path(
 ) {
     let pops: CryptoResult<Vec<PopBytes>> = keys
         .iter()
-        .map(|(secret_key, public_key)| multi_sig::create_pop(*public_key, secret_key.clone()))
+        .map(|(secret_key, public_key)| multi_sig::create_pop(public_key, secret_key))
         .collect();
     let signatures: Vec<IndividualSignatureBytes> = keys
         .iter()
-        .map(|(secret_key, _)| multi_sig::sign(message, secret_key.clone()))
+        .map(|(secret_key, _)| multi_sig::sign(message, secret_key))
         .collect();
     let pops = pops.expect("PoP generation failed");
     let signature = multi_sig::combine(&signatures);
@@ -53,20 +53,20 @@ fn test_happy_path(
     let pop_verification: CryptoResult<()> = public_keys
         .iter()
         .zip(pops)
-        .try_for_each(|(public_key, pop)| multi_sig::verify_pop(pop, *public_key));
+        .try_for_each(|(public_key, pop)| multi_sig::verify_pop(&pop, public_key));
     let individual_verification: CryptoResult<()> = public_keys
         .iter()
         .zip(signatures.clone())
         .try_for_each(|(public_key, signature)| {
-            multi_sig::verify_individual(message, signature, *public_key)
+            multi_sig::verify_individual(message, &signature, public_key)
         });
     assert!(pop_verification.is_ok(), "PoP verification failed");
     assert!(
         individual_verification.is_ok(),
         "Individual signature verification failed"
     );
     assert!(
-        multi_sig::verify_combined(message, signature, &public_keys).is_ok(),
+        multi_sig::verify_combined(message, &signature, &public_keys).is_ok(),
         "Signature verification failed"
     );
     (signatures, signature, public_keys)
@@ -94,9 +94,9 @@ proptest! {
       evil_signature in arbitrary::individual_signature_bytes()
     ) {
         let (secret_key, public_key) = keys;
-        let signature = multi_sig::sign(&message, secret_key);
+        let signature = multi_sig::sign(&message, &secret_key);
         prop_assume!(evil_signature != signature);
-        assert!(multi_sig::verify_individual(&message, evil_signature, public_key).is_err())
+        assert!(multi_sig::verify_individual(&message, &evil_signature, &public_key).is_err())
     }
 
     #[test]
@@ -105,9 +105,9 @@ proptest! {
       evil_pop in arbitrary::pop_bytes()
     ) {
         let (secret_key, public_key) = keys;
-        let pop = multi_sig::create_pop(public_key, secret_key).expect("Failed to create PoP");
+        let pop = multi_sig::create_pop(&public_key, &secret_key).expect("Failed to create PoP");
         prop_assume!(evil_pop != pop);
-        assert!(multi_sig::verify_pop(evil_pop, public_key).is_err())
+        assert!(multi_sig::verify_pop(&evil_pop, &public_key).is_err())
     }
 
     #[test]
@@ -118,6 +118,6 @@ proptest! {
     ) {
         let (_signatures, signature, public_keys) = test_happy_path(&keys, &message);
         prop_assume!(evil_signature != signature);
-        assert!(multi_sig::verify_combined(&message, evil_signature, &public_keys).is_err())
+        assert!(multi_sig::verify_combined(&message, &evil_signature, &public_keys).is_err())
     }
 }

rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/crypto.rs

@@ -68,7 +68,7 @@ pub fn sign_message(message: &[u8], secret_key: &SecretKey) -> IndividualSignatu
 }
 
 pub fn create_pop(public_key: &PublicKey, secret_key: &SecretKey) -> Pop {
-    let public_key_bytes = PublicKeyBytes::from(public_key.clone());
+    let public_key_bytes = PublicKeyBytes::from(public_key);
     let mut domain_separated_public_key: Vec<u8> = vec![];
     domain_separated_public_key
         .extend(DomainSeparationContext::new(DOMAIN_MULTI_SIG_BLS12_381_POP).as_bytes());
@@ -98,7 +98,7 @@ pub fn verify_individual_message_signature(
     verify_point(&hash, signature, public_key)
 }
 pub fn verify_pop(pop: &Pop, public_key: &PublicKey) -> bool {
-    let public_key_bytes = PublicKeyBytes::from(public_key.clone());
+    let public_key_bytes = PublicKeyBytes::from(public_key);
     let mut domain_separated_public_key: Vec<u8> = vec![];
     domain_separated_public_key
         .extend(DomainSeparationContext::new(DOMAIN_MULTI_SIG_BLS12_381_POP).as_bytes());

rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/tests.rs

@@ -64,7 +64,7 @@ mod stability {
     fn public_key_to_g1() {
         let mut csprng = ChaCha20Rng::seed_from_u64(42);
         let (_secret_key, public_key) = multi_crypto::keypair_from_rng(&mut csprng);
-        let public_key_bytes = PublicKeyBytes::from(public_key);
+        let public_key_bytes = PublicKeyBytes::from(&public_key);
         assert_eq!(
             hex::encode(multi_crypto::hash_public_key_to_g1(&public_key_bytes.0[..]).serialize()),
             "b02fd0d54faab7498924d7e230f84b00519ea7f3846cd30f82b149c1f172ad79ee68adb2ea2fc8a2d40ffdf3fd5df02a"
@@ -75,14 +75,14 @@ mod stability {
     fn secret_key_from_fixed_rng() {
         let mut csprng = ChaCha20Rng::seed_from_u64(9000);
         let (secret_key, public_key) = multi_crypto::keypair_from_rng(&mut csprng);
-        let secret_key_bytes = SecretKeyBytes::from(secret_key);
+        let secret_key_bytes = SecretKeyBytes::from(&secret_key);
 
         assert_eq!(
             hex::encode(serde_cbor::to_vec(&secret_key_bytes).unwrap()),
             "582020bfd7f85be7ce1f54ea1b0d750ae3324ab7897fde3235e189ec697f0fade983"
         );
 
-        let public_key_bytes = PublicKeyBytes::from(public_key);
+        let public_key_bytes = PublicKeyBytes::from(&public_key);
 
         assert_eq!(
             hex::encode(serde_cbor::to_vec(&public_key_bytes).unwrap()),
@@ -144,7 +144,7 @@ mod basic_functionality {
         let points: HashSet<_> = (0..number_of_public_keys as u64)
             .map(|_| {
                 let (_secret_key, public_key) = multi_crypto::keypair_from_rng(&mut csprng);
-                let public_key_bytes = PublicKeyBytes::from(public_key);
+                let public_key_bytes = PublicKeyBytes::from(&public_key);
                 let g1 = multi_crypto::hash_public_key_to_g1(&public_key_bytes.0[..]);
                 let bytes = g1.serialize();
                 // It suffices to prove that the first 32 bytes are distinct.  More requires a
@@ -195,10 +195,10 @@ mod advanced_functionality {
     fn verify_pop_throws_error_on_public_key_bytes_with_unset_compressed_flag() {
         let (secret_key, public_key) = multi_crypto::keypair_from_seed([1, 2, 3, 4]);
         let pop = multi_crypto::create_pop(&public_key, &secret_key);
-        let pop_bytes = PopBytes::from(pop);
-        let mut public_key_bytes = PublicKeyBytes::from(public_key);
+        let pop_bytes = PopBytes::from(&pop);
+        let mut public_key_bytes = PublicKeyBytes::from(&public_key);
         public_key_bytes.0[G2Bytes::FLAG_BYTE_OFFSET] &= !G2Bytes::COMPRESSED_FLAG;
-        match api::verify_pop(pop_bytes, public_key_bytes) {
+        match api::verify_pop(&pop_bytes, &public_key_bytes) {
             Err(e) => assert!(e.to_string().contains("Point decoding failed")),
             Ok(_) => panic!("error should have been thrown"),
         }
@@ -208,16 +208,16 @@ mod advanced_functionality {
     fn verify_pop_throws_error_on_public_key_bytes_not_on_curve() {
         let (secret_key, public_key) = multi_crypto::keypair_from_seed([1, 2, 3, 4]);
         let pop = multi_crypto::create_pop(&public_key, &secret_key);
-        let pop_bytes = PopBytes::from(pop);
-        let mut public_key_bytes = PublicKeyBytes::from(public_key);
+        let pop_bytes = PopBytes::from(&pop);
+        let mut public_key_bytes = PublicKeyBytes::from(&public_key);
         // Zero out the bytes, set the compression flag.
         // This represents x = 0, which happens to have no solution
         // on the G2 curve.
         for i in 0..G2Bytes::SIZE {
             public_key_bytes.0[i] = 0;
         }
         public_key_bytes.0[G2Bytes::FLAG_BYTE_OFFSET] |= G2Bytes::COMPRESSED_FLAG;
-        match api::verify_pop(pop_bytes, public_key_bytes) {
+        match api::verify_pop(&pop_bytes, &public_key_bytes) {
             Err(e) => assert!(e.to_string().contains("Point decoding failed")),
             Ok(_) => panic!("error should have been thrown"),
         }
@@ -227,16 +227,16 @@ mod advanced_functionality {
     fn verify_pop_throws_error_on_public_key_bytes_not_in_subgroup() {
         let (secret_key, public_key) = multi_crypto::keypair_from_seed([1, 2, 3, 4]);
         let pop = multi_crypto::create_pop(&public_key, &secret_key);
-        let pop_bytes = PopBytes::from(pop);
-        let mut public_key_bytes = PublicKeyBytes::from(public_key);
+        let pop_bytes = PopBytes::from(&pop);
+        let mut public_key_bytes = PublicKeyBytes::from(&public_key);
         // By manual rejection sampling, we found an x-coordinate with a
         // solution, which is unlikely to have order r.
         for i in 0..G2Bytes::SIZE {
             public_key_bytes.0[i] = 0;
         }
         public_key_bytes.0[G2Bytes::FLAG_BYTE_OFFSET] |= G2Bytes::COMPRESSED_FLAG;
         public_key_bytes.0[5] = 3;
-        match api::verify_pop(pop_bytes, public_key_bytes) {
+        match api::verify_pop(&pop_bytes, &public_key_bytes) {
             Err(e) => assert!(e.to_string().contains("Point decoding failed")),
             Ok(_) => panic!("error should have been thrown"),
         }

rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types/arbitrary.rs

@@ -37,22 +37,27 @@ pub fn combined_signature() -> impl Strategy<Value = CombinedSignature> {
 }
 
 pub fn secret_key_bytes() -> impl Strategy<Value = SecretKeyBytes> {
-    key_pair().prop_map(|(secret_key, _public_key)| secret_key.into())
+    key_pair().prop_map(|(secret_key, _public_key)| SecretKeyBytes::from(&secret_key))
 }
 pub fn public_key_bytes() -> impl Strategy<Value = PublicKeyBytes> {
-    public_key().prop_map(|public_key| public_key.into())
+    public_key().prop_map(|public_key| PublicKeyBytes::from(&public_key))
 }
 pub fn key_pair_bytes() -> impl Strategy<Value = (SecretKeyBytes, PublicKeyBytes)> {
-    key_pair().prop_map(|(secret_key, public_key)| (secret_key.into(), public_key.into()))
+    key_pair().prop_map(|(secret_key, public_key)| {
+        (
+            SecretKeyBytes::from(&secret_key),
+            PublicKeyBytes::from(&public_key),
+        )
+    })
 }
 pub fn individual_signature_bytes() -> impl Strategy<Value = IndividualSignatureBytes> {
-    individual_signature().prop_map(|signature| signature.into())
+    individual_signature().prop_map(|signature| IndividualSignatureBytes::from(&signature))
 }
 pub fn pop_bytes() -> impl Strategy<Value = PopBytes> {
-    pop().prop_map(|pop| pop.into())
+    pop().prop_map(|pop| PopBytes::from(&pop))
 }
 pub fn combined_signature_bytes() -> impl Strategy<Value = CombinedSignatureBytes> {
-    combined_signature().prop_map(|signature| signature.into())
+    combined_signature().prop_map(|signature| CombinedSignatureBytes::from(&signature))
 }
 
 //////////////////