Merge branch 'mathias-CRP-2298-cspnidkgtranscript-to-cspthresholdsigp…

…ublickey' into 'master'

chore(crypto): CRP-2298: Change NiDkgTranscript to ThresholdSigPublicKey conversions to TryFrom and remove internal panics

Change a number of conversions from different `NiDkgTranscript` types to `ThresholdSigPublicKey` types for BLS 12 381 from `From` to `TryFrom` and remove internal panics. 

See merge request dfinity-lab/public/ic!16426

rs/consensus/src/consensus/batch_delivery.rs

@@ -24,6 +24,7 @@ use ic_protobuf::{
     log::consensus_log_entry::v1::ConsensusLogEntry,
     registry::{crypto::v1::PublicKey as PublicKeyProto, subnet::v1::InitialNiDkgTranscriptRecord},
 };
+use ic_types::crypto::threshold_sig::ThresholdSigPublicKey;
 use ic_types::{
     batch::{Batch, BatchMessages, BlockmakerMetrics},
     consensus::{
@@ -362,7 +363,8 @@ fn generate_dkg_response_payload(
                 InitialNiDkgTranscriptRecord::from(high_threshold_transcript.clone());
 
             // This is what we expect consensus to reply with.
-            let threshold_sig_pk = high_threshold_transcript.public_key();
+            let threshold_sig_pk = ThresholdSigPublicKey::try_from(high_threshold_transcript)
+                .expect("should extract public key from high threshold transcript");
             let subnet_threshold_public_key = PublicKeyProto::from(threshold_sig_pk);
             let key_der: Vec<u8> =
                 ic_crypto_utils_threshold_sig_der::threshold_sig_public_key_to_der(

rs/crypto/internal/crypto_lib/types/src/sign/threshold_sig/public_key.rs

@@ -5,6 +5,7 @@ use crate::sign::threshold_sig::public_key::bls12_381::{
 };
 use serde::{Deserialize, Serialize};
 use std::hash::Hash;
+use thiserror::Error;
 
 /// A threshold signature public key.
 #[derive(Copy, Clone, Debug, Eq, Hash, PartialOrd, Ord, PartialEq, Serialize, Deserialize)]
@@ -20,23 +21,34 @@ impl From<CspThresholdSigPublicKey> for bls12_381::PublicKeyBytes {
     }
 }
 
-impl From<&CspNiDkgTranscript> for CspThresholdSigPublicKey {
-    // The conversion is deliberately implemented directly, i.e., without
-    // using From-conversions involving intermediate types, because the
-    // panic on empty coefficients is currently specific to converting
-    // the transcript to the contained threshold signature public key.
-    fn from(csp_ni_dkg_transcript: &CspNiDkgTranscript) -> Self {
-        match PublicKeyBytes::try_from(csp_ni_dkg_transcript) {
-            Ok(public_key_bytes) => Self::ThresBls12_381(public_key_bytes),
-            Err(err) => match err {
-                CspNiDkgTranscriptThresholdSigPublicKeyBytesConversionError::CoefficientsEmpty => {
-                    panic!("coefficients empty")
-                }
-            },
+/// Converting an NI-DKG transcript to a BLS 12 381 CspThresholdSigPublicKey struct failed.
+#[derive(Clone, Debug, PartialEq, Eq, Hash, Error)]
+pub enum CspNiDkgTranscriptToCspThresholdSigPublicKeyConversionError {
+    #[error("the public coefficients of the threshold public key are empty")]
+    CoefficientsEmpty,
+}
+
+impl From<CspNiDkgTranscriptThresholdSigPublicKeyBytesConversionError>
+    for CspNiDkgTranscriptToCspThresholdSigPublicKeyConversionError
+{
+    fn from(err: CspNiDkgTranscriptThresholdSigPublicKeyBytesConversionError) -> Self {
+        match err {
+            CspNiDkgTranscriptThresholdSigPublicKeyBytesConversionError::CoefficientsEmpty => {
+                CspNiDkgTranscriptToCspThresholdSigPublicKeyConversionError::CoefficientsEmpty
+            }
         }
     }
 }
 
+impl TryFrom<&CspNiDkgTranscript> for CspThresholdSigPublicKey {
+    type Error = CspNiDkgTranscriptToCspThresholdSigPublicKeyConversionError;
+
+    fn try_from(csp_ni_dkg_transcript: &CspNiDkgTranscript) -> Result<Self, Self::Error> {
+        let public_key_bytes = PublicKeyBytes::try_from(csp_ni_dkg_transcript)?;
+        Ok(Self::ThresBls12_381(public_key_bytes))
+    }
+}
+
 impl From<bls12_381::PublicKeyBytes> for CspThresholdSigPublicKey {
     fn from(public_key_bytes: bls12_381::PublicKeyBytes) -> Self {
         CspThresholdSigPublicKey::ThresBls12_381(public_key_bytes)

rs/crypto/test_utils/ni-dkg/src/initial_config/tests.rs

@@ -12,6 +12,7 @@ use ic_registry_keys::make_catch_up_package_contents_key;
 use ic_registry_proto_data_provider::ProtoRegistryDataProvider;
 use ic_types::crypto::threshold_sig::ni_dkg::config::receivers::NiDkgReceivers;
 use ic_types::crypto::threshold_sig::ni_dkg::config::NiDkgThreshold;
+use ic_types::crypto::threshold_sig::ThresholdSigPublicKey;
 use ic_types::{Height, NumberOfNodes, RegistryVersion, SubnetId};
 use ic_types_test_utils::ids::{node_test_id, NODE_1, SUBNET_1};
 use rand::Rng;
@@ -207,7 +208,8 @@ fn should_get_master_key_associated_with_transcript_public_key() {
 
     let (transcript, secret) = initial_dkg_transcript_and_master_key(config, &receiver_keys, rng);
 
-    let pk = transcript.public_key();
+    let pk = ThresholdSigPublicKey::try_from(&transcript)
+        .expect("should extract public key from high threshold transcript");
 
     let test_message = rng.gen::<[u8; 32]>();
 

rs/messaging/src/message_routing.rs

@@ -29,6 +29,7 @@ use ic_registry_provisional_whitelist::ProvisionalWhitelist;
 use ic_registry_subnet_features::SubnetFeatures;
 use ic_registry_subnet_type::SubnetType;
 use ic_replicated_state::{NetworkTopology, ReplicatedState, SubnetTopology};
+use ic_types::crypto::threshold_sig::ThresholdSigPublicKey;
 use ic_types::{
     batch::Batch,
     crypto::KeyPurpose,
@@ -752,15 +753,21 @@ impl BatchProcessorImpl {
                 .map_err(|err| registry_error("public key in transcript", Some(*subnet_id), err))?
                 .value
                 .map(|transcripts| {
-                    ic_crypto_utils_threshold_sig_der::public_key_to_der(
-                        &transcripts.high_threshold.public_key().into_bytes(),
-                    )
-                    .map_err(|err: String| {
-                        Persistent(format!(
-                            "'public key to DER for subnet {}' failed, err: {}",
+                    match ThresholdSigPublicKey::try_from(&transcripts.high_threshold) {
+                        Ok(public_key) => ic_crypto_utils_threshold_sig_der::public_key_to_der(
+                            &public_key.into_bytes(),
+                        )
+                        .map_err(|err: String| {
+                            Persistent(format!(
+                                "'public key to DER for subnet {}' failed, err: {}",
+                                *subnet_id, err
+                            ))
+                        }),
+                        Err(err) => Err(Persistent(format!(
+                            "'public key from transcript for subnet {}' failed, err: {:?}",
                             *subnet_id, err
-                        ))
-                    })
+                        ))),
+                    }
                 })
                 .transpose()?
                 .ok_or_else(|| not_found_error("public key in transcript", Some(*subnet_id)))?;

rs/messaging/src/message_routing/tests.rs

@@ -666,7 +666,9 @@ fn try_read_registry_succeeds_with_fully_specified_registry_records() {
             let subnet_topology = network_topology.subnets.get(&subnet_id).unwrap();
             assert_eq!(
                 ic_crypto_utils_threshold_sig_der::public_key_to_der(
-                    &transcript.public_key().into_bytes()
+                    &ThresholdSigPublicKey::try_from(transcript)
+                        .expect("should extract public key from high threshold transcript")
+                        .into_bytes()
                 )
                 .unwrap(),
                 subnet_topology.public_key,

rs/prep/src/subnet_configuration.rs

@@ -21,6 +21,7 @@ use ic_protobuf::registry::{
 };
 use ic_registry_subnet_features::SubnetFeatures;
 use ic_registry_subnet_type::SubnetType;
+use ic_types::crypto::threshold_sig::ni_dkg::ThresholdSigPublicKeyError;
 use ic_types::{
     crypto::{
         threshold_sig::{
@@ -119,6 +120,12 @@ pub enum InitializeSubnetError {
         source: ThresholdSigPublicKeyBytesConversionError,
     },
 
+    #[error("NI-DKG transcript threshold signature public key: {source}")]
+    NiDkgTranscriptToThresholdSigPublicKey {
+        #[from]
+        source: ThresholdSigPublicKeyError,
+    },
+
     #[error("crypto error: {source}")]
     Crypto {
         #[from]
@@ -338,9 +345,9 @@ impl SubnetConfig {
             &dkg_dealing_encryption_pubkeys,
             &mut rand::rngs::OsRng,
         );
-        let subnet_threshold_signing_public_key = PublicKey::from(ThresholdSigPublicKey::from(
+        let subnet_threshold_signing_public_key = PublicKey::from(ThresholdSigPublicKey::try_from(
             &ni_dkg_transcript_high_threshold,
-        ));
+        )?);
 
         let subnet_dkg = CatchUpPackageContents {
             initial_ni_dkg_transcript_low_threshold: Some(InitialNiDkgTranscriptRecord::from(

rs/types/types/src/crypto/threshold_sig.rs

@@ -1,10 +1,12 @@
 //! Defines threshold signature types.
-use crate::crypto::threshold_sig::ni_dkg::NiDkgTranscript;
+use crate::crypto::threshold_sig::ni_dkg::{NiDkgTranscript, ThresholdSigPublicKeyError};
 use crate::crypto::AlgorithmId;
 use ic_crypto_internal_types::sign::threshold_sig::ni_dkg::CspNiDkgTranscript;
-use ic_crypto_internal_types::sign::threshold_sig::public_key::bls12_381;
 pub use ic_crypto_internal_types::sign::threshold_sig::public_key::bls12_381::ThresholdSigPublicKeyBytesConversionError;
 use ic_crypto_internal_types::sign::threshold_sig::public_key::CspThresholdSigPublicKey;
+use ic_crypto_internal_types::sign::threshold_sig::public_key::{
+    bls12_381, CspNiDkgTranscriptToCspThresholdSigPublicKeyConversionError,
+};
 use ic_protobuf::registry::crypto::v1::AlgorithmId as AlgorithmIdProto;
 use ic_protobuf::registry::crypto::v1::PublicKey as PublicKeyProto;
 use serde::{Deserialize, Serialize};
@@ -42,11 +44,25 @@ impl From<ThresholdSigPublicKey> for CspThresholdSigPublicKey {
     }
 }
 
-impl From<&NiDkgTranscript> for ThresholdSigPublicKey {
-    fn from(ni_dkg_transcript: &NiDkgTranscript) -> Self {
+impl From<CspNiDkgTranscriptToCspThresholdSigPublicKeyConversionError>
+    for ThresholdSigPublicKeyError
+{
+    fn from(err: CspNiDkgTranscriptToCspThresholdSigPublicKeyConversionError) -> Self {
+        match err {
+            CspNiDkgTranscriptToCspThresholdSigPublicKeyConversionError::CoefficientsEmpty => {
+                ThresholdSigPublicKeyError::CoefficientsEmpty
+            }
+        }
+    }
+}
+
+impl TryFrom<&NiDkgTranscript> for ThresholdSigPublicKey {
+    type Error = ThresholdSigPublicKeyError;
+
+    fn try_from(ni_dkg_transcript: &NiDkgTranscript) -> Result<Self, Self::Error> {
         let csp_ni_dkg_transcript = CspNiDkgTranscript::from(ni_dkg_transcript);
-        let csp_threshold_sig_pubkey = CspThresholdSigPublicKey::from(&csp_ni_dkg_transcript);
-        ThresholdSigPublicKey::from(csp_threshold_sig_pubkey)
+        let csp_threshold_sig_pubkey = CspThresholdSigPublicKey::try_from(&csp_ni_dkg_transcript)?;
+        Ok(ThresholdSigPublicKey::from(csp_threshold_sig_pubkey))
     }
 }
 impl From<bls12_381::PublicKeyBytes> for ThresholdSigPublicKey {

rs/types/types/src/crypto/threshold_sig/ni_dkg.rs

@@ -1,7 +1,6 @@
 //! Types for non-interactive distributed key generation (NI-DKG).
 pub use crate::crypto::threshold_sig::ni_dkg::config::receivers::NiDkgReceivers;
 use crate::crypto::threshold_sig::ni_dkg::config::NiDkgThreshold;
-use crate::crypto::threshold_sig::ThresholdSigPublicKey;
 #[cfg(test)]
 use crate::NodeId;
 use crate::NumberOfNodes;
@@ -14,6 +13,7 @@ use serde::{Deserialize, Serialize};
 use std::collections::BTreeSet;
 use std::convert::TryFrom;
 use strum_macros::EnumIter;
+use thiserror::Error;
 
 pub mod config;
 pub mod errors;
@@ -143,6 +143,12 @@ impl From<NiDkgDealing> for CspNiDkgDealing {
     }
 }
 
+#[derive(Error, Debug)]
+pub enum ThresholdSigPublicKeyError {
+    #[error("threshold signature public key coefficients empty")]
+    CoefficientsEmpty,
+}
+
 /// Summarizes a distributed key generation.
 #[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]
 pub struct NiDkgTranscript {
@@ -250,13 +256,6 @@ impl NiDkgTranscript {
     }
 }
 
-impl NiDkgTranscript {
-    /// Computes the threshold-committee public key from the transcript.
-    pub fn public_key(&self) -> ThresholdSigPublicKey {
-        ThresholdSigPublicKey::from(self)
-    }
-}
-
 /// Converts an NI-DKG transcript into the corresponding protobuf
 /// representation.
 impl From<NiDkgTranscript> for InitialNiDkgTranscriptRecord {