Merge branch 'alex/do-not-deserialize-transcripts-in-idkg' into 'master'

perf(crypto): CRP-2225 do not redundantly deserialize dealings in IDKG

Improves the runtime of `verify_dealing_private` and `load_transcript_*` by 10%-20%, depending on the number of nodes. 

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

rs/crypto/internal/crypto_service_provider/src/api/canister_threshold.rs

@@ -12,7 +12,10 @@ use ic_types::crypto::canister_threshold_sig::error::{
     IDkgVerifyTranscriptError, ThresholdEcdsaCombineSigSharesError, ThresholdEcdsaSignShareError,
     ThresholdEcdsaVerifyCombinedSignatureError, ThresholdEcdsaVerifySigShareError,
 };
-use ic_types::crypto::canister_threshold_sig::{ExtendedDerivationPath, ThresholdEcdsaSigInputs};
+use ic_types::crypto::canister_threshold_sig::{
+    idkg::{BatchSignedIDkgDealing, IDkgDealingBytes},
+    ExtendedDerivationPath, ThresholdEcdsaSigInputs,
+};
 use ic_types::crypto::AlgorithmId;
 use ic_types::{NodeIndex, NumberOfNodes, Randomness, RegistryVersion};
 use std::collections::{BTreeMap, BTreeSet};
@@ -38,7 +41,7 @@ pub trait CspIDkgProtocol {
     fn idkg_verify_dealing_private(
         &self,
         algorithm_id: AlgorithmId,
-        dealing: &IDkgDealingInternal,
+        dealing: IDkgDealingBytes,
         dealer_index: NodeIndex,
         receiver_index: NodeIndex,
         receiver_public_key: &MEGaPublicKey,
@@ -79,7 +82,7 @@ pub trait CspIDkgProtocol {
     /// if necessary.
     fn idkg_load_transcript(
         &self,
-        dealings: &BTreeMap<NodeIndex, IDkgDealingInternal>,
+        dealings: &BTreeMap<NodeIndex, BatchSignedIDkgDealing>,
         context_data: &[u8],
         receiver_index: NodeIndex,
         public_key: &MEGaPublicKey,
@@ -90,7 +93,7 @@ pub trait CspIDkgProtocol {
     /// in the canister secret key store.
     fn idkg_load_transcript_with_openings(
         &self,
-        dealings: &BTreeMap<NodeIndex, IDkgDealingInternal>,
+        dealings: &BTreeMap<NodeIndex, BatchSignedIDkgDealing>,
         openings: &BTreeMap<NodeIndex, BTreeMap<NodeIndex, CommitmentOpening>>,
         context_data: &[u8],
         receiver_index: NodeIndex,

rs/crypto/internal/crypto_service_provider/src/canister_threshold/mod.rs

@@ -33,7 +33,10 @@ use ic_types::crypto::canister_threshold_sig::error::{
     IDkgVerifyTranscriptError, ThresholdEcdsaCombineSigSharesError, ThresholdEcdsaSignShareError,
     ThresholdEcdsaVerifyCombinedSignatureError, ThresholdEcdsaVerifySigShareError,
 };
-use ic_types::crypto::canister_threshold_sig::{ExtendedDerivationPath, ThresholdEcdsaSigInputs};
+use ic_types::crypto::canister_threshold_sig::{
+    idkg::{BatchSignedIDkgDealing, IDkgDealingBytes},
+    ExtendedDerivationPath, ThresholdEcdsaSigInputs,
+};
 use ic_types::crypto::AlgorithmId;
 use ic_types::{NodeIndex, NumberOfNodes, Randomness, RegistryVersion};
 
@@ -70,7 +73,7 @@ impl CspIDkgProtocol for Csp {
     fn idkg_verify_dealing_private(
         &self,
         algorithm_id: AlgorithmId,
-        dealing: &IDkgDealingInternal,
+        dealing: IDkgDealingBytes,
         dealer_index: NodeIndex,
         receiver_index: NodeIndex,
         receiver_public_key: &MEGaPublicKey,
@@ -157,7 +160,7 @@ impl CspIDkgProtocol for Csp {
 
     fn idkg_load_transcript(
         &self,
-        dealings: &BTreeMap<NodeIndex, IDkgDealingInternal>,
+        dealings: &BTreeMap<NodeIndex, BatchSignedIDkgDealing>,
         context_data: &[u8],
         receiver_index: NodeIndex,
         public_key: &MEGaPublicKey,
@@ -178,7 +181,7 @@ impl CspIDkgProtocol for Csp {
 
     fn idkg_load_transcript_with_openings(
         &self,
-        dealings: &BTreeMap<NodeIndex, IDkgDealingInternal>,
+        dealings: &BTreeMap<NodeIndex, BatchSignedIDkgDealing>,
         openings: &BTreeMap<NodeIndex, BTreeMap<NodeIndex, CommitmentOpening>>,
         context_data: &[u8],
         receiver_index: NodeIndex,

rs/crypto/internal/crypto_service_provider/src/vault/api.rs

@@ -22,7 +22,10 @@ use ic_types::crypto::canister_threshold_sig::error::{
     IDkgCreateDealingError, IDkgLoadTranscriptError, IDkgOpenTranscriptError, IDkgRetainKeysError,
     IDkgVerifyDealingPrivateError, ThresholdEcdsaSignShareError,
 };
-use ic_types::crypto::canister_threshold_sig::ExtendedDerivationPath;
+use ic_types::crypto::canister_threshold_sig::{
+    idkg::{BatchSignedIDkgDealing, IDkgDealingBytes},
+    ExtendedDerivationPath,
+};
 use ic_types::crypto::{AlgorithmId, CryptoError, CurrentNodePublicKeys};
 use ic_types::{NodeId, NodeIndex, NumberOfNodes, Randomness};
 use serde::{Deserialize, Serialize};
@@ -760,7 +763,7 @@ pub trait IDkgProtocolCspVault {
     fn idkg_verify_dealing_private(
         &self,
         algorithm_id: AlgorithmId,
-        dealing: &IDkgDealingInternal,
+        dealing: IDkgDealingBytes,
         dealer_index: NodeIndex,
         receiver_index: NodeIndex,
         receiver_key_id: KeyId,
@@ -771,7 +774,7 @@ pub trait IDkgProtocolCspVault {
     /// if necessary.
     fn idkg_load_transcript(
         &self,
-        dealings: &BTreeMap<NodeIndex, IDkgDealingInternal>,
+        dealings: &BTreeMap<NodeIndex, BatchSignedIDkgDealing>,
         context_data: &[u8],
         receiver_index: NodeIndex,
         key_id: &KeyId,
@@ -781,7 +784,7 @@ pub trait IDkgProtocolCspVault {
     /// See [`crate::api::CspIDkgProtocol::idkg_load_transcript_with_openings`].
     fn idkg_load_transcript_with_openings(
         &self,
-        dealings: &BTreeMap<NodeIndex, IDkgDealingInternal>,
+        dealings: &BTreeMap<NodeIndex, BatchSignedIDkgDealing>,
         openings: &BTreeMap<NodeIndex, BTreeMap<NodeIndex, CommitmentOpening>>,
         context_data: &[u8],
         receiver_index: NodeIndex,

rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/idkg/mod.rs

@@ -29,6 +29,7 @@ use ic_types::crypto::canister_threshold_sig::error::{
     IDkgCreateDealingError, IDkgLoadTranscriptError, IDkgOpenTranscriptError, IDkgRetainKeysError,
     IDkgVerifyDealingPrivateError,
 };
+use ic_types::crypto::canister_threshold_sig::idkg::{BatchSignedIDkgDealing, IDkgDealingBytes};
 use ic_types::crypto::AlgorithmId;
 use ic_types::{NodeIndex, NumberOfNodes};
 use parking_lot::{RwLockReadGuard, RwLockWriteGuard};
@@ -74,17 +75,23 @@ impl<R: Rng + CryptoRng, S: SecretKeyStore, C: SecretKeyStore, P: PublicKeyStore
     fn idkg_verify_dealing_private(
         &self,
         algorithm_id: AlgorithmId,
-        dealing: &IDkgDealingInternal,
+        dealing: IDkgDealingBytes,
         dealer_index: NodeIndex,
         receiver_index: NodeIndex,
         receiver_key_id: KeyId,
         context_data: &[u8],
     ) -> Result<(), IDkgVerifyDealingPrivateError> {
         debug!(self.logger; crypto.method_name => "idkg_verify_dealing_private");
         let start_time = self.metrics.now();
+        let internal_dealing = IDkgDealingInternal::deserialize(dealing.as_ref()).map_err(|e| {
+            IDkgVerifyDealingPrivateError::InvalidArgument(format!(
+                "failed to deserialize internal dealing: {:?}",
+                e
+            ))
+        })?;
         let result = self.idkg_verify_dealing_private_internal(
             algorithm_id,
-            dealing,
+            &internal_dealing,
             dealer_index,
             receiver_index,
             receiver_key_id,
@@ -102,19 +109,25 @@ impl<R: Rng + CryptoRng, S: SecretKeyStore, C: SecretKeyStore, P: PublicKeyStore
 
     fn idkg_load_transcript(
         &self,
-        dealings: &BTreeMap<NodeIndex, IDkgDealingInternal>,
+        dealings: &BTreeMap<NodeIndex, BatchSignedIDkgDealing>,
         context_data: &[u8],
         receiver_index: NodeIndex,
         key_id: &KeyId,
         transcript: IDkgTranscriptInternalBytes,
     ) -> Result<BTreeMap<NodeIndex, IDkgComplaintInternal>, IDkgLoadTranscriptError> {
         let start_time = self.metrics.now();
+        let internal_dealings =
+            idkg_internal_dealings_from_verified_dealings(dealings).map_err(|e| {
+                IDkgLoadTranscriptError::SerializationError {
+                    internal_error: format!("failed to deserialize internal dealing: {:?}", e),
+                }
+            })?;
         let internal_transcript = IDkgTranscriptInternal::deserialize(transcript.as_ref())
             .map_err(|e| IDkgLoadTranscriptError::SerializationError {
                 internal_error: e.0,
             })?;
         let result = self.idkg_load_transcript_internal(
-            dealings,
+            &internal_dealings,
             context_data,
             receiver_index,
             key_id,
@@ -132,20 +145,21 @@ impl<R: Rng + CryptoRng, S: SecretKeyStore, C: SecretKeyStore, P: PublicKeyStore
 
     fn idkg_load_transcript_with_openings(
         &self,
-        dealings: &BTreeMap<NodeIndex, IDkgDealingInternal>,
+        dealings: &BTreeMap<NodeIndex, BatchSignedIDkgDealing>,
         openings: &BTreeMap<NodeIndex, BTreeMap<NodeIndex, CommitmentOpening>>,
         context_data: &[u8],
         receiver_index: NodeIndex,
         key_id: &KeyId,
         transcript: IDkgTranscriptInternalBytes,
     ) -> Result<(), IDkgLoadTranscriptError> {
         let start_time = self.metrics.now();
+        let internal_dealings = idkg_internal_dealings_from_verified_dealings(dealings)?;
         let internal_transcript = IDkgTranscriptInternal::deserialize(transcript.as_ref())
             .map_err(|e| IDkgLoadTranscriptError::SerializationError {
                 internal_error: e.0,
             })?;
         let result = self.idkg_load_transcript_with_openings_internal(
-            dealings,
+            &internal_dealings,
             openings,
             context_data,
             receiver_index,
@@ -802,3 +816,19 @@ fn would_idkg_retain_modify_public_key_store<P: PublicKeyStore>(
             }
         })
 }
+
+fn idkg_internal_dealings_from_verified_dealings(
+    verified_dealings: &BTreeMap<NodeIndex, BatchSignedIDkgDealing>,
+) -> Result<BTreeMap<NodeIndex, IDkgDealingInternal>, IDkgLoadTranscriptError> {
+    verified_dealings
+        .iter()
+        .map(|(index, signed_dealing)| {
+            let dealing = IDkgDealingInternal::try_from(signed_dealing).map_err(|e| {
+                IDkgLoadTranscriptError::SerializationError {
+                    internal_error: format!("failed to deserialize internal dealing: {:?}", e),
+                }
+            })?;
+            Ok((*index, dealing))
+        })
+        .collect()
+}

rs/crypto/internal/crypto_service_provider/src/vault/remote_csp_vault/mod.rs

@@ -23,7 +23,10 @@ use ic_types::crypto::canister_threshold_sig::error::{
     IDkgCreateDealingError, IDkgLoadTranscriptError, IDkgOpenTranscriptError, IDkgRetainKeysError,
     IDkgVerifyDealingPrivateError, ThresholdEcdsaSignShareError,
 };
-use ic_types::crypto::canister_threshold_sig::ExtendedDerivationPath;
+use ic_types::crypto::canister_threshold_sig::{
+    idkg::{BatchSignedIDkgDealing, IDkgDealingBytes},
+    ExtendedDerivationPath,
+};
 use ic_types::crypto::{AlgorithmId, CurrentNodePublicKeys};
 use ic_types::{NodeId, NodeIndex, NumberOfNodes, Randomness};
 use std::collections::{BTreeMap, BTreeSet};
@@ -168,7 +171,7 @@ pub trait TarpcCspVault {
     // Corresponds to `IDkgProtocolCspVault.idkg_verify_dealing_private`
     async fn idkg_verify_dealing_private(
         algorithm_id: AlgorithmId,
-        dealing: IDkgDealingInternal,
+        dealing: IDkgDealingBytes,
         dealer_index: NodeIndex,
         receiver_index: NodeIndex,
         receiver_key_id: KeyId,
@@ -177,7 +180,7 @@ pub trait TarpcCspVault {
 
     // Corresponds to `IDkgProtocolCspVault.idkg_load_transcript`
     async fn idkg_load_transcript(
-        dealings: BTreeMap<NodeIndex, IDkgDealingInternal>,
+        dealings: BTreeMap<NodeIndex, BatchSignedIDkgDealing>,
         context_data: Vec<u8>,
         receiver_index: NodeIndex,
         key_id: KeyId,
@@ -187,7 +190,7 @@ pub trait TarpcCspVault {
     // Corresponds to `IDkgProtocolCspVault.idkg_load_transcript_with_openings`
     #[allow(clippy::too_many_arguments)]
     async fn idkg_load_transcript_with_openings(
-        dealings: BTreeMap<NodeIndex, IDkgDealingInternal>,
+        dealings: BTreeMap<NodeIndex, BatchSignedIDkgDealing>,
         openings: BTreeMap<NodeIndex, BTreeMap<NodeIndex, CommitmentOpening>>,
         context_data: Vec<u8>,
         receiver_index: NodeIndex,

rs/crypto/internal/crypto_service_provider/src/vault/remote_csp_vault/tarpc_csp_vault_client.rs

@@ -39,7 +39,10 @@ use ic_types::crypto::canister_threshold_sig::error::{
     IDkgCreateDealingError, IDkgLoadTranscriptError, IDkgOpenTranscriptError, IDkgRetainKeysError,
     IDkgVerifyDealingPrivateError, ThresholdEcdsaSignShareError,
 };
-use ic_types::crypto::canister_threshold_sig::ExtendedDerivationPath;
+use ic_types::crypto::canister_threshold_sig::{
+    idkg::{BatchSignedIDkgDealing, IDkgDealingBytes},
+    ExtendedDerivationPath,
+};
 use ic_types::crypto::{AlgorithmId, CurrentNodePublicKeys};
 use ic_types::{NodeId, NumberOfNodes, Randomness};
 use serde::{Deserialize, Serialize};
@@ -589,7 +592,7 @@ impl IDkgProtocolCspVault for RemoteCspVault {
     fn idkg_verify_dealing_private(
         &self,
         algorithm_id: AlgorithmId,
-        dealing: &IDkgDealingInternal,
+        dealing: IDkgDealingBytes,
         dealer_index: NodeIndex,
         receiver_index: NodeIndex,
         receiver_key_id: KeyId,
@@ -598,7 +601,7 @@ impl IDkgProtocolCspVault for RemoteCspVault {
         self.tokio_block_on(self.tarpc_csp_client.idkg_verify_dealing_private(
             context_with_timeout(self.rpc_timeout),
             algorithm_id,
-            dealing.clone(),
+            dealing,
             dealer_index,
             receiver_index,
             receiver_key_id,
@@ -613,7 +616,7 @@ impl IDkgProtocolCspVault for RemoteCspVault {
 
     fn idkg_load_transcript(
         &self,
-        dealings: &BTreeMap<NodeIndex, IDkgDealingInternal>,
+        dealings: &BTreeMap<NodeIndex, BatchSignedIDkgDealing>,
         context_data: &[u8],
         receiver_index: NodeIndex,
         key_id: &KeyId,
@@ -636,7 +639,7 @@ impl IDkgProtocolCspVault for RemoteCspVault {
 
     fn idkg_load_transcript_with_openings(
         &self,
-        dealings: &BTreeMap<NodeIndex, IDkgDealingInternal>,
+        dealings: &BTreeMap<NodeIndex, BatchSignedIDkgDealing>,
         openings: &BTreeMap<NodeIndex, BTreeMap<NodeIndex, CommitmentOpening>>,
         context_data: &[u8],
         receiver_index: NodeIndex,

rs/crypto/internal/crypto_service_provider/src/vault/remote_csp_vault/tarpc_csp_vault_server.rs

@@ -36,7 +36,10 @@ use ic_types::crypto::canister_threshold_sig::error::{
     IDkgCreateDealingError, IDkgLoadTranscriptError, IDkgOpenTranscriptError, IDkgRetainKeysError,
     IDkgVerifyDealingPrivateError, ThresholdEcdsaSignShareError,
 };
-use ic_types::crypto::canister_threshold_sig::ExtendedDerivationPath;
+use ic_types::crypto::canister_threshold_sig::{
+    idkg::{BatchSignedIDkgDealing, IDkgDealingBytes},
+    ExtendedDerivationPath,
+};
 use ic_types::crypto::{AlgorithmId, CurrentNodePublicKeys};
 use ic_types::{NodeId, NumberOfNodes, Randomness};
 use std::collections::{BTreeMap, BTreeSet};
@@ -352,7 +355,7 @@ impl<C: CspVault + 'static> TarpcCspVault for TarpcCspVaultServerWorker<C> {
         self,
         _: context::Context,
         algorithm_id: AlgorithmId,
-        dealing: IDkgDealingInternal,
+        dealing: IDkgDealingBytes,
         dealer_index: NodeIndex,
         receiver_index: NodeIndex,
         receiver_key_id: KeyId,
@@ -362,7 +365,7 @@ impl<C: CspVault + 'static> TarpcCspVault for TarpcCspVaultServerWorker<C> {
         let job = move || {
             vault.idkg_verify_dealing_private(
                 algorithm_id,
-                &dealing,
+                dealing,
                 dealer_index,
                 receiver_index,
                 receiver_key_id,
@@ -375,7 +378,7 @@ impl<C: CspVault + 'static> TarpcCspVault for TarpcCspVaultServerWorker<C> {
     async fn idkg_load_transcript(
         self,
         _: context::Context,
-        dealings: BTreeMap<NodeIndex, IDkgDealingInternal>,
+        dealings: BTreeMap<NodeIndex, BatchSignedIDkgDealing>,
         context_data: Vec<u8>,
         receiver_index: NodeIndex,
         key_id: KeyId,
@@ -397,7 +400,7 @@ impl<C: CspVault + 'static> TarpcCspVault for TarpcCspVaultServerWorker<C> {
     async fn idkg_load_transcript_with_openings(
         self,
         _: context::Context,
-        dealings: BTreeMap<NodeIndex, IDkgDealingInternal>,
+        dealings: BTreeMap<NodeIndex, BatchSignedIDkgDealing>,
         openings: BTreeMap<NodeIndex, BTreeMap<NodeIndex, CommitmentOpening>>,
         context_data: Vec<u8>,
         receiver_index: NodeIndex,

rs/crypto/src/sign/canister_threshold_sig/idkg/dealing.rs

@@ -111,14 +111,6 @@ pub fn verify_dealing_private<C: CspIDkgProtocol>(
             params.transcript_id(),
         )));
     }
-    let internal_dealing =
-        IDkgDealingInternal::deserialize(&signed_dealing.idkg_dealing().internal_dealing_raw)
-            .map_err(|e| {
-                IDkgVerifyDealingPrivateError::InvalidArgument(format!(
-                    "failed to deserialize internal dealing: {:?}",
-                    e
-                ))
-            })?;
     let dealer_index = params
         .dealer_index(signed_dealing.dealer_id())
         .ok_or_else(|| {
@@ -135,7 +127,7 @@ pub fn verify_dealing_private<C: CspIDkgProtocol>(
 
     csp_client.idkg_verify_dealing_private(
         params.algorithm_id(),
-        &internal_dealing,
+        signed_dealing.idkg_dealing().dealing_as_bytebuf(),
         dealer_index,
         self_receiver_index,
         &self_mega_pubkey,